feat(cli): heartbeat the local progress spinner with elapsed time#108
Merged
Conversation
The local writer phase (`Authoring workflow with Workforce persona...`) can sit for 5-25 minutes on large specs. Today the spinner shows a static message the entire time, so users have no signal whether the writer is making progress, hung in skill install, blocked on the network, or close to the 20-min SIGTERM. The only way to investigate is to `ps aux | grep claude` and inspect the writer subprocess by hand. This change adds a periodic refresh to `createLocalProgressReporter`: every 5 seconds (configurable via `--`-flagless `RICKY_PROGRESS_HEARTBEAT_MS` env var or the `localProgressHeartbeatMs` dep), the spinner text is re-rendered with an elapsed-time suffix derived from when the last `onProgress` message was set. Output goes from: ⠋ Authoring workflow with Workforce persona... (sits silent for 20 minutes) to: ⠋ Authoring workflow with Workforce persona... (5s) ⠙ Authoring workflow with Workforce persona... (10s) … ⠼ Authoring workflow with Workforce persona... (4m 55s) … The heartbeat is opt-out (`localProgressHeartbeatMs: 0`) for tests and strict-output integrations; the default 5s is unintrusive but visible enough to confirm the process is alive between meaningful phase transitions. The setInterval handle is `.unref()`'d so it never blocks the process from exiting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PR adds a configurable heartbeat mechanism to the CLI spinner that periodically refreshes with elapsed-time suffixes. It extends ChangesCLI Spinner Heartbeat Feature
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/surfaces/cli/commands/cli-main.test.ts (1)
2053-2054: ⚡ Quick winStabilize these heartbeat tests by removing real-time sleeps.
These checks depend on short real-time delays and may intermittently miss interval ticks under CI load. Prefer fake timers (
vi.useFakeTimers+vi.advanceTimersByTimeAsync) so elapsed-heartbeat assertions are deterministic.Proposed deterministic timer pattern
+ vi.useFakeTimers(); + try { - await cliMain({ + const run = cliMain({ argv: ['run', 'workflows/generated/issue-3.ts', '--foreground'], output: ttyOutputSink(), isTTY: true, createProgressSpinner, runInteractive: runner, localProgressHeartbeatMs: 4, now, }); + await vi.advanceTimersByTimeAsync(30); + await run; + } finally { + vi.useRealTimers(); + }Also applies to: 2108-2109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/surfaces/cli/commands/cli-main.test.ts` around lines 2053 - 2054, Replace the real-time sleep-based waits in the heartbeat tests (the setTimeout(...) before returning fakeInteractiveResult({ ok: true, localResult: stagedLocalResult() }) and the similar sleep at lines 2108-2109) with deterministic fake timers: call vi.useFakeTimers() at the start of the test, replace the await new Promise(setTimeout...) with await vi.advanceTimersByTimeAsync(...) for the same elapsed interval, and restore timers after the test; ensure you advance the timers enough to trigger the heartbeat interval being asserted in the test so assertions on elapsed-heartbeat become deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/surfaces/cli/commands/cli-main.test.ts`:
- Around line 2053-2054: Replace the real-time sleep-based waits in the
heartbeat tests (the setTimeout(...) before returning fakeInteractiveResult({
ok: true, localResult: stagedLocalResult() }) and the similar sleep at lines
2108-2109) with deterministic fake timers: call vi.useFakeTimers() at the start
of the test, replace the await new Promise(setTimeout...) with await
vi.advanceTimersByTimeAsync(...) for the same elapsed interval, and restore
timers after the test; ensure you advance the timers enough to trigger the
heartbeat interval being asserted in the test so assertions on elapsed-heartbeat
become deterministic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 115ef99c-d89b-4dd3-bead-9dc627fafada
📒 Files selected for processing (2)
src/surfaces/cli/commands/cli-main.test.tssrc/surfaces/cli/commands/cli-main.ts
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
Ricky Eval ReviewRun: Passed: 11 | Needs human: 43 | Reviewable: 43 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesThese cases passed deterministic checks and include captured Ricky output for a human verdict against their REVIEW
|
This was referenced May 15, 2026
docs(local): name the JS-watchdog's native-hang limitation + recommend outer timeout(1) wrapper
#116
Merged
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
Make the local writer phase visibly alive. Today the spinner sits on
Authoring workflow with Workforce persona...for 5–25 minutes on large specs with no signal whether the writer is making progress, hung in skill install, blocked on the network, or close to the 20-min SIGTERM. The only diagnostic available isps aux | grep claude --modeland reading the writer subprocess's args by hand.This PR adds a periodic refresh to
createLocalProgressReporter. Every 5 seconds (configurable), the spinner text is re-rendered with an elapsed-time suffix derived from when the lastonProgressmessage was set:Before
After
Why now
Caught this debugging a multi-spec parallel run today: 4 ricky processes sat at 0% CPU for 24 minutes with empty per-spec logs. Looked completely hung. Turned out each had a healthy
claude --model claude-sonnet-4-6child at 3% CPU writing real output — but the only way to know that was a manualps aux. A simple elapsed-time tick on the spinner would have eliminated the guesswork.What changed
createLocalProgressReporternow tracks the lastonProgressmessage + the time it was set, and starts asetInterval(default 5s) that re-renders the spinner text withformatElapsed(now - messageSetAt). The interval is.unref()'d so it never blocks process exit.progressSpinnerTextgains an optionalelapsedMsarg and suppresses the suffix for elapsed < 1s (avoids "(0s)" flicker right after a new message lands).formatElapsedhelper renders compactNsfor under a minute,Nm SSspast a minute.CliMainDeps.localProgressHeartbeatMsandCliMainDeps.nowoverrides for deterministic tests. Also honorsRICKY_PROGRESS_HEARTBEAT_MSenv var so users can tighten or disable the cadence without touching code.Test plan
Two new tests in
cli-main.test.ts(123 pass, was 121):refreshes the spinner text with an elapsed-time suffix while the local writer phase is silent— injects a deterministic clock + 4ms heartbeat cadence, advances the clock 65s, and verifies the spinner text picks up both(Ns)and(Nm XXs)forms.honors localProgressHeartbeatMs: 0 to disable the elapsed-time heartbeat entirely— confirms the existing behavior is preserved for callers that explicitly opt out (e.g. integrations that want strict, byte-for-byte spinner output).All 121 pre-existing
cli-main.test.tstests still pass — the heartbeat is invisible to existing tests because their runners return synchronously before the default 5s tick fires.Out of scope
Two natural follow-ups that I deliberately kept out of this PR to keep the diff small and the review surface focused:
.livelog file. The writer already has anonProgresschunk callback ({ stream, text }) that is currently unused at the CLI layer; piping it to.workflow-artifacts/ricky-persona-debug/writer/<digest>-live/output.live.txtwould let power userstail -ffor real-time visibility into what the writer model is actually generating. Worth its own PR with care for log-size / rotation.Resolving persona...,Installing skills...,Spawning writer (claude/claude-sonnet-4-6)...,Streaming output...would make the heartbeat even more useful. Needs hooks into the persona resolver + harness-kit which I didn't want to touch here.🤖 Generated with Claude Code