fix: stream agent output in resume --headless via a shared event handler - #400
fix: stream agent output in resume --headless via a shared event handler#400seniorquico wants to merge 1 commit into
resume --headless via a shared event handler#400Conversation
|
@seniorquico is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughHeadless run and resume execution now share structured engine-event logging and persisted session-state tracking through a new event handler. Resume wiring, interruption handling, tests, and CLI documentation are updated accordingly. ChangesHeadless event flow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/resume.tsx (1)
498-506: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winResume now tracks
activeTaskIdsbut never resets them on interrupt.
run --headlessresets in_progress tasks to open before persistinginterrupted(seesrc/commands/run.tsxgracefulShutdown). Here the handler populatesactiveTaskIdsontask:activated, buthandleSignalpersists them as-is, so tasks stayin_progressin the tracker until the next resume's stale-session recovery clears them. Mirroring the run behavior removes that window and matches the shared-behavior goal of the PR.🛠️ Align with the run path
const handleSignal = async (): Promise<void> => { logger.info('system', 'Interrupted, stopping...'); + const activeTasks = getActiveTasks(headlessEvents.getState()); + if (activeTasks.length > 0) { + logger.info('system', `Resetting ${activeTasks.length} in_progress task(s) to open...`); + const resetCount = await engine.resetTasksToOpen(activeTasks); + if (resetCount > 0) { + headlessEvents.setState(clearActiveTasks(headlessEvents.getState())); + } + } // Save interrupted state headlessEvents.setState({ ...headlessEvents.getState(), status: 'interrupted' }); await savePersistedSession(headlessEvents.getState());🤖 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/commands/resume.tsx` around lines 498 - 506, Update resume's handleSignal to reset every task referenced by activeTaskIds from in_progress to open before persisting the interrupted session, mirroring run.tsx's gracefulShutdown behavior. Ensure the reset occurs before savePersistedSession and preserves the interrupted status and cleanup sequence.
🧹 Nitpick comments (3)
src/commands/run.tsx (1)
3259-3270: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShutdown paths are now byte-identical; extract one helper.
gracefulShutdownandhandleSigtermrepeat the same reset-active-tasks + persist-interrupted sequence verbatim. A singlefinalizeShutdown()used by both removes the drift risk.♻️ Suggested extraction
+ const resetAndMarkInterrupted = async (): Promise<void> => { + const activeTasks = getActiveTasks(headlessEvents.getState()); + if (activeTasks.length > 0) { + logger.info('system', `Resetting ${activeTasks.length} in_progress task(s) to open...`); + const resetCount = await engine.resetTasksToOpen(activeTasks); + if (resetCount > 0) { + headlessEvents.setState(clearActiveTasks(headlessEvents.getState())); + } + } + headlessEvents.setState({ ...headlessEvents.getState(), status: 'interrupted' }); + await savePersistedSession(headlessEvents.getState()); + };Also applies to: 3302-3312
🤖 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/commands/run.tsx` around lines 3259 - 3270, Extract the duplicated active-task reset and interrupted-session persistence sequence from gracefulShutdown and handleSigterm into a shared finalizeShutdown() helper. Have both shutdown paths call this helper, preserving the existing resetTasksToOpen, clearActiveTasks, status update, and savePersistedSession behavior.src/commands/headless-events.ts (1)
60-64: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winSerialize the fire-and-forget persistence writes.
persist()starts an unawaitedsavePersistedSession(currentState)on every event. Bursty events (task:activated→iteration:completed→task:completed) can have their writes interleave, so a later write may land before an earlier one and leave.ralph-tui/session.jsonholding a stale snapshot. Also, silently discarding every save failure means a persistently unwritable session file is invisible.A simple promise chain keeps ordering and lets you surface failures once.
♻️ Serialize writes and log failures
+ let persistChain: Promise<void> = Promise.resolve(); + const persist = (): void => { - savePersistedSession(currentState).catch(() => { - // Silently continue on save errors - }); + const snapshot = currentState; + persistChain = persistChain + .then(() => savePersistedSession(snapshot)) + .catch((error: unknown) => { + logger.warn( + 'session', + `Failed to persist session state: ${error instanceof Error ? error.message : String(error)}` + ); + }); };🤖 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/commands/headless-events.ts` around lines 60 - 64, Update persist() in the headless event handling flow to serialize savePersistedSession(currentState) calls through a shared promise chain, ensuring each write starts only after the previous one settles. Preserve fire-and-forget behavior for callers, but surface persistence failures through the existing logging mechanism instead of silently discarding them.tests/commands/headless-events.test.ts (1)
104-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
satisfies EngineEventoveras EngineEventin the event literals.The
ascasts disable structural checking, so a rename in the engine event union (e.g.previousError,action,tasks) would leave these tests compiling and asserting stale behavior.satisfies EngineEventkeeps the literals validated against the union while preserving inference.♻️ Example
- } as EngineEvent); + } satisfies EngineEvent);Also applies to: 170-196, 290-296
🤖 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 `@tests/commands/headless-events.test.ts` around lines 104 - 109, Replace the `as EngineEvent` assertions on the event literals passed to `handler.handleEvent` with `satisfies EngineEvent`, including the occurrences around the referenced additional ranges. Keep each literal’s inferred types and existing test behavior unchanged while enabling structural validation against the `EngineEvent` union.
🤖 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.
Outside diff comments:
In `@src/commands/resume.tsx`:
- Around line 498-506: Update resume's handleSignal to reset every task
referenced by activeTaskIds from in_progress to open before persisting the
interrupted session, mirroring run.tsx's gracefulShutdown behavior. Ensure the
reset occurs before savePersistedSession and preserves the interrupted status
and cleanup sequence.
---
Nitpick comments:
In `@src/commands/headless-events.ts`:
- Around line 60-64: Update persist() in the headless event handling flow to
serialize savePersistedSession(currentState) calls through a shared promise
chain, ensuring each write starts only after the previous one settles. Preserve
fire-and-forget behavior for callers, but surface persistence failures through
the existing logging mechanism instead of silently discarding them.
In `@src/commands/run.tsx`:
- Around line 3259-3270: Extract the duplicated active-task reset and
interrupted-session persistence sequence from gracefulShutdown and handleSigterm
into a shared finalizeShutdown() helper. Have both shutdown paths call this
helper, preserving the existing resetTasksToOpen, clearActiveTasks, status
update, and savePersistedSession behavior.
In `@tests/commands/headless-events.test.ts`:
- Around line 104-109: Replace the `as EngineEvent` assertions on the event
literals passed to `handler.handleEvent` with `satisfies EngineEvent`, including
the occurrences around the referenced additional ranges. Keep each literal’s
inferred types and existing test behavior unchanged while enabling structural
validation against the `EngineEvent` union.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 01f9b02d-63f7-40f5-a5f6-9f35ec788272
📒 Files selected for processing (5)
src/commands/headless-events.tssrc/commands/resume.tsxsrc/commands/run.tsxtests/commands/headless-events.test.tswebsite/content/docs/cli/resume.mdx
1 participant
Fixed #399. Wide open to feedback if this approach is incorrect, and I'm happy to keep iterating on a solution!
resume --headlessproduced two plainconsole.loglines per iteration andnothing in between. Iterations that take many minutes therefore emitted zero
output for the whole iteration, which is indistinguishable from a hang.
The cause is duplication, not configuration.
run --headlessandresume --headlesseach had their own engine event subscription, and resume's wasa subset:
run --headlessresume --headless(before)agent:outputengine:warningiteration:retryingiteration:skippedtask:selectedtask:activated/task:completed[timestamp] [level] [component] messageconsole.logThe engine emits
agent:output(src/engine/index.ts:1182) regardless; resumejust never subscribed to it.
Change
Extract run's headless event handler into
src/commands/headless-events.ts(
createHeadlessEventHandler) and use it from both commands. Resume now gets thefull structured log stream, including agent output streamed as the iteration runs.
The handler owns the session state it maintains and exposes
getState()/setState()so shutdown paths can still mark a session interrupted.Command-specific concerns stay in their commands: notifications, remote server,
and
engineStartTime/lastErrortracking remain inrunas a second listener;lock release remains in
resume.run --headlessoutput is unchanged — the switch moved verbatim, and the sharedlistener registers first, so log-then-notify ordering is preserved.
resume --headlessnow also maintainsactiveTaskIdsinsession.jsonthe wayrundoes, which makes stale-task recovery consistent between the two.--- Iteration 1: title ---→[INFO] [progress] Iteration 1/20: Working on <id> - <title>). That's the pointof the change, but it is a user-visible output change for anyone grepping resume
stdout.
Notes
Resume inherits run's verbosity: for agents that emit structured JSONL, every
non-empty line is logged under the
agentcomponent. That is existingrunbehavior, now consistent across both commands, but it is more output than resume
used to produce.
Summary by CodeRabbit
New Features
Documentation
resume --headless, including streamed agent output.