fix(backlog): auto-respawn triage for items orphaned in idea#169
Open
tstapler wants to merge 13 commits into
Open
fix(backlog): auto-respawn triage for items orphaned in idea#169tstapler wants to merge 13 commits into
tstapler wants to merge 13 commits into
Conversation
reconcileOrphanedTriageItems (session/backlog_lifecycle.go) previously only wrote a stuck row and notified an operator once a triage session's crash/kill/ restart went undetected past the 2h staleness threshold — its own doc comment claimed the item "self-heals once it leaves idea," but nothing ever moved it out of idea, so it sat there forever until a human noticed and manually re-triggered triage (docs/tasks/backlog-feature-improvement.md, row 8). Adds a TriageRespawner seam (mirrors the existing PRFixSpawner pattern) and wires BacklogService.AutoRetriggerTriage into it via the existing TriggerTriage entry point, following the same shape as AutoReopenForPRFix/ AutoReopenAfterFailedReview: status guard, an elapsed-time "might still be running" check (headless triage has no liveness signal, so age is the only proxy — mirrors reconcileOrphanedTriageItems' own staleness gate), and a cap on triage-session count (reusing maxAutoReworkIterations/notifyReworkCapHit) to stop a persistently-crashing item from re-triggering forever. Dispatched async under the existing reviewSem limiter so TriggerTriage's dispatch never blocks the synchronous reconcile sweep. Fires on the same tick the 2h staleness threshold is crossed — no separate grace period, matching reconcileStaleWorkSessions' established pattern (the staleness check itself is the "don't act on first detection" gate). Doc comment on reconcileOrphanedTriageItems corrected to describe what the code now actually does, rather than the previously-false self-heal claim. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…us-desync bug Second, compounding root cause for "nothing merges": GitHub repo-level allow_auto_merge is disabled, so EnablePRAutoMerge's gh pr merge --auto call has been silently failing on every PR. Also notes the pr_pending status-desync bug blocking ReconcilePRPending for PR #157, and a fabricated subagent report incident from this audit pass.
…icy scope Confirmed via direct code read: GetPRStatus never distinguishes a still-running check from a passed one, so ReconcilePRPending's healthy-PR branch can fire the ready-to-merge notification before CI finishes. Also records the backlog-pr-mergeability-policy SDD scoping outcome and an open design tension between repo-level auto-merge (enabled this session) and the plan's per-item-gated auto-merge design.
…ng don't self-heal Systematic pass over all 7 StuckReasons: only pr_pending auto-recovers. orphaned_triage's code comment falsely claims self-healing it doesn't actually do. bouncing has no escalation path. Also confirms the MCP controller-startup issue from the earlier audit doesn't affect backlog-spawned sessions, which are fully wired synchronously.
…075c8f84c7f # Conflicts: # server/dependencies.go # server/services/backlog_service_triage.go # session/backlog_lifecycle.go # session/backlog_lifecycle_test.go
…stuck rows Live-verified via the deployed reconciliation sweep: the abandoned-review auto-respawn correctly fires exactly once for newly-notified rows, but rows already marked notified before the fix deployed never trigger a respawn since it shares the same notify-once gate. Correct going forward, just doesn't retroactively catch pre-existing stuck items.
Contributor
✅ Registry ValidationTest Coverage: 10/172 features have
|
Contributor
Go Benchmarks (Tier 1) |
Contributor
E2E RPC Latency |
Contributor
Frontend Terminal Throughput |
Contributor
📊 Feature E2E CoverageFeature coverage report unavailable
|
Contributor
🎬 E2E Feature Demos2 shard(s) recorded feature flows for this PR. recordings shard 1 Demo preview opens directly in browser (single-file HTML). Raw WebM recordings in ZIP. Expires after 30 days. |
…session/tmux session/integration_test.go's TestMain reaper only recognized "test_coldrestore_" socket names, silently missing "test-isolated-" (the name testSocketOnce in session/tmux generates and every package's tests share). session/mux had no reaper at all. A SIGKILLed test binary in either package left its tmux server running indefinitely — confirmed live in production as 4 multi-day-old orphaned "tmux: server" processes eating memory and holding stale worktree paths. Consolidate into testutil/tmuxreap, a dependency-free leaf package (unlike testutil itself, which already imports session/session-mux/session-tmux and so can't be imported by their internal-package test files without a cycle). All three packages' TestMain now call the same ReapLeakedTestServers / StartTestServerWatchdog, covering the full set of test socket prefixes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XXewUpVzfJEKJoKMg99JNN
Ad-hoc merges into ~40 backlog/agent worktrees (done manually this session to land the tmux-reaper fix) showed the gap plainly: worktrees silently drift behind main until someone remembers to merge by hand, so fixes on main (test hygiene, lint rules, bugfixes) don't reach in-progress or idle backlog worktrees on their own. `make sync-worktrees` merges main into every worktree, skips any with uncommitted changes, and aborts + reports (without forcing a resolution) any real conflict for manual follow-up. Safe to re-run — a no-op once everything's current. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XXewUpVzfJEKJoKMg99JNN
…ext file Considered a Go-side periodic reconciler (session/git.MergeMainIntoWorktree already exists and is used by AutoReopenForPRFix's syncPRBranchWithMain) but that only helps a narrow case — SpawnSessionFromItem always branches a fresh worktree off current main on every real reopen, so external auto-merge would mostly be a no-op and would need its own "is this worktree actively in use" safety check to avoid yanking the tree out from under a running agent. Simpler and safer: WriteBacklogContextFile already regenerates .backlog-context.md on every spawn AND every re-attach (see AttachSessionToItem in backlog_service_sync.go) — the harness's existing state-management touchpoint the agent reads as its briefing. Telling the agent to merge main as its own first step is inherently safe (it's the one about to use the worktree) and it can actually resolve a trivial conflict, where a background script could only abort and flag it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XXewUpVzfJEKJoKMg99JNN
…re done An item could reach "done" once PrURL existed at all — an open, unmerged, or later-reverted PR still has PrURL set, so it always satisfied the old guard. "Approval" (a PASS review verdict) and "shipped" (code actually on main) were being conflated: a PASS verdict says the code is good, not that it landed. Three separate call sites could drive a review→done transition, and only one (the TransitionBacklogItemStatus RPC) had any shipped-code guard at all — TriggerReReview's and SubmitManualReview's auto-transition-on-PASS bypassed it entirely by calling storage.TransitionBacklogItemStatus directly. All three now route through the same isCodeShippedToMain check. "Shipped" means the most recent work session's commit is an ancestor of main — checked locally AND via origin (a PR merged remotely on GitHub doesn't require a local pull to count; a commit merged directly to main locally never went through a PR at all). Implemented as git.IsCommitOnMain using go-git (no subshell — see the new prefer-go-git-over-subshells rule), with an origin fetch that's best-effort so an offline/unreachable remote doesn't block the local-merge case. Verification failure fails closed: if the commit can't be confirmed shipped (or the check errors), the transition is blocked rather than silently trusting a stale PrURL. The RPC path keeps its override_reason escape hatch; the two internal auto-transition paths have none by design — they leave the item in review for a human to decide via the RPC path instead. ErrPRRequired renamed to ErrCodeNotOnMain to match what it actually checks now; no external callers referenced the old name (verified via repo-wide grep, zero test references either). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XXewUpVzfJEKJoKMg99JNN
Contributor
✅ Registry ValidationTest Coverage: 10/172 features have
|
These match .gitignore rules already but were tracked from before the rule was added, so gitignore had no effect and automation kept committing per-worktree state to them. git rm --cached only; local copies are untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JqpFYpiUbPMTnZrNW4grg7
Contributor
✅ Registry ValidationTest Coverage: 10/172 features have
|
6 tasks
tstapler
added a commit
that referenced
this pull request
Jul 20, 2026
…kstop (#195) .backlog-context.md and .claude/commands/backlog/*.md are written into every backlog work-session worktree, then git-excluded via $GIT_DIR/info/exclude. That only stops NEW files from being staged — it does nothing once a file is already tracked, which is why this repo's own history has ~20 manual "chore(backlog): untrack backlog context/command files" commits, and why two currently-open PRs (#169, #170) both carry an incidental deletion of a stale, committed .backlog-context.md from an unrelated item's context. WriteSlashCommands and WriteBacklogContextFile now call selfHealWorktreeScaffolding before writing: it walks the git index (via go-git, not a subshell) for any entry matching backlogExcludePatterns and untracks it (git-rm-cached semantics — working tree file is left alone), so a branch that ever got one of these files committed self-heals on the very next spawn/reattach/reopen instead of requiring a manual untrack commit. Also adds a CI backstop (.github/workflows/backlog-scaffolding-guard.yml) that fails any PR whose diff adds/modifies a backlogExcludePatterns file, as a second, independent layer for this repo. This only protects stapler-squad's own repo — it cannot reach into the arbitrary target repos backlog items point at, which is why the self-heal above (not CI) is the primary fix. On the "scoped by session/backlog item so it's not reused poorly" half of the request: chose to harden the existing worktree-relative storage rather than relocate it to a session/item-scoped path outside the worktree (~/.stapler-squad/backlog-context/<item>/...). Both WriteBacklogContextFile and WriteSlashCommands already do a full atomic overwrite on every spawn/reattach/reopen, so no session ever reads content from a different item/session — the actual "reused poorly" risk was exclusively the already-tracked git pollution this fix closes. Relocating storage is a larger, real architecture change (touches taskProtocolBlock's literal `.backlog-context.md` references, needs a session UUID that doesn't exist yet at the pre-spawn write callsite, and increases collision surface with other agents' concurrent work in these same files) and is left as a follow-up rather than folded into this bug fix. CleanupSlashCommands/CleanupBacklogContextFile remain uncalled from any production teardown path, and that's intentional, not an oversight: shipViaAgentOrFallback relies on ship.md still existing after a work session exits review to re-invoke /backlog/ship as a one-shot call. Wiring cleanup into a review-exit teardown would delete ship.md out from under that path. Documented this directly in both functions' doc comments so it isn't "fixed" again by mistake. Regression tests: a worktree with .backlog-context.md (or .claude/commands/backlog/status.md) already committed gets it untracked on the next Write call with fresh on-disk content; respawning the same worktree path for a different item never leaks the prior item's content; the self-heal helper no-ops cleanly on a non-git directory and leaves unrelated tracked files alone. Claude-Session: https://claude.ai/code/session_01BxNAMeGteuzNyN46Q4zAn1 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
reconcileOrphanedTriageItemspreviously only detected and notified when an idea-status item's triage session died mid-run — its own doc comment falsely claimed the item "self-heals once it leaves idea," but nothing ever moved it out of idea.TriageRespawnerseam (mirrorsPRFixSpawner) andBacklogService.AutoRetriggerTriage, which re-triggers triage via the existingTriggerTriageentry point once the item is confirmed orphaned (2h+ stale, per the existing staleness gate), with an active-session re-check and amaxAutoReworkIterationscap to prevent runaway retriggers.reconcileOrphanedTriageItemsto describe what the code now actually does.Context
Found during a systematic autonomy-gap audit (
docs/tasks/backlog-feature-improvement.md, row 8): idea-status items whose triage session crashed, was killed, or never completed after a server restart were flagged and notified but never actually retried — they sat stuck atideaforever until a human noticed and manually re-triggered triage.Changes
session/backlog_lifecycle.go: newTriageRespawnerinterface +SetTriageRespawner/getTriageRespawner;reconcileOrphanedTriageItemsnow dispatches an async respawn (bounded by the existingreviewSemlimiter) once past the existing 2h staleness threshold, in addition to the existing notify; doc comment corrected.server/services/backlog_service_triage.go: newAutoRetriggerTriage(implementsTriageRespawner) andhasFreshTriageSessionhelper — status guard, "might still be running" re-check (headless triage has no liveness signal, so elapsed time since session creation is the proxy), rework-cap guard reusingmaxAutoReworkIterations/notifyReworkCapHit, then delegates to the existingTriggerTriageRPC handler.server/dependencies.go: wiresbacklogLifecycleListener.SetTriageRespawner(backlogSvc).TestReconcileOrphanedTriageItems_AutoRetriggersTriage_When_SessionStale,TestReconcileOrphanedTriageItems_NoRespawn_WhenNoTriageRespawnerConfigured(session package);TestAutoRetriggerTriage_ReworkCapHit_LeavesInIdeaAndNotifies,TestAutoRetriggerTriage_FreshTriageSession_SkipsWithoutDoubleSpawn,TestAutoRetriggerTriage_NoActiveSession_TriggersTriage(server/services package).Impact
triageSem(max 8 concurrent).Testing
go test ./session/...— full suite passesgo test ./server/services/...— full suite passesgolangci-lint run --enable=nilnil,staticcheck,ineffassign,govet ./session/... ./server/...— 0 issuesReviewer Notes
hasFreshTriageSessionstaleness re-check (no liveness signal exists for headless triage sessions, unlike work/review sessions) and the choice to reusel.reviewSemfor the dispatch limiter (matches the precedent set by the sibling abandoned-review-respawn fix, PR fix(backlog): auto-respawn review for items stuck abandoned in review #168, currently open/unmerged).reconcileStaleWorkSessions' established pattern in this file.