fix(bin): brief workers against blocking sleeps and one-shot writes - #2678
Open
Inthuson wants to merge 4 commits into
Open
fix(bin): brief workers against blocking sleeps and one-shot writes#2678Inthuson wants to merge 4 commits into
Inthuson wants to merge 4 commits into
Conversation
Two worker-behaviour learnings were recorded in learnings.md and never reached the generated brief, which is the only text a worker actually reads. Blocking sleeps cost 16.7 hours of agent time in ten days while firstmate retyped the correction seven times by hand, and a single final report write silently lost three whole scout reports in one day. Ship and scout rules now forbid waiting in a foreground blocking sleep and name the alternatives in order: a harness-tracked background job, else a brief poll with other work between checks, else record the thing as unverified and move on. The scout report and the secondmate charter's detailed answer now require writing the deliverable incrementally, section by section, straight to its file, and name the transient API error that ends the turn silently so a worker retries rather than restarts. Tests assert the generated brief per variant, not the script source, because the defect was text that was never emitted at all.
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
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
Close two worker-behaviour learnings that were written down in data/learnings.md and then never reached the place workers actually read: the generated brief scaffold in bin/fm-brief.sh.
Why it matters, with the measured cost. Firstmate hand-typed "stop using blocking sleeps" to workers seven times between 2026-08-14 and 2026-08-17. Blocking sleeps rose from 1.05 hours a day to a peak of 5.33, and cost 16.7 hours of agent time in ten days. The correct fix was recorded in data/learnings.md on 2026-08-11 in the words "Brief against blocking sleeps". Nine days later bin/fm-brief.sh contained the word "sleep" zero times. The actual defect is that a learning about worker behaviour does not close by being written in learnings.md, because workers never read that file; it closes when it reaches the scaffold. Both items were verified absent at zero occurrences before this change.
Fix 1, how to wait and what never to do. A worker waiting on its own background job must not sit in a foreground blocking sleep. Five workers in one day did exactly that, one in "sleep 420" at 19 percent context, another in "sleep 115". From outside it is indistinguishable from a stall, so it burns supervisor attention as well as the worker context the wait was meant to protect. The generated brief must tell the worker how to wait correctly: prefer a harness-native tracked background job whose completion resumes or notifies the same agent; otherwise poll briefly and do other useful work between checks; otherwise record the thing as unverified and move on; and never use a foreground blocking sleep as a wait.
Fix 2, write deliverables incrementally. Included deliberately, because it is the same defect with a different symptom and firstmate has now hand-typed it into every scout brief it wrote today; that repetition is the signal it belongs in the scaffold rather than in firstmate's fingers. Report-producing and document-producing workers must write section by section, straight to the deliverable file, never composing the whole thing and writing once at the end. Recorded cause, from data/learnings.md 2026-08-18: "API Error: The system encountered an unexpected error during processing" clusters on the long-output step and ends the turn silently, so a single final write loses the entire document. It hit three scouts in one day. Incremental writing turns that from losing a document into losing a section. The same entry notes the recovery is a one-line nudge, so the brief should also say the error is transient and to retry rather than restart.
Constraints accepted for this task. Keep it short: brief space is scarce and competes with the task description, so two tight blocks, not two essays; if a section grows past a short paragraph, cut it rather than indenting it. Both fixes belong in the GENERATED brief text, in the sections where a worker will actually read them, not in the script's header comments. The scout and ship variants both need fix 1. Fix 2 matters most for scout and any document-producing variant; use judgement rather than pasting it everywhere. Do not weaken or reword the existing safety contracts: the worktree-isolation assertion, the status protocol, the delivery-mode definitions of done, and the Herdr lab declaration all stay exactly as they are. Colocated tests must assert the generated brief actually contains both instructions, for each variant that should carry them; a test that only checks the script runs is worthless here, because the whole bug is text that was never emitted. shellcheck-clean at the pinned version. No em-dashes anywhere, including commit messages and the PR body. Never add an agent name as a commit co-author. This is firstmate's own shared tracked material, so .agents/skills/firstmate-coding-guidelines/SKILL.md was read and followed.
Definition of done, in one line: bin/fm-brief.sh emits both instructions, tests prove it for every variant that needs them, and firstmate never has to type either correction to a worker again.
Implementation decisions and tradeoffs made while doing the work, which a reviewer reading only the diff would not know. Fix 1 was added as rule 8 in BOTH the ship and the scout Rules blocks, because that block is where the worker already reads its operational constraints, and it states all four alternatives in the priority order given above. Fix 2 was added to the scout Definition of done and ALSO to the secondmate charter's detailed-answer path, because that path is explicitly the same scout-report pattern and has the same whole-document loss mode; it was deliberately NOT pasted into ship briefs, which deliver code through commits. That judgement call is recorded in a test comment rather than pinned by an absence assertion, deliberately, so that a later decision to add it does not have to fight a brittle test. A five-line note was added to the script header as well, because AGENTS.md section 11 makes bin/fm-brief.sh and its help the owner of scaffold content and generated variants, so --help should record that these contracts exist; the header note is a short summary pointer, not a second full copy of the brief text, to respect the coding guidelines' one-owner rule. The scout Definition of done was reordered so the stand-alone content bar stays adjacent to its own sentence and the write mechanics follow it, keeping the two "what" sentences together before the "how". No AGENTS.md change was made: section 11 already delegates scaffold ownership to bin/fm-brief.sh and its help, and adding scaffold detail there would violate the coding guidelines' size discipline.
Verification performed. The two new tests were proven non-vacuous, which was the point that mattered most: briefs were generated from the pre-change script at HEAD and all five assertion needles appeared in zero of them, so the assertions would have failed before this change. All 22 tests in tests/fm-brief.test.sh pass. bin/fm-lint.sh passes at pinned ShellCheck 0.11.0; actionlint is not installed locally, and no workflow files were changed. Every added line was scanned for non-ASCII to confirm no em-dashes. Related test files that consume fm-brief were run and pass: fm-ask-user-authority, fm-secondmate-safety, fm-tangle-guard, fm-subagent-pretool-check.
Known pre-existing failure, unrelated to this change and not caused by it: tests/fm-decision-hold-lifecycle.test.sh fails in this environment because the tasks-axi mise shim has no version set, and that test never references fm-brief.
What Changed
bin/fm-brief.shadds rule 8 to both the ship and scout Rules blocks: never wait in a foreground blockingsleep, because from outside it is indistinguishable from a stall and burns the context the wait was meant to protect. The rule names the alternatives in priority order (harness-tracked background job whose completion resumes or notifies you, then brief polling with other useful work between checks, then record the thing as unverified and move on).API Error: The system encountered an unexpected error during processingthat ends the turn silently plus its retry-rather-than-restart recovery. A short note in the script header records that these two generated contracts exist.tests/fm-brief.test.shgains two tests wired into the run list that assert the generated brief text itself: the wait rule and all four of its clauses across the three ship modes (no-mistakes,direct-PR,local-only) and the scout variant, and the incremental-write instruction plus recorded cause and recovery for the scout report and the secondmate charter.Risk Assessment
✅ Low: The change is a bounded, additive set of generated-prompt lines plus behavior tests over that generated output, it is byte-identical to the author's intake commit, it satisfies every source-verifiable acceptance criterion, and no brief consumer, parser, or competing fleet contract is affected.
Testing
Ran the colocated fm-brief suite (22 tests, all pass) and proved the two new tests non-vacuous by running them against the pre-change script, where they fail. For product-level evidence I generated all five brief variants from both the base commit and this change and compared them: all five instruction needles appear zero times pre-change, and after the change rule 8 renders in the three ship modes plus scout while the incremental-write block renders in the scout Definition of done and the secondmate detailed-answer path. A path-normalized diff of every generated brief shows additions only, confirming the worktree-isolation assertion, status protocol, delivery-mode definitions of done and Herdr declaration are untouched. I also encoded a generated scout brief through fm-operational-input.sh, the same payload fm-spawn.sh delivers, to show both instructions reach the worker's actual launch prompt, and checked the --help header pointer and the --herdr-lab variants. Four related consumer test files pass; the fm-decision-hold-lifecycle file the author flagged simply skips here for a missing tasks-axi shim. No screenshots apply: the end-user surface is generated markdown delivered as an agent prompt, so the rendered brief text and encoded launch prompt are the reviewer-visible artifacts. Overall result: green, no actionable findings.
Evidence: Instruction occurrences per generated brief variant, before vs after
=== BASE 1cb900c (pre-change) === ship-no-mistakes no-blocking-sleep=0 wait-ladder=0 incremental-write=0 recorded-cause=0 ship-direct-PR no-blocking-sleep=0 wait-ladder=0 incremental-write=0 recorded-cause=0 ship-local-only no-blocking-sleep=0 wait-ladder=0 incremental-write=0 recorded-cause=0 scout-task no-blocking-sleep=0 wait-ladder=0 incremental-write=0 recorded-cause=0 mate-charter no-blocking-sleep=0 wait-ladder=0 incremental-write=0 recorded-cause=0 === TARGET 07bfb7f (this change) === ship-no-mistakes no-blocking-sleep=1 wait-ladder=1 incremental-write=0 recorded-cause=0 ship-direct-PR no-blocking-sleep=1 wait-ladder=1 incremental-write=0 recorded-cause=0 ship-local-only no-blocking-sleep=1 wait-ladder=1 incremental-write=0 recorded-cause=0 scout-task no-blocking-sleep=1 wait-ladder=1 incremental-write=1 recorded-cause=1 mate-charter no-blocking-sleep=0 wait-ladder=0 incremental-write=1 recorded-cause=1Evidence: Generated brief text a worker reads (scout Definition of done, ship Rules item 8, secondmate detailed-answer path)
### scout brief: Definition of done The report must stand alone: what you did, what you found, the evidence (commands run, output, file:line references), and what you recommend. Write it incrementally, section by section, straight to that file; never compose the whole report and write it once at the end. A single long final write can hit a transientAPI Error: The system encountered an unexpected error during processingthat ends the turn silently and loses everything unwritten; that error is transient, so retry rather than restart. ### ship brief (mode=no-mistakes): Rules item 8 8. Never wait in a foreground blockingsleep: from outside it is indistinguishable from a stall, and it burns the very context the wait was meant to protect. To wait on your own background job, prefer a harness-tracked background job whose completion resumes or notifies you; otherwise poll briefly and do other useful work between checks; otherwise record the thing as unverified and move on. ### secondmate charter: detailed-answer path For a detailed answer (an investigation, a plan, an audit), write it to a doc under your home'sdata/... Write that doc incrementally, section by section, straight to the file; never compose the whole document and write it once at the end. A single long final write can hit a transientAPI Error: ...that ends the turn silently and loses everything unwritten; that error is transient, so retry rather than restart.Evidence: Path-normalized diff of every generated brief, base vs target (additions only, existing contracts intact)
--- scout-task/brief.md 38a39,43 > 8. Never wait in a foreground blockingsleep: from outside it is indistinguishable from a stall, > and it burns the very context the wait was meant to protect. > To wait on your own background job, prefer a harness-tracked background job whose completion > resumes or notifies you; otherwise poll briefly and do other useful work between checks; > otherwise record the thing as unverified and move on. 42a48,49 > Write it incrementally, section by section, straight to that file; never compose the whole report and write it once at the end. > A single long final write can hit a transientAPI Error: ...; that error is transient, so retry rather than restart. --- mate-charter/brief.md 29a30,31 > Write that doc incrementally, section by section, straight to the file; never compose the whole document and write it once at the end. > A single long final write can hit a transientAPI Error: ...; that error is transient, so retry rather than restart. (removed/modified lines across all five variants: 0)Evidence: Encoded launch prompt actually delivered to the worker agent (scout)
needles in the delivered launch prompt (bin/fm-operational-input.sh encode launch-brief < brief.md): Never wait in a foreground blockingsleep-> 1 prefer a harness-tracked background job whose completion -> 1 Write it incrementally, section by section, straight to that file -> 1 that error is transient, so retry rather than restart -> 1/tmp/no-mistakes-evidence/01M0FKB1BBA0KVGQTNSFACZ7BS/briefs-target/home/data) - Evidence: Generated briefs, base script 1cb900c (all five variants, instructions absent) (local file:/tmp/no-mistakes-evidence/01M0FKB1BBA0KVGQTNSFACZ7BS/briefs-base/home/data)Evidence: New tests fail against the pre-change script (non-vacuity proof)
not ok - no-mistakes: brief does not forbid waiting in a foreground blocking sleep exit=1Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-brief.sh:473- bin/fm-brief.sh:473 (and the identical scout copy at :349) ends rule 8's ladder with "otherwise record the thing as unverified and move on", with no exception for a wait the Definition of done itself requires and no pointer to the brief's own sanctioned long-external-wait mechanism. The ship DOD requiresdone: PR {url} checks green(no-mistakes) and rule 4 already definespaused: {why}for a known external wait firstmate rechecks on a long cadence, which is the correct behavior for a wait the worker cannot shorten. As written, a worker that cannot use a tracked background job and cannot usefully poll can read the third fallback as license to end the turn with a DOD-required verification silently unverified, rather than declaringpaused:and staying accountable for it. The four-step ladder is verbatim what the intent specified, so this is flagged for your decision, not changed: a single clause ("never for a check your Definition of done requires - declarepaused:instead") would close it inside the stated brevity bar.bin/fm-brief.sh:237- The secondmate charter received fix 2 (bin/fm-brief.sh:237-238) but not fix 1: it has no Rules block and no instruction against a foreground blocking sleep. A persistent secondmate waits on its own crewmates through the full firstmate lifecycle, and grep confirms no other surface it loads carries this contract (AGENTS.md never mentions sleeps; AGENTS.md:536 and .agents/skills/process-event-sources/SKILL.md:63 forbid only running a registered source's blocking command in a conversational turn). So the authorized failure - an agent parked in a foregroundsleepwaiting on work it started, indistinguishable from a stall - stays reachable for secondmates, and firstmate could still have to hand-type that correction, against the stated definition of done ("firstmate never has to type either correction to a worker again"). Your intent scoped fix 1 to "the scout and ship variants", so I am not treating this as a contradiction or expanding scope: it is a scope call for you, and the cheapest close would be one line on the charter's escalation/operating path rather than a Rules block the charter does not have.🔧 Fix: point required waits at paused, brief secondmate on waiting
2 issues (1 warning, 1 info) still open:
bin/fm-brief.sh:225- The new secondmate clause sanctions apaused:escalation for a wait the charter's own escalation rules exclude. Line 224 scopes the clause to "When you wait on a crewmate or on a background job of your own", and line 225 then says to "declare a wait you can neither shorten nor abandon with$PAUSED_VERB: {why}below" -belowbeing the Escalation section, whose appends go to the MAIN firstmate's status file ($STATE/$ID.status, line 247). But that same section already scopes the verb narrowly: line 249 permitspaused:"only when your domain is deliberately idling on a known external wait you expect to clear on its own", and line 259 says "Routine internal supervision, heartbeats, retries, and crewmate churn stay inside your own home and must not touch that status file." A wait on your own crewmate is internal churn, not an external wait, so the Operating model now sanctions what the Escalation section forbids, with two owners for one verb. Concrete path: the secondmate spawns crewmate fm-x for a long task, cannot shorten or abandon the wait, and appendspaused: waiting on crewmate fm-x. bin/fm-watch.sh:1044 then flips that secondmate window from skipped stale detection into the paused cadence, and handle_paused_stale (bin/fm-watch.sh:351-356) re-surfaces a firstmate wake reading "confirm the wait still holds" once per PAUSE_RESURFACE_SECS for routine internal supervision; charter line 258 then requires a second append, aresolved:line, to close it. Net effect is supervisor wake-ups for exactly the churn the charter isolates, plus a persistent secondmate labelled as deliberately idling while it is actively supervising (its own DOD at line 262 says it is persistent and an empty queue needs no status line at all). This also brushes the intent's constraint "Do not weaken or reword the existing safety contracts: the worktree-isolation assertion, the status protocol, ...": rule 8's paused pointer exists because a mode=no-mistakes ship DOD requires a wait, and the charter's DOD requires none, so the cheapest close is to drop the paused pointer from the charter (keeping the three-rung ladder, which is the part the accepted finding asked for) or to scope it explicitly to a genuinely external wait in line 249's words.bin/fm-brief.sh:476- Noting a residual, deliberately scoped gap rather than asking for a broader rule. Rule 8's ladder (ship copy here, identical scout copy at :351) forbids exactly one shape, "a foreground blockingsleep", and its new last rung tells the worker to declare$PAUSED_VERB: {why}for a required wait. Unlike rules 5 and 6, which each end "and stop", the new clause does not say the turn ends there, and apaused:declaration is not inert on the supervision side: bin/fm-watch.sh:365-373 documents that apaused:declaration deliberately stops treating an over-long BUSY pane as a wedge suspect, "identifies that live foreground call as the expected external wait". So "declarepaused: waiting for CI, then block in the foreground on a non-sleep command" (gh run watch, a blocking poll) reads as compliant: it is not asleep, and the declaration now shields the busy pane from wedge escalation, which is the same stall-shaped, context-burning wait this change exists to remove. This is informational because the intent itself scopes the forbidden shape tosleep("never use a foreground blocking sleep as a wait") and explicitly caps the added text's size, so widening the prohibition to any blocking foreground wait, or adding "and stop", would go past what was asked. Flagging the tradeoff only, so it is a known residual rather than an assumed-closed one.🔧 Fix: keep secondmate waits in own home, end paused rung
2 issues (1 warning, 1 info) still open:
bin/fm-brief.sh:225- The secondmate charter's new waiting ladder misdirects a wait on the secondmate's OWN crewmate away from the mechanism the always-loaded supervision contract owns. Line 224 scopes the clause to "When you wait on a crewmate or on a background job of your own", and line 225's first rung says "Prefer a harness-tracked background job whose completion resumes or notifies you" while its third rung says "otherwise record the thing as unverified and move on". A secondmate is a firstmate in its own home whose job description is the local AGENTS.md (charter line 220, and bin/fm-home-seed.sh:426 refuses a home without it), so section 8 binds it: AGENTS.md:384-385 requires exactly one live supervision cycle using the emitted protocol and states "Do not substitute another harness's wait shape, use shell&, or create a second cycle when a healthy one already exists", AGENTS.md:387 states "No turn ends blind while work is under way, including turns described as holding or waiting", and AGENTS.md:167 puts the exact wait or wake mechanism under that emitted protocol. Concrete path: the secondmate spawns crewmate fm-x, follows rung 1 and starts a background poller for fm-x (a second cycle / substituted wait shape), or follows rung 3 and records fm-x's result as unverified and ends the turn with fm-x in flight (a blind turn end). The contradiction is adjacent inside the generated charter itself: line 222 already names "watcher" as the lifecycle mechanism, line 223 says "Do not invent a second delegation system", and the charter's own Definition of done (lines 262-265) has the secondmate reconcile in-flight work and then wait silently. The round-3 fix corrected only the final rung, which now correctly says an unshortenable wait "belongs to this home's own watcher and status cycle", so it stopped one rung short: the cheapest close is to send the whole crewmate-wait case to that same cycle and scope the three rungs to a background job of the secondmate's own, keeping the accepted criterion (never wait in a foreground blockingsleep) intact and the clause no longer than it is now. Flagged for your decision rather than changed, because your round-1 instruction deliberately extended this contract to cover waiting on crewmates.bin/fm-brief.sh:477- Noting a residual so the crewmate copy is not assumed closed by the round-3 secondmate fix. Rule 8's sentence antecedent is "To wait on your own background job" (line 474, identical scout copy at :349), and its last rung sends that wait to$PAUSED_VERB: {why}(line 477, scout :352). Rule 4 permits that verb "ONLY when you are deliberately idling on a known external wait you expect to clear on its own" (lines 460-463) and AGENTS.md:395 states the same contract fleet-wide ("a bounded external wait expected to clear on its own"); bin/fm-watch.sh:352 then renders the resurfaced firstmate wake as "awaiting external - declared pause". So a worker whose Definition of done depends on its own long background job declares an internal wait under an external-wait verb and firstmate rechecks it under a reason that misnames it. This is informational, not a request to change: the realistic mode=no-mistakes DOD wait (done: PR {url} checks green, line 420) is genuinely external; a crewmate, unlike the secondmate, has no own-home watcher cycle to route an internal wait to, so the pause verb is the only parking signal it has and the watcher behavior it selects (the bounded 3600s pause cadence instead of a 240s wedge escalation) is the desired one; and you affirmed this exact pointer twice, adding "and stop" to it in round 2.🔧 Fix: drop paused pointer and secondmate wait ladder
1 info still open:
bin/fm-brief.sh:52- Informational, no action needed: the three pipeline fix-round commits net to exactly zero against the author's intake commit, so the reviewed tree is byte-identical to 5ed7c6f.git diff 5ed7c6f..HEADis empty, which independently confirms every part of the round-4 removal instruction landed: the pause-verb pointer is absent from both rule 8 copies (bin/fm-brief.sh:345-349 and :469-473 end at "otherwise record the thing as unverified and move on"), the secondmate Operating model (:219-226) carries no waiting ladder, the header summary at :52-56 again attributes the waiting contract to ship and scout rules while attributing the incremental-write contract to every document-producing scaffold (accurate: scout report at :354-355 and secondmate detailed answer at :237-238 are the only two), and tests/fm-brief.test.sh shrank back rather than being edited, with 22 functions defined and 22 registered. The practical consequence is only PR history shape: the branch carries three review-round commits whose combined diff is nothing, so a reader diffing HEAD~1 sees a removal that is really a revert to the accepted intake change.✅ **Test** - passed
✅ No issues found.
bash tests/fm-brief.test.sh(all 22 tests pass, includingtest_briefs_forbid_waiting_in_a_blocking_sleepandtest_document_producing_briefs_require_incremental_writes)Non-vacuity: extracted base commit 1cb900c withgit archive, copied the new test file in, ran it against the pre-change script - it fails (not ok - no-mistakes: brief does not forbid waiting in a foreground blocking sleep)Generated all five variants from both the base and target scripts (fm-brief.sh <id> demo-proj --mode no-mistakes|direct-PR|local-only,--scout,--secondmate alpha) and counted each instruction needle per variant: 0 occurrences pre-change everywhere, correct placement post-changePath-normalizeddiffof every generated brief, base vs target: 0 removed or modified lines, additions only (existing safety contracts intact)./bin/fm-operational-input.sh encode launch-brief < .../scout-task/brief.md- confirmed both instructions survive into the actual launch prompt fm-spawn.sh delivers to the agent./bin/fm-brief.sh ship-lab demo --mode no-mistakes --herdr-laband--scout --herdr-lab- rule 8 present alongside the Herdr declaration./bin/fm-brief.sh --help- header pointer note renders in the CLI helpbash tests/fm-ask-user-authority.test.sh,bash tests/fm-secondmate-safety.test.sh,bash tests/fm-tangle-guard.test.sh,bash tests/fm-subagent-pretool-check.test.sh(all pass)bash tests/fm-decision-hold-lifecycle.test.sh- skips in this environment (skip: tasks-axi not found), does not reference fm-briefgrep -P '[^\x00-\x7F]'over every added source line and every generated brief - no em-dashes or other non-ASCIIgit status --porcelain- worktree clean, no transient test artifacts left behind✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.