feat(session): track and surface subagent count in WAITING_FOR_AGENT status - #312
Draft
tstapler wants to merge 35 commits into
Draft
feat(session): track and surface subagent count in WAITING_FOR_AGENT status#312tstapler wants to merge 35 commits into
tstapler wants to merge 35 commits into
Conversation
No end-user-facing UX applies (pure CI/infra change) — lightweight DX pass instead, comparing failure surfacing to golangci-lint annotations vs plain-log precedent (ESLint/gofmt) and flagging --ci flag as a zero-cost quick win. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Documents the BacklogStatus enum pattern, mark3labs/mcp-go tool registration/handler shape, and confirms the ent status column needs no migration for a new terminal value.
Evaluates TOML parsing (go-toml v2), regex engine (stdlib RE2), and hot-reload (fsnotify) choices, plus config-aggregation frameworks (Viper/koanf) and existing plugin-system prior art (Caddy/Vector/Grafana), for the user-extensible agent detector plugin loader. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers RE2/ReDoS guarantees and limits, fsnotify hot-reload gotchas (temp-file-rename, debounce, inotify limits, macOS/Linux differences), TOML/config pitfalls, the atomic-swap concurrency pattern to reuse from worktree_git.go, and fail2ban/ESLint/VS Code prior art. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Documents existing table/sort patterns to mirror (SessionDetailDrawer's Tools Breakdown, backlog page's aria-sort), the proto gaps blocking AC-1 (per-turn) and AC-6 (cache split), and concrete labeling/empty-state/ sort-order decisions for AC-1 through AC-3.
Research pass on what commonly breaks when adding to an already-live analytics feature: the #280 silent-$0.00 precedent, TokenStore RWMutex contention, jumping-list risk for async-sorted cost data, stale registry schema.json vs real frontend entry shape, and the backlogItemEventSender pattern needed before WatchInsights is testable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Documents edge cases and unstated scope for the 6 gap-closing ACs: AC-1 requires a proto change (TurnTimeline isn't exposed over the wire, not just unwired in the frontend); AC-2's SessionList.tsx has zero token data wired in today (TokenBadge is unused dead code, contradicting requirements.md); AC-3 needs no backend change (ListSessionTokens sort_by already implemented, SessionsTable.tsx's sort is just hardcoded); AC-4 needs the same narrow-interface refactor WatchBacklogItems already established for testing connect-go streaming RPCs; AC-5's 5 target files share one +feature marker with an existing aggregated registry entry, creating a collision risk; AC-6 is free (client-side formula, data already on the wire).
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.
Phase 3 architecture/task-breakdown plan (7 verified pipeline hops from regex capture through proto to the SubStatusChip badge) plus ADR-001 documenting the statusCacheEntry cache-coherence requirement between GetCurrentStatus and GetStatusAndIdleInfo. Also commits the requirements and research docs from earlier phases that were still untracked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 3 planning artifacts: plan.md (3 phases, 6 epics, 14 stories, 41 tasks, 30-term glossary) plus four ADRs covering the go-toml/v2 dependency, the registry-level copy-on-write snapshot (vs. the existing unused StatusDetector YAML loader), TOML schema v1, and the RE2 trust boundary. Also picks up the architecture/features research files that were still untracked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 3 planning artifact for the token-cost-tracking gap-closure project:
domain glossary, pattern decisions (new-RPC vs bolt-on for AC-1, interface
extraction for AC-4's WatchInsights test, client-side derivation for AC-6),
risk control, and a 5-phase task breakdown sequenced by risk (AC-4/AC-5
low-risk first, AC-2's new SessionList data join last). Also commits the
requirements.md and research/{build-vs-buy,stack}.md artifacts from earlier
phases that were left uncommitted.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Wireframes, interaction flows, error/empty states, and UX acceptance criteria for the 4 user-facing surfaces in Phases 2-4 of the implementation plan (SessionsTable click-to-sort, ModelBreakdownChart cache-hit-rate label, SessionDetailDrawer per-turn table, SessionList Sort: Cost option). Flags a concrete contrast/layout risk in reusing TokenBadge.css.ts's badgeVariant.warning for outlier-turn highlighting in a table-cell context. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial/architecture review, UX design, validation plan, and pre-mortem for the subagent-spawn-tracking feature; plan.md patched to resolve 2 architecture-review CONCERNS, 1 adversarial-review safety concern, 2 cross-artifact-consistency BLOCKERs (title-attribute contradiction with ux.md, missing NaN/negative test coverage), and the pre-mortem's P1 (corrected a false "no debounce precedent" claim). Triad review (Product/UX/Engineering) verdict: READY TO BUILD.
…-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).
Maps all 6 requirements.md acceptance criteria plus plan.md's Phase 1-2 story-level Given/When/Then scenarios to concrete Go test names, organized as unit vs. integration, so implementation starts test-first.
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>
…or detector-plugins Phase 4 (sdd:4-validate): validation.md test suite, pre-mortem.md failure analysis, and cross-artifact consistency check. The pre-mortem's P1 finding (DetectForProgram/the plugin registry has zero production call sites, so a loaded plugin would never change what a user's session actually shows) was independently confirmed by adversarial-review.md's own Blocker and by direct repo verification (idle.go's live detection path uses getDefaultPatterns(), not the registry this plan builds). Resolved by adding Epic 2.4 to plan.md, wiring ClaudeController.Start to resolve its detector via the new registry snapshot instead of the always-generic default. Also folds in a Hardening Addendum (compile-time budget, total-file-count cap, rebuildSnapshot context cancellation, InitPlugins re-entrancy guard, non-fatal seed-file-write) closing 5 previously-open adversarial-review Concerns, and requirements.md amendments (Target User, Success Metric, Risky Assumption sections; corrected plugin-directory and version-field wording) from a 3-round product/engineering triad review.
project_plans/backlog-status-transitions/research/architecture.md belongs to an unrelated, concurrently in-progress backlog item and was swept into the previous commit because it was already staged in the shared index at commit time. Untracking (not deleting) it here so it returns to the working tree for its own session to commit under its own message.
…rl-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>
Phase 3 plan grounded in the actual DeleteSession/EventExited lifecycle code (not just research assumptions) — records ADR-001 for the storage-independence decision the grounding surfaced: DeleteSession deletes the Session row synchronously before Destroy() teardown, so the CompletionSummary entity must have no FK edge to Session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 3 plan.md resolves all requirements.md Open Questions explicitly: timestamp-watermark dedup (ADR-001, resolving the build-vs-buy/pitfalls ID-based vs. architecture.md watermark contradiction), a concrete "substantive feedback" length filter, always-attempted one-shot Copilot review requests, an unconditional legacy-login gh CLI version workaround, and brings the pre-existing STUCK_REASON_PR_NEEDS_FIX proto-enum gap in scope since the reuse-first design depends on it. Also commits the earlier phase 1/2 requirements.md and research/*.md artifacts, which were sitting uncommitted per .claude/rules/sdd-planning-artifacts-commit.md.
project_plans/context-health-monitoring/research/stack.md belongs to an unrelated, concurrently in-progress backlog item and was swept into the previous commit because it was already staged in the shared index at commit time (same failure mode as 3c2f0b3). Untracking (not deleting) it here so it returns to the other item's own uncommitted working state.
Requirements, research, plan, ADRs, and validation docs for per-session context-health signal detection (backlog item 2b435aa4).
…owup Completes sdd:4-validate: validation.md (test coverage mapping) and pre-mortem.md (5 failure modes, 1 P1) written; cross-artifact consistency check found 2 BLOCKER contradictions (dedup mechanism, migration claim) and 2 CONCERNs, all patched into requirements.md. Also resolves the sole adversarial-review.md BLOCKER (zero-value timestamp dedup blind spot) by switching Task 1.1.2d's parse-failure fallback to time.Now(), and folds in pre-mortem's P1 finding (multi-item batch coverage logging) as new plan.md tasks 3.1.2e/f. Readiness gate: PASS.
Phase 4 validation/pre-mortem/triad-review pass on top of existing requirements/research/plan (Phases 1-3, from an earlier uncommitted session): fixes a P1 filterByProject worktree-path bug, closes a BLOCKER gap where Story 2.2.3 (click-through navigation) was referenced but never written, resolves adversarial/architecture review blockers, and switches SessionHitCard to a real <a> element per triad UX review.
Confirms pkg/classifier.RuleBasedClassifier is the live auto-approve rule engine (session/approval_policy.go is dead code), and that CI status caching/transport/badge rendering already exist end-to-end for the session list/detail views — the diff viewer just needs to be wired into existing infra, not build new fetch/cache logic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Also picks up requirements.md and research/*.md left uncommitted from earlier phases, per .claude/rules/sdd-planning-artifacts-commit.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ewer Closes the AC5 override-affordance, storage-lookup error-path, and AC6 stale-CI-race blockers flagged by adversarial review; adds architecture review and UX design artifacts. Adversarial review verdict: BLOCKED -> CONCERNS. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tus-diff-viewer Completes the sdd:4-validate phase (validation.md requirement->test mapping, pre-mortem.md failure modes with P1 resolved), and closes the "no scoped override" gap design/ux.md's Exit-Path Analysis flagged as failing by adding the "Approve anyway" wireframe/interaction spec and threading the resulting Task 1.1.2b interval-injection decision and Task 1.1.3d verification task into implementation/plan.md.
…ING_FOR_AGENT status Captures the numeric count from the three WaitingForAgent regex patterns (background agents, shells still running, monitors still running) at the exact match, threads it through the detection call chain via a new DetectWithContextAndCountFromLines method (leaving the interface-pinned DetectWithContextFromLines untouched), plumbs it through the shared statusCacheEntry cache for coherence between GetCurrentStatus and GetStatusAndIdleInfo, exposes it as Session.subagent_count (proto field 72), and renders it in SubStatusChip's WAITING_FOR_AGENT chip text/title with correct singular/plural, falling back to the original plain text for 0/undefined/negative/NaN. Also files BUG-053 for two pre-existing, unrelated jest failures (SessionDetail.embedded.test.tsx, BacklogEmptyState.test.tsx) discovered while running the full frontend suite during Phase 6 validation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148xoQRvZTN64YLzYKwgByY
Owner
Author
|
Closing as superseded: this branch's last known commit (654c601) is already present on main, so this item's work has already shipped through another path. No further fix is needed here. |
Owner
Author
|
Reopening — the auto-close comment compared against commit 654c601, which predates this PR's actual feature commit (c03abf2). Verified |
Code review on PR #312 found that SubStatusChip hardcoded "background agent(s)" for the subagent_count badge even though the count is sourced from three distinct WaitingForAgent regex patterns (background agents, shells still running, monitors still running) — a "2 shells still running" match rendered as "Waiting for 2 Agents", which is wrong. Switch to source-neutral "task(s)" wording, matching the requirements doc's "⊕ 3 tasks" badge language. Also adds a cheap default-zero regression test for InstanceToProto's SubagentCount passthrough. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The 5 conflicting files are add/add: this branch created independent stale drafts of planning docs for ci-status-diff-viewer, flaky-hook-url-tests, and token-cost-tracking (other backlog items) before they were shipped and merged to main via separate PRs (#304 et al). Taking main's version for all 5 — this branch's copies are superseded drafts, not this PR's actual work. # Conflicts: # project_plans/ci-status-diff-viewer/design/ux.md # project_plans/ci-status-diff-viewer/implementation/plan.md # project_plans/flaky-hook-url-tests/research/features.md # project_plans/token-cost-tracking/implementation/plan.md # project_plans/token-cost-tracking/requirements.md
Contributor
✅ Registry ValidationTest Coverage: 24/181 features have
|
Contributor
Go Benchmarks (Tier 1) |
Contributor
E2E RPC Latency |
Contributor
UX Analysis
|
Contributor
📊 Feature E2E CoverageFeature coverage report unavailable
|
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. |
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
When Claude Code spawns background agents, shells, or monitors, the parent session's
WAITING_FOR_AGENTchip shows generic unnumbered text ("Waiting for Agents") even though the terminal already contains the count (e.g. "✻ Waiting for 2 background agents to finish"). This PR captures that count and surfaces it end-to-end: detection → proto → UI.Closes the underlying backlog item (migrated from issue #183).
What Changed
session/detection/detector.go,pattern_set.go): added a capturing group to the threeWaitingForAgentregex patterns (background agents, shells still running, monitors still running).PatternSet.MatchLinesnow extracts the count at the exact match viaFindStringSubmatch+ guardedstrconv.Atoi— never a separate/decoupled pass.StatusDetector.DetectWithContextAndCountFromLinesthreads the count through the existing multi-line reverse-scan chain. The pre-existingDetectWithContextFromLinesis left untouched (it's pinned by theTerminalDetectorinterface and consumed byreview_queue_determiner.goplus several test files).session/claude_controller.go):statusCacheEntrygains asubagentCountfield; bothGetCurrentStatusandGetStatusAndIdleInfowrite it consistently to the sharedatomic.Pointer[statusCacheEntry]cache so the two methods never disagree for the same tail hash (seeproject_plans/subagent-spawn-tracking/decisions/ADR-001-subagent-count-cache-coherence.md).int32 subagent_count = 72;field onSession(proto/session/v1/types.proto), regenerated viamake proto-gen.server/adapters/instance_adapter.go): copies the count onto the wireSessionmessage unconditionally.SubStatusChip.tsx,SessionRow.tsx): theWAITING_FOR_AGENTchip's text andtitlenow include the count with correct singular/plural (e.g. "Waiting for 2 Agents"), falling back to the original plain text for0/undefined/negative/NaN.docs/bugs/open/BUG-053-...mdfor two pre-existing, unrelated jest failures discovered during full-suite validation (confirmed unrelated viagit stash+ isolated re-run).Full planning trail (requirements → research → plan with adversarial review → validation/pre-mortem → this implementation) is in
project_plans/subagent-spawn-tracking/.Test plan
go build $(go list ./... | grep -v '/server/web$')— cleango test ./session/detection/... ./session ./server/adapters/...— all pass, including new unit/integration tests covering 0/1/N/no-match, multi-pattern-collision ("winning line wins", not summed), CR-segment threading, and cache coherence betweenGetCurrentStatus/GetStatusAndIdleInfomake build— Next.js + Go binary build succeedsmake test— passes except one pre-existing, already-documented flake (BUG-051,session/tmuxunder full-suite parallel load), confirmed unrelated to this diffmake lint— 0 issuescd web-app && npx jest --testPathPatterns="SubStatusChip"— 22/22 passnpx tsc --noEmit— cleangit diff --stat go.mod go.sum web-app/package.json— empty (no new dependency)sdd:6-verify— 4 parallel review agents (Go idioms, React/TS idioms, architecture, refactor-candidates), 0 BLOCKER/MUST FIX findings, one small pluralization-dedupe appliedCo-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_0148xoQRvZTN64YLzYKwgByY