Code review of 1185 (merge 0f4b0c95) found the retry subscriber's central
invariant did not hold: the MergeFailure->Merge bounce caused by its own
retry reset both its attempt counter and the block subscriber's counter,
so the shared merge_failure_block_threshold budget was unreachable and a
deterministic gates failure retried forever.
One subscriber now owns one counter driving both policies:
- Counter survives PipelineEvent::MergeRetryStarted bounces (finding 1);
a third consecutive failure blocks even with retries in between.
- Mixed failure kinds share the single budget (finding 5).
- Retries respect recovery: no counting or scheduling while a mergemaster
is active, and perform_auto_retry re-checks before firing (finding 2).
- perform_auto_retry applies the same eligibility gates as
assign_merge_stage (review hold, frozen, blocked, unmet deps) so freeze
now stops a retry loop (finding 4).
- Per-story scheduling generations invalidate stale sleeping timers
(finding 6).
- One-shot startup scan schedules a catch-up retry for stories already
parked in GatesFailed, so restarts no longer strand them (finding 3);
kept out of the periodic reconciler to avoid re-retrying exhausted
stories every tick.
- Chat is notified only after the merge actually starts; a failed trigger
logs instead of claiming a retry ran (finding 7).
- Config reads moved onto spawn_blocking (finding 8, bug 1170 class).
Deletes merge_failure_retry_subscriber.rs; notification plumbing
(WatcherEvent::MergeAutoRetry et al) is unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fHdm92yjvguPi2LiXfLB9
Killed:
- rebuild_and_restart (in-container cargo self-compile): the MCP tool,
the `rebuild` chat command in all four transports, the web-ui bot
command, and the underlying function. This was the path that caused
the exec() deadlocks.
- upgrade_sled gateway MCP tool: second entry point to sled upgrades,
defaulted to serving the gateway's own macOS binary to Linux sleds.
- GET /api/huskies-binary (both sled and gateway route trees): served
current_exe(), wrong platform when the gateway is macOS. Superseded
by /api/artifacts/ which now also serves on the gateway route tree.
- `huskies upgrade` CLI subcommand and --source flag: third way of
doing the same download-and-replace. Escape hatch for a bricked sled
is `docker cp` + restart.
Kept, distinct jobs: `project-rebuild` (container/image updates),
`rebuild gateway` + script/local-release (gateway self-update).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fHdm92yjvguPi2LiXfLB9
The merge gate classifier was matching trigger keywords like
`missing_doc_comments` inside passing-test name lines
(e.g. `test agents::gates::tests::classify_lint_from_missing_doc_comments ... ok`),
causing every gate failure to be mis-classified as Lint and bounced
back to a fixup coder. Strip `test … … ok` lines before scanning for
lint triggers. Also removes the temporary diagnostic block in
runner.rs that confirmed the bug.
Applied directly to master because the 1101 feature branch carried
stale work from an earlier incarnation of the story that semantically
conflicted with master's later diagnostic commit (`is_fixup` deleted
on the branch, referenced on master).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bug 1101's reframed AC1: when a non-success merge runs, log the typed
GateFailureKind, the matched classifier-trigger substring (if any) and
~90 chars of surrounding context. Fires on every gate failure regardless
of routing, so the next fixup-loop bounce will tell us which substring is
fooling classify() into Fmt|Lint|SourceMapCheck on what's actually a Test
failure.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stop_agent had the same order-of-operations bug fixed in the watchdog:
status flipped to Failed before the claude process was verified gone,
opening the idempotency window that allowed a duplicate spawn to race
in alongside the surviving process.
Now follows the three-step protocol:
1. Read worktree path under a read-only lock (no mutation).
2. SIGKILL the worktree's process tree via process_kill and block
until verified gone — start_agent's Running/Pending whitelist
continues to reject duplicate spawns throughout.
3. Only then mutate the agent record, abort the task handle, and
drop the child_killers entry.
Falls back to the old portable_pty SIGHUP path (with a warning) when
no worktree was recorded, matching the watchdog's behaviour.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `crate::process_kill` — reliable SIGKILL-with-verify primitives used
across the server in place of the various ad-hoc kill paths that ignored
their kill-effective return values. The module exposes three pieces:
- `sigkill_pids_and_verify(pids)`: SIGKILL each pid and block (up to 2s)
until every pid is verified gone. Returns survivors if not.
- `pids_matching(pattern)`: pgrep -f wrapper.
- `descendant_pids(root)`: recursive pgrep -P walker for process trees.
Wires the watchdog's limit-termination path through it, and reorders the
protocol to fix the duplicate-coder bug observed on story 1086 (2026-05-15):
Before: check_agent_limits set status=Failed before the kill ran. The
kill itself was `portable_pty::ChildKiller::kill()`, which sends SIGHUP
on Unix — claude-code ignores SIGHUP, so the process kept running while
the agent record was already marked terminated. The idempotency check
in `start_agent` whitelists Running/Pending, so the next auto-assign
pass spawned a fresh agent alongside the still-alive prior one. Two
claude PIDs sharing one session_id, racing on the same worktree.
After: status update is moved OUT of check_agent_limits and into the
caller AFTER the kill is verified. The kill itself is now SIGKILL-the-
process-tree-in-the-worktree, with explicit verification that every pid
is gone. The idempotency window is closed.
The existing watchdog test suite (14 tests) still passes; 7 new tests
cover the process_kill primitives directly.
`agents/pool/process.rs`'s `kill_all_children` and `kill_child_for_key`
still use the old portable_pty SIGHUP path — they have the same bug but
in lower-impact code paths (shutdown, operator stop). They will be
migrated under a separate story to keep this commit focused.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The orientation bundle was 96 KB per coder spawn with 85 KB of that being
source-map.json — a static symbol listing that drowned out the workflow rules
in AGENT.md and likely explains why PLAN.md ceremony is being skipped (the
instruction is ~5% of the bundle, buried under a wall of symbols). Agents are
excellent at grep on demand, so the source map adds little value as a preloaded
cheat sheet. File stays on disk for the merge-time source-map-check doc-coverage
gate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1018's merge_failure_block_subscriber counted every MergeFailure transition
toward the 3-strike block threshold, but mergemaster's recovery iterations
(squash → fail → fix → retry) emit multiple MergeFailure transitions while
making real progress. Story 997 was blocked at 10:59:46 while mergemaster
was still resolving conflicts and would have succeeded a minute later.
Fix: pass the AgentPool to the subscriber. When a mergemaster agent is in
the pool for the story, MergeFailure transitions are recovery iterations
in progress and do NOT increment the consecutive-failure counter. Block
only fires for the genuinely-stuck case (no recovery agent attached and N
consecutive failures accumulate).
Tests:
- mergemaster_running_suppresses_block: 3 failures with recovery_running=true
→ counter stays empty, story stays in MergeFailure
- no_mergemaster_still_blocks_at_threshold: 3 failures with recovery_running=false
→ blocks (1018 behaviour preserved)
All 2938 tests pass.
The progress-aware no-progress cap (3 consecutive byte-identical diffs)
doesn't catch the degenerate pattern where the agent keeps making
DIFFERENT file edits each session but never commits — every respawn
resets the no-progress counter, infinite loop, budget burns.
Adds ContentKey::CommitRecoveryTotalAttempts: an absolute counter that
increments on every commit-recovery respawn regardless of progress.
TOTAL_ATTEMPTS_CAP = 8; when hit, block with reason 'agent flapped — N
respawns without ever committing'.
Two caps now bound the recovery loop:
- NO_PROGRESS_CAP (3): catches stuck-agent (same diff repeatedly)
- TOTAL_ATTEMPTS_CAP (8): catches flapping-agent (different diffs, no commits)
Easy to tune the constant lower if we see runaway in practice.
All 2936 tests pass.
The existing commit-recovery path blocked stories on the 2nd consecutive
exit-without-commit. For long sweep refactors (e.g. story 997, the typed
retries payload migration), claude-code's session-length boundary
naturally terminates the coder mid-sweep before it can commit — even
though substantial file-edit progress is being made each session. The
old cap-of-1 misclassified normal mid-flight progress as 'agent declined
to commit'.
New behaviour:
- Each commit-recovery respawn captures a worktree-diff byte-length
fingerprint (git diff master | wc -c).
- If the fingerprint differs from the previous attempt the agent made
file-edit progress, the no-progress counter resets to 1.
- If the fingerprint is byte-identical (no new edits between exits),
increment the no-progress counter.
- Block only when the counter reaches NO_PROGRESS_CAP (3) — i.e. three
consecutive respawns where the agent did literally nothing.
Adds ContentKey::CommitRecoveryDiffFingerprint to store the prior
fingerprint. Updates the existing block-test to reflect the new cap
semantics; existing 'first respawn issued' test continues to pass.
All 2935 tests pass.
Temporary diagnostic added to reap_stale_merge_jobs to surface the t,
current_boot, and decoded values being compared on every reap pass.
Will revert once the disappearance bug is understood.
When deterministic-merge produces a clean git squash but the post-squash
compile fails (typical when master gained a Stage payload field after the
feature branch forked — e.g. story 1018 hit `error[E0063]: missing field
plan` after 1010's PlanState landed), the failure is morally a merge
conflict that git's diff3 missed: the conflicting literal lives in a
different file from the type definition that changed on master. Routing
it as GatesFailed left mergemaster idle and the story stuck.
Changes:
- gates.rs GateFailureKind::classify: detect rustc compile errors
(`error[E\d+]`) as Build instead of falling through to Test. Clippy
errors (`error[clippy::...]`) still classify as Lint.
- agents/merge/mod.rs: new MergeResult::to_merge_failure_kind() method.
GateFailure with failure_kind=Build maps to ConflictDetected (so the
existing 998 subscriber auto-spawns mergemaster). Other gate failures
stay GatesFailed.
- agents/pool/pipeline/merge/runner.rs: replace the inline match with a
call to the new method.
Tests: 6 new unit tests covering the classifier branch and every
to_merge_failure_kind arm. All 2932 tests pass.
Huskies Agent
TimmyandClaude Fable 5