fix(backlog): escalate bouncing items to the rework-cap terminal state#170
Open
tstapler wants to merge 4 commits into
Open
fix(backlog): escalate bouncing items to the rework-cap terminal state#170tstapler wants to merge 4 commits into
tstapler wants to merge 4 commits into
Conversation
reconcileBouncingItems detected items non-convergently bouncing between in_progress and review (>= bounceThreshold cycles, no PASS verdict) but only sent a notification -- it has no way to stop the auto-reopen loop, so a bouncing item just kept bouncing forever with a human expected to notice. The existing rework_cap detector (AutoReopenAfterFailedReview's workCount check) can miss the same non-convergence: workCount only counts actually -created work sessions, but the spawn-failure rollback path records an extra in_progress->review status event without a new work session, so the cycle count can outpace the work-session count. Apply the same isBouncing test AutoReopenAfterFailedReview's respawn choke point uses and, when it trips, escalate through the existing notifyReworkCapHit terminal treatment -- the same StuckReasonReworkCap row and notification shape rework_cap already uses -- rather than respawning again. reconcileBouncingItems now also checks for an already-open rework_cap row before notifying, so the two detectors don't fire two differently -shaped "needs manual attention" signals for the same underlying condition. Regression tests: TestAutoReopenAfterFailedReview_should_escalateToReworkCap_When_BouncingBelowWorkCountCap, TestAutoReopenAfterFailedReview_should_reopenNormally_When_BelowBounceThreshold, TestReconcileBouncingItems_should_skipDuplicateNotify_When_AlreadyEscalatedToReworkCap Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
✅ Registry ValidationTest Coverage: 10/172 features have
|
Contributor
Go Benchmarks (Tier 1) |
Contributor
E2E RPC Latency |
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. |
Contributor
📊 Feature E2E CoverageFeature coverage report unavailable
|
Contributor
Frontend Terminal Throughput |
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>
tstapler
force-pushed
the
worktree-agent-ab0fc502ce6a6c03b
branch
from
July 26, 2026 18:06
cdfa81f to
524ab18
Compare
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
reconcileBouncingItemsdetected non-converging bouncing (item thrashesin_progress<->reviewwith no PASS verdict) but only sent a notification — nothing stopped the auto-reopen loop, so a bouncing item just kept bouncing forever with a human expected to notice.AutoReopenAfterFailedReview(the actual respawn choke point, invoked synchronously the moment any non-PASS review verdict lands) now applies the identicalisBouncingtest the periodic detector uses, and — when it trips — escalates through the existingnotifyReworkCapHitterminal treatment (sameStuckReasonReworkCaprow, same notification shape) instead of respawning again.workCount(the existing rework-cap counter) only counts actually-created work sessions, but the spawn-failure rollback path records an extrain_progress->reviewstatus event with no new work session, so the cycle count can outpace the work-session count and evade the workCount cap entirely.reconcileBouncingItemsnow checks for an already-openrework_caprow before notifying, so the two detectors don't fire two differently-shaped "needs manual attention" signals for the same underlying non-convergence.Context
Scoped from the 2026-07-17 evening "systematic autonomy-gap audit" in
docs/tasks/backlog-feature-improvement.md, which flaggedbouncingas a real gap: "no escalation/different-approach retry once non-converging, just a notification." Live example in that audit: an item bouncedin_progress<->review4 times across 3 sessions with no PASS verdict, dead with no self-service retry affordance.Changes
session/stuck_decisions.go: exportedBounceThreshold,BounceLookback,IsBouncing(thin aliases/wrappers around the existing unexported values) soserver/servicescan reuse the identical bounce test.session/storage.go: addedStorage.CountReviewCyclesSincewrapper (mirrors the existingGetMostRecentReviewVerdictForItemwrapper pattern) so the services package can query cycle counts without reaching into the unexportedEntRepository.server/services/backlog_service_triage.go:AutoReopenAfterFailedReviewnow runs the bounce check after the existingworkCount >= maxAutoReworkIterationscheck and, if it trips, callsnotifyReworkCapHitand returns without respawning.session/backlog_lifecycle.go:reconcileBouncingItemsnow skips its own notification (but still writes/marks the bouncing row for UI visibility) when an openrework_caprow already exists for the item.TestAutoReopenAfterFailedReview_should_escalateToReworkCap_When_BouncingBelowWorkCountCap,TestAutoReopenAfterFailedReview_should_reopenNormally_When_BelowBounceThreshold,TestReconcileBouncingItems_should_skipDuplicateNotify_When_AlreadyEscalatedToReworkCap.Impact
sessionandserver/servicespackages only (backlog reconciliation loop). No proto/schema/UI changes.CountReviewCyclesSince) per non-PASS review verdict, same queryreconcileBouncingItemsalready runs periodically.Testing
go build ./session/... ./server/services/...— cleango vet ./session/... ./server/services/...— cleangolangci-lint run ./session/... ./server/services/...— 0 issuesgo test ./session/...— full suite greengo test ./server/services/...— full suite greenReviewer Notes
reconcileBouncingItems(periodic) andAutoReopenAfterFailedReview(synchronous) — the periodic sweep is the backstop for reopen paths that bypass the synchronous check (e.g. a manual reopen).🤖 Generated with Claude Code