fix(bin): prove steer delivery with payload deltas - #2814
Closed
kunchenguid wants to merge 28 commits into
Closed
Conversation
…omposer A steer that the harness never took could be recorded as delivered. The pre-Enter check that was supposed to catch that located the composer inside a pane capture and read its contents - but the composer region is not a fact the terminal carries. Locating it means inferring it from one vendor's current drawing conventions, across harness x version x backend x capture fidelity x cursor availability x pane width x transient overlays. Two successive attempts to hold that inference spent twenty review rounds on narrow shape fixes without converging, and one cosmetic keybinding-hint row under opencode's composer was enough to make the region parser answer "no composer here" for a healthy steer. Ask a different question. fm_composer_delivery_delta_verdict takes two whole captures and the payload and asks only whether the payload became newly present between them - never where the composer is. Acceptance needs both a novelty test (the payload's normalized tail appears among the rows a diff reports as added) and an occurrence-count test (its whole-screen count strictly rose). They fail in different directions, so their conjunction can only move toward refusal: a pane that redraws every row defeats novelty, and the count still refuses. The earlier scrollback false pass - a stale copy of the payload already on screen while the real send was swallowed - is excluded by time rather than by location, so it cannot arise at all: a copy that was already there is in the before capture, so it is neither an addition nor a new occurrence. Location is a shape question; time is not. Every adapter's obligation on the send path collapses to capture, type, capture. The capability descriptor, cursor row, and identity probe are gone from it, and with them the styled/plain fork that made the same real screen accept under tmux and refuse under cmux, orca, and herdr's plain fallback. zellij also loses its own private acceptance rule and now proves delivery the way every other backend does; fm_composer_extract_selected_content had no remaining caller and is deleted with it, while the shape engine it used stays in place for the classifier. There is deliberately no bypass. A harness whose composer cannot be proven is refused rather than steered blind, and the existing exit-3 "unconfirmed" status already covers the post-Enter side. The pre-Enter proof only gates pressing Enter: the shape-aware post-Enter confirmation is untouched, so the two proofs bracket the Enter and the pre-Enter half never has to be airtight alone. Also kept from the abandoned attempt, because they were sound and separable: durable full-payload pending-reply recovery with its per-correlation lock, and the Claude launch-readiness gate. That gate is deliberately demoted - a modal that swallows produces no delta, so the delivery proof refuses whatever the gate scorer thinks. Its remaining jobs are refusing to type into a modal at all (codex's numbered dialog treats typed digits as control input, and typing the probe into it exited the pane), giving a specific diagnostic, and answering the spawn-time question where there is no payload to diff against. Two deviations from the approved design, both from evidence measured here: - The anchor is capped at 24 normalized characters, not 48. A single-row composer that horizontally scrolls shows only a window of its buffer: the away-mode end-to-end reference leaves 33 characters visible, so 48 turns that healthy injection into a refusal (bisected: 32 passes, 34/37/48 fail). This does not trade safety for reach, because acceptance still requires the anchor to be both newly added and newly counted in the same instant it was typed. - fm_composer_extract_selected_content is deleted rather than kept. The design kept it only because zellij still consumed it, and this change removes that consumer. Verified live against every installed harness in isolated tmux servers: claude 2.1.239, codex 0.147.0, opencode 1.14.46, pi and pi-signed 0.84.1, grok 1.0.5, muse 0.2.1, and cursor 2026.08.11 all accept a really-typed payload and refuse an untyped pane on both capture fidelities. Cursor is covered for the first time, since the proof reads no cursor. Grok passes the delivery proof in the same run in which it fails the empty-composer shape matrix - a vendor drift that predates this change and needs its own work item, and the sharpest statement of what the new shape buys. Portable regressions run everywhere from real captures checked in under tests/fixtures/composer-delta/: eleven scenarios across eight harnesses, asserted on plain and ANSI captures and required to agree, including three real swallows. One of those - opencode's vendor update modal - is not recognised by the gate scorer and is refused anyway, which is the recorded reason that scorer must never be load-bearing for send safety.
The pre-Enter delivery proof needs 24 characters of anchor before an appearance on screen means anything. Real steers often have fewer - `2`, `y`, `approved`, `/exit` - and both ways out of that were wrong: refusing them breaks ordinary fleet traffic including every lifecycle exit, and giving them a weaker test reopens the false delivery the proof exists to close, because a live agent pane redraws rows containing `2` constantly. So neither happens. A short payload is now typed with a fresh random suffix that lifts it to a full-length anchor, proven at that anchor, and then the suffix alone is erased and the erase is proven. The agent still receives exactly the payload the caller wrote, and what was proven to be in the composer contains the payload's own characters. The erase proof reads two counts. An incomplete erase can only leave `<payload><suffix-prefix>` behind, so the nonce's first character still sits immediately after the payload - that boundary string must not have gained an occurrence. An over-erase eats the payload's tail instead, so the payload's own count must be unchanged; the suffix shares no character with the payload, which is what makes that count decidable. Also: - Drop the pre-type composer classification the short-payload attempt had reintroduced, and the environment-variable exemption that let a caller declare its own postcondition. A short exit command needs no exemption now that it carries a full-length anchor. - Move the type-and-prove sequence into one shared owner every adapter calls with three primitives, replacing five copies of the ordering. - Add the erase key per backend: BSpace on tmux and Backspace on zellij, both verified against a real pane; lowercase `backspace` on herdr and cmux. Orca's send API exposes no such key, so it refuses a short payload before typing rather than stranding a suffix it cannot remove. - Give a stranded suffix a named warning carrying the exact text and suffix length, so a typed-unproven request can actually be settled.
The verification envelope only delivers the caller's exact payload if the backend's erase key removes exactly one character per press, and that is a vendor fact per backend: the key name belongs to each tool's own vocabulary and the byte it resolves to belongs to the pane. A stub can only confirm the spelling its author already assumed. - Add a live guard to each backend's real smoke test, failing with the backend and version named. - Record the dated results: tmux BSpace and zellij Backspace both remove exactly one character, verified against real panes. herdr and cmux carry the mapping but are not live-verified here, and the evidence table says so rather than implying they are. Orca has no such key at all, so it refuses a short payload before typing. - Record that a raw backspace byte through the literal-text channel was measured and rejected: 0x08 erased nothing against a real tmux pane and 0x7f did, which is exactly the convention a named key delegates to the backend. - Teach the shared fake-pane helper the erase key and a submitted log, so a stub models a real composer instead of the raw type log. A stub that ignores the erase still refuses, which stays the intended proof. - Classify the composer-delta fixtures README, which landed unclassified earlier on this branch and failed the documentation audience check. - Quote two probe-absent sentinel assignments that shellcheck reads as arithmetic once the shared library grows.
The lint step's own fixer wedged before it ran, so these are applied directly. Each is fixed at the source rather than blanket-suppressed. - Quote the hyphenated string sentinels shellcheck reads as arithmetic subtraction: the envelope's error sentinel and the pending-reply escalation kinds. Quoting is the real fix and removes an ambiguity a human reader shares. - Annotate the three test stubs that shadow a function the code under test calls indirectly, which shellcheck cannot see. Narrow per-site directives with a reason, never a file-wide disable. - Drop SIM_WIRE. It was assigned and never read: the assertions read the wire from its file instead, because a variable set inside the command-substitution subshell never reaches the caller. The file stays; only the dead variable goes.
Confidence Score: 4/5The trailing-newline recovery defect should be fixed before merging because it violates the promised lossless durable payload recovery. Automatic recovery captures the persisted request through shell command substitution, so valid trailing newline bytes are removed before the repost is sent. Files Needing Attention: bin/fm-pending-reply-lib.sh, tests/fm-pending-reply.test.sh Reviews (1): Last reviewed commit: "no-mistakes(document): Document unproven..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| local rec=$1 corr body token msg | ||
| corr=$(fm_pending_reply_get "$rec" corr_id) | ||
| summary=$(fm_pending_reply_get "$rec" request_summary) | ||
| body=$(fm_pending_reply_request_body "$rec") || return 1 |
There was a problem hiding this comment.
When a persisted request ends with newline characters, body=$(fm_pending_reply_request_body "$rec") removes them before constructing the recovery message, causing automatic recovery to repost a payload that differs from the original request.
Owner
Author
|
Superseded and dropped by captain decision 2026-08-22: the payload-delta/three-state composer-verification approach is being replaced wholesale by the inbox+doorbell steering design (data/fm-send-reliability-reframe-s1/report.md), which removes the composer-verification problem entirely rather than solving it. Not landing this branch. Follow-up implementation tracked as fm-send-inbox-doorbell-r1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Re-implement fm-send steer delivery verification using the captain-APPROVED payload-delta proof, replacing the shape-parsing read-back that was abandoned. This is firstmate's SHARED, TRACKED transport core - the steer channel the whole fleet relies on.
APPROVED DESIGN (captain decision 1): implement the delivery proof in the ONE owner bin/fm-composer-lib.sh. Normalize both captures and the payload identically (strip ANSI/box-drawing/whitespace, keep row structure), anchor on the last min(24,len) normalized chars of the payload TAIL, and accept ONLY when the anchor is both (A) newly ADDED in the before->after row diff AND (B) its whole-screen occurrence COUNT increased. This excludes the round-1 scrollback false-pass by TIME not location, so that false-pass is impossible by construction. Keep it refusing rather than proceeding (non-zero + clear diagnostic, never a silent partial or false success) and backend-independent.
CAPTAIN DECISION 2 (fresh PR): branch off main; carry over ONLY the cleanly-separable KEEP set from the now-CLOSED branch fm/remote-send-confirm-readback-fix-r1 - the pending-reply durability half (full-payload durable recovery), the launch-readiness gate (claude_wait_for_ready in bin/fm-spawn.sh, fm_composer_screen_is_gated, fm_composer_pre_type_ok's GATED branch only), the locking, and the behavior-through-fm-send tests. Do NOT carry the shape-parsing read-back. PR #2761 is closed as superseded and must not be reopened.
CAPTAIN DECISION 3 (no bypass): do NOT add an FM_SEND_UNPROVEN escape hatch or any equivalent caller-declared exemption. A harness whose composer cannot be proven is refused, not steered blind. The existing exit-3 "unconfirmed" already covers the post-Enter case.
REPLACE (not keep): fm_composer_pre_enter_verdict was replaced wholesale by the delta verdict - its caps/cursor-row/identity params, the styled=1 vs styled=0 fork, the fm_composer_classify_screen precondition, and both fm_composer_extract_selected_content calls are gone. The caps/cursor_row/identity fetches were stripped from the send path in bin/fm-tmux-lib.sh fm_tmux_submit_core and from every per-backend copy (including zellij's private acceptance rule) so every backend uses the ONE shared verdict. The whole visible pane is captured identically before and after, replacing FM_COMPOSER_ACCEPT_LINES cropping.
CAPTAIN'S LATER OPTION C (accepted, and the substance of this branch's redesign): an earlier round of this work refused every payload shorter than the 24-character anchor minimum, which broke real fleet traffic - a decision answer like "2", a "yes", and every fm-control lifecycle exit command (/exit, /quit). The captain rejected both ways out that were tried. Short steers must WORK, not be refused; and there must be NO weaker-proof path for them, because a weaker test reopens the false-accept this whole design exists to close. The captain named three candidate approaches - a verifiable envelope/marker wrapping the payload, a confirmation handshake, or an occurrence count robust to coincidental modal text - and asked for the SOUNDEST of them.
CHOSEN APPROACH AND WHY THE ALTERNATIVES WERE REJECTED. A previous attempt on this branch tried "occurrence count plus a positively classified empty composer before the write, and a count rising by exactly one". That was analyzed and found UNSOUND and has been removed: a live agent pane redraws rows containing a single character constantly (a token counter ticking from 1.9k to 2.1k is a newly added row, containing "2", with the count risen by exactly one), so an empty-composer precondition does not exclude the coincidence. A confirmation handshake was rejected as not generically available across harnesses. The envelope was chosen: for a payload shorter than the anchor minimum, fm-send now MANUFACTURES the missing entropy. It types the payload followed by a fresh random suffix sized so the combined string is exactly the anchor length, proves that combined anchor through the same unchanged delta verdict, then erases exactly the suffix and proves the erase. The agent still receives precisely the payload the caller wrote, and because the proven anchor contains the payload's own characters, this is a proof ABOUT the payload rather than a proxy for it. The suffix goes AFTER the payload because erasing consumes from the tail; a leading suffix would need cursor movement no backend agrees on.
THE ERASE PROOF, and a hole found and closed during implementation. The first erase check compared the full combined anchor's count back to its before value. A test drove the hardest under-erase case and showed that check is wrong: dropping even one character from the tail destroys the full-length anchor, so its absence proves nothing and a composer still holding suffix characters would have been accepted, sending noise to the agent. The check was replaced with a boundary probe. Erasing consumes from the tail, so an incomplete erase can only ever leave payload+suffix-prefix, and every such leftover down to a single character still has the suffix's FIRST character immediately after the payload - so the test is whether that two-character boundary string gained an occurrence. The second check catches the opposite direction: an over-erase eats the payload's tail, so the payload's own occurrence count must be unchanged from the just-typed capture. The suffix alphabet deliberately EXCLUDES every character the payload contains, which is what makes that second count decidable - the payload cannot occur inside the suffix or across the boundary, so removing the suffix cannot move the count on its own.
FINDING 2 FROM THE PRIOR REVIEW ROUND (typed-unproven had no operator resolution path) is fixed: bin/fm-send.sh gained --typed-confirm and --typed-abandon, which settle a typed-unproven pending-reply record by correlation id, the diagnostic that creates that state now names both commands with the concrete id, and the watcher escalates a typed_unproven record on the ordinary cadence so it can never silently block teardown. A stranded suffix additionally gets a named stderr warning carrying the exact text and suffix length, so "inspect the pane" is an actionable instruction.
fm-control lifecycle exit now works with NO exemption. The earlier attempt added an FM_COMPOSER_DELIVERY_POSTCONDITION_EXTERNAL environment variable letting fm-control declare its own postcondition; that has been deleted as exactly the bypass shape decision 3 forbids. A short exit command needs no exemption once it carries a full-length anchor. The agent-state wait remains the authoritative post-exit proof, as it always was.
ARCHITECTURE: the type-and-prove sequence moved into one shared owner, fm_composer_typed_delivery_core, which every adapter calls with three primitives (capture, literal, erase), replacing five copies of the ordering - the same pattern fm_composer_submit_retry_core already uses.
BACKEND ERASE KEY: added per backend, because the key name belongs to each tool's own vocabulary and the byte it resolves to belongs to the pane. tmux BSpace and zellij Backspace were both verified live against real panes to remove exactly one character per press (zellij rejects tmux's BSpace spelling outright). herdr and cmux carry the lowercase backspace mapping consistent with their own vocabularies but are NOT live-verified on this machine, and the evidence table says so explicitly rather than implying otherwise. Orca's send API exposes only text, Enter, and interrupt, so a character cannot be taken back off its composer at all - the shared core therefore refuses a short payload on Orca BEFORE typing anything, rather than stranding a suffix it has no way to remove. Sending a raw backspace byte through the literal-text channel instead of a named key was measured and rejected: 0x08 erased nothing against a real tmux pane and 0x7f did, which is precisely the convention a named key delegates to the backend.
TESTS: the design's benches are regressions in existing tests/ files, with no new runner. tests/fm-composer-lib.test.sh covers the delta proof accepting a shape-free echo, refusing a gated trust dialog, refusing a swallowed send, and the round-1 scrollback copy yielding no new occurrence; plus the envelope's freshness, suffix-alphabet exclusion, exact erase count, verbatim treatment of a self-proving payload, refusal of a short multi-row payload, and both erase-proof directions. A pane simulator drives fm_composer_typed_delivery_core through its real interface for the end-to-end cases including "a short steer is proven and leaves ONLY the payload in the composer". Per firstmate-coding-guidelines' harness-dependent-checks rule, each backend's erase key also has a live guard in its real smoke test that fails naming the backend and version, and docs/verification/runtime-backends.md records the dated results and the commands that refresh them. The behavior-through-fm-send tests from the KEEP set are retained.
Also in this change: the composer-delta fixtures README landed unclassified earlier on this branch and failed bin/fm-doc-audience-check.sh, so it is classified as maintainer-verification; and two probe-absent sentinel assignments are quoted because shellcheck reads them as arithmetic once the shared library grows past its analysis limit. Both were pre-existing branch defects that lint and the doc check caught.
DELIVERY: mode=no-mistakes, yolo OFF - the captain owns the merge, so do not merge. This changes shared steer/transport core the whole fleet relies on; the PR should note that homes pick it up after merge plus a self-update, and that landing timing is coordinated with the main firstmate. Escalate any ask-user finding rather than deciding it; no --yes.
ADDENDUM - what happened in the previous run and what is already settled. A first validation run on this branch reached the lint step with review, test, and document all COMPLETED and review reporting ZERO findings, after thirteen rounds of review fixes that are all present as commits on this branch. Its lint fixer then wedged - the agent process sat at 0% CPU for seventeen minutes and emitted no output at all - so that worker was terminated by exact pid and the run went terminal with "step lint failed". Custody was recovered from the local gate: the pipeline had rebased onto a newer main, and the two original commits were verified patch-identical through that rebase before the branch was moved, with the pre-recovery head anchored first. Nothing was lost.
The lint findings that run never got to fix have since been fixed directly and committed, since fixing them is the documented action after a failed run: the hyphenated string sentinels shellcheck misreads as arithmetic are now quoted, the three test stubs that shadow indirectly-invoked functions carry narrow per-site directives with reasons rather than a file-wide disable, and a dead SIM_WIRE variable was removed rather than silenced because the assertions read the wire from its file instead. bin/fm-lint.sh now exits clean, bin/fm-doc-audience-check.sh is clean, and tests/fm-composer-lib.test.sh and tests/fm-pending-reply.test.sh both pass with 34 and 40 checks and no failures.
Decisions already made and NOT open for relitigation, each ruled on explicitly by the captain during the previous run: after a successful write, EVERY unproven outcome is typed-unproven and there is no retryable post-write shape, because no screen delta can distinguish a delivered-but-unrendered write from a swallowed one; only the pre-write refusals stay retryable, being a gated modal refused before typing and a write that did not go out. The flattened row matching in the delta proof is deliberate and load-bearing because it is what makes a WRAPPED payload provable. Shape-parsing must not be reopened in any form. The low-level stranded-suffix warning stays purely descriptive and the request-owning layer owns the settlement instruction, because the shared transport core has no concept of a pending request and the lifecycle commands call it with no correlation id. The erase proof's payload-retention barrier compares for EQUALITY against the stage-one capture. And a bounded, mechanism-less residual is knowingly accepted and documented: there is no shape-free positive proof that the payload survived the final erase keystroke, because once the erase is correct nothing high-entropy remains to anchor on.
What Changed
--typed-confirm/--typed-abandonsettlement commands.Risk Assessment
Testing
Inspected the target diff, ran focused shared-core, fm-send, lifecycle, pending-reply, daemon, remote-delivery, and backend adapter tests, then manually exercised a short steer through the real fm-send executable; all checks passed, and the transcript confirms exit 0, exact suffix erasure, Enter submission, and payload-only delivery. No screenshot was applicable because the changed user surface is terminal transport rather than graphical UI.
Evidence: Short-steer fm-send end-to-end transcript
Source: Short-steer fm-send end-to-end transcript
Evidence: Shared composer delivery proof tests
Source: Shared composer delivery proof tests
Evidence: fm-send strict behavior tests
Source: fm-send strict behavior tests
Evidence: Remote delivery and settlement tests
Source: Remote delivery and settlement tests
Evidence: Lifecycle control tests
Source: Lifecycle control tests
Evidence: Pending-reply durability tests
Source: Pending-reply durability tests
Evidence: Daemon typed-unproven retry containment tests
Source: Daemon typed-unproven retry containment tests
Evidence: Backend abstraction tests
Source: Backend abstraction tests
Evidence: cmux backend tests
Source: cmux backend tests
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed (2) ✅
bin/fm-pending-reply-lib.sh:1022- The required “full-payload durable recovery” is not lossless for requests ending in newlines.body=$(fm_pending_reply_request_body ...)strips all trailing newlines, and line 1103 performs another command substitution on the assembled recovery message. Thus a persisted request such ascommand\n\nis recovered ascommandwithout error. Preserve the body through result variables or another trailing-newline-safe boundary all the way intoprepared_message.bin/fm-composer-lib.sh:1477- The checkpoint accepts any occurrence ofpayload + first suffix character, although this can be only two characters (for example2k). On an unverified backend whose backspace over-erases during the first N−1 keys, ordinary pane churn can add that short probe at the checkpoint and remove it after the final key while preserving the payload occurrence count; both erase verdicts then pass and Enter submits a damaged or empty composer. This exceeds the explicitly accepted residual limited to the final erase. Revisit whether the checkpoint needs stronger send-specific evidence or explicitly authorize this additional probabilistic containment.bin/fm-send.sh:744- Everytyped-unprovenresult is reported as a pane capture failure, but the shared core also emits it after a successful capture when the delta proof fails, an erase key fails, or either erase proof fails. The preceding warning may name the real cause, leaving operators with contradictory diagnostics. Use a neutral “delivery or suffix removal could not be proven” summary while retaining the detailed transport warning.🔧 Fix: Clarify unproven delivery and erase residuals
2 errors still open:
bin/fm-composer-lib.sh:1241- The whole-screen COUNT proof uses non-overlapping matches, so it can increase even when the actual occurrence count does not. For a 24-character anchor of allas, a before capture containing 25as has two overlapping occurrences but reports 1; an unrelated redraw to24*a + b + 24*astill has two actual occurrences, reports 2, and supplies an added row containing the anchor. The delta verdict therefore accepts although the payload was swallowed. Count overlapping occurrences at this shared boundary so the required occurrence increase is real.bin/fm-pending-reply-lib.sh:353- The heredoc write that constructs the durable pending-reply record is unchecked. If it fails partway, such as under ENOSPC, execution still chmods and renames the truncated temporary file and returns a correlation id, allowing delivery to proceed without a valid durable recovery record. Check the write result and remove both temporary/body files before returning failure.🔧 Fix: Harden occurrence counting and pending record writes
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-composer-lib.test.shbash tests/fm-send-strict.test.shbash tests/fm-send-remote-delivery.test.shbash tests/fm-control.test.shbash tests/fm-pending-reply.test.shbash tests/fm-daemon.test.shbash tests/fm-backend.test.shbash tests/fm-backend-cmux.test.shbash tests/fm-backend-herdr.test.shbash tests/fm-backend-orca.test.shbash tests/fm-backend-zellij.test.shHermetic end-to-end invocation of./bin/fm-send.sh demo '2'against an echoing tmux pane, verifying the random 23-character suffix was erased before Enter and the agent received exactly2.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.