fix(detection): detect indented spinners and CR-overwritten esc-to-interrupt#108
Merged
Conversation
…pture for OneShot
Two improvements to OneShot (-p) session control:
1. **Fix initial prompt not submitting** (\r instead of \n): The session driver
was sending initialPrompt + "\n" (LF) to the PTY. Claude Code's interactive
readline needs \r (CR) to submit — the same signal a physical Enter key sends.
Sessions received the typed text but never executed it, causing inactivity
timeouts. Confirmed by log: "sent initial prompt" followed by 10-min idle.
The startup dialog answers ("1\n") work because those menus handle both,
but Claude's readline interface only responds to \r.
2. **Capture claude session_id from --output-format json output**: OneShot
sessions now launch with -p --output-format json. When the session exits,
the driver parses the JSON output for "session_id" and stores it as
ConversationUUID on the instance. Subsequent restarts automatically use
--resume <uuid>, sending Claude back into the same conversation with full
context instead of re-running the task from scratch.
Supporting infrastructure:
- SetClaudeConversationUUID() — thread-safe setter that fires a save callback
- SetClaudeSessionIDSavedCallback() — wired in service layer to flush to DB
- wireClaudeSessionIDCallback() — registered on all creation paths including
loadInstancesWithWiring (startup) and CreateDirectorySession (backlog)
- Prompt is now appended even when claudeSessionID is set for OneShot
sessions so continuation prompts are delivered after --resume
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New JSON columns (programs, subcommands, etc.) were NOT NULL with no
SQL-level DEFAULT, causing the SQLite copy-table migration to fail for
existing rows. Adding Optional() makes the columns nullable so old rows
can be copied; new rows still get []string{} from the Go-level Default.
Also adds build backup + auto-rollback to install-service:
- make install-service saves stapler-squad.prev before building
- health check polls /health for 15s after service start
- on failure, auto-restores .prev and restarts the service
- make rollback provides a manual escape hatch
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These files are auto-generated by ent and covered by .gitignore. Force-added in the previous commit by mistake. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…terrupt
Two compounding bugs caused active sessions to appear idle when Claude Code's
task manager panel was open:
1. The `claude_thinking_verb` pattern required the spinner at column 0
(`(?m)^[spinner]`). When a spinner appears as a sub-item inside the task
manager panel it is indented by 2 spaces (" ✽ Roosting…"), so the pattern
failed to match. Fix: `(?m)^\s*[spinner]` allows any leading whitespace.
2. `DetectFromLines` applied `collapseCarriageReturns` before scanning, which
silently discarded "esc to interrupt" when the task manager panel overwrote
it via `\r` on the same terminal row. The scan then continued upward and
found "✻ Baked for 18m 15s" (StatusSuccess), mapping to IdleStateWaiting
via the `default` branch instead of IdleStateActive. Fix: scan CR-split
segments in reverse (last-written → first-written) in both DetectFromLines
and DetectWithContextFromLines; the last segment still takes precedence
(preserves "esc to interrupt\r? for shortcuts" → Idle), but earlier segments
are checked when the last one is only Ready/Unknown.
Also adds explicit `StatusInputRequired` and `StatusSuccess` cases to
`mapStatusToIdleState` (both already resolved to `IdleStateWaiting` via the
`default` branch — making them explicit documents intent and prevents future
omissions from silently passing).
Regression tests in bug_regression_test.go cover:
- Indented spinner variants (2-space, 4-space, tab)
- CR-collapse: esc-to-interrupt preserved when overwritten by task manager
- CR-collapse: idle state still detected when overwritten by "? for shortcuts"
- Full-content DetectFromLines returning Active despite older Success scrollback
- InputRequired dialog detected correctly over older Success completion line
- "Esc to cancel" (capital E) correctly NOT matching esc_to_interrupt (Active)
- mapStatusToIdleState explicit coverage for all DetectedStatus values
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
DetectFromLines / DetectWithContextFromLines: earlier CR segments now only promote high-urgency statuses (Active, NeedsApproval, InputRequired, Error). Low-urgency statuses (Success, Processing, Idle) in overwritten segments no longer override the visually-last segment. TestMapStatusToIdleState_ExplicitCoverage: replace IdleState(-1) sentinel with a dedicated skipMustNot bool field to eliminate the type-cast hack. TestBug1_IndentedSpinner_NoRegression: add inline clause-level comments explaining which regex clause rejects each false-positive case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ent test Add missing DetectedStatus rows (Idle, Ready, TestsFailing, Unknown) to TestMapStatusToIdleState_ExplicitCoverage so the test actually covers every status value, including the default fall-through cases. Add TestCRCollapse_LastSegmentSuccessIsAuthoritative to document and guard the asymmetry: when the LAST CR segment produces Success it is authoritative (mirrors TestBug1_CRCollapse_EscToInterrupt_Preserved where the earlier Active segment wins). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
✅ Registry ValidationTest Coverage: 93/131 features have
|
Contributor
Go Benchmarks (Tier 1) |
There was a problem hiding this comment.
Pull request overview
This PR improves Claude Code session status detection to correctly identify active and input-required states when the task manager UI is present (indented spinners and carriage-return overwrites), and adds regression coverage to prevent these cases from reappearing. It also includes related repo maintenance changes: making ApprovalRule JSON criteria fields nullable for safer SQLite migrations and adding an install-service health check with automatic rollback support.
Changes:
- Fix detection patterns and line-scanning logic to handle indented spinners and
\r-overwritten status lines (while keeping the “last visual segment is authoritative” behavior). - Add regression fixtures + tests (including a new snapshot case) for the reported detection failures and boundary cases.
- Add service install health-check + rollback workflow and adjust ent schema nullability for JSON criteria fields.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| session/session_driver.go | Removes a stray blank line in diff context (no functional change). |
| session/instance_claude.go | Removes a stray blank line in diff context (no functional change). |
| session/ent/schema/approvalrule.go | Makes JSON criteria fields optional/nullable with defaults to ease SQLite migrations. |
| session/ent/runtime.go | Removes generated ent runtime file from tracking. |
| session/ent/migrate/schema.go | Removes generated ent migration schema file from tracking. |
| session/ent/hook/hook.go | Removes generated ent hook file from tracking. |
| session/ent/approvalrule/where.go | Removes generated ent predicate file from tracking. |
| session/ent/approvalrule/approvalrule.go | Removes generated ent constants/helpers file from tracking. |
| session/ent/approvalrule.go | Removes generated ent model file from tracking. |
| session/ent/approvalrule_update.go | Removes generated ent update builder file from tracking. |
| session/ent/approvalrule_create.go | Removes generated ent create/upsert builder file from tracking. |
| session/detection/testdata/claude_input_required_with_success_scrollback.txt | Adds fixture for “InputRequired below prior success scrollback” case. |
| session/detection/testdata/claude_active_task_manager.txt | Adds fixture for active task manager overlay (indented spinner + status bar). |
| session/detection/snapshot_test.go | Adds snapshot coverage for the active task manager overlay fixture. |
| session/detection/idle.go | Adds explicit StatusInputRequired and StatusSuccess mappings to document intent. |
| session/detection/detector.go | Updates spinner regex to allow indentation; adds CR-segment reverse scanning in line-based detectors. |
| session/detection/bug_regression_test.go | Adds regression tests for indented spinners, CR overwrite behavior, and mapping coverage. |
| scripts/install-service.sh | Adds health check and auto-rollback behavior after installing/updating the service. |
| Makefile | Adds binary backup/rollback targets and wires them into install-service. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+566
to
+568
| // ^\s* allows leading whitespace so indented spinners (e.g. task manager sub-items) | ||
| // are detected: " ✽ Roosting… (9m 52s · ↓ 2.8k tokens)" | ||
| Pattern: `(?m)^\s*[·✢✳✶✻✽●*]\s+[A-Z][a-zA-Z'\-éèêàâùûôîïëüöäÿæœ]*(?:…|\.{1,3})`, |
Owner
Author
There was a problem hiding this comment.
Fixed in the latest commit: changed to so only horizontal whitespace (spaces/tabs) is consumed, preventing RE2 from matching across newlines.
Owner
Author
There was a problem hiding this comment.
Fixed in the latest commit: changed the leading anchor from ^\s* to ^[ \t]* (and the post-spinner whitespace class too) so only horizontal whitespace (spaces/tabs) is consumed — RE2 cannot consume a newline and match a spinner on a different line.
Comment on lines
+276
to
+280
| url="http://localhost:8543/health" | ||
| printf "==> Waiting for service to be healthy" | ||
| while [ "$elapsed" -lt "$max_wait" ]; do | ||
| if curl -sf "$url" >/dev/null 2>&1; then | ||
| printf "\n" |
Owner
Author
There was a problem hiding this comment.
Fixed: added a command -v curl guard so the health check is skipped entirely (with an informational message) if curl is not available, preventing false rollbacks.
Comment on lines
+28
to
+31
| if got != StatusActive && got != StatusProcessing { | ||
| t.Errorf("Detect(%q) = %s, want StatusActive (indented spinner must be detected)", | ||
| tc.input, got) | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed: error message now reads "want StatusActive or StatusProcessing" to match the two-value acceptance condition.
Contributor
E2E RPC Latency |
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. |
…est error - Replace `\s*` with `[ \t]*` and `\s+` with `[ \t]+` in claude_thinking_verb pattern so it cannot consume newlines and match spinners across lines - Guard health-check curl call in install-service.sh with `command -v curl` so absence of curl skips the check rather than triggering rollback - Update TestBug1_IndentedSpinner error message to mention both accepted statuses (StatusActive or StatusProcessing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
✅ Registry ValidationTest Coverage: 93/131 features have
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
✅ Registry ValidationTest Coverage: 93/131 features have
|
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
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
Fixes two status detection bugs affecting sessions running Claude Code's task manager panel:
Bug 1 — Active session detected as idle when the task manager panel is open:
claude_thinking_verbregex required the spinner character at column 0 (^[spinner]). Spinners inside the task manager are indented 2 spaces (✽ Roosting…), so they never matched.↑/↓ to selectwith a\rprefix on the same row.collapseCarriageReturnsdiscarded the hidden "esc to interrupt" text, so the session appeared idle.Bug 2 — InputRequired not flagged when a selection dialog appears below a prior completion line:
mapStatusToIdleStatelacked explicit cases forStatusInputRequiredandStatusSuccess, both falling todefault: return IdleStateWaiting. Added explicit cases to document intent.Changes
session/detection/detector.go(?m)^[spinner]→(?m)^\s*[spinner]to allow indented spinnersDetectFromLines/DetectWithContextFromLines: scan CR-split segments in reverse; the last (visual) segment is authoritative, but earlier segments can promote high-urgency statuses (Active, NeedsApproval, InputRequired, Error) — low-urgency statuses (Success, Processing) in overwritten earlier segments are not promotedsession/detection/idle.gocase StatusInputRequiredandcase StatusSuccessinmapStatusToIdleStateTests
session/detection/bug_regression_test.go— 11 regression tests covering both bugs and boundary cases (CR-collapse, idle-stays-idle, last-segment-authoritative)session/detection/snapshot_test.go— new snapshot entry for active session with task manager overlaysession/detection/testdata/— two new fixtures🤖 Generated with Claude Code