diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 54fb1ebc7d..fd5ec6c9c5 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -9,7 +9,7 @@ { "name": "doperpowers", "description": "Emerges by humans, Converges by Agent", - "version": "7.50.1", + "version": "7.51.0", "source": "./", "author": { "name": "SSFSKIM", diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 03386549b4..efc8919b5a 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "doperpowers", "description": "Emerges by humans, Converges by Agents", - "version": "7.50.1", + "version": "7.51.0", "author": { "name": "SSFSKIM", "email": "supremekim17@gmail.com" diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 8c23f02a3c..d8354fd440 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "doperpowers", - "version": "7.50.1", + "version": "7.51.0", "description": "A two-track software-development methodology for coding agents: a human-gated controlled track (brainstorm, plan, TDD, review, ship) plus an autonomous board loop for unattended, well-scoped work.", "author": { "name": "SSFSKIM", diff --git a/docs/doperpowers/plans/2026-08-12-dp51-deferrals-dp60.md b/docs/doperpowers/plans/2026-08-12-dp51-deferrals-dp60.md new file mode 100644 index 0000000000..e03948e541 --- /dev/null +++ b/docs/doperpowers/plans/2026-08-12-dp51-deferrals-dp60.md @@ -0,0 +1,351 @@ +# dp#51 deferrals + dp#60 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use doperpowers:subagent-driven-development (recommended) or doperpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close the six items of spec `docs/doperpowers/specs/2026-08-12-dp51-deferrals-dp60-design.md` (v1.1): the dp#60 meta-truncation bug, the gh-path QAGENT answer return, typed successor-claim escalation, list-read truncation hardening + the arkho contract pin, the bootstrap parity fence, and drill assertion anchoring. + +**Architecture:** All client-side shell/python in `skills/issue-tracker/scripts/` and `skills/reviewing-prs/scripts/`, plus tests. No arkho code changes (one arkho ISSUE is filed). Items are independent; tasks are ordered so §1's helper lands before its consumer. + +**Tech Stack:** bash 3.2-compatible shell, python3 heredocs, the repo's fixture mock (`tests/claude-code/board-api/mock-server.py`), `t`/`nt` helpers. + +## Global Constraints + +- The spec is authoritative: `docs/doperpowers/specs/2026-08-12-dp51-deferrals-dp60-design.md` v1.1. Read your task's spec section before coding. +- Every new test assertion must be shown RED against the task's parent commit before the fix lands (stash the source change or render from `git show :`). +- bash 3.2: no `${var:+...}` with quotes inside heredoc-adjacent expansions, no associative arrays, herestrings for `grep -Fq` (pipefail SIGPIPE). +- Board tokens never in fixtures, logs, or committed files. +- Commits: conventional style, NO Co-Authored-By or attribution lines. +- Thin client invariant (`_board_api.py` header): client checks only cheap argv validation; the server owns legality. +- Mock fixture discipline: mock external responses from real observed shapes; nested error envelope is `{"error":{"code":"...","message":"..."}}`. + +--- + +### Task 1: §1 value grammar + `meta_match` helper in `_board.py` + +**Files:** +- Modify: `skills/issue-tracker/scripts/_board.py` (META_RE block, `parse_meta` `:238-251`, `strip_meta` `:254-257`, `render_body` `:273-281`) +- Test: `tests/issue-tracker/test-board-scripts.sh` + +**Interfaces:** +- Produces: `B.meta_match(body)` → rightmost `re.Match` of `META_RE` or `None`. `B.render_body(body, meta)` now normalizes `\r`/`\n` in values to a single space and calls `B.die(...)` when a value contains ``. + +- [ ] **Step 1: Write the failing tests.** In `tests/issue-tracker/test-board-scripts.sh`, add a `meta-grammar:` section (follow the file's existing section style — direct `_py` heredocs are fine where the file already does that). Cases: + +```bash +# (a) dp#60 reproduction: a body whose PROSE quotes a marker example keeps +# its prose across a meta write, and parse_meta reads the REAL block. +# Build the body in python to control bytes exactly: +# prose = "Docs about the block:\n\n \n\nMore prose." +# body = B.render_body(prose, {"pr": "https://real/1"}) +# assert B.parse_meta(body) == {"pr": "https://real/1"} +# assert "More prose." in B.strip_meta(body) +# assert B.strip_meta(body).rstrip() == prose.rstrip() +# (b) round-trip: B.render_body(B.strip_meta(body), B.parse_meta(body)) == body +# (c) contract_hash(body) == contract_hash of a body with DIFFERENT meta, +# same prose (hash covers prose only) +# (d) grammar: render_body({"note": "line1\nline2"}) renders "note: line1 line2" +# (e) grammar: render_body({"note": "x\n` (#60); every match ends at end-of-string + (`\\s*$`), so the rightmost START is the actual block. render_body's value + grammar (single-line, no marker tokens) is what makes this sound: the + block itself can never contain a marker.""" + m, pos = None, 0 + body = body or "" + while True: + nxt = META_RE.search(body, pos) + if not nxt: + return m + m, pos = nxt, nxt.start() + 1 +``` + +`parse_meta`: replace `m = META_RE.search(body or "")` with `m = meta_match(body)`. +`strip_meta`: + +```python +def strip_meta(body): + """The body WITHOUT its trailing board:meta block — the ticket's own text, + with the board's bookkeeping removed.""" + m = meta_match(body) + return ((body or "")[:m.start()] if m else (body or "")).rstrip("\n") +``` + +`render_body`: before building the block — + +```python + clean = {} + for k, v in meta.items(): + if not v: + continue + v = " ".join(str(v).splitlines()) # EVERY separator parse_meta's + # splitlines() would honor — + # \r\n alone leaves \v/\f/U+2028 + # etc. injectable as forged keys + if "" in v: + die("meta value %r cannot carry a board:meta marker token" % k) + clean[k] = v + meta = clean +``` + +(replacing the existing `meta = {k: v for k, v in meta.items() if v}` line). + +- [ ] **Step 4: Run the new section AND the full `tests/issue-tracker/test-board-scripts.sh`** — all green (the file has many existing meta round-trip pins; they must survive). + +- [ ] **Step 5: Commit** `fix(board): rightmost meta block + value grammar — gh meta writes stop truncating marker-quoting bodies (#60)`. + +### Task 2: §1 `board-body.sh` uses the shared helper + +**Files:** +- Modify: `skills/issue-tracker/scripts/board-body.sh:64-83` (the gh-half inline walk) +- Test: `tests/claude-code/board-api/test-edge-verbs.sh` (existing pins) + +**Interfaces:** +- Consumes: `B.meta_match` from Task 1. + +- [ ] **Step 1:** Replace the inline `while True` walk (`board-body.sh:73-79`) with `m = B.meta_match(old)`, keeping the splice line and the comment's first paragraph (trim the now-redundant leftmost-first explanation to a pointer: "the helper walks to the rightmost match — see _board.meta_match (#60)"). +- [ ] **Step 2:** Run `tests/claude-code/board-api/test-edge-verbs.sh` — the meta-splice pins ("keeps the TRAILING meta block byte-for-byte", "the quoted marker example did not survive as a splice point") stay green. +- [ ] **Step 3: Commit** `refactor(board-body): splice via the shared meta_match helper`. + +### Task 3: §2 QAGENT role — stamp, three-way arm, `--pr` re-supply + +**Files:** +- Modify: `skills/reviewing-prs/scripts/review-dispatch.sh` (`_spawn_reviewer`, after the `board-bind.sh` call at `:931`) +- Modify: `skills/issue-tracker/scripts/board-answer.sh` (`:185-209`) +- Test: `tests/issue-tracker/test-board-scripts.sh` (Finding-D section `:957-999`), `tests/reviewing-prs/test-review-dispatch.sh` + +**Interfaces:** +- Consumes: registry meta JSON has top-level `"name"` (verified); `implement-dispatch.sh:848-870` is the stamp pattern to mirror. +- Produces: gh-spawned reviewer metas carry `role: QAGENT`; `board-answer.sh` returns qagent parks to `in-review`. + +- [ ] **Step 1: Failing tests (board-answer side).** In `test-board-scripts.sh` after the existing Finding-D cases: + +```bash +# QAGENT with pr: meta → in-review, no --pr from the caller +# (fixture meta: role: QAGENT, no pre-park; ticket meta carries pr: https://…) +# → assert status:in-review +# QAGENT with NO pr: meta → in-progress + warning +# → assert status:in-progress AND output contains "no pr: meta" +# legacy meta with NO role but registry name review-pr- → in-review +# (name-inference rung) +``` + +Also amend the `:987` assertion text: "an unrecorded pre-park with an IMPLEMENT role falls back on in-progress". + +- [ ] **Step 2: Run — all three FAIL** (current code returns in-progress everywhere). + +- [ ] **Step 3: Implement board-answer.sh.** Replace `:199`'s single line with: + +```python + role = (meta.get("role") or "").upper() + if not role and str(meta.get("name") or "").startswith(("review-pr-", "review-epic-")): + role = "QAGENT" # pre-stamp reviewers: the deterministic worker + # name is the only role record they carry + if role == "ARCHITECT": + ret = "in-design" + elif role == "QAGENT": + ret = "in-review" + else: + ret = "in-progress" +``` + +CHECK FIRST how `meta` is loaded (`board-answer.sh:104-160`): if the dict is the registry-file JSON, `name` is already a key; if it is a sub-object, read the name from the enclosing record and thread it through the same tab-separated line (`:200-201`) — extend that line rather than re-reading files. + +**The pr value must CROSS the python→shell boundary explicitly** — the +current output line (`:200-201`) carries five fields +(`uuid engine status updated ret`) and the shell `read` at `:204` names +exactly those; `set -u` makes an unread `$pr` a hard abort. So: + +- In the python: when the role arm selects `in-review`, read + `pr = B.parse_meta(tickets[tid]["body"]).get("pr")` (the body is + already fetched for pre-park). If absent, demote `ret` to + `in-progress`, set `pr = ""`, and print the warning + `relay: # — QAGENT return wants in-review but the ticket has no pr: meta; falling back to in-progress` + to stderr. Emit `pr` as a SIXTH tab-separated field (empty when not + in-review). +- In the shell: extend the `read` to + `IFS=$'\t' read -r uuid engine status updated ret pr`, and build the + transition argv conditionally: + +```bash +if [ "$ret" = in-review ]; then + "$SCRIPT_DIR/board-transition.sh" "$tid" "$ret" \ + "answers relayed — resuming bound session ${uuid:0:8}" --pr "$pr" +else + "$SCRIPT_DIR/board-transition.sh" "$tid" "$ret" \ + "answers relayed — resuming bound session ${uuid:0:8}" +fi +``` + +(`ret == in-review` implies non-empty `$pr` by the python demotion, so +the `--pr` arm never passes an empty value. Verify board-transition.sh's +flag parsing accepts `--pr` in that position — mirror an existing caller +if it must precede the note.) The test asserts the ACTUAL transition +argv (capture via the test's board-transition stub or the state file), +not only the final status. + +- [ ] **Step 4: Run the section — green.** Run the whole `test-board-scripts.sh`. + +- [ ] **Step 5: Failing test (stamp side).** In `tests/reviewing-prs/test-review-dispatch.sh`, at the existing bound-meta assertion (`:418` area), add: the spawned reviewer's registry meta carries `"role": "QAGENT"`. Verify RED (nothing stamps it), then implement in `_spawn_reviewer` after `board-bind.sh` succeeds (`review-dispatch.sh:931`), mirroring `implement-dispatch.sh:848-870`: + +```bash + # Persist role: QAGENT into the registry meta — board-answer.sh's + # needs-human fallback reads it to return a qagent park to in-review + # instead of in-progress. Non-fatal, same shape as implement-dispatch's + # gh-side role write; pre-stamp metas are covered by the name-inference + # rung in board-answer.sh. + T_UUID="$uuid" DAEMON_HOME="$DAEMON_HOME" python3 - <<'PY' \ + || echo "$name: role meta write failed (non-fatal)" >&2 + ... (read-modify-write-under-lock; m["role"] = "QAGENT") +PY +``` + +Copy the lock discipline from `implement-dispatch.sh:848-870` exactly. + +- [ ] **Step 6: Run `tests/reviewing-prs/test-review-dispatch.sh` — green. Commit** `fix(board-answer): qagent parks return to in-review — role stamp, name inference, pr re-supply`. + +### Task 4: §3 typed claim errors + counting + +**Files:** +- Modify: `skills/issue-tracker/scripts/_board_api.py` (`claim_successor` `:145`, near `RunEnded` `:24`) +- Modify: `skills/issue-tracker/scripts/_sweep_api.sh` (`_resume_one` exits `:1157-1167`, `_attempts` `:1393-1412`, `_escalate` body text) +- Test: `tests/claude-code/board-api/test-sweep-resume.sh` + +**Interfaces:** +- Consumes: arkho answers 409 `{"error":{"code":"nonce-consumed",...}}` and `{"error":{"code":"stale-resume",...}}` (verified in claims.js). +- Produces: `claim_successor` distinguishable outcomes; `_attempts fail` callable with no run argument. + +- [ ] **Step 1: Failing tests.** Four new fixtures/scenarios in `test-sweep-resume.sh` (follow its `"once":true` fixture idiom, `:65-94`): + +```bash +# (a) claim-successor → 500 {"error":{"code":"internal","message":"boom"}} +# → t "a claim ERROR charges a recovery cycle" "recovery cycle 1 of 3" +# t "and the journal is kept as the replay handle" (journal file exists) +# (b) claim-successor → 409 {"error":{"code":"nonce-consumed",...}} +# → nt (no "recovery cycle"); journal file REMOVED; sweep exits 0 +# (c) claim-successor → 409 {"error":{"code":"stale-resume",...}} +# → same as (b) +# (d) claim-successor → 200 {"claimed":false} +# → nt (no "recovery cycle"); journal removed; exit 0 (pins the +# existing behavior the spec keeps) +``` + +- [ ] **Step 2: Verify (a) FAILS** (no cycle charged today) and (b)/(c) fail on the journal assertion (today both die and keep it). (d) may already pass — keep it as the guard pin. + +- [ ] **Step 3: Implement client typing.** In `_board_api.py`, mirror the `RunEnded` pattern: a `ClaimObsolete(Exception)` carrying `.code`, raised by `claim_successor` when the 409 envelope's `error.code` is `nonce-consumed` or `stale-resume`. Do it inside `claim_successor` (catch the die path — if `request()` structure makes that awkward, add an `on_codes={...}` hook to `request()` in the same style `run-ended` is special-cased; keep the change minimal and local). + +- [ ] **Step 4: Implement sweep routing.** In `_resume_one`'s claim block, the python that calls `A.claim_successor` prints a typed sentinel for the two obsolete codes (e.g. `OBSOLETE ` on stdout) instead of dying; the shell exit `:1157-1161` becomes: + +```bash +# obsolete journal (nonce-consumed / stale-resume): the JOURNAL is done, +# not the substrate — drop it uncharged; the feed re-serves the ticket +# with a fresh nonce (nonce-consumed) or its new state governs (stale-resume). +# Any OTHER claim error is a fault: charge the cycle, keep the journal. +``` + +with three arms: obsolete → `rm -f "$CLAIMS_DIR/$nonce.json"`, log, `return 0`; error → `_attempts "$tid" fail`, journal kept, `return 1`; granted → continue. `_attempts` (`:1393`): guard the run-release block (`:1403-1412`) with `[ -n "${2:-}" ]` — wait, signature is `_attempts fail `; make the third argument optional and skip `A.end_run` when absent. + +- [ ] **Step 5: `_escalate` wording.** Body text: "Three recovery cycles failed for ticket #%s (successor claim, resume, or fresh spawn)…" — title unchanged (dedupe key). + +- [ ] **Step 6: Run the full `test-sweep-resume.sh` — green** (the existing cycle-1/2/3 pins at `:464-501` must survive the `_attempts` signature change). **Commit** `fix(sweep): type the successor-claim failures — obsolete journals drop uncharged, faults count (#51)`. + +### Task 5: §3 one attempt per ticket per tick + reconcile honors suppression + +**Files:** +- Modify: `skills/issue-tracker/scripts/_sweep_api.sh` (`_reconcile_successors` `:971-1038`, `phase_resume` `:1505-1535`) +- Test: `tests/claude-code/board-api/test-sweep-resume.sh` + +- [ ] **Step 1: Failing tests.** (a) one ticket with a kept journal (replay) AND on the needing-resume feed in the same tick → count claim POSTs in the mock log, assert exactly 1 (today: 2). (b) a suppressed ticket with a standing journal → reconcile makes NO claim POST and the journal file survives. (c) **lift-this-tick interaction:** a suppression that LIFTS this tick (its `/tickets` fixture shows the env-issue `done`) + a standing journal for the same ticket + the ticket on the feed → exactly ONE claim POST, and it carries the JOURNAL's nonce (the replay), not a fresh one; no second journal file exists after the tick. +- [ ] **Step 2: Implement — ordering is the fix.** `phase_resume` currently runs `_reconcile_successors` FIRST (`:1511`), then the lift loop, then the feed. With a suppression-aware reconcile that order strands journals: reconcile skips the still-suppressed ticket, the lift then removes the suppression, and the feed claims a FRESH nonce — the old journal survives to replay beside the new successor on a later tick. Reorder to **lift → reconcile → feed**: + - Move the lift loop (`:1512-1518`) ABOVE `_reconcile_successors`. The existing "Unfinished successor claims first" comment reasons about reconcile-before-FEED, which the new order preserves; amend the comment to say lift runs first so a just-lifted ticket replays its own standing journal instead of minting a fresh nonce. + - `_reconcile_successors`: skip (journal untouched) when `_suppressed "$tid"`; when it DOES act on a ticket (replay/settle/orphan — any arm that consumes the journal or claims), append the tid to a tick ledger file. The ledger must exist BEFORE reconcile runs and be readable in `phase_resume`'s feed loop — reconcile is called from `phase_resume`, so create it there (`ledger="$(mktemp "$SCRATCH/resumed.XXXXXX")"`) and pass/export it; check how `_reconcile_successors` receives scope today (global vs args) and follow that shape. + - Feed loop: skip tids in the ledger (same style as the `_suppressed` skip), logging `resume: #$tid — already replayed this tick`. +- [ ] **Step 3: Green + full file. Commit** `fix(sweep): lift before reconcile; one recovery attempt per ticket per tick; reconcile honors suppression`. + +### Task 6: §4 `_check_lift` absent-row guard + +**Files:** +- Modify: `skills/issue-tracker/scripts/_sweep_api.sh:744-761` (`_check_lift`) +- Test: `tests/claude-code/board-api/test-sweep-resume.sh` + +- [ ] **Step 1: Failing test.** A suppression record for a ticket that is MISSING from the `/tickets` fixture → the suppression record file survives the tick (today: both lift conditions fire and it is removed). +- [ ] **Step 2: Implement.** In the `_check_lift` python (`:753-755`): absent rows are UNKNOWN, not moved/closed — + +```python +cur = rows.get(str(rec["ticket"])) +moved = cur is not None and cur != rec["state"] +env = rows.get(str(rec["env_issue"])) +closed = env in ("done", "wontfix") # absent env-issue: unknown, keep waiting +``` + +Add the mirror of the write-site comment (`:1438`): "AN ABSENT ROW IS NOT A STATE — a truncated or partial /tickets read must never lift a suppression." +- [ ] **Step 3: Green + full file. Commit** `fix(sweep): an absent /tickets row never lifts a suppression`. + +### Task 7: §4 arkho contract issue + +**Files:** none (outward action). + +- [ ] **Step 1:** `gh issue create -R SSFSKIM/arkho-a1-board-service` — title: `contract pin (A2): /queue/decisions and /tickets are read whole — unbounded until a paged envelope exists`. Body: the client reads both routes whole (`_board_api.py queue_decisions/tickets`); the destructive-consumer example (`_check_lift`, now guarded client-side); quote `API.md:383-384`/`:900-903` ("A2/A3 contract territory") as the invitation; ask that (1) any future cap on these two routes arrive WITH a response envelope (cursor/total) so truncation is detectable, (2) the existing 500-caps on `/answers/unrelayed` and `/runs/needing-resume` be added to API.md's "Boundary bounds" table. Reference doperpowers#51. +- [ ] **Step 2:** Record the issue URL in the SDD ledger and in the spec's `## Outcomes & Retrospective` material notes. + +### Task 8: §5 renderer fails closed + call-site binding assertions + +**Files:** +- Modify: `skills/reviewing-prs/scripts/review-dispatch.sh:848-868` (`_render_prompt`) +- Test: `tests/reviewing-prs/test-review-dispatch.sh`, `tests/claude-code/board-api/test-review-dispatch-claim.sh` + +- [ ] **Step 1:** Enumerate every `{{NAME}}` in `review-worker-bootstrap.md` per mode (after mode-strip) and diff against the `P_*` sets at the four call sites (`:625`, `:776`, `:1512`). Fix any call site that omits a placeholder its mode renders (expect none — but this step is the proof). +- [ ] **Step 2: Failing test.** In `test-review-dispatch.sh`: a render driven with one `P_*` deliberately unset → dispatcher exits nonzero naming the placeholder (drive `_render_prompt` the way the suite drives dispatch; if only reachable through a full dispatch, assert the dispatch fails loudly). Verify RED (today it renders blank and succeeds). +- [ ] **Step 3: Implement.** Port `implement-dispatch.sh:109-114`'s unresolved-placeholder check into `_render_prompt` (`review-dispatch.sh:866`): substitute known, collect unknown, print `unrendered placeholders: ` to stderr and exit 1. +- [ ] **Step 4:** Add captured-prompt content assertions in both suites: gh (`test-review-dispatch.sh`, prompt file from the stub `:78`) asserts `BIND_READY_FILE`/`SKILL_FILE`/`IMPLEMENT_PROTOCOL_FILE`/`BOARD_SCRIPTS` lines are non-empty; api (`test-review-dispatch-claim.sh`, `prompt()` `:235`) additionally `TICKET_BODY_FILE`. Non-empty = the binding line has a value after the colon (anchor on the rendered line shape). +- [ ] **Step 5: Both suites green. Commit** `fix(review-dispatch): unresolved bootstrap placeholders fail the render; suites pin critical bindings non-empty`. + +### Task 9: §5 static parity fence + +**Files:** +- Create: `tests/reviewing-prs/test-bootstrap-parity.sh` +- Modify: `tests/reviewing-prs/` runner registration (see how sibling tests are invoked — `tests/claude-code/run-skill-tests.sh` and/or a local runner) + +- [ ] **Step 1:** Write the fence per spec §5 piece 2, all assertions in one new file (use `t`/`nt`-style local helpers or the `assert_contains` idiom from `test-skill-entrypoint.sh:17-45`): + - honesty pins: `BOOTSTRAP_TEMPLATE=` path literal in `review-dispatch.sh`; mode-fence regex literal; `{{(\w+)}}` substitution literal; + - render all four modes via a local ~20-line python renderer over the real template with a complete fixture (every placeholder set to `X-` so emptiness is impossible and value-tracing trivial); + - load-bearing sentences in every mode's render: the skill-pin sentence, the precedence clause, the worktree caveat, and each mode's read-it-live rewording pinned `t` in its own render and `nt` in the other pair-member's; + - roster relation (gh ⊂ api modulo the pinned four; api adds exactly its pinned extras); + - block-boundary tail check (strip mode-owned + binding lines; remainder identical within each pair); + - no `mode:` fence and no `{{` in any render; + - implement lane: `worker-bootstrap.md` with/without `api-only`, outside-region identity, roster `+TICKET_BODY_FILE +PARENT_PIN`. +- [ ] **Step 2: Prove the fence bites:** scratch-edit one shared-tail word inside a single mode block copy (temporary file), rerun, watch it fail; discard the scratch. Also temporarily gate a shared binding line into one mode fence in a template COPY and watch the boundary check fail. +- [ ] **Step 3: Register in the runner, run, green. Commit** `test(reviewing-prs): bootstrap parity fence — four modes, pinned sentences, roster and boundary checks`. + +### Task 10: §6 drill cosmetics + +**Files:** +- Modify: `tests/claude-code/board-api/integration/test-transcript-diff.sh` (`:151`, `:214`, `:100-115`), `test-protocol-walk.sh` (`:73-74,79,96-97`), `test-crash-boundaries.sh` (`:76,86,122,135,154`), `test-resume-first.sh` (`:95-96,103`), `test-escalation.sh` (`:76,107,114`), `test-human-verbs.sh` (`:162,175`), `transcript-compare.py` (`:103`) + +- [ ] **Step 1:** Apply the spec §6 list: delimit `owner_line()`/`row()` scalar output (bracket or trailing `.`), anchor every listed assertion to the closed form; JSON ids closed with `,`/`}`; the `grep -q` regex at `test-escalation.sh:76` anchored `[,}]`; `:151` → non-empty + exact match; `:214` → `#4242` phrase; record `%T`-unsubstituted argv alongside executed argv in the walk capture and compare THAT in `transcript-compare.py:103`. +- [ ] **Step 2:** These are tightenings — every touched drill must still PASS as-is (unit-runnable parts; the integration drills need `ARKHO_DIR` + docker — run what the environment allows, and say exactly which drills ran). +- [ ] **Step 3:** Discrimination probe (not committed): in a scratch copy, offset one ticket id (e.g. make the walk register twice so ids differ) and confirm the anchored assertions now FAIL where the old substrings passed; note the probe result in the task report. +- [ ] **Step 4: Commit** `test(drills): anchor id assertions; compare unsubstituted argv (#51 cosmetics)`. + +### Task 11: Final verification + +- [ ] **Step 1:** Full suites: `tests/issue-tracker/test-board-scripts.sh`; every `tests/claude-code/board-api/test-*.sh`; `tests/reviewing-prs/test-review-dispatch.sh`, `test-skill-entrypoint.sh`, `test-bootstrap-parity.sh`; `tests/claude-code/board-api/integration/` drills if `ARKHO_DIR`+docker available (exit 77 = SKIP is acceptable, say so); `scripts/lint-shell.sh`. +- [ ] **Step 2:** Execute the spec's `## Acceptance` items as written (the marker-quoting body write; the qagent park answer walk; the 500-fixture escalation; the missing-row suppression survival; the parity-fence scratch-edit bite check). +- [ ] **Step 3:** Report each acceptance item's actual command + output in the task report. diff --git a/docs/doperpowers/specs/2026-08-12-dp51-deferrals-dp60-design.md b/docs/doperpowers/specs/2026-08-12-dp51-deferrals-dp60-design.md new file mode 100644 index 0000000000..14a6ca43da --- /dev/null +++ b/docs/doperpowers/specs/2026-08-12-dp51-deferrals-dp60-design.md @@ -0,0 +1,698 @@ +# dp#51 deferrals + dp#60 — design + +**Purpose.** Close out the tail of the A1/A2 board program: the five +non-consumption deferrals recorded on dp#51 after PR #61 shipped, plus +dp#60, the live gh-mode meta-truncation bug found during that work. Six +bounded items, each independently shippable; together they remove every +known correctness hole in the client toolkit short of the arkho-side +contract work they pin. + +All six were investigated against the code before this spec was written; +every load-bearing claim below carries its file:line. Base: +`dp51-deferrals-dp60` branched from main `d8427d73` (v7.48.0). + +--- + +## §1 dp#60 — gh-mode meta writes truncate marker-quoting bodies + +**The bug.** `_board.META_RE` (`_board.py:189`) is +`\n?\s*$` with `re.S`. `re.search` is +leftmost-first and the lazy `.*?` spans from the FIRST marker occurrence +to the trailing `-->` — so on a body whose PROSE quotes a +``; the rightmost start is the actual block (#60).""" + m, pos = None, 0 + body = body or "" + while True: + nxt = META_RE.search(body, pos) + if not nxt: + return m + m, pos = nxt, nxt.start() + 1 +``` + +- `parse_meta`: `m = meta_match(body)` in place of `META_RE.search`. +- `strip_meta`: `m = meta_match(body)`; return + `(body[:m.start()] if m else body or "").rstrip("\n")` — byte-offset + slicing, no `.sub`. (`m.end() == len(body)` because `\s*$` consumes to + the end, so slicing at `m.start()` is the whole removal.) +- `render_body`, `contract_hash`, `update_meta`: unchanged — they inherit + correctness through `strip_meta`/`parse_meta`. +- `board-body.sh` gh half: replace its inline walk (`:73-79`) with + `B.meta_match(old)` — same behavior, one author. The raw-splice + property is untouched: the helper returns a match object; the splice + keeps consuming `old[m.start():]` verbatim. + +**Behavior preserved:** on bodies with zero or one marker the helper +finds the same match `META_RE.search` found; only multi-occurrence +bodies change, and for those the old answer was the bug. + +**Value grammar enforced (adversarial-review finding, reproduced).** +Rightmost matching is only correct when the real trailing block cannot +itself CONTAIN a marker — and today it can: `render_body` +(`_board.py:280`) writes values verbatim, so a `note` value carrying +`"\n` is NOT rejected (v1.2.1, task-review finding, +fuzz-proven): after the splitlines collapse every value sits behind its +`key: ` prefix, so a `-->` can never reach line start where `META_RE` +requires it — rejecting it would brick every pre-fix ticket whose +stored note carries an arrow (update_meta re-renders every parsed key +on every write) and was the only realistic trigger of a TORN WRITE in +`apply_state` (label already moved, meta write then dies). The +surviving marker check must run BEFORE any external write in +`apply_state`'s callers reaches GitHub — validate, then write. With +the grammar enforced, the rightmost match is the real block by +construction. + +**Tests (RED against the parent commit):** a body whose prose quotes a +full marker-shaped example AND carries a real trailing block — +`parse_meta` returns the real block's keys; `strip_meta` keeps the prose +including the quoted example; an `update_meta` round-trip preserves the +prose byte-for-byte outside the block; `contract_hash` of that body +equals the hash of the prose. Home: `tests/issue-tracker/test-board-scripts.sh` +(the gh-mode pin) — plus the existing board-body drill keeps passing. + +--- + +## §2 gh-path board-answer QAGENT role fallback + +**The gap (three parts, all verified).** + +1. `board-answer.sh:199` — the whole gh-leg role resolution: + `ret = "in-design" if role == "ARCHITECT" else "in-progress"`. No + QAGENT arm: even a stamped `role: QAGENT` returns `in-progress`. +2. gh-mode `review-dispatch.sh` never stamps `role` at all — + `_spawn_reviewer` writes no role/lane (contrast: the api claim path + stamps both at `review-dispatch.sh:1561`; gh `implement-dispatch.sh` + stamps role at `:848-870`). So the meta is doubly silent. +3. Even with both fixed, `board-transition.sh:238-257` refuses + `in-review` without `--pr` unless `pre-park == "in-review"` — and the + fallback fires precisely when there is no recorded pre-park. + +Live surface: parks that reach `needs-human` from outside `PRE_PARK` +(`_board.py:60-66`) — for a qagent, realistically +`in-review → needs-info → needs-human`. Damage when it fires: the ticket +lands at `in-progress`, the review sweep's stale-reviewer arm +(`review-dispatch.sh:1115-1160`) sees an off-review ticket and retires +the resumed qagent's meta out from under it. + +**The fix — three small, matching pieces.** + +1. **Stamp the role at gh reviewer spawn.** In `_spawn_reviewer` + (`review-dispatch.sh:876`, after the `board-bind.sh` call at `:931`), + write `role: QAGENT` into the registry meta — same non-fatal + read-modify-write-under-lock shape as `implement-dispatch.sh:848-870` + (role only; `lane` stays an api-claim concept). Applies to both + `review-pr-*` and `review-epic-*` spawns — an epic scale reviewer is + the same protocol. +2. **Add the QAGENT arm, with a legacy rung.** `board-answer.sh:199` + becomes a three-way: `ARCHITECT → in-design`, `QAGENT → in-review`, + else `in-progress` — where the role is resolved in two rungs: the + `role:` meta first; when absent, infer QAGENT from the registry + record's deterministic worker `name` (`review-pr-*` / + `review-epic-*`; the registry JSON carries `"name"` at top level, + verified live). The inference rung covers every reviewer parked + before this version deploys and every spawn whose non-fatal stamp + write failed — without it the fix is upgrade-gated + (adversarial-review finding). +3. **Re-supply `--pr` from the ticket's own meta.** When the QAGENT arm + selects `in-review`, `board-answer.sh` reads the ticket's `pr:` meta + (`parse_meta(tickets[tid]["body"]).get("pr")` — it already parses + this body for `pre-park`) and passes it as `--pr` to + `board-transition.sh`. The gate at `board-transition.sh:238` is then + satisfied on its own terms — no gate relaxation. A ticket that entered + `in-review` via the gh path necessarily carries `pr:` (the `--pr` + write stamps it); if it is nonetheless absent, keep the prior + `in-progress` fallback and print a one-line warning naming the missing + meta — fail-open to the old behavior, never a hard death on the + answer path. + +**Out of scope (recorded, not built):** the adjacent wrong-pre-park case +— a qagent that bounces `in-review → ready-for-architect` and then parks +records `pre-park: in-design`, which the role fallback never reaches. +That is a PRE_PARK design question, not a fallback bug; noted for the +issue. + +**Tests (RED first):** in `tests/issue-tracker/test-board-scripts.sh`'s +Finding-D section (`:957-999`): a `role: QAGENT` meta with no pre-park +and a `pr:` meta → `status:in-review` without the caller supplying +`--pr`; a `role: QAGENT` meta with no `pr:` meta → `status:in-progress` +plus the warning line; amend the `:987` wording ("a non-architect role +falls back on in-progress") to name IMPLEMENT. In +`tests/reviewing-prs/test-review-dispatch.sh`: the spawned reviewer's +registry meta carries `"role": "QAGENT"` (today nothing asserts any role +there). + +--- + +## §3 successor-claim escalation + +**The gap (verified).** `_resume_one` (`_sweep_api.sh:1084`) charges the +existing per-ticket counter (`_attempts`, `:1393`; +`$SUPPRESS_DIR/.attempts-`) only at four post-grant exits +(`:1186,1248,1300,1325`). Both claim-failure exits bypass it: + +- (a) claim errored (`:1157-1161`) — `return 1`, journal deliberately + kept (the claim may have landed); +- (b) `claimed:false` (`:1163-1167`) — `return 0`, journal removed. + +A ticket whose claim persistently errors churns forever, and worse: the +kept journal is re-classified `replay` by `_reconcile_successors` +(`:971`) next tick while the feed ALSO re-serves the same ticket with a +fresh nonce — two claims and one leaked journal file per tick. + +**Rulings.** + +1. **Type the claim errors before counting (adversarial-review + finding, both codes verified in arkho source).** Not every claim + error is a substrate fault — arkho answers two typed 409s that mean + "this JOURNAL is obsolete", not "the substrate is sick": + - `nonce-consumed` (`claims.js` nonceReplay: "a nonce on an ended + run is spent, not replayable") — the predecessor's run ended; + replaying this nonce is doomed forever. Action: DROP the journal + (`rm` it), no cycle charged, `return 0` — the feed re-serves the + ticket with a fresh nonce next tick. + - `stale-resume` (`claims.js:393`) — the ticket moved after the + feed read. Action: DROP the journal, no cycle charged, `return 0` + — the ticket's new state governs; ordinary dispatch handles it. + Everything else on the error exit — transport death after retries, + 5xx, malformed grant (missing runId/fence/bearer), untyped + refusals — IS a fault: charge `_attempts "$tid" fail` (no run + argument; there is nothing to release) and KEEP the journal as + today. Client plumbing: `_board_api.py` already routes one typed 409 + (`RunEnded`, `:24,:104,:124`); extend the same pattern so + `claim_successor` surfaces `nonce-consumed` and `stale-resume` to + the sweep distinguishably. + Path (b) `claimed:false` stays uncounted: it is the server's + backpressure (lane cap, eligibility), a healthy wait state. + Escalating it would write a suppression whose documented effect + (`:744-761`, `:1535`, `:1572`) is to remove a healthy ticket from + BOTH the resume and dispatch phases until a human closes an + env-issue — strictly worse than the churn. With (b) uncounted, no + reset-on-grant is needed: the counter keeps its existing reset at + full delivery (`:1343`), and every counted event is a real fault. +2. **One attempt per ticket per tick.** `_reconcile_successors` runs + before `phase_resume` (`:1509`); when it replays a nonce for ticket + T, `phase_resume` must skip T this tick (record replayed tids in a + tick-scoped temp file, consult it in the feed loop — same shape as + the suppression skip at `:1535`). This kills both the double-charge + and the journal-per-tick leak in one move. +3. **Reconcile honors suppression — and lift runs FIRST.** + `_reconcile_successors` currently replays through a suppression (it + runs before the skip). It gains the same `_suppressed` check + `phase_resume` has: a suppressed ticket's journal is left in place, + untouched, until the suppression lifts. The phase order moves to + lift → reconcile → feed (today reconcile runs first, + `_sweep_api.sh:1511`): with suppression-aware reconcile in the OLD + order, a suppression lifting mid-tick would skip the journal in + reconcile, lift, and then claim a FRESH nonce from the feed — + stranding the old journal to replay beside the new successor later + (plan review finding). Lift-first lets a just-lifted ticket replay + its own standing journal. +4. **Generalize the escalation wording.** `_escalate`'s env-issue title + stays (deterministic-title dedupe depends on it); the body's "Three + resume and fresh-spawn cycles failed" becomes "Three recovery cycles + failed (successor claim, resume, or fresh spawn)" — true for every + counted cause. + +**`_attempts fail` without a run:** the current implementation releases +the undeliverable run (`:1403-1412`); with no second argument it must +skip the release cleanly. Guard, don't refactor. + +**Tests (RED first), in `test-sweep-resume.sh`:** a `claim-successor` +fixture answering 500 → "recovery cycle 1 of 3" printed, journal KEPT; +a 409 `nonce-consumed` fixture → journal REMOVED, no cycle, exit 0; a +409 `stale-resume` fixture → journal REMOVED, no cycle, exit 0; a +`claimed:false` fixture → no cycle charged, journal removed, exit 0 +(the untested exits at `:1160`/`:1166` get their first fixtures, and +both typed 409s get one each); a replayed nonce + the same ticket on +the feed in one tick → exactly one claim POST on the wire; a suppressed +ticket with a standing journal → reconcile leaves it alone, no claim +POST. + +--- + +## §4 list-read truncation & the pagination contract + +**Headline correction (verified):** the deferral's premise was inverted. +`_sweep_api.sh` never reads `/queue/*`; the capped endpoints +(`limit 500`) are `/answers/unrelayed` (drained correctly in a loop, +`:604-608`) and `/runs/needing-resume` (single level-triggered read per +tick — safe; backlog >500 is unreachable short of a multi-day sweep +outage). The endpoints the client reads whole — `/queue/decisions` +(`server.js:404-424`) and `/tickets` (`tickets.js:299-307`) — have NO +server cap today, return bare arrays with no envelope, and arkho's +API.md defers real paging to "A2/A3 contract territory" twice +(`API.md:383-384`, `:900-903`). + +**Rulings.** + +1. **No cursor-follow is built.** There is no cursor on the wire to + follow; building against an imagined contract is speculation. The + timeline `cursor` field (`timeline.js:64`) is the designated growth + seam when arkho takes it up. +2. **Pin the contract outward.** File an arkho issue stating the + contract A2 relies on: `/queue/decisions` and `/tickets` are read + whole and MUST stay unbounded until a paged read exists — any future + cap must arrive with a response envelope (cursor/total) so truncation + is detectable, plus a note that `API.md`'s "Boundary bounds" table + omits the two existing 500-caps. The issue body quotes the two + "A2/A3 contract territory" lines back at the server — arkho invited + this pin. +3. **Fix the one silently destructive consumer.** `_check_lift` + (`_sweep_api.sh:753-755`): a ticket absent from the `/tickets` read + makes `rows.get(...)` return `None`, so BOTH lift conditions fire — + the suppression lifts, the ladder re-runs, and a fresh env-issue is + minted every three cycles. This is the exact bug class the write-site + guard at `:1438-1450` documents ("AN EMPTY READ IS NOT A STATE"). + Mirror it at the read site: an absent ticket row means UNKNOWN — + keep the suppression this tick (the env-issue row absent means the + same; treat `None` as "not closed"). One guard, both `.get` sites. +4. **Nothing else changes.** The other truncation-fragile consumers + (`board-show.sh:26`'s "no ticket #N", `board-lint.sh:54`, the map) + are honest against today's contract — the contract pin in (2) is what + keeps them honest tomorrow. No length-sentinel: on an unbounded + route, an exactly-500 response is a legal board size, and a sentinel + would cry wolf on it. + +**Tests (RED first), in `test-sweep-resume.sh`:** a suppression record +whose ticket is MISSING from the `/tickets` fixture → the suppression +survives the tick (no lift, no rm), and the sweep prints nothing louder +than its normal line. (The arkho issue is an action, not a test.) + +--- + +## §5 bootstrap-prompt comparison fence + +**The hazard (verified).** The review bootstrap is one template +(`review-worker-bootstrap.md`, 203 lines) with four mutually exclusive +`` blocks — gh (`pr`, `scale`) and api (`api`, +`api-scale`) framings authored separately around a SHARED tail (skill +pin `:126-129`, worktree caveat `:152-156`, bindings roster, manifest +snapshots `:199-203`) that is interleaved with mode blocks, not one +contiguous slice. Nothing compares a gh render to an api render; the +relay-prompt drill (`test-transcript-diff.sh:230-283`) exists because +this exact two-authors drift already happened in an 8-line prompt — this +one is 203 lines. Bonus asymmetry: `review-dispatch.sh:866` renders +unknown `{{X}}` as EMPTY, while `implement-dispatch.sh:109-114` hard-errors. + +**Piece 1 — the renderer fails closed (production change).** +`review-dispatch.sh:866` substitutes unknown `{{X}}` with `""` — a +binding a mode block forgot renders as a silent blank, and no +downstream assertion can tell "empty by design" from "erased". Bring it +to parity with the implement renderer (`implement-dispatch.sh:109-114`, +which lists the unresolved names and exits 1). Both existing dispatch +suites already capture rendered prompts through their `daemon-spawn` +stubs (`test-review-dispatch.sh:78`, `test-review-dispatch-claim.sh:54`) +— those suites gain assertions that the CRITICAL bindings render +non-empty at the real call sites (`BIND_READY_FILE`, `SKILL_FILE`, +`IMPLEMENT_PROTOCOL_FILE`, `BOARD_SCRIPTS`, plus `TICKET_BODY_FILE` on +the api side), so a call site that stops supplying one now dies loudly +in the dispatcher AND fails a test. All four render call sites +(`:625`, `:776`, `:1512` ×2 modes) must be verified to supply their +full placeholder set before the hard-fail lands. + +**Piece 2 — a revised static fence, no dispatcher.** The render is a +pure function of template + `P_*` env (`_render_prompt`, +`review-dispatch.sh:848-868`). New file +`tests/reviewing-prs/test-bootstrap-parity.sh`: + +- Carry a minimal copy of the renderer (mode-fence regex + placeholder + substitution), driven over the REAL template file with one fixed + `P_*` fixture; render all four modes. **Honesty pins on the copy:** + assert the dispatcher still points at the same template path + (`BOOTSTRAP_TEMPLATE=`, `review-dispatch.sh:136` — the + `test-skill-entrypoint.sh:302` idiom) and still carries the + mode-fence regex and the `{{(\w+)}}` substitution literally. +- **Load-bearing sentences pinned in EVERY mode's render** — the relay + drill's idiom (`test-transcript-diff.sh:252-268`) applied here: the + skill-pin sentence ("dispatcher-pinned copy", the "over any + same-named skill" precedence clause), the read-it-live rule (each of + the four separately-authored rewordings pinned t/nt in both + directions, so two-authors drift stays visible instead of silent — + this is where the drift lives, and a shared-tail diff cannot see it), + and the worktree-bootstrap caveat. +- **Roster relation:** the gh render's `- \`NAME\`:` binding names + minus `PR_NUMBER/PR_URL/HEAD_REF/HEAD_SHA` must be a subset of the + api render's; the api render adds exactly `TICKET_BODY_FILE` (+ + `CLOSURE_PACKAGE`/`INTEGRATION_REF` on the scale pair). Anything else + fails. +- **Block-boundary check:** after stripping mode-owned lines and + binding lines, the remaining tail of the two renders in a pair must + be identical. This is NOT an authorship fence (the tail is one source + region — same source, same output); it pins the block BOUNDARIES: a + shared line accidentally swallowed into one mode's fence (the + gating-error class §8-of-#61 hit with the eligibility chip) surfaces + here and nowhere else. +- **No cross-contamination / nothing unrendered:** no `mode:` fence + survives any render; no `{{` survives any render (meaningful now + that unknowns hard-fail rather than blank). +- **Implement lane, same file, small section:** render + `worker-bootstrap.md` with and without the `api-only` region and + assert everything outside the region identical, roster relation + `+TICKET_BODY_FILE +PARENT_PIN`. + +What legitimately differs (mode-block prose beyond the pinned +sentences, `BASE_REF` sentinel vs branch, `TECH_DEBT_ISSUE=none`) is +either inside stripped blocks or an explicitly pinned difference — the +fence pins nothing that is supposed to vary. + +--- + +## §6 drill cosmetics + +All in the suite's own idioms (verified inventory; the two literal +numerics plus ~16 interpolated-id substrings, `helpers.sh:9-21` `t`/`nt` +being unanchored `grep -qF`): + +1. `test-transcript-diff.sh:151` — `t "…registered a ticket" "1" …` is + satisfied by any id containing `1`; replace with a non-empty check + + exact-line assertion in the file's own style (`[ -n … ]` as at + `:236`). +2. `test-transcript-diff.sh:214` — anchor `"4242"` into the refusal + phrase actually printed (`"#4242"` + trailing token), the way `:215` + already asserts a full phrase. +3. **Delimit the emitters once, fix four drills:** `row()` + (`test-human-verbs.sh:64-65`) already brackets its list fields — + extend the same delimiter to its scalar fields and to `owner_line()` + (`test-protocol-walk.sh:73`), then anchor the consuming assertions + (`owner=$RUN` → closed form) across `test-protocol-walk.sh:74,96`, + `test-crash-boundaries.sh:122,135,154`, `test-resume-first.sh:103`, + `test-human-verbs.sh:162`. +4. JSON-body ids: close with the next JSON token — + `"\"ticketId\":$T1,"` shape — at `test-protocol-walk.sh:97`, + `test-crash-boundaries.sh:76,86`, `test-escalation.sh:107`; regex + form `\"ticketId\":$T_TID[,}]` for the real `grep -q` at + `test-escalation.sh:76`. +5. `test-resume-first.sh:95-96` — the `nt` on `BOARD_RUN_ID=$RUN` fails + for the WRONG reason if a longer id contains `$RUN`; close both with + the line's trailing delimiter. +6. **Argv comparator:** record the walk's argv UNSUBSTITUTED (`%T` + kept) alongside the executed argv in `test-transcript-diff.sh:100-115`, + and compare the unsubstituted form in `transcript-compare.py:103` — + the per-side ticket id never enters the compared surface, while the + deliberate literal `4242` (step 6) stays literal. No blanket + `\d+ → ` normalizer: it would erase the known-ticket / + unknown-ticket distinction the step-6 probe tests. + +Each anchoring change must be shown to still PASS (they are +tightenings, not behavior changes) and at least one representative per +class shown to catch its wrong-reason pass (mutate the emitter/id in a +scratch copy — discrimination probe, not a committed test). + +--- + +## Acceptance + +- `printf` a body that quotes a `` grammar check +was a torn-write trigger (fuzz-refuted, v1.2.1); the bearer 0600 mode +leak through `_stamp_meta`'s shared tail; the meta opener rule itself +needed FOUR formulations (rightmost → content walk → whole-interior → +splitlines line model + segment fallback) — each hand revision broke a +shape the previous one got right, and the round ended properly only +when a 140k-body property fuzz (self-validated against all three +superseded implementations) replaced hand reasoning. The fuzzer is +committed; the next revision of that rule should be judged by it. + +**Gaps / accepted boundaries:** a quoted example that is all-legal +`key: value` lines with no closer and no surrounding prose is +byte-indistinguishable from a legacy-nested block (documented, +v1.2.4); a poisoned legacy body's forged in-block keys are its actual +content — the fix bounds strip damage, it cannot un-poison; the +legality-drift fence and remaining PRE_PARK vocabulary question stay +on dp#51's successor list. + +**Lessons:** (1) when hand-reasoned corrections of one rule fail +repeatedly on individually-obvious cases, stop and build the generator +— the fuzzer found the stable point in one pass and costs 0.3s to +keep. (2) A grammar check on a write path must be fuzzed against the +data already at rest before it ships — "unrepresentable" values had +been representable for months. (3) The stale `arkho-a1-board-service` +checkout cost a worker a 32-failure false regression; environment +pins belong in the ledger the moment they are discovered. + +## Revision Notes + +- v1.0 (2026-08-12): initial spec from four parallel code + investigations (qagent role, escalation counter, pagination reality, + fence/drill inventory). +- v1.2.5 (2026-08-13, PR-65 panel flow-back, four findings all + adopted): (1) §2's fail-open DEMOTION is overturned for a recognized + QAGENT — demote-and-resume strands the ticket (in-progress + + stale-reviewer retirement, the very failure §2 fixes); a QAGENT park + with no `pr:` now REFUSES the relay and stays parked (the posted + answers survive; the message names the recovery). Fail-open remains + for the IMPLEMENT/no-role rung only. (2) §3's one-attempt invariant + extends to phase 4: dispatchers receive the tick ledger (as they do + BOARD_SUPPRESS_DIR) and skip tick-ledgered tickets; a successful + dispatcher bind also clears `.attempts-` — a delivered recovery + is a recovery, whichever phase delivered it. (3) §1's candidate test + admits indentation (`lstrip() == opener`; META_RE was always + unanchored) so an indented REAL trailing block cannot lose to a + column-zero quoted example; indented quoted examples stay excluded + by interior legality; the fuzzer grammar gains indented-real-opener + bodies. (4) §1's validate-before-write hoists one caller up: + board-transition.sh validates meta BEFORE ensure_labels/surface-label + writes, closing the remaining torn-write window (labels persisted, + transition failed). +- v1.2.4 (2026-08-13, convergence flow-back): v1.2.3's + between-adjacent-candidates rule has a hole (shape C, reproduced by + the reviewer): prose QUOTING a legacy-nested example followed by a + real trailing block — the segment between the quoted outer and + quoted nested opener is all-legal (`note: line1`), so the quoted + opener won. Corrected rule: a candidate qualifies iff its WHOLE + interior — every line strictly between its opener and the FINAL + closer — is block-legal, where legal = known-key `key: value` or a + line-start nested marker; blank lines, prose, and any intermediate + `-->` disqualify. Equivalent single pass: classify lines once, choose + the first candidate after the LAST illegal line (last candidate as + fallback, preserving old rightmost behavior on all-illegal tails). + Shape B still resolves to the outer opener (its interior is entries + + nested marker line only); shapes A and C resolve to the real block + (the quoted example's closer/prose lines disqualify every quoted + opener). Known ambiguous boundary (accepted): a quoted example that + is all-legal kv lines with NO closer, NO blank and NO prose before + the real opener is byte-indistinguishable from a legacy-nested block + and resolves as one. +- v1.2.3 (2026-08-13, final-panel flow-back): §1's rightmost rule is + refined to a content-based candidate walk. Two panel findings, both + confirmed: (1) a LEGACY body whose pre-grammar client stored a marker + inside a meta value (shape B) made the rightmost walk pick the nested + opener — parse forged keys, and strip_meta left the outer block's + head behind as prose, a regression against the old leftmost strip + boundary; (2) the per-marker end-anchored rescan was O(N²) (1.77s at + 4000 markers; snapshot() runs parse_meta per issue). One rewrite + fixes both: candidates are line-start openers from the leftmost + match onward; walking left to right, a candidate is the real opener + iff every line between it and the next candidate is a known-key + `key: value` line (blank or prose lines disqualify — a real block + interior contains only its own entries); two regex scans total. On a + poisoned legacy body the guarantee is the STRIP boundary and rewrite + round-trip stability — the forged key inside the block's own lines + is that body's actual content and is not recoverable. +- v1.2.2 (2026-08-13, Task 9 review flow-back): §5 Piece 2's roster + parenthetical was wrong — `CLOSURE_PACKAGE`/`INTEGRATION_REF` are + carried by BOTH members of the scale pair (they are scale-mode + bindings, not api additions); the api side adds exactly + `TICKET_BODY_FILE` on both pairs. The landed fence pins the stronger + truth; this note corrects the spec to match. +- v1.2.1 (2026-08-13, Task 1 review flow-back): the `-->` half of the + value grammar is dropped — fuzz-proven harmless (8 keys × 8 + arrow-values × 4 prose shapes, zero mismatches: the collapsed value + always sits behind `key: `, never at line start), and rejecting it + bricked pre-fix arrow-bearing notes on every `update_meta` + read-modify-write AND was the only realistic trigger of a torn write + (`apply_state` moves the label before the meta write dies). The + `\s*$", re.S) +# Every boundary `str.splitlines()` honours — meta_match must cut interior lines +# exactly where parse_meta will, or a value folded on U+2028 (or CR, \v, \x85…) +# hides a `-->` and the prose behind it inside one apparently-legal line. +LINE_SEP_RE = re.compile("\r\n|[\n\r\v\f\x1c\x1d\x1e\x85\u2028\u2029]") META_KEYS = ("spawned-by", "relates-to", "branch", "pr", "plan", "pre-park", "parent-pin", "note") @@ -235,9 +239,113 @@ def graphql(query, **variables): # ── board:meta body block ──────────────────────────────────────────────── +def _block_line(line): + """True for a line that could legally sit INSIDE a meta block — a + `key: value` whose key is one of ours. Blank lines, prose and a quoted + example's `-->` are all False.""" + return ":" in line and line.split(":", 1)[0].strip() in META_KEYS + + +def meta_match(body): + """The META_RE match on the body's REAL trailing block, chosen by content. + + Every META_RE match ends at end-of-string (`\\s*$`), so the openers compete + and only the start differs. Two prose shapes make both the leftmost and the + rightmost opener wrong: + + - QUOTED (#60): the prose documents the block, so a marker-shaped example + sits above the real one. Leftmost anchors on the example and its lazy + middle runs to the real `-->` — a leftmost strip deletes the prose + between them. + - LEGACY-NESTED: a pre-grammar client stored a meta VALUE containing a + verbatim `` cannot, so the choice + is the FIRST opener standing after the LAST such line. Judging only the gap + between adjacent candidates is not enough: in QUOTED-NESTED that gap holds + the quoted example's own entries and reads legal, and the quoted opener + wins (spec v1.2.4). + + When no opener clears the last illegal line the block is noncanonical — an + unknown key, a comment, hand-edited spacing — and the fall back is to the + FIRST opener of the last run of candidates, i.e. the one that opened the + segment the illegal line landed in. Taking the LAST opener instead (the old + rightmost behavior) reopens shape B whenever a legacy block carries BOTH a + nested marker and an unknown key: the unknown key fences off every + candidate, and the nested opener wins. + + The pass splits lines on every separator `str.splitlines()` honours, not + `\\n` alone. parse_meta reads the block that way, so a quoted example that + uses U+2028 (or CR, or \\v) would otherwise fold its `-->` and the prose + after it into one interior line that reads legal — the quoted opener wins + and the next meta write truncates the body. + + One classification pass over the span plus two regex scans: the walk is + linear in the body, where the old rightmost loop re-ran the end-anchored + regex per marker (O(N²) on a marker-dense body). + + The returned start never includes META_RE's optional leading `\\n`: the + search is anchored at the chosen opener, where `\\n?` matches empty. + Byte-offset consumers (strip_meta, board-body.sh's splice) therefore keep + the separator newline and must normalize it themselves — strip_meta's + `.rstrip("\\n")` does.""" + body = body or "" + m0 = META_RE.search(body) + if not m0: + return None + head = len("` with nothing but whitespace behind it can + # occur at only one offset — and m0's lazy middle ends exactly there. + close = m0.end(1) + first = m0.start() + (1 if body[m0.start()] == "\n" else 0) + # One pass over the span: each candidate opener is recorded with the fence + # standing at the time — the offset past the last line so far that cannot be + # block interior — so the fallback can find the segment it opened. + opens, fence, pos = [], first, first + while pos < close: + sep = LINE_SEP_RE.search(body, pos, close) + line = body[pos:sep.start() if sep else close] + marker = line.lstrip() + if marker == "`) carries no colon, so it fences. + cand = pos + (len(line) - len(marker)) + # A nested marker is legal interior either way. It is a CANDIDATE + # only when a real `\n` follows — META_RE cannot match otherwise — + # and the closer still leaves room for a block to open here; a + # trailing `` has none. + if sep is not None and sep.group() == "\n" and cand + head <= close: + opens.append((cand, fence)) + elif not _block_line(line): + fence = (sep.end() if sep else close) + pos = sep.end() if sep else close + for start, _ in opens: + if start >= fence: + return META_RE.search(body, start) + floor = opens[-1][1] + return META_RE.search(body, next(s for s, _ in opens if s >= floor)) + + def parse_meta(body): """The trailing `` block → dict (absent keys omitted).""" - m = META_RE.search(body or "") + m = meta_match(body) meta = {} if not m: return meta @@ -254,7 +362,8 @@ def parse_meta(body): def strip_meta(body): """The body WITHOUT its trailing board:meta block — the ticket's own text, with the board's bookkeeping removed.""" - return META_RE.sub("", body or "").rstrip("\n") + m = meta_match(body) + return ((body or "")[:m.start()] if m else (body or "")).rstrip("\n") def contract_hash(body): @@ -270,17 +379,75 @@ def contract_hash(body): return hashlib.sha256(strip_meta(body).encode("utf-8")).hexdigest()[:12] -def render_body(body, meta): - """Body with its meta block replaced by `meta` (dropped when meta is empty). - Everything outside the block is preserved byte-for-byte.""" - base = strip_meta(body) - meta = {k: v for k, v in meta.items() if v} +def clean_meta(meta): + """Meta values normalized to the block's grammar — one line, no opening + marker — and refused when they cannot live there. Empty values dropped. + + parse_meta reads the block line-wise, so a multi-line value is silent + corruption at best and a forged key at worst; `` is NOT refused: the + collapse leaves every value behind its `key: ` prefix, and META_RE requires + the closer at line start, so an arrow can never terminate the block early + (fuzz-proven, spec v1.2.1). Refusing it bricked every stored note carrying + an ASCII arrow, since update_meta re-renders every key it parsed. + + Split out of render_body so a caller that makes OTHER remote writes first + can validate ahead of them — see check_meta_write.""" + clean = {} + for k, v in meta.items(): + if not v: + continue + # EVERY separator parse_meta's splitlines() would honour — \r\n alone + # leaves \v, \f, \x1c–\x1e, \x85, U+2028/U+2029 injectable as keys. + v = " ".join(str(v).splitlines()) + if "\n" % (base, block) +def render_body(body, meta): + """Body with its meta block replaced by `meta` (dropped when meta is empty). + Everything outside the block is preserved byte-for-byte.""" + return compose_body(strip_meta(body), meta) + + def _nums(val): """'#12 #7' / '12,7' → ['12', '7'] (issue-number refs in a meta value).""" return re.findall(r"\d+", val or "") @@ -833,14 +1000,17 @@ def apply_state(tickets, tid, to, why, extra_meta=None, bookkeeping=False): extra_meta lets the caller fold branch/pr into the same body write.""" n = tickets[tid] old = n["state"] + updates = {"note": why or None} + updates.update(extra_meta or {}) + # Validate the meta write BEFORE the label write (idempotent — a caller + # that writes labels of its own ahead of this one has already run it). + check_meta_write(n["body"], updates) if to in TERMINAL: # strip status labels first so a closed issue never carries one edit_labels(tid, remove=[STATUS_PREFIX + s for s in n["status_labels"]]) close(tid, to) else: set_state_label(tid, n, to) - updates = {"note": why or None} - updates.update(extra_meta or {}) update_meta(tid, n, **updates) if why: if bookkeeping: diff --git a/skills/issue-tracker/scripts/_board_api.py b/skills/issue-tracker/scripts/_board_api.py index 67b35de59a..e7ce719459 100644 --- a/skills/issue-tracker/scripts/_board_api.py +++ b/skills/issue-tracker/scripts/_board_api.py @@ -25,6 +25,20 @@ class RunEnded(Exception): """409 run-ended — the caller's run was reaped; callers route, not die.""" +class ClaimObsolete(Exception): + """A 409 saying the caller's CLAIM HANDLE is spent, not that the board is + sick — `nonce-consumed` (the predecessor's run ended, so that nonce can + never be replayed) and `stale-resume` (the ticket moved after the feed + read). Routed, not died on: the caller drops its journal uncharged and the + ticket comes back around on a fresh nonce. Carries `.code` so the caller + can say which one it met. + """ + + def __init__(self, code, message): + super().__init__(message) + self.code = code + + def die(msg): print("error: %s" % msg, file=sys.stderr) raise SystemExit(1) @@ -99,9 +113,13 @@ def _error(payload, status): env.get("message") or payload[:400]) -def request(method, path, body=None, principal="auto", ok=(200,), retry=None): +def request(method, path, body=None, principal="auto", ok=(200,), retry=None, + obsolete_codes=()): """One HTTP exchange. Dies with the contract's error identifier on - refusal; raises RunEnded on 409 run-ended (callers route on it).""" + refusal; raises RunEnded on 409 run-ended (callers route on it), and + ClaimObsolete on any code the caller named in `obsolete_codes` — named + per route rather than globally, because the same code is a routable + outcome on one route and an ordinary refusal on another.""" if retry is None: retry = method == "GET" data = json.dumps(body).encode() if body is not None else None @@ -122,6 +140,8 @@ def request(method, path, body=None, principal="auto", ok=(200,), retry=None): code, message = _error(e.read().decode(), e.code) if code == "run-ended": raise RunEnded(message) from None + if code in obsolete_codes: + raise ClaimObsolete(code, message) from None # a refusal is an answer, never retried die("%s %s refused: %s — %s" % (method, path, code, message)) except (urllib.error.URLError, OSError) as e: @@ -146,7 +166,8 @@ def claim_successor(ticket_id, nonce, lease_minutes=None): body = {"ticketId": int(ticket_id), "dispatchNonce": nonce} if lease_minutes is not None: body["leaseMinutes"] = lease_minutes - return request("POST", "/runs/claim-successor", body, "automation", retry=True) + return request("POST", "/runs/claim-successor", body, "automation", retry=True, + obsolete_codes=("nonce-consumed", "stale-resume")) def needing_resume(): diff --git a/skills/issue-tracker/scripts/_claim_journal.sh b/skills/issue-tracker/scripts/_claim_journal.sh index bd39214b83..9f7dc00157 100644 --- a/skills/issue-tracker/scripts/_claim_journal.sh +++ b/skills/issue-tracker/scripts/_claim_journal.sh @@ -18,6 +18,8 @@ # _claim_lane_cap L the lane cap to replay that lane under # _claim_drop_journal N remove a nonce's journal (and its body file) # _claim_retire_worker U retire a spawned session by uuid +# _claim_suppress_dir print the suppression directory (the sweep's +# failed-cycle counts live beside its records) # _api_py the API-client python runner (_binding.sh) # DAEMON_HOME registry root @@ -114,10 +116,12 @@ PY _reconcile_claims() { local actions lines line act nonce lane run extra actions="$(T_DHOME="$DAEMON_HOME" T_LANES="$CLAIM_LANES" \ + T_SUPPRESS="$(_claim_suppress_dir)" \ T_GRACE="${BOARD_CLAIM_INFLIGHT_GRACE:-120}" python3 - <<'PY' import glob, json, os, time home = os.environ["T_DHOME"] lanes = set(os.environ["T_LANES"].split(",")) +suppress = os.environ.get("T_SUPPRESS") or "" grace = float(os.environ["T_GRACE"]) # run id -> the uuid of the meta carrying it. The uuid is needed by the # `stranded` arm, which has a worker to retire and not merely a run to end. @@ -177,8 +181,12 @@ for p in sorted(glob.glob(os.path.join(home, "board-claims", "*.json"))): run = j.get("run_id") daemon = j.get("daemon") or "" control = j.get("control") or "" - if (run and str(run) in live and control and not alive(j.get("pid")) - and not os.path.exists(os.path.join(control, "bind-ready.json.ack"))): + # A journalled control dir with no ack in it is a handover that has not + # crossed its startup barrier — the review lane's durability line. Until it + # does, the delivery may still be undone (see the two arms below). + ack_pending = bool(control) and not os.path.exists( + os.path.join(control, "bind-ready.json.ack")) + if run and str(run) in live and ack_pending and not alive(j.get("pid")): # A BOUND RUN WHOSE WORKER NEVER CROSSED ITS STARTUP BARRIER. The # review handover is bind -> publish the barrier -> wait for the ack, # and the journal is marked only after the ack; a crash before the @@ -197,13 +205,62 @@ for p in sorted(glob.glob(os.path.join(home, "board-claims", "*.json"))): # outlives that grace while it waits for the ack. Pid reuse errs into # `repaired`, which sends nothing. print("stranded\x1f%s\x1f%s\x1f%s\x1f%s" % (nonce, lane, run, live[str(run)])) + elif run and str(run) in live and ack_pending: + # The same shape with the writer STILL ALIVE: a peer between its bind + # and the ack it is waiting on. Sealing that journal would close a + # record somebody else is still writing, and the delivery is not + # durable yet — if the ack never lands, the arm above retires the + # worker and releases the run. Left entirely alone, counter included. + print("inflight\x1f%s\x1f%s\x1f%s\x1f%s" % (nonce, lane, run, j.get("pid"))) elif run and str(run) in live: # the spawn DID complete — its worker is in the registry — and only # the marker write was lost. Repair it in place; nothing to send. + # + # THE RESET COMES FIRST AND THE SEAL SECOND. A sealed journal is + # skipped by every later pass, so anything left until after that write + # happens once or never: the dispatcher clears the ticket's + # failed-cycle count one line ahead of its own marker write, and this + # arm — the crash that lost that write — does the same. Both steps are + # idempotent, so a crash between them costs nothing: the journal is + # still open and the next pass redoes both. + ticket = str(j.get("ticket") or "") + failed = "" + if ticket and suppress: + try: + os.remove(os.path.join(suppress, ".attempts-" + ticket)) + except FileNotFoundError: + # ENOENT ANSWERS TWO OPPOSITE QUESTIONS: the counter is + # already gone (the reset is done), or the directory holding + # it cannot be seen at all — an operator BOARD_SUPPRESS_DIR + # whose volume unmounted, a path that moved. Sealing on the + # second reading loses the same way EACCES did: the mount + # comes back carrying the count, and the journal that would + # have cleared it is closed forever. + # + # REACHABILITY tells them apart, not the directory alone. The + # sweep creates it the first time it counts anything, so on a + # registry that has never had a failed cycle it legitimately + # does not exist — treating that as a fault would stall every + # repair on every healthy fleet. A directory whose PARENT is + # gone too is a path this process cannot see. + parent = os.path.dirname(suppress.rstrip("/")) or "." + if not (os.path.isdir(suppress) or os.path.isdir(parent)): + failed = "the suppression directory is unreachable" + except OSError as e: + # A REMOVAL THAT FAILED IS NOT A REMOVAL — a read-only or + # unmounted registry, a permission change. The count is still + # standing, and sealing on top of it hides that from every + # later pass. Skip the seal: the journal stays open and the + # next one retries the pair. + failed = e.strerror or str(e) + if failed: + print("resetfailed\x1f%s\x1f%s\x1f%s\x1f%s %s" + % (nonce, lane, run, ticket, failed)) + continue j["spawn_completed"] = True with open(p, "w") as f: json.dump(j, f) - print("repaired\x1f%s\x1f%s\x1f%s\x1f" % (nonce, lane, run)) + print("repaired\x1f%s\x1f%s\x1f%s\x1f%s" % (nonce, lane, run, ticket)) elif run and daemon and daemon in names: # A session by that name exists but no meta carries the run: the spawn # landed and the bind did not. The worker is alive with its bearer in @@ -239,7 +296,11 @@ PY unreadable) echo "reconcile: unreadable json at $nonce — left untouched; no claim under it can be reconciled until it is repaired or removed by hand" >&2 ;; repaired) - echo "reconcile: $nonce did spawn (run $run) — marker repaired" ;; + # The failed-cycle reset that belongs to this repair already ran, in + # the same process, ahead of the seal — see the arm above. + echo "reconcile: $nonce did spawn (run $run) — marker repaired${extra:+ (#$extra)}" ;; + resetfailed) + echo "reconcile: $nonce did spawn (run $run) but the failed-cycle reset failed (#$extra) — the marker is NOT written, so the next pass retries both steps; the ticket would otherwise escalate early on a much later fault" >&2 ;; stranded) # The one handoff crash that leaves a LIVE-LOOKING run nobody will ever # work: bound, but the startup barrier never opened. Retire the worker diff --git a/skills/issue-tracker/scripts/_sweep_api.sh b/skills/issue-tracker/scripts/_sweep_api.sh index fdb68f58b5..833ba8d3e0 100755 --- a/skills/issue-tracker/scripts/_sweep_api.sh +++ b/skills/issue-tracker/scripts/_sweep_api.sh @@ -46,6 +46,9 @@ # this budget. # BOARD_SUPPRESS_DIR (exported to the dispatchers) suppression # records; they read, this tick writes +# BOARD_RESUMED_LEDGER (exported to the dispatchers) the tickets this +# tick already attempted a recovery for; they +# read, phase 3 writes # IMPLEMENT_MODEL LOCAL_REPO model pin / repo for a successor fresh spawn # BOARD_API_URL BOARD_CREDENTIALS_FILE resolved by _binding.sh set -euo pipefail @@ -739,8 +742,16 @@ _suppressed() { [ -f "$SUPPRESS_DIR/$1.json" ]; } # Lift the suppression on ticket $1 if either trigger fired. Both are checked # every tick because either one alone is a trap: an operator who moves the # ticket should not also have to close the env-issue, and closing the -# env-issue is the natural "I fixed the substrate" gesture. A ticket that has -# fallen off the listing entirely (terminal) reads as moved, which is right. +# env-issue is the natural "I fixed the substrate" gesture. +# +# AN ABSENT ROW IS NOT A STATE — a truncated or partial /tickets read must +# never lift a suppression. The listing is read whole, with no cursor and no +# envelope, so a short read looks exactly like a smaller board; treating a +# missing row as a value fired BOTH triggers on it (absent != the recorded +# state, and absent read as "closed"), lifting every suppression the read +# could not see and minting the human a fresh env-issue every three cycles. +# Absent is UNKNOWN on both sides: keep waiting for a read that can name it. +# This is the read-site mirror of the write-site guard at _escalate. _check_lift() { T_TID="$1" T_DIR="$SUPPRESS_DIR" _api_py - <<'PY' import json, os @@ -751,8 +762,10 @@ if not os.path.exists(path): with open(path) as f: rec = json.load(f) rows = {str(t["id"]): t["state"] for t in A.tickets(principal="automation")} -moved = rows.get(str(rec["ticket"])) != rec["state"] -closed = rows.get(str(rec["env_issue"])) in (None, "done", "wontfix") +cur = rows.get(str(rec["ticket"])) +moved = cur is not None and cur != rec["state"] +env = rows.get(str(rec["env_issue"])) +closed = env in ("done", "wontfix") # absent env-issue: unknown, keep waiting if moved or closed: os.remove(path) print("suppression lifted for #%s — %s" % @@ -967,7 +980,29 @@ PY # worker whose bind never landed. NOT ended (that would kill it) and # not replayed — reported, and the server's lease reclaim owns it. # -# Runs BEFORE the feed is read, so anything it releases is served in this tick. +# Runs BEFORE the feed is read, so anything it releases is served in this tick, +# and AFTER the lift pass, so a just-lifted ticket replays its own standing +# journal here rather than minting a fresh nonce down in the feed loop. +# +# A SUPPRESSED TICKET IS FROZEN, ITS JOURNAL INCLUDED. Suppression means "stop +# spending recovery on this ticket until a human clears the substrate", and a +# reconciliation that kept replaying the journal spent it anyway: every replay +# charged another cycle, every third cycle re-escalated, and each re-escalation +# rewrote the suppression record with the state read seconds earlier in the same +# tick — so _check_lift's `moved` could never fire and the "move the ticket" +# half of the escalation's own instructions was inert. The journal is left +# exactly where it is; it is still the retry handle when the suppression lifts. +# The price of "untouched" is real and small: a suppressed ticket's undelivered +# successor run now holds until its lease expires rather than being released +# here, and the orphan warning is silent for the duration of the suppression. +# +# $RESUMED_LEDGER (phase_resume owns it) collects the tickets this TICK +# ATTEMPTED a recovery for — written before the attempt, because a guard that +# leaves the ticket for the next tick has spent this ticket's turn either way. +# Of the reconciliation arms only replay writes it: settle and orphaned make no +# claim, and settle's release is designed to be served by this very tick's +# feed. The feed loop writes it too — an attempt is an attempt whichever door +# it came through, and phase 4 reads the ledger to stay off both. _reconcile_successors() { local plan lines line act nonce run tid sess daemon transcript [ -d "$CLAIMS_DIR" ] || return 0 @@ -1033,10 +1068,25 @@ PY done <<<"$plan" for line in ${lines[@]+"${lines[@]}"}; do IFS=$'\x1f' read -r act nonce run tid sess daemon <<<"$line" + if [ -n "$tid" ] && _suppressed "$tid"; then + echo "resume: successor claim $nonce for #$tid is suppressed — the journal stands untouched until the suppression lifts" + continue + fi case "$act" in replay) + # The ledger is CONSULTED here as well as written, or the invariant + # holds only across the hand-off to the feed and not within this pass: + # two unfinished journals naming one ticket are two replay rows, and + # each one claimed and charged. Reachable because the intermediate + # commit on this branch minted an extra journal per tick during a claim + # fault, so a registry that ticked on it arrives here holding several. + if [ -n "$tid" ] && grep -qxF -- "$tid" "$RESUMED_LEDGER"; then + echo "resume: successor claim $nonce waits — #$tid already had its one recovery attempt this tick" + continue + fi echo "resume: successor claim $nonce never reached a run — replaying it for #$tid" - [ -z "$tid" ] || _resume_one "$tid" "$nonce" || true ;; + [ -z "$tid" ] || { printf '%s\n' "$tid" >> "$RESUMED_LEDGER" + _resume_one "$tid" "$nonce" || true; } ;; orphaned) echo "resume: successor claim $nonce spawned $daemon for run $run but never bound it — the session is live and is NOT being ended; it holds no bearer at rest, so no later relay or resume can speak for it (retire it by hand once it is done)" >&2 _journal "$CLAIMS_DIR/$nonce.json" "$run" 1 "$tid" "$daemon" "$sess" ;; @@ -1084,7 +1134,7 @@ PY _resume_one() { local tid="$1" nonce="${2:-}" dir exports ids text prompt transcript delivered="" local pre_pending="" post_pending="" lane="" role="" - local C_CLAIMED=0 C_RUN="" C_FENCE="" C_BEARER="" C_SESS="" C_PIN="" + local C_CLAIMED=0 C_RUN="" C_FENCE="" C_BEARER="" C_SESS="" C_PIN="" C_OBSOLETE="" # AN UNRESOLVED FORK IS NOT A RESUMABLE SESSION, and that is settled BEFORE # the claim so this tick never mints a successor run it cannot deliver. # daemon-resume stamps status=error + pending_short when a fork LAUNCHED @@ -1128,8 +1178,14 @@ _resume_one() { exports="$(T_TID="$tid" T_NONCE="$nonce" T_BODY="$dir/body.md" _api_py - <<'PY' import os, shlex import _board_api as A -out = A.claim_successor(os.environ["T_TID"], os.environ["T_NONCE"]) def q(k, v): print("%s=%s" % (k, shlex.quote(str(v)))) +try: + out = A.claim_successor(os.environ["T_TID"], os.environ["T_NONCE"]) +except A.ClaimObsolete as e: + # A spent handle, not a sick board — handed back as a fact for the shell + # to route on rather than as the failure exit every other refusal takes. + q("C_OBSOLETE", e.code) + raise SystemExit(0) if not out.get("claimed", True): q("C_CLAIMED", 0) raise SystemExit(0) @@ -1155,11 +1211,33 @@ with open(os.environ["T_BODY"], "w") as f: f.write(out.get("body") or "") PY )" || { - # The journal STAYS: a claim that died on the wire may still have landed. + # A FAULT, and the first exit that ever charged for one: transport death + # after retries, a 5xx, a malformed grant, an untyped refusal. Left + # uncounted, a ticket whose claim persistently errors churns forever — and + # the kept journal is re-classified `replay` next tick while the feed ALSO + # re-serves the ticket, so the churn costs two claims and a leaked journal + # a tick. The journal STAYS: a claim that died on the wire may still have + # landed, and it is the only handle a replay has. echo "resume: #$tid — successor claim failed; journal $nonce kept" >&2 + _attempts "$tid" fail return 1 } eval "$exports" + # THE JOURNAL IS OBSOLETE, NOT THE SUBSTRATE SICK. `nonce-consumed` says the + # predecessor's run ended, so this nonce is spent for good; `stale-resume` + # says the ticket moved after the feed read, so its new state governs. + # Neither is a fault to charge: the handle is dropped and the ticket comes + # back around — on a fresh nonce, or through ordinary dispatch. + if [ -n "$C_OBSOLETE" ]; then + rm -f "$CLAIMS_DIR/$nonce.json" + echo "resume: #$tid — the successor journal is obsolete ($C_OBSOLETE); dropped uncharged, and the ticket comes back around on its own" + return 0 + fi + # Uncharged on purpose: no grant is the server's BACKPRESSURE — a lane cap, + # an eligibility rule — and a healthy wait state. A suppression written from + # it would take a healthy ticket out of both the resume and the dispatch + # phase until a human closed an env-issue, which is strictly worse than the + # wait it would be "fixing". if [ "$C_CLAIMED" != 1 ]; then rm -f "$CLAIMS_DIR/$nonce.json" echo "resume: #$tid — the board granted no successor" @@ -1460,8 +1538,9 @@ import _board_api as A tid = os.environ["T_TID"] payload = {"title": "stuck resume: ticket #%s cannot be revived" % tid, "category": "env-issue", - "body": "Three resume and fresh-spawn cycles failed for ticket " - "#%s. The sweep has SUPPRESSED that ticket: phase 3 skips " + "body": "Three recovery cycles failed for ticket #%s (successor " + "claim, resume, or fresh spawn). " + "The sweep has SUPPRESSED that ticket: phase 3 skips " "it and phase 4 releases any claim that yields it. " "Investigate the session/daemon substrate, then either " "move ticket #%s (any transition) or close this env-issue " @@ -1505,17 +1584,27 @@ PY phase_resume() { local dir f tid tids=() mkdir -p "$SUPPRESS_DIR" - # Unfinished successor claims first: a run this machine already holds but - # never delivered keeps its ticket off the feed below, so reconciling after - # the read would postpone every such recovery by a whole tick. - _reconcile_successors - # Lift first: a suppression that no longer holds must not cost this tick a - # resume it could have made. + # LIFT FIRST: a suppression that no longer holds must not cost this tick a + # resume it could have made — and, now that reconciliation honors suppression, + # lifting after it would strand journals. A suppression lifting mid-tick in + # the old order left the journal untouched in reconcile, dropped the record, + # and then let the feed claim a FRESH nonce: the old journal survived to + # replay beside the new successor on a later tick. Lifting first lets a + # just-lifted ticket replay its own standing journal. for f in "$SUPPRESS_DIR"/*.json; do [ -e "$f" ] || continue _check_lift "$(basename "$f" .json)" \ || echo "resume: suppression check for $(basename "$f" .json) failed" >&2 done + # Unfinished successor claims before the feed: a run this machine already + # holds but never delivered keeps its ticket off the feed below, so + # reconciling after the read would postpone every such recovery by a whole + # tick. ONE RECOVERY ATTEMPT PER TICKET PER TICK: the ledger carries the + # tickets reconciliation already claimed for into the feed loop, where a + # ticket re-served by the feed would otherwise buy a second successor claim, + # a second charged cycle and a second journal in the same tick. + RESUMED_LEDGER="$(mktemp "$SCRATCH/resumed.XXXXXX")" + _reconcile_successors dir="$(mktemp -d "$SCRATCH/feed.XXXXXX")" _api_py - > "$dir/feed" <<'PY' || { echo "resume: needing-resume feed unavailable this tick" >&2; return 0; } import _board_api as A @@ -1536,10 +1625,20 @@ PY echo "resume: suppressed — skipping #$tid" continue fi + if grep -qxF -- "$tid" "$RESUMED_LEDGER"; then + echo "resume: #$tid — already replayed this tick" + continue + fi _budget_left || { echo "resume: tick budget exhausted — the rest of the feed rides the next tick"; break; } # Every live run's lease, refreshed ahead of a recovery that may block for # the whole bound — including the runs this ticket has nothing to do with. _tick_renew + # LEDGERED BEFORE THE ATTEMPT, exactly as the replay arm does it. A feed + # recovery that faults or releases leaves the ticket unowned and equally + # spent, and phase 4 would otherwise claim and spawn it in the same tick. + # A recovery that SUCCEEDS makes the ticket owned, so the record costs it + # nothing. + printf '%s\n' "$tid" >> "$RESUMED_LEDGER" _resume_one "$tid" || true done } @@ -1569,8 +1668,15 @@ phase_dispatch() { # unless the `all` tick set it: a phase asked for by name is its own tick # with its own clock, and is no more gated inside the dispatchers than it is # at the case arm below. + # BOARD_RESUMED_LEDGER travels for the same reason BOARD_SUPPRESS_DIR does: + # ONE RECOVERY ATTEMPT PER TICKET PER TICK is an invariant of the TICK. A + # replay that FAULTED leaves its ticket unowned, so the ordinary lane claim + # below could pick that very ticket seconds later and spend a second attempt + # on it. Empty on a phase asked for by name — that tick made no recovery + # attempt, so it fences nothing. local env_common=(BOARD_SUPPRESS_DIR="$SUPPRESS_DIR" DAEMON_HOME="$DAEMON_HOME" DAEMON_SCRIPTS="$DAEMON_SCRIPTS" LOCAL_REPO="${LOCAL_REPO:-$BOARD_ROOT}" + BOARD_RESUMED_LEDGER="${RESUMED_LEDGER:-}" BOARD_TICK_DEADLINE="${TICK_DEADLINE:-}") env "${env_common[@]}" "$impl" --sweep \ || echo "dispatch: the implement lanes failed this tick" >&2 diff --git a/skills/issue-tracker/scripts/board-answer.sh b/skills/issue-tracker/scripts/board-answer.sh index 9bea6f95fc..dca2e55382 100755 --- a/skills/issue-tracker/scripts/board-answer.sh +++ b/skills/issue-tracker/scripts/board-answer.sh @@ -10,10 +10,12 @@ # land on the TICKET first (the ticket is the record), the ticket returns to # its parking lane's in-flight state (pre-park: meta; when absent, the bound # worker's own lane from its registry meta — in-design for an Architect, -# in-progress otherwise), and the bound session is resumed with the answers -# relayed verbatim — the worker keeps its orientation and re-states its gate -# verdict before proceeding. No judge is reintroduced: the relay is -# mechanical, the human is the author, the ticket is the record. +# in-review for a QAgent with the ticket's recorded pr: re-supplied, +# in-progress otherwise; a review-lane return with no pr: to re-supply is +# REFUSED and the ticket stays parked), and the bound session is resumed with +# the answers relayed verbatim — the worker keeps its orientation and +# re-states its gate verdict before proceeding. No judge is reintroduced: +# the relay is mechanical, the human is the author, the ticket is the record. # # API binding: the park-answer call IS the record (no separate comment), and # the RETURN STATE IS THE SERVER'S — it reads the bound run's lane, so none of @@ -133,7 +135,16 @@ fi # Validate the park + find the binding; post the [answers] comment only once # the relay is certain to proceed (a refused relay posts nothing — the human # can still comment by hand and take the fresh-dispatch path). -info="$(T_ID="$tid" T_ANSWERS="$answers" T_DHOME="$DAEMON_HOME" _py - <<'PY' | tail -n 1 +# +# A FUNCTION, not an inline "$(...)": bash 3.2 scans a command substitution +# with a matcher that does not understand the heredoc it contains, so every +# apostrophe in this prose — worker's, doesn't — toggles its quote state. At +# an ODD count the matcher is stuck in single-quote mode, stops counting +# parens, and the substitution fails to find its close. The old body survived +# on an even count; one more apostrophe of prose broke it. A function body +# goes through the real parser instead, so the prose here is free again. +_probe_binding() { + T_ID="$tid" T_ANSWERS="$answers" T_DHOME="$DAEMON_HOME" _py - <<'PY' | tail -n 1 import glob import json import os @@ -193,20 +204,64 @@ else: # (implement-dispatch.sh), rather than hardcoding in-progress: an # Architect resumed there would land in a state its protocol cannot # exit (LEGAL["in-progress"] has no ready-for-implementer edge). - # Workers bound before this fix (or spawned by any other route) carry - # no role — for those, in-progress stays the fallback, matching prior - # behavior; it was only ever wrong for the architect lane. - ret = "in-design" if (meta.get("role") or "").upper() == "ARCHITECT" else "in-progress" -print("%s\t%s\t%s\t%s\t%s" % (meta.get("uuid", ""), meta.get("engine", "claude"), - meta.get("status", "?"), meta.get("updated", "?"), ret)) + # Workers spawned by a route that stamps no role fall through to + # in-progress, matching prior behavior — the default is only ever wrong + # for a lane whose protocol cannot exit in-progress. + role = (meta.get("role") or "").upper() + if not role and str(meta.get("name") or "").startswith(("review-pr-", + "review-epic-")): + # Reviewers bound before the role stamp: the deterministic worker + # name is the only role record they carry. + role = "QAGENT" + if role == "ARCHITECT": + ret = "in-design" + elif role == "QAGENT": + ret = "in-review" + else: + ret = "in-progress" +pr = "" +if ret == "in-review": + # in-review is the one return state board-transition will not write on + # trust: the ticket must carry a PR link, so the link is re-supplied from + # the ticket's own pr: meta. + pr = B.parse_meta(tickets[tid]["body"]).get("pr") or "" + if not pr: + # AND WHEN THERE IS NONE, THE PARK HOLDS. Demoting to in-progress and + # resuming looked like the gentle fallback and was the stranding this + # whole return exists to prevent: the reviewer wakes on a ticket in a + # lane it owns no branch in and cannot exit, review-dispatch's + # stale-reviewer arm retires its meta once idle, and the ticket sits + # in-progress with no PR and nobody bound. A reviewer-lane ticket with + # no pr: is an anomaly — the in-review entry gate stamps it — and an + # anomaly pauses rather than guessing. The park is the safe state: the + # answers are already posted (that comment lands before this), so the + # human loses nothing by fixing the meta and re-running. + B.die("#%s returns to the review lane but the ticket carries no pr: " + "meta — the relay is REFUSED and #%s stays parked at " + "needs-human. Your answers are already on the ticket, so " + "nothing is lost: restore the pr: link in the body's " + "board:meta block (board-body.sh splices that block through " + "byte-for-byte, so edit it there), then re-run " + "`board-answer.sh %s --posted`." % (tid, tid, tid)) +print("%s\t%s\t%s\t%s\t%s\t%s" % (meta.get("uuid", ""), meta.get("engine", "claude"), + meta.get("status", "?"), meta.get("updated", "?"), + ret, pr)) PY -)" -IFS=$'\t' read -r uuid engine status updated ret <<<"$info" +} +info="$(_probe_binding)" +IFS=$'\t' read -r uuid engine status updated ret pr <<<"$info" [ -n "$uuid" ] || die "binding lookup failed" echo "relay: #$tid → $engine session ${uuid:0:8} (status=$status, last-updated=$updated, return=$ret)" -"$SCRIPT_DIR/board-transition.sh" "$tid" "$ret" \ - "answers relayed — resuming bound session ${uuid:0:8}" +# ret=in-review implies a non-empty pr (the python refuses the relay +# otherwise), so the flag arm never passes an empty link. +if [ "$ret" = in-review ]; then + "$SCRIPT_DIR/board-transition.sh" "$tid" "$ret" \ + "answers relayed — resuming bound session ${uuid:0:8}" --pr "$pr" +else + "$SCRIPT_DIR/board-transition.sh" "$tid" "$ret" \ + "answers relayed — resuming bound session ${uuid:0:8}" +fi if [ -n "$posted" ]; then block="(already on the ticket — read the latest comments: gh issue view $tid --comments)" diff --git a/skills/issue-tracker/scripts/board-body.sh b/skills/issue-tracker/scripts/board-body.sh index a4ca854ab7..487f7c6253 100755 --- a/skills/issue-tracker/scripts/board-body.sh +++ b/skills/issue-tracker/scripts/board-body.sh @@ -61,22 +61,16 @@ old = B.gh(["issue", "view", tid, "-R", B.repo(), "--json", "body", "--jq", ".body"]) new = open(env["T_FILE"]).read() -# The raw splice. META_RE is consulted for a byte OFFSET only — the matched +# The raw splice. The match is consulted for a byte OFFSET only — the matched # text is carried through untouched, never parsed and never re-rendered, so # unknown keys, comment lines and noncanonical spacing all survive a client # older than whatever wrote them. # -# The block that counts is the LAST one. META_RE is leftmost-first and its -# `.*?` will happily span from a marker-like example QUOTED in the prose all -# the way to the real trailing `-->`, so splicing at the first match would -# carry the prose this edit was told to replace. Walk to the rightmost match. -m = None -pos = 0 -while True: - nxt = B.META_RE.search(old, pos) - if not nxt: - break - m, pos = nxt, nxt.start() + 1 +# Which opener starts the real block is decided by the interior between the +# candidates, not by position — see _board.meta_match (#60). Its start never +# includes META_RE's optional leading newline, so the separator is this +# splice's to supply. +m = B.meta_match(old) if m: # Only the newlines AROUND the block are normalized (to render_body's # blank-line-then-block shape); its own bytes are untouched. diff --git a/skills/issue-tracker/scripts/board-migrate-gh.sh b/skills/issue-tracker/scripts/board-migrate-gh.sh index 60c7cb01a7..390e173caa 100755 --- a/skills/issue-tracker/scripts/board-migrate-gh.sh +++ b/skills/issue-tracker/scripts/board-migrate-gh.sh @@ -207,8 +207,12 @@ for tid, n in sorted(legacy.items(), key=lambda kv: int(kv[0][1:])): if append: what.append("pre-spec md") def write_body(num=num, gn=gn, want_meta=want_meta, append=append): - base = B.META_RE.sub("", gn["body"] or "").rstrip("\n") - B.set_body(num, B.render_body(base + append, want_meta)) + # Exactly ONE strip, and it is the rightmost one: a leftmost + # strip cuts from a marker QUOTED in the ticket's prose, and a + # SECOND strip (render_body's own) would then cut a quoted + # example that ENDS the prose. This rewrite is one-shot (#60). + base = B.strip_meta(gn["body"]) + B.set_body(num, B.compose_body(base + append, want_meta)) act("%s: body += %s" % (ref, " + ".join(what)), write_body) print("%s: %d action(s)%s" % ("migrated" if apply else "dry-run", diff --git a/skills/issue-tracker/scripts/board-transition.sh b/skills/issue-tracker/scripts/board-transition.sh index 7bdce009ad..9cf3f3912c 100755 --- a/skills/issue-tracker/scripts/board-transition.sh +++ b/skills/issue-tracker/scripts/board-transition.sh @@ -315,25 +315,6 @@ if to in B.DISPATCHABLE and "(pre-spec: fill in)" in (n.get("body") or ""): B.die("#%s is still a pre-spec skeleton — fill the body (gh issue edit " "%s --body-file ) before a dispatchable lane state" % (tid, tid)) -B.ensure_labels() - -# Surface re-match (spec "Matching moment 2"): entering a dispatchable lane -# state re-runs identifier matching over the CURRENT body — the documented -# two-step registration flow (skeleton birth, body fleshed out afterward via -# `gh issue edit`) means register-time matching may have seen only a title. -# Add-only, labels only; relates edges are the sweep pass's backstop. -if to in B.DISPATCHABLE: - _reg = B.surfaces_registry() - if _reg is not None: - _hits = [s_ for s_ in B.match_identifiers( - _reg, n["title"] + "\n" + B.strip_meta(n.get("body") or "")) - if s_ not in n["surfaces"]] - for s_ in _hits: - B.ensure_surface_label(s_) - B.edit_labels(tid, add=(B.SURFACE_PREFIX + s_,)) - if _hits: - print("surface: += %s" % " ".join(_hits)) - extra = {} if env["T_BRANCH"]: extra["branch"] = env["T_BRANCH"] @@ -361,6 +342,32 @@ if to == "needs-human" and cur in B.PRE_PARK: extra["pre-park"] = B.PRE_PARK[cur] if cur == "needs-human" and to != "needs-human": extra["pre-park"] = None +# THE META WRITE IS VALIDATED AHEAD OF EVERY LABEL WRITE ON THIS PATH. +# apply_state validates before its own label write, but this script writes +# labels of its own first (ensure_labels, then the surface re-match), so a note +# the grammar refuses tore the ticket: labels persisted, transition failed, and +# nothing rolls them back. Same helper apply_state uses, same `extra`. +B.check_meta_write(n["body"], dict(extra, note=note or None)) + +B.ensure_labels() + +# Surface re-match (spec "Matching moment 2"): entering a dispatchable lane +# state re-runs identifier matching over the CURRENT body — the documented +# two-step registration flow (skeleton birth, body fleshed out afterward via +# `gh issue edit`) means register-time matching may have seen only a title. +# Add-only, labels only; relates edges are the sweep pass's backstop. +if to in B.DISPATCHABLE: + _reg = B.surfaces_registry() + if _reg is not None: + _hits = [s_ for s_ in B.match_identifiers( + _reg, n["title"] + "\n" + B.strip_meta(n.get("body") or "")) + if s_ not in n["surfaces"]] + for s_ in _hits: + B.ensure_surface_label(s_) + B.edit_labels(tid, add=(B.SURFACE_PREFIX + s_,)) + if _hits: + print("surface: += %s" % " ".join(_hits)) + lines = [B.apply_state(tickets, tid, to, note, extra_meta=extra)] # Sweep: first active child pulls its epic chain to each parent's own diff --git a/skills/reviewing-prs/scripts/review-dispatch.sh b/skills/reviewing-prs/scripts/review-dispatch.sh index 8f31233516..bbd778f38e 100755 --- a/skills/reviewing-prs/scripts/review-dispatch.sh +++ b/skills/reviewing-prs/scripts/review-dispatch.sh @@ -342,7 +342,12 @@ PY } # Write one field into daemon <1>'s registry meta, under the same lock -# board-bind.sh takes (read-modify-write; unknown keys are preserved). +# board-bind.sh takes (read-modify-write; unknown keys are preserved), and +# under its write_meta mode discipline too. This helper lands on API metas — +# a failure retirement stamps one — and an API meta holds the run bearer. +# Recreating that file at the umask default republishes the secret +# world-readable, and permanently: the api claim path's own stamp preserves +# whatever mode it finds, so a widened mode is never narrowed again. _stamp_meta() { # DAEMON_HOME="$DAEMON_HOME" M_UUID="$1" M_KEY="$2" M_VAL="$3" python3 - <<'PY' import fcntl, json, os @@ -354,9 +359,18 @@ try: with open(path) as f: m = json.load(f) m[os.environ["M_KEY"]] = os.environ["M_VAL"] + mode = 0o600 if m.get("run_bearer") else os.stat(path).st_mode & 0o777 tmp = path + ".tmp" - with open(tmp, "w") as f: + # The mode argument applies only to an inode this open CREATES; a .tmp left + # by an earlier crash is an existing inode at whatever mode it had. Unlink + # first, create exclusively, chmod against a narrowing umask. + try: + os.unlink(tmp) + except FileNotFoundError: + pass + with os.fdopen(os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_EXCL, mode), "w") as f: json.dump(m, f, indent=2) + os.chmod(tmp, mode) os.replace(tmp, path) finally: fcntl.flock(lock, fcntl.LOCK_UN) @@ -836,7 +850,13 @@ EOF2 # Bootstrap render: every P_* var in the environment fills the matching # {{PLACEHOLDER}}, plus the two BASE-ref manifest snapshots (capped, with -# their absent-file fallbacks). A placeholder with no P_* renders empty. +# their absent-file fallbacks). A placeholder no call site supplies is a HARD +# ERROR, never a prompt shipped with a hole in it (implement-dispatch's +# _render_bootstrap has always worked this way): rendered as a blank it reads +# to the worker as "bound to nothing", and no downstream assertion can tell +# that apart from a value that is empty by design. The check runs over the +# mode-stripped TEMPLATE, not the output — the manifest snapshots and any other +# injected content are data, and a `{{...}}` inside them is not an unfilled slot. # # The template also carries `…` blocks: the # block whose X is this run's P_REVIEW_MODE survives, every other block is @@ -863,7 +883,11 @@ subs["RISK_MANIFEST"] = readcap(os.environ["RISK_FILE"]) or \ "(no repo risk-surface manifest at .doperpowers/risk-surfaces.md — the always-on categories are the only risk surfaces)" subs["REPO_FACTS"] = readcap(os.environ["FACTS_FILE"]) or \ "(no repo-facts manifest at .doperpowers/repo-facts.md — no declared validation commands or evidence add-ons to cross-check against)" -print(re.sub(r"\{\{(\w+)\}\}", lambda m: subs.get(m.group(1), ""), t)) +missing = sorted(n for n in set(re.findall(r"\{\{(\w+)\}\}", t)) if n not in subs) +if missing: + sys.stderr.write("unrendered placeholders: %s\n" % " ".join(missing)) + sys.exit(1) +print(re.sub(r"\{\{(\w+)\}\}", lambda m: subs[m.group(1)], t)) PY } @@ -936,6 +960,21 @@ _spawn_reviewer() { # &2 return 1 fi + # Persist role: QAGENT into the registry meta. board-answer.sh's + # needs-human fallback (no recorded pre-park:) reads it back to return + # this park to in-review — a reviewer resumed into in-progress owns no + # implementation branch and has no legal exit. Non-fatal: metas written + # before this stamp are still recognized by board-answer's review-pr-* / + # review-epic-* name inference. + # + # GH MODE ONLY — this tail is shared with the api claim path, which stamps + # role itself (alongside lane and nonce) in the one write that also has to + # hold the run bearer's 0600. Two stamps racing that file buys nothing and + # risks the mode. CLAIM_JOURNAL is this file's gh/api discriminator. + if [ -z "${CLAIM_JOURNAL:-}" ]; then + _stamp_meta "$uuid" role QAGENT \ + || echo "$name: role meta write failed (non-fatal)" >&2 + fi fi if ! READY="$bind_ready" LEDGER="$ledger" UUID="$uuid" TICKET="${issue:-none}" python3 - <<'PY' import json, os @@ -1317,7 +1356,27 @@ PY # (it writes these files; we only read them). A claim is the only way to learn # WHICH ticket the server picked, so suppression can only be honored after the # fact — by handing the run straight back. -_api_suppressed() { [ -f "${BOARD_SUPPRESS_DIR:-$DAEMON_HOME/board-suppress}/$1.json" ]; } +_api_suppress_dir() { echo "${BOARD_SUPPRESS_DIR:-$DAEMON_HOME/board-suppress}"; } +_api_suppressed() { [ -f "$(_api_suppress_dir)/$1.json" ]; } + +# The same sweep's resume phase records every ticket it already attempted a +# recovery for THIS TICK. A replay that faulted leaves its ticket unowned, so +# the server can hand it to an ordinary lane claim moments later — a second +# attempt inside the one tick the ledger holds to one. Absent (a dispatcher run +# by hand, a phase asked for by name) it fences nothing. +_api_tick_ledgered() { + [ -n "${BOARD_RESUMED_LEDGER:-}" ] && [ -f "$BOARD_RESUMED_LEDGER" ] \ + && grep -qxF -- "$1" "$BOARD_RESUMED_LEDGER" +} + +# A delivered recovery is a recovery, whichever phase delivered it: the failed +# cycle count is the sweep's ladder to an env-issue escalation, and a count +# left standing after a successful dispatch escalates a much later, unrelated +# fault two rungs early. Cleared ONE LINE AHEAD of the journal's durable mark +# (_api_mark_spawned), so the only crash that can skip it is the one +# reconciliation still sees (`repaired`), which clears it there. +_api_attempts_clear() { rm -f "$(_api_suppress_dir)/.attempts-$1"; } +_claim_suppress_dir() { _api_suppress_dir; } _api_end_run() { # — best-effort release of a claimed run T_RUN="$1" T_REASON="$2" _api_py - <<'PY' || true @@ -1333,6 +1392,7 @@ PY # The handoff is done: the journal may no longer be replayed, only observed. # Called from the END of _spawn_reviewer, once the handoff is durable. _api_mark_spawned() { + [ -z "${CLAIM_TICKET:-}" ] || _api_attempts_clear "$CLAIM_TICKET" _journal_write "$CLAIM_JOURNAL" "$CLAIM_LANE" "$CLAIM_RUN" 1 \ "${CLAIM_TICKET:-}" "${CLAIM_DAEMON:-}" "${CLAIM_CONTROL:-}" } @@ -1421,6 +1481,15 @@ PY _api_drop_journal "$nonce" return 1 fi + if _api_tick_ledgered "$C_TICKET"; then + # Head-of-line, like suppression above: the server picks, so the only + # refusal available is to hand the run straight back. The next tick serves + # the ticket if it is still unowned. + echo "#$C_TICKET already had its one recovery attempt this tick — releasing run $C_RUN_ID; lane $lane stands down this tick" + _api_end_run "$C_RUN_ID" abandoned + _api_drop_journal "$nonce" + return 1 + fi local name engine tmp control_dir prompt name="$C_TICKET-api-$lane" # Ticket and daemon name are journalled BEFORE the spawn, not after it. The diff --git a/tests/claude-code/board-api/integration/drill-lib.sh b/tests/claude-code/board-api/integration/drill-lib.sh index 1a7963cbae..b1b143fada 100755 --- a/tests/claude-code/board-api/integration/drill-lib.sh +++ b/tests/claude-code/board-api/integration/drill-lib.sh @@ -196,6 +196,19 @@ import json, os, sys tid = os.environ["T_ID"] print(next((str(t["owner_run"]) for t in json.load(sys.stdin) if str(t["id"]) == tid), "(absent)"))'; } +# ---- closing an id --------------------------------------------------------- +# `t`/`nt` match with `grep -qF`, which has no anchor of its own, so an id that +# ENDS what it is printed in is satisfied by every id it prefixes: `owner=17` +# is a substring of `owner=170`, and a drill asserting the first would pass on +# the second — the right verdict for the wrong reason. Two ways to close one, +# and every drill here uses one of them: +# +# eol give each line of an emitter's output an explicit terminator, +# for text this suite does not author (an env dump, a JSON tail). +# owner_line the suite's own emitters carry their delimiter in the value. +eol() { sed 's/$/;/' "$@"; } +owner_line() { echo "owner=[$(ticket_owner "$1")]"; } + # ---- the scripted worker sessions ----------------------------------------- # No model call anywhere in this tier. The stubs stand in for the daemon layer # itself, so a drill whose claim IS about that layer would have to say so; none diff --git a/tests/claude-code/board-api/integration/test-crash-boundaries.sh b/tests/claude-code/board-api/integration/test-crash-boundaries.sh index 3af3f91637..ec63d6617b 100755 --- a/tests/claude-code/board-api/integration/test-crash-boundaries.sh +++ b/tests/claude-code/board-api/integration/test-crash-boundaries.sh @@ -53,7 +53,6 @@ register() { # register — prints the ticket id } sentinels() { printf 'sentinels=%s\n' \ "$(grep -c 'board-relay answer:' "$1" 2>/dev/null || echo 0)"; } -owner_line() { echo "owner=$(ticket_owner "$1")"; } # =========================================================================== # RELAY BOUNDARIES — one ticket, parked and answered, killed at each point. @@ -73,7 +72,7 @@ in_repo RESUME_DIE_BEFORE=1 "$SCRIPTS/board-answer.sh" "$T1" "sqlite" >"$OUT_A" t "the human's answer is recorded even though the relay dies" \ "answered #$T1 → in-progress" cat "$OUT_A" t "the failed delivery is reported, not swallowed" "returned no delivery" cat "$OUT_A" -t "an undelivered answer stays on the feed" "\"ticketId\":$T1" \ +t "an undelivered answer stays on the feed" "\"ticketId\":$T1," \ api automation GET /answers/unrelayed t "and nothing reached the worker" "sentinels=0" sentinels "$TRANSCRIPT" @@ -83,7 +82,7 @@ in_repo RESUME_DIE_AFTER=1 "$SCRIPTS/_sweep_api.sh" relay >"$OUT_B" 2>&1 || true t "a death after the injection still reports a failed resume" \ "returned no delivery" cat "$OUT_B" t "the prompt landed exactly once" "sentinels=1" sentinels "$TRANSCRIPT" -t "and the answer is STILL unacked — never ack-and-drop" "\"ticketId\":$T1" \ +t "and the answer is STILL unacked — never ack-and-drop" "\"ticketId\":$T1," \ api automation GET /answers/unrelayed OUT_B2="$DRILL_TMP/relay-b2.out" @@ -118,8 +117,8 @@ printf '{"lane": "implementer", "run_id": null, "spawn_completed": false}\n' \ OUT_D="$DRILL_TMP/dispatch-d.out" in_repo "$DISPATCH" --sweep >"$OUT_D" 2>&1 || true t "the orphaned nonce is replayed, not re-claimed" "never reached a run — replaying the claim" cat "$OUT_D" -t "and the server answers the SAME run" "BOARD_RUN_ID=$RUN_D" cat "$SPAWN_LOG" -t "so the ticket still has exactly one owner" "owner=$RUN_D" owner_line "$T2" +t "and the server answers the SAME run" "BOARD_RUN_ID=$RUN_D;" eol "$SPAWN_LOG" +t "so the ticket still has exactly one owner" "owner=[$RUN_D]" owner_line "$T2" # ---- (e) the run was claimed and never spawned ---------------------------- T3="$(register 'crash drill — claimed but never spawned')" @@ -132,7 +131,7 @@ OUT_E="$DRILL_TMP/dispatch-e.out" in_repo "$DISPATCH" --sweep >"$OUT_E" 2>&1 || true t "a run nothing local can vouch for is ended" "claimed run $RUN_E but never spawned — ending it" cat "$OUT_E" t "and the freed ticket is dispatched afresh" "claimed #$T3 run=" cat "$OUT_E" -nt "on a NEW run — the ended one never reaches a worker" "owner=$RUN_E" owner_line "$T3" +nt "on a NEW run — the ended one never reaches a worker" "owner=[$RUN_E]" owner_line "$T3" # ---- (f) spawned, and the dispatcher died before the bind ----------------- # SPAWN_KILL_PARENT makes the stub kill the dispatcher after the session is @@ -151,10 +150,10 @@ OUT_F2="$DRILL_TMP/dispatch-f2.out" in_repo "$DISPATCH" --sweep >"$OUT_F2" 2>&1 || true t "a spawned-but-unbound session is reported" "but never bound it" cat "$OUT_F2" t "and explicitly NOT ended" "is NOT being ended" cat "$OUT_F2" -t "the live run still owns its ticket" "owner=$RUN_F" owner_line "$T4" +t "the live run still owns its ticket" "owner=[$RUN_F]" owner_line "$T4" after_f="$(grep -c "SPAWN name=$T4-api-implementer" "$SPAWN_LOG" || true)" -t "and no second worker is spawned onto it" "spawns=$before_f" \ - printf 'spawns=%s\n' "$after_f" +t "and no second worker is spawned onto it" "spawns=[$before_f]" \ + printf 'spawns=[%s]\n' "$after_f" nt "no boundary case ever reached the gh CLI" "GH INVOKED" cat "$GH_LOG" diff --git a/tests/claude-code/board-api/integration/test-escalation.sh b/tests/claude-code/board-api/integration/test-escalation.sh index bb0d97fc86..f8ff8b1539 100755 --- a/tests/claude-code/board-api/integration/test-escalation.sh +++ b/tests/claude-code/board-api/integration/test-escalation.sh @@ -73,11 +73,12 @@ SQL wait_until 30 "the service to reclaim run $run (cycle $CYCLE)" \ env T_TOK="$AUTOMATION_TOKEN" T_URL="$BOARD_API_URL" T_TID="$TID" bash -c \ 'curl -s -H "authorization: Bearer $T_TOK" "$T_URL/runs/needing-resume" \ - | grep -q "\"ticketId\":$T_TID"' || exit 1 + | grep -q "\"ticketId\":$T_TID[,}]"' || exit 1 } broken_resume() { in_repo RESUME_MUST_FAIL=1 SPAWN_MUST_FAIL=1 "$SCRIPTS/_sweep_api.sh" resume; } -owner_line() { echo "owner=$(ticket_owner "$1")"; } -suppression() { cat "$DAEMON_HOME/board-suppress/$TID.json" 2>/dev/null || echo "no suppression record"; } +# `eol` because the record is JSON the drill did not author: `"env_issue": 9` +# ENDS its line, so only an explicit terminator keeps it from matching 90. +suppression() { eol "$DAEMON_HOME/board-suppress/$TID.json" 2>/dev/null || echo "no suppression record"; } # ---- cycles 1 and 2: counted, never escalated ----------------------------- arm_cycle @@ -85,7 +86,7 @@ OUT1="$DRILL_TMP/cycle1.out"; broken_resume >"$OUT1" 2>&1 || true t "cycle 1 is counted" "recovery cycle 1 of 3" cat "$OUT1" t "neither vehicle delivered" "neither vehicle delivered" cat "$OUT1" t "and the undeliverable successor is released, not left to squat" \ - "owner=None" owner_line "$TID" + "owner=[None]" owner_line "$TID" nt "nothing is escalated yet" "env-issue" cat "$OUT1" t "and no suppression record is written" "no suppression record" suppression @@ -103,24 +104,24 @@ EID="$(sed -n "s/.*env-issue #\([0-9][0-9]*\).*/\1/p" "$OUT3" | head -1)" t "the env-issue is born needs-human" "#$EID needs-human" in_repo "$SCRIPTS/board-list.sh" t "and names the stuck ticket" "ticket #$TID cannot be revived" \ in_repo "$SCRIPTS/board-list.sh" -t "the suppression record freezes the board state it stuck in" '"state": "in-progress"' suppression -t "and names the env-issue that lifts it" "\"env_issue\": $EID" suppression +t "the suppression record freezes the board state it stuck in" '"state": "in-progress",' suppression +t "and names the env-issue that lifts it" "\"env_issue\": $EID;" suppression # Automation holds no transition authority — the ticket is left where it was. t "the stuck ticket is NOT parked by automation" "in-progress" ticket_state "$TID" # ---- the suppression is honoured by both readers -------------------------- arm_cycle OUT4="$DRILL_TMP/cycle4.out"; broken_resume >"$OUT4" 2>&1 || true -t "a suppressed ticket is skipped by the resume phase" "suppressed — skipping #$TID" cat "$OUT4" +t "a suppressed ticket is skipped by the resume phase" "suppressed — skipping #$TID;" eol "$OUT4" nt "and no cycle is charged for a ticket nobody tried" "recovery cycle" cat "$OUT4" -t "so no successor is opened on it" "owner=None" owner_line "$TID" +t "so no successor is opened on it" "owner=[None]" owner_line "$TID" OUT5="$DRILL_TMP/dispatch.out" sweep dispatch >"$OUT5" 2>&1 || true t "a dispatcher that draws a suppressed ticket hands the run straight back" \ "#$TID is suppressed — releasing run" cat "$OUT5" t "and that lane stands down for the tick" "stands down this tick" cat "$OUT5" -t "leaving the ticket unowned" "owner=None" owner_line "$TID" +t "leaving the ticket unowned" "owner=[None]" owner_line "$TID" # ---- the human fixes the substrate and closes the env-issue --------------- arm_cycle diff --git a/tests/claude-code/board-api/integration/test-human-verbs.sh b/tests/claude-code/board-api/integration/test-human-verbs.sh index 31f14a1372..32d03b62b4 100755 --- a/tests/claude-code/board-api/integration/test-human-verbs.sh +++ b/tests/claude-code/board-api/integration/test-human-verbs.sh @@ -47,10 +47,12 @@ register() { # register <title> <category> <priority> [opts...] — prints the printf '%s\n' "$out" | awk 'END{print $1}' } -# The server's own projection for one ticket, as `k=v` lines — GET /tickets is +# The server's own projection for one ticket, as `k=[v]` lines — GET /tickets is # the route every one of these fields is published on, so the assertions read # what a consumer reads. A null prints as `-`: "no parent" is an ordinary -# answer and must be assertable as one. +# answer and must be assertable as one. EVERY value is bracketed, scalars +# included: `parent=[9]` cannot be satisfied by the row of a ticket parented +# under #90, which is exactly what the bare `parent=9` allowed. row() { # row <ticket> api automation GET /tickets | T_ID="$1" python3 -c ' import json, os, sys @@ -61,7 +63,7 @@ if t is None: def v(k): return "-" if t.get(k) is None else t[k] for k in ("state", "priority", "parent", "branch", "pr_url", "owner_run"): - print("%s=%s" % (k, v(k))) + print("%s=[%s]" % (k, v(k))) print("blocked_by=[%s]" % " ".join(str(b) for b in t.get("blocked_by") or [])) print("relates=[%s]" % " ".join(str(x) for x in t.get("relates") or []))' } @@ -89,6 +91,9 @@ run_rc() { # run_rc <cmd...> printf '%s\nrc=%s\n' "$out" "$rc" } +# The ticket a claim drew, delimited: `drew #1` is a substring of `drew #12`. +drew() { echo "[drew #$1]"; } + end_run() { # end_run <run-id> — the abandon path, through its own route api automation POST "/runs/$1/end" '{"reason":"abandoned"}' >/dev/null } @@ -116,8 +121,8 @@ t "and cutting one that is not is no-such-edge" "no-such-edge" \ # A is older than B and would be drawn first; blocked, it is passed over, and # with B taken the lane has nothing left to give. IFS=$'\t' read -r RUN_B TICK_B _ _ <<<"$(claim_run implementer human-verbs-blocked)" -t "the blocked leaf is passed over for the younger one" "drew #$TID_B" \ - echo "drew #$TICK_B" +t "the blocked leaf is passed over for the younger one" "[drew #$TID_B]" \ + drew "$TICK_B" t "and a blocked ticket is beyond the dispatcher's reach entirely" '"claimed":false' \ api automation POST /runs/claim \ '{"lane":"implementer","dispatchNonce":"human-verbs-blocked-probe"}' @@ -126,17 +131,17 @@ t "the unblock reports the cut" "#$TID_A: blocked_by -= #$TID_B" \ in_repo "$SCRIPTS/board-edge.sh" "$TID_A" --unblock "$TID_B" t "and the projection is empty again" "blocked_by=[]" row "$TID_A" IFS=$'\t' read -r RUN_A TICK_A _ _ <<<"$(claim_run implementer human-verbs-unblocked)" -t "the same claim now draws the ticket the edge was holding back" "drew #$TID_A" \ - echo "drew #$TICK_A" +t "the same claim now draws the ticket the edge was holding back" "[drew #$TID_A]" \ + drew "$TICK_A" end_run "$RUN_A" end_run "$RUN_B" -t "the abandoned run released the ticket" "owner_run=-" row "$TID_A" +t "the abandoned run released the ticket" "owner_run=[-]" row "$TID_A" # ---- 3. the re-grade, and its write-if-changed noop ------------------------- REGRADE="$(in_repo "$SCRIPTS/board-priority.sh" "$TID_A" P0 2>&1)" || true t "the re-grade reports the grade that committed" "#$TID_A: → P0" printf '%s\n' "$REGRADE" nt "a real change is not a noop" "(noop)" printf '%s\n' "$REGRADE" -t "and the board shows it" "priority=P0" row "$TID_A" +t "and the board shows it" "priority=[P0]" row "$TID_A" t "re-sending the standing grade writes nothing, and says so" "#$TID_A: → P0 (noop)" \ in_repo "$SCRIPTS/board-priority.sh" "$TID_A" P0 @@ -159,10 +164,10 @@ TID_E="$(register "human-verbs epic E" enhancement P2 --state ready-for-architec --body-file "$(spec e 'Epic E exists to be a parent and then to stop being one.')")" t "the reparent reports the destination" "#$TID_A: parent = #$TID_E" \ in_repo "$SCRIPTS/board-edge.sh" "$TID_A" --parent "$TID_E" -t "and the row moves with it" "parent=$TID_E" row "$TID_A" +t "and the row moves with it" "parent=[$TID_E]" row "$TID_A" t "the orphan reports the clear" "#$TID_A: parent cleared" \ in_repo "$SCRIPTS/board-edge.sh" "$TID_A" --orphan -t "and the row has no parent" "parent=-" row "$TID_A" +t "and the row has no parent" "parent=[-]" row "$TID_A" # ---- 6. the body edit, and the run that forbids it ------------------------- BODY2="$(spec a2 'The sharpened statement of work, written while nobody owned the ticket.')" @@ -172,7 +177,7 @@ t "re-sending the standing body writes nothing" "#$TID_A: body rewritten (noop)" in_repo "$SCRIPTS/board-body.sh" "$TID_A" --body-file "$BODY2" IFS=$'\t' read -r RUN_A2 TICK_A2 _ _ <<<"$(claim_run implementer human-verbs-body)" -t "the P0 ticket is what the lane draws" "drew #$TID_A" echo "drew #$TICK_A2" +t "the P0 ticket is what the lane draws" "[drew #$TID_A]" drew "$TICK_A2" BODY3="$(spec a3 'The edit that must not reach a worker holding an older assignment.')" OWNED="$(run_rc in_repo "$SCRIPTS/board-body.sh" "$TID_A" --body-file "$BODY3")" # The body IS the claim-time assignment: an edit under an open run would reach @@ -187,7 +192,7 @@ t "once the run ends, the same edit commits" "#$TID_A: body rewritten" \ SPEC7="$(spec park 'The statement of work a park birth keeps while its question stands.')" TID_P="$(register "human-verbs park birth" enhancement P2 --state needs-human \ --note "which database?" --body-file "$SPEC7")" -t "the birth parks the ticket" "state=needs-human" row "$TID_P" +t "the birth parks the ticket" "state=[needs-human]" row "$TID_P" t "the note is the standing question, verbatim" "question=which database?" \ park_question "$TID_P" # There is no body read route, so the write-if-changed noop IS the read: a @@ -203,7 +208,7 @@ SPIKE="$(in_repo "$SCRIPTS/board-register.sh" "human-verbs spike" spike P2 \ --body-file "$(spec spike 'A spike whose first act is a design pass.')" 2>&1)" || true t "the spike is born" "$BOARD_API_URL/tickets/" printf '%s\n' "$SPIKE" TID_S="$(printf '%s\n' "$SPIKE" | awk 'END{print $1}')" -t "into the architect queue, as R2 rules" "state=ready-for-architect" row "$TID_S" +t "into the architect queue, as R2 rules" "state=[ready-for-architect]" row "$TID_S" # The client used to warn that this birth diverged from the server; arkho#7 # closed that gap, and the retired warning must not have survived it. nt "and carries no divergence note" "arkho/issues/7" printf '%s\n' "$SPIKE" @@ -213,13 +218,13 @@ TID_L="$(register "human-verbs leaf close" enhancement P1 \ --body-file "$(spec l 'The leaf that has to reach done through the review lane.')")" t "the leaf goes in-flight with its working ref" "#$TID_L: → in-progress" \ in_repo "$SCRIPTS/board-transition.sh" "$TID_L" in-progress --branch feat/human-verbs -t "and the branch is recorded on the row" "branch=feat/human-verbs" row "$TID_L" +t "and the branch is recorded on the row" "branch=[feat/human-verbs]" row "$TID_L" t "the artifact carries it into review" "#$TID_L: → in-review" \ in_repo "$SCRIPTS/board-transition.sh" "$TID_L" in-review --pr https://example.test/pr/9 -t "with the PR url on the row" "pr_url=https://example.test/pr/9" row "$TID_L" +t "with the PR url on the row" "pr_url=[https://example.test/pr/9]" row "$TID_L" IFS=$'\t' read -r RUN_L TICK_L FENCE_L BEARER_L <<<"$(claim_run qagent human-verbs-close)" -t "the review lane draws the leaf" "drew #$TID_L" echo "drew #$TICK_L" +t "the review lane draws the leaf" "[drew #$TID_L]" drew "$TICK_L" # THE CLOSE IS THE REVIEW RUN'S. A1 grants `in-review → done` to a worker only # on the qagent lane and only with a URL-shaped pr_url (a numeric one is an # epic's closure package) — which is exactly the path R1 said this fork's @@ -227,7 +232,7 @@ t "the review lane draws the leaf" "drew #$TID_L" echo "drew #$TICK_L" t "and the run that claimed it closes it" "#$TID_L: → done" \ in_repo BOARD_RUN_TOKEN="$BEARER_L" BOARD_RUN_ID="$RUN_L" BOARD_RUN_FENCE="$FENCE_L" \ "$SCRIPTS/board-transition.sh" "$TID_L" "done" -t "the board shows the leaf closed" "state=done" row "$TID_L" +t "the board shows the leaf closed" "state=[done]" row "$TID_L" # Every verb above went over the API. The gh CLI has no business on this path. nt "the gh CLI is never invoked across the whole flow" "GH INVOKED" cat "$GH_LOG" diff --git a/tests/claude-code/board-api/integration/test-protocol-walk.sh b/tests/claude-code/board-api/integration/test-protocol-walk.sh index 1f4eeb5326..e768e5e5d6 100755 --- a/tests/claude-code/board-api/integration/test-protocol-walk.sh +++ b/tests/claude-code/board-api/integration/test-protocol-walk.sh @@ -70,13 +70,12 @@ bind_confirmed() { T_P="$DAEMON_HOME/$UUID.json" python3 -c ' import json, os print("bind_confirmed=%s" % json.load(open(os.environ["T_P"])).get("bind_confirmed"))'; } t "the bind is confirmed by the SERVER, not assumed locally" "bind_confirmed=True" bind_confirmed -owner_line() { echo "owner=$(ticket_owner "$1")"; } -t "the server records the run as the ticket's owner" "owner=$RUN" owner_line "$TID" +t "the server records the run as the ticket's owner" "owner=[$RUN]" owner_line "$TID" # OWNER EXCLUSIVITY, server-side: a second dispatch tick finds nothing to take. OUT_D2="$DRILL_TMP/dispatch2.out" in_repo "$DISPATCH" --sweep >"$OUT_D2" 2>&1 || true -nt "an owned ticket is not claimed a second time" "claimed #$TID" cat "$OUT_D2" +nt "an owned ticket is not claimed a second time" "claimed #$TID run=" cat "$OUT_D2" t "a bare claim on the lane answers empty" '"claimed":false' \ api automation POST /runs/claim '{"lane":"implementer","dispatchNonce":"walk-exclusivity-probe"}' @@ -93,8 +92,8 @@ t "a write carrying the WRONG fence is refused" "fence-mismatch" \ "$SCRIPTS/board-transition.sh" "$TID" needs-human "wrong fence" t "the park is written" "#$TID: → needs-human" \ worker "$SCRIPTS/board-transition.sh" "$TID" needs-human "which db?" -t "the park keeps its run bound — it is a pause, not a death" "owner=$RUN" owner_line "$TID" -t "and the question reaches the decisions queue" "\"ticket_id\":$TID" \ +t "the park keeps its run bound — it is a pause, not a death" "owner=[$RUN]" owner_line "$TID" +t "and the question reaches the decisions queue" "\"ticket_id\":$TID," \ api human GET /queue/decisions # THE PARK IS THE HUMAN'S TO ANSWER — and nothing local enforces that. diff --git a/tests/claude-code/board-api/integration/test-resume-first.sh b/tests/claude-code/board-api/integration/test-resume-first.sh index 4b020a8310..0f2f22192f 100755 --- a/tests/claude-code/board-api/integration/test-resume-first.sh +++ b/tests/claude-code/board-api/integration/test-resume-first.sh @@ -60,7 +60,8 @@ update board.run set lease_expires_at = now() - interval '5 minutes', last_write SQL feed_has_stuck() { needing_resume_ids | grep -qxF "$STUCK"; } wait_until 30 "the service to reclaim run $RUN" feed_has_stuck || exit 1 -t "the reclaimed ticket is on the resume feed" "$STUCK" needing_resume_ids +resume_feed() { needing_resume_ids | eol; } +t "the reclaimed ticket is on the resume feed" "$STUCK;" resume_feed t "and the board shows it unowned, still in flight" "in-progress" ticket_state "$STUCK" # ---- the fresh ticket waiting behind it ----------------------------------- @@ -92,15 +93,17 @@ t "the resume is delivered BEFORE any fresh worker is started" \ # ---- and the successor is a real successor --------------------------------- SUCC_FENCE=$((FENCE + 1)) -t "the successor carries fence + 1" "BOARD_RUN_FENCE=$SUCC_FENCE" cat "$RESUME_LOG" -nt "on a run that is not the reclaimed one" "BOARD_RUN_ID=$RUN" cat "$RESUME_LOG" +# Each env line is closed by `eol`: without a terminator `BOARD_RUN_ID=7` is a +# substring of `BOARD_RUN_ID=70`, and the `nt` below — the assertion that the +# successor is NOT the reclaimed run — would fail for that wrong reason. +t "the successor carries fence + 1" "BOARD_RUN_FENCE=$SUCC_FENCE;" eol "$RESUME_LOG" +nt "on a run that is not the reclaimed one" "BOARD_RUN_ID=$RUN;" eol "$RESUME_LOG" t "the successor is told to read its own timeline first" \ "board-show.sh $STUCK" cat "$PROJECTS/$UUID.jsonl" # The successor run id, read off the tick's own report, so the ownership # assertion below names the run this tick actually opened. SUCC_RUN="$(sed -n "s/^resume: #$STUCK run \([0-9][0-9]*\) .*/\1/p" "$OUT" | head -1)" -owner_line() { echo "owner=$(ticket_owner "$1")"; } -t "and the board hands the ticket to that successor" "owner=$SUCC_RUN" owner_line "$STUCK" +t "and the board hands the ticket to that successor" "owner=[$SUCC_RUN]" owner_line "$STUCK" t "with the ticket still in flight — a resume is not a re-queue" \ "in-progress" ticket_state "$STUCK" diff --git a/tests/claude-code/board-api/integration/test-transcript-diff.sh b/tests/claude-code/board-api/integration/test-transcript-diff.sh index c96860cb17..e02e3d3fcf 100755 --- a/tests/claude-code/board-api/integration/test-transcript-diff.sh +++ b/tests/claude-code/board-api/integration/test-transcript-diff.sh @@ -11,12 +11,21 @@ # statement makes ("same scripts, same arguments, same refusal vocabulary"), # and pins the residue: # -# STRICT, no allowance every step's ARGV is identical, and every step's -# EXIT STATUS is identical — the two bindings agree, -# step for step, on what is legal and what is -# refused. This is the drift fence that matters: a -# second client-side copy of the legality table -# (the X5 hazard) shows up here first. +# STRICT, no allowance every step's EXIT STATUS is identical — the two +# bindings agree, step for step, on what is legal +# and what is refused. THIS is the drift fence that +# matters: a second client-side copy of the legality +# table (the X5 hazard) shows up here first, as an +# rc the other side does not answer with. +# The argv is compared too, but read what that buys: +# both walks iterate ONE `STEPS` array, so the argv +# is the drill's own input rather than either +# binding's output, and the comparison can only +# catch a capture that lost or misaligned a step. It +# was never a behavioral fence — before the compared +# form became the unsubstituted one it differed only +# by each walk's ticket id, which is a way to fail +# falsely, not a way to catch drift. # NORMALIZED, pinned what each step PRINTS, with transport tokens # erased. Every surviving difference must be named # in transcript-compare.py's PINNED table with the @@ -85,6 +94,15 @@ STEPS=( # walk <repo> <capture-file> <bind-hook> — runs every step, recording argv, # exit status and the merged output stream (a refusal is worker-visible too). +# The argv is recorded TWICE: as executed, and as written above with `%T` still +# in it. The second is what the comparator compares — the two walks register +# their own tickets and the ids are never equal, so an executed-argv comparison +# would either always fail or have to normalize digits away, and normalizing +# digits would also erase step 6's deliberate literal 4242 (the known-ticket / +# unknown-ticket distinction that step exists to probe). What that comparison +# is NOT is a fence on binding behavior: both sides read the same `STEPS`, so +# it can only catch a capture that dropped or misaligned a step. The exit +# status is the fence (see the header). # The hook runs once, after the register, with the new ticket id: it is where # each binding's dispatcher would have bound a worker to the ticket, and the # park at step 7 needs that binding to be a PAUSE rather than a disposition. @@ -107,11 +125,16 @@ walk() { DAEMON_SCRIPTS="$DAEMON_SCRIPTS" BOARD_CREDENTIALS_FILE="$BOARD_CREDENTIALS_FILE" \ BOARD_REPO="$GH_STUB_REPO" GH_STUB_STATE="$GH_STUB_STATE" \ "$SCRIPTS/${call[0]}" "${call[@]:1}") 2>&1 )" || rc=$? + # `argv` is FORENSIC — what actually ran, for reading a failure back. The + # comparator reads `argv_raw` and nothing reads `argv`; wiring an assertion + # to it would be asserting on the ticket id the two walks cannot share. T_I="$i" T_RC="$rc" T_OUT="$out" T_ARGV="$(printf '%s\n' "${call[@]}")" \ + T_ARGV_RAW="$(printf '%s\n' "${argv[@]}")" \ python3 -c ' import json, os print(json.dumps({"i": int(os.environ["T_I"]), "rc": int(os.environ["T_RC"]), "argv": os.environ["T_ARGV"].splitlines(), + "argv_raw": os.environ["T_ARGV_RAW"].splitlines(), "out": os.environ["T_OUT"]}))' >>"$cap" if [ "$i" -eq 1 ]; then tid="$(printf '%s\n' "$out" | awk 'END{print $1}')" @@ -148,7 +171,19 @@ PY } GH_CAP="$DRILL_TMP/capture-gh.jsonl" GH_TID="$(walk "$GH_REPO" "$GH_CAP" gh_bind)" -t "the gh-mode walk registered a ticket" "1" printf '%s\n' "$GH_TID" +# Nothing below is meaningful without an id, so the empty case dies here rather +# than failing an assertion further down. What the id IS gets asserted against +# the BOARD that holds it, the way the API half asserts through `ticket_state` +# — the want that stood here was the substring `1`, satisfied by every id +# containing a 1 and by a walk that printed nothing but the number. +[ -n "$GH_TID" ] || { echo "FAIL $(basename "$0") — the gh-mode walk registered no ticket"; exit 1; } +gh_ticket() { T_S="$GH_STUB_STATE" T_ID="$1" python3 -c ' +import json, os +it = json.load(open(os.environ["T_S"]))["issues"].get(os.environ["T_ID"]) +print("#%s %s" % (os.environ["T_ID"], "(no such issue on the stub board)" + if it is None else it["title"]))'; } +t "the gh-mode walk registered its ticket on the stub board" \ + "#$GH_TID transcript diff walk" gh_ticket "$GH_TID" # ---- side B: API mode, on the real service --------------------------------- # The gh stub stays on PATH and stays armed: nothing in this half may touch it. @@ -209,12 +244,21 @@ for line in open(sys.argv[1]): if r["i"] == want: print(r["out"]) break' "$1"; } +step_line() { step_out "$1" "$2" | eol; } for cap in "$GH_CAP" "$API_CAP"; do side=gh; [ "$cap" = "$GH_CAP" ] || side=api - t "the $side refusal names the unknown ticket" "4242" step_out "$cap" 6 t "the $side refusal names the illegal edge itself" \ "in-progress → ready-for-implementer" step_out "$cap" 9 done +# THE UNKNOWN TICKET, per side. A bare `4242` is satisfied by the number turning +# up anywhere in either transcript, so each half pins the phrase it actually +# prints around the id — gh resolves the ticket against its snapshot and says so +# (the id ends the line, hence `eol`), the API names the route it refused. The +# refusal IDENTIFIER is deliberately left unpinned here: which code stands behind +# each sentence is step 6's pinned divergence, not this assertion's claim. +t "the gh refusal names the unknown ticket" "unknown issue: #4242;" step_line "$GH_CAP" 6 +t "the API refusal names the unknown ticket" "POST /tickets/4242/transition refused:" \ + step_out "$API_CAP" 6 # The PR requirement is the one refusal whose SENTENCE does not match: gh mode # explains the requirement and its destination in prose, while the API answers # with the contract identifier and then the server's own account of the edge. diff --git a/tests/claude-code/board-api/integration/transcript-compare.py b/tests/claude-code/board-api/integration/transcript-compare.py index 5098aa22e4..36d2e94c8f 100755 --- a/tests/claude-code/board-api/integration/transcript-compare.py +++ b/tests/claude-code/board-api/integration/transcript-compare.py @@ -1,16 +1,28 @@ #!/usr/bin/env python3 """transcript-compare.py — the transcript-diff drill's judge. -Reads two capture files (one step per line, JSON: {i, argv, rc, out}) produced -by running THE SAME verb sequence under the two bindings, and reports, per -step, whether the worker-visible surface is the same. +Reads two capture files (one step per line, JSON: {i, argv, argv_raw, rc, out}) +produced by running THE SAME verb sequence under the two bindings, and reports, +per step, whether the worker-visible surface is the same. Two levels, and the split is the whole point: - STRICT the argv and the exit status must match, step for step. "Same - scripts, same arguments, same refusal vocabulary" (spec § Purpose) - begins here: if the two bindings disagree about whether a call is - legal, nothing downstream is comparable. + STRICT the EXIT STATUS must match, step for step. "Same scripts, same + arguments, same refusal vocabulary" (spec § Purpose) begins here: if + the two bindings disagree about whether a call is legal, nothing + downstream is comparable. + + The argv is compared as well, but it is a CAPTURE-INTEGRITY check, + not a behavioral one: both walks iterate one shared step list, so + the argv is the drill's input rather than either binding's output, + and the only thing this can catch is a capture that dropped or + misaligned a step. The form compared is `argv_raw` — the step as + WRITTEN, `%T` unsubstituted — because each walk registers its own + ticket and the two ids are never equal; comparing the executed argv + only ever added a way to fail falsely. The alternative, normalizing + digits out of the executed argv, would also erase step 6's literal + 4242, and that step is precisely the known ticket / unknown ticket + distinction. NORMALIZED stdout/stderr is compared after transport tokens are erased — urls, timestamps, ids, session uuids. What survives that is content, @@ -100,8 +112,9 @@ def main(gh_path, api_path): print("STEP %d MISSING (gh=%s api=%s)" % (i, g is not None, a is not None)) bad += 1 continue - if g["argv"] != a["argv"]: - print("STEP %d ARGV-MISMATCH\n gh : %s\n api: %s" % (i, g["argv"], a["argv"])) + if g["argv_raw"] != a["argv_raw"]: + print("STEP %d ARGV-MISMATCH\n gh : %s\n api: %s" + % (i, g["argv_raw"], a["argv_raw"])) bad += 1 continue if g["rc"] != a["rc"]: @@ -112,7 +125,7 @@ def main(gh_path, api_path): same = norm(g["out"]) == norm(a["out"]) pin = PINNED.get(i) if same and not pin: - print("STEP %d IDENTICAL %s" % (i, " ".join(g["argv"]))) + print("STEP %d IDENTICAL %s" % (i, " ".join(g["argv_raw"]))) elif same and pin: print("STEP %d PINNED-BUT-IDENTICAL(%s) — the table is stale, drop the entry" % (i, pin[0])) @@ -121,7 +134,7 @@ def main(gh_path, api_path): print("STEP %d DIVERGES(%s) — %s" % (i, pin[0], pin[1])) else: print("STEP %d UNEXPECTED-DIVERGENCE %s\n gh : %s\n api: %s" - % (i, " ".join(g["argv"]), norm(g["out"]), norm(a["out"]))) + % (i, " ".join(g["argv_raw"]), norm(g["out"]), norm(a["out"]))) bad += 1 print("VERDICT: %s (%d step(s) unaccounted for)" % ("accounted" if not bad else "UNACCOUNTED", bad)) diff --git a/tests/claude-code/board-api/test-dispatch-claim.sh b/tests/claude-code/board-api/test-dispatch-claim.sh index 808d27c996..337154155d 100755 --- a/tests/claude-code/board-api/test-dispatch-claim.sh +++ b/tests/claude-code/board-api/test-dispatch-claim.sh @@ -113,6 +113,11 @@ wait_for_port "$PORT2" || { echo "FAIL mock server never listened on $PORT2"; ex r="$(apirepo "$PORT")" DH="$(mktemp -d)" # pinned: a fall-through would read the operator's registry OUT="$(mktemp)" +# A standing failed-cycle count for the ticket this claim will yield. Only the +# sweep's own resume reset it, so a ticket the DISPATCHER recovered kept the +# stale count and a much later, unrelated fault escalated early. A delivered +# recovery is a recovery, whichever phase delivered it. +mkdir -p "$DH/board-suppress"; echo 2 > "$DH/board-suppress/.attempts-12" # A board-scripts overlay whose board-bind.sh SNAPSHOTS the claim journal at the # instant it runs and then execs the real one. Ordering is only observable from # INSIDE that window — after the fact every order looks the same. @@ -138,6 +143,8 @@ chmod +x "$BSOVL/board-bind.sh" t "the granted claim is reported" "claimed #12 run=41 lane=architect" cat "$OUT" nt "api dispatch never invokes gh" "GH-CALLED" cat "$MARKER" +attempts12() { [ -e "$DH/board-suppress/.attempts-12" ] && echo "count kept" || echo "count cleared"; } +t "a delivered dispatch clears the ticket's failed-cycle count" "count cleared" attempts12 # --- the worker's environment: the run credentials it cannot run without ---- t "worker got the run bearer" "BOARD_RUN_TOKEN=tok-w" cat "$DH/spawn-capture.txt" @@ -248,8 +255,14 @@ printf '{"lane": "implementer", "run_id": 99, "spawn_completed": false}\n' \ printf 'orphaned assignment\n' > "$DH2/board-claims/nonce-b.body.md" # (c) the spawn DID complete — its worker is right there in the registry — # and only the marker write was lost. -printf '{"lane": "architect", "run_id": 41, "spawn_completed": false}\n' \ +printf '{"lane": "architect", "run_id": 41, "spawn_completed": false, "ticket": "12"}\n' \ > "$DH2/board-claims/nonce-c.json" +# ...and the delivery it confirms is where the ticket's failed-cycle count is +# cleared when the inline reset never ran. The reset sits one line ahead of the +# marker write, so THIS crash — bind landed, marker lost — is exactly the +# window that leaves a durable recovery beside a stale counter, and a much +# later unrelated fault would then escalate two rungs early. +mkdir -p "$DH2/board-suppress"; echo 2 > "$DH2/board-suppress/.attempts-12" printf '{"uuid":"cccc0001","current":"cccc0001","name":"12-api-architect","status":"working","run_id":41,"lane":"architect","ticket":"12"}' \ > "$DH2/cccc0001.json" # (d) THE SPAWN LANDED, THE BIND DID NOT — a crash inside daemon-spawn uuid @@ -306,6 +319,9 @@ gone() { [ -e "$1" ] && echo "still-there" || echo "gone"; } t "the stranded journal is dropped" "gone" gone "$DH2/board-claims/nonce-b.json" t "so is its orphaned assignment body" "gone" gone "$DH2/board-claims/nonce-b.body.md" t "a lost marker is repaired, not replayed" '"spawn_completed": true' cat "$DH2/board-claims/nonce-c.json" +attempts12r() { [ -e "$DH2/board-suppress/.attempts-12" ] && echo "count kept" || echo "count cleared"; } +t "and the delivery it confirms clears the ticket's failed-cycle count" \ + "count cleared" attempts12r # --- (d) a spawned-but-unbound run is never ended -------------------------- nt "a live unbound run is NOT ended" '"path": "/runs/77/end"' cat "$FIX2.log" t "its journal is kept, closed to replay" '"spawn_completed": true' \ @@ -549,4 +565,191 @@ OUT6C="$(mktemp)" SWEEP6 BOARD_TICK_DEADLINE=not-a-number > "$OUT6C" 2>&1 || true t "an unparseable deadline is no gate either" '"path": "/runs/claim"' cat "$FIX6.log" +# ========================================================================= +# Scenario 7 — ONE RECOVERY ATTEMPT PER TICKET PER TICK REACHES THIS PHASE +# TOO. The sweep's resume phase records every ticket it attempted a recovery +# for this tick; phase 4 hands that ledger across the same way it hands the +# suppression directory. Without it an ordinary lane claim could pick the very +# ticket whose replay just faulted — a second attempt in the same tick, over +# the invariant the ledger exists to hold. The claim is how we learn WHICH +# ticket the server picked, so the refusal is a release, as suppression's is. +# ========================================================================= +PORT7="$(free_port)" +FIX7="$(mktemp)"; : > "$FIX7.log" +cat > "$FIX7" <<'JSON' +[ + {"method":"POST","path":"/runs/claim","status":200,"once":true, + "body":{"runId":71,"ticketId":33,"fence":1,"bearer":"tok-l","plan":null, + "body":"ledgered work","parentPin":null}}, + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}}, + {"method":"POST","path":"/runs/71/end","status":200,"body":{"ended":true}}, + {"method":"POST","path":"/runs/71/bind","status":200,"body":{"bound":true}} +] +JSON +python3 "$TESTS_DIR/mock-server.py" "$FIX7" "$PORT7" & MOCK7=$! +trap 'kill $MOCK $MOCK2 $MOCK3 $MOCK4 $MOCK5 $MOCK6 $MOCK7 2>/dev/null' EXIT +wait_for_port "$PORT7" || { echo "FAIL mock server never listened on $PORT7"; exit 1; } + +r7="$(apirepo "$PORT7")" +DH7="$(mktemp -d)" +LEDGER7="$(mktemp)"; echo 33 > "$LEDGER7" +OUT7="$(mktemp)" +( cd "$r7" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" \ + DAEMON_HOME="$DH7" DAEMON_SCRIPTS="$DS" LOCAL_REPO="$r7" \ + BOARD_CREDENTIALS_FILE="$CREDS" BOARD_RESUMED_LEDGER="$LEDGER7" \ + "$DISPATCH" --sweep ) > "$OUT7" 2>&1 || true + +t "a claim yielding a tick-ledgered ticket is released" \ + "#33 already had its one recovery attempt this tick" cat "$OUT7" +t "and that run is ended" '"path": "/runs/71/end"' cat "$FIX7.log" +nt "so no worker is spawned for it" "ARGS name=33" \ + bash -c "cat '$DH7/spawn-capture.txt' 2>/dev/null || echo none" +journal7() { ls "$DH7/board-claims"/*.json >/dev/null 2>&1 && echo "journal kept" || echo "journal dropped"; } +t "and the journal is dropped with it" "journal dropped" journal7 + +# ========================================================================= +# Scenario 8 — THE RESET LANDS BEFORE THE SEAL. Reconciliation's `repaired` +# arm marks the journal spawn_completed, and a sealed journal is skipped by +# every later pass — so anything left to do AFTER that write is done once or +# never. With the failed-cycle reset on the far side of it, a crash in +# between left a durable recovery beside a stale counter with nothing that +# could ever revisit it: the early-escalation bug, one window later. +# Reset-then-seal is crash-safe instead — both steps are idempotent, and a +# crash between them leaves the journal open for the next pass to redo both. +# The seal is made to FAIL here (read-only journal), which is the same +# window: the reset must already be durable when the write does not land. +# ========================================================================= +PORT8="$(free_port)" +FIX8="$(mktemp)"; : > "$FIX8.log" +cat > "$FIX8" <<'JSON' +[ + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}} +] +JSON +python3 "$TESTS_DIR/mock-server.py" "$FIX8" "$PORT8" & MOCK8=$! +trap 'kill $MOCK $MOCK2 $MOCK3 $MOCK4 $MOCK5 $MOCK6 $MOCK7 $MOCK8 2>/dev/null' EXIT +wait_for_port "$PORT8" || { echo "FAIL mock server never listened on $PORT8"; exit 1; } + +r8="$(apirepo "$PORT8")" +DH8="$(mktemp -d)"; mkdir -p "$DH8/board-claims" "$DH8/board-suppress" +printf '{"uuid":"eeee0001","current":"eeee0001","name":"15-api-implementer","status":"working","run_id":43,"lane":"implementer","ticket":"15"}' \ + > "$DH8/eeee0001.json" +printf '{"lane": "implementer", "run_id": 43, "spawn_completed": false, "ticket": "15"}\n' \ + > "$DH8/board-claims/nonce-i.json" +chmod 444 "$DH8/board-claims/nonce-i.json" +echo 2 > "$DH8/board-suppress/.attempts-15" +OUT8="$(mktemp)" +( cd "$r8" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" \ + DAEMON_HOME="$DH8" DAEMON_SCRIPTS="$DS" LOCAL_REPO="$r8" \ + BOARD_CREDENTIALS_FILE="$CREDS" "$DISPATCH" --sweep ) > "$OUT8" 2>&1 || true +chmod 644 "$DH8/board-claims/nonce-i.json" + +t "a seal that never lands still leaves the count cleared" "count cleared" \ + bash -c "[ -e '$DH8/board-suppress/.attempts-15' ] && echo 'count kept' || echo 'count cleared'" +t "and the unsealed journal is left for the next pass to redo" \ + '"spawn_completed": false' cat "$DH8/board-claims/nonce-i.json" + +# ========================================================================= +# Scenario 9 — A RESET THAT FAILED IS NOT A RESET. Absent is the one benign +# outcome (nothing to clear); every other removal error — a read-only +# registry, a permission change, a full or unmounted volume — means the count +# is still standing, and sealing the journal on top of it hides that from +# every later pass. So the seal is skipped and the journal stays open: the +# next pass retries the pair, both steps being idempotent. +# ========================================================================= +PORT9="$(free_port)" +FIX9="$(mktemp)"; : > "$FIX9.log" +cat > "$FIX9" <<'JSON' +[ + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}} +] +JSON +python3 "$TESTS_DIR/mock-server.py" "$FIX9" "$PORT9" & MOCK9=$! +trap 'kill $MOCK $MOCK2 $MOCK3 $MOCK4 $MOCK5 $MOCK6 $MOCK7 $MOCK8 $MOCK9 2>/dev/null' EXIT +wait_for_port "$PORT9" || { echo "FAIL mock server never listened on $PORT9"; exit 1; } + +r9="$(apirepo "$PORT9")" +DH9="$(mktemp -d)"; mkdir -p "$DH9/board-claims" "$DH9/board-suppress" +printf '{"uuid":"eeee0002","current":"eeee0002","name":"16-api-implementer","status":"working","run_id":44,"lane":"implementer","ticket":"16"}' \ + > "$DH9/eeee0002.json" +printf '{"lane": "implementer", "run_id": 44, "spawn_completed": false, "ticket": "16"}\n' \ + > "$DH9/board-claims/nonce-j.json" +echo 2 > "$DH9/board-suppress/.attempts-16" +chmod 555 "$DH9/board-suppress" # an unlink needs write on the DIRECTORY +OUT9="$(mktemp)" +( cd "$r9" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" \ + DAEMON_HOME="$DH9" DAEMON_SCRIPTS="$DS" LOCAL_REPO="$r9" \ + BOARD_CREDENTIALS_FILE="$CREDS" "$DISPATCH" --sweep ) > "$OUT9" 2>&1 || true +chmod 755 "$DH9/board-suppress" + +t "a failed reset leaves the journal open for the next pass" \ + '"spawn_completed": false' cat "$DH9/board-claims/nonce-j.json" +t "and says so, naming the ticket whose count still stands" \ + "failed-cycle reset failed" cat "$OUT9" +t "the count is still there to be retried" "count kept" \ + bash -c "[ -e '$DH9/board-suppress/.attempts-16' ] && echo 'count kept' || echo 'count cleared'" + +# ========================================================================= +# Scenario 10 — ENOENT ANSWERS TWO OPPOSITE QUESTIONS. os.remove says "no +# such file" both when the counter is already gone (benign: the reset is +# finished) and when the DIRECTORY it lives in cannot be seen at all — an +# operator BOARD_SUPPRESS_DIR whose volume unmounted, a path that moved. +# Sealing on the second reading is the same bug as swallowing EACCES: the +# mount returns carrying the stale count, and the journal that would have +# cleared it is closed forever. +# +# The two are told apart by REACHABILITY, not by the directory alone. The +# sweep creates that directory the first time it counts anything, so on a +# registry that has never had a failed cycle it legitimately does not exist — +# the common, healthy case, pinned second below. An absent directory whose +# PARENT is also gone is a path this process cannot see, where absence proves +# nothing. +# ========================================================================= +PORT10="$(free_port)" +FIX10="$(mktemp)"; : > "$FIX10.log" +cat > "$FIX10" <<'JSON' +[ + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}} +] +JSON +python3 "$TESTS_DIR/mock-server.py" "$FIX10" "$PORT10" & MOCK10=$! +trap 'kill $MOCK $MOCK2 $MOCK3 $MOCK4 $MOCK5 $MOCK6 $MOCK7 $MOCK8 $MOCK9 $MOCK10 2>/dev/null' EXIT +wait_for_port "$PORT10" || { echo "FAIL mock server never listened on $PORT10"; exit 1; } +r10="$(apirepo "$PORT10")" + +mkjrepaired() { # mkjrepaired <registry> <ticket> <run> — one repaired-shaped journal + mkdir -p "$1/board-claims" + printf '{"uuid":"aaaa%s","current":"aaaa%s","name":"%s-api-implementer","status":"working","run_id":%s,"lane":"implementer","ticket":"%s"}' \ + "$2" "$2" "$2" "$3" "$2" > "$1/aaaa$2.json" + printf '{"lane": "implementer", "run_id": %s, "spawn_completed": false, "ticket": "%s"}\n' \ + "$3" "$2" > "$1/board-claims/nonce-$2.json" +} +SWEEP10() { # SWEEP10 <registry> [env...] — one dispatch tick against this board + local dh="$1"; shift + ( cd "$r10" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" \ + DAEMON_HOME="$dh" DAEMON_SCRIPTS="$DS" LOCAL_REPO="$r10" \ + BOARD_CREDENTIALS_FILE="$CREDS" "$@" "$DISPATCH" --sweep ) +} + +# (a) the suppression path is UNREACHABLE — its parent does not exist either. +DHA10="$(mktemp -d)"; mkjrepaired "$DHA10" 17 45 +OUTA10="$(mktemp)" +SWEEP10 "$DHA10" BOARD_SUPPRESS_DIR="$DHA10/gone-volume/board-suppress" \ + > "$OUTA10" 2>&1 || true +t "an unreachable suppression path does not seal the journal" \ + '"spawn_completed": false' cat "$DHA10/board-claims/nonce-17.json" +t "and reports the reset it could not make" \ + "failed-cycle reset failed" cat "$OUTA10" + +# (b) ...and the registry that simply never counted anything still seals. The +# sweep creates that directory on demand, so its absence under a live +# registry is the healthy default, not a fault — reading it as one would +# stall every repair on every fleet that has had no failed cycle. +DHB10="$(mktemp -d)"; mkjrepaired "$DHB10" 18 46 +OUTB10="$(mktemp)" +SWEEP10 "$DHB10" > "$OUTB10" 2>&1 || true +t "a registry with no suppression directory repairs normally" \ + '"spawn_completed": true' cat "$DHB10/board-claims/nonce-18.json" +nt "and reports no failure" "failed-cycle reset failed" cat "$OUTB10" + finish diff --git a/tests/claude-code/board-api/test-review-dispatch-claim.sh b/tests/claude-code/board-api/test-review-dispatch-claim.sh index 559d8773d4..ca314a7743 100755 --- a/tests/claude-code/board-api/test-review-dispatch-claim.sh +++ b/tests/claude-code/board-api/test-review-dispatch-claim.sh @@ -143,6 +143,11 @@ wait_for_port "$PORT2" || { echo "FAIL mock server never listened on $PORT2"; ex r="$(apirepo "$PORT")" DH="$(mktemp -d)" # pinned: a fall-through would read the operator's registry OUT="$(mktemp)" +# A standing failed-cycle count for the ticket this claim will yield. Only the +# sweep's own resume reset it, so a ticket the DISPATCHER recovered kept the +# stale count and a much later, unrelated fault escalated early. A delivered +# recovery is a recovery, whichever phase delivered it. +mkdir -p "$DH/board-suppress"; echo 2 > "$DH/board-suppress/.attempts-9" # BOARD_RUN_TOKEN is set on the way in ON PURPOSE: a --sweep launched from a # worker's own shell inherits that worker's run bearer, and the client hands # BOARD_RUN_TOKEN back for ANY principal once it is in env — so every claim @@ -157,6 +162,8 @@ OUT="$(mktemp)" t "the granted claim is reported" "claimed #9 run=51 lane=qagent" cat "$OUT" nt "api review dispatch never invokes gh" "GH-CALLED" cat "$MARKER" +attempts9() { [ -e "$DH/board-suppress/.attempts-9" ] && echo "count kept" || echo "count cleared"; } +t "a delivered dispatch clears the ticket's failed-cycle count" "count cleared" attempts9 # --- the worker's environment: the run credentials it cannot review without - t "worker got the run bearer" "BOARD_RUN_TOKEN=tok-q" cat "$DH/spawn-capture.txt" @@ -223,6 +230,15 @@ t "the meta records the ticket" '"ticket": "9"' meta # Without the bearer at rest every later relay/resume of this reviewer has no # token to speak with — the sweep reads it back out of exactly this field. t "the bearer is stored at rest" '"run_bearer": "tok-q"' meta +# ...and stored 0600, the way board-bind wrote it. Every bookkeeping stamp that +# touches this meta afterwards has to hold that line: one rewrite at the umask +# default republishes the run bearer world-readable, and the api path's own +# stamp then faithfully preserves the widened mode. +meta_mode() { + python3 -c 'import glob, os, sys +print("%o" % (os.stat(glob.glob(sys.argv[1])[0]).st_mode & 0o777))' "$DH/bbbb0001-*.json" +} +t "the bearer meta is not world-readable" "600" meta_mode barrier() { local f; f="$(find "$DH" -name bind-ready.json -type f -print | head -1)" [ -n "$f" ] || { echo "no-barrier"; return; } @@ -256,6 +272,23 @@ nt "nothing was left unrendered" "{{" prompt t "the base binding says it is unresolved" '`BASE_REF`: UNRESOLVED' prompt t "the worker is told to read the base off the PR" "gh pr view <n> --json" prompt t "the manifests name the ref they came from" '`MANIFEST_REF`: ' prompt +# The bindings an api reviewer cannot function without, pinned on the VALUE +# side: a `NAME`: assertion passes just as well against a rendered blank, which +# is the shape a call site that stopped supplying a placeholder used to take. +bound() { # bound <NAME> <prompt-file> — reads the rendered roster line shape + local v; v="$(sed -n "s/^- \`$1\`: \(.*\)$/\1/p" "$2" | head -1)" + [ -n "$v" ] && echo "$1 bound: $v" || echo "$1 UNBOUND" +} +skill_pin() { # skill_pin <prompt-file> — SKILL_FILE renders in prose, not on the roster + local v; v="$(sed -n 's/.*dispatcher-pinned copy at `\([^`]*\)`.*/\1/p' "$1" | head -1)" + [ -n "$v" ] && echo "SKILL_FILE bound: $v" || echo "SKILL_FILE UNBOUND" +} +API_PROMPT="$DH/prompt-9-api-qagent.md" +t "the barrier file binding carries a value" "BIND_READY_FILE bound" bound BIND_READY_FILE "$API_PROMPT" +t "the implement contract carries a value" "IMPLEMENT_PROTOCOL_FILE bound" bound IMPLEMENT_PROTOCOL_FILE "$API_PROMPT" +t "the board scripts binding carries a value" "BOARD_SCRIPTS bound" bound BOARD_SCRIPTS "$API_PROMPT" +t "the assignment file binding carries a value" "TICKET_BODY_FILE bound" bound TICKET_BODY_FILE "$API_PROMPT" +t "the pinned protocol path carries a value" "SKILL_FILE bound" skill_pin "$API_PROMPT" # --- the triggered form: gh-only, and it says so --------------------------- triggered() { @@ -284,8 +317,14 @@ printf '{"lane": "qagent", "run_id": 99, "spawn_completed": false}\n' \ printf 'orphaned assignment\n' > "$DH2/board-claims/nonce-b.body.md" # (c) the spawn DID complete — its worker is right there in the registry — # and only the marker write was lost. -printf '{"lane": "qagent", "run_id": 51, "spawn_completed": false}\n' \ +printf '{"lane": "qagent", "run_id": 51, "spawn_completed": false, "ticket": "9"}\n' \ > "$DH2/board-claims/nonce-c.json" +# ...and the delivery it confirms is where the ticket's failed-cycle count is +# cleared when the inline reset never ran. The reset sits one line ahead of the +# marker write, so THIS crash — bind landed, marker lost — is exactly the +# window that leaves a durable recovery beside a stale counter, and a much +# later unrelated fault would then escalate two rungs early. +mkdir -p "$DH2/board-suppress"; echo 2 > "$DH2/board-suppress/.attempts-9" printf '{"uuid":"cccc0001","current":"cccc0001","name":"9-api-qagent","status":"working","run_id":51,"lane":"qagent","ticket":"9"}' \ > "$DH2/cccc0001.json" # (d) another dispatcher's journal, mid-handoff. Replaying it here would spawn @@ -325,6 +364,9 @@ gone() { [ -e "$1" ] && echo "still-there" || echo "gone"; } t "the stranded journal is dropped" "gone" gone "$DH2/board-claims/nonce-b.json" t "so is its orphaned assignment body" "gone" gone "$DH2/board-claims/nonce-b.body.md" t "a lost marker is repaired, not replayed" '"spawn_completed": true' cat "$DH2/board-claims/nonce-c.json" +attempts9r() { [ -e "$DH2/board-suppress/.attempts-9" ] && echo "count kept" || echo "count cleared"; } +t "and the delivery it confirms clears the ticket's failed-cycle count" \ + "count cleared" attempts9r t "another lane's journal is left untouched" '"run_id": 77, "spawn_completed": false' \ cat "$DH2/board-claims/nonce-d.json" # --- (e) a spawned-but-unbound run is never ended -------------------------- @@ -455,6 +497,22 @@ printf '{"lane": "qagent", "run_id": 71, "spawn_completed": false, "ticket": "42 printf '{"lane": "qagent", "run_id": 72, "spawn_completed": false}\n' \ > "$DH4/board-claims/nonce-u.json" printf 'undelivered review assignment\n' > "$DH4/board-claims/nonce-u.body.md" +# (v) THE SAME SHAPE AS (s) WITH A LIVE WRITER: bound, no ack, and the process +# that claimed it is still running — a peer between its bind and the ack it +# is waiting for. That handover legitimately outlives any mtime grace (the +# bound is 120 seconds), so liveness is the only thing that separates it +# from (s). It is NOT a completed delivery: if that reviewer never acks, +# the peer retires it and releases the run. Reading it as "marker lost" +# seals a journal somebody else is still writing and — the reason it is +# pinned here — clears the ticket failed-cycle ladder for a delivery that +# may be about to be undone. +CTL_V="$DH4/43-api-qagent-control.inflight"; mkdir -p "$CTL_V" +printf '{"uuid": "ffff0003", "ticket": "43", "ledger": "x"}\n' > "$CTL_V/bind-ready.json" +printf '{"uuid":"ffff0003","current":"ffff0003","name":"43-api-qagent","status":"working","run_id":73,"lane":"qagent","ticket":"43"}' \ + > "$DH4/ffff0003.json" +printf '{"lane": "qagent", "run_id": 73, "spawn_completed": false, "ticket": "43", "daemon": "43-api-qagent", "control": "%s", "pid": %s}\n' \ + "$CTL_V" "$$" > "$DH4/board-claims/nonce-v.json" +mkdir -p "$DH4/board-suppress"; echo 2 > "$DH4/board-suppress/.attempts-43" OUT4="$(mktemp)" ( cd "$r4" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" \ @@ -470,6 +528,16 @@ t "and its run ended so the ticket requeues" '"path": "/runs/70/end"' cat "$FIX t "ended as abandoned" '\"reason\": \"abandoned\"' cat "$FIX4.log" gone4() { [ -e "$1" ] && echo "still-there" || echo "gone"; } t "the journal is dropped" "gone" gone4 "$DH4/board-claims/nonce-s.json" +# --- (v) a delivery still waiting on its ack is not a delivery ------------- +t "a bound handover under a live writer is left in flight" \ + "is in flight under a live dispatcher" cat "$OUT4" +attempts43() { [ -e "$DH4/board-suppress/.attempts-43" ] && echo "count kept" || echo "count cleared"; } +t "and its ticket keeps its failed-cycle count until the ack lands" \ + "count kept" attempts43 +t "its journal is left open for the peer to mark" '"spawn_completed": false' \ + cat "$DH4/board-claims/nonce-v.json" +nt "and its run is not ended" '"path": "/runs/73/end"' cat "$FIX4.log" + # --- (t) a reviewer that DID cross its barrier is left alone --------------- nt "an acked reviewer's run is never ended" '"path": "/runs/71/end"' cat "$FIX4.log" nt "and its worker is never retired" "retire ffff0002" \ @@ -574,6 +642,13 @@ t "the closure package binding rides the prompt" '`CLOSURE_PACKAGE`: 3141' cat " t "the integration ref binding rides the prompt" \ '`INTEGRATION_REF`: epic/e9-integration' cat "$SCALE_PROMPT" t "the assignment file still rides an api-scale prompt" '`TICKET_BODY_FILE`: ' cat "$SCALE_PROMPT" +# The value side on the api-scale call site too — the same P_* block serves +# both api modes, but only the api render was pinned on values before. +t "the api-scale barrier file carries a value" "BIND_READY_FILE bound" bound BIND_READY_FILE "$SCALE_PROMPT" +t "the api-scale implement contract carries a value" "IMPLEMENT_PROTOCOL_FILE bound" bound IMPLEMENT_PROTOCOL_FILE "$SCALE_PROMPT" +t "the api-scale board scripts carry a value" "BOARD_SCRIPTS bound" bound BOARD_SCRIPTS "$SCALE_PROMPT" +t "the api-scale assignment file carries a value" "TICKET_BODY_FILE bound" bound TICKET_BODY_FILE "$SCALE_PROMPT" +t "the api-scale protocol pin carries a value" "SKILL_FILE bound" skill_pin "$SCALE_PROMPT" t "the scale prompt orders the integration checkout" \ "git fetch origin epic/e9-integration" cat "$SCALE_PROMPT" # THE CHECKOUT NAMES THE REF THE FETCH JUST WROTE. `git fetch origin <ref>` on a @@ -728,4 +803,102 @@ nt "so no rendered branch name is baked into the base fetch" \ t "the manifests are read from that same ref" '`MANIFEST_REF`: trunk' cat "$TRUNK_PROMPT" nt "and none of it went through gh" "GH-CALLED" cat "$MARKER" +# ========================================================================= +# Scenario 9 — AN EPIC CLAIM WITH NO INTEGRATION BRANCH STILL DISPATCHES, and +# its INTEGRATION_REF binding renders PRESENT AND EMPTY. This is the one place +# where an empty rendered value is the contract rather than a defect: the +# dispatcher deliberately does not refuse the spawn (refusing strands the +# ticket with no park note naming the gap), so the empty binding is what the +# api-scale block's own empty-ref check reads to park the epic itself, with a +# note. Now that an unsupplied placeholder hard-fails the render, that +# distinction lives one line away from the check — a later "reject empty +# values too" tightening would look obviously right, pass every other test, +# and silently convert a worker-parks-with-a-note into a stranded ticket. +# ========================================================================= +PORT9="$(free_port)" +FIX9="$(mktemp)"; : > "$FIX9.log" +cat > "$FIX9" <<'JSON' +[ + {"method":"POST","path":"/runs/claim","status":200,"once":true, + "body":{"claimed":true,"runId":81,"ticketId":95,"fence":1,"bearer":"noref-bearer", + "body":"branch-less epic assignment","pr":"5150","branch":null, + "parentPin":null}}, + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}}, + {"method":"POST","path":"/runs/81/bind","status":200,"body":{"ok":true}} +] +JSON +python3 "$TESTS_DIR/mock-server.py" "$FIX9" "$PORT9" & MOCK9=$! +trap 'kill $MOCK $MOCK2 $MOCK3 $MOCK4 $MOCK5 $MOCK6 $MOCK7 $MOCK8 $MOCK9 2>/dev/null' EXIT +wait_for_port "$PORT9" || { echo "FAIL mock server never listened on $PORT9"; exit 1; } + +r9="$(apirepo "$PORT9")" +DH9="$(mktemp -d)" +OUT9="$(mktemp)" +( cd "$r9" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" \ + DAEMON_HOME="$DH9" DAEMON_SCRIPTS="$DS" LOCAL_REPO="$r9" \ + BOARD_CREDENTIALS_FILE="$CREDS" REVIEW_MAX_CONCURRENT=2 \ + REVIEW_ACK_POLLS=400 REVIEW_ACK_DELAY=0.02 \ + "$DISPATCH" --sweep ) > "$OUT9" 2>&1 || true + +NOREF_PROMPT="$DH9/prompt-95-api-qagent.md" +binding_shape() { # binding_shape <NAME> <prompt-file> — absent / empty / valued + local v + if ! grep -q "^- \`$1\`:" "$2"; then echo "LINE-ABSENT"; return; fi + v="$(sed -n "s/^- \`$1\`: \{0,1\}\(.*\)\$/\1/p" "$2" | head -1)" + [ -n "$v" ] && echo "PRESENT-WITH-VALUE" || echo "PRESENT-AND-EMPTY" +} +t "a branch-less epic claim still reaches a worker" "claimed #95 run=81" cat "$OUT9" +nt "the empty integration ref does not fail the render closed" \ + "unrendered placeholders" cat "$OUT9" +t "the api-scale variant is still the one rendered" \ + "SCALE REVIEWER of recomposition epic #95" cat "$NOREF_PROMPT" +t "the integration ref renders present and empty, not missing" \ + "PRESENT-AND-EMPTY" binding_shape INTEGRATION_REF "$NOREF_PROMPT" +t "and the WORKER is the party that parks it, with a note" \ + 'needs-human "scale review: the claim carried no integration ref"' cat "$NOREF_PROMPT" + +# ========================================================================= +# Scenario 10 — ONE RECOVERY ATTEMPT PER TICKET PER TICK REACHES THIS PHASE +# TOO. The sweep's resume phase records every ticket it attempted a recovery +# for this tick; phase 4 hands that ledger across the same way it hands the +# suppression directory. Without it the review lane could claim the very +# ticket whose replay just faulted — a second attempt in the same tick, over +# the invariant the ledger exists to hold. The claim is how we learn WHICH +# ticket the server picked, so the refusal is a release, as suppression's is. +# ========================================================================= +PORT10="$(free_port)" +FIX10="$(mktemp)"; : > "$FIX10.log" +cat > "$FIX10" <<'JSON' +[ + {"method":"POST","path":"/runs/claim","status":200,"once":true, + "body":{"runId":71,"ticketId":33,"fence":1,"bearer":"tok-l","plan":null, + "body":"ledgered review","parentPin":null}}, + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}}, + {"method":"POST","path":"/runs/71/end","status":200,"body":{"ended":true}}, + {"method":"POST","path":"/runs/71/bind","status":200,"body":{"bound":true}} +] +JSON +python3 "$TESTS_DIR/mock-server.py" "$FIX10" "$PORT10" & MOCK10=$! +trap 'kill $MOCK $MOCK2 $MOCK3 $MOCK4 $MOCK5 $MOCK6 $MOCK7 $MOCK8 $MOCK9 $MOCK10 2>/dev/null' EXIT +wait_for_port "$PORT10" || { echo "FAIL mock server never listened on $PORT10"; exit 1; } + +r10="$(apirepo "$PORT10")" +DH10="$(mktemp -d)" +LEDGER10="$(mktemp)"; echo 33 > "$LEDGER10" +OUT10="$(mktemp)" +( cd "$r10" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" \ + DAEMON_HOME="$DH10" DAEMON_SCRIPTS="$DS" LOCAL_REPO="$r10" \ + BOARD_CREDENTIALS_FILE="$CREDS" REVIEW_MAX_CONCURRENT=2 \ + REVIEW_ACK_POLLS=400 REVIEW_ACK_DELAY=0.02 \ + BOARD_RESUMED_LEDGER="$LEDGER10" \ + "$DISPATCH" --sweep ) > "$OUT10" 2>&1 || true + +t "a claim yielding a tick-ledgered ticket is released" \ + "#33 already had its one recovery attempt this tick" cat "$OUT10" +t "and that run is ended" '"path": "/runs/71/end"' cat "$FIX10.log" +nt "so no reviewer is spawned for it" "ARGS name=33" \ + bash -c "cat '$DH10/spawn-capture.txt' 2>/dev/null || echo none" +journal10() { ls "$DH10/board-claims"/*.json >/dev/null 2>&1 && echo "journal kept" || echo "journal dropped"; } +t "and the journal is dropped with it" "journal dropped" journal10 + finish diff --git a/tests/claude-code/board-api/test-sweep-resume.sh b/tests/claude-code/board-api/test-sweep-resume.sh index ed314c297b..6e46f94a25 100755 --- a/tests/claude-code/board-api/test-sweep-resume.sh +++ b/tests/claude-code/board-api/test-sweep-resume.sh @@ -725,9 +725,11 @@ kill $DMOCKS 2>/dev/null || true # cheapest place to see which directory this phase is actually working in. # ========================================================================= SUPD="$TDIR/operator-suppress"; mkdir -p "$SUPD" -# env-issue 999 is on no listing this board serves, so the lift fires on the -# closed-env-issue trigger without depending on any other scenario's leftovers. -printf '%s\n' '{"ticket": 12, "state": "in-progress", "env_issue": 999}' \ +# Env-issue 90 is `done` in this board's standing listing, so the lift fires on +# the closed-env-issue trigger without depending on any other scenario's +# leftovers. (An env-issue absent from the listing would NOT do: absent is +# unknown, not closed — see the absent-row scenarios at the end of this file.) +printf '%s\n' '{"ticket": 12, "state": "in-progress", "env_issue": 90}' \ > "$SUPD/12.json" : > "$FIX.log" OUTS="$TDIR/suppressdir.out" @@ -739,4 +741,450 @@ t "the configured directory is the one this phase reads" \ supd_record() { [ -e "$SUPD/12.json" ] && echo "still-there" || echo "gone"; } t "and the record it acted on was the operator's" "gone" supd_record +# ========================================================================= +# A CLAIM FAILURE IS NOT ONE KIND OF EVENT. Both claim exits used to bypass +# the recovery counter, so a ticket whose claim errored churned forever — and +# the kept journal was re-classified `replay` next tick while the feed ALSO +# re-served the ticket: two claims and a leaked journal per tick. +# +# Counting every claim failure is the wrong fix, because arkho answers two +# typed 409s that mean THIS JOURNAL is obsolete, not that the substrate is +# sick: `nonce-consumed` (the predecessor's run ended, so replaying its nonce +# is doomed forever) and `stale-resume` (the ticket moved after the feed +# read). Those drop the journal uncharged and let the next tick re-serve the +# ticket on a fresh nonce. Everything else on that exit — transport death, +# 5xx, an untyped refusal — IS a fault: charged, journal kept. +# +# `claimed:false` stays uncharged on purpose: it is the server's backpressure, +# and a suppression written from it would remove a HEALTHY ticket from both +# the resume and the dispatch phase until a human closed an env-issue. +# +# Its own fixture world, with a fresh registry per scenario — a kept journal +# and the attempt counter must not leak from one scenario into the next. +# ========================================================================= +CFIX="$TDIR/fix-claim.json"; : > "$CFIX.log" +cat > "$CFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200,"once":true, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":500,"once":true, + "body":{"error":{"code":"internal","message":"boom"}}}, + + {"method":"GET","path":"/runs/needing-resume","status":200,"once":true, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":409,"once":true, + "body":{"error":{"code":"nonce-consumed", + "message":"a nonce on an ended run is spent, not replayable"}}}, + + {"method":"GET","path":"/runs/needing-resume","status":200,"once":true, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":409,"once":true, + "body":{"error":{"code":"stale-resume", + "message":"ticket moved since the feed read"}}}, + + {"method":"GET","path":"/runs/needing-resume","status":200,"once":true, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":200,"once":true, + "body":{"claimed":false}}, + + {"method":"GET","path":"/runs/needing-resume","status":200,"body":[]} +] +JSON +CPORT="$(free_port)" +python3 "$TESTS_DIR/mock-server.py" "$CFIX" "$CPORT" & CMOCK=$! +wait_for_port "$CPORT" || { echo "FAIL mock server never listened on $CPORT"; exit 1; } +CREPO="$(mkrepo)"; mkdir -p "$CREPO/.doperpowers" +printf '{"binding":"api","url":"http://127.0.0.1:%s"}' "$CPORT" > "$CREPO/.doperpowers/board.json" +CSW() { # CSW <registry> — one resume tick against the claim-failure board + ( cd "$CREPO" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" HOME="$TESTHOME" \ + DAEMON_HOME="$1" DAEMON_SCRIPTS="$DS" BOARD_CREDENTIALS_FILE="$CREDS" \ + "$SCRIPTS/_sweep_api.sh" resume ) +} +journals() { # journals <registry> — how many claim journals it is holding + local n=0 f + for f in "$1"/board-claims/*.json; do [ -e "$f" ] || continue; n=$((n + 1)); done + echo "journals=$n" +} + +CDHA="$TDIR/dh-claim-fault"; mkdir -p "$CDHA" +OUTCA="$TDIR/claim-fault.out" +CSW "$CDHA" > "$OUTCA" 2>&1 || true +t "a claim that FAULTS charges a recovery cycle" "recovery cycle 1 of 3" cat "$OUTCA" +t "and the journal is kept as the replay handle" "journals=1" journals "$CDHA" + +CDHB="$TDIR/dh-claim-nonce"; mkdir -p "$CDHB" +OUTCB="$TDIR/claim-nonce.out" +CSW "$CDHB" > "$OUTCB" 2>&1 || true +t "a nonce-consumed claim names the journal obsolete" \ + "journal is obsolete (nonce-consumed)" cat "$OUTCB" +nt "and is not reported as a fault" "successor claim failed" cat "$OUTCB" +nt "so no recovery cycle is charged" "recovery cycle" cat "$OUTCB" +t "the spent journal is dropped" "journals=0" journals "$CDHB" + +CDHC="$TDIR/dh-claim-stale"; mkdir -p "$CDHC" +OUTCC="$TDIR/claim-stale.out" +CSW "$CDHC" > "$OUTCC" 2>&1 || true +t "a stale-resume claim names the journal obsolete too" \ + "journal is obsolete (stale-resume)" cat "$OUTCC" +nt "and is not reported as a fault either" "successor claim failed" cat "$OUTCC" +nt "so no recovery cycle is charged for it" "recovery cycle" cat "$OUTCC" +t "and that journal is dropped as well" "journals=0" journals "$CDHC" + +CDHD="$TDIR/dh-claim-none"; mkdir -p "$CDHD" +OUTCD="$TDIR/claim-none.out" +CSW "$CDHD" > "$OUTCD" 2>&1 || true +t "backpressure is a wait state, not a failure" \ + "the board granted no successor" cat "$OUTCD" +nt "it charges no recovery cycle" "recovery cycle" cat "$OUTCD" +t "and leaves no journal behind" "journals=0" journals "$CDHD" +kill $CMOCK 2>/dev/null || true + +# ========================================================================= +# ONE RECOVERY ATTEMPT PER TICKET PER TICK, AND A SUPPRESSION THAT FREEZES +# THE JOURNAL TOO. +# +# A kept fault journal is re-classified `replay` by reconciliation while the +# feed ALSO re-serves the same ticket, so one ticket bought two successor +# claims, two charged cycles and a second journal every tick: the documented +# three-cycle ladder fired in two ticks and journals grew for as long as the +# fault lasted. Reconciliation now records the tickets it claimed for, and the +# feed loop skips them. +# +# Reconciliation also replayed straight THROUGH a suppression — spending the +# recovery the suppression exists to stop, and re-escalating every third cycle. +# Each re-escalation rewrote the suppression record with the state read seconds +# earlier in the SAME tick, so _check_lift's `moved` compared a state against +# itself and the "move the ticket" half of the escalation's own instructions +# could never lift anything. The journal is now left standing while suppressed, +# and the lift pass runs BEFORE reconciliation — which is also what keeps a +# suppression lifting mid-tick from stranding its journal: the just-lifted +# ticket replays its own nonce rather than the feed minting a fresh one beside +# it. +# +# Each scenario gets its own board and its own registry: a standing journal, an +# attempt counter and a suppression record all have to start from a known state. +# ========================================================================= +RMOCKS="" +rboard() { # rboard <fixtures-file> — a throwaway board; sets RREPO and RLOG + local port; port="$(free_port)" + RLOG="$1.log"; : > "$RLOG" + python3 "$TESTS_DIR/mock-server.py" "$1" "$port" & + RMOCKS="$RMOCKS $!" + wait_for_port "$port" || { echo "FAIL mock server never listened on $port"; exit 1; } + RREPO="$(mkrepo)"; mkdir -p "$RREPO/.doperpowers" + printf '{"binding":"api","url":"http://127.0.0.1:%s"}' "$port" > "$RREPO/.doperpowers/board.json" +} +RSW() { # RSW <registry> — one resume tick against the current rboard + ( cd "$RREPO" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" HOME="$TESTHOME" \ + DAEMON_HOME="$1" DAEMON_SCRIPTS="$DS" BOARD_CREDENTIALS_FILE="$CREDS" \ + "$SCRIPTS/_sweep_api.sh" resume ) +} +claims() { # claims <log> — successor-claim POSTs that reached the wire + echo "claims=$(grep -c '"path": "/runs/claim-successor"' "$1" || true)" +} +standing_journal() { # standing_journal <registry> — which journals are on disk + # A glob rather than `ls`: an EXISTING but empty directory makes ls print + # nothing at all, which would pass an absence assertion by accident. + local f n=0 + for f in "$1"/board-claims/*; do + [ -e "$f" ] || continue + basename "$f"; n=$((n + 1)) + done + [ "$n" -gt 0 ] || echo "no journals" +} +mkjournal() { # mkjournal <registry> <nonce> <ticket> — an unfinished claim + mkdir -p "$1/board-claims" + printf '{"lane":"successor","run_id":null,"spawn_completed":false,"ticket":"%s"}\n' \ + "$3" > "$1/board-claims/$2.json" +} + +# ---- a standing journal AND the same ticket on the feed, one tick --------- +AFIX="$TDIR/fix-once.json" +cat > "$AFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":500, + "body":{"error":{"code":"internal","message":"boom"}}}, + {"method":"POST","path":"/tickets","status":200,"once":true, + "body":{"id":93,"state":"needs-human"}}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":12,"state":"in-progress","priority":"P1","title":"the stuck one"}, + {"id":93,"state":"needs-human","priority":null, + "title":"stuck resume: ticket #12 cannot be revived"}]} +] +JSON +rboard "$AFIX" +ADH="$TDIR/dh-once"; mkdir -p "$ADH" +mkjournal "$ADH" n-standing 12 +OUTO1="$TDIR/once-tick1.out" +RSW "$ADH" > "$OUTO1" 2>&1 || true +t "a replayed journal and the feed are ONE attempt" "claims=1" claims "$RLOG" +t "the ticket the replay already spent is skipped on the feed" \ + "already replayed this tick" cat "$OUTO1" +t "so exactly one cycle is charged" "recovery cycle 1 of 3" cat "$OUTO1" +nt "not two" "recovery cycle 2 of 3" cat "$OUTO1" +t "and no second journal is minted" "journals=1" journals "$ADH" + +OUTO2="$TDIR/once-tick2.out" +RSW "$ADH" > "$OUTO2" 2>&1 || true +t "the ladder advances exactly one rung per tick" \ + "recovery cycle 2 of 3" cat "$OUTO2" +nt "and no rung beyond it" "recovery cycle 3 of 3" cat "$OUTO2" +nt "so the second tick escalates nothing" "escalated #12" cat "$OUTO2" +t "and still holds one journal" "journals=1" journals "$ADH" + +OUTO3="$TDIR/once-tick3.out" +RSW "$ADH" > "$OUTO3" 2>&1 || true +t "the THIRD tick is the one that escalates" \ + "escalated #12 → env-issue #93 (suppressed)" cat "$OUTO3" +t "three ticks of a claim fault cost three claims" "claims=3" claims "$RLOG" + +# ---- the ticket MOVED: the lift must be able to see it ------------------- +# Re-escalation rewrites the suppression record with the state read moments +# earlier, so a reconcile that runs first makes `moved` structurally unable to +# fire — the operator does exactly what the env-issue body says and nothing +# happens. The counter starts at 2 so this tick's single charge reaches the +# rung that re-escalates. +BFIX="$TDIR/fix-moved.json" +cat > "$BFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200,"body":[]}, + {"method":"POST","path":"/runs/claim-successor","status":500, + "body":{"error":{"code":"internal","message":"boom"}}}, + {"method":"POST","path":"/tickets","status":200, + "body":{"id":90,"state":"needs-human"}}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":12,"state":"needs-human","priority":"P1","title":"the stuck one"}, + {"id":90,"state":"needs-human","priority":null, + "title":"stuck resume: ticket #12 cannot be revived"}]} +] +JSON +rboard "$BFIX" +BDH="$TDIR/dh-moved"; mkdir -p "$BDH/board-suppress" +printf '%s\n' '{"ticket": 12, "state": "in-progress", "env_issue": 90}' \ + > "$BDH/board-suppress/12.json" +printf '2\n' > "$BDH/board-suppress/.attempts-12" +mkjournal "$BDH" n-moved 12 +OUTM="$TDIR/moved.out" +RSW "$BDH" > "$OUTM" 2>&1 || true +t "a suppression whose ticket moved lifts before any replay can rewrite it" \ + "suppression lifted for #12 — the ticket moved" cat "$OUTM" + +# ---- a suppressed ticket's journal is frozen too ------------------------- +CFIX2="$TDIR/fix-frozen.json" +cat > "$CFIX2" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":500, + "body":{"error":{"code":"internal","message":"boom"}}}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":12,"state":"in-progress","priority":"P1","title":"the stuck one"}, + {"id":90,"state":"needs-human","priority":null,"title":"stuck resume"}]} +] +JSON +rboard "$CFIX2" +FDH="$TDIR/dh-frozen"; mkdir -p "$FDH/board-suppress" +printf '%s\n' '{"ticket": 12, "state": "in-progress", "env_issue": 90}' \ + > "$FDH/board-suppress/12.json" +mkjournal "$FDH" n-frozen 12 +OUTF="$TDIR/frozen.out" +RSW "$FDH" > "$OUTF" 2>&1 || true +t "a suppressed ticket's journal is left where it is" \ + "the journal stands untouched" cat "$OUTF" +t "and buys no successor claim" "claims=0" claims "$RLOG" +nt "and charges no recovery cycle" "recovery cycle" cat "$OUTF" +t "the retry handle survives the suppression" "n-frozen.json" \ + standing_journal "$FDH" + +# ---- a suppression that lifts THIS tick replays its own journal ---------- +DFIX="$TDIR/fix-lifting.json" +cat > "$DFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":500, + "body":{"error":{"code":"internal","message":"boom"}}}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":12,"state":"in-progress","priority":"P1","title":"the stuck one"}, + {"id":90,"state":"done","priority":null,"title":"stuck resume"}]} +] +JSON +rboard "$DFIX" +LDH="$TDIR/dh-lifting"; mkdir -p "$LDH/board-suppress" +printf '%s\n' '{"ticket": 12, "state": "in-progress", "env_issue": 90}' \ + > "$LDH/board-suppress/12.json" +mkjournal "$LDH" n-lifted 12 +OUTLF="$TDIR/lifting.out" +RSW "$LDH" > "$OUTLF" 2>&1 || true +t "the suppression lifts on the closed env-issue" \ + "suppression lifted for #12 — the env-issue closed" cat "$OUTLF" +t "and the just-lifted ticket costs exactly one claim" "claims=1" claims "$RLOG" +t "which carries the STANDING journal's nonce, not a fresh one" \ + '\"dispatchNonce\": \"n-lifted\"' cat "$RLOG" +t "so no second journal is stranded beside it" "journals=1" journals "$LDH" +t "and the one on disk is still the original" "n-lifted.json" \ + standing_journal "$LDH" + +# ---- two standing journals for ONE ticket are still one attempt ---------- +# The ledger closed the reconcile→feed door; this is the reconcile→reconcile +# one. Reconciliation walks a row per journal file, so two unfinished successor +# journals naming the same ticket were two claims and two ladder rungs inside a +# single tick — the very shape this task exists to close. Not a steady state +# (a replay reuses its own nonce and overwrites its own file), but the +# intermediate commit on this branch minted an extra journal per tick during a +# claim fault, so a registry that ticked on it arrives holding several and this +# pass must not charge through them. +EFIX="$TDIR/fix-twojournals.json" +cat > "$EFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200, + "body":[{"ticketId":12,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":500, + "body":{"error":{"code":"internal","message":"boom"}}}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":12,"state":"in-progress","priority":"P1","title":"the stuck one"}]} +] +JSON +rboard "$EFIX" +TDH="$TDIR/dh-two-journals"; mkdir -p "$TDH" +mkjournal "$TDH" n-a 12 +mkjournal "$TDH" n-b 12 +OUTT="$TDIR/two-journals.out" +RSW "$TDH" > "$OUTT" 2>&1 || true +t "a second journal for the same ticket waits its turn" \ + "#12 already had its one recovery attempt this tick" cat "$OUTT" +t "so two journals still cost one claim" "claims=1" claims "$RLOG" +t "and one ladder rung" "recovery cycle 1 of 3" cat "$OUTT" +nt "not two" "recovery cycle 2 of 3" cat "$OUTT" +t "the waiting journal is left untouched" "n-b.json" standing_journal "$TDH" + +# ---- AN ABSENT ROW IS NOT A STATE ---------------------------------------- +# `/tickets` is read whole, with no cursor and no envelope, so a truncated or +# partial listing is indistinguishable from a smaller board. Reading a missing +# row as a value fired BOTH lift triggers on it — `moved` because None is not +# the recorded state, `closed` because None sat in the closed tuple — and one +# short read lifted every suppression it could not see: the ladder re-ran and +# the human collected a fresh env-issue every three cycles. Absent is UNKNOWN, +# and the suppression waits for a read that can name the state. +suppression_present() { # suppression_present <registry> <ticket> + if [ -e "$1/board-suppress/$2.json" ]; then echo "still-there"; else echo "gone"; fi +} +tick_exit() { # tick_exit <registry> <out> — run a tick, record its exit status + if RSW "$1" > "$2" 2>&1; then echo "tick exit=0" >> "$2" + else echo "tick exit=$?" >> "$2"; fi +} + +GFIX="$TDIR/fix-absent-ticket.json" +cat > "$GFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200,"body":[]}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":90,"state":"needs-human","priority":null,"title":"stuck resume"}]} +] +JSON +rboard "$GFIX" +GDH="$TDIR/dh-absent-ticket"; mkdir -p "$GDH/board-suppress" +printf '%s\n' '{"ticket": 12, "state": "in-progress", "env_issue": 90}' \ + > "$GDH/board-suppress/12.json" +OUTG="$TDIR/absent-ticket.out" +tick_exit "$GDH" "$OUTG" +t "a suppression whose ticket is off the listing is not lifted" \ + "still-there" suppression_present "$GDH" 12 +nt "and the tick says nothing about lifting it" "suppression lifted" cat "$OUTG" +t "and the tick still succeeds" "tick exit=0" cat "$OUTG" + +# The env-issue half of the same read. `closed` alone must not fire on absence: +# an env-issue nobody can see is not an env-issue somebody closed. +HFIX="$TDIR/fix-absent-env.json" +cat > "$HFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200,"body":[]}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":12,"state":"in-progress","priority":"P1","title":"the stuck one"}]} +] +JSON +rboard "$HFIX" +HDH="$TDIR/dh-absent-env"; mkdir -p "$HDH/board-suppress" +printf '%s\n' '{"ticket": 12, "state": "in-progress", "env_issue": 90}' \ + > "$HDH/board-suppress/12.json" +OUTH="$TDIR/absent-env.out" +tick_exit "$HDH" "$OUTH" +t "an env-issue off the listing is not a closed env-issue" \ + "still-there" suppression_present "$HDH" 12 +nt "so nothing is lifted on it" "suppression lifted" cat "$OUTH" +t "and that tick succeeds too" "tick exit=0" cat "$OUTH" +# ---- THE TICK LEDGER REACHES PHASE 4 ------------------------------------- +# One recovery attempt per ticket per tick is an invariant of the TICK, not of +# phase 3. The ledger travelled no further than phase_resume, so after a replay +# FAULT left the ticket unowned an ordinary lane claim could pick that same +# ticket seconds later — a second attempt inside the tick the ledger exists to +# hold to one. It rides to the dispatchers exactly as BOARD_SUPPRESS_DIR does. +IFIX="$TDIR/fix-ledger-dispatch.json" +cat > "$IFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200,"body":[]}, + {"method":"POST","path":"/runs/claim-successor","status":500, + "body":{"error":{"code":"internal","message":"boom"}}}, + {"method":"POST","path":"/runs/claim","status":200,"once":true, + "body":{"runId":70,"ticketId":12,"fence":1,"bearer":"tok-x","plan":null, + "body":"work on","parentPin":null}}, + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}}, + {"method":"POST","path":"/runs/70/end","status":200,"body":{"ended":true}}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":12,"state":"in-progress","priority":"P1","title":"the stuck one"}]} +] +JSON +rboard "$IFIX" +IDH="$TDIR/dh-ledger-dispatch"; mkdir -p "$IDH" +mkjournal "$IDH" n-led 12 +OUTI="$TDIR/ledger-dispatch.out" +( cd "$RREPO" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" HOME="$TESTHOME" \ + DAEMON_HOME="$IDH" DAEMON_SCRIPTS="$DS" BOARD_CREDENTIALS_FILE="$CREDS" \ + "$SCRIPTS/_sweep_api.sh" all ) > "$OUTI" 2>&1 || true +t "phase 3 spends the ticket's one attempt on the replay" \ + "replaying it for #12" cat "$OUTI" +t "and phase 4 refuses the same ticket on the same tick" \ + "#12 already had its one recovery attempt this tick" cat "$OUTI" +t "releasing the run it was handed" '"path": "/runs/70/end"' cat "$RLOG" +nt "so nothing is spawned for it" "SPAWN name=12" cat "$SPAWN_LOG" + +# ---- ...AND THE FEED PATH IS AN ATTEMPT TOO ------------------------------ +# The ledger recorded only the tickets reconciliation replayed for. A ticket +# served by the ORDINARY feed whose recovery then faulted (or released) is +# equally unowned and equally spent, and phase 4 could claim and spawn it in +# the same tick — the same double attempt through the other door. +JFIX="$TDIR/fix-ledger-feed.json" +cat > "$JFIX" <<'JSON' +[ + {"method":"GET","path":"/runs/needing-resume","status":200, + "body":[{"ticketId":13,"state":"in-progress","predecessorRunId":41}]}, + {"method":"POST","path":"/runs/claim-successor","status":500, + "body":{"error":{"code":"internal","message":"boom"}}}, + {"method":"POST","path":"/runs/claim","status":200,"once":true, + "body":{"runId":72,"ticketId":13,"fence":1,"bearer":"tok-y","plan":null, + "body":"work on","parentPin":null}}, + {"method":"POST","path":"/runs/claim","status":200,"body":{"claimed":false}}, + {"method":"POST","path":"/runs/72/end","status":200,"body":{"ended":true}}, + {"method":"GET","path":"/tickets","status":200, + "body":[{"id":13,"state":"in-progress","priority":"P1","title":"the stuck one"}]} +] +JSON +rboard "$JFIX" +JDH="$TDIR/dh-ledger-feed"; mkdir -p "$JDH" +OUTJL="$TDIR/ledger-feed.out" +( cd "$RREPO" && env PATH="$STUB:$PATH" GH_STUB_MARKER="$MARKER" HOME="$TESTHOME" \ + DAEMON_HOME="$JDH" DAEMON_SCRIPTS="$DS" BOARD_CREDENTIALS_FILE="$CREDS" \ + "$SCRIPTS/_sweep_api.sh" all ) > "$OUTJL" 2>&1 || true +t "the feed spends the ticket's one attempt as surely as a replay" \ + "recovery cycle 1 of 3" cat "$OUTJL" +t "and phase 4 refuses the same ticket on the same tick" \ + "#13 already had its one recovery attempt this tick" cat "$OUTJL" +t "releasing the run it was handed" '"path": "/runs/72/end"' cat "$RLOG" +nt "so nothing is spawned for it" "SPAWN name=13" cat "$SPAWN_LOG" + +# shellcheck disable=SC2086 # RMOCKS is a deliberate word-split pid list +kill $RMOCKS 2>/dev/null || true + finish diff --git a/tests/claude-code/run-skill-tests.sh b/tests/claude-code/run-skill-tests.sh index d86c5a5a10..4879965953 100755 --- a/tests/claude-code/run-skill-tests.sh +++ b/tests/claude-code/run-skill-tests.sh @@ -111,6 +111,11 @@ tests=( "board-api/integration/test-escalation.sh" "board-api/integration/test-human-verbs.sh" "board-api/integration/test-lease-renewal.sh" + # reviewing-prs: the suites in tests/reviewing-prs/ are hand-run (the spec's + # acceptance invokes them as a glob). The bootstrap parity fence is listed + # here because it is hermetic and sub-second — no dispatcher, no gh, no + # ports, just the two bootstrap templates rendered in-process. + "../reviewing-prs/test-bootstrap-parity.sh" ) # Integration tests (slow, full execution) diff --git a/tests/issue-tracker/meta-grammar-fuzz.py b/tests/issue-tracker/meta-grammar-fuzz.py new file mode 100644 index 0000000000..efcf75fef7 --- /dev/null +++ b/tests/issue-tracker/meta-grammar-fuzz.py @@ -0,0 +1,206 @@ +#!/usr/bin/env python3 +"""Property fuzz over the board:meta opener walk (_board.meta_match). + +Which opener starts the real block is decided by content, and that rule was +hand-corrected three times — each time by a body shape the previous +formulation missed, and twice by a shape an EARLIER formulation had handled. +The case pins in test-board-scripts.sh are therefore a record of what was +found, not evidence of coverage. This is the coverage: bodies composed from a +component grammar with the intended opener recorded at generation time. + +It has teeth. Against the four superseded implementations, at the default +seed and size, it reports: + + 7daa2122 (rightmost walk) 1981 / 5000 + dc896415 (adjacent-candidate gap) 827 / 5000 + c71a8670 (whole interior, \\n-split, last-cand.) 1240 / 5000 + 14e96b7f (column-zero candidate test) 711 / 5000 + +Run against another implementation with BOARD_SCRIPTS=<dir>; that is how a +candidate rewrite of the rule should be judged before it lands. + +Properties, per body: + P1 meta_match picks the opener the generator intended + P2 strip_meta returns the generated prose + P3 the rewrite (compose_body over strip+parse) lands on one block, keeps + the prose, and is idempotent + P4 the block's bytes survive verbatim from the match offset — what + board-body.sh's raw splice carries through + +Two regions are excluded because the body genuinely does not determine the +answer, not because the rule is weak there: + + - A quoted example that is all-legal `key: value` lines with no closer, no + blank line and no prose before the real opener is byte-indistinguishable + from a legacy block whose value carried a nested marker (spec v1.2.4). + The generator always closes the prose with a colon-free guard line. + - A block whose illegal interior line PRECEDES a nested marker in the same + block — `<!-- board:meta / weird: x / <!-- board:meta / pr: v / -->` — is + equally indistinguishable from a block starting at the nested opener. The + generator emits nested markers before any illegal interior line. +""" +import os +import random +import sys +import traceback + +_HERE = os.path.dirname(os.path.abspath(__file__)) +_DEFAULT = os.path.join(_HERE, "..", "..", "skills", "issue-tracker", "scripts") +sys.path.insert(0, os.environ.get("BOARD_SCRIPTS", _DEFAULT)) +import _board as B # noqa: E402 + +SEPS = ["\n", "\r", "\r\n", "\v", "\f", "\x1c", "\x1d", "\x1e", "\x85", + "\u2028", "\u2029"] +WORDS = ["board", "ticket", "prose", "docs", "review", "relay", "lane"] +UNKNOWN_KEYS = ["weird-key", "x-legacy", "Note", "TODO", "spawned_by", "PR"] +OPENER = "<!-- board:meta" + + +def words(rng, lo=1, hi=3): + return " ".join(rng.choice(WORDS) for _ in range(rng.randint(lo, hi))) + + +def known_kv(rng): + """Legal block interior — and the dangerous kind of prose line.""" + return "%s:%s%s" % (rng.choice(B.META_KEYS), rng.choice([" ", " "]), + words(rng)) + + +def illegal_kv(rng): + """`key: value` with a key that is not ours: noncanonical block content + inside a block, ordinary prose outside one.""" + return "%s: %s" % (rng.choice(UNKNOWN_KEYS), words(rng)) + + +def guard(rng): + """A line that can never be block interior — no colon at all.""" + return words(rng, 2, 5) + + +def prose_element(rng): + """A prose line, or a whole quoted example. Returns (lines, self_guarding), + where a quoted example guards itself only by supplying its own `-->`.""" + r = rng.random() + if r < 0.22: + return [guard(rng)], True + if r < 0.36: + return [""], True + if r < 0.48: + return ["%s:" % guard(rng)], True # "Docs about the block:" + if r < 0.60: + return [illegal_kv(rng)], True + if r < 0.68: + return [known_kv(rng)], False # legal-looking prose + lines = [OPENER, known_kv(rng)] + if rng.random() < 0.35: # quoted legacy-nested + lines += [OPENER, known_kv(rng)] + if rng.random() < 0.3: + lines.append(illegal_kv(rng)) + closed = rng.random() < 0.7 + if closed: + lines.append("-->") + indent = rng.choice(["", "", " ", " "]) + lines = [indent + ln for ln in lines] + sep = rng.choice(SEPS) if rng.random() < 0.45 else "\n" + if sep != "\n": # folds onto one \n-line + return [sep.join(lines)], closed + return lines, closed + + +def real_block(rng): + """The one intended trailing block.""" + interior = [known_kv(rng) for _ in range(rng.randint(1, 3))] + if rng.random() < 0.4: # a legacy nested marker + interior.append(OPENER) + interior += [known_kv(rng) for _ in range(rng.randint(1, 2))] + fallback = rng.random() < 0.45 + if fallback: # …then noncanonical lines + for _ in range(rng.randint(1, 2)): + interior.append(rng.choice([illegal_kv(rng), "# %s" % guard(rng), + "<!-- %s -->" % guard(rng)])) + return interior, fallback + + +def make_body(rng): + lines = [] + for _ in range(rng.randint(0, 6)): + el, self_guarding = prose_element(rng) + lines += el + if not self_guarding: + lines.append(guard(rng)) + if lines: + lines.append(guard(rng)) # the structural guard + prose = "" + for i, ln in enumerate(lines): + if i: + prose += rng.choice(SEPS) if rng.random() < 0.3 else "\n" + prose += ln + interior, fallback = real_block(rng) + # THE REAL BLOCK MAY BE INDENTED — META_RE's opener is unanchored, so a + # block nested under a list item or a quote is a real trailing block, and a + # column-zero quoted example above it must not win on position. Its CLOSER + # is never indented: `\n-->\s*$` is what makes it a block at all. + # The indent belongs to the PROSE side of the opener offset (meta_match + # returns the `<`), which is what the byte consumers splice against. + indent = rng.choice(["", "", "", " ", " "]) + block = "%s\n%s\n-->\n" % (OPENER, "\n".join(indent + ln for ln in interior)) + attach = rng.choice(["\n\n", "\n"]) if prose else "" + head = prose + attach + indent + return {"body": head + block, "opener": len(head), + "prose": head, "block": block, "fallback": fallback} + + +def check(c): + body, want = c["body"], c["opener"] + bad = [] + m = B.meta_match(body) + if m is None or m.start() != want: + return [("P1", "opener %s, wanted %d" % (m and m.start(), want))] + base = B.strip_meta(body) + if base != c["prose"].rstrip("\n"): + bad.append(("P2", "strip=%r wanted %r" % (base, c["prose"].rstrip("\n")))) + if body[m.start():].strip("\n") != c["block"].strip("\n"): + bad.append(("P4", "block bytes not verbatim")) + meta = B.parse_meta(body) + if not meta: + return bad # an all-unknown block is dropped, by design + b2 = B.compose_body(base, meta) + # compose_body always writes `base.rstrip("\n") + "\n\n" + block`, an empty + # base included (the block then carries two leading newlines — cosmetic, + # and idempotent). + want2 = len(base.rstrip("\n")) + 2 + m2 = B.meta_match(b2) + if m2 is None or m2.start() != want2: + bad.append(("P3", "rewrite opener %s, wanted %d" % (m2 and m2.start(), want2))) + elif B.strip_meta(b2) != base.rstrip("\n"): + bad.append(("P3", "rewrite dropped prose")) + elif B.compose_body(B.strip_meta(b2), B.parse_meta(b2)) != b2: + bad.append(("P3", "rewrite not idempotent")) + return bad + + +def main(): + seed = int(sys.argv[1]) if len(sys.argv) > 1 else 20260813 + n = int(sys.argv[2]) if len(sys.argv) > 2 else 5000 + rng = random.Random(seed) + kinds, shown = {}, [] + for i in range(n): + c = make_body(rng) + try: + bad = check(c) + except Exception: + bad = [("EXC", traceback.format_exc().strip().splitlines()[-1])] + for prop, detail in bad: + kinds[prop] = kinds.get(prop, 0) + 1 + if len(shown) < 5: + shown.append((i, prop, detail, c)) + print("seed=%d n=%d divergences=%d %s" + % (seed, n, sum(kinds.values()), kinds or "(none)")) + for i, prop, detail, c in shown: + print("\n--- iter %d %s: %s\nbody=%r\nwant_opener=%d fallback=%s" + % (i, prop, detail, c["body"], c["opener"], c["fallback"])) + return 1 if kinds else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/issue-tracker/test-board-scripts.sh b/tests/issue-tracker/test-board-scripts.sh index 7ca0e246f8..c9d389df9c 100755 --- a/tests/issue-tracker/test-board-scripts.sh +++ b/tests/issue-tracker/test-board-scripts.sh @@ -984,7 +984,7 @@ cat > "$DAEMON_HOME/22222222-1111-2222-3333-444444444444.json" <<META "updated": "2026-07-12T00:00:00Z"} META run board-answer.sh "$fb_impl_t" "answer" >/dev/null -assert_contains "$(state "s['issues']['$fb_impl_t']['labels']")" "status:in-progress" "unrecorded pre-park with a non-architect role falls back on in-progress" +assert_contains "$(state "s['issues']['$fb_impl_t']['labels']")" "status:in-progress" "an unrecorded pre-park with an IMPLEMENT role falls back on in-progress" out="$(run board-register.sh "Unknown-role fallback probe" enhancement P2 --body-file "$SPEC_BODY")" fb_unk_t="${out%% *}" @@ -998,6 +998,66 @@ META run board-answer.sh "$fb_unk_t" "answer" >/dev/null assert_contains "$(state "s['issues']['$fb_unk_t']['labels']")" "status:in-progress" "a meta with no role at all (pre-fix daemon) preserves the prior default: in-progress" +# The QAgent lane is the third rung. A reviewer resumed into in-progress owns +# no implementation branch and has no legal exit; it belongs in in-review. +# Reaching in-review from an UNRECORDED pre-park is itself the argv proof that +# board-answer re-supplied --pr: board-transition refuses in-review without the +# flag unless the pre-park: meta records in-review (asserted absent below), so +# the recorded-pr shortcut cannot be what let this through. +out="$(run board-register.sh "QAgent fallback probe" enhancement P2 --body-file "$SPEC_BODY")" +fb_qa_t="${out%% *}" +run board-transition.sh "$fb_qa_t" in-progress "picked up" --pr https://github.com/test/repo/pull/88 >/dev/null +run board-transition.sh "$fb_qa_t" needs-info "need more research" >/dev/null +run board-transition.sh "$fb_qa_t" needs-human "human decision needed" >/dev/null +assert_not_contains "$(state "s['issues']['$fb_qa_t']['body']")" "pre-park:" "needs-info -> needs-human records no pre-park on the qagent probe either" +cat > "$DAEMON_HOME/44444444-1111-2222-3333-444444444444.json" <<META +{"uuid": "44444444-1111-2222-3333-444444444444", "role": "QAGENT", + "status": "idle", "ticket": "$fb_qa_t", "cwd": "$WORK", + "updated": "2026-07-12T00:00:00Z"} +META +run board-answer.sh "$fb_qa_t" "ship it" >/dev/null +assert_contains "$(state "s['issues']['$fb_qa_t']['labels']")" "status:in-review" "unrecorded pre-park with a QAGENT role returns to in-review, re-supplying the ticket's own --pr" + +# ...and only when there IS a PR to re-supply. With no pr: meta the review +# lane is unwritable, and the fallback that demoted to in-progress and resumed +# anyway RE-CREATED the stranding this return exists to prevent: the reviewer +# wakes in a lane it cannot exit and review-dispatch's stale-reviewer arm +# retires it, leaving the ticket in-progress with no PR and nobody bound. The +# park is the safe state — the relay is refused and the ticket stays there. +out="$(run board-register.sh "QAgent no-PR probe" enhancement P2 --body-file "$SPEC_BODY")" +fb_qnp_t="${out%% *}" +run board-transition.sh "$fb_qnp_t" needs-info "need more research" >/dev/null +run board-transition.sh "$fb_qnp_t" needs-human "human decision needed" >/dev/null +cat > "$DAEMON_HOME/55555555-1111-2222-3333-444444444444.json" <<META +{"uuid": "55555555-1111-2222-3333-444444444444", "role": "QAGENT", + "status": "idle", "ticket": "$fb_qnp_t", "cwd": "$WORK", + "updated": "2026-07-12T00:00:00Z"} +META +assert_fails run board-answer.sh "$fb_qnp_t" "answer" +out="$(run board-answer.sh "$fb_qnp_t" --posted 2>&1 || true)" +assert_contains "$out" "no pr: meta" "the refusal names the meta that is missing" +assert_contains "$out" "--posted" "…and the re-run that recovers it once the link is restored" +assert_contains "$(state "s['issues']['$fb_qnp_t']['labels']")" "status:needs-human" "the PR-less QAGENT park STAYS parked — no demotion, no resume" +assert_not_contains "$(state "s['issues']['$fb_qnp_t']['labels']")" "status:in-progress" "…and never lands in the lane the reviewer cannot exit" +# The answers are posted before the refusal, so the human loses nothing by +# fixing the pr: meta and re-running --posted. +assert_contains "$(state "s['issues']['$fb_qnp_t']['comments'][-1]")" "[answers]" "the answers comment survives the refusal" + +# Reviewers bound before the role stamp carry no role: — their deterministic +# registry name is the only role record they have. +out="$(run board-register.sh "Legacy reviewer probe" enhancement P2 --body-file "$SPEC_BODY")" +fb_leg_t="${out%% *}" +run board-transition.sh "$fb_leg_t" in-progress "picked up" --pr https://github.com/test/repo/pull/89 >/dev/null +run board-transition.sh "$fb_leg_t" needs-info "need more research" >/dev/null +run board-transition.sh "$fb_leg_t" needs-human "human decision needed" >/dev/null +cat > "$DAEMON_HOME/66666666-1111-2222-3333-444444444444.json" <<META +{"uuid": "66666666-1111-2222-3333-444444444444", "name": "review-pr-89", + "status": "idle", "ticket": "$fb_leg_t", "cwd": "$WORK", + "updated": "2026-07-12T00:00:00Z"} +META +run board-answer.sh "$fb_leg_t" "ship it" >/dev/null +assert_contains "$(state "s['issues']['$fb_leg_t']['labels']")" "status:in-review" "a pre-stamp reviewer meta is recognized by its review-pr-* registry name" + unset DAEMON_SCRIPTS STUB_STATE # ---- spike lane (category spike) --------------------------------------------- @@ -1852,6 +1912,383 @@ assert_not_contains "$(state "s['issues']['$legacy_rfa']['labels']")" "status:re assert_not_contains "$(state "s['issues']['$legacy_rfa']['labels']")" "status:needs-human" \ "the superseded label is swapped out, not left alongside (that would read as conflict)" +echo "board-migrate-gh (marker-quoting body):" +# The migration writes bodies too — strip the meta block, append the pre-spec md, +# re-render — and it is a ONE-SHOT rewrite of a real ticket's statement of work. +# A leftmost strip deletes everything from a marker QUOTED in the prose onward +# (#60), so the migration path needs the same rightmost rule as every other +# consumer. +MIG_BODY="$TEST_ROOT/migrate-marker-body.md" +printf '## Problem & intent\n\nThe block looks like:\n\n<!-- board:meta\npr: example\n-->\n\n## Success criteria\n\n- the prose after the example survives a migration\n' > "$MIG_BODY" +# Born with a note so the issue carries a REAL trailing block: the quoted +# example only misleads META_RE while a real block anchors the `-->\s*$`. +mig60="$(run board-register.sh "Documents the meta block (migrated)" bug P2 \ + --body-file "$MIG_BODY" --state needs-human --note "waiting on A")" +mig60="${mig60%% *}" +cat > "$LEGACY/board-marker.json" <<J +{"version": 1, "next_id": 2, "tickets": { + "T1": {"title": "Documents the meta block (migrated)", "md": "tickets/T4.md", + "state": "needs-human", "category": "bug", "note": null, + "parent": null, "blocked_by": [], "spawned_by": null, "relates_to": [], + "branch": "feat/marker", "pr": null, "created": "2026-07-01", + "updated": "2026-07-05", "gh": $mig60} +}} +J +printf -- '---\nid: T4\n---\n# T4\n' > "$LEGACY/tickets/T4.md" +run board-migrate-gh.sh --board "$LEGACY/board-marker.json" --apply >/dev/null +mig60_body="$(state "s['issues']['$mig60']['body']")" +assert_contains "$mig60_body" "the prose after the example survives a migration" \ + "the migration keeps the prose after a quoted marker (#60)" +assert_contains "$mig60_body" "## Success criteria" "…including the headings after it" +assert_contains "$mig60_body" "branch: feat/marker" "…while still applying the migrated meta" + +# The same body shape with the example at the very END of the prose. That one +# still satisfies META_RE's `\s*$`, so a SECOND strip deletes it as if it were +# the block: write_body strips, then hands the result to a renderer that strips +# again. Exactly one strip may happen (task-2 review I1). +migtail="$(run board-register.sh "Ends with the meta example" bug P2 \ + --state needs-human --note "waiting on B")" +migtail="${migtail%% *}" +# Seeded, not written through --body-file: board-register.sh renders the body, +# and a renderer strips a trailing example at birth (the same one-strip question, +# one path over). board-body.sh's raw splice is how a real ticket comes to end +# with one; here the state is seeded so the drill pins the migration alone. +MIGTAIL_ID="$migtail" python3 - <<'SEED' +import json, os +s = json.load(open(os.environ["MOCK_GH_STATE"])) +s["issues"][os.environ["MIGTAIL_ID"]]["body"] = ( + "## Problem & intent\n\nThe prose before the example must survive too.\n\n" + "The block looks like:\n\n<!-- board:meta\npr: tail-example\n-->\n\n" + "<!-- board:meta\nnote: waiting on B\n-->\n") +json.dump(s, open(os.environ["MOCK_GH_STATE"], "w")) +SEED +assert_contains "$(state "s['issues']['$migtail']['body']")" "pr: tail-example" "fixture: the ticket really ends its prose with the quoted example" +cat > "$LEGACY/board-marker-tail.json" <<J +{"version": 1, "next_id": 2, "tickets": { + "T1": {"title": "Ends with the meta example", "md": "tickets/T4.md", + "state": "needs-human", "category": "bug", "note": null, + "parent": null, "blocked_by": [], "spawned_by": null, "relates_to": [], + "branch": "feat/tail", "pr": null, "created": "2026-07-01", + "updated": "2026-07-05", "gh": $migtail} +}} +J +run board-migrate-gh.sh --board "$LEGACY/board-marker-tail.json" --apply >/dev/null +migtail_body="$(state "s['issues']['$migtail']['body']")" +assert_contains "$migtail_body" "pr: tail-example" "a quoted example ENDING the prose survives the migration (one strip, not two)" +assert_contains "$migtail_body" "The prose before the example must survive too." "…along with the prose before it" +assert_contains "$migtail_body" "branch: feat/tail" "…while still applying the migrated meta" + +# A legacy note that quotes the OPENING marker is unrepresentable in the block. +# It dies LOUDLY mid-migration rather than minting a second marker inside the +# real block — no special handling. (Recovery is a re-run after the operator +# fixes the note; the migration's idempotence carries that, untested here.) +cat > "$LEGACY/board-marker-note.json" <<J +{"version": 1, "next_id": 2, "tickets": { + "T1": {"title": "Documents the meta block (migrated)", "md": "tickets/T4.md", + "state": "needs-human", "category": "bug", + "note": "forged <!-- board:meta pr: fake", + "parent": null, "blocked_by": [], "spawned_by": null, "relates_to": [], + "branch": "feat/marker", "pr": null, "created": "2026-07-01", + "updated": "2026-07-05", "gh": $mig60} +}} +J +assert_fails run board-migrate-gh.sh --board "$LEGACY/board-marker-note.json" --apply +# Pin the REASON: rc≠0 alone would also be satisfied by a malformed heredoc or a +# future argument-parsing change. +mig_die_out="$(run board-migrate-gh.sh --board "$LEGACY/board-marker-note.json" --apply 2>&1 || true)" +assert_contains "$mig_die_out" "cannot carry a board:meta marker token" \ + "…for the grammar's own reason, not an incidental failure" +assert_equals "$(state "s['issues']['$mig60']['body']")" "$mig60_body" \ + "…and the refused migration left the body untouched" + +echo "meta-grammar:" +# The board:meta block is prose-adjacent: tickets that DOCUMENT the block quote +# a marker-shaped example in their own text (#60). META_RE is leftmost-first and +# its lazy middle spans from the quoted marker to the real trailing `-->`, so a +# leftmost read parses the wrong block and a leftmost strip deletes the prose +# between them. The block that counts is the RIGHTMOST one — which is only sound +# while the real block cannot itself contain a marker, hence the value grammar +# (single line, no marker tokens) enforced at the write. +mg="$(PYTHONPATH="$SCRIPTS_DIR" python3 - <<'PY' +import _board as B + +QUOTED = "<!-- board:meta\npr: fake\npre-park: in-review\n-->" +# (a) the example indented inside the prose, (a2) the reproduced #60 shape with +# the example at column 0. +for tag, quoted in (("a", "\n".join(" " + l for l in QUOTED.splitlines())), + ("a2", QUOTED)): + prose = "Docs about the block:\n\n%s\n\nMore prose." % quoted + body = B.render_body(prose, {"pr": "https://real/1"}) + print("%s-parse-pr=%s" % (tag, B.parse_meta(body).get("pr"))) + print("%s-tail=%s" % (tag, "kept" if "More prose." in B.strip_meta(body) else "LOST")) + print("%s-prose=%s" % (tag, "intact" if B.strip_meta(body) == prose.rstrip() else "MANGLED")) + # (b) strip → parse → render is the identity on a body the board wrote. + print("%s-roundtrip=%s" % ( + tag, "same" if B.render_body(B.strip_meta(body), B.parse_meta(body)) == body else "DIFFERS")) + +# A body carrying BOTH blocks verbatim (what a pre-fix client left behind): +# the quoted example's keys must not leak into the parse. +both = ("Docs about the block:\n\n%s\n\nMore prose." + "\n\n<!-- board:meta\npr: https://real/1\n-->\n" % QUOTED) +print("leak-pre-park=%s" % B.parse_meta(both).get("pre-park")) +print("both-parse-pr=%s" % B.parse_meta(both).get("pr")) +print("both-tail=%s" % ("kept" if "More prose." in B.strip_meta(both) else "LOST")) + +# (c) contract_hash covers the prose only — same prose, different meta, same id. +other = both.replace("pr: https://real/1", "pr: https://other/2") +print("hash-prose-only=%s" % ("yes" if B.contract_hash(both) == B.contract_hash(other) else "no")) + +# (d) a multi-line value collapses to one line rather than minting block lines. +print("d-block=%r" % B.render_body("prose", {"note": "line1\nline2"})) + +# (e) `-->` in a value is LEGAL (spec v1.2.1): the collapse leaves every value +# behind its `key: ` prefix, and META_RE needs the closer at LINE START, so an +# arrow can never terminate the block early. Refusing it bricked every stored +# note carrying an ASCII arrow — update_meta re-renders every key it parsed. +ARROW = "parked, in-review --> needs-info" +arrow = B.render_body("prose", {"note": ARROW}) +print("e-parse=%s" % B.parse_meta(arrow).get("note")) +print("e-prose=%s" % ("intact" if B.strip_meta(arrow) == "prose" else "MANGLED")) +print("e-roundtrip=%s" % ( + "same" if B.render_body(B.strip_meta(arrow), B.parse_meta(arrow)) == arrow else "DIFFERS")) +# …even one that would otherwise REACH line start: the collapse folds it back. +print("e-multi=%s" % B.parse_meta(B.render_body("prose", {"note": "a\n--> b"})).get("note")) + +# (f) EVERY separator parse_meta's str.splitlines() honours — \r\n-only +# normalization leaves the rest injectable as forged keys. +SEPS = {"lf": "\n", "cr": "\r", "crlf": "\r\n", "vt": "\v", "ff": "\f", + "fs": "\x1c", "gs": "\x1d", "rs": "\x1e", "nel": "\x85", + "ls": "\u2028", "ps": "\u2029"} +for name, sep in sorted(SEPS.items()): + bad = [] + if B.parse_meta(B.render_body("prose", {"note": "safe%spr: https://forged/1" % sep})).get("pr"): + bad.append("pr") + if B.parse_meta(B.render_body("prose", {"note": "safe%spre-park: in-review" % sep})).get("pre-park"): + bad.append("pre-park") + print("f-%s=%s" % (name, ("FORGED:" + ",".join(bad)) if bad else "clean")) +PY +)" +assert_contains "$mg" "a-tail=kept" "an INDENTED marker example survives a meta write" +assert_contains "$mg" "a-prose=intact" "…byte-for-byte outside the block" +assert_contains "$mg" "a-parse-pr=https://real/1" "parse_meta reads the real block, not the indented example" +assert_contains "$mg" "a-roundtrip=same" "strip → parse → render is the identity (indented example)" +assert_contains "$mg" "a2-tail=kept" "a column-0 marker example survives a meta write (#60)" +assert_contains "$mg" "a2-prose=intact" "…byte-for-byte outside the block" +assert_contains "$mg" "a2-parse-pr=https://real/1" "parse_meta reads the real block, not the quoted example" +assert_contains "$mg" "a2-roundtrip=same" "strip → parse → render is the identity (column-0 example)" +assert_contains "$mg" "leak-pre-park=None" "keys from a quoted example never leak into the parse" +assert_contains "$mg" "both-parse-pr=https://real/1" "the rightmost block is the one parsed" +assert_contains "$mg" "both-tail=kept" "stripping a two-marker body keeps the prose between them" +assert_contains "$mg" "hash-prose-only=yes" "contract_hash covers the prose only" +assert_contains "$mg" "d-block=" "render_body returned a block" +assert_contains "$mg" "note: line1 line2" "a multi-line meta value renders as ONE line" +assert_contains "$mg" "e-parse=parked, in-review --> needs-info" "an arrow-bearing value round-trips verbatim" +assert_contains "$mg" "e-prose=intact" "…and leaves the prose byte-for-byte" +assert_contains "$mg" "e-roundtrip=same" "…strip → parse → render is the identity on it" +assert_contains "$mg" "e-multi=a --> b" "an arrow that would reach line start collapses onto one line" +for sep in cr crlf ff fs gs lf ls nel ps rs vt; do + assert_contains "$mg" "f-$sep=clean" "a value split by $sep cannot forge a meta key" +done + +# (g) The LEGACY-NESTED shape, the mirror image of the quoted example: a +# pre-grammar client stored a meta VALUE carrying a verbatim opener, so the +# REAL block's interior holds a second marker. Reading the LAST opener anchors +# on that nested one and the strip cuts INSIDE the block, leaving half of it +# behind as prose. The forged key sits inside the stored block whichever opener +# wins — that content is legacy corruption and unrecoverable — but the block's +# BOUNDARY is recoverable, and that is what is pinned here: the strip lands on +# the outer opener, and a rewrite leaves the prose byte-stable while collapsing +# the body to one clean block. +mgb="$(PYTHONPATH="$SCRIPTS_DIR" python3 - <<'PY' +import _board as B + +SHAPE_B = "prose\n\n<!-- board:meta\nnote: line1\n<!-- board:meta\npr: forged\n-->\n" +print("g-strip=%r" % B.strip_meta(SHAPE_B)) +rewritten = B.render_body(SHAPE_B, B.parse_meta(SHAPE_B)) +print("g-rewrite-prose=%r" % B.strip_meta(rewritten)) +print("g-rewrite-markers=%d" % rewritten.count("<!-- board:meta")) +print("g-rewrite-stable=%s" % ( + "same" if B.render_body(rewritten, B.parse_meta(rewritten)) == rewritten + else "DIFFERS")) +PY +)" +assert_contains "$mgb" "g-strip='prose'" "a legacy NESTED marker strips at the outer opener, not the nested one" +assert_contains "$mgb" "g-rewrite-prose='prose'" "…and a rewrite of that body leaves the prose byte-stable" +assert_contains "$mgb" "g-rewrite-markers=1" "…collapsing it to a single clean block" +assert_contains "$mgb" "g-rewrite-stable=same" "…which is itself a fixed point" + +# (i) The two shapes COMPOSED: prose that QUOTES a legacy-nested example, with +# the real block further down. Judging only the gap between adjacent candidates +# fails here — the gap between the quoted outer opener and the quoted nested +# one holds nothing but that example's own `note:` entry, so it reads legal and +# the quoted opener wins, leaking its keys and truncating the body from the +# example onward. The interior is only block-legal if it is legal ALL the way +# to the closer, which the example's `-->` and the prose after it break. +mgc="$(PYTHONPATH="$SCRIPTS_DIR" python3 - <<'PY' +import _board as B + +QUOTED_NESTED = "<!-- board:meta\nnote: line1\n<!-- board:meta\npr: forged\n-->" +prose = "Docs about the block:\n\n%s\n\nMore prose." % QUOTED_NESTED +body = "%s\n\n<!-- board:meta\npr: https://real/1\n-->\n" % prose +meta = B.parse_meta(body) +print("i-parse-pr=%s" % meta.get("pr")) +print("i-leak-note=%s" % meta.get("note")) +print("i-tail=%s" % ("kept" if "More prose." in B.strip_meta(body) else "LOST")) +print("i-prose=%s" % ("intact" if B.strip_meta(body) == prose else "MANGLED")) +PY +)" +assert_contains "$mgc" "i-parse-pr=https://real/1" "a QUOTED legacy-nested example is not read as the real block" +assert_contains "$mgc" "i-leak-note=None" "…so its keys never leak into the parse" +assert_contains "$mgc" "i-tail=kept" "…and the strip keeps the prose after it" +assert_contains "$mgc" "i-prose=intact" "…byte-for-byte, quoted example included" + +# The block a client older than the grammar wrote is noncanonical — unknown +# keys, comment lines, hand spacing — and no opener clears its interior. The +# LAST candidate then wins, which is the old rightmost behavior and what +# board-body.sh's raw splice carries through untouched. +mgnc="$(PYTHONPATH="$SCRIPTS_DIR" python3 - <<'PY' +import _board as B + +body = ("Docs:\n\n<!-- board:meta\npr: fake\n-->\n\nMore prose." + "\n\n<!-- board:meta\nweird-key: x\npr: https://real/1\n-->\n") +print("nc-parse-pr=%s" % B.parse_meta(body).get("pr")) +print("nc-tail=%s" % ("kept" if "More prose." in B.strip_meta(body) else "LOST")) +PY +)" +assert_contains "$mgnc" "nc-parse-pr=https://real/1" "a block carrying an UNKNOWN key is still found (segment fallback)" +assert_contains "$mgnc" "nc-tail=kept" "…without eating the prose above it" + +# (j) The opener walk must cut interior lines exactly where parse_meta will. +# parse_meta reads the block with str.splitlines(), which honours U+2028, bare +# CR and \v among others; a walk that splits on \n alone folds a quoted +# example's `-->` and the prose behind it into ONE line that reads like a legal +# `note:` entry, so the quoted opener wins and the next meta write truncates +# the body. Same separator set as (f), coming the other way: (f) stops a value +# from FORGING a key, (j) stops one from HIDING a disqualifying line. +mgsep="$(PYTHONPATH="$SCRIPTS_DIR" python3 - <<'PY' +import _board as B + +SEPS = {"lf": "\n", "cr": "\r", "crlf": "\r\n", "vt": "\v", "ff": "\f", + "fs": "\x1c", "gs": "\x1d", "rs": "\x1e", "nel": "\x85", + "ls": "\u2028", "ps": "\u2029"} +for name, sep in sorted(SEPS.items()): + folded = "note: example%s-->%sMore prose." % (sep, sep) + body = ("Docs:\n\n<!-- board:meta\n%s\n" + "<!-- board:meta\npr: https://real/1\n-->\n" % folded) + meta = B.parse_meta(body) + bad = [] + if meta.get("pr") != "https://real/1": + bad.append("parse") + if meta.get("note"): + bad.append("leak") + if "More prose." not in B.strip_meta(body): + bad.append("truncated") + print("j-%s=%s" % (name, ",".join(bad) if bad else "clean")) +PY +)" +for sep in cr crlf ff fs gs lf ls nel ps rs vt; do + assert_contains "$mgsep" "j-$sep=clean" "a quoted example folded on $sep cannot hide its own closer" +done + +# (k) The two legacy damages COMBINED: a pre-grammar block whose value carries a +# nested marker AND which also holds an unknown key. The unknown key is not +# block-legal, so it fences off every candidate and the walk falls back — and a +# fallback to the LAST candidate lands on the NESTED opener, re-opening exactly +# the strip regression (g) pins. The fallback belongs to the segment the illegal +# line landed in, so it is that segment's FIRST opener. +mgmix="$(PYTHONPATH="$SCRIPTS_DIR" python3 - <<'PY' +import _board as B + +MIXED = ("prose\n\n<!-- board:meta\nnote: line1\n" + "<!-- board:meta\npr: forged\nweird-key: x\n-->\n") +print("k-strip=%r" % B.strip_meta(MIXED)) +rewritten = B.render_body(MIXED, B.parse_meta(MIXED)) +print("k-rewrite-prose=%r" % B.strip_meta(rewritten)) +print("k-rewrite-markers=%d" % rewritten.count("<!-- board:meta")) +PY +)" +assert_contains "$mgmix" "k-strip='prose'" "a legacy block with a nested marker AND an unknown key still strips at the outer opener" +assert_contains "$mgmix" "k-rewrite-prose='prose'" "…and a rewrite leaves the prose byte-stable" +assert_contains "$mgmix" "k-rewrite-markers=1" "…collapsing it to a single clean block" + +# Every case above is a shape somebody found the hard way, which makes them a +# record of what was looked for rather than evidence of coverage — the opener +# rule was corrected three times, twice by a shape an EARLIER version handled. +# The fuzz is the coverage: bodies composed from a component grammar with the +# intended opener known at generation time, fixed seed, ~0.3s. It catches all +# three superseded rules on its own (see the script's header for the counts). +fuzz_out="$(PYTHONPATH="$SCRIPTS_DIR" python3 "$SCRIPT_DIR/meta-grammar-fuzz.py" 2>&1 || true)" +fuzz_head="$(head -1 <<<"$fuzz_out")" +assert_contains "$fuzz_head" "divergences=0" "the opener walk survives a generated-body fuzz ($fuzz_head)" +grep -Fq "divergences=0" <<<"$fuzz_head" || echo "$fuzz_out" + +# (h) The opener walk must not rescan per marker. The rightmost walk re-ran the +# end-anchored regex once per opener — O(N²), 1.7s on a 4000-marker body, and +# snapshot() pays parse_meta per issue against a server that accepts 1MB +# bodies. The bound is deliberately loose: the regression is seconds, not +# milliseconds, and a tight bound only buys CI flake. +mgperf="$(PYTHONPATH="$SCRIPTS_DIR" python3 - <<'PY' +import time +import _board as B + +body = "prose\n\n" + ("<!-- board:meta\n" * 4000) + "note: x\n-->\n" +t0 = time.time() +B.parse_meta(body) +B.strip_meta(body) +elapsed = time.time() - t0 +print("perf-seconds=%.3f" % elapsed) +print("perf-fast=%s" % ("yes" if elapsed < 0.5 else "SLOW")) +PY +)" +assert_contains "$mgperf" "perf-fast=yes" \ + "a marker-dense body parses without a per-marker rescan (${mgperf//$'\n'/ })" + +# The OPENING marker is unrepresentable inside the block — refuse loudly rather +# than write a body whose real block contains a second marker. +mg_die() { PYTHONPATH="$SCRIPTS_DIR" python3 -c "import _board as B; B.render_body('prose', {'note': 'x\n$1'})"; } +assert_fails mg_die '<!-- board:meta' +die_out="$(mg_die '<!-- board:meta' 2>&1 || true)" +assert_contains "$die_out" "note" "the refusal names the offending key" +# Pin the message: die_out is captured with 2>&1, so a traceback echoing the +# source line would also contain "note" and pass the assert above. +assert_contains "$die_out" "cannot carry a board:meta marker token" "…with the refusal's own message, not a traceback" + +# The refusal must land BEFORE any remote write. apply_state writes the status +# label first and re-renders EVERY parsed key on the meta write that follows, so +# a die in between left the ticket relabelled with a stale note (task review C1). +mg_tid="$(run board-register.sh "Meta grammar drill" bug P2 --body-file "$SPEC_BODY")" +mg_tid="${mg_tid%% *}" +run board-transition.sh "$mg_tid" in-progress >/dev/null +mg_labels_before="$(state "s['issues']['$mg_tid']['labels']")" +mg_body_before="$(state "s['issues']['$mg_tid']['body']")" +mg_log_mark="$(wc -l < "$MOCK_GH_LOG")" +assert_fails run board-transition.sh "$mg_tid" needs-human 'forged: <!-- board:meta' +mg_log_tail="$(tail -n "+$((mg_log_mark + 1))" "$MOCK_GH_LOG")" +assert_not_contains "$mg_log_tail" "status:needs-human" "the refused transition never wrote the status label" +assert_equals "$(state "s['issues']['$mg_tid']['labels']")" "$mg_labels_before" "…labels untouched" +assert_equals "$(state "s['issues']['$mg_tid']['body']")" "$mg_body_before" "…body untouched" + +# …and the arrow-bearing note that C1's repro tore now goes through end-to-end. +out="$(run board-transition.sh "$mg_tid" needs-human 'human input needed: in-review --> needs-info')" +assert_contains "$out" "#$mg_tid: in-progress → needs-human" "an arrow-bearing park note transitions cleanly" +assert_contains "$(state "s['issues']['$mg_tid']['labels']")" "status:needs-human" "…the label moved" +assert_contains "$(state "s['issues']['$mg_tid']['body']")" "note: human input needed: in-review --> needs-info" "…and the note is the real one" + +# #60 at the CLI, not the library: the bug was found as a live failure of +# board-transition → apply_state → update_meta over a body that DOCUMENTS the +# meta block. Drive that whole path over such a body. +MARKER_BODY="$TEST_ROOT/marker-body.md" +printf '## Problem & intent\n\nThe block looks like:\n\n<!-- board:meta\npr: example\n-->\n\n## Success criteria\n\n- the prose after the example survives a meta write\n' > "$MARKER_BODY" +# Birth with a note so the ticket carries a REAL trailing block: the quoted +# example only misleads META_RE while a real block anchors the `-->\s*$` it +# spans to. The transition is then an ordinary meta write over that body. +mg60="$(run board-register.sh "Documents the meta block" bug P2 --body-file "$MARKER_BODY" --state needs-human --note "waiting on A")" +mg60="${mg60%% *}" +run board-transition.sh "$mg60" in-progress >/dev/null +mg60_body="$(state "s['issues']['$mg60']['body']")" +assert_contains "$mg60_body" "the prose after the example survives a meta write" "a gh-mode meta write keeps the prose after a quoted marker (#60)" +assert_contains "$mg60_body" "## Success criteria" "…including the headings after it" + echo if [[ "$FAILURES" -gt 0 ]]; then echo "$FAILURES test(s) FAILED" diff --git a/tests/issue-tracker/test-board-surface.sh b/tests/issue-tracker/test-board-surface.sh index 55ab3ccc60..73a0502391 100755 --- a/tests/issue-tracker/test-board-surface.sh +++ b/tests/issue-tracker/test-board-surface.sh @@ -148,6 +148,25 @@ out="$(run board-transition.sh "$n5" ready-for-implementer)" assert_contains "$out" "surface: += recommend-rpc" "T8: transition reports the re-match" assert_contains "$(state "s['issues']['$n5']['labels']")" "surface:recommend-rpc" "T8: label added on lane entry" +# ---- T8b: the meta refusal lands BEFORE every label write ------------------ +# apply_state validates the meta ahead of its own label write, but this entry +# edge writes labels of its own first — ensure_labels, then the surface +# re-match's create+add — so a note the meta grammar refuses tore the ticket: +# surface labels persisted, transition failed (PR-65 panel F4). Nothing rolls +# those back, and the ticket then reads as a lane member it never entered. +echo "T8b: a refused meta write mutates no label at all" +out="$(run board-register.sh "제목만 있는 두 번째" bug P2)" +n5b="${out%% *}" +printf 'this body also names recommend_for_student\n' | gh issue edit "$n5b" -R test/repo --body-file - >/dev/null +mark="$(wc -l < "$MOCK_GH_LOG")" +assert_fails run board-transition.sh "$n5b" ready-for-implementer 'forged: <!-- board:meta' +tail_log="$(tail -n "+$((mark + 1))" "$MOCK_GH_LOG")" +assert_not_contains "$tail_log" '"--add-label"' "T8b: no label was added" +assert_not_contains "$tail_log" '"--remove-label"' "T8b: none removed either" +assert_not_contains "$tail_log" '["label", "create"' "T8b: and none created" +assert_not_contains "$(state "s['issues']['$n5b']['labels']")" "surface:" "T8b: the ticket carries no surface label" +assert_not_contains "$(state "s['issues']['$n5b']['labels']")" "status:ready-for-implementer" "T8b: nor the lane it never entered" + # ---- T9: live-worker deferral of the relates body write -------------------- echo "T9: relates edge defers on a live bound worker" cat > "$DAEMON_HOME/aaaa0001-0000-4000-8000-000000000000.json" <<EOF diff --git a/tests/reviewing-prs/test-bootstrap-parity.sh b/tests/reviewing-prs/test-bootstrap-parity.sh new file mode 100755 index 0000000000..5555afe513 --- /dev/null +++ b/tests/reviewing-prs/test-bootstrap-parity.sh @@ -0,0 +1,317 @@ +#!/usr/bin/env bash +# Static parity fence for the two worker bootstraps. No dispatcher, no gh, no +# ports: the render is a pure function of template + P_* environment, so this +# suite carries its own minimal copy of both renderers and drives them over the +# REAL template files with one fixture that sets every placeholder to a +# traceable `X-<NAME>`. +# +# The copy is only worth what its fidelity to the dispatcher is, so the first +# section pins the dispatcher's own lines: the template path, the mode-fence +# regex, the `{{(\w+)}}` substitution, and the strip-then-substitute order. +# Everything after that is parity: sentences that must reach every worker +# whatever mode it runs in, the four separately-authored read-it-live +# rewordings pinned in both directions, the binding roster relation between a +# gh render and its api partner, and the block boundaries. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +TEMPLATE="$REPO_ROOT/skills/reviewing-prs/references/review-worker-bootstrap.md" +DISPATCH="$REPO_ROOT/skills/reviewing-prs/scripts/review-dispatch.sh" +IMPL_TEMPLATE="$REPO_ROOT/skills/implementing/references/worker-bootstrap.md" +IMPL_DISPATCH="$REPO_ROOT/skills/implementing/scripts/implement-dispatch.sh" + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +FAILS=0 +ok() { echo "ok $1"; } +bad() { local name="$1"; shift; echo "FAIL $name"; for l in "$@"; do echo " $l"; done; FAILS=$((FAILS + 1)); } +t() { # t <name> <wanted-substring> <file> + if grep -qF -- "$2" "$3"; then ok "$1"; else bad "$1" "wanted: $2" "in: $3"; fi +} +nt() { # nt <name> <forbidden-substring> <file> — the inverse of t + if grep -qF -- "$2" "$3"; then bad "$1" "must NOT appear: $2" "in: $3"; else ok "$1"; fi +} +eq() { # eq <name> <want> <got> + if [ "$2" = "$3" ]; then ok "$1"; else bad "$1" "want: [$2]" "got: [$3]"; fi +} +before() { # before <name> <first> <second> <file> + local a b + a="$(grep -nF -- "$2" "$4" | cut -d: -f1 | head -1 || true)" + b="$(grep -nF -- "$3" "$4" | cut -d: -f1 | head -1 || true)" + if [ -n "$a" ] && [ -n "$b" ] && [ "$a" -lt "$b" ]; then ok "$1" + else bad "$1" "expected before: $2" "expected after: $3" "found: [$a] [$b]"; fi +} + +echo "honesty pins — the copied renderer still matches review-dispatch.sh:" +t "dispatcher renders the same bootstrap template file" \ + 'BOOTSTRAP_TEMPLATE="$SKILL_DIR/references/review-worker-bootstrap.md"' "$DISPATCH" +t "dispatcher carries the mode-fence regex literally" \ + 't = re.sub(r"<!-- mode:([\w-]+) -->\n(.*?)<!-- /mode:\1 -->\n",' "$DISPATCH" +t "the surviving block is the run's REVIEW_MODE" \ + 'lambda m: m.group(2) if m.group(1) == mode else "", t, flags=re.S)' "$DISPATCH" +t "mode comes from the P_REVIEW_MODE binding, defaulting to pr" \ + 'mode = subs.get("REVIEW_MODE", "pr")' "$DISPATCH" +t "P_* environment fills the matching placeholder" \ + 'subs = {k[2:]: v for k, v in os.environ.items() if k.startswith("P_")}' "$DISPATCH" +t "dispatcher carries the {{(\\w+)}} substitution literally" \ + 'print(re.sub(r"\{\{(\w+)\}\}", lambda m: subs[m.group(1)], t))' "$DISPATCH" +t "an unsupplied placeholder is collected off the mode-stripped template" \ + 'missing = sorted(n for n in set(re.findall(r"\{\{(\w+)\}\}", t)) if n not in subs)' "$DISPATCH" +t "…and fails the render closed rather than shipping a hole" \ + 'sys.stderr.write("unrendered placeholders: %s\n" % " ".join(missing))' "$DISPATCH" +before "modes are stripped BEFORE the missing-placeholder check" \ + 't = re.sub(r"<!-- mode:([\w-]+) -->' 'missing = sorted(' "$DISPATCH" +before "…and the check runs before the substitution that would fill them" \ + 'missing = sorted(' 'print(re.sub(r"\{\{(\w+)\}\}"' "$DISPATCH" +# RISK_MANIFEST and REPO_FACTS are the two bindings no P_* variable supplies — +# the dispatcher injects them from files, between the mode strip and the +# missing-placeholder check. This fixture binds them like any other placeholder, +# so the fence cannot feel that injection moving: below the missing check, every +# real render would die reporting those two names as unrendered while this suite +# stayed green. Hence the position pin. +t "the risk manifest is injected, not supplied by a P_* binding" \ + 'subs["RISK_MANIFEST"] = readcap(os.environ["RISK_FILE"]) or' "$DISPATCH" +t "the repo facts are injected, not supplied by a P_* binding" \ + 'subs["REPO_FACTS"] = readcap(os.environ["FACTS_FILE"]) or' "$DISPATCH" +before "the manifest snapshots are injected AFTER the mode strip" \ + 't = re.sub(r"<!-- mode:([\w-]+) -->' 'subs["RISK_MANIFEST"] = readcap(' "$DISPATCH" +before "…and BEFORE the missing-placeholder check, which they satisfy" \ + 'subs["REPO_FACTS"] = readcap(' 'missing = sorted(' "$DISPATCH" + +echo +echo "honesty pins — the copied renderer still matches implement-dispatch.sh:" +t "implement dispatcher renders the same bootstrap template file" \ + 'BOOTSTRAP_TEMPLATE="${IMPLEMENT_BOOTSTRAP_TEMPLATE:-$SKILL_DIR/references/worker-bootstrap.md}"' "$IMPL_DISPATCH" +t "the api-only region is kept only under the api binding" \ + 'keep = os.environ.get("P_BINDING") == "api"' "$IMPL_DISPATCH" +t "implement dispatcher carries the api-only regex literally" \ + 't = re.sub(r"(?s)<!-- api-only[^>]*-->\n(.*?)<!-- /api-only -->\n",' "$IMPL_DISPATCH" +t "implement substitution leaves an unknown placeholder standing" \ + 'out = re.sub(r"\{\{(\w+)\}\}", lambda m: subs.get(m.group(1), m.group(0)), t)' "$IMPL_DISPATCH" +t "…and a standing placeholder fails the render" \ + 'sys.stderr.write("unrendered placeholder(s): %s\n" % " ".join(left))' "$IMPL_DISPATCH" + +# ---- the fixture and the two renderer copies -------------------------------- +# Every placeholder the template carries is bound to `X-<NAME>`: emptiness is +# impossible (a blank would read as "bound to nothing" and pass a naive +# assertion), and every rendered value traces back to the slot it came from. +# REVIEW_MODE is the one exception — the dispatcher's own render puts the mode +# string in that binding, because the same value selects the block. +for n in $(grep -o '{{[A-Z_]*}}' "$TEMPLATE" "$IMPL_TEMPLATE" | sed 's/.*{{//; s/}}//' | sort -u); do + export "P_$n=X-$n" +done + +render_review() { # render_review <mode> — mirrors _render_prompt (review-dispatch.sh) + P_REVIEW_MODE="$1" python3 - "$TEMPLATE" <<'PY' +import os, re, sys +t = open(sys.argv[1]).read() +subs = {k[2:]: v for k, v in os.environ.items() if k.startswith("P_")} +mode = subs.get("REVIEW_MODE", "pr") +t = re.sub(r"<!-- mode:([\w-]+) -->\n(.*?)<!-- /mode:\1 -->\n", + lambda m: m.group(2) if m.group(1) == mode else "", t, flags=re.S) +missing = sorted(n for n in set(re.findall(r"\{\{(\w+)\}\}", t)) if n not in subs) +if missing: + sys.exit("unrendered placeholders: %s" % " ".join(missing)) +sys.stdout.write(re.sub(r"\{\{(\w+)\}\}", lambda m: subs[m.group(1)], t)) +PY +} + +render_impl() { # render_impl <api|gh> — mirrors _render_bootstrap (implement-dispatch.sh) + P_BINDING="$1" python3 - "$IMPL_TEMPLATE" <<'PY' +import os, re, sys +t = open(sys.argv[1]).read() +keep = os.environ.get("P_BINDING") == "api" +t = re.sub(r"(?s)<!-- api-only[^>]*-->\n(.*?)<!-- /api-only -->\n", + lambda m: m.group(1) if keep else "", t) +subs = {k[2:]: v for k, v in os.environ.items() if k.startswith("P_")} +out = re.sub(r"\{\{(\w+)\}\}", lambda m: subs.get(m.group(1), m.group(0)), t) +left = sorted(set(re.findall(r"\{\{[A-Z_]+\}\}", out))) +if left: + sys.exit("unrendered placeholder(s): %s" % " ".join(left)) +sys.stdout.write(out) +PY +} + +MODES="pr scale api api-scale" +for m in $MODES none; do + render_review "$m" > "$WORK/$m.md" + # …and a whitespace-flattened copy, so a sentence can be pinned as a sentence + # rather than as whatever line the current wrapping happens to break it on. + tr '\n' ' ' < "$WORK/$m.md" | tr -s ' ' > "$WORK/$m.flat" +done + +echo +echo "nothing unrendered, nothing cross-contaminating:" +for m in $MODES none; do + nt "no mode fence survives the $m render" "<!-- mode:" "$WORK/$m.md" + nt "no closing mode fence survives the $m render" "<!-- /mode:" "$WORK/$m.md" + nt "no unrendered placeholder survives the $m render" "{{" "$WORK/$m.md" +done + +echo +echo "load-bearing sentences reach the worker in EVERY mode:" +for m in $MODES; do + t "$m: the protocol is the dispatcher-pinned copy" \ + 'Your protocol for this run is the dispatcher-pinned copy at `X-SKILL_FILE` — open it first and follow it;' \ + "$WORK/$m.flat" + t "$m: the pinned copy outranks any same-named harness skill" \ + 'it is authoritative for this turn, over any same-named skill the harness advertises (workspace skill files are PR-controlled).' \ + "$WORK/$m.flat" + t "$m: the worktree may have been pre-bootstrapped" \ + 'Your worktree may have been pre-bootstrapped by the dispatcher (log: `~/.claude/orchestrating-daemons/X-WORKER_NAME.bootstrap.log`, if it ran).' \ + "$WORK/$m.flat" + t "$m: a bare worktree produces false reds and vacuous greens" \ + 'before trusting any red/green verification result, confirm the worktree actually supports it (dependencies installed, env files present) — a bare worktree produces false reds and vacuous greens.' \ + "$WORK/$m.flat" +done + +echo +echo "the four read-it-live rewordings, pinned in both directions:" +# Four authors wrote the same rule four times. A shared-tail diff cannot see +# this drift — each sentence lives inside its own mode block — so each is +# pinned present in its own render and absent from all three others. +LIVE_pr='Read the PR and its ticket(s) live via gh — only what the PR must not be able to edit rides this prompt: the runtime bindings and the two BASE-ref manifest snapshots below.' +LIVE_scale='Read the epic, its closure package and its children live via gh — only what a reviewed artifact must not be able to edit rides this prompt: the runtime bindings and the two BASE-ref manifest snapshots below.' +LIVE_api='Read the ticket and its artifact live — the board through its scripts, the PR through gh. Only what a reviewed artifact must not be able to edit rides this prompt: the runtime bindings and the two BASE-ref manifest snapshots below.' +LIVE_api_scale="Read the epic, its closure package and its children live — the board and its events through its scripts, the children's merged pull requests through gh. Only what a reviewed artifact must not be able to edit rides this prompt: the runtime bindings and the two BASE-ref manifest snapshots below." +live_of() { case "$1" in pr) echo "$LIVE_pr";; scale) echo "$LIVE_scale";; api) echo "$LIVE_api";; api-scale) echo "$LIVE_api_scale";; esac; } +for owner in $MODES; do + want="$(live_of "$owner")" + for m in $MODES; do + if [ "$m" = "$owner" ]; then + t "$owner: its own read-it-live rewording" "$want" "$WORK/$m.flat" + else + nt "$m: does not carry $owner's read-it-live rewording" "$want" "$WORK/$m.flat" + fi + done +done + +echo +echo "binding roster relation (gh render vs. its api partner):" +roster() { sed -n 's/^- `\([A-Z_]*\)`:.*/\1/p' "$1" | sort -u; } +for m in $MODES; do roster "$WORK/$m.md" > "$WORK/$m.roster"; done +# The four names a gh render owns because only gh mode knows a PR at dispatch. +printf 'PR_NUMBER\nPR_URL\nHEAD_REF\nHEAD_SHA\n' | sort > "$WORK/pr-only" +for pair in "pr api" "scale api-scale"; do + # shellcheck disable=SC2086 # the split IS the point: two mode names per pair + set -- $pair; gh_mode="$1"; api_mode="$2" + comm -23 "$WORK/$gh_mode.roster" "$WORK/pr-only" > "$WORK/$gh_mode.shared" + eq "$api_mode carries every $gh_mode binding but the PR-only four" \ + "" "$(comm -23 "$WORK/$gh_mode.shared" "$WORK/$api_mode.roster" | tr '\n' ' ')" + eq "$api_mode adds exactly TICKET_BODY_FILE over $gh_mode" \ + "TICKET_BODY_FILE" "$(comm -13 "$WORK/$gh_mode.shared" "$WORK/$api_mode.roster" | tr '\n' ' ' | sed 's/ $//')" +done +# The scale extras are carried by BOTH members of the scale pair — they are the +# pair's own bindings, not something the api side adds. +for m in scale api-scale; do + t "$m carries the closure package binding" "- \`CLOSURE_PACKAGE\`: X-CLOSURE_PACKAGE" "$WORK/$m.md" + t "$m carries the integration ref binding" "- \`INTEGRATION_REF\`: X-INTEGRATION_REF" "$WORK/$m.md" +done +for m in pr api; do + nt "$m carries no closure package binding" "- \`CLOSURE_PACKAGE\`:" "$WORK/$m.md" + nt "$m carries no integration ref binding" "- \`INTEGRATION_REF\`:" "$WORK/$m.md" +done + +echo +echo "block boundaries (shared tail identical within each pair):" +# The strip anchors are PINNED, not derived from the fences — that is the whole +# point. Strip what a mode legitimately owns (its framing block and its +# read-it-live block, first line through last) plus the binding lines, and the +# two renders of a pair must agree byte for byte. A shared line dragged inside +# one mode's fence still stands in that mode's tail while its partner has lost +# it, and the tails diverge; derive the anchors from the fences instead and the +# same edit would move the strip with it and hide itself. +tail_of() { # tail_of <render> <start> <end> [<start> <end> …] + python3 - "$@" <<'PY' +import re, sys +lines = open(sys.argv[1]).read().split("\n") +for start, end in zip(sys.argv[2::2], sys.argv[3::2]): + if start not in lines: + sys.exit("block-start anchor missing: %r" % start) + i = lines.index(start) + if end not in lines[i:]: + sys.exit("block-end anchor missing after its start: %r" % end) + del lines[i:lines.index(end, i) + 1] +print("\n".join(l for l in lines if l.strip() and not re.match(r"- `[A-Z_]+`:", l))) +PY +} +mode_tail() { # mode_tail <mode> <anchors…> — writes $WORK/<mode>.tail + local m="$1"; shift + if tail_of "$WORK/$m.md" "$@" > "$WORK/$m.tail" 2> "$WORK/$m.tailerr"; then ok "$m: mode blocks strip at their pinned boundaries" + else bad "$m: mode blocks strip at their pinned boundaries" "$(cat "$WORK/$m.tailerr")"; fi +} +mode_tail pr \ + 'You are a REVIEW worker for PR #X-PR_NUMBER (X-PR_URL) in X-REPO,' \ + 'head branch X-HEAD_REF, base X-BASE_REF).' \ + 'Read the PR and its ticket(s) live via gh — only what the PR must not be' \ + 'manifest snapshots below.' +mode_tail api \ + "You are a REVIEW worker — the board's \`qagent\` lane — on ticket #X-ISSUE_NUMBER" \ + '`repo-facts.md`) and use those instead.' \ + 'Read the ticket and its artifact live — the board through its scripts, the PR' \ + 'prompt: the runtime bindings and the two BASE-ref manifest snapshots below.' +mode_tail scale \ + 'You are the SCALE REVIEWER of recomposition epic #X-ISSUE_NUMBER in' \ + 'X-SCALE_RANGE_NOTE' \ + 'Read the epic, its closure package and its children live via gh — only what' \ + 'bindings and the two BASE-ref manifest snapshots below.' +mode_tail api-scale \ + 'You are the SCALE REVIEWER of recomposition epic #X-ISSUE_NUMBER in' \ + 'The scale-review section of the protocol governs your verdicts.' \ + 'Read the epic, its closure package and its children live — the board and its' \ + 'runtime bindings and the two BASE-ref manifest snapshots below.' +for pair in "pr api" "scale api-scale"; do + # shellcheck disable=SC2086 # the split IS the point: two mode names per pair + set -- $pair + if diff -u "$WORK/$1.tail" "$WORK/$2.tail" > "$WORK/$1-$2.diff"; then + ok "$1 and $2 share an identical tail outside their own blocks" + else + bad "$1 and $2 share an identical tail outside their own blocks" "$(cat "$WORK/$1-$2.diff")" + fi +done + +echo +echo "implement lane — the api-only region and nothing else:" +render_impl api > "$WORK/impl-api.md" +render_impl gh > "$WORK/impl-gh.md" +nt "no api-only fence survives the api render" "<!-- api-only" "$WORK/impl-api.md" +nt "no api-only fence survives the gh render" "<!-- api-only" "$WORK/impl-gh.md" +nt "no unrendered placeholder survives the api render" "{{" "$WORK/impl-api.md" +nt "no unrendered placeholder survives the gh render" "{{" "$WORK/impl-gh.md" +t "the api render delivers the ticket body as a file" \ + 'is pinned at: X-TICKET_BODY_FILE — read it first; it is your statement of' "$WORK/impl-api.md" +t "the api render carries the parent pin" \ + '`PARENT_PIN`: X-PARENT_PIN — the parent ticket and the position its event' "$WORK/impl-api.md" +if tail_of "$WORK/impl-api.md" \ + 'Your assignment (the ticket body, delivered by the claim that dispatched you)' \ + 'that is the only route by which the lineage check ever sees it.' \ + > "$WORK/impl-api.tail" 2> "$WORK/impl-api.tailerr"; then + ok "the api-only region strips at its pinned boundaries" +else + bad "the api-only region strips at its pinned boundaries" "$(cat "$WORK/impl-api.tailerr")" +fi +tail_of "$WORK/impl-gh.md" > "$WORK/impl-gh.tail" +if diff -u "$WORK/impl-api.tail" "$WORK/impl-gh.tail" > "$WORK/impl.diff"; then + ok "everything outside the api-only region renders identically" +else + bad "everything outside the api-only region renders identically" "$(cat "$WORK/impl.diff")" +fi +# The implement roster is not a `- \`NAME\`:` list — its two api-only bindings +# are prose — so the roster here is the set of traceable values that reached +# the render. +xnames() { { grep -o 'X-[A-Z_]*' "$1" || true; } | sed 's/^X-//' | sort -u; } +xnames "$WORK/impl-api.md" > "$WORK/impl-api.roster" +xnames "$WORK/impl-gh.md" > "$WORK/impl-gh.roster" +eq "the api render drops none of the gh bindings" \ + "" "$(comm -23 "$WORK/impl-gh.roster" "$WORK/impl-api.roster" | tr '\n' ' ')" +eq "the api render adds exactly TICKET_BODY_FILE and PARENT_PIN" \ + "PARENT_PIN TICKET_BODY_FILE" \ + "$(comm -13 "$WORK/impl-gh.roster" "$WORK/impl-api.roster" | tr '\n' ' ' | sed 's/ $//')" + +echo +if [ "$FAILS" -gt 0 ]; then + echo "$FAILS test(s) FAILED"; exit 1 +fi +echo "all tests passed" diff --git a/tests/reviewing-prs/test-review-dispatch.sh b/tests/reviewing-prs/test-review-dispatch.sh index 37f62da0cf..146e302995 100755 --- a/tests/reviewing-prs/test-review-dispatch.sh +++ b/tests/reviewing-prs/test-review-dispatch.sh @@ -34,6 +34,14 @@ assert_not_contains() { assert_file_exists() { if [[ -f "$1" ]]; then pass "$2"; else fail "$2"; echo " missing: $1"; fi } +# A binding is only bound when it arrives with a VALUE. Anchored on the rendered +# roster line shape (- `NAME`: value), so a binding that rendered as a blank — +# the shape an unsupplied placeholder used to take — reads as unbound here. +assert_bound() { # assert_bound <prompt> <NAME> <lane> + local v; v="$(printf '%s\n' "$1" | sed -n "s/^- \`$2\`: \(.*\)$/\1/p" | head -1)" + if [[ -n "$v" ]]; then pass "\`$2\` renders with a value ($3)"; else + fail "\`$2\` renders with a value ($3)"; echo " binding line absent or empty"; fi +} # ---- environment -------------------------------------------------------------- export HOME="$TEST_ROOT/home"; mkdir -p "$HOME" @@ -416,6 +424,7 @@ echo "triggered dispatch:" out="$("$DISPATCH" 5)" assert_contains "$(cat "$SPAWN_LOG")" "spawn:--no-wait review-pr-5" "spawns --no-wait with the registry name" assert_contains "$(cat "$DAEMON_HOME/aaaa0001-0000-4000-8000-000000000000.json")" '"ticket": "7"' "ticketed review worker is bound for board-answer resume" +assert_contains "$(cat "$DAEMON_HOME/aaaa0001-0000-4000-8000-000000000000.json")" '"role": "QAGENT"' "reviewer meta records its lane so an answered park returns to in-review, not in-progress" WT="$LOCAL_REPO/.claude/worktrees/review-pr-5" assert_equals "$(git -C "$WT" rev-parse HEAD)" "$HEAD_SHA" "worktree checked out at the PR head SHA" if git -C "$WT" symbolic-ref -q HEAD >/dev/null; then @@ -448,6 +457,42 @@ assert_contains "$PROMPT" "$REPO_ROOT/skills/implementing/SKILL.md" "prompt carr assert_contains "$PROMPT" "scripts/review-engine.sh" "prompt binds the engine script path" assert_contains "$PROMPT" '`CODEX_REVIEW_MODEL`:' "prompt binds the engine model" assert_contains "$PROMPT" '`CODEX_REVIEW_EFFORT`:' "prompt binds the engine effort" +# The bindings a reviewer cannot function without, pinned on the VALUE side: +# an existing `NAME`: assertion passes just as well against a rendered blank. +assert_bound "$PROMPT" BIND_READY_FILE pr +assert_bound "$PROMPT" IMPLEMENT_PROTOCOL_FILE pr +assert_bound "$PROMPT" BOARD_SCRIPTS pr +SKILL_PIN="$(printf '%s\n' "$PROMPT" | sed -n 's/.*dispatcher-pinned copy at `\([^`]*\)`.*/\1/p' | head -1)" +if [[ -n "$SKILL_PIN" ]]; then pass "SKILL_FILE renders a protocol path"; else + fail "SKILL_FILE renders a protocol path"; fi + +# ---- an unsupplied bootstrap placeholder fails the render ---------------------- +# The renderer used to substitute an unknown {{X}} with "", so a binding a mode +# block asks for and no call site supplies shipped as a silent blank — and no +# downstream assertion can tell "empty by design" from "erased". Driven through +# a copy of the skill whose template carries one placeholder nothing fills +# (the template path is derived from the script's own dir, so the copy IS the +# lever); the sibling skills the dispatcher sources are symlinked back. +echo "unrendered placeholder fails closed:" +ALT_SKILLS="$TEST_ROOT/alt-skills"; mkdir -p "$ALT_SKILLS" +ln -s "$REPO_ROOT/skills/orchestrating-daemons" "$ALT_SKILLS/orchestrating-daemons" +cp -R "$REPO_ROOT/skills/reviewing-prs" "$ALT_SKILLS/reviewing-prs" +printf '\n- `FORGOTTEN_BINDING`: {{FORGOTTEN_BINDING}}\n' \ + >> "$ALT_SKILLS/reviewing-prs/references/review-worker-bootstrap.md" +reset_state +rm -f "$PROMPT_DIR/review-pr-5.prompt" +if ALT_OUT="$("$ALT_SKILLS/reviewing-prs/scripts/review-dispatch.sh" 5 2>&1)"; then + fail "a placeholder no call site supplies fails the dispatch" +else + pass "a placeholder no call site supplies fails the dispatch" +fi +assert_contains "$ALT_OUT" "unrendered placeholders: FORGOTTEN_BINDING" "the render failure names the placeholder" +assert_not_contains "$(cat "$SPAWN_LOG")" "review-pr-5" "no reviewer is spawned on a failed render" +if [[ -f "$PROMPT_DIR/review-pr-5.prompt" ]]; then + fail "no prompt reaches a worker on a failed render" +else + pass "no prompt reaches a worker on a failed render" +fi # Ticket ownership is exclusive: the reviewer replaces the finished implement # worker as board-answer's resume target. @@ -1704,6 +1749,11 @@ assert_not_contains "$EPIC_PROMPT" "PR_NUMBER" "scale prompt carries no PR frami assert_not_contains "$EPIC_PROMPT" "HEAD_SHA" "scale prompt carries no PR-head bindings" assert_contains "$EPIC_PROMPT" '`BASE_REF`: main' "scale prompt binds the engine base (the branch the epic integrates into)" assert_contains "$EPIC_PROMPT" '`WORKER_NAME`: review-epic-20' "scale prompt binds the registry identity the startup barrier verifies" +# The value side, on the scale call site too: the hard-fail sees a missing +# binding, never an empty-valued one, and this lane has its own P_* block. +assert_bound "$EPIC_PROMPT" BIND_READY_FILE scale +assert_bound "$EPIC_PROMPT" IMPLEMENT_PROTOCOL_FILE scale +assert_bound "$EPIC_PROMPT" BOARD_SCRIPTS scale # This epic has no `branch:` meta, so the worktree sits on the default branch # itself — there is no aggregate range to hand the engine, and the prompt must # say so instead of leaving the worker to review nothing. @@ -2325,6 +2375,28 @@ assert_not_contains "$OUT_UNEXP" "BOARD_REPO is unset" "no scale subprocess dies rm -f "$MOCK_DIR/board-issues.json" +# ---- _stamp_meta mode discipline ---------------------------------------------- +# The shared bookkeeping write (retired_from, closure_package, the gh role +# stamp). It lands on API metas too — a retirement can stamp one — and an API +# meta holds the run bearer at 0600. Recreating it at the umask default +# republishes that secret world-readable, permanently: the api path's own stamp +# preserves whatever mode it finds. Exercised directly, since no path in this +# gh-mode suite puts a bearer at rest. +echo "_stamp_meta mode discipline:" +eval "$(sed -n '/^_stamp_meta() {/,/^}/p' "$DISPATCH")" +mode_of() { python3 -c 'import os, sys +print("%o" % (os.stat(sys.argv[1]).st_mode & 0o777))' "$1"; } +printf '%s' '{"uuid": "u1", "run_bearer": "SECRET-TOKEN"}' > "$DAEMON_HOME/u1.json" +chmod 600 "$DAEMON_HOME/u1.json" +_stamp_meta u1 retired_from failure +assert_equals "$(mode_of "$DAEMON_HOME/u1.json")" "600" "a run-bearer meta survives the stamp at 0600" +assert_contains "$(cat "$DAEMON_HOME/u1.json")" '"retired_from": "failure"' "and the stamp still wrote its field" +printf '%s' '{"uuid": "u2"}' > "$DAEMON_HOME/u2.json" +chmod 600 "$DAEMON_HOME/u2.json" +_stamp_meta u2 retired_from failure +assert_equals "$(mode_of "$DAEMON_HOME/u2.json")" "600" "any narrowed meta keeps the mode it already had" +rm -f "$DAEMON_HOME/u1.json" "$DAEMON_HOME/u2.json" + echo if [[ "$FAILURES" -gt 0 ]]; then echo "$FAILURES test(s) FAILED"; exit 1