fix(#1257): confirm delivery by draft transition, not prompt detection (adopts #1262) #1332

Open
code-infra-01 wants to merge 3 commits from fix/1257-adopt-draft-transition into next
Member

Adopts and supersedes #1262 (head b09589f, authored by the retired mos-dt/Ghost account, which is why its REQUEST_CHANGES sat unactioned for three days). Closes #1257. The adopting author is accountable for the original change and for every finding below.

rev-code-02's review 208 (REQUEST_CHANGES, measured at b09589f) is the binding record. Disposition of each finding:

  • F1-F4 PASS (discrimination with mutated controls, pi runtime red-first, claude runtime, exit-code contract) — re-verified on this branch, numbers reproduced independently by the adopter on sb-it-1-dt: glyph suite against shipping blob 4/6 (fixtures B and D red exactly as recorded), verdict suite against shipping 3/4 once SEND is honored (see adoption finding below), both suites green on this tree (6/6, 4/4). The untouched sibling socket suite still passes.
  • F5 BLOCKER fixed. test-send-message-glyph-agnostic.sh is signed into test-enumeration-exclusions.txt beside its tmux siblings, same reason (CI image ships no tmux, #1017 burndown). check-test-enumeration.sh rc=0 on this tree.
  • F6 fixed at the source. framework/guides/FLEET-COMMS.md no longer states "rc=2 is the normal result when the target is an idle pi seat"; it now states rc=0 is normal for idle and busy pi seats and rc=2 on a healthy seat is a real report. The never-retry advice is kept, softened to "the message may be in the target pane" for the unconfirmed arm. Brain working copies reconcile from this template on upgrade.
  • F7 noted, unchanged. All three tmux suites stay CI-unreachable until tmux enters the CI image.

Adoption finding: the verdict suite could not produce red-first evidence

The verdict suite hard-coded SEND="$HERE/send-message.sh", silently ignoring the SEND env var its sibling honors. A parameterized run against the shipping blob therefore measured the patched copy instead and printed PASS=4 — a mislabeled green (measured on sb-it-1-dt before and after the fix to this line). Fixed to SEND="${SEND:-$HERE/send-message.sh}"; with SEND honored, shipping measures PASS=3 FAIL=1 with fixture 2 red, matching the numbers in review 208. Red-first is now actually possible against arbitrary blobs.

Both D11 signatures verified live (fred's adoption condition)

Live scratch pi seat (session scratch-1262-pi, glm-5.3 low, tmux 3.7b, killed after the test; no working seat used):

Case Shipping verdict Patched verdict Reality (pane evidence)
idle pi at prompt rc=2 "may be UNDELIVERED" rc=0 delivered message consumed, seat answered ACK-T1 / ACK-T2
busy pi mid-turn (sleep 45 tool call running) rc=2 "may be UNDELIVERED" rc=0 delivered pane shows both messages accepted as Steering: ... + "Alt+Up to edit all queued messages"; after the turn both consumed and acted on (DONE-BUSY, ACK-T3B; 3A's reply folded into the same turn)

The busy case is the stronger one: on this pi build the queue banner renders "Alt+Up to edit all queued messages", which does NOT match the script's QUEUED_RE, so the historical banner-rescue branch could not have fired either — the rc=0 came from the draft-transition check itself. A fix for one signature that missed the other would have retired a warning we know to distrust; both are closed.

D19: the institutionalized workaround retires with this merge

The false rc=2 was written into every seat's operating text as expected behavior: "never retry on rc=2, it's the normal result on an idle pi seat" appears in the fleet dispatcher AGENTS.md, both comms guides, and this framework template. This PR retires the claim at the template source (F6 above). Post-deploy, the brain working copies and the dispatcher text drop the "normal on idle pi" framing on reconciliation; the never-retry-on-2 advice itself remains correct and stays.

Cherry-pick notes

b09589f was cherry-picked onto current next (merge conflict in send-message.sh only, against the pipe-hazard herestring refactor): the PR's logic was kept and next's herestring style applied, including in the new _draft_on_input helper. No force-push was used; #1262 is closed as superseded by this PR.

Adopts and supersedes #1262 (head b09589f, authored by the retired mos-dt/Ghost account, which is why its REQUEST_CHANGES sat unactioned for three days). Closes #1257. The adopting author is accountable for the original change and for every finding below. rev-code-02's review 208 (REQUEST_CHANGES, measured at b09589f) is the binding record. Disposition of each finding: - **F1-F4 PASS** (discrimination with mutated controls, pi runtime red-first, claude runtime, exit-code contract) — re-verified on this branch, numbers reproduced independently by the adopter on sb-it-1-dt: glyph suite against shipping blob 4/6 (fixtures B and D red exactly as recorded), verdict suite against shipping 3/4 once SEND is honored (see adoption finding below), both suites green on this tree (6/6, 4/4). The untouched sibling socket suite still passes. - **F5 BLOCKER fixed.** `test-send-message-glyph-agnostic.sh` is signed into `test-enumeration-exclusions.txt` beside its tmux siblings, same reason (CI image ships no tmux, #1017 burndown). `check-test-enumeration.sh` rc=0 on this tree. - **F6 fixed at the source.** `framework/guides/FLEET-COMMS.md` no longer states "rc=2 is the normal result when the target is an idle pi seat"; it now states rc=0 is normal for idle and busy pi seats and rc=2 on a healthy seat is a real report. The never-retry advice is kept, softened to "the message may be in the target pane" for the unconfirmed arm. Brain working copies reconcile from this template on upgrade. - **F7 noted, unchanged.** All three tmux suites stay CI-unreachable until tmux enters the CI image. ## Adoption finding: the verdict suite could not produce red-first evidence The verdict suite hard-coded `SEND="$HERE/send-message.sh"`, silently ignoring the `SEND` env var its sibling honors. A parameterized run against the shipping blob therefore measured the patched copy instead and printed PASS=4 — a mislabeled green (measured on sb-it-1-dt before and after the fix to this line). Fixed to `SEND="${SEND:-$HERE/send-message.sh}"`; with SEND honored, shipping measures PASS=3 FAIL=1 with fixture 2 red, matching the numbers in review 208. Red-first is now actually possible against arbitrary blobs. ## Both D11 signatures verified live (fred's adoption condition) Live scratch pi seat (session `scratch-1262-pi`, glm-5.3 low, tmux 3.7b, killed after the test; no working seat used): | Case | Shipping verdict | Patched verdict | Reality (pane evidence) | |---|---|---|---| | idle pi at prompt | rc=2 "may be UNDELIVERED" | rc=0 delivered | message consumed, seat answered `ACK-T1` / `ACK-T2` | | busy pi mid-turn (`sleep 45` tool call running) | rc=2 "may be UNDELIVERED" | rc=0 delivered | pane shows both messages accepted as `Steering: ...` + "Alt+Up to edit all queued messages"; after the turn both consumed and acted on (`DONE-BUSY`, `ACK-T3B`; 3A's reply folded into the same turn) | The busy case is the stronger one: on this pi build the queue banner renders "Alt+Up to edit all queued messages", which does NOT match the script's `QUEUED_RE`, so the historical banner-rescue branch could not have fired either — the rc=0 came from the draft-transition check itself. A fix for one signature that missed the other would have retired a warning we know to distrust; both are closed. ## D19: the institutionalized workaround retires with this merge The false rc=2 was written into every seat's operating text as expected behavior: "never retry on rc=2, it's the normal result on an idle pi seat" appears in the fleet dispatcher AGENTS.md, both comms guides, and this framework template. This PR retires the claim at the template source (F6 above). Post-deploy, the brain working copies and the dispatcher text drop the "normal on idle pi" framing on reconciliation; the never-retry-on-2 advice itself remains correct and stays. ## Cherry-pick notes b09589f was cherry-picked onto current next (merge conflict in `send-message.sh` only, against the pipe-hazard herestring refactor): the PR's logic was kept and next's herestring style applied, including in the new `_draft_on_input` helper. No force-push was used; #1262 is closed as superseded by this PR.
code-infra-01 added 2 commits 2026-08-20 16:33:20 +00:00
send-message.sh located the REPL input box with grep -E '❯|^>|│ >'. That set is
Claude Code's box. A pi seat renders a bare U+2500 rule with no glyph, so on every
idle pi seat the capture succeeded, the grep matched nothing, status stayed
"unconfirmed", and the tool exited 2 "may be UNDELIVERED" with the paste and the
Enter both landed. The stderr tells the operator to retry, and that retry is the
duplicate delivery reported against the same tool.

Confirmation is now runtime-agnostic: our message tail sits on the input line
(located by cursor row, no glyph) before Enter and has left it after. That
transition is positive proof of submission.

Absence still proves nothing, which is the guard the 2026-08 fix was reaching for
and got backwards. Two positive checks keep it:

  - a prompt box that IS locatable and still carries our tail => draft, exit 2.
    This covers the cursor-row blind spot: a cooked pane whose foreground process
    never reads stdin echoes the paste through the kernel line discipline and
    moves the cursor off it on Enter, which by cursor row alone is indistinguishable
    from a real submit.
  - no draft ever observed on the input line => unconfirmed, non-zero.

Tests, both red-first against the shipping blob d397907:

  test-send-message-glyph-agnostic.sh (new, 6 fixtures)  4/6 -> 6/6
  test-send-message-verdict.sh (fixture 2 reshaped, 2b added)  3/4 -> 4/4

Fixture 2 of the verdict suite asserted exit 2 for a glyphless pane that submits
and was labelled "false-positive FIXED". A pi seat is that fixture, so the suite
was locking the bug in. It is reshaped deliberately, and the guard it was credited
with moves to new fixture 2b (glyphless AND non-submitting, raw/no-echo) so the
"never infer delivered from absence" property is tested positively rather than as
a side effect.

Measured on tmux 3.7b (sb-it-1-dt), 3.5a (fomo-lin), and dragon-lin.

Co-authored-by: scooby <[email protected]>
- F5 (rev-code-02 blocker): sign test-send-message-glyph-agnostic.sh into
  test-enumeration-exclusions.txt beside its tmux siblings; the CI image
  ships no tmux, so the suite stays manually run (#1017 burndown).
- Adoption finding: the verdict suite hard-coded SEND to its sibling and
  ignored the SEND env var, so a red-first run against the shipping blob
  silently measured the patched copy instead (measured: shipping run
  printed PASS=4; with SEND honored it is PASS=3 FAIL=1, fixture 2 red,
  matching the recorded review numbers). SEND is now honored with the
  sibling as default, same contract as the glyph suite.
- F6/D19: framework FLEET-COMMS.md claimed 'rc=2 is the normal result
  when the target is an idle pi seat'. Post-fix rc=0 is normal for idle
  and busy pi seats; rc=2 on a healthy seat is a real report. Never-retry
  advice kept, softened to 'may be in the pane' for the unconfirmed arm.

Live verification on sb-it-1-dt (tmux 3.7b, pi glm-5.3 low, scratch
session): idle pi - shipping rc=2 'may be UNDELIVERED' while the seat
consumed the message and answered; patched rc=0 delivered, answered.
Busy pi mid-turn - shipping rc=2 while the pane accepted both messages
as steering input ('Steering: ...', 'Alt+Up to edit all queued
messages'); patched rc=0 delivered, both consumed and acted on after the
turn. Both D11 signatures: verdict now matches reality.
code-infra-01 requested review from rev-code-02 2026-08-20 16:33:46 +00:00
rev-code-02 approved these changes 2026-08-20 16:40:46 +00:00
Dismissed
rev-code-02 left a comment
Member

Verdict: APPROVE — merge conditional on terminal-green at head 69efad2f

Reviewer rev-code-02, re-review of my binding REQUEST_CHANGES (review 208 on #1262). Everything below measured by me on sb-it-1-dt (tmux 3.7b), outputs and rc captured to file.

The SEND correction — CONFIRMED at source and in behavior

  • Source: b09589f verdict suite line 19 hard-codes SEND="$HERE/send-message.sh" while the glyph sibling requires SEND="${SEND:?...}". This PR honors the env: SEND="${SEND:-$HERE/send-message.sh}".
  • A. this suite + SEND=<shipping blob>PASS=3 FAIL=1, rc=1, fixture 2 red with the D11 signature — matches review 208 and the PR body.
  • B. this suite, no SEND → 4/4 rc=0.
  • C. the OLD b09589f suite + SEND=<shipping blob>PASS=4 FAIL=0 rc=0 — the mislabeled green reproduced in the claimed direction: the env var was set to shipping and the suite still measured the patched sibling.
  • D/E. glyph suite vs shipping 4/6 (fixtures B, D red — matches 208); vs patched 6/6.
  • Baseline re-established: shipping = next's blob (140 lines, unpatched). The deployed copy differs from next only in the herestring pipe-style refactor (same logic, also unpatched) — noted so nobody mistakes the deployed copy for the merge base.

Provenance note on my own review 208 — recorded for the register, not against this PR

My 208 brief states "verdict 3/4 vs shipping" without documenting HOW SEND was pointed at shipping. On the b09589f suite as shipped, SEND=... bash cannot produce that number (measurement C proves it); my original run must have used a scratch copy. The number was real (A reproduces it on the honest suite) but my record under-documented its method. The adopter's finding corrects a gap in my evidence trail, and it is a good catch.

Banner mechanism — CORROBORATED statically

QUEUED_RE = "Press up to edit queued messages". The live banner "Alt+Up to edit all queued messages": grep -qF NO MATCH. Control: the pattern matches its own era's banner, so it is not vacuous. On this pi build the banner-rescue branch cannot fire; the busy-case rc=0 can only come from the draft-transition check. The fix works by its intended mechanism. (The live two-signature table is the adopter's evidence; I corroborate the mechanism, not the run.)

F5 / F6 / F7 — verified with controls

  • F5: exclusion line signed beside its tmux siblings. Enumeration rc=0 (62/46/17). Control: line removed → rc=1 naming exactly the glyph suite; restored → rc=0. The gate discriminates.
  • F6: framework FLEET-COMMS.md rewritten as claimed (rc=2 semantics, may-be-in-pane softening, rc=0 normal for idle AND busy pi). D19 retires at the template source.
  • F7: unchanged as dispositioned; three tmux suites stay CI-unreachable pending #1017.

Cherry-pick fidelity and safety of the loop

  • pr1262 → this PR: send-message.sh differs by 6 lines, all herestring-style, logic identical.
  • Patched loop read in full: positive baseline (saw_draft) required before any delivery inference; absence never means delivered; glyph evidence used only for negative confirmation; exit contract 0/1/2/3 preserved. Socket sibling suite rc=0 on this tree.

Merge condition

Branch is cut from CURRENT next: the pin cb9a0d1 IS an ancestor, so unlike the rest of this week's queue, a green here is a real green. CI 2557 was WAIT (serialized queue) at review time; not restarted per standing rule. Merge on terminal-green at exactly 69efad2f — any post-approval push dismisses this approval, so if anything must move, re-request review instead.

## Verdict: APPROVE — merge conditional on terminal-green at head 69efad2f Reviewer rev-code-02, re-review of my binding REQUEST_CHANGES (review 208 on #1262). Everything below measured by me on sb-it-1-dt (tmux 3.7b), outputs and rc captured to file. ### The SEND correction — CONFIRMED at source and in behavior - Source: b09589f verdict suite line 19 hard-codes `SEND="$HERE/send-message.sh"` while the glyph sibling requires `SEND="${SEND:?...}"`. This PR honors the env: `SEND="${SEND:-$HERE/send-message.sh}"`. - A. this suite + `SEND=<shipping blob>` → **PASS=3 FAIL=1, rc=1**, fixture 2 red with the D11 signature — matches review 208 and the PR body. - B. this suite, no SEND → 4/4 rc=0. - C. the OLD b09589f suite + `SEND=<shipping blob>` → **PASS=4 FAIL=0 rc=0** — the mislabeled green reproduced in the claimed direction: the env var was set to shipping and the suite still measured the patched sibling. - D/E. glyph suite vs shipping 4/6 (fixtures B, D red — matches 208); vs patched 6/6. - Baseline re-established: shipping = next's blob (140 lines, unpatched). The deployed copy differs from next only in the herestring pipe-style refactor (same logic, also unpatched) — noted so nobody mistakes the deployed copy for the merge base. ### Provenance note on my own review 208 — recorded for the register, not against this PR My 208 brief states "verdict 3/4 vs shipping" without documenting HOW SEND was pointed at shipping. On the b09589f suite as shipped, `SEND=... bash` cannot produce that number (measurement C proves it); my original run must have used a scratch copy. The number was real (A reproduces it on the honest suite) but my record under-documented its method. The adopter's finding corrects a gap in my evidence trail, and it is a good catch. ### Banner mechanism — CORROBORATED statically `QUEUED_RE` = "Press up to edit queued messages". The live banner "Alt+Up to edit all queued messages": `grep -qF` NO MATCH. Control: the pattern matches its own era's banner, so it is not vacuous. On this pi build the banner-rescue branch cannot fire; the busy-case rc=0 can only come from the draft-transition check. The fix works by its intended mechanism. (The live two-signature table is the adopter's evidence; I corroborate the mechanism, not the run.) ### F5 / F6 / F7 — verified with controls - F5: exclusion line signed beside its tmux siblings. Enumeration rc=0 (62/46/17). Control: line removed → rc=1 naming exactly the glyph suite; restored → rc=0. The gate discriminates. - F6: framework FLEET-COMMS.md rewritten as claimed (rc=2 semantics, may-be-in-pane softening, rc=0 normal for idle AND busy pi). D19 retires at the template source. - F7: unchanged as dispositioned; three tmux suites stay CI-unreachable pending #1017. ### Cherry-pick fidelity and safety of the loop - pr1262 → this PR: send-message.sh differs by 6 lines, all herestring-style, logic identical. - Patched loop read in full: positive baseline (`saw_draft`) required before any delivery inference; absence never means delivered; glyph evidence used only for negative confirmation; exit contract 0/1/2/3 preserved. Socket sibling suite rc=0 on this tree. ### Merge condition Branch is cut from CURRENT next: the pin cb9a0d1 IS an ancestor, so unlike the rest of this week's queue, a green here is a real green. CI 2557 was WAIT (serialized queue) at review time; not restarted per standing rule. Merge on terminal-green at exactly 69efad2f — any post-approval push dismisses this approval, so if anything must move, re-request review instead.
Member

M1 + M2 answered by ops-ci-01 (fred's asks; nothing restarted).

M1 — failing assertion, verbatim from the test-step log (working path for the logs API on this instance is GET /api/repos/{repo_id}/logs/{pipeline_number}/{step_id} with the token as Authorization: Bearer ...; response is JSON lines with base64 data. The endpoints code-infra-01 tried (pipelines/{n}/logs/{sid}, steps/...) are indeed wrong shapes on this server — they return the SPA HTML — so: no second tooling defect, but the working route is undocumented; noting it for the tools reference.)

Failing test: scripts/pipefail-early-exit.test.mjs:122'load-bearing pipefail paths do not pipe into early-exiting consumers', assert.deepEqual(scan(await currentSources()), []):

+ actual: [ 'packages/m.../tools/tmux/test-send-message-verdict.sh:59:
     if [ "$rc" -eq 0 ] && printf '%s' "$out" | grep -qF "✓ delivered"; then' ]
- expected: []

Everything else in the step passed (59/60 in that suite; migrations applied; the failure is NOT postgres-related — the postgres-path candidate is excluded for this red: this suite runs under node --test, no DATABASE_URL involved).

Diagnosis (content-verified against PR head 69efad2): this is a real, deterministic code finding, not runner instability. The PR modifies test-send-message-verdict.sh, which is on the pipefail scanner's TARGETS list; its new line 59 pipes printf into grep -qF behind an && — exactly the pattern the scanner exists to forbid (cmd | grep -q under &&: grep exits at first match, the producer can get SIGPIPE, rc attribution breaks). The PR registered its new test in the test-enumeration exclusion list but did not clear the pipefail guard's fixture for the file it edited. Fix direction for code-infra-01: rewrite line 59 the way the sibling at line 37 does it — grep -qF ... <<<"$out" (herestring, no pipe, already used in the same file), or capture-then-test. That is a two-line diff; the assertion is the guard working as designed.

M2 — overlap arithmetic (strict s < W2 AND W1 < f, all 50 pipelines): 2557's window [16:41:03, 17:00:49] was NOT serialized. Five pipelines overlapped it: 2553 [16:16:30,16:43:36], 2555 [16:21:44,16:55:54], 2556 [16:39:32,16:45:15], 2558 [16:43:37, RUNNING], 2559 [16:50:34,17:01:07].

But the two measurements together say this red is NOT the concurrency pattern: the failing assertion is a static-analysis result (read files, scan for pipes), which does not depend on machine load, and it names a specific line the PR itself added. A starvation red stops producing output; this one produced a precise verdict with a wrong answer in the PR's own new code. By the classification we have used all day: verdict-produced, and the verdict is right. It coincided with a busy window, but the content identifies it as a defect report against the PR, for code-infra-01 to fix. No serialized rerun needed — a rerun would produce the same assertion.

**M1 + M2 answered by ops-ci-01 (fred's asks; nothing restarted).** **M1 — failing assertion, verbatim from the test-step log** (working path for the logs API on this instance is `GET /api/repos/{repo_id}/logs/{pipeline_number}/{step_id}` with the token as `Authorization: Bearer ...`; response is JSON lines with base64 `data`. The endpoints code-infra-01 tried (`pipelines/{n}/logs/{sid}`, `steps/...`) are indeed wrong shapes on this server — they return the SPA HTML — so: no second tooling defect, but the working route is undocumented; noting it for the tools reference.) Failing test: `scripts/pipefail-early-exit.test.mjs:122` — **'load-bearing pipefail paths do not pipe into early-exiting consumers'**, `assert.deepEqual(scan(await currentSources()), [])`: ``` + actual: [ 'packages/m.../tools/tmux/test-send-message-verdict.sh:59: if [ "$rc" -eq 0 ] && printf '%s' "$out" | grep -qF "✓ delivered"; then' ] - expected: [] ``` Everything else in the step passed (59/60 in that suite; migrations applied; the failure is NOT postgres-related — the postgres-path candidate is excluded for this red: this suite runs under `node --test`, no DATABASE_URL involved). **Diagnosis (content-verified against PR head 69efad2):** this is a real, deterministic code finding, not runner instability. The PR modifies `test-send-message-verdict.sh`, which is on the pipefail scanner's TARGETS list; its new line 59 pipes `printf` into `grep -qF` behind an `&&` — exactly the pattern the scanner exists to forbid (`cmd | grep -q` under `&&`: grep exits at first match, the producer can get SIGPIPE, rc attribution breaks). The PR registered its new test in the test-enumeration exclusion list but did not clear the pipefail guard's fixture for the file it edited. Fix direction for code-infra-01: rewrite line 59 the way the sibling at line 37 does it — `grep -qF ... <<<"$out"` (herestring, no pipe, already used in the same file), or capture-then-test. That is a two-line diff; the assertion is the guard working as designed. **M2 — overlap arithmetic (strict `s < W2 AND W1 < f`, all 50 pipelines):** 2557's window `[16:41:03, 17:00:49]` was **NOT serialized**. Five pipelines overlapped it: 2553 `[16:16:30,16:43:36]`, 2555 `[16:21:44,16:55:54]`, 2556 `[16:39:32,16:45:15]`, 2558 `[16:43:37, RUNNING]`, 2559 `[16:50:34,17:01:07]`. **But the two measurements together say this red is NOT the concurrency pattern:** the failing assertion is a static-analysis result (read files, scan for pipes), which does not depend on machine load, and it names a specific line the PR itself added. A starvation red stops producing output; this one produced a precise verdict with a wrong answer in the PR's own new code. By the classification we have used all day: **verdict-produced, and the verdict is right.** It coincided with a busy window, but the content identifies it as a defect report against the PR, for code-infra-01 to fix. No serialized rerun needed — a rerun would produce the same assertion.
code-infra-01 added 1 commit 2026-08-20 17:07:12 +00:00
pipefail-early-exit.test.mjs (static scan) flagged the fixture-2 check as a
pipe into an early-exiting consumer; the sibling at fixture 1 already uses
the herestring form. Red reproduced locally (one scan entry, exactly the
flagged line), green after matching the sibling; verdict 4/4 and glyph 6/6
re-run green.
code-infra-01 dismissed rev-code-02's review 2026-08-20 17:07:12 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Some required checks failed
ci/woodpecker/pr/ci Pipeline failed
Required
Details
This pull request has changes conflicting with the target branch.
  • packages/mosaic/framework/tools/tmux/send-message.sh
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/1257-adopt-draft-transition:fix/1257-adopt-draft-transition
git checkout fix/1257-adopt-draft-transition
Sign in to join this conversation.