fix(runTask): fast-exit must not treat an empty stub branch as merged work - #59
Merged
Merged
Conversation
Default Node execFileSync buffer (1MB) is too small for a real diff package spanning many files (hit during the bs-inventory pilot's Task 13, a 207-file MUI Premium/Pro removal) — git commands would throw ENOBUFS/maxBuffer exceeded instead of returning the diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
The 2026-07-18 fix for the safety-classifier blocking state-write calls
("frame the content as already-verified bookkeeping") turned out not to
resolve the problem — it's exactly that framing that kept getting
flagged as Instruction Poisoning across two real bs-inventory pilot runs
(2026-07-20/21). An isolated agent with no memory of the run has no way
to confirm "trust me, this already happened," which reads exactly like
an injection attempt.
Evidence from those same runs: agents that independently verified
against real repo state (git log, task reports) before writing passed
the classifier without issue. So writeState()/deleteState()/
appendLedger() now instruct the agent to verify the content against the
actual repo (git log, git merge-base, .cys/task-<id>-report.md) before
writing, with an explicit "don't write, report the discrepancy instead"
escape hatch if verification fails — turning the task from "copy this
claim" into "verify and record," which is the pattern that already
worked in practice.
Full context in .cys/pending.md (reopened the 2026-07-18 entry rather
than filing a new one).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
…f-review checks Two real bugs shipped past cys:plan's existing self-review checklist in the bs-inventory pilot (2026-07-20/21), both caught only by the executor's adversarial reviewer, not by writing the plan itself: - BuildKardex used one shared balance accumulator, tested with a single product/warehouse; two other tasks' sample code fed it a whole tenant's multi-product/multi-warehouse movements, silently mixing balances — invisible without tracing the real call sites. - PLE field 4 used the warehouse's internal ID instead of Warehouse.rucEstablishmentCode, which the design spec named explicitly — written from a general sense of "the warehouse" rather than the spec's literal field mapping. Neither "does every function have a test" nor "do types match across tasks" catches either class, so both are now explicit self-review steps. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
…fusing Fixing the classifier false-positive (previous commit) surfaced a real follow-on gap: writeState()'s snapshot is built when the write is queued, but enqueueMainRepo serializes it behind other real repo work, so by the time the agent actually runs, the content can be genuinely stale — not fabricated, just overtaken by real progress. The agent was correctly verifying this (as designed) but only knew how to refuse and report, with no automatic retry anywhere in the script — so state.json stayed frozen at whatever the last accepted write was, observed in a live bs-inventory pilot run staying pinned at the very first snapshot (before any task had even started) for the rest of the run. writeState() now tells the agent that staleness alone isn't a red flag, and to correct the specific task entries itself from what it verifies (real branch/SHA for done tasks; status "in_progress" *and* a "phase" for started-but-unmerged tasks — a status with no phase is an incomplete correction) rather than stopping at "this doesn't match, I won't write it." Refusal is now reserved for entries that look fabricated outright (a "done" SHA that doesn't exist anywhere in the repo's history), not merely out of date. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
settle() and markInProgress() each called writeState() directly, so every task transition dispatched its own [state] agent — with several tasks settling close together (a common pattern: every dependency-free task starting around the same time) this meant one agent per transition, even though a single fresh write right after the cluster would capture the same information. requestWriteState() now coalesces: if a write is already in flight when a new request arrives, it just flags "there's more to record" instead of spawning another agent; the in-flight write's own loop notices the flag once it finishes and does one more pass (reading fresh state at that point) before settling. Crash-resume safety is unchanged — it still writes at least once per settling wave — but the agent count drops with how much settling clusters in real runs. Also fixed a duplicated sentence left over from the previous commit's writeState() prompt edit (harmless but confusing repeated text). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
Same pattern as the state-write coalescing (previous commit): each appendLedger() call dispatched its own [ledger] agent, so several lines queued close together (e.g. multiple tasks failing/conflicting around the same time) fired one agent per line. appendLedger() is now a thin coalescing wrapper — concurrent calls join a pending batch, and one [ledger] agent appends (and spot-checks) all of them together via the new appendLedgerBatch(). Public call sites are unchanged (still appendLedger(line)). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
…agent
Both requestWriteState() and appendLedger() now log a line
("[state] agrupó N pedidos..." / "[ledger] agrupó N líneas...")
whenever a batch actually groups more than one request, so it's
visible in the run's own log output — not just inferable from
comparing agent counts — whether the coalescing is doing anything in
a given run.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
state/ledger/state-clear agents inherit the session's reasoning effort, spending implementation-grade reasoning on mechanical work (verify git, write a JSON file, append log lines). Real pilot data (bs-inventory, 2026-07-21): bookkeeping agents accounted for 35 of 77 dispatched agents. effort:'low' cuts their per-call cost without touching the quality-bearing agents (implement/review/fix/merge/final-review). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
The Workflow resume cache invalidates by prefix — any edit to the first agent call's prompt (e.g. the writeState fixes) re-runs EVERYTHING after it live. Combined with mid-run stop/resume cycles, already-finished tasks kept getting re-dispatched from scratch: the bs-inventory pilot (2026-07-21) re-invoked tasks 4/5/6 three to five times each, and every re-invocation paid a full re-implementation check (testcontainers suite included), a fresh adversarial review, and a merge attempt — 63 review agents and 57 merge agents for a 10-task run, with three implementers independently reporting "this task was already done, why was I dispatched?" in their concerns. Two-part fix: - The implementer prompt now opens with a FAST-EXIT CHECK: if branch task-N exists AND is already an ancestor of the integration branch, report alreadyMerged: true immediately (two read-only git commands, no test re-runs, no worktree). - runTask short-circuits on impl.alreadyMerged: settle(done) directly, skipping the review and merge stages entirely — there is nothing new to review or merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
… work Edge case found before it ever ran: a task branch created right before a run was cut off (e.g. session limit hitting between branch creation and the first commit) points at the integration tip with zero commits of its own — trivially "an ancestor," which the fast-exit would have read as "already merged" and silently skipped the task's entire implementation. Real instance: bs-inventory's task-10 stub after the 2026-07-21 session-limit cutoff. The check now also requires the branch to carry real commits of its own in the integration branch's history; an empty stub gets deleted and the task implements normally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145sjAtP5b5kvhR1XCgg3K3
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.
Summary
Fixes to the state/ledger bookkeeping agents surfaced by two real
bs-inventorypilot runs (2026-07-20/21): the safety classifier kept flagging the "already-verified" framing as Instruction Poisoning,state.jsoncould stay frozen at its first snapshot, bookkeeping agents were a plurality of all dispatched agents, andrunTaskkept re-dispatching already-merged tasks on resume.Type of change
Main changes
writeState()/deleteState()/appendLedger()verify against real repo state before writing instead of just asserting, with a "report the discrepancy" escape hatch (fixes the Instruction Poisoning false positive).requestWriteState()/appendLedger()coalesce concurrent calls into one agent, and log how many got batched.state/ledger/state-clear) run ateffort: 'low'.runTaskfast-exits tasks already merged into the integration branch (skips re-review/re-merge), while still requiring real commits of its own so an empty stub branch implements normally.bin/review-package.js: raisedexecFileSyncmaxBufferto 1GB (was hittingENOBUFSon large diff packages).cys:planself-review checklist: added caller-cardinality and spec-field-literalness checks.Version
featrule)Checklist
npm test)