feat: stream interrupt, session recovery, and resume (Phase 03)#15
Merged
Conversation
Deliver Esc-to-interrupt with partial text preservation, tool subprocess kill, auto-resume on user-only tail, /resume regenerate, and server-side stream buffering. Expand .gitignore for local-only paths and document session recovery in README. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
railway-app
Bot
temporarily deployed
to
lavish-courage / MoCode-TUI-pr-15
📝 WalkthroughWalkthroughAdds full session interrupt/resume support: pressing Esc kills in-flight tool subprocesses, finalizes pending tool parts, and snapshots the SSE stream into a server-side ChangesSession Interrupt / Resume Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant SessionChat
participant useChat
participant LocalChatTransport
participant ServerChatRoute
participant StreamReplayBuffer
User->>SessionChat: press Esc
SessionChat->>useChat: interrupt()
useChat->>useChat: killTrackedToolProcesses()
useChat->>useChat: finalizeInterruptedAssistant(messages)
useChat->>ServerChatRoute: chat.stop() (abort signal)
ServerChatRoute->>StreamReplayBuffer: clearActiveStream(buffer)
ServerChatRoute->>ServerChatRoute: normalizeInterruptedMessages()
ServerChatRoute->>ServerChatRoute: shouldPersistOnFinish() → db.session.update
useChat->>ServerChatRoute: PATCH /sessions/:id (pruned messages)
SessionChat->>SessionChat: resolvePreResponseEsc() → restore composer text
User->>SessionChat: /resume command
SessionChat->>useChat: continueGeneration(mode, model)
useChat->>useChat: trimMessagesForRegenerate()
useChat->>ServerChatRoute: POST /chat (regenerate from last user msg)
ServerChatRoute->>StreamReplayBuffer: registerStreamBuffer + ingest(sseStream)
StreamReplayBuffer-->>SessionChat: SSE chunks via createReplayStream()
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
🚅 Deployed to the MoCode-TUI-pr-15 environment in lavish-courage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli/src/lib/local-chat-transport.ts (1)
88-99: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKey the active stream by
chatId.
sendMessages()acceptschatId, but Line 212 discards it andreconnectToStream()returns whichever stream last wrote the singleton slot. If this transport instance is reused across sessions, reconnecting session B can attach to session A's stream, and A finishing can also clear B's active stream. Please store{ chatId, stream }(or a map) and only clear/return the matching entry.Proposed fix
- private activeStream: ReadableStream<UIMessageChunk> | null = null; + private activeStream: + | { chatId: string | undefined; stream: ReadableStream<UIMessageChunk> } + | null = null; @@ - const stream = new ReadableStream<UIMessageChunk>({ + const stream = new ReadableStream<UIMessageChunk>({ start(controller) { const reader = rawStream.getReader(); void (async () => { try { while (true) { const { done, value } = await reader.read(); if (done) { - self.activeStream = null; + if (self.activeStream?.stream === stream) self.activeStream = null; controller.close(); return; } controller.enqueue(value); } } catch (error) { - self.activeStream = null; + if (self.activeStream?.stream === stream) self.activeStream = null; controller.error(error); } })(); }, cancel(reason) { - self.activeStream = null; + if (self.activeStream?.stream === stream) self.activeStream = null; return rawStream.cancel(reason); }, }); - this.activeStream = stream; + this.activeStream = { chatId, stream }; @@ () => { - this.activeStream = null; + if (this.activeStream?.stream === stream) this.activeStream = null; }, { once: true }, ); - void chatId; return stream; } @@ - async reconnectToStream(): Promise<ReadableStream<UIMessageChunk> | null> { - return this.activeStream; + async reconnectToStream({ + chatId, + }: { + chatId: string; + }): Promise<ReadableStream<UIMessageChunk> | null> { + return this.activeStream?.chatId === chatId ? this.activeStream.stream : null; } }Also applies to: 166-220
🤖 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 `@packages/cli/src/lib/local-chat-transport.ts` around lines 88 - 99, The active stream handling in LocalChatTransport is using a single shared slot, which can mix up reconnects between different chats. Update sendMessages() and reconnectToStream() to key the stored stream by chatId (for example, keep a { chatId, stream } entry or a map) so only the matching session is returned or cleared. Make sure the logic in LocalChatTransport uses the chatId argument to isolate streams per conversation and prevent one session from overwriting another.packages/server/src/routes/chat.ts (1)
221-254: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear the active replay entry when finish persistence fails.
If
db.session.update(...)throws here, thecatchlogs and exits withoutclearActiveStream(id).getActiveStreamResponse()will keep serving that registered buffer from the process-wide registry, so a failed finish leaves stale replay state and leaked buffer memory behind until another request overwrites the same session.Proposed fix
async onFinish(event) { try { let messagesToPersist = event.messages; @@ } catch (error) { + clearActiveStream(id); console.error("Failed to persist chat finish", { error, sessionId: id }); } },🤖 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 `@packages/server/src/routes/chat.ts` around lines 221 - 254, The finish-handling path in chat.ts leaves stale replay state when db.session.update fails because clearActiveStream(id) only runs after the update succeeds. Update the try/catch around the session persistence logic so clearActiveStream(id) is also executed in the failure path for the same session id, ensuring the active stream registry is cleaned up even when “Failed to persist chat finish” is logged. Use the existing clearActiveStream, db.session.update, and the surrounding try/catch in the chat route to place the cleanup so it always runs for this finish flow.
🧹 Nitpick comments (1)
packages/cli/src/screens/new-session.tsx (1)
96-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove this drive-by formatting fix.
This whitespace change violates the Karpathy behavioral guidelines added in this PR: "Touch only code required by the request. Do not 'improve' adjacent code, comments, or formatting" and "no drive-by refactors." Revert to keep the diff strictly scoped to session interrupt/resume.
🤖 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 `@packages/cli/src/screens/new-session.tsx` at line 96, Revert the unrelated formatting-only change in the new-session screen so the diff stays scoped to the session interrupt/resume work. Specifically, restore the original single-line style for the conditional around state handling in new-session.tsx and avoid any non-functional formatting edits in the new-session component.Source: Learnings
🤖 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.
Inline comments:
In `@packages/cli/src/hooks/use-chat.ts`:
- Around line 342-359: The useEffect in use-chat.ts is re-triggering local
session persistence on every rerender because it depends on
options?.onPersistError even though that callback is only needed inside the
persist error path. Move the error handler used by scheduleLocalSessionPersist
into a ref, or memoize the callback in session.tsx where it is passed inline, so
chat.status === "ready" rerenders do not repeatedly call persistFn() and rewrite
the session file.
- Around line 244-246: The late-output skip guard in use-chat.ts is missing the
bash rejection path, so a rejected `requestBashApproval()` can still call
`chat.addToolOutput(...)` after Esc adds the tool call id to
`skipToolOutputIdsRef`. Update the `verdict === "reject"` branch to check
`shouldSkipToolOutput()` (or the same skip-set condition) before adding tool
output, matching the existing MCP and post-execution local output handling. Keep
the fix aligned with the existing `shouldSkipToolOutput`,
`skipToolOutputIdsRef`, and `chat.addToolOutput` flow so interrupted assistant
turns are not mutated.
- Around line 416-424: The `/resume` trimming logic in `use-chat` is rewriting
metadata for every retained message via the `map()` over `trimmed`, which
mutates historical mode/model provenance. Update the logic around `userIndex`
and `trimmed` so only the last retained user message that is being regenerated
gets `params.mode` and `params.model`, and leave earlier messages’ metadata
unchanged.
In `@packages/cli/src/lib/bot-message-footer.test.ts`:
- Around line 36-45: The usage assertion in the bot message footer test is too
loose because the regex in formatAssistantFooter only requires one token count
to appear. Update the test case for formatAssistantFooter so it verifies all
three usage values from usage (inputTokens, outputTokens, totalTokens) are
rendered, either by asserting each value separately or by matching the exact
footer fragment, using the existing test name that checks usage token counts
when present.
In `@packages/cli/src/lib/bot-message-footer.ts`:
- Around line 41-45: Suppress token counts in formatAssistantFooter() while a
message is still streaming so footer text matches bot-message.tsx. Update the
footer-building logic in bot-message-footer.ts to only append usage.inputTokens
and usage.outputTokens when the message is complete, using the existing
params/usage handling in formatAssistantFooter() to gate this behavior for
in-flight assistant messages.
In `@packages/cli/src/lib/local-chat-transport.test.ts`:
- Around line 120-192: The reconnect tests only verify the happy path with the
same chatId that created the stream, so they don’t prove the chatId parameter is
honored. Update the LocalChatTransport reconnect coverage by adding an assertion
in reconnectToStream that a mismatched chatId does not return the active stream
once session tracking is in place, and keep the existing
sendMessages/reconnectToStream tests aligned with the chatId-based session
contract.
In `@packages/cli/src/lib/stream-interrupt.ts`:
- Around line 83-96: The resume check in detectResumeEligibility is too broad
because any assistant tail with visible content is treated as resumable. Update
detectResumeEligibility to require an explicit interrupted/incomplete signal
before returning "partial-assistant" instead of relying on
hasVisibleAssistantContent(last) alone, and keep the "none" path for completed
assistant replies. If completed assistant tails cannot occur here, document that
invariant near detectResumeEligibility or the surrounding resume logic.
In `@packages/cli/src/screens/session.tsx`:
- Around line 135-148: Auto-resume in the session screen is reading eligibility
from raw initialMessages and then resuming with the current mode/model, which
can miss normalized recovered turns and replay them with the wrong submission
metadata. Update the useEffect flow to derive the recovered turn from the
normalized chat state/source of truth used by useChat, and pass the original
last-user-message resume metadata into continueGeneration rather than the live
prompt config. Use the existing detectResumeEligibility,
shouldAutoResumeOnMount, and continueGeneration paths to keep the resume
decision and regenerated turn aligned.
In `@packages/server/src/lib/active-stream-registry.ts`:
- Around line 10-20: `clearActiveStream()` currently deletes by `sessionId`
only, so an older request can clear a newer stream buffer in
`active-stream-registry`. Update `clearActiveStream` to accept the registered
`StreamReplayBuffer` (or equivalent request identity) and only remove the entry
if it still matches the current `activeStreams` value for that session. Then
update the finish/error cleanup call sites that use `registerStreamBuffer()` so
they pass the same buffer they registered, keeping `GET /:id/stream` scoped to
the correct active stream.
In `@packages/server/src/lib/stream-buffer.test.ts`:
- Around line 27-31: The stream-buffer tests are using setTimeout-based sleeps
to guess when ingestion has reached a certain point, which makes the replay
assertions nondeterministic. Update the affected cases in stream-buffer.test.ts
to use an explicit synchronization signal from the source stream instead of
waiting, so createReplayStream is called only after the intended chunk has been
buffered. Reuse the existing source/gate pattern in the test setup and adjust
the assertions around buffer.ingest and createReplayStream accordingly.
In `@packages/server/src/routes/chat.ts`:
- Around line 252-264: The chat failure logs currently include raw error
objects, which can leak prompt or tool payloads into server logs. Update the
error handling in the chat route’s streaming/persistence callbacks and the outer
catch around the chat handler to log only a sanitized shape, using the existing
sessionId context and a safe message/metadata extracted from the error rather
than the full error object. Keep the onError return value unchanged, but replace
any console.error usage that passes the raw error with a redacted summary in the
chat handler flow.
In `@packages/server/src/routes/sessions.ts`:
- Around line 14-16: The updateSessionSchema in sessions.ts currently accepts
any JSON array for messages, which can store malformed transcripts that later
break chat resume/submit in chat.ts. Tighten the PATCH validation by reusing the
same UI message validator used by the chat submit flow (the one feeding
validateUIMessages and MocodeUIMessage[]) so only valid persisted session
messages reach Prisma in the session update path.
---
Outside diff comments:
In `@packages/cli/src/lib/local-chat-transport.ts`:
- Around line 88-99: The active stream handling in LocalChatTransport is using a
single shared slot, which can mix up reconnects between different chats. Update
sendMessages() and reconnectToStream() to key the stored stream by chatId (for
example, keep a { chatId, stream } entry or a map) so only the matching session
is returned or cleared. Make sure the logic in LocalChatTransport uses the
chatId argument to isolate streams per conversation and prevent one session from
overwriting another.
In `@packages/server/src/routes/chat.ts`:
- Around line 221-254: The finish-handling path in chat.ts leaves stale replay
state when db.session.update fails because clearActiveStream(id) only runs after
the update succeeds. Update the try/catch around the session persistence logic
so clearActiveStream(id) is also executed in the failure path for the same
session id, ensuring the active stream registry is cleaned up even when “Failed
to persist chat finish” is logged. Use the existing clearActiveStream,
db.session.update, and the surrounding try/catch in the chat route to place the
cleanup so it always runs for this finish flow.
---
Nitpick comments:
In `@packages/cli/src/screens/new-session.tsx`:
- Line 96: Revert the unrelated formatting-only change in the new-session screen
so the diff stays scoped to the session interrupt/resume work. Specifically,
restore the original single-line style for the conditional around state handling
in new-session.tsx and avoid any non-functional formatting edits in the
new-session component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e123a36-353d-4bde-9da4-9521178e5145
📒 Files selected for processing (44)
.cursor/rules/karpathy-behavioral-guidelines.mdc.gitignoreAGENTS.mdREADME.mdREADME.zh-CN.mdpackages/cli/src/components/command-menu/commands.tsxpackages/cli/src/components/command-menu/types.tspackages/cli/src/components/dialogs/sessions-dialog.tsxpackages/cli/src/components/input-bar.tsxpackages/cli/src/components/messages/bot-message.tsxpackages/cli/src/components/session-shell.tsxpackages/cli/src/hooks/use-chat-autoresume.test.tspackages/cli/src/hooks/use-chat-continue.test.tspackages/cli/src/hooks/use-chat-persist.test.tspackages/cli/src/hooks/use-chat-persist.tspackages/cli/src/hooks/use-chat.tspackages/cli/src/lib/bot-message-footer.test.tspackages/cli/src/lib/bot-message-footer.tspackages/cli/src/lib/composer-restore.test.tspackages/cli/src/lib/composer-restore.tspackages/cli/src/lib/local-chat-transport.test.tspackages/cli/src/lib/local-chat-transport.tspackages/cli/src/lib/local-tools.tspackages/cli/src/lib/stream-error.test.tspackages/cli/src/lib/stream-error.tspackages/cli/src/lib/stream-interrupt.test.tspackages/cli/src/lib/stream-interrupt.tspackages/cli/src/lib/tool-process-registry.tspackages/cli/src/providers/session-chat-actions.tsxpackages/cli/src/screens/new-session.tsxpackages/cli/src/screens/session.tsxpackages/cli/src/utils/list-scroll-nav.tspackages/server/src/index.tspackages/server/src/lib/active-stream-registry.tspackages/server/src/lib/chat-abort.tspackages/server/src/lib/stream-buffer.test.tspackages/server/src/lib/stream-buffer.tspackages/server/src/lib/stream-interrupt.tspackages/server/src/routes/chat-abort.test.tspackages/server/src/routes/chat-stream.test.tspackages/server/src/routes/chat.tspackages/server/src/routes/sessions.tspackages/shared/src/assistant-content.tspackages/shared/src/index.ts
Comment on lines
+36
to
+45
| test("includes usage token counts when present (D-21)", () => { | ||
| const footer = formatAssistantFooter({ | ||
| mode: Mode.BUILD, | ||
| model: "claude-sonnet-4-6", | ||
| durationMs: 500, | ||
| streaming: false, | ||
| usage: { inputTokens: 100, outputTokens: 50, totalTokens: 150 }, | ||
| }); | ||
| expect(footer).toMatch(/100|150|50/); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Tighten this usage assertion so it proves all counts are rendered.
/100|150|50/ succeeds when any one of those numbers is present, so this test still passes if the formatter drops two of the three token counts. Assert each expected value (or the exact footer fragment) instead.
Suggested fix
const footer = formatAssistantFooter({
mode: Mode.BUILD,
model: "claude-sonnet-4-6",
durationMs: 500,
streaming: false,
usage: { inputTokens: 100, outputTokens: 50, totalTokens: 150 },
});
- expect(footer).toMatch(/100|150|50/);
+ expect(footer).toContain("100");
+ expect(footer).toContain("50");
+ expect(footer).toContain("150");
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test("includes usage token counts when present (D-21)", () => { | |
| const footer = formatAssistantFooter({ | |
| mode: Mode.BUILD, | |
| model: "claude-sonnet-4-6", | |
| durationMs: 500, | |
| streaming: false, | |
| usage: { inputTokens: 100, outputTokens: 50, totalTokens: 150 }, | |
| }); | |
| expect(footer).toMatch(/100|150|50/); | |
| }); | |
| test("includes usage token counts when present (D-21)", () => { | |
| const footer = formatAssistantFooter({ | |
| mode: Mode.BUILD, | |
| model: "claude-sonnet-4-6", | |
| durationMs: 500, | |
| streaming: false, | |
| usage: { inputTokens: 100, outputTokens: 50, totalTokens: 150 }, | |
| }); | |
| expect(footer).toContain("100"); | |
| expect(footer).toContain("50"); | |
| expect(footer).toContain("150"); | |
| }); |
🤖 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 `@packages/cli/src/lib/bot-message-footer.test.ts` around lines 36 - 45, The
usage assertion in the bot message footer test is too loose because the regex in
formatAssistantFooter only requires one token count to appear. Update the test
case for formatAssistantFooter so it verifies all three usage values from usage
(inputTokens, outputTokens, totalTokens) are rendered, either by asserting each
value separately or by matching the exact footer fragment, using the existing
test name that checks usage token counts when present.
Comment on lines
+14
to
+16
| const updateSessionSchema = z.object({ | ||
| messages: z.array(z.unknown()), | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate persisted session messages before writing them.
messages: z.array(z.unknown()) lets any JSON array into session.messages, but packages/server/src/routes/chat.ts later casts that column back to MocodeUIMessage[] and feeds it into validateUIMessages(...). A malformed PATCH can poison the stored transcript and break the next submit/resume for that session. Reuse the same message validator as the chat submit route here before calling Prisma.
Proposed fix
const updateSessionSchema = z.object({
- messages: z.array(z.unknown()),
+ messages: z.array(
+ z.custom(
+ (value) =>
+ value != null &&
+ typeof value === "object" &&
+ "id" in value &&
+ "parts" in value,
+ "Invalid message payload",
+ ),
+ ),
});Also applies to: 96-110
🤖 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 `@packages/server/src/routes/sessions.ts` around lines 14 - 16, The
updateSessionSchema in sessions.ts currently accepts any JSON array for
messages, which can store malformed transcripts that later break chat
resume/submit in chat.ts. Tighten the PATCH validation by reusing the same UI
message validator used by the chat submit flow (the one feeding
validateUIMessages and MocodeUIMessage[]) so only valid persisted session
messages reach Prisma in the session update path.
Guard bash reject after Esc interrupt, preserve historical message metadata on /resume, scope stream registry cleanup by buffer identity, stabilize local persist callback ref, key LocalChatTransport streams by chatId, and derive auto-resume from normalized chat state. Adds unit tests for each fix. Co-authored-by: Cursor <cursoragent@cursor.com>
railway-app
Bot
temporarily deployed
to
lavish-courage / MoCode-TUI-pr-15
Native local write tools previously executed without confirmation while bash blocklist and MCP write tools already had approval dialogs. Co-authored-by: Cursor <cursoragent@cursor.com>
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 (2)
packages/server/src/routes/chat.ts (2)
128-130: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winScope outer-catch cleanup to this request’s buffer.
clearActiveStream(id)deletes whatever stream is currently registered for the session. A failed second POST can therefore clear another in-flight stream for the same session; pass this request’sreplayBufferso the registry’s identity guard is used.Proposed fix
const userId = c.get("userId"); const { id, messages, mode, model, mcpTools } = c.req.valid("json"); + const replayBuffer = new StreamReplayBuffer(); try { @@ - const replayBuffer = new StreamReplayBuffer(); @@ } catch (error) { - clearActiveStream(id); + clearActiveStream(id, replayBuffer); const message = error instanceof Error ? error.message : "Chat request failed";Also applies to: 172-172, 269-270
🤖 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 `@packages/server/src/routes/chat.ts` around lines 128 - 130, The outer-catch cleanup in chat request handling is too broad because clearActiveStream(id) can remove a newer in-flight stream for the same session. Update the error cleanup in the chat route flow to pass this request’s replayBuffer into clearActiveStream so the registry identity guard only clears the matching stream. Apply the same scoped cleanup wherever the chat route’s try/catch or teardown paths call clearActiveStream, using the existing replayBuffer variable to locate the correct request-owned buffer.
214-223: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear the replay registry before skipping persistence.
When
shouldPersistOnFinishreturns false, this returns without clearing the buffer registered inconsumeSseStream; the pending-tool-call path can leave a stale closed stream in the active registry.Proposed fix
) { + clearActiveStream(id, replayBuffer); return; }🤖 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 `@packages/server/src/routes/chat.ts` around lines 214 - 223, The early return in the `shouldPersistOnFinish` branch leaves the SSE replay buffer registered by `consumeSseStream` in the active registry. Before returning false, make sure the stream is removed/cleared from the replay registry so the pending-tool-call path cannot leave a stale closed stream behind. Update the cleanup flow around the `shouldPersistOnFinish` check in `chat.ts` to always unregister the stream before skipping persistence.
🤖 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 `@packages/server/src/routes/chat.ts`:
- Around line 128-130: The outer-catch cleanup in chat request handling is too
broad because clearActiveStream(id) can remove a newer in-flight stream for the
same session. Update the error cleanup in the chat route flow to pass this
request’s replayBuffer into clearActiveStream so the registry identity guard
only clears the matching stream. Apply the same scoped cleanup wherever the chat
route’s try/catch or teardown paths call clearActiveStream, using the existing
replayBuffer variable to locate the correct request-owned buffer.
- Around line 214-223: The early return in the `shouldPersistOnFinish` branch
leaves the SSE replay buffer registered by `consumeSseStream` in the active
registry. Before returning false, make sure the stream is removed/cleared from
the replay registry so the pending-tool-call path cannot leave a stale closed
stream behind. Update the cleanup flow around the `shouldPersistOnFinish` check
in `chat.ts` to always unregister the stream before skipping persistence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3de3f8e4-c72b-4936-bc95-a7364ac61703
📒 Files selected for processing (17)
packages/cli/src/hooks/use-chat-persist.test.tspackages/cli/src/hooks/use-chat.tspackages/cli/src/lib/bot-message-footer.test.tspackages/cli/src/lib/bot-message-footer.tspackages/cli/src/lib/local-chat-transport.test.tspackages/cli/src/lib/local-chat-transport.tspackages/cli/src/lib/local-write-approval-ui.tspackages/cli/src/lib/local-write-approval.test.tspackages/cli/src/lib/local-write-approval.tspackages/cli/src/lib/stream-interrupt-resume.test.tspackages/cli/src/lib/stream-interrupt.tspackages/cli/src/screens/session.tsxpackages/server/src/lib/active-stream-registry.test.tspackages/server/src/lib/active-stream-registry.tspackages/server/src/lib/stream-buffer.test.tspackages/server/src/lib/stream-buffer.tspackages/server/src/routes/chat.ts
✅ Files skipped from review due to trivial changes (1)
- packages/cli/src/lib/local-write-approval.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/cli/src/hooks/use-chat-persist.test.ts
- packages/cli/src/lib/bot-message-footer.ts
- packages/server/src/lib/stream-buffer.ts
- packages/cli/src/lib/bot-message-footer.test.ts
- packages/server/src/lib/stream-buffer.test.ts
- packages/cli/src/lib/local-chat-transport.ts
- packages/cli/src/screens/session.tsx
- packages/cli/src/hooks/use-chat.ts
railway-app
Bot
temporarily deployed
to
lavish-courage / MoCode-TUI-pr-15
5 tasks
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
Interrupted by userand subprocesses are killed immediately./resume(regenerate from last user turn)..gitignoreexpanded for local-only paths (.mocode/,.agents/,tmp/, etc.); Karpathy behavioral guidelines added to AGENTS.md.Test plan
bun test packages/cli packages/server— 193 tests passInterrupted by user, generation stops/resumeregeneratesMade with Cursor
Summary by CodeRabbit
New Features
/resumecommand and improved Esc behavior for interrupted chats.Bug Fixes
Documentation