fix(bin): keep a declared pause absorbed and make the listed decision key the one that closes it - #2749
Conversation
|
Speaking as Kun's firstmate: Corrective — two named defects: (1) a live crew's declared VISION: aligns (obligations close by records; a captain answer must not fall through the cracks; idle declared waits must not spend tokens). Security: no. Overlap (do not land together): This is waiting on the author, not the captain. Fork CI is approved (runs 32508738381 CI now in_progress; 32508738229 Require no-mistakes). no-mistakes is currently red: the body has the signature line but is missing the structured |
|
Speaking as Kun's firstmate: Two stacked fixes: (1) absorb a live crew's declared pause on the long cadence; (2) render the fold's decision key and deliver a captain answer even when a named key is not open (nonzero, close nothing). Class: (1) is corrective. (2) is a default send-path change — delivery no longer cancels on a missing key. That is waiting on CI, not on a captain decision. VISION: pause absorb aligns (token efficiency, not always-on noise). Send-key honesty aligns (never drop a captain answer). Cannot yet land: no-mistakes/CI must finish, and this overlaps #2748 on Waiting on CI/author rebase after the overlap lands — not waiting on the captain. |
47eaace to
a94147d
Compare
A crew that declares `paused:` is idling on a known external wait, so its stale pane must be absorbed and rechecked only on the long pause cadence. `pause_state_class` instead re-read agent liveness on every distinct stale hash and let a live or unreadable agent override the authoritative paused verdict, returning `none`. Because a held dev server repaints, every repaint was a fresh first sighting, so the stale path queued another contentless `stale: <window>` wake every minute or two while `.watch-triage.log` simultaneously logged the same pane as absorbed. The authoritative crew-state read now decides first, and liveness only breaks the remaining tie: a `paused:` or `captain-held` status whose crew `fm-crew-state.sh` can no longer confirm still surfaces once unless the agent is confirmed dead, so a captain hold over a crew that never declared the wait itself cannot silence a live decision gate. Once the bounded cadence is granted, the cheap `.paused-<key>` path holds it without re-deciding per stale hash until its recheck marker ages out. Regression tests cover the three boundaries: a live declared pause absorbs every churny stale hash with no queued wake, a live idle crew without a declared pause still surfaces on the unchanged schedule, and a declared pause past the bounded window still re-surfaces once as a labeled recheck. The existing live-gate case moves to `captain-held`, where the surface-once-when-unconfirmable rule actually applies.
The OPEN DECISIONS section printed no key at all whenever the fold landed on the shared "default" bucket. A decision whose note text still carried a "[key=...]" token therefore advertised that token as its key, and the section's single generic hint invited answering with it. fm-send refused that key - and because the refusal cancelled the send, the captain's answer was dropped rather than delivered, leaving a real decision open until someone noticed and resent it by hand. Every entry now renders the key the fold decided, "default" included, and carries its own ready-to-run close command underneath, so no key has to be inferred from note prose; a "[key=...]" inside a note stays quoted verbatim as the worker's own words. Note and command are charged to the byte cap together, so an entry is dropped whole rather than listed with no way to close it, and a task id outside the plain-slug charset gets a pointer instead of a string an agent would paste into a shell. fm-send now delivers the answer even when a named key is not open. That is a deliberate change to the safety property, not a convenience relaxation. The mis-state the flag exists to prevent is a decision that looks answered while its record stays open, and cancelling the send never prevented it - it produced the worse version, where the decision stayed open AND no worker ever received the answer the captain believed was given. Delivering the text is no more dangerous than the plain steer that is always allowed, because the flag's power is the close, and that is exactly what an unmatched key still withholds. "Delivered, not closed" is also the outcome the post-delivery append-failure path already treats as the safe direction, since an open decision re-surfaces on every later presentation. The diagnostic stays loud: it exits nonzero, names the key, and says not to resend, while any other named key that is open still closes. Malformed usage - a key outside the slug charset, a duplicate key, --key, an explicit backend target, an empty message - is still refused before anything is sent, because those are argument errors with no answer to deliver. The generated briefs now show the keyed form explicitly, so what a worker is told to write matches what the fold reads. The parse itself already honors the inline marker after kunchenguid#2202; the regression tests cover the inline-marker form and the keyless fallback, and prove the agreement end to end by replaying the drain's own printed commands through the real fm-send.
…nmatched-key behavior
a94147d to
19a81f6
Compare
Tiraso12
changed the title
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains from the previously reported follow-up issues. Reviews (4): Last reviewed commit: "no-mistakes(document): clarify partial r..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ro paths The header's exit status contract said every nonzero other than 3 means the send failed and nothing may be assumed delivered, but several paths exit nonzero only AFTER the submit is confirmed: the pending-reply delivery commit, a failed closing append, and the unmatched --resolve-key this branch added. A caller reading the contract literally would treat an answer that already landed as unsent and resend it, duplicating the instruction. State the exception as a class rather than an enumeration that would drift: a nonzero whose stderr says the text was delivered and not to resend reports an unreconciled ledger, never an undelivered message.
…atched-key OPEN DECISIONS treated `-foo` as a pasteable task id, so the printed command was `fm-send.sh -foo ...` and fm-send parsed it as a flag. A leading dash is now the same unpasteable case as an illegal charset, and the drain prints a pointer instead. The unmatched --resolve-key path still delivers then exits 1. The header now names that post-delivery nonzero as a ledger mismatch, not an undelivered send, so a caller does not resend an answer that already landed.
Tiraso12
changed the title
|
Speaking as Kun's firstmate: VISION: aligns. OPEN DECISIONS now prints the fold's actual close key (including Class: corrective. Two named defects: (1) listed decision key vs the key that actually closes; (2) declared pause re-decided per pane hash. Send path still delivers an unmatched Security: none. No new network, no credential handling, no workflow-file change. Leading-dash ids are no longer pasted as runnable close commands. Overlap: CI: HEAD Workflows approved: yes (CI 32587660954, NM 32587660911, NM 32587687976). Not land-eligible. Waiting on CI, not the captain. |
Tiraso12
changed the title
2 participants
Intent
Fix the existing open PR #2749 so it no longer conflicts with main, keeping both unique fixes (fix(watcher): keep a live crew's declared pause absorbed; fix(bin): make the listed decision key the one that closes it) plus the pipeline doc commit, rebased onto latest origin/main which already includes the merged #2748 hold-cadence work. Now with no-mistakes v1.53.0: drive the pipeline so the PR body gets the structured pipeline attestation comment (no-mistakes-pipeline-attestation:v1 HTML comment) that 1.46+ writes, alongside the existing 'Updates from git push no-mistakes' line, so the required 'PR must be raised via no-mistakes' check passes. Do not change product code unless the new pipeline requires a fix. Do not restore refuse-before-send behavior. Do not merge the PR.
What Changed
bin/fm-watch.sh:pause_state_classnow treats a crew's ownpaused:/captain-helddeclaration as authoritative instead of re-reading agent liveness on every poll. A crew the authoritative read already names paused stays absorbed even while its agent is alive, the cheap.paused-<key>cadence holds without any liveness read until the recheck marker ages pastSTALE_ESCALATE_SECS, andwindow_kindis only read on the poll that actually re-decides a declared wait - so a live crew holding a dev server no longer emits a contentless stale wake on each repaint.bin/fm-wake-drain.sh: every OPEN DECISIONS entry renders the key the fold actually decided (including thedefaultbucket a keyless line opens) plus its own ready-to-runbin/fm-send.sh <task> --resolve-key <key>close command, so the listing can no longer advertise a[key=...]token from the note prose thatfm-sendwould reject. Task ids that are unsafe to paste - outside the plain-slug charset or leading-dash - get a pointer instead of a command, and an entry's note and command are charged to the section byte budget together so nothing is listed without a way to close it.bin/fm-send.sh: a--resolve-keynaming a key that is not open no longer refuses before sending; it warns pre-send, delivers the answer, closes whichever other named keys are open, then exits nonzero with a "delivered, do not resend" diagnostic. Malformed usage (bad charset, duplicate key,--key, explicit backend target, empty message) is still refused outright, the exit-status contract in the header now documents the post-delivery nonzero paths, andbin/fm-brief.shplusdocs/architecture.mdwere updated to match. Tests for all three scripts were extended, including an end-to-end case that replays the drain's printed close command verbatim through the realfm-send.Risk Assessment
Testing
Ran the four suites covering the changed surfaces plus five neighbouring ones (all pass), then proved each fix is a real regression by reverting only the three product scripts to the base commit and watching the new tests go red. Product-level evidence comes from two before/after CLI transcripts driving the real drain, send and watcher scripts: the OPEN DECISIONS listing now prints the key the fold decided with its own close command, and running that command verbatim closes the decision, while a note-prose key delivers the answer and exits nonzero instead of refusing to send; the live declared-pause scenario drops from 8 contentless stale wakes out of 8 repaints to 0, with the bounded long-cadence re-surface still firing once. No tmux exists on this host, so the terminal backend and fm-crew-state are stubbed exactly as the repo's own suites stub them; the change has no rendered UI surface, so no screenshots apply. The pipeline-attestation comment required by the intent is produced by the push/PR phases and is outside this phase's ownership.
Evidence: Evidence index (what each artifact shows, how to reproduce)
Source: Evidence index (what each artifact shows, how to reproduce)
Evidence: Decision-key E2E transcript (before/after, real drain + send)
Source: Decision-key E2E transcript (before/after, real drain + send)
BEFORE (base 1231b6ae) $ bin/fm-wake-drain.sh OPEN DECISIONS (still open, folded from the durable status logs - not just the latest line): banner needs-decision: which banner color for the launch header theme needs-decision: pick a [key=red] or [key=blue] theme toy-store [key=totals-pool-separation] needs-decision: display-only or engine bug OPEN DECISIONS: close one by answering it: bin/fm-send.sh <task> --resolve-key <key> '<answer>' $ bin/fm-send.sh theme --resolve-key red "go with red" exit: 1 error: --resolve-key 'red': no open decision or blocker with that key ... nothing was sent. text typed into the crew pane: '' <-- the captain's answer was dropped AFTER (target e3c595a) $ bin/fm-wake-drain.sh OPEN DECISIONS (still open, folded from the durable status logs - not just the latest line): banner [key=default] needs-decision: which banner color for the launch header close it: bin/fm-send.sh banner --resolve-key default '<answer>' theme [key=default] needs-decision: pick a [key=red] or [key=blue] theme close it: bin/fm-send.sh theme --resolve-key default '<answer>' toy-store [key=totals-pool-separation] needs-decision: display-only or engine bug close it: bin/fm-send.sh toy-store --resolve-key totals-pool-separation '<answer>' OPEN DECISIONS: answer each one with the close command printed under it; the key that closes a decision is the bracketed key before its verb, never a [key=...] inside the note text. $ bin/fm-send.sh toy-store --resolve-key totals-pool-separation "engine bug - fix the pool split" # the printed command, verbatim exit: 0 text typed into the crew pane: engine bug - fix the pool split toy-store status log now: resolved [key=totals-pool-separation]: answered: engine bug - fix the pool split $ bin/fm-send.sh theme --resolve-key red "go with red" exit: 1 error: the answer was delivered to sess:fm-theme, but no decision was closed for --resolve-key 'red' ... Do not resend the answer. text typed into the crew pane: 'go with red' <-- delivered, closes nothingEvidence: Live declared-pause churn E2E transcript (before/after, real watcher)
Source: Live declared-pause churn E2E transcript (before/after, real watcher)
crew 'held' status log: paused: holding the dev server while the captain reviews crew is LIVE (pane_current_command=grok), pane repaints once per second BEFORE (base 1231b6ae) - 8 pane repaints, one long-lived watcher repaint 1: WOKE FIRSTMATE -> stale: test:fm-held repaint 2: WOKE FIRSTMATE -> stale: test:fm-held ... (3-7 identical) ... repaint 8: WOKE FIRSTMATE -> stale: test:fm-held supervision turns burned on a crew that is fine: 8 of 8 total stale rows queued: 8 AFTER (target e3c595a) - same 8 repaints repaint 1: absorbed (watcher still blocking, captain not interrupted) ... (2-7 identical) ... repaint 8: absorbed (watcher still blocking, captain not interrupted) supervision turns burned on a crew that is fine: 0 of 8 (wake queue empty - no wake was ever queued) [2026-08-22T15:06:25-0400] absorbed stale (paused, awaiting external, age 49s): test:fm-held --- absorbed, not silenced: past the bounded window --- watcher printed: stale: test:fm-held (paused 501s, awaiting external - declared pause, rechecked on a long cadence not a wedge; confirm the wait still holds)Evidence: Regression proof: drain test red against pre-fix code
Source: Regression proof: drain test red against pre-fix code
not ok - open section is missing the decision's own answerer-closes commandEvidence: Regression proof: fm-send test red against pre-fix code
Source: Regression proof: fm-send test red against pre-fix code
not ok - the diagnostic should say the answer already landed (missing: 'Do not resend the answer')Evidence: Regression proof: watcher test red against pre-fix code
Source: Regression proof: watcher test red against pre-fix code
not ok - a live declared pause woke firstmate on churny stale hash 1: stale: test:fm-heldEvidence: Reproduction script: decision-key E2E
Source: Reproduction script: decision-key E2E
Evidence: Reproduction script: live declared-pause churn E2E
Source: Reproduction script: live declared-pause churn E2E
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-wake-drain.sh:173- The new per-entry close command is printed for reserved-namespace decision keys, where it provably closes nothing while reporting success. Concrete path: fm-pending-reply-lib escalates a marked request by appendingblocked [key=pending-reply-abc123]: pending-reply-missed: task=ios ...to the parent status log. The drain (verified by running it) printsios [key=pending-reply-abc123] blocked: ...plusclose it: bin/fm-send.sh ios --resolve-key pending-reply-abc123 '<answer>'. fm-send accepts the key because it is open per status_open_decisions (bin/fm-send.sh:462), delivers, appendsresolved [key=pending-reply-abc123]: answered: <text>, and exits 0. Re-folding that log shows _fm_decision_key_transition_allowed (bin/fm-classify-lib.sh:320) rejects the close - the noteanswered: ...does not speak the reserved namespace's vocabulary - so the key is still open. Every later drain reprints the same command, so the captain re-answers the same escalation each supervision turn while fm-send keeps reporting a clean close. This contradicts the invariant declared in the same hunk ('it can never advertise a key that would close nothing') and in docs/architecture.md:52. Suggested earliest shared boundary rather than a second symptom patch: _fm_decision_key_transition_allowed already owns the rule - have print_open_decisions_section consult it with the note fm-send would write and degrade to a pointer naming the owning library for a row it would reject, and have fm-send apply the same check so a reserved key it cannot honestly close is counted as unmatched instead of closed.✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh tests/fm-wake-drain-open-decisions.test.sh tests/fm-wake-drain-unread-status.test.sh tests/fm-send-resolve-key.test.sh- all pass at the target commitbin/fm-test-run.sh tests/fm-watch-triage.test.sh- all pass, including the three new live-declared-pause testsbin/fm-test-run.sh tests/fm-wake-drain-open-decisions-cursor.test.sh tests/fm-classify-decision-key.test.sh tests/fm-captain-hold-lifecycle.test.sh tests/fm-brief.test.sh tests/fm-documentation-audiences.test.sh- neighbouring suites over the same fold, brief text and docs, all passRegression proof:git checkout 1231b6ae -- bin/fm-watch.sh bin/fm-wake-drain.sh bin/fm-send.shthen re-ran the three suites - each fails on its new test (not ok - open section is missing the decision's own answerer-closes command,not ok - the diagnostic should say the answer already landed,not ok - a live declared pause woke firstmate on churny stale hash 1), then restored withgit checkout HEAD -- bin/...Manual E2E:fm-decision-key-e2e.shdrives the realbin/fm-wake-drain.sh+bin/fm-send.shover a crafted fleet state (inline key, keyless, note-prose key) against base and target bin, replaying the printed close command verbatimManual E2E:fm-paused-churn-e2e.shdrives the realbin/fm-watch.shthrough 8 dev-server repaints under a declaredpaused:with a live pane, re-arming after each surfaced wake and draining/acking as firstmate does, against base and target binVerifiedgit status --porcelainclean after testing; transient /tmp fixtures removed✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.