feat(backlog): durable escalation signals for bouncing/multi-reason stuck items - #444
Merged
Merged
Conversation
Requirements, research, plan.md, and ADR-001/ADR-002 for the multi-reason stuck-state escalation and capped-while-bouncing durable marker feature.
…plumbing Epic 1.1 of backlog-bounce-escalation: two new synthetic, aggregate StuckReason values (multiple_reasons, bounce_cap_exhausted) made valid and round-trippable end to end — Go domain type, proto enum, RPC mapping — ahead of the detector logic (Epic 1.2/1.3) that will set/resolve them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
…s in UI Epic 2.1 of backlog-bounce-escalation: adds labels/icons/GROUP_ORDER entries and a distinct chipEscalated style for the two new synthetic StuckReason values (MULTIPLE_REASONS, BOUNCE_CAP_EXHAUSTED) so they render instead of falling back to "Unknown reason"; excludes them from the otherReasonsCount badge so an item's own escalation row doesn't inflate its own badge; and extends the existing resolved-ghost mechanism to show a de-escalation banner when a multiple_reasons row resolves while the item stays open elsewhere. Also fixes a latent gap in the existing justResolved ghost mechanism: grouped items were derived only from the current items list, so a resolved item's ghost card (added to resolvedGhosts) could never actually render since it had already dropped out of `items` — visibleItems now re-includes still-tracked ghost items until their fade timer clears them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
Adds reconcileMultiReasonEscalation, registered after self_heal in ReconcileStuck: marks a durable multiple_reasons row for items with 2+ simultaneously open non-escalation stuck reasons, dwell-gated notify-once, and resolves it once the count drops back below threshold. Excludes abandoned_review from the count when it's structurally coupled to a gate-blocked bouncing row, per ADR-001. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
Epic 1.3 of the backlog-bounce-escalation plan: when the bouncing remediation gate parks (justParked) while bouncing itself is still open, mark a durable StuckReasonBounceCapExhausted row and upgrade the park notification from generic WARNING/HIGH to ERROR/URGENT framing naming the retry loop specifically — durable evidence the retry loop isn't converging, not just an ordinary single-reason park. - autoReopenWithBackoffGate now takes the item's actual itemStatus (both call sites pass BacklogStatus(item.Status), not a hardcoded in_progress) so MarkStuck's expectedStatus precondition doesn't silently no-op for review-status items. - reconcileBouncingItems resolves bounce_cap_exhausted alongside bouncing at both resolve sites; selfHealStuck gets a mirrored backstop case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
… signals Bumps testIds/lastModified on both per-feature files to cover the new multiple_reasons/bounce_cap_exhausted stuck-reason tests, and fixes a stale test-name reference (contain12Entries -> contain16Entries) found while updating. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
…tion - Gate Signal 2's ERROR/URGENT notify on MarkStuck's applied result, so a failed-closed expectedStatus precondition can't fire a notification with no durable row behind it (architecture review CONCERN). - Combine the identical bouncing/bounce_cap_exhausted selfHealStuck switch cases into one, and extract the repeated resolve-both-reasons pairing in reconcileBouncingItems into resolveBouncingAndCapExhausted (refactor review). - Evaluate the abandoned_review/bouncing coupling exclusion via the already-fetched bouncing row + evaluateRemediation directly, instead of RemediationBlocked's redundant FindOpenStuckStates re-query (Go idiom review). - Document why the frontend de-escalation banner only covers MULTIPLE_REASONS, not BOUNCE_CAP_EXHAUSTED (React review SUGGEST). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
go.mod unchanged; these are checksums for existing transitive dependencies (go-runewidth, tablewriter) that hadn't been recorded yet in this worktree until make build/make test ran full-repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
Contributor
✅ Registry ValidationTest Coverage: 46/193 features have
|
Contributor
Go Benchmarks (Tier 1) |
Contributor
E2E RPC Latency |
Contributor
UX Analysis
|
Contributor
Frontend Terminal Throughput |
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. |
Owner
Author
CI status: 2 failures, both pre-existing on
|
tstapler
marked this pull request as ready for review
August 12, 2026 15:40
7 tasks
Owner
Author
|
Update: opened #445 fixing the deterministic part of this — force-added the 3 generated proto files PR #433 dropped due to The Go |
tstapler
added a commit
that referenced
this pull request
Aug 12, 2026
…ng, isolate tmux-flaky import tests Three related fixes, all surfaced by the same incident (see this PR's first commit and #444's comments): 1. git rm --cached every currently-tracked file under gen/proto/go/ and web-app/src/gen/. These paths are gitignored, but historically force-added (git add -f) as an inconsistent, undocumented practice -- confirmed no ADR or written policy exists for this. Every make target that consumes generated code (build/test/lint) already depends on proto-gen; committing generated output on top of that is redundant and creates exactly the drift risk that broke #433 (an incomplete/stale force-add went unnoticed). 2. Fix .github/workflows/lint.yml: the Jest step ran BEFORE the "Generate protobuf code" step. This only ever "worked" because generated files happened to be pre-committed -- once (1) removes that crutch, the step order bug becomes a hard failure instead of a latent one, so it must be fixed in the same commit. Moved protobuf/ent codegen before Jest, golangci-lint, the import-cycle check, and the feature-catalog validation step -- matching the correct order already used by .github/actions/prepare/action.yml and documented in build.yml's #144 comment. 3. Add .github/workflows/generated-proto-guard.yml: a hard CI backstop (matching backlog-scaffolding-guard.yml's established pattern) that fails any PR whose diff adds/modifies a file under gen/ or web-app/src/gen/, regardless of how it got there. This has happened more than once, including via AI-agent-driven commits that git add -f a generated file to make a local build pass -- this makes the mistake structurally impossible to merge instead of relying on review catching it. 4. Isolate the three session/import_commit_test.go tests that reach a real instance.Start() (PersistsAndLinksAndSuspends, CompensatingDeletesInstance, ReturnsError_When_AliveCheckerRejects) via the existing NewTmuxSessionWithServerSocket-style isolation (InstanceOptions.TmuxServerSocket, threaded through a new CommitImportParams.TmuxServerSocket field), reusing instance_cold_restore_test.go's coldRestoreSocket(t) helper. These tests previously hit the shared default tmux server, which is single-threaded and contends with every other test doing real tmux operations under CI's full parallel -race suite -- TestCommitImportExternalSession_PersistsAndLinksAndSuspends_ When_StartAndSuspendSucceed's CI failure ("cold start: tmux dead" -> 10s of failed tmux list-sessions calls -> timeout) is exactly this contention pattern, already documented as a known class of flake in this codebase's own session_service_program_test.go and server_integration_test.go comments. Verified: go build clean, full `session` package passes under -race (67s), all 4 previously-broken Jest suites pass (27/27), actionlint clean on both changed/new workflow files, `make proto-gen` regenerates all files identically from a clean state. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
tstapler
added a commit
that referenced
this pull request
Aug 12, 2026
…ng, isolate tmux-flaky import tests Three related fixes, all surfaced by the same incident (see this PR's first commit and #444's comments): 1. git rm --cached every currently-tracked file under gen/proto/go/ and web-app/src/gen/. These paths are gitignored, but historically force-added (git add -f) as an inconsistent, undocumented practice -- confirmed no ADR or written policy exists for this. Every make target that consumes generated code (build/test/lint) already depends on proto-gen; committing generated output on top of that is redundant and creates exactly the drift risk that broke #433 (an incomplete/stale force-add went unnoticed). 2. Fix .github/workflows/lint.yml: the Jest step ran BEFORE the "Generate protobuf code" step. This only ever "worked" because generated files happened to be pre-committed -- once (1) removes that crutch, the step order bug becomes a hard failure instead of a latent one, so it must be fixed in the same commit. Moved protobuf/ent codegen before Jest, golangci-lint, the import-cycle check, and the feature-catalog validation step -- matching the correct order already used by .github/actions/prepare/action.yml and documented in build.yml's #144 comment. 3. Add .github/workflows/generated-proto-guard.yml: a hard CI backstop (matching backlog-scaffolding-guard.yml's established pattern) that fails any PR whose diff adds/modifies a file under gen/ or web-app/src/gen/, regardless of how it got there. This has happened more than once, including via AI-agent-driven commits that git add -f a generated file to make a local build pass -- this makes the mistake structurally impossible to merge instead of relying on review catching it. 4. Isolate the three session/import_commit_test.go tests that reach a real instance.Start() (PersistsAndLinksAndSuspends, CompensatingDeletesInstance, ReturnsError_When_AliveCheckerRejects) via the existing NewTmuxSessionWithServerSocket-style isolation (InstanceOptions.TmuxServerSocket, threaded through a new CommitImportParams.TmuxServerSocket field), reusing instance_cold_restore_test.go's coldRestoreSocket(t) helper. These tests previously hit the shared default tmux server, which is single-threaded and contends with every other test doing real tmux operations under CI's full parallel -race suite -- TestCommitImportExternalSession_PersistsAndLinksAndSuspends_ When_StartAndSuspendSucceed's CI failure ("cold start: tmux dead" -> 10s of failed tmux list-sessions calls -> timeout) is exactly this contention pattern, already documented as a known class of flake in this codebase's own session_service_program_test.go and server_integration_test.go comments. Verified: go build clean, full `session` package passes under -race (67s), all 4 previously-broken Jest suites pass (27/27), actionlint clean on both changed/new workflow files, `make proto-gen` regenerates all files identically from a clean state. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx
tstapler
added a commit
that referenced
this pull request
Aug 13, 2026
…#445) * fix(ci): run buf generate proto before Jest in lint workflow Jest ran before proto generation, so a PR that forgets to commit its generated proto bindings (as #433 did) passes lint's Jest step locally against stale/missing gen/ output instead of failing fast at the same step that would have caught it — the exact gap that let main's Jest job go red without any single CI step surfacing "these files are missing." Reordering closes AC6 of backlog item 41cca909 (main branch CI red): buf generate proto now runs first, so any future gap between committed generated files and a fresh regen fails Jest module resolution in CI immediately rather than silently reaching main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): stop committing generated protobuf code, fix lint.yml ordering, isolate tmux-flaky import tests Three related fixes, all surfaced by the same incident (see this PR's first commit and #444's comments): 1. git rm --cached every currently-tracked file under gen/proto/go/ and web-app/src/gen/. These paths are gitignored, but historically force-added (git add -f) as an inconsistent, undocumented practice -- confirmed no ADR or written policy exists for this. Every make target that consumes generated code (build/test/lint) already depends on proto-gen; committing generated output on top of that is redundant and creates exactly the drift risk that broke #433 (an incomplete/stale force-add went unnoticed). 2. Fix .github/workflows/lint.yml: the Jest step ran BEFORE the "Generate protobuf code" step. This only ever "worked" because generated files happened to be pre-committed -- once (1) removes that crutch, the step order bug becomes a hard failure instead of a latent one, so it must be fixed in the same commit. Moved protobuf/ent codegen before Jest, golangci-lint, the import-cycle check, and the feature-catalog validation step -- matching the correct order already used by .github/actions/prepare/action.yml and documented in build.yml's #144 comment. 3. Add .github/workflows/generated-proto-guard.yml: a hard CI backstop (matching backlog-scaffolding-guard.yml's established pattern) that fails any PR whose diff adds/modifies a file under gen/ or web-app/src/gen/, regardless of how it got there. This has happened more than once, including via AI-agent-driven commits that git add -f a generated file to make a local build pass -- this makes the mistake structurally impossible to merge instead of relying on review catching it. 4. Isolate the three session/import_commit_test.go tests that reach a real instance.Start() (PersistsAndLinksAndSuspends, CompensatingDeletesInstance, ReturnsError_When_AliveCheckerRejects) via the existing NewTmuxSessionWithServerSocket-style isolation (InstanceOptions.TmuxServerSocket, threaded through a new CommitImportParams.TmuxServerSocket field), reusing instance_cold_restore_test.go's coldRestoreSocket(t) helper. These tests previously hit the shared default tmux server, which is single-threaded and contends with every other test doing real tmux operations under CI's full parallel -race suite -- TestCommitImportExternalSession_PersistsAndLinksAndSuspends_ When_StartAndSuspendSucceed's CI failure ("cold start: tmux dead" -> 10s of failed tmux list-sessions calls -> timeout) is exactly this contention pattern, already documented as a known class of flake in this codebase's own session_service_program_test.go and server_integration_test.go comments. Verified: go build clean, full `session` package passes under -race (67s), all 4 previously-broken Jest suites pass (27/27), actionlint clean on both changed/new workflow files, `make proto-gen` regenerates all files identically from a clean state. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(ci): widen tmux session-create timeout in CI test job (env-overridable) The socket-isolation fix (previous commit) removed cross-test tmux contention, but TestCommitImportExternalSession_PersistsAndLinksAndSuspends_ When_StartAndSuspendSucceed still failed deterministically twice in the same CI run (Makefile's coverage-then-verbose-rerun fallback) with the same "timed out waiting for tmux session" error -- a fresh, isolated tmux -L server still has to fork and become responsive within sessionCreateTimeout, and a fully CPU-saturated CI runner (every package's -race suite running concurrently) can push that past 10s on pure scheduling delay, independent of any lock/socket contention. sessionCreateTimeout is now a var, computed once from the optional STAPLER_SQUAD_TMUX_CREATE_TIMEOUT_SECONDS env var (unset/invalid -> unchanged 10s default -- zero production behavior change). Set to 30s in build.yml's "Run tests with coverage" step only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * debug(tmux): log raw tmux output on session-existence-check failure Diagnostic-only change for the still-unresolved TestCommitImportExternalSession CI flake (see PR #445 comments): DoesSessionExistNoCache's failure log only ever included the generic Go error ("exit status 1"), never tmux's own stderr text -- the one piece of evidence that would distinguish "server never came up" from some other failure mode. Also logs immediately after a successful `new-session` command, including its stderr, since the CI failure's wrapped err is <nil> (new-session itself reports success) while the subsequent list-sessions poll never finds it for the entire timeout window -- a pattern that already reproduced identically before this PR's socket-isolation and timeout changes, meaning neither of those addressed the actual root cause. This commit exists purely to get the missing evidence on the next CI run; expect a fast follow-up once the actual tmux error text is visible. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(tmux): set remain-on-exit before session creation to close a race with fast-exiting programs Root cause of TestCommitImportExternalSession_PersistsAndLinksAndSuspends_ When_StartAndSuspendSucceed's CI flake, found via the diagnostic logging added in the previous commit. CI evidence (PR #445): new-session command succeeded session=... serverSocket=test_coldrestore_... DoesSessionExistNoCache: ... sessions=[""] (not visible yet) DoesSessionExistNoCache: ... output="no server running on <socket>" [repeats for the entire poll window -- never recovers] `new-session -d` reports success, but every subsequent list-sessions call says the SERVER isn't running at all -- not "session not found yet", the whole server is gone. This test's candidate uses Program: "true", which exits in microseconds. t.setRemainOnExit() (which prevents tmux's default behavior of destroying a pane/window/session when its program exits) was only ever called AFTER Start()'s poll loop confirms the session exists -- but "true" can already have exited and torn down the session (and, since it was the server's only session on a freshly-isolated socket, the server itself) before that point is ever reached. This race is always present, not new to this PR's socket-isolation work: it likely also explains the identical failure signature seen pre-isolation, on the shared default socket's first-ever invocation. Fix: set remain-on-exit as a server-wide default (`set-option -g`) BEFORE the new-session command, not after. This command's own invocation safely spins up a sessionless server if none exists for this socket yet (a zero- session server doesn't exit-on-empty until it's HAD a session), so a server that has to be freshly created for this call already has the option active from its very first session -- before any program, however fast, gets a chance to run. Best-effort (log + continue on failure), matching every other non-essential tmux option set in this function. No behavior change for the common case: every session already ends up with remain-on-exit on via the existing (unchanged) later call -- this only moves that same eventual state earlier, closing the window rather than changing the outcome. Verified: go build clean; session/tmux and session packages pass under -race (45s / 62s); 10 consecutive runs of the previously-flaky test pass under `taskset -c 0 GOMAXPROCS=1` (single-core constrained, closer to a GitHub Actions runner's profile than this sandbox's 24 cores). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(tmux): chain start-server + exit-empty off + remain-on-exit into one invocation Previous commit's fix (separate set-option -g remain-on-exit before new-session) did not actually work -- confirmed by re-running in CI, which failed identically, plus a new log line proving why: the set-option command itself failed with "error connecting to <socket>" on a brand-new socket, because set-option (unlike new-session) does not implicitly start a server. Root cause, now fully nailed down empirically (manual `tmux -L <socket> start-server` / `set-option` reproduction, not guesswork): a tmux server that reaches zero sessions exits almost instantly by default (the `exit-empty` option defaults to on). `start-server` alone succeeds, but the server it starts is already gone by the time ANY subsequent, separate tmux invocation connects to check or configure it -- there is no window to run a second command against the same server unless it's chained into the SAME invocation. Fix: `tmux -L <socket> start-server \; set-option -g exit-empty off \; set-option -g remain-on-exit on` as one chained command (tmux's own `;` command-separator syntax, parsed by tmux from distinct argv elements -- no shell is involved via exec.Cmd, so no escaping needed). This keeps the server alive through its own zero-session startup window (exit-empty off) AND protects the session new-session is about to create from being destroyed the instant a fast-exiting program (e.g. Program="true") exits (remain-on-exit on) -- both options are active before any program ever gets a chance to run. Verified empirically before touching the test suite: manually reproduced the exact failure (start-server succeeds, very next `set-option` invocation reports "no server running") and the exact fix (chained invocation keeps the session visible after a `true` program exits) via raw tmux invocations, matching buildTmuxCommand's plain-argv (no-shell) construction exactly. Verified in Go: TestCommitImportExternalSession_PersistsAndLinksAndSuspends_ When_StartAndSuspendSucceed now completes in ~0.08-0.1s (down from 1.66s) -- it hits the fast existence-check path immediately after creation instead of falling through the 5-retry poll loop, meaning the session is now reliably visible right away, not eventually. 20/20 consecutive runs pass under `taskset -c 0 GOMAXPROCS=1 -count=20`. Full session and session/tmux packages pass under -race (61s / 45s). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * refactor(tmux): extract server preconfigure into its own function t.start() was already at cognitive complexity 48 (over the gocognit threshold of 40) on main before this branch's tmux race-condition fix added ~1 more point. Pulling the new start-server/set-option logic into preconfigureServerBeforeSession() keeps start() at its pre-existing 48 (verified via gocognit) instead of nudging it to 49, so the lint complexity gate doesn't flag this PR's diff for pre-existing debt it didn't introduce. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RetjyvJVU24VieQ3GUMJqm --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…-backlog-bounce-escalation # Conflicts: # gen/proto/go/session/v1/backlog.pb.go # gen/proto/go/session/v1/events.pb.go # gen/proto/go/session/v1/github_user.pb.go # gen/proto/go/session/v1/headless.pb.go # gen/proto/go/session/v1/insights.pb.go # gen/proto/go/session/v1/session.pb.go # gen/proto/go/session/v1/session_summary.pb.go # gen/proto/go/session/v1/types.pb.go # gen/proto/go/session/v1/unfinished.pb.go # proto/session/v1/backlog.proto # server/services/backlog_service_stuck.go # server/services/backlog_stuck_rpc_test.go # session/domain/backlog.go # session/domain/backlog_test.go # web-app/src/components/backlog-stuck/stuckReason.css.ts # web-app/src/components/backlog-stuck/stuckReason.ts # web-app/src/gen/session/v1/backlog_pb.ts
Contributor
✅ Registry ValidationTest Coverage: 46/193 features have
|
Contributor
📊 Feature E2E CoverageFeature coverage report unavailable
|
…ity below gate golangci-lint's gocognit gate (limit 40, only-new-issues) flagged reconcileMultiReasonEscalation at complexity 54 on PR #444's CI run (https://github.com/tstapler/stapler-squad/actions/runs/31664698295/job/94336632619). Split the per-item decision tree into categorizeOpenStuckRows, excludeStructurallyCoupledAbandonedReview, deescalateMultiReasonIfNeeded, notifyMultiReasonEscalationIfReady, and reconcileMultiReasonEscalationForItem so each piece is independently readable; behavior is unchanged (all 6 TestReconcileMultiReasonEscalation_* cases and the rest of ./session/ pass). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZFcZyivXenBipwthBDfdE
Contributor
✅ Registry ValidationTest Coverage: 48/194 features have
|
# Conflicts: # session/backlog_lifecycle_stuck_test.go
Contributor
✅ Registry ValidationTest Coverage: 49/196 features have
|
tstapler
added a commit
that referenced
this pull request
Aug 16, 2026
…tuck items (#444) * chore(sdd): planning artifacts for backlog-bounce-escalation Requirements, research, plan.md, and ADR-001/ADR-002 for the multi-reason stuck-state escalation and capped-while-bouncing durable marker feature. * chore(sdd): planning artifacts for backlog-bounce-escalation * feat(backlog): add multiple_reasons/bounce_cap_exhausted StuckReason plumbing Epic 1.1 of backlog-bounce-escalation: two new synthetic, aggregate StuckReason values (multiple_reasons, bounce_cap_exhausted) made valid and round-trippable end to end — Go domain type, proto enum, RPC mapping — ahead of the detector logic (Epic 1.2/1.3) that will set/resolve them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * feat(backlog): surface multi-reason/bounce-cap-exhausted stuck reasons in UI Epic 2.1 of backlog-bounce-escalation: adds labels/icons/GROUP_ORDER entries and a distinct chipEscalated style for the two new synthetic StuckReason values (MULTIPLE_REASONS, BOUNCE_CAP_EXHAUSTED) so they render instead of falling back to "Unknown reason"; excludes them from the otherReasonsCount badge so an item's own escalation row doesn't inflate its own badge; and extends the existing resolved-ghost mechanism to show a de-escalation banner when a multiple_reasons row resolves while the item stays open elsewhere. Also fixes a latent gap in the existing justResolved ghost mechanism: grouped items were derived only from the current items list, so a resolved item's ghost card (added to resolvedGhosts) could never actually render since it had already dropped out of `items` — visibleItems now re-includes still-tracked ghost items until their fade timer clears them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * feat(backlog): add multi-reason escalation detector (Signal 1) Adds reconcileMultiReasonEscalation, registered after self_heal in ReconcileStuck: marks a durable multiple_reasons row for items with 2+ simultaneously open non-escalation stuck reasons, dwell-gated notify-once, and resolves it once the count drops back below threshold. Excludes abandoned_review from the count when it's structurally coupled to a gate-blocked bouncing row, per ADR-001. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * feat(backlog): add capped-while-bouncing escalation marker (Signal 2) Epic 1.3 of the backlog-bounce-escalation plan: when the bouncing remediation gate parks (justParked) while bouncing itself is still open, mark a durable StuckReasonBounceCapExhausted row and upgrade the park notification from generic WARNING/HIGH to ERROR/URGENT framing naming the retry loop specifically — durable evidence the retry loop isn't converging, not just an ordinary single-reason park. - autoReopenWithBackoffGate now takes the item's actual itemStatus (both call sites pass BacklogStatus(item.Status), not a hardcoded in_progress) so MarkStuck's expectedStatus precondition doesn't silently no-op for review-status items. - reconcileBouncingItems resolves bounce_cap_exhausted alongside bouncing at both resolve sites; selfHealStuck gets a mirrored backstop case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * chore(registry): update backlog-stuck registry entries for escalation signals Bumps testIds/lastModified on both per-feature files to cover the new multiple_reasons/bounce_cap_exhausted stuck-reason tests, and fixes a stale test-name reference (contain12Entries -> contain16Entries) found while updating. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(backlog): apply sdd:6-verify Layer 1+2 findings for bounce escalation - Gate Signal 2's ERROR/URGENT notify on MarkStuck's applied result, so a failed-closed expectedStatus precondition can't fire a notification with no durable row behind it (architecture review CONCERN). - Combine the identical bouncing/bounce_cap_exhausted selfHealStuck switch cases into one, and extract the repeated resolve-both-reasons pairing in reconcileBouncingItems into resolveBouncingAndCapExhausted (refactor review). - Evaluate the abandoned_review/bouncing coupling exclusion via the already-fetched bouncing row + evaluateRemediation directly, instead of RemediationBlocked's redundant FindOpenStuckStates re-query (Go idiom review). - Document why the frontend de-escalation banner only covers MULTIPLE_REASONS, not BOUNCE_CAP_EXHAUSTED (React review SUGGEST). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * chore(deps): record go.sum checksums surfaced by full-repo build go.mod unchanged; these are checksums for existing transitive dependencies (go-runewidth, tablewriter) that hadn't been recorded yet in this worktree until make build/make test ran full-repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(backlog): reduce reconcileMultiReasonEscalation cognitive complexity below gate golangci-lint's gocognit gate (limit 40, only-new-issues) flagged reconcileMultiReasonEscalation at complexity 54 on PR #444's CI run (https://github.com/tstapler/stapler-squad/actions/runs/31664698295/job/94336632619). Split the per-item decision tree into categorizeOpenStuckRows, excludeStructurallyCoupledAbandonedReview, deescalateMultiReasonIfNeeded, notifyMultiReasonEscalationIfReady, and reconcileMultiReasonEscalationForItem so each piece is independently readable; behavior is unchanged (all 6 TestReconcileMultiReasonEscalation_* cases and the rest of ./session/ pass). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZFcZyivXenBipwthBDfdE --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
tstapler
added a commit
that referenced
this pull request
Aug 16, 2026
…e cadence (#511) * fix(session): suppress duplicate autonomous nudges within cooldown Add exact/near-exact nudge deduplication and a WAIT directive so the autonomous driver stops re-steering on a fixed idle-settle cadence once the agent has already stated it's waiting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(autonomous-driver): make nudge-suppression wait cancellable and add missing test coverage Address 5 code-review findings on the nudge-dedup branch: 1. Replace the bare time.Sleep(nudgeCooldown) in run()'s suppression branch with a ctx-aware select, matching waitForPaneSettle/waitForRateLimitClear, so Stop() can cancel the wait near-immediately instead of blocking up to the full 3-minute cooldown. 2. Add an integration test that drives a WAIT directive into the suppression branch and asserts Stop() returns the run loop promptly, not after the full cooldown. 3. Add a unit test for parseOrchestrationResponse's WAIT case, matching the existing per-directive test style. 4. Extract nextLastNudge (pure function) so the "lastSentNudge only updates after both SendKeys calls succeed" invariant is directly unit-testable, and add tests for both the failed- and succeeded-delivery paths. 5. Escape "<"/">" in lastNudgeText before interpolating it into the <last_nudge> prompt block, since it's the system's own prior LLM output being round-tripped and could otherwise close the tag early. * fix(autonomous-driver): suppress duplicate autonomous nudges within cooldown Add nudge dedup (session/nudge_dedup.go, already present) wiring into the driver's send path, plus a nil-pointer fix in the test double: Instance.TmuxAlive() calls i.pm().HasSession() unconditionally, but fakeSendKeysProcessManager embedded a nil ProcessManager without stubbing HasSession(). Added a stub so TmuxAlive() short-circuits cleanly instead of panicking. Known gap: TestAutonomousDriver_run_should_suppressSend_When_NextMessageMatchesLastNudge still fails via a timeout, root cause unresolved — flagged in review request. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(autonomous-driver): re-steer on idle-settle cadence, not a fixed cooldown timer When a nudge was suppressed as a duplicate, the driver blocked on the full nudgeCooldown (3min) fixed timer before re-polling, even though the agent had already signaled it was waiting. Replace that with the same waitForIdle mechanism a real send uses (bounded by a 5-minute timeout), so a suppressed turn re-polls as soon as the session settles instead of stalling on an arbitrary fixed cadence. Verified via go build ./..., go vet ./session/..., golangci-lint run ./session/..., and go test ./session/... -run 'AutonomousDriver|Nudge|Idle' (all pass). Full suite (go test ./... -count=1) is otherwise green except a known unrelated hang in session/unfinished/gogitstore, filed separately as backlog item 0d406bcc-a5a8-4af7-a388-8512584330da. * fix(autonomous-driver): re-arm duplicate-nudge suppression on new pane output isDuplicateNudge previously suppressed an identical repeat nudge for the full nudgeCooldown window regardless of what happened on the pane in the meantime. That could wrongly swallow a legitimately repeated instruction if the agent produced new output and genuinely needed to hear the same message again before the cooldown expired. Thread the current pane tail into isDuplicateNudge/nextLastNudge so the guard re-arms immediately once the pane differs (after whitespace-only normalization) from the snapshot recorded when the last nudge was delivered, bypassing the cooldown in that case. Verified via go build ./..., go test ./session/... -run 'TestIsDuplicateNudge|TestNextLastNudge|TestNormalizeNudgeText|TestAutonomousDriver' (24 passed), and go test ./session/... (3866 passed, 1 failed — the already-filed gogitstore hang, backlog item 0d406bcc-a5a8-4af7-a388-8512584330da — unrelated to this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(e2e): resolve ambiguous omnibar submit-button locator (#415) * feat(session): add markdown notes to sessions Adds a single free-form markdown `note` field to Session, editable from a new NotePanel in the session detail view's Info tab, rendered as markdown (react-markdown + remark-gfm, headings remapped to non-heading elements to avoid a11y heading-order violations), with a SessionCard badge indicator. Persisted as a column on the ent Session row (note is set unconditionally on Update, unlike sibling guarded fields, so clearing it actually persists). Closes the session-notes backlog item's 4 acceptance criteria; see project_plans/session-notes/ for requirements/research/plan/validation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Td8sMJxNvWwz9KiwYcyaTF * fix(session-notes): close sdd:6-verify findings on session notes Fixes found by the sdd:6-verify review-agent pass on the session-notes feature (Go idiom, React/CSS idiom, architecture, and refactor reviewers run in parallel): - NotePanel: add a live, byte-accurate character count and a client-side save guard that blocks saves exceeding session.MaxNoteLength before hitting the server, closing the UTF-16-char vs UTF-8-byte cap mismatch flagged in the plan's own adversarial review (multi-byte text like CJK could pass the textarea's char-based maxLength yet still be rejected server-side with no client-side warning). - NotePanel: restore focus to the Edit/Add-note button after Save or Cancel exits edit mode, per design/ux.md Surface 3 AC11 (focus must never fall back to <body>). - NotePanel: add `open` to the <details> element — design/ux.md AC2 requires the panel to default open (1-click view), but it was rendering collapsed. - SessionCard: drop the redundant Radix Tooltip wrapper on the note badge — it duplicated the native `title` tooltip, showing two overlapping tooltips on hover. Matches this file's other simple badges (workflowBadge, pendingProgramChange), which use `title` alone. - ent schema: fix a stale comment pointing at a nonexistent NOTE_MAX_LENGTH symbol in the wrong file, and correct "chars" to "bytes" to match the actual byte-based validation. Regenerated ent. - tests/e2e/session-notes.spec.ts: fix three locators that never matched/were ambiguous against the current UI (stale "one-off" radio label vs. actual "Temporary (no git)"; ambiguous getByLabel("Program") matching 10 elements including seeded session rows). A fourth, unrelated ambiguous-locator issue in the shared New Session flow is filed separately as backlog item 1fe5aabb. All Go and frontend tests pass (full suites), build/lint/lint-css-tokens/ registry-diff all green. Two pre-existing -race-detector test flakes (unrelated to this diff) filed as backlog item e271db3d. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NARaAW9E5J8GV3wP6e59XT * fix(session-notes): fix UpdateSession note-validation ordering bug Found by the code:review Gate 2 pass (testing-quality and architecture agents independently converged on this): the note-length check in UpdateSession ran after Title/Category mutations had already applied to live in-memory Instance state (SetTitleDirect/SetCategory publish synchronously via snapshot.Store, not staged until SaveInstances). A combined request like {Title: "new", Note: <10001 bytes>} would return InvalidArgument while the title had already changed in memory and been broadcast to concurrent readers (e.g. WatchSessions), with no persistence to storage — a rejected request partially applying itself. Fixed by hoisting the note-length validation to run before any mutating Set* call. Added 3 regression tests: - TestUpdateSession_NoteExceedsMaxLength_LeavesOtherFieldsUnmutated: proves a combined Title+oversized-Note request leaves Title unmutated in the live poller instance, not just unwritten to storage (storage alone can't catch this bug, since SaveInstances never runs on this error path either way). - TestUpdateSession_NoteLengthValidation_IsByteAccurate: the existing length test used ASCII only, which can't distinguish byte-length from rune-count validation. Uses a multi-byte string under the rune-count cap but over the byte cap to prove the check is actually byte-accurate. - TestUpdateSession_UnrelatedFieldUpdate_PreservesExistingNote: guards the interaction between the RPC handler's conditional Note mutation and ent_repository.go's deliberately unconditional SetNote on every Update call — an unrelated field update must not clobber an existing note. Also hoists a NotePanel.tsx TextEncoder allocation to module scope (was allocated fresh on every render/keystroke). A pre-existing, unrelated MAJOR finding from the database-review agent (UpdateSession rewrites every live session's row via SaveInstances on any single-field edit) is filed as backlog item 085dcac1, not fixed here — touches a shared persistence path other work depends on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NARaAW9E5J8GV3wP6e59XT * fix(session): close data race on Instance.Status read in reconcileSessions reconcileSessions() read inst.Status directly from the poller goroutine with no synchronization against transitionToLocked's write, caught by -race as TestServer_should_WriteUnchangedHookURL_When_StartedOnExplicitPort (tracked as a "pre-existing flake" in issue #271, but actually a real bug). Reads outside an in-flight actor command now go through GetStatus()'s lock-free published snapshot, the pattern the codebase already documents as the correct one. Reads inside the three sendCtx closures can't use that snapshot (it's stale until the in-flight command returns), so they fall back to the same i.mu.RLock()-guarded read transitionToLocked uses for itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NARaAW9E5J8GV3wP6e59XT * fix(session-notes): note updates weren't reflected in the UI, and the default list view had no note badge at all Writing a real e2e test for session-notes.spec.ts surfaced two bugs the previous unit/component tests couldn't catch: 1. setNoteLocked mutated Instance.Note without bumping UpdatedAt. The frontend's upsertSession reducer (sessionsSlice.ts) skips applying an incoming session as a no-op-dedup optimization whenever its updatedAt matches the stored value — so on any session whose UpdatedAt hadn't otherwise moved (e.g. freshly created and still idle), a note save succeeded server-side but never appeared in the UI. Fixed by bumping UpdatedAt in setNoteLocked; added TestUpdateSession_NoteUpdate_BumpsUpdatedAt as a regression guard (confirmed it fails without the fix). 2. AC3 ("SessionCard shows a visual indicator") was only ever implemented in SessionCard.tsx (grid view). The app's actual default list view renders via a separate SessionRow.tsx component that never got the note-badge treatment at all. Added the same badge there, with its own Jest coverage and a shared testid so the e2e test can find it in either view density. SessionsPage.ts's getSessionCard/getSessionCards helper also only matched the grid view's testid — updated to match both. Also fixes an unrelated ambiguous-locator bug in the new e2e spec itself (the "Create Session" submit button matched 4 elements via a broad /create|start/i regex) and pre-seeds the first-visit onboarding dialog as dismissed, following the existing ci-status-badge.spec.ts pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KHqrogw1sKMCUQCGsi4TrQ * fix(session-notes): NotePanel's save-error UI could never fire, and SessionCard's note tooltip diverged from the plan spec sdd:6-verify's parallel review agents (React idioms + architecture) found two real issues in the note feature's shipped code: 1. MUST FIX: useSessionService.updateSession never rejects on RPC failure — it catches internally, dispatches a global setError, and resolves to null. NotePanel's save-error UI (aria-live assertive message, textarea preserved) is entirely gated on the onSave promise rejecting, so a real backend failure would silently exit edit mode as if the save succeeded. Fixed the SessionDetailView onSave wrapper to check the resolved value and throw when null. Added SessionDetailView.note-error.test.tsx (mirrors SessionDetailView.summary-tab.test.tsx's mock harness) proving the error path now fires on a null result and stays silent on success. 2. CONCERN: SessionCard's note badge used a native `title` attribute for its tooltip while the plan (validation.md) specified the shared `Tooltip` component — the same one SessionCard already uses elsewhere and the one used in SessionRow's badge (added in the prior commit). Switched SessionCard to `<Tooltip>` to match; updated its test to mock Tooltip (the same pattern this file already uses for Modal) since Radix's real tooltip only reveals its label on a delayed hover via a portal, not a static DOM attribute. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KHqrogw1sKMCUQCGsi4TrQ * fix(e2e): resolve ambiguous omnibar submit-button locator session-notes.spec.ts and session-completion-summary.spec.ts used getByRole("button", { name: /create|start/i }), which strict-mode matches 4 elements in the omnibar (the "+" FAB, the mode-toggle badge, and two functionally-identical "Create Session" submit buttons). Add data-testid="omnibar-create-session-button" to the canonical form-footer submit button in OmnibarCreationPanel.tsx and expose it via a new SessionsPage.createSessionSubmitButton page-object field, per e2e-test-conventions.md. Both specs now use the page-object locator. Also fixes two pre-existing bugs in session-completion-summary.spec.ts that were masking verification of the above (never previously reached since the ambiguous locator failed first): - missing onboarding-dialog dismiss init script, causing the tour overlay to intercept the submit click - the omnibar overlay had no scroll affordance, so the footer became unreachable once "Advanced Options" pushed the modal past the viewport height (Omnibar.css.ts: overlay gains overflowY: auto) Follow-ups filed for unrelated pre-existing debt found in the process: 7266baf1 (session-completion-summary's Summary tab never enables) and 3bac1d3f (filed for the overlay scroll bug before the fix above landed in this same PR — safe to close as already-fixed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K7tyqVXWEzJC4JhbU6Tox5 --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * feat(ssq-hooks): replace open-code proxy wrapper with native plugin hook (#431) * feat(ssq-hooks): replace open-code proxy wrapper with native plugin hook Replaces installOpenCode()'s bash-wrapper proxy with patchOpenCodeHooks(), which installs a plugin to ~/.config/opencode/plugins/ subscribing to @opencode-ai/plugin's tool.execute.before hook (confirmed live: a real throw-to-block, pre-execute hook the prior R4 research missed by only reading the SDK's static config types). check --opencode is a fourth thin input/output adapter around the existing classifier.Classify() engine, matching the Claude/Gemini/Antigravity adapter pattern. Escalate maps to deny (fail-closed), per ADR-027 — tool.execute.before has no ask/dialog fallback, so treating the classifier's catch-all as allow would silently weaken the policy. A full live end-to-end test (real binary, real opencode session, real classifier) caught and fixed a genuine bug: OpenCode's write/edit tool args use camelCase filePath, but the classifier's FilePattern rules match file_path (snake_case) — .env/.git write-protection rules were silently never matching. Fixed via normalizeOpenCodeToolInput() in the input adapter, with regression tests. Also: fixes 24 broken opencode agent/skill frontmatter files that blocked `opencode run` entirely in this dev environment (invalid tools: shape, null description), updates project_plans/antigravity-opencode-parity's stale R4/E7/ST-03 docs to cite the plugin API, and records live verification findings (plugin registration mechanism, subagent/batch-tool coverage) in project_plans/opencode-native-hooks/. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4Aozgai3gzQ8FkYkxcfNb * fix(ssq-hooks): address code review findings on OpenCode plugin adapter - Add a bounded timeout to the generated plugin's execFileSync call so a stall in ssq-hooks fails closed within a fixed window instead of hanging the OpenCode session indefinitely. - Embed the binary path into the generated JS via encoding/json instead of fmt's %q (Go/strconv escaping, not JS string-literal escaping — e.g. Go's \a has no JS equivalent). - Invert writeOpenCodeHookDecision's switch so AutoAllow is the only explicit non-deny case; cmd/ is excluded from golangci-lint's exhaustive check, so this is the only guard against a future classifier.ClassificationDecision value silently falling through to allow instead of failing closed per ADR-027. - Stub PATH in the installOpenCode tests so they no longer shell out to the real host `opencode` binary (was ~0.85s/test, host-dependent; now ~0.02s and hermetic). - Add a guarded `node --check` assertion on the generated plugin so a future template edit that breaks JS syntax fails CI, not just at runtime in a user's OpenCode session. - Collapse the three byte-identical runXDecisionSubprocess test helpers into one shared runDecisionSubprocess. - Add missing test coverage: AutoDeny without a RuleID (parity with the Gemini adapter's coverage), and a payload with no tool_input key at all (OpenCode's real zero-arg-tool shape, since JSON.stringify drops undefined-valued keys). Found via a 4-agent parallel review (testing/code-quality/architecture/ security) + adversarial skeptic pass on PR #431. Deferred as pre-existing, out-of-scope collateral debt (not fixed here): installOpenCode's 4th near-identical copy of the "copy binary to ~/.local/bin" block shared with installClaude/installGemini/installAgy, and the unbounded io.ReadAll(os.Stdin) in parseOpenCodePayload (identical to the pre-existing pattern in parseGeminiPayload) — both touch adapters unrelated to this PR's OpenCode scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4Aozgai3gzQ8FkYkxcfNb * fix(import-external-session): commit missing generated proto bindings PR #433 added proto/session/v1/import.proto and the Go/TS source code that consumes its generated types, but never force-added the generated output -- gen/ and web-app/src/gen/ are blanket-gitignored, and every other generated file in those trees is force-added to override that (git check-ignore confirms both new paths hit the blanket rule). This silently dropped the generated files from the PR, breaking every CI job that doesn't run a full proto regen before testing: - lint (Jest): 4 suites failed to run with "Could not locate module @/gen/session/v1/import_pb" (ImportPreviewDialog, ConfirmKillDialog, ImportSessionsContainer, ImportExternalSessionsPanel test files). - Go build succeeded only because `make build`'s proto-gen step regenerates these files on the fly before `go test` runs in CI; local/isolated Go tooling without that step would also fail. Ran `make proto-gen` and force-added the three resulting files (import.pb.go, import.connect.go, import_pb.ts). Verified: all 4 previously-failing Jest suites now pass (27/27), `tsc --noEmit` clean, `go build ./server/services/... ./gen/... ./session/...` clean. Note: two additional CI failures on main (TestCommitImportExternalSession_ PersistsAndLinksAndSuspends_When_StartAndSuspendSucceed and TestSearchClaudeHistory_DedupOversamplesBeforeTruncatingToRequestedLimit) do not reproduce locally, including under the full `session`/ `server/services` package suites with -race matching CI's exact invocation -- these look like genuine CI-environment-specific timing races (tmux session-creation timing; a cross-test temp-dir race unrelated to this fix's files) rather than something this commit's scope can fix. Tracked in backlog item 41cca909. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(ci): untrack generated proto files committed before guard policy gen/proto/go/session/v1/import.pb.go, its connect counterpart, and web-app/src/gen/session/v1/import_pb.ts were committed on this branch by ad72decae (merged in from main) to fix a build that was missing them. Main has since added a hard CI guard (generated-proto-guard.yml, no-checked-in-generated-protos) that fails any PR committing files under gen/ or web-app/src/gen/, since those paths are gitignored and every consuming target regenerates them fresh via buf generate proto. origin/main tracks zero files under either path. Untrack these three so this branch matches main's current policy; regenerated content is byte-identical to what was committed (verified via buf generate proto), so nothing behavioral changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZFcZyivXenBipwthBDfdE --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * feat(launcher-presets): pre-configured agent launch commands from a JSON config file (#451) * feat(launcher-presets): pre-configured agent launch commands from a JSON config file Adds ~/.stapler-squad/launcher-presets.json, a hand-edited file of named argv-based launch shortcuts surfaced as a one-click "Presets" section in the Omnibar creation panel, plus a preset:<id> typed shorthand. Selecting a preset prefills program/extraArgs/workingDir; the user still reviews and explicitly submits. Backend: GetLauncherPresets RPC reads and validates the file fresh on every call (no caching, so edits appear without a server restart); malformed files surface a load_error rather than crashing or silently dropping presets. A new extra_args carrier threads a preset's argv[1:] through CreateSessionRequest -> Instance -> buildLaunchCommand, composing after any profile/alias-resolved cli_flags. Security fix (pre-mortem P1): buildLaunchCommand's plainProgram branch now shell-quotes each whitespace-split token of Program independently (via a new shellQuoteFields helper, shared with the CLIFlags loop) rather than using it verbatim. This closes a command-injection path where a preset's argv[0] could contain shell metacharacters, while preserving legitimate multi-word Program values (e.g. "sleep 300") that rely on ordinary shell word-splitting -- verified empirically that whole-string quoting (as originally specified) would have silently broken that case. Frontend: useLauncherPresets/PresetDetector/OmnibarPresetList, fetched once in OmnibarContext and shared (not independently re-fetched) between the PresetDetector registration and the visible list. Known pre-existing, unrelated issues observed and left as-is: - OmnibarCreationPanel.attach.test.tsx: 6 image-upload tests fail identically with or without this change (confirmed via git stash). - session package: TestSessionRecoveryScenarios failed once in a full-suite run under tmux resource contention, passed cleanly on two subsequent full-suite reruns -- non-deterministic, unrelated to this diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGbGJbpnUTERfSUQZzFHu7 * fix(launcher-presets): expand Presets section when config load fails The Presets section defaulted to collapsed whenever presets.length === 0, which is also true when the backend reports a load_error (GetLauncherPresets returns zero presets alongside the error by design). That hid a malformed config's error behind a collapsed section, contradicting AC4's "fails loudly" requirement -- caught by review, confirmed via a real browser e2e test that this bug is invisible to jsdom/RTL (CSS-only collapse state is never enforced by jsdom, so a naive component test can't tell collapsed from expanded). Fix: auto-expand on either a loaded preset or a load_error, only a genuinely empty error-free state stays collapsed. Added aria-expanded to the section header as a semantic, testable state hook, and a Playwright test that verifies the error is visible immediately in a real browser without needing to click to expand. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGbGJbpnUTERfSUQZzFHu7 * fix(launcher-presets): validate empty argv elements, close test gaps Found by the code:review pass (4 parallel agents: testing, code quality, architecture, security) on PR #451 -- no BLOCKER/CRITICAL findings, but three real gaps: - validateLauncherPresets accepted argv containing blank/whitespace-only elements (e.g. ["", ""]), producing a broken launch command with a stray leading space and the actual program elided. Now rejects them the same way empty argv is rejected. - The unsupported-version validation branch had zero test coverage. - useLauncherPresets' stale-fetch guard (a refetch's response landing after an older in-flight fetch resolves) was completely untested -- verified the new test actually catches this by temporarily disabling the guard and confirming the test fails, then restoring it. Also added the missing Space-key activation test for OmnibarPresetList rows (Enter was already covered). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGbGJbpnUTERfSUQZzFHu7 * fix(registry): add GetLauncherPresets to scanner's methodToID map The launcher-presets feature added a new GetLauncherPresets RPC to session.proto and its registry entry (docs/registry/features/backend/ launcher-presets/get.json, id "launcher_presets:get") but never added the corresponding methodToID entry in tools/scanner/backend/ proto_scanner.go. TestMethodToIDCompleteness and TestScanProto_NoUnmappedMethods caught the gap in CI (Test job / "Run scanner unit tests" step). --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * feat(detection): detect context-compaction state from Claude Code output (#455) * fix(backlog): release triageInFlight before optional auto-spawn, not after TestTriggerTriage_RefineWithFeedback was flaky in CI: a second TriggerTriage call for the same item could get a spurious AlreadyExists even after the item's status had already flipped to Ready, because triageInFlight stayed held through the goroutine's post-completion work (auto-spawn's own I/O, final logging) instead of being released as soon as the triage result was actually persisted. A real user hitting "retry" right as an item shows Ready could hit the same rejection. Move UpdateItemSessionEnded + triageInFlight.Delete up to right after the status transition and persistence steps, before the optional auto-spawn call — keeping ended_at and triageInFlight cleared together (both feed the orphan-liveness check in IsTriageLive/tombstoneOrphanTriageSessions), just earlier than before. Verified: go test -race -count=10 on the affected tests, plus the full server/services suite, all green; golangci-lint clean. * fix(ci): install buf directly with retry instead of buf-setup-action bufbuild/buf-setup-action downloads its binary from GitHub Releases and only retries for ~30s total (2 retries, 15s/12s waits) before giving up. That wasn't enough during an observed buf.build/GitHub Releases blip today: "socket hang up" on the release asset CDN, reproduced across 4+ consecutive job failures over ~20 minutes (Build, Registry Validation, Benchmarks jobs) on main and on PR #453. buf also publishes the raw per-platform binary (no .tar.gz), so install it directly with scripts/retry-with-backoff.sh (-n 5 -s 10, ~150s+ budget) — the same convention already used by this repo's "Build web UI" step for its own external-network flake (next/font fetching from fonts.gstatic.com). Applied everywhere buf-setup-action was used: the shared .github/actions/prepare composite action (covers Lint, Benchmarks, Registry Validation, E2E video, demo-publish, MCP integration), build.yml's web-build-smoke job (a separate copy that intentionally skips prepare's pre-steps), and release.yml/release-please.yml. Verified the install script end-to-end locally (downloads, chmod, buf --version succeeds); actionlint clean on all four files. * fix(ci): cache buf binary to avoid the network entirely on cache hit Complements the previous commit's retry wrapper: cache $RUNNER_TEMP/buf-bin keyed on buf-1.41.0-${{ runner.os }}-${{ runner.arch }} before installing, skip the download when the cached binary is already present. Most runs now never touch GitHub Releases at all instead of just retrying it harder. * fix(session): use safeexec.CommandContext in diffhash test Direct os/exec.Command calls are blocked by the repo's custom lint rule (no WaitDelay, zombie-process risk). Found while running `make lint` for an unrelated change; fixed here per the fix-collateral-debt convention rather than deferred. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Er6ZcYUmePAShWfCqM9XnA * feat(detection): detect context-compaction state from Claude Code output Adds StatusCompacting as a first-class DetectedStatus/SubStatus, distinct from the existing "N% until auto-compact" approaching-threshold indicator, so the session card shows a dedicated "⟳ Compacting context" badge instead of leaving the card on the generic "Thinking…" chip for the full duration of an auto-compaction (previously indistinguishable, sometimes 30-60s). - session/detection/: new StatusCompacting enum value, Compacting pattern group checked before Active/Processing in PatternSet.MatchLines, wired through categoryName/mapStatusToIdleState/proto_mapping.go. - proto/session/v1/types.proto: append-only DETECTED_STATUS_COMPACTING and SUB_STATUS_COMPACTING enum values (both required per ADR-001 — SubStatus drives the session-card badge precedence rule). - server/adapters/{instance_adapter,review_queue_adapter}.go: map StatusCompacting to SUB_STATUS_COMPACTING, guarded by a mandatory table-driven parity test across every DetectedStatus value. - session/review_queue_determiner.go: the no-controller status switch also needed StatusCompacting, or a compacting session with no live controller could fall through to the idle-timeout fallback and get spuriously queued as ReasonIdle (caught in review, regression test added). - web-app: deriveWorkingState.ts, StatusBadge.tsx, and SubStatusChip.tsx/ .css.ts get the new case; SubStatusChip renders the badge with role=status/aria-label/title, matching chipWaitingForAgent's calm, non-alarm token pair per the UX spec. - The compacting_conversation regex is INFERRED, not verified against a live Claude Code capture — no live compaction was reachable in this session's tmux scrollback. A temporary bake-in canary (detector.go, fires at most once per detector, never logs raw PTY text) flags a near-miss once real usage starts. Follow-up backlog item filed: 9637b022-a3d7-4639-ba37-fe2842e5d6dc. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Er6ZcYUmePAShWfCqM9XnA --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(backlog): close reconcileBouncingItems' direct in_progress->done gate bypass reconcileBouncingItems called the raw storage-layer TransitionBacklogItemStatus straight from item.Status (which can be in_progress) to done, using the item's current status as the CAS precondition. That raw layer has no knowledge of WorkflowEngine or TransitionGuard, so this silently skipped two checks the guarded front-door RPC path always enforces: in_progress->done isn't even a legal edge in validTransitions (only review->done and pr_pending->done reach done), and TransitionGuard's ErrVerdictRequired gate, which normally requires a recorded PASS verdict before an item can be marked done. Add transitionBouncingItemToDone: it records a genuine PASS verdict via recordTerminalReviewVerdict (documenting the external verification — merged PR or commit shipped to main — as the justification), then walks the item to done via the state machine's own legal edges (in_progress->review, then review->done) instead of the illegal direct hop. Both call sites in reconcileBouncingItems now use it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(makefile): match service-installed stapler-squad in restart pkill restart-web/web-dev killed existing instances with pkill -f "^\./stapler-squad", anchored to a relative-path invocation. When stapler-squad is installed as a system service (install-service/ install-service-profile), scripts/install-service.sh embeds an absolute STAPLER_SQUAD_BIN path directly into ExecStart (systemd) and ProgramArguments (launchd), so a service-managed process's argv never starts with "./" and the old pattern could never match it - leaving a service-managed instance running alongside a freshly restarted dev one. New pattern (^|/)stapler-squad([[:space:]]|$) matches relative, absolute, and bare-PATH invocations of the binary while still excluding the distinct stapler-squad-cov coverage binary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(worktree): stop silently fabricating disconnected repos on missing paths findGitRepoRoot silently created a brand-new, disconnected git repo with a fake "Initial commit" whenever repoPath didn't exist, instead of erroring — turning a transient/missing-path bug into a worktree built from history that shares nothing with the real repo. Now errors immediately when the path is missing, while leaving the narrower "repo exists but has zero commits" initial-commit fallback untouched. Also closes a related gap: CreateBacklogWorktree resolves a backlog item's stored RepoPath and hands it straight to the worktree constructors, bypassing EnsureRepoCloned's interrupted-clone detection entirely. Added isCorruptedClone/RepairCorruptedGitRepo (shared by both EnsureRepoCloned and CreateBacklogWorktree) to detect and re-clone a repo left with an unresolvable HEAD by a killed `git clone` subprocess. * test(worktree): add RepairCorruptedGitRepo regression coverage Covers the repair path, the healthy-repo no-op, and the not-a-git-repo no-op for CreateBacklogWorktree's corruption-repair gap fixed in d24ae6049. Also files BUG-069 for an unrelated gogitstore test hang discovered while verifying this via `go test ./session/...`. * feat(backlog): persist full raw output when a headless triage/review call fails (#328) Investigating stuck triage item be676dab (an 8h52m headless triage session with no usable result) showed there was no way to recover what the LLM actually returned: ParseHeadlessTriageResult's parse-failure log line only includes a ~200-byte preview, and the log file itself rotates out of ~/.stapler-squad/logs/ within a few hours — by the time an operator investigates, both the full output and the log line describing it are gone. Nothing durable/queryable in the DB recorded a call failure's raw output either; end_reason (classifyHeadlessCallError's bucket) already existed but was never even surfaced over the wire. Root cause: TriggerTriage's callErr and parseErr branches (server/services/backlog_service_triage.go) discarded `raw` entirely on failure, and TriggerReReview's real callErr branch didn't even create an ItemSession row to record anything against. Fix, reusing this repo's existing scrollback/transcript-file precedent (session/review_transcript.go's WriteReviewTranscriptFile) rather than a new storage mechanism: - session.WriteHeadlessFailureCapture writes the size-capped (256KB, tail-kept) raw output to a durable file under a new config dir (~/.stapler-squad/headless-failures/, config.HeadlessFailureCaptureDirOrDefault), deliberately NOT inside the existing per-item triage-artifacts dir (which readPlanFile feeds into later review/triage prompts — writing there would leak raw failure text into future LLM context). - New ItemSession.failure_capture_path column (ent schema + migration) references the file; a new orthogonal Update method sets it alongside the existing end_reason. TriggerTriage wires this into both its callErr and parseErr branches; TriggerReReview's callErr branch now also creates a best-effort audit ItemSession row (previously nothing was persisted there at all) with the same capture + classified end_reason. - classifyHeadlessCallError generalized to take an explicit call budget (was hardcoded to triageCallBudget) so TriggerReReview can share it. - end_reason and failure_capture_path added to the ItemSession proto message and threaded through to the frontend; BlockedNotice now renders the classified failure reason + capture path instead of an unexplained "No diagnostic data recorded." for a failed headless call. Separately investigated (per the same brief) whether the zombie-reviewer distinction (list_workspace_peers' status=Active/lifecycle=gone) that PR #320 fixed is persisted anywhere durable: confirmed it already is — reconcileStuckReviewItems' zombie-session detection writes a DB-backed BacklogStuckState row (survives restart, queryable via FindOpenStuckStates), not just an in-memory/log signal. No gap there; no change made. Regression tests: session/headless_failure_capture_test.go (write/truncate/ no-op behavior) plus three end-to-end tests exercising the real TriggerTriage/ TriggerReReview goroutines through a fake headless pool, asserting the full raw output survives to a durable, DB-referenced file after both a parse failure and a call error. make build && make lint: clean. go test ./session/... ./config/... ./server/services/...: all green. Full `make test`/frontend jest hit two failures under parallel load — the documented pre-existing session/tmux flake (TestEnsureServerRunning_NoOp, PR #320 precedent) and an unrelated CAS-racer test (TestReportDuplicate_ReportsDistinctMessage_WhenCASPreconditionFails); both pass reliably in isolation (confirmed 5/5). Two frontend suites (SessionDetail.embedded.test.tsx, BacklogEmptyState.test.tsx) also fail in isolation on components this change never touches — pre-existing on this branch, unrelated to this diff (3701 tests / 269 suites green otherwise). Claude-Session: https://claude.ai/code/session_01W3683CH7Fs9zYR2yP3Dpba Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): gate PR comments on actionable findings (#462) * chore(sdd): planning artifacts for pr-comment-check-runs Requirements, research (stack/features/architecture/pitfalls/ux/build-vs-buy), implementation plan, and two ADRs (Commit Status API vs Check Runs API; hybrid doc-convention + Go primitive + script enforcement) for the comment-noise-reduction feature. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(sdd): planning artifacts for ci-workflow-comment-gating * feat(ci): add reusable comment-gate decision modules with tests Pure, unit-tested decision logic (hasRegression, isActionable, videoAnomaly) for the CI-comment-gating work in the next commits — kept separate from the workflow YAML so the actual branching logic has real regression coverage instead of only being exercisable via a live scratch-PR push. Includes a workflow-YAML invariant test (AC #7) that parses the real .github/workflows/*.yml files and asserts the existing blocking checks (Axe, RPC-test-coverage, registry-validation) are untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs * feat(ci): gate benchmark.yml's 3 PR comment jobs on real regressions benchmark.yml's go-tier1, frontend-throughput, and e2e-latency jobs posted an advisory comment on every PR touching benchmarked code, regardless of whether anything regressed. Each now only posts/updates when its gate module reports a real regression, and deletes a stale "regression detected" comment once a later push clears it. go-tier1 uses a 20% threshold against benchstat's own significance test ("~" for insignificant deltas); frontend-throughput/e2e-latency use a coarser 2x-swing threshold since they're single-sample Playwright measurements with no significance test to filter noise. While wiring this up, found and worked around a pre-existing bug: the pinned benchstat version (golang.org/x/perf's table-format rewrite) dropped the `-delta-test` flag entirely, so build.yml's benchmark-gate job has been silently failing to parse its own comparison output on every run (confirmed live: run 31634486527's logs show "flag provided but not defined: -delta-test", masked by the step's missing `pipefail`). This commit does not add that flag to benchmark.yml, and does not touch build.yml's benchmark-gate job — that's a separate, already-broken blocking check on a different workflow, out of this item's scope; filing a follow-up issue for it separately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs * feat(ci): gate ux-analysis.yml PR comment on actionable findings The UX analysis comment posted an always-present status table on every PR, even when Axe passed, Lighthouse scored well, and Claude UX analysis found nothing. Now only posts/updates when Axe fails, Lighthouse drops below 70 or fails to measure (explicit isNaN check — NaN < 70 is false in JS, so a bare comparison would silently treat a Lighthouse crash as a pass), or Claude UX analysis reports >=1 finding; deletes a stale comment once everything's green. Also emits findings_count from analyze.ts so the gate has a real signal instead of the previous static "Advisory" row. Also pins actions/github-script to the SHA benchmark.yml already uses (drive-by consistency fix, folded in here rather than a separate commit since it touches the same line the gate restructuring does) and adds a concurrency block scoped to the PR number so an out-of-order run can't delete a comment a newer run just posted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs * feat(ci): gate build.yml feature-coverage comment on real deltas The feature-coverage comment updated on every PR even when the percentage hadn't moved. Now parses the previous percentage from a dedicated marker (<!-- feature-coverage:pct=NN.N -->, owned exclusively by this script rather than scraped from the free-form summary sentence) and only posts/updates when it actually changed — an unchanged value is left untouched, not deleted, since it's the steady state rather than a resolved problem. build.yml also runs on push-to-main with 5 unrelated jobs, so it can't take a workflow-level concurrency block without cancelling those too; instead adds an in-script recency guard that skips the mutation if a newer push has already superseded the commit this run was triggered for. Also wires the new tools/ci-gates unit tests into this job so they actually run in CI, and pins actions/github-script to the same SHA as the other workflows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs * feat(ci): gate e2e-video.yml notify comment on artifact anomalies The notify job posted a comment on every PR that recorded feature videos, whether or not anything went wrong — pure navigation content, never itself an action item. Now only posts when zero video artifacts were produced (a genuine anomaly); the normal-case demo-preview and recording links move to the step summary instead of disappearing entirely. Deletes a stale anomaly comment once a later push produces artifacts again. Adds a concurrency block scoped to the PR number (same shape as ux-analysis.yml) so an out-of-order run can't delete a comment a newer run just posted, and pins actions/github-script to the same SHA as the other workflows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs * fix(ci): checkout in e2e-video's notify job, dead gate code, untested coverage-pct parse sdd:6-verify's review agents (idiom + architecture, run independently) both caught the same real bug: e2e-video.yml's notify job never had an actions/checkout step, so the require('./tools/ci-gates/video-anomaly.js') added in the prior commit threw MODULE_NOT_FOUND on every run — confirmed live in PR #462's run logs (94327489537), silently swallowed by the step's own continue-on-error: true. The comment-gating logic for this workflow has never actually executed. Fixed by adding the missing checkout. Also, per the same review: deleted gateAction() from benchmark-regression.js and coverage-delta.js — both were unit-tested but never called by any workflow (each workflow re-implements the same post/delete/noop branching inline), so the tests exercised code that wasn't the code actually running in CI. Extracted build.yml's untested bash PCT regex scrape into coverage-delta.js's parseCoveragePct (now unit-tested), and added the end-to-end malformed-marker isActionable test plan.md's Task 3.1.1e called for but was missing. Deduplicated hasThroughputRegression/hasLatencyRegression into a shared crossesThreshold helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs * fix(ci): run coverage report after registry-generate, skip comment on unparseable pct Reviewer verdict on the backlog item caught a real gap: PR #462's own live run posted a "coverage report unavailable" comment on an otherwise fully green PR, contradicting AC #6. Root cause (confirmed via run 31662399128's logs, "Registry not found, skipping coverage report"): build.yml's "Generate feature E2E coverage report" step ran before "Check new RPCs have tests" — the step that actually runs `make registry-generate` and produces the gitignored docs/registry/backend-features.json the coverage tool reads. This predates this item's gating work entirely (feature-coverage.ts has always run against a nonexistent registry file on every PR); the prior unconditional-posting behavior just meant nobody noticed the number was always "N/A". Fixed by reordering the two steps so the registry file exists by the time coverage generation runs. Also hardened the comment step itself: a NaN currentPct (tool genuinely fails even with the registry present) now skips posting entirely instead of falling through to "post unconditionally, malformed marker" — there's nothing actionable in an unparseable percentage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs * fix(ci): fix marker orphaning, crash-swallowing, findings ambiguity, PR-code exec /code:review's 5-agent parallel pass (Gate 2 of github:pr-ship) found four real issues, three converged on independently by 2+ agents: 1. CRITICAL (architecture review): coverage-delta.js's marker format change (bare `<!-- feature-coverage -->` -> `:pct=NN.N`) meant findExisting could never see a comment posted before this deploys, so every PR with an already-open comment gets a permanent duplicate on its next run (this workflow never deletes on unchanged, so nothing self-heals it). Fixed by widening the lookup prefix to match both formats while keeping the stricter regex for parsing the value. 2. MAJOR (architecture review): benchmark.yml's go-tier1 gate used a bare `if node -e ...`, which is exempt from bash's -e/pipefail, so any crash in the gate script (not just a real "no regression") landed in the same else-branch as a legitimate false result — silently suppressing the PR comment on failure, exactly the top risk pre-mortem.md names for this item. Fixed to distinguish a genuine crash (fail the step loudly) from a real boolean result. 3. CRITICAL/MAJOR (testing-quality + code-quality reviews, converged independently): ux-analysis.yml's findings_count collapsed "the Claude analysis step never completed" and "it completed and found zero issues" into the same value, so a crash partway through analyze.ts's main() silently read as "clean" to the new gate. Fixed with a sentinel (-1, written before any real work, overwritten only on successful completion) that the gate treats as actionable — while an *absent* count (the whole step conditionally skipped, e.g. no ANTHROPIC_API_KEY configured) stays non-actionable by design, so this doesn't reintroduce noise for repos without the key set. 4. MAJOR (security review): e2e-video.yml's notify job previously executed zero PR-controlled code; the prior commit's checkout fix (for the MODULE_NOT_FOUND bug) meant it now requires a file from the PR's own checked-out branch inside a pull-requests:write job — a real, narrowly-scoped new attack surface for a fork PR. Fixed by inlining the one-line videoAnomaly check instead of requiring the external module, removing the checkout entirely. (The security review also flagged the same require() pattern in the other 3 workflows' comment steps, but those jobs already execute far riskier PR-controlled code — compiling and running the PR's own Go binary, Playwright tests, and analyze.ts — with equal or greater privilege, predating this item entirely; restructuring that is a separate, cross-cutting CI-architecture change out of scope here.) Discarded after the skeptic pass: sticky-comment CRUD duplication across the 4 workflows (already an explicit, documented scope decision in plan.md's Pattern Decisions — not new); a `gateAction()`/test-name doc/code mismatch in validation.md (docs-only, no functional impact); mixed --frozen-lockfile convention across tools/* install steps (genuinely mixed precedent in this repo already, not a regression). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WJAr4V2JjoKm7Z2ZEB6qs --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * feat(backlog): durable escalation signals for bouncing/multi-reason stuck items (#444) * chore(sdd): planning artifacts for backlog-bounce-escalation Requirements, research, plan.md, and ADR-001/ADR-002 for the multi-reason stuck-state escalation and capped-while-bouncing durable marker feature. * chore(sdd): planning artifacts for backlog-bounce-escalation * feat(backlog): add multiple_reasons/bounce_cap_exhausted StuckReason plumbing Epic 1.1 of backlog-bounce-escalation: two new synthetic, aggregate StuckReason values (multiple_reasons, bounce_cap_exhausted) made valid and round-trippable end to end — Go domain type, proto enum, RPC mapping — ahead of the detector logic (Epic 1.2/1.3) that will set/resolve them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * feat(backlog): surface multi-reason/bounce-cap-exhausted stuck reasons in UI Epic 2.1 of backlog-bounce-escalation: adds labels/icons/GROUP_ORDER entries and a distinct chipEscalated style for the two new synthetic StuckReason values (MULTIPLE_REASONS, BOUNCE_CAP_EXHAUSTED) so they render instead of falling back to "Unknown reason"; excludes them from the otherReasonsCount badge so an item's own escalation row doesn't inflate its own badge; and extends the existing resolved-ghost mechanism to show a de-escalation banner when a multiple_reasons row resolves while the item stays open elsewhere. Also fixes a latent gap in the existing justResolved ghost mechanism: grouped items were derived only from the current items list, so a resolved item's ghost card (added to resolvedGhosts) could never actually render since it had already dropped out of `items` — visibleItems now re-includes still-tracked ghost items until their fade timer clears them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * feat(backlog): add multi-reason escalation detector (Signal 1) Adds reconcileMultiReasonEscalation, registered after self_heal in ReconcileStuck: marks a durable multiple_reasons row for items with 2+ simultaneously open non-escalation stuck reasons, dwell-gated notify-once, and resolves it once the count drops back below threshold. Excludes abandoned_review from the count when it's structurally coupled to a gate-blocked bouncing row, per ADR-001. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * feat(backlog): add capped-while-bouncing escalation marker (Signal 2) Epic 1.3 of the backlog-bounce-escalation plan: when the bouncing remediation gate parks (justParked) while bouncing itself is still open, mark a durable StuckReasonBounceCapExhausted row and upgrade the park notification from generic WARNING/HIGH to ERROR/URGENT framing naming the retry loop specifically — durable evidence the retry loop isn't converging, not just an ordinary single-reason park. - autoReopenWithBackoffGate now takes the item's actual itemStatus (both call sites pass BacklogStatus(item.Status), not a hardcoded in_progress) so MarkStuck's expectedStatus precondition doesn't silently no-op for review-status items. - reconcileBouncingItems resolves bounce_cap_exhausted alongside bouncing at both resolve sites; selfHealStuck gets a mirrored backstop case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * chore(registry): update backlog-stuck registry entries for escalation signals Bumps testIds/lastModified on both per-feature files to cover the new multiple_reasons/bounce_cap_exhausted stuck-reason tests, and fixes a stale test-name reference (contain12Entries -> contain16Entries) found while updating. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(backlog): apply sdd:6-verify Layer 1+2 findings for bounce escalation - Gate Signal 2's ERROR/URGENT notify on MarkStuck's applied result, so a failed-closed expectedStatus precondition can't fire a notification with no durable row behind it (architecture review CONCERN). - Combine the identical bouncing/bounce_cap_exhausted selfHealStuck switch cases into one, and extract the repeated resolve-both-reasons pairing in reconcileBouncingItems into resolveBouncingAndCapExhausted (refactor review). - Evaluate the abandoned_review/bouncing coupling exclusion via the already-fetched bouncing row + evaluateRemediation directly, instead of RemediationBlocked's redundant FindOpenStuckStates re-query (Go idiom review). - Document why the frontend de-escalation banner only covers MULTIPLE_REASONS, not BOUNCE_CAP_EXHAUSTED (React review SUGGEST). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * chore(deps): record go.sum checksums surfaced by full-repo build go.mod unchanged; these are checksums for existing transitive dependencies (go-runewidth, tablewriter) that hadn't been recorded yet in this worktree until make build/make test ran full-repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx * fix(backlog): reduce reconcileMultiReasonEscalation cognitive complexity below gate golangci-lint's gocognit gate (limit 40, only-new-issues) flagged reconcileMultiReasonEscalation at complexity 54 on PR #444's CI run (https://github.com/tstapler/stapler-squad/actions/runs/31664698295/job/94336632619). Split the per-item decision tree into categorizeOpenStuckRows, excludeStructurallyCoupledAbandonedReview, deescalateMultiReasonIfNeeded, notifyMultiReasonEscalationIfReady, and reconcileMultiReasonEscalationForItem so each piece is independently readable; behavior is unchanged (all 6 TestReconcileMultiReasonEscalation_* cases and the rest of ./session/ pass). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZFcZyivXenBipwthBDfdE --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): widen hook-URL wait budget to 60s and serialize race gate with -p 1 (#300) * chore(sdd): architecture research for flaky-hook-url-tests * chore(sdd): implementation plan + ADR for flaky-hook-url-tests Phase 3 planning artifacts: creative pass, domain glossary, pattern decisions, task breakdown (3 epics / 4 stories / 9 tasks), and ADR-001 justifying -p 1 over an isolated second -race invocation for the CI contention mitigation. Also commits the earlier requirements.md and research/*.md that were still uncommitted from phases 1-2. * chore(sdd): test validation plan for flaky-hook-url-tests Maps each requirements.md scope item and plan.md task to concrete verification: regression checks for the two affected integration tests, a stress/flake-verification repro (-count=10 under artificial CPU contention), the Task 1.2.4 non-fatal-teardown check, the Task 2.1.1 coverage-artifact/-race-scope check, and the Task 2.1.2 averaged (>=3-run) wall-clock measurement. No user-facing surface, so UX Acceptance Tests section is N/A; no schema change, so Migration Test is N/A. * chore(sdd): validation/review artifacts + plan patches for flaky-hook-url-tests Adds architecture review, adversarial review, and pre-mortem docs from SDD Phase 3/4, and patches plan.md to resolve the cross-artifact consistency blocker (missing runner-concurrency check task) and the pre-mortem P1 item (testSocketOnce misdiagnosis prevention). * chore(sdd): adoption plan for ci-hookurl-race-flake Research and planning surfaced that this backlog item duplicates the already-planned, already-reviewed project_plans/flaky-hook-url-tests/ (same root cause, same two tests, ADR-001 + full task breakdown never implemented). implementation/plan.md consolidates rather than re-derives: it adopts that plan/ADR wholesale, reproduces the full Epic/Story/Task breakdown here for self-containment, and re-verifies every file:line reference against the current tree (all accurate, no drift found). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(sdd): validation artifacts + review-driven patches for ci-hookurl-race-flake Phase 4 (validate) for the flaky hook-URL/MCP-URL CI test backlog item: validation.md, pre-mortem.md, architecture-review.md, adversarial-review.md. Cross-artifact consistency review surfaced 3 blockers (AC #1's N/method never decided, a citation to a "Success Metrics" section that didn't exist in this project's own requirements.md, and a stale file path) — fixed by adding this project's own Success Metrics section (N=20, distinct decision from the sibling project) and correcting the path. Pre-mortem's 3 P1 items (unfolded review concerns, un-rebutted build-vs-buy divergence, unenforced AC#4 evidence gate) are folded directly into plan.md and research/build-vs-buy.md rather than left as unread siblings for Phase 5. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): widen hook-URL wait budget to 60s and serialize race gate with -p 1 Fixes intermittent CI timeouts in TestServer_should_WriteUnchangedHookURL_When_StartedOnExplicitPort and TestServer_should_WriteRealPortIntoSessionHooksAndMCPURL_When_StartedWithPortZeroThenSessionCreated (backlog ci-hookurl-race-flake, GitHub issue #192), consolidating the never-implemented flaky-hook-url-tests plan with this ticket's own AC #4/#5 latency-measurement requirement. - Widen waitForPermissionRequestHookCommand's two call sites 30s -> 60s, matching the helper's own pre-existing doc comment (never applied). - Convert all 4 hand-rolled poll loops in server_integration_test.go to require.Eventually (3) / testutil/wait.WaitForCondition (1, kept non-fatal for the teardown helper). - Add -p 1 to the gating go test -race invocation in build.yml to reduce cross-package CPU contention; coverage total verified byte-identical before/after (25.5%). - Add a stress-repro comment above both flaky tests, fixing a latent regex bug in the originally-planned repro command (TestServer_should_Write.*HookURL only matched one of the two tests; corrected to TestServer_should_Write.*(HookURL|MCPURL)). - Flip ADR-001 status from Proposed to Accepted now that -p 1 actually ships. Measured evidence (recorded in project_plans/ci-hookurl-race-flake/implementation/validation.md): hook-injection pipeline latency under -race + artificial contention: 20/20 passed, elapsed 7.29s-8.39s (median 7.74s), ~7.5x headroom under the 60s budget -- confirms tmux spin-up (not InjectHookConfig's write) dominates, so no approval_handler.go change is made. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R2k5hFUMTMsktDkeE5J7LC --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(tmux): raise session-create poll loop's backoff cap to reduce self-inflicted subprocess contention (#463) TestCommitImportExternalSession_PersistsAndLinksAndSuspends_When_StartAndSuspendSucceed has been cited as failing on `main` (7 of 8 recent Build runs since 2026-08-12T19:18 UTC) with "cold restore Start failed ... timed out waiting for tmux session ...: <nil>" -- TmuxSession.start()'s poll loop (session/tmux/tmux.go) hitting sessionCreateTimeout after `tmux new-session -d` reported success (err == nil) but DoesSessionExistNoCache() never saw it in time. Investigation (VERIFIED via `gh run view`/`git show <sha>:path` on every cited failing run, not just ancestry): all 5 failing runs analyzed (910592f4b, 950ab86df, d2376370c, 03b2fab88, 19221d9d5) have ZERO occurrences of STAPLER_SQUAD_TMUX_CREATE_TIMEOUT_SECONDS in their own .github/workflows/build.yml tree -- they all predate commit 1bb310edb (2026-08-12 17:15 UTC), which already turned sessionCreateTimeout into an env-overridable var and set it to 30s in CI's "Run tests with coverage" step specifically to give this test's isolated tmux -L server enough scheduling headroom under -race + concurrent packages. Those 5 runs come from long-running worktree/backlog-automation branches that branched off main before that fix landed and were only later pushed, so their CI ran a stale pre-fix tree despite recent run timestamps. CI run 31660107168, for the current tip of main (which does have the fix), completed its Test job with **success** -- confirmed live via `gh run view 31660107168 --json jobs`. Local reproduction (`go test ./session -run TestCommitImportExternalSession... -race -count=20`) also passes reliably in ~0.08-0.09s/iteration, ruling out a logic bug or deadlock in the commit-import path itself. So sessionCreateTimeout was already fixed; this commit is a smaller, independently-justified hardening for the same root cause it documents: the poll loop's own exponential backoff was capped at 50ms, which after ~4 doublings (~75ms) means DoesSessionExistNoCache() forks a real `tmux list-sessions` subprocess roughly every 50ms for the rest of the (CI-widened, 30s) timeout window -- up to ~600 subprocess spawns in the worst case. Every one of those is gated CPU/scheduling work competing with the very tmux-server fork/exec the loop is waiting on, on the same CPU-starved runner diagnosed as the timeout's root cause -- i.e. the poll loop was worsening the exact contention it exists to tolerate. Raising the cap to 250ms cuts worst-case subprocess-spawn count roughly 5x while staying invisible in the common case, where the session is already visible within the first few low-delay iterations. Verified: go vet ./session/...; go test ./session/tmux/... -race -short; go test ./session -short -count=1; go test ./session -run TestCommitImportExternalSession_PersistsAndLinksAndSuspends... -race -count=20 (all pass); golangci-lint run ./session/tmux/... (0 issues). Claude-Session: https://claude.ai/code/session_01YZFcZyivXenBipwthBDfdE Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(session): delete shells before session to avoid FK constraint failure Shell.session is a Required() edge, so any Shell rows still pointing at a session trip a FOREIGN KEY constraint when the session row is deleted. Delete() already manually cascades Worktree, DiffStats, ClaudeMetadata, and ClaudeSession — add Shell to that same manual-cascade sequence. Addresses the "DeleteSession FK constraint failures" error category from /errors/. * docs(bugs): file BUG-070 for flaky gogitstore mmap repack test TestMmapIndex_PinnedReadersSurviveConcurrentRe…
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
Backlog items in
review/in_progresscan bounce (rework → re-review → rework...) without ever converging, and the only signal was a one-time WARN notification with no escalation once themaxAutoReworkIterationscap is repeatedly hit. This adds two durable, queryable escalation signals reusing the existingbacklog_stuck_statesinfrastructure (no new tables/migrations, per ADR-001):multiple_reasonsrow and a dwell-gated ERROR/URGENT notification. The structurally-coupledbouncing+abandoned_reviewpair is excluded from the count (they gate on the same remediation backoff, so counting both inflates "2 reasons" into "nearly every bouncing item").MaxRemediationAttemptswhilebouncingis still open gets a durablebounce_cap_exhaustedrow with a differentiated ERROR/URGENT notify (was WARNING/HIGH), distinguishable from an ordinary park.Requirement item 3 (whether flaky-test-classified items need a distinct review strategy) is deferred per ADR-002, with a follow-up backlog item filed.
What Changed
session/domain/backlog.go— two newStuckReasonconstants + proto/RPC mappingsession/stuck_decisions.go— pure threshold/dwell predicates (isMultiReasonEscalated,multiReasonEscalationNotifyReady)session/backlog_lifecycle.go—reconcileMultiReasonEscalationdetector (Signal 1), registered afterself_heal;resolveBouncingAndCapExhaustedhelper;selfHealStuckbackstop casesession/backlog_lifecycle_review.go—autoReopenWithBackoffGateextended withitemStatusparam + Signal 2'sjustParkedbranchweb-app/src/components/backlog-stuck/*— labels/icons/GROUP_ORDERentries, distinctchipEscalatedCSS (not reusing any existing chip color),otherReasonsCountself-exclusion, de-escalation confirmation banner (reuses existingjustResolvedghost-card pattern)docs/registry/features/**— registry entries updated for both surfacesFull planning trail:
project_plans/backlog-bounce-escalation/(requirements, research, plan, validation, both ADRs).Test plan
go test ./session/...— full package green (51s), including 24 new tests across both signalsgo build ./...,go vet ./session/...,gofmt— cleanmake lint— 0 issuesnpx jest --testPathPatterns="backlog-stuck"— 109/109 passing,tsc --noEmitcleansdd:6-verify(idiom/architecture/refactor review across 4 parallel agents) — 1 CONCERN + 3 SUGGESTs found and fixed, re-verifiedbacklog_stuck_statesdata: 4 currently-open items would escalate under Signal 1, the coupled-pair exclusion correctly suppresses a 5th; Signal 2's condition is currently vacuous (no bouncing row at the attempt cap)🤖 Generated with Claude Code
https://claude.ai/code/session_01SW6Nbaqim2AS5knaHVY5dx