observe-run: prescribe a backgrounded force-run, so a long run reads as a long run - #262
Conversation
…d call it outlives Step 1 told the caller to hold a foreground `Bash` call — "it blocks for the whole run, which is the point". A foreground call in this harness is moved to the background at 600 seconds and hands back nothing of the stream when it is, while the runs measured on 2026-08-10 were 32m 48s (producer) and 22m 30s (vetter). So the instruction could not be honoured for a producer run, and the ten minutes of silence it did buy read as a hang: vetter run 20260810T102325Z was interrupted and killed three auditors deep, working. Step 1 now starts the run in a background call, routes the progress read to that call's output file and the reattach to the printed `log:` path and `watch-run`, and says outright that no `tail`, `head` or filter may sit between the stream and the reader — the first attempt on 2026-08-10 piped through `tail` and saw nothing before any timeout was reached. The kill claim is re-established rather than carried over: stopping the background task kills the whole process tree, the runner among it, and the flock goes with the descriptor. Probed against a stand-in of the same shape (a parent streaming a child holding an `flock` on an open descriptor) for a call backgrounded from the start and for one the ceiling backgrounded; neither process survived and the lock re-locked immediately. The rule is pinned by `long_run_step_faults`, over step 1 alone: the emphasised call mode must be `background`, both buffering readers must be named as commands, and the reattach route must name the printed line and the subcommand. Closes #260 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR updates the Changesobserve-run execution workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/human-fsm/commands/observe-run.md`:
- Around line 61-66: Update the explanation in the observe-run documentation:
state that tail without -f waits for EOF, while head emits the requested lines
and may close the pipe early, causing force-run to receive a broken-pipe error.
Preserve the guidance to avoid piping the stream and read the file directly.
In `@pr-review-report-rs/src/main.rs`:
- Around line 63691-63699: Update the validation logic around the
BUFFERING_READERS and REATTACH_ROUTE checks to reject contradictory
instructions, not merely accept token mentions: require explicit prohibition of
each buffering reader and affirmative routing for each reattach token. Add
negative fixtures covering prescribed buffering and denied reattachment, while
preserving faults for missing required instructions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1e2c637f-abc8-46ed-abd5-e69b025c2482
📒 Files selected for processing (4)
.claude-plugin/marketplace.jsonplugins/human-fsm/.claude-plugin/plugin.jsonplugins/human-fsm/commands/observe-run.mdpr-review-report-rs/src/main.rs
… it from the step Step 1 of `/observe-run` carried the measurement as narrative: the two run durations, the killed vetter run and what it had already done, and the post-mortem on the `tail` pipe. That is the account of an incident, and the account belongs in `mistake-journal.jsonl` where authoring pays for it once, not in a command a model reads to find out what to do. LJ-0004 is that incident, verified against the artifacts rather than carried over from the narrative: the foreground instruction as it read, the two piped invocations and the empty task output file ten minutes in, the interrupt at 7m 07s, the truncated trace at 52 tool calls with three auditors dispatched and no `final` metrics row, and the backgrounded producer call that returned in seconds as the counter-example. Its rule is this step, so it is an addressed kind and not a rule-less backlog member. Step 1 keeps the rule — background call, read its output file, nothing buffering between the stream and the reader, `log:` plus `watch-run` as the route back — and cites `[LJ-0004]` where the narrative was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s one `tail` and `head` were described as failing the same way — "both hold the whole stream until the writer exits". Only `tail` does. `head -n` takes its lines and closes the pipe; `force-run` then stops echoing but keeps waiting on the run, so the call still blocks to the end and hands back a stale head of the stream instead of the stream. Both are banned, for their own reasons. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reworked at Every figure was re-derived from the artifacts, and three did not surviveThe narrative this PR carried was written from recollection. Before it went into the journal — where a fabricated measurement is the worst defect the artifact can have, which is why
And what did not:
Two things the artifacts gave that the recollection did not, and both are in the entry:
|
| step 1 | |
|---|---|
before this PR (f5f685b) |
1,852 |
this PR at ad44137 |
3,770 |
| exposition cut | −194 |
head correctness fix (below) |
+271 |
| now | 3,847 |
The exposition cut is real but small, because ad44137's step 1 was already mostly rule. The net is +77, and it buys a correct statement of a mechanism that was wrong.
CodeRabbit
Swept via GraphQL rather than the check state. Two unresolved threads, both answered, both resolved.
head was described wrongly, and that is fixed. The step said tail and head "both hold the whole stream until the writer exits". Only tail does. Tracing head through force_run_mode gives a different symptom from the broken-pipe abort CodeRabbit proposed: stream_run_output returns the moment writeln! errors, and the next statement is child.wait() — so the runner keeps going, force-run keeps blocking, the pipeline does not return early, and the caller gets the first few lines delivered at the end. Both readers are now banned, each for its own mechanism.
The gate accepts a step that keeps every token and means the opposite. True, and it is the blind spot the comment above the gate already states: GOOD_STEP's step 2 is deliberately wrong so the scoping test can prove a rule satisfied next door is not satisfied here. Not tightened, because requiring a literal prohibition phrase turns the gate into a checksum of two sentences — exactly what an_unemphasised_mention_of_the_other_mode_is_not_a_prescription exists to prevent — and any phrase it demanded could be negated in the following clause anyway. Raised for a human rather than silently changed.
Gates
-
mainmerged in, twice:d854b15(Journal LLM mistakes separately from the prompt, so rules can be deleted #258, which is what makes appending to the journal safe — it turned the seeded-journal tests from equality to presence/containment) andf8cfe7d(Make CLAUDE.md a router: 47,797 → 20,983 bytes, with a pointer per moved file #263, theCLAUDE.mdrouter). Both clean; the full suite was re-run on the merge commit, not trusted to the absence of markers. -
nix develop .#rust -c cargo test— 1,388 passed, 0 failed (1,267 unit + 121 integration). -
pre-commit run --all-filesunder the pinned rainixrust-shell— green, with the onerustfmtrewrite committed and the re-run clean. -
Mutation pass, 7 mutants over the two things that carry the new behaviour — the journal line and the step's citation. 6 kill-expected, 6 killed; 1 control survived as intended. Every anchor was asserted to match exactly once before the mutant was applied, every run routed its output through a file, and every run reported its own test count (1,267) so a zero-match filter could not fake a survival. Baseline and restored both green.
mutant verdict LJ-0004'srule→ absentKILLED LJ-0004's populationinteractive→vetterKILLED evidence loses the tool calls 52quoteKILLED evidence loses the empty-task-file quote KILLED [LJ-0004]→[LJ-0009]in the step (citation resolves to nothing)KILLED citation moved out of step 1 into step 2 KILLED CONTROL — the entry's narrative reworded ( 7m 07s→ "a long silence")SURVIVED, as intended: the gate reads the entry's structure and its quoted evidence, not its prose
One thing found and left alone
every_journal_citation_in_a_prompt_resolves_to_an_entry scans RUNNER_CONTEXT_FILES — campaign-prompt.txt, campaign-worker-prompt.txt, review-prompt.txt, CLAUDE.md. A plugin command is not in that set, so the [LJ-0004] this step now carries is outside the repo's general citation gate. That is arguably right — a plugin command is read when a human invokes it, not shipped into every cron turn, which is the cost the set is about — but it means nothing generic would catch the citation going stale.
Pinned specifically instead, in journal_tests::the_journal_records_the_incident_the_observe_run_rule_came_from: the id must resolve to an entry, and it must appear in step 1, not merely somewhere in the file. Both are mutation-verified above. Whether the general scan should widen to cover plugins/human-fsm/commands/ is a design call, left for a human.
Also noted, not touched: cargo clippy -D warnings locally flags collapsible_match at main.rs:4233. It is byte-identical to origin/main and dates to 944a214 (2026-08-03), so it is a local-toolchain-vs-pinned-CI difference and not this branch's.
The gate guarded the wrong direction. It watched for someone editing `**background**` back to `**foreground**` — a regression nobody has made — while the mistake that actually happened was a human following a wrong instruction. It could not see the failure that would matter, a step saying `**background**` in a sentence meaning the opposite, and its three constants were a second source of truth for what one paragraph must contain. Removes the whole `#260` block: `BASH_CALL_MODES`, `BUFFERING_READERS`, `REATTACH_ROUTE`, `LongRunStepFault`, `emphasised_call_modes`, `long_run_step_faults`, `command_step`, `GOOD_STEP` and all eight tests. Nothing outside the block referenced any of it. The entry-shape assertions over LJ-0004 go with it, for the same reason: the parser already validates the entry on every read, so asserting its fields again was a second source of truth. What is left is the one property nothing else guards — `every_journal_citation_in_a_prompt_resolves_to_an_entry` scans the runner context files, and a plugin command is not one, so a `[LJ-0004]` that resolved to nothing would go unnoticed. Step 1's buffering ban is cut to the operative instruction and one clause on why, naming `tail` because that is the one a caller reaches for; the `head` mechanism was speculative and the citation is what the detail is for. The kill paragraph keeps its corrected mechanism and its probe, without the stand-in's anatomy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Cut the conformance gate, at RemovedThe whole It guarded the wrong direction. What it watched for was someone editing The entry-shape assertions over Kept
The one Rust thing left is 28 lines, and it is not a conformance gate over the prose: fn the_observe_run_citation_resolves()
CompressedThe buffering ban was two mechanisms at length.
Step 1 byte count — it does not get under, and here is exactly where
−822 from the last head, but +1,096 over the 1,852 target. I did not trim substance to reach it. Per paragraph, against what was there before:
Everything above 1,852 is on the keep list: the invocation paragraph, the pipe ban, the route back, and the corrected kill mechanism. Getting under the old number means deleting one of those. The old number bought its size by being wrong — 156 bytes that could not be followed. The two pre-existing untouched paragraphs are 1,085 bytes of the 2,948, and the Gates
Diffstat, whole PR against
|
…t of step 1 alone The replacement paragraph argued its case at length — the 600-second ceiling, the blank screen, "read as a hang and killed", "returns in seconds", the EOF mechanism, the empty output file. All of it is incident narrative, and LJ-0004 is where that lives now; the step gets the instruction and the citation. Four paragraphs become one: background call, read the file it names, no pipe of any kind, one read then move on, and the reattach route when the file is gone. The kill correction keeps its second sentence and nothing else. The pre-existing paragraphs are restored byte-identical to main. This PR's diff is two hunks in text this PR owns; the fast-forward, watch-filter and --force paragraphs are not this PR's to edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Cut to the instruction, at The whole change to the command-Run it in a **foreground** `Bash` call. It blocks for the whole run, which is
-the point; wrapping it in anything that returns immediately abandons the wait.
+Start it in a **background** `Bash` call and read the output file it names.
+Don't pipe it — not `tail`, not `head`, nothing: anything between the stream and
+you holds it. One read when you want to know where the run is, then get on with
+something else; the exit arrives as a task notification. If that file is gone,
+the `log:` line names where the same bytes are still going and step 2's
+`watch-run` reattaches there. [LJ-0004]
**Killing mid-run is sometimes right** — the 2026-08-09 run was killed once.
-Kill this call and the runner dies with it, which releases the flock, because
-the lock is an open descriptor on the runner process.
+Stopping the background task kills the runner and releases the flock [LJ-0004].
At the previous head the same region ran to 2,055 bytes across four paragraphs. It is now one paragraph and one sentence. Everything cut — the 600-second ceiling, the blank screen, the killed run, "returns in seconds", the EOF mechanism, the empty output file, the probe anatomy — is in Restored byte-identical to Operative instructions, all present: background One thing to flag on the kill citation
Gates
DiffstatThe journal entry, the two version bumps and the 28-line |
`[LJ-0004]` pointed at an entry that evidences only half the claim. The incident does show that stopping the task kills the runner — the killed run wrote no `END` line and no `final` metrics row — but the flock release rests on a probe the entry does not carry, and it should not: a successful mechanism check is not a mistake, and padding a mistake journal with one to justify a citation is the same defect in a different file. The sentence stands as a bare instruction. The invocation paragraph keeps its citation, which is fully backed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reviewed c91d605: approve |
1 participant
Closes #260
The instruction that could not be followed
/human-fsm:observe-runstep 1 said:A foreground
Bashcall is moved to the background at 600 seconds. The producerrun measured on 2026-08-10 took 32m 48s and the vetter 22m 30s, so the call is
backgrounded before the run ends every time — and it hands back nothing of the
stream when it is. Ten minutes of blank screen is indistinguishable from a hang,
which is how vetter run
20260810T102325Z— fast-forwarded, both stops walked,three auditors dispatched and working — was interrupted and killed.
What step 1 says now
Start it in a background
Bashcall and read that call's output file as therun goes. Backgrounded from the start it returns in seconds naming the file, and
the fast-forward verdict, the
log:reattach path andrunning:are readablethere immediately — the earliest point at which "this is working" is provable,
instead of the latest. Read the file when you want to know where the run is, one
read at a time; the run's exit arrives as a task notification.
Nothing may sit between the stream and the reader:
force-runflushes everyline as it prints it, and
tail/headboth hold the whole stream until thewriter exits. That was the first thing tried on 2026-08-10 and it showed nothing
well before any timeout was reached, so the step now says it outright rather
than leaving it to be rediscovered.
And if that output file is gone — restarted session, or a run this command did
not start — the
log:line names where the same bytes are still being appendedand step 2's
watch-runreattaches there.force-runitself is untouched. Its fast-forward, exit-3 refusal, policy-stopwalk, streaming and exit code are what the issue says they are.
The kill claim, re-established rather than carried over
Step 1 asserted that killing the call kills the runner and releases the flock.
It still does, and it was probed rather than assumed — against a stand-in of the
same shape as
force-run, a parent that streams a child holding anflockon anopen descriptor:
flock -nsucceeded at onceflock -nsucceeded at onceThe whole process tree the task started is killed, the runner among it, and the
lock goes with the descriptor. The paragraph now states that mechanism and says
where the claim comes from.
Two other harness facts were probed the same way and are what the new text rests
on: a foreground call that exceeds its timeout is moved to the background
rather than killed, with its output going to a task file readable line-by-line as
it accumulates; and
<stream> | tail -20against a source emitting one line asecond produced zero bytes in 8 seconds, which is the buffering the step now
bans by name.
Scope
No
force-runbehaviour changed. The Rust diff is test-only — a gate over thestep's text — plus the plugin version bump both
version-hygienejobs require.Nothing in the issue turned out to need a code change to make the instruction
honest, so nothing was widened.
One thing worth a human's eye, raised here rather than acted on: step 2 still
opens "Only needed for a run this command did not start, or one whose stream was
cut." That is now also the route when a backgrounded task's output file is gone,
and step 1 points at it, but step 2's own sentence was left as it was.
QA
Discriminating tests:
marketplace_tests::observe_run_step_1_prescribes_an_invocation_a_long_run_survives(the file), plus seven unit tests over the pure gate —
a_step_prescribing_the_mode_a_long_run_survives_is_clean,a_step_that_prescribes_a_foreground_call_is_the_instruction_260_is_about,an_unemphasised_mention_of_the_other_mode_is_not_a_prescription,a_step_that_emphasises_no_mode_prescribes_nothing,a_buffering_reader_the_step_never_names_is_not_banned,a_backgrounded_run_with_no_route_back_to_its_stream_is_a_fault,the_rule_is_read_in_its_own_step_and_a_neighbour_neither_breaks_nor_satisfies_it.Each fails on base: the base
observe-run.mdstep 1 is exactly the**foreground**prescription the file test rejects, with notail/headbanand no reattach route in the step, so all four fault variants fire on it —
verified by applying that base text as mutant
md: prescribes a foreground calland by dropping each token, below.Mutations applied — 15 mutants, 13 kill-expected and 13 killed, each probe
reporting the number of tests it ran (32) so a zero-match filter cannot fake a
survival:
observe-run.md:**background**→**foreground**observe-run.md:`tail`unbackticked in step 1observe-run.md:`head`unbackticked in step 1observe-run.md:`watch-run`unbackticked in step 1observe-run.md:`log:`unbackticked in step 1observe-run.md: step 1 heading unnumbered (## Force the run …)observe-run.md: step 1 renumbered## 3.BASH_CALL_MODES:"foreground"dropped from the vocabularylong_run_step_faults:PrescribesForegroundnever raisedlong_run_step_faults:NoModePrescribednever raisedlong_run_step_faults: buffering-reader check inverted (!dropped)long_run_step_faults: reattach-route check deletedcommand_step: sibling predicate →|_l| false(step runs to EOF)emphasised_call_modes:**{mode}**→{mode}(every mention counts)9m 01s/4m 12s, "came to be read as a hang and killed" → "was mistaken for a hang and interrupted"One further mutant is reported as a known blind spot rather than a gap:
negating the ban in place (
**Nothing may sit between the stream and you.**→**Anything may sit…**) SURVIVES. No gate over prose can see a step thatkeeps every token and means the opposite; that limit is stated in the comment
above the gate rather than left implied. A sixteenth mutant — re-titling step 1
while keeping
## 1.— also survives, correctly: a re-titled step 1 is stillstep 1, which is precisely what the scope keys on.
Oracle: the issue's own constraints, not the code. The four faults are its four
bullets — a producer run always outlives a foreground call; the stream must
reach the reader unbuffered; killing must still work or be disclaimed; a
backgrounded run must stay observable. The durations (32m 48s / 22m 30s), the
600s ceiling and the killed run
20260810T102325Zcome from the issue body.The harness facts the new text asserts — foreground-over-timeout is backgrounded
not killed, the task output file streams live, stopping the task kills the tree
and frees the flock,
| tailyields nothing — were each measured directly, notreasoned from documentation.
Category check: the issue asks for four properties and all four are covered.
Three are pinned by the gate (no foreground prescription; the buffering ban
names its readers; the reattach route names the printed line and the
subcommand). The fourth — the flock claim — is a fact about the process tree
rather than about the text, so it is established by the probe table above and
the step states how it was established; a test asserting the sentence exists
would pin the wrong thing.
Gates run:
nix develop .#rust -c cargo testgreen (1234 unit + 113 integration,0 failed) and
nix develop github:rainlanguage/rainix/53e96a7d0a97d7c7c75c3b2412521324776fdac6#rust-shell -c pre-commit run --all-filesgreen, with what
denofmtandrustfmtwrote committed.No producer or vetter run was forced to test this — both kill switches are in
place and the runs that motivated the issue cost $5.95 and $12.51.
Summary by CodeRabbit
New Features
Bug Fixes