feat(cli): show Claude compaction as a summary card with token delta - #1691
feat(cli): show Claude compaction as a summary card with token delta#1691junmo-kim wants to merge 19 commits into
Conversation
Replace the plain 'Compaction completed' line with a pi-style '📦 Compacted (<before> → <after> tokens)' fallback built from the system/compact_boundary metadata, and prefix the start notice with 📦. The SDK stream carries no summary body (measured), so the delta line is the terminal state of the completion event.
Claude Code echoes slash-command bookkeeping back over the SDK as plain user-role messages wrapped in <local-command-*> tags. Flag them isMeta so the existing downstream filters drop them instead of leaking raw stdout.
Keep the header (title, token delta, timestamp) always visible and hide the summary body behind a chevron toggle. Toggle state is component-local and non-persistent.
There was a problem hiding this comment.
Findings
- [Major] Later-turn
/compactnever enters the new summary path — command detection only runs for the initial turn, while subsequent turns are pushed directly; evidencecli/src/claude/claudeRemote.ts:406.
Suggested fix:const enqueueTurn = (next: Turn) => { const command = parseSpecialCommand(next.message) if (command.type === "compact") beginCompactCommand() mode = next.mode messages.push({ type: "user", message: { role: "user", content: next.message } }) } // Use from both applyInitialTurn and scheduleNextMessage.
- [Major] A resumed or repeated compaction can emit the previous summary — the poll accepts any existing latest
isCompactSummaryrow before the new row is flushed; evidencecli/src/claude/utils/compactSummaryLookup.ts:66.
Suggested fix:type CompactSummaryEntry = { uuid: string; text: string } // Snapshot immediately before enqueueing /compact. compactSummaryBaselineUuid = await readLatestCompactSummaryUuid(transcriptPath) // During polling, accept only a newly appended entry. if (entry && entry.uuid !== compactSummaryBaselineUuid) return entry.text
Summary
- Review mode: initial
- Two major correctness issues found: the feature is skipped after the first turn, and repeated/resumed compactions can pair stale summary text with the new token delta.
Testing
- Not run (automation); static review only.
- GitHub
integrationcheck is failing atbun run test:cli:integration; job logs were unavailable while the workflow run remained in progress. The maintestjob was pending at review time. - Missing regression coverage: normal turn followed by
/compact; existing summary followed by a delayed new summary.
HAPI Bot
| ? `Compaction failed: ${compactFailure}` | ||
| : 'Compaction completed'; | ||
| logger.debug(`[claudeRemote] ${completionEvent}`); | ||
| compactSummary = await lookupCompactSummary(compactFailure); |
There was a problem hiding this comment.
[MAJOR] Later-turn /compact never reaches this new path
isCompactCommand is set only by applyInitialTurn(), but after the first result scheduleNextMessage() pushes next.message directly without calling parseSpecialCommand. Therefore a /compact sent after a normal turn leaves this condition false: no boundary suppression, token capture, transcript lookup, or structured summary event. One claudeRemote() call explicitly accepts multiple turns, so this affects the normal in-session case.
Suggested fix:
const enqueueTurn = (next: { message: string; mode: EnhancedMode }) => {
const command = parseSpecialCommand(next.message)
if (command.type === "compact") {
isCompactCommand = true
compactFailure = null
compactTokensBefore = undefined
compactTokensAfter = undefined
opts.onCompletionEvent?.("📦 Compaction started")
}
mode = next.mode
messages.push({ type: "user", message: { role: "user", content: next.message } })
}
// Call enqueueTurn(next) from both applyInitialTurn() and scheduleNextMessage().Add a regression test that sends a normal first turn/result, then /compact, boundary, and result.
| try { | ||
| const content = await readFile(transcriptPath, 'utf8'); | ||
| const summary = extractCompactSummaryFromTranscript(content); | ||
| if (summary !== null) return summary; |
There was a problem hiding this comment.
[MAJOR] This can return the previous compaction summary
The lookup scans the entire transcript and returns as soon as any latest isCompactSummary row exists. A resumed session or a second /compact already has such a row; when the new row is delayed (the exact case this polling handles), the first attempt immediately returns the old summary and the caller emits it with the new boundary token delta.
Suggested fix:
type CompactSummaryEntry = { uuid: string; text: string }
// Capture this immediately before enqueueing /compact.
compactSummaryBaselineUuid = await readLatestCompactSummaryUuid(transcriptPath)
// Poll until Claude appends a different summary entry.
const entry = extractLatestCompactSummaryEntry(content)
if (entry && entry.uuid !== compactSummaryBaselineUuid) return entry.textAdd a test with an old summary present at poll start and a new summary appended after one sleep.
There was a problem hiding this comment.
Findings
- [Major] The transcript baseline is not established before
/compactis enqueued —beginCompactCommand()startsstat()without awaiting it, and both callers push the command immediately afterward. If the result is handled before that promise resolves,compactTranscriptBaselineis still0(or the previous compaction’s offset), sofindLatestCompactSummary()can promote an olderisCompactSummaryrow with the new token delta. A transientstat()failure has the same stale-summary outcome because it is converted to offset0; evidencecli/src/claude/claudeRemote.ts:151.
Suggested fix:let compactTranscriptBaseline: number | null = 0 const getTranscriptBytes = async (): Promise<number | null> => { if (!currentSessionId) return 0 try { return (await stat(transcriptPath)).size } catch { return null } } const beginCompactCommand = async () => { isCompactCommand = true opts.onCompletionEvent?.("📦 Compaction started") compactTranscriptBaseline = await getTranscriptBytes() } // Await this before messages.push(...) in both turn paths. if (specialCommand.type === "compact") await beginCompactCommand() // If the baseline is null, skip summary promotion and use the fallback line.
Summary
- Review mode: follow-up after new commits
- One major correctness issue remains in the stale-summary guard: its asynchronous baseline can still associate a previous summary with the current compaction.
Testing
- Not run (automation); static review only.
- Add a regression test with a pre-existing summary and a deliberately delayed baseline lookup, then verify the new
/compactcannot emit that stale summary. - Current GitHub checks at submission preparation:
integration= FAILURE;test= IN_PROGRESS.
HAPI Bot
| if (opts.onCompletionEvent) { | ||
| opts.onCompletionEvent('📦 Compaction started'); | ||
| } | ||
| void getTranscriptBytes().then((bytes) => { |
There was a problem hiding this comment.
[MAJOR] Baseline capture can still race the compact result
This stat() is fire-and-forget, while both callers immediately enqueue /compact. If Claude reaches the result before this promise resolves, lookupCompactSummary() reads offset 0 (or the prior compact’s offset) and can return the old isCompactSummary row again. Also, a transient stat() failure becomes 0, so a later successful transcript read has the same stale-summary behavior.
Suggested fix:
let compactTranscriptBaseline: number | null = 0
const beginCompactCommand = async () => {
isCompactCommand = true
opts.onCompletionEvent?.("📦 Compaction started")
compactTranscriptBaseline = await getTranscriptBytes() // null on stat failure
}
if (specialCommand.type === "compact") await beginCompactCommand()
messages.push(/* /compact */)
// In lookupCompactSummary:
if (compactTranscriptBaseline === null) return undefinedPlease cover a resumed/second compact with an old summary present and the baseline lookup deliberately delayed.
There was a problem hiding this comment.
Findings
- [Major] Broad
<local-command-*>filtering removes supported CLI output — settingisMetahere causesOutgoingMessageQueueto drop every tagged remote-Claude message beforesendClaudeSessionMessage. The client contract intentionally renders non-compaction slash-command output such as<local-command-stdout>as a CLI-output block, so commands like/contextand plugin commands can now lose their output. Evidencecli/src/claude/utils/sdkToLogConverter.ts:291; related handlingcli/src/claude/utils/OutgoingMessageQueue.ts:125,docs/api/client-contract/messages.md:50.
Suggested fix:Remove the generic converter-level// claudeRemote.ts: suppress only bookkeeping for the active manual compact. const isManualCompactBookkeeping = isCompactCommand && message.type === 'user' && typeof (message as SDKUserMessage).message.content === 'string' && (message as SDKUserMessage).message.content.startsWith('<local-command-') if (!isManualCompactBoundary && !isManualCompactBookkeeping) { opts.onMessage(message) }
isMetarule so other slash-command output still reaches the existing CLI-output renderer.
Summary
Review mode: follow-up after new commits
One major regression remains. The prior transcript-baseline race is not present at the current head.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub checks:
testsucceeded;integrationfailed in four untouchedcli/src/runner/runner.integration.test.tscases. A causal link to this diff was not found. - Add a regression test proving a non-
/compact<local-command-stdout>message is not marked meta and remains renderable.
HAPI Bot
| // not chat content — flag them meta so the same downstream | ||
| // filters hide them instead of leaking raw stdout into the UI. | ||
| const contentText = typeof userMsg.message.content === 'string' ? userMsg.message.content : null | ||
| if (contentText && contentText.startsWith('<local-command-')) { |
There was a problem hiding this comment.
[MAJOR] Scope suppression to the active /compact turn
isMeta messages are dropped by OutgoingMessageQueue before reaching sendClaudeSessionMessage. This predicate therefore removes every remote-Claude <local-command-*> output, including the non-compaction slash-command stdout that docs/api/client-contract/messages.md:50 explicitly routes to the CLI-output renderer.
Suggested fix:
// In claudeRemote.ts, while isCompactCommand is true:
const isManualCompactBookkeeping =
isCompactCommand &&
message.type === 'user' &&
typeof (message as SDKUserMessage).message.content === 'string' &&
(message as SDKUserMessage).message.content.startsWith('<local-command-')
if (!isManualCompactBoundary && !isManualCompactBookkeeping) {
opts.onMessage(message)
}Then remove this generic converter rule and add a regression test showing non-/compact local-command stdout remains visible.
There was a problem hiding this comment.
Findings
- [Major] Poll only the transcript tail -
readFile()allocates and reads the entire append-only session transcript on every 500 ms attempt, and only then discards bytes beforeminBytes. A long-running session can therefore incur up to ten full-file reads/allocations exactly when compaction is requested, causing avoidable I/O stalls or OOM. The repo's existing incremental reader already range-reads from a byte cursor for this reason. Evidencecli/src/claude/utils/compactSummaryLookup.ts:70; related contextcli/src/claude/utils/sessionScanner.ts:201.
Suggested fix:// Factor/reuse the range-read loop from readSessionLog: const chunk = await readFileRange(transcriptPath, minBytes) const summary = extractCompactSummaryFromTranscript(chunk.toString('utf8'))
- [Minor] Scope the "Compacted" suppression to an active /compact turn - the converter has no command-state context, so any supported
<local-command-stdout>Compacted</local-command-stdout>emitted by another slash/plugin command is now marked meta and dropped. The client contract permits arbitrary local-command stdout; the exact text is not a protocol discriminator. Evidencecli/src/claude/utils/sdkToLogConverter.ts:291.
Suggested fix:Remove the converter-level rule.// claudeRemote.ts const isManualCompactStdout = isCompactCommand && message.type === 'user' && typeof (message as SDKUserMessage).message.content === 'string' && /^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test( (message as SDKUserMessage).message.content.trim() ) if (!isManualCompactBoundary && !isManualCompactStdout) opts.onMessage(message)
Summary
Review mode: follow-up after new commits
Two issues remain at the current head: repeated whole-transcript reads during polling, and a content-only suppression rule that can hide unrelated CLI output.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub
integrationfailed in four untouchedcli/src/runner/runner.integration.test.tscases; a causal link to this diff was not found. - GitHub
testwas still in progress at review time. - Add a large-transcript/range-read regression test and a non-/compact exact-
Compactedstdout regression test.
HAPI Bot
| const sleep = opts?.sleep ?? ((ms: number) => new Promise<void>((resolve) => setTimeout(resolve, ms))); | ||
| for (let attempt = 0; attempt < attempts; attempt++) { | ||
| try { | ||
| const buf = await readFile(transcriptPath); |
There was a problem hiding this comment.
[MAJOR] Read only bytes after the baseline
readFile() still allocates the entire append-only transcript before subarray(minBytes). With 10 polling attempts, long sessions can repeatedly reread/allocate the full JSONL and stall or OOM during compaction. sessionScanner.ts already uses a byte-range read loop for this exact scaling concern.
Suggested fix:
// Factor/reuse the range-read loop from readSessionLog:
const chunk = await readFileRange(transcriptPath, minBytes)
const summary = extractCompactSummaryFromTranscript(chunk.toString('utf8'))| // That one echo is CLI plumbing — flag it meta so the downstream | ||
| // filters hide it. Other local-command output (e.g. /context) | ||
| // is rendered intentionally as a CLI-output block by the client | ||
| // contract and must stay visible. |
There was a problem hiding this comment.
[MINOR] Gate suppression on the active /compact turn
This content-only rule also drops a non-/compact slash/plugin command whose legitimate stdout is exactly Compacted. Local-command stdout is otherwise a supported CLI-output payload, so the text alone is not a safe discriminator.
Suggested fix:
// In claudeRemote.ts, where isCompactCommand is available:
const isManualCompactStdout =
isCompactCommand &&
message.type === 'user' &&
typeof (message as SDKUserMessage).message.content === 'string' &&
/^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(
(message as SDKUserMessage).message.content.trim()
)
if (!isManualCompactBoundary && !isManualCompactStdout) opts.onMessage(message)Then remove this converter-level rule.
There was a problem hiding this comment.
Findings
- [Minor] Finish the requested byte range before parsing —
FileHandle.read()may return fewer bytes than requested, but the new polling path ignoresbytesReadand parses the entire zero-filled buffer. A short read can therefore miss the compact-summary row and incorrectly fall back to the plain completion line. The existing session scanner explicitly loops for this case. Evidencecli/src/claude/utils/compactSummaryLookup.ts:78; related contextcli/src/claude/utils/sessionScanner.ts:225.
Suggested fix:let bytesRead = 0 while (bytesRead < buf.length) { const result = await handle.read(buf, bytesRead, buf.length - bytesRead, minBytes + bytesRead) if (result.bytesRead === 0) break bytesRead += result.bytesRead } const summary = extractCompactSummaryFromTranscript(buf.subarray(0, bytesRead).toString('utf8'))
- [Minor] Scope the literal
Compactedsuppression to an active/compactturn — the converter has no command-state context, so any other slash/plugin command whose supported local-command stdout is exactlyCompactedis now marked meta and hidden. Evidencecli/src/claude/utils/sdkToLogConverter.ts:293.
Suggested fix:Remove the converter-level content-only rule.// claudeRemote.ts, where isCompactCommand is available const isManualCompactStdout = isCompactCommand && message.type === 'user' && typeof (message as SDKUserMessage).message.content === 'string' && /^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test( (message as SDKUserMessage).message.content.trim() ) if (!isManualCompactBoundary && !isManualCompactStdout) opts.onMessage(message)
Summary
Review mode: follow-up after new commits
The previous whole-transcript polling issue is fixed by range reads. Two correctness gaps remain: incomplete range reads can miss the summary, and the literal stdout suppression can hide unrelated command output.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub
testpassed. - GitHub
integrationfailed in four untouchedcli/src/runner/runner.integration.test.tscases; a causal link to this diff was not found. - Add a mocked short-read regression test and a non-
/compactexact-Compactedstdout regression test.
HAPI Bot
| const size = (await handle.stat()).size; | ||
| if (size > minBytes) { | ||
| const buf = Buffer.alloc(size - minBytes); | ||
| await handle.read(buf, 0, buf.length, minBytes); |
There was a problem hiding this comment.
[MINOR] Finish the range read before parsing
FileHandle.read() may return fewer bytes than requested. This call ignores bytesRead, so a short read leaves the rest of the allocated buffer zero-filled; the parser can then miss a later compact-summary row and incorrectly fall back to the plain completion line. The existing sessionScanner.ts range reader loops until the requested range is filled or EOF.
Suggested fix:
let bytesRead = 0
while (bytesRead < buf.length) {
const result = await handle.read(buf, bytesRead, buf.length - bytesRead, minBytes + bytesRead)
if (result.bytesRead === 0) break
bytesRead += result.bytesRead
}
const summary = extractCompactSummaryFromTranscript(
buf.subarray(0, bytesRead).toString('utf8')
)| // is rendered intentionally as a CLI-output block by the client | ||
| // contract and must stay visible. | ||
| const contentText = typeof userMsg.message.content === 'string' ? userMsg.message.content : null | ||
| if (contentText && /^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(contentText.trim())) { |
There was a problem hiding this comment.
[MINOR] Gate suppression on the active /compact turn
This converter has no command-state context, so any other slash/plugin command whose valid local-command stdout is exactly Compacted is also marked meta and hidden. Move this check to claudeRemote.ts, where isCompactCommand is available, and remove the converter-level rule.
Suggested fix:
const isManualCompactStdout =
isCompactCommand &&
message.type === 'user' &&
typeof (message as SDKUserMessage).message.content === 'string' &&
/^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(
(message as SDKUserMessage).message.content.trim()
)
if (!isManualCompactBoundary && !isManualCompactStdout) {
opts.onMessage(message)
}There was a problem hiding this comment.
Findings
- [Minor] Refresh the context-usage sample after compaction — the new event displays
post_tokens, but it does not carryusage; the web reducer therefore keeps the last pre-compaction usage until Claude produces another model response. This can leave the status bar showing ~34k while the adjacent card says ~2k. Evidencecli/src/claude/claudeRemoteLauncher.ts:526; related contextweb/src/chat/reducer.ts:177andcli/src/pi/runPi.ts:1063.
Suggested fix:// Preserve compactTokensAfter through onReady, even when no summary was found. const usage = compactContextTokens === undefined ? null : convertAgentMessage({ type: 'usage', inputTokens: 0, outputTokens: 0, contextTokens: compactContextTokens }, session.getModel()) if (usage) session.client.sendAgentMessage(usage)
- [Minor] Scope the literal
Compactedsuppression to the active/compactturn — this converter has no command-state context, so a user-authored literal or another local command returning exactly this stdout is marked meta and silently hidden. Evidencecli/src/claude/utils/sdkToLogConverter.ts:293.
Suggested fix:Remove the converter-level content-only rule.// claudeRemote.ts, while isCompactCommand is available const isManualCompactStdout = isCompactCommand && message.type === 'user' && typeof (message as SDKUserMessage).message.content === 'string' && /^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test( (message as SDKUserMessage).message.content.trim() ) opts.onMessage(isManualCompactStdout ? { ...message, isMeta: true } : message)
Summary
Review mode: follow-up after new commits
The range-read loop from the prior review is now present. Two Minor gaps remain: the post-compaction context sample is not propagated to the status bar, and the global literal-output suppression can hide unrelated content.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub
testpassed. - GitHub
integrationfailed in four untouchedcli/src/runner/runner.integration.test.tscases; a causal link to this diff was not found. - Add regressions for the post-compaction usage update, exact
Compactedtext outside/compact, and mocked shortFileHandle.read()results.
HAPI Bot
| } | ||
| if (compactSummary) { | ||
| logger.debug(`[remote]: Compact summary promoted (${compactSummary.summary.length} chars)`); | ||
| session.client.sendSessionEvent({ |
There was a problem hiding this comment.
[MINOR] Refresh context usage after compaction
This event displays tokensAfter, but it does not create a usage-bearing message. reduceChatMessages() chooses latestUsage only from msg.usage, while the compact result is documented here as all-zero, so the status bar keeps the old pre-compaction context until the next model response. Pi handles the same gap by emitting a context-only usage update after its summary.
Suggested fix:
// Preserve compactTokensAfter through onReady, even without a summary.
const usage = compactContextTokens === undefined ? null : convertAgentMessage({
type: 'usage',
inputTokens: 0,
outputTokens: 0,
contextTokens: compactContextTokens
}, session.getModel())
if (usage) session.client.sendAgentMessage(usage)| // is rendered intentionally as a CLI-output block by the client | ||
| // contract and must stay visible. | ||
| const contentText = typeof userMsg.message.content === 'string' ? userMsg.message.content : null | ||
| if (contentText && /^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(contentText.trim())) { |
There was a problem hiding this comment.
[MINOR] Gate suppression on the active /compact turn
This content-only check has no command-state context. A user-authored literal—or another local command whose valid stdout is exactly Compacted—is therefore marked meta and silently hidden. Mark the SDK message meta in claudeRemote.ts, where isCompactCommand is available, and remove this global converter rule.
Suggested fix:
const isManualCompactStdout =
isCompactCommand &&
message.type === 'user' &&
typeof (message as SDKUserMessage).message.content === 'string' &&
/^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(
(message as SDKUserMessage).message.content.trim()
)
opts.onMessage(isManualCompactStdout ? { ...message, isMeta: true } : message)There was a problem hiding this comment.
Findings
- [Minor] Scope the literal
Compactedsuppression to the active/compactturn — the converter has no command-state context, so any other slash/plugin command whose legitimate stdout is exactlyCompactedis marked meta and silently removed byOutgoingMessageQueue. Evidencecli/src/claude/utils/sdkToLogConverter.ts:293.
Suggested fix:Remove the converter-level content-only rule.// claudeRemote.ts, before opts.onMessage(message) const content = message.type === 'user' && typeof message.message.content === 'string' ? message.message.content.trim() : null const isManualCompactStdout = isCompactCommand && content !== null && /^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(content) if (!isManualCompactBoundary && !isManualCompactStdout) { opts.onMessage(message) }
Summary
Review mode: follow-up after new commits
The new commits resolve the prior post-compaction context-usage finding. The content-only stdout suppression remains unresolved; no additional findings met the 80% confidence threshold.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub
integrationfailed inbun run test:cli:integration; logs were unavailable while the workflow run remained active, so a causal link was not established. - GitHub
testwas still in progress at review time. - Add a regression proving exact
Compactedstdout remains visible outside an active/compactturn.
HAPI Bot
| // is rendered intentionally as a CLI-output block by the client | ||
| // contract and must stay visible. | ||
| const contentText = typeof userMsg.message.content === 'string' ? userMsg.message.content : null | ||
| if (contentText && /^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(contentText.trim())) { |
There was a problem hiding this comment.
[MINOR] Gate suppression on the active /compact turn
This content-only predicate has no command-state context. A non-/compact slash/plugin command whose valid <local-command-stdout> is exactly Compacted is therefore marked meta and dropped by OutgoingMessageQueue, even though local-command stdout is a supported CLI-output payload.
Suggested fix:
// claudeRemote.ts, before opts.onMessage(message)
const content = message.type === 'user' && typeof message.message.content === 'string'
? message.message.content.trim()
: null
const isManualCompactStdout =
isCompactCommand &&
content !== null &&
/^<local-command-stdout>\s*Compacted\s*<\/local-command-stdout>$/.test(content)
if (!isManualCompactBoundary && !isManualCompactStdout) {
opts.onMessage(message)
}Remove this converter-level rule and add a non-/compact regression.
There was a problem hiding this comment.
Findings
- [Minor] Arm later-turn compaction only when the command is enqueued —
beginCompactCommand()setsisCompactCommandbefore its awaited transcriptstat; because the response loop intentionally continues consuming autonomous messages after a result, a result arriving in that window is consumed as the compact result and clears the state before/compactis pushed. Evidencecli/src/claude/claudeRemote.ts:321(related stream behavior atcli/src/claude/claudeRemote.ts:469).
Suggested fix:const baseline = specialCommand.type === 'compact' ? await getTranscriptBytes() : null if (specialCommand.type === 'compact') { isCompactCommand = true compactTranscriptBaseline = baseline opts.onCompletionEvent?.('📦 Compaction started') } messages.push({ type: 'user', message: { role: 'user', content: next.message } })
Summary
Review mode: follow-up after new commits
The new commits resolve the prior content-only stdout-suppression finding. One newly exposed concurrency race remains in later-turn compact arming.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub checks at submission: test=SUCCESS, integration=FAILURE, pr-review=IN_PROGRESS. Integration failure logs were unavailable while the workflow run was still active, so a causal link was not established.
- Add a regression that delays transcript baseline capture while an autonomous post-result message arrives, and assert that no compact completion/state reset occurs before
/compactis enqueued.
HAPI Bot
| // /compact can arrive on any turn, not just the initial | ||
| // one — arm the compaction tracking here too so later | ||
| // turns get the same summary/token completion output. | ||
| await beginCompactCommand(); |
There was a problem hiding this comment.
[MINOR] Arm compact state only when the command is enqueued
beginCompactCommand() sets isCompactCommand = true and then awaits the transcript stat. On this later-turn call, /compact has not been pushed yet. The response loop is explicitly designed to keep consuming autonomous messages after a result, so another result can land during that await; the result handler then treats it as the compact result, emits/clears compact state, and the real /compact loses boundary/stdout suppression and summary promotion.
Suggested fix:
const baseline = specialCommand.type === 'compact'
? await getTranscriptBytes()
: null
if (specialCommand.type === 'compact') {
isCompactCommand = true
compactTranscriptBaseline = baseline
opts.onCompletionEvent?.('📦 Compaction started')
}
messages.push({ type: 'user', message: { role: 'user', content: next.message } })Keep the state assignment and messages.push() in the same synchronous continuation, with no await between them.
There was a problem hiding this comment.
Findings
- [Minor] Publish the compact outcome before accepting the next turn — the new background lookup waits for
onReady, andscheduleNextMessage()then consumes queued input before the summary/fallback is emitted. Because chat order usesinvokedAt ?? createdAt, a delayed transcript flush can place “Context compacted” below the following user turn instead of beside the/compactresult. Evidencecli/src/claude/claudeRemote.ts:487(ready/input release atcli/src/claude/claudeRemote.ts:507andcli/src/claude/claudeRemote.ts:516).
Suggested fix:If the result carrier must flush immediately, split that flush into a separate callback; keep the completion/summary before the ready event.// Keep SDK response consumption non-blocking, but do not announce ready or // consume the next prompt until the compact outcome has been published. const completion = (async () => { const summary = await lookupCompactSummary( failure, sessionId, baseline, tokensBefore, tokensAfter ) const event = summary ? undefined : buildCompactCompletionEvent(failure, tokensBefore, tokensAfter) await opts.onReady(event, summary, tokensAfter) scheduleNextMessage() })()
Summary
Review mode: follow-up after new commits
The prior compact-arming race is no longer present. One ordering regression remains in the asynchronous completion path.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub checks at submission: test=SUCCESS; integration=FAILURE. The failed job reports four runner session-spawn/tracking assertions in
cli/src/runner/runner.integration.test.ts; no causal link to these changed paths was established. - Add a regression with a deferred summary lookup and an already queued next prompt; assert compact summary/fallback is emitted before ready and before that prompt is consumed.
HAPI Bot
| tokensBefore, | ||
| tokensAfter | ||
| ); | ||
| await readyFlushed; |
There was a problem hiding this comment.
[MINOR] Emit the compact outcome before ready accepts another turn
This waits for onReady before publishing the summary/fallback. The launcher sends the ready event inside that callback, and line 516 immediately starts fetching the next queued prompt. When the transcript row takes any of the allowed 5 seconds to appear, that prompt is consumed and timestamped before onCompactSummary / onCompletionEvent, so the card lands below the following user turn.
Suggested fix:
const completion = (async () => {
const summary = await lookupCompactSummary(
failure, sessionId, baseline, tokensBefore, tokensAfter
)
const event = summary
? undefined
: buildCompactCompletionEvent(failure, tokensBefore, tokensAfter)
await opts.onReady(event, summary, tokensAfter)
scheduleNextMessage()
})()Run this task in the background so SDK response consumption remains non-blocking; if result flushing must stay immediate, split the flush from the ready announcement.
There was a problem hiding this comment.
Findings
- [Major] Stop a deferred compact completion before retrying the failed stream — the completion promise is detached and only awaited after normal stream exhaustion. If response iteration throws while transcript polling is pending,
claudeRemotereturns immediately; the launcher restores/compactfor retry, while the old promise can later callscheduleNextMessage()and dequeue the following queued prompt into the abandonedmessagesiterable. This can duplicate compaction and lose the next prompt. Evidencecli/src/claude/claudeRemote.ts:490; related retry restorationcli/src/claude/claudeRemoteLauncher.ts:652.
Suggested fix:let responseFailed = false // Inside the deferred completion: await opts.onReady(completionEvent, compactSummary, tokensAfter) if (!responseFailed) scheduleNextMessage() // Before rethrowing a response-stream error: responseFailed = true await Promise.allSettled(pendingCompactCompletions)
Summary
Review mode: follow-up after new commits
One Major lifecycle race remains in the asynchronous compact-completion path. The prior output-before-ready ordering finding is resolved at this head.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub checks at submission preparation:
integration= FAILURE;test= IN_PROGRESS. Failure logs were unavailable while the workflow run remained active, so no causal link was established. - Add a regression where the SDK generator yields the compact result, then throws before a deferred summary lookup resolves; assert the compact command is not restored/replayed and the next queued prompt is not consumed by the failed attempt.
HAPI Bot
| scheduleNextMessage(); | ||
| })(); | ||
| pendingCompactCompletions.add(completion); | ||
| void completion |
There was a problem hiding this comment.
[MAJOR] Stop this deferred completion before retrying a failed SDK stream
This promise is detached, and the outer catch does not await it. If the response iterator throws while the transcript lookup is pending, the launcher restores /compact and starts another attempt; this old promise can then resolve and call scheduleNextMessage(), dequeuing the following prompt into an abandoned messages iterable. That can duplicate the compaction and lose queued input.
Suggested fix:
let responseFailed = false
// In this completion:
await opts.onReady(completionEvent, compactSummary, tokensAfter)
if (!responseFailed) scheduleNextMessage()
// In the outer catch, before rethrow:
responseFailed = true
await Promise.allSettled(pendingCompactCompletions)Please add a regression where the SDK stream throws after the compact result but before a deferred summary resolves.
There was a problem hiding this comment.
Findings
- [Major] Propagate failures from the deferred compact completion —
opts.onReady()now runs in a detached promise, but its rejection is only logged. The next-message fetch is therefore never scheduled and the still-open prompt/response loop can wait forever, leaving queued input stuck. Evidencecli/src/claude/claudeRemote.ts:503; callback work that can reject starts atcli/src/claude/claudeRemoteLauncher.ts:519.
Suggested fix:let compactCompletionError: Error | null = null void completion .finally(() => pendingCompactCompletions.delete(completion)) .catch((error) => { compactCompletionError = error instanceof Error ? error : new Error(String(error)) messages.setError(compactCompletionError) }) // After awaiting pending completions on normal stream exhaustion: if (compactCompletionError) throw compactCompletionError
Summary
Review mode: follow-up after new commits
One Major lifecycle issue remains. The prior deferred-completion retry race is resolved at this head, but callback failures in the same detached path are swallowed and can hang the remote session.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub checks:
testpassed;integrationfailed in unrelated runner spawn/session assertions. No causal link to the touched files was established. - Add a regression where compact summary lookup resolves and
onReadyrejects; assertclaudeRemoterejects/relaunches and does not leave the next prompt waiting.
HAPI Bot
| pendingCompactCompletions.add(completion); | ||
| void completion | ||
| .finally(() => pendingCompactCompletions.delete(completion)) | ||
| .catch((error) => logger.debug('[claudeRemote] compact completion callback failed', error)); |
There was a problem hiding this comment.
[MAJOR] Propagate deferred callback failures instead of swallowing them
opts.onReady() is now inside a detached promise. If messageQueue.flush() or later callback work rejects, this catch only logs it; scheduleNextMessage() is skipped, messages stays open, and the SDK response loop can wait forever with the next queued prompt never consumed. Non-compact results still propagate onReady errors through the outer catch.
Suggested fix:
let compactCompletionError: Error | null = null
void completion
.finally(() => pendingCompactCompletions.delete(completion))
.catch((error) => {
compactCompletionError = error instanceof Error ? error : new Error(String(error))
messages.setError(compactCompletionError)
})
// After awaiting pending completions on normal stream exhaustion:
if (compactCompletionError) throw compactCompletionErrorPlease add a regression where summary lookup resolves and onReady rejects; the call should reject/relaunch rather than hang.
There was a problem hiding this comment.
Findings
- [Major] Do not turn cancellation into a successful compact completion - an aborted lookup returns no summary, then the new coordinator builds the normal success fallback and can publish it before the response iterator surfaces AbortError. This can emit a false "Compacted" event and reopen input during a user abort or session switch. Evidence
cli/src/claude/claudeRemote.ts:527.
Suggested fix:const compactSummary = await lookupCompactSummary(/* ... */) if (compactCompletionAbort.signal.aborted) { throw new AbortError('Compaction completion aborted') }
- [Major] Correlate the result with an observed compact signal -
compactState.activeis armed before the prompt is pushed, but this branch claims any nextresult. The same stream explicitly supports autonomous results, so one arriving before the compact status/boundary clears the compact state, polls too early, relays the real boundary, and may admit the next prompt before compaction finishes. Evidencecli/src/claude/claudeRemote.ts:505.
Suggested fix:// ActiveCompact sawCompactSignal: boolean // Set true on status === 'compacting', compact_result, or a manual boundary. if (compactState.active && !compactState.active.sawCompactSignal) { continue // unrelated autonomous result }
- [Minor] Preserve an empty failure reason for the generic formatter - a failed status without
compact_errorstores"Compaction failed"; the formatter then produces"Compaction failed: Compaction failed". Evidencecli/src/claude/claudeRemote.ts:479.
Suggested fix:compactState.active.failure = reason
Summary
Review mode: follow-up after new commits
The prior deferred onReady failure is fixed. Two Major lifecycle races and one Minor fallback-formatting issue remain at the latest reviewed head.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub checks:
testpassed;integrationfailed in unchanged runner spawn/session assertions. No causal link to the touched files was established. - Add regressions for aborting while summary polling is pending, an autonomous
resultbetween compact enqueue and compact status/boundary, and failed compact status withoutcompact_error.
HAPI Bot
| tokensAfter, | ||
| compactCompletionAbort.signal | ||
| ); | ||
| const completionEvent = compactSummary |
There was a problem hiding this comment.
[MAJOR] Do not publish compact success after cancellation
findLatestCompactSummary returns null when compactCompletionAbort fires. This line then builds the normal success fallback, so if that promise wins the race before pendingResponseNext surfaces its AbortError, onReady publishes a false "Compacted" event and scheduleNextMessage() reopens input during a user abort/session switch.
Suggested fix:
const compactSummary = await lookupCompactSummary(/* ... */)
if (compactCompletionAbort.signal.aborted) {
throw new AbortError('Compaction completion aborted')
}Please add a regression that aborts the external signal while summary polling is pending and asserts no completion/ready event and no next-message fetch.
| logger.debug(`[claudeRemote] ${completionEvent}`); | ||
| isCompactCommand = false; | ||
| compactFailure = null; | ||
| if (compactState.active) { |
There was a problem hiding this comment.
[MAJOR] Correlate this result with an observed compact signal
compactState.active is armed before the prompt is pushed, but this branch claims any next result. The stream explicitly supports unrelated autonomous results (see the handling below at lines 536-546), so one arriving between enqueue and the compact status/boundary clears the state and starts transcript polling too early. The real manual boundary is then relayed, its token delta is missed, and the next prompt can be admitted before compaction actually completes.
Suggested fix:
// ActiveCompact
sawCompactSignal: boolean
// Set true on status === 'compacting', compact_result, or a manual boundary.
if (compactState.active && !compactState.active.sawCompactSignal) {
continue // unrelated autonomous result
}Add a regression with an autonomous result before the compact status/boundary; only the later compact result should complete the command.
| : ''; | ||
| compactFailure = reason.length > 0 ? reason : 'Compaction failed'; | ||
| logger.debug(`[claudeRemote] Compaction reported as failed: ${compactFailure}`); | ||
| compactState.active.failure = reason.length > 0 ? reason : 'Compaction failed'; |
There was a problem hiding this comment.
[MINOR] Keep the generic failure sentinel empty
When compact_result === 'failed' has no compact_error, this stores "Compaction failed". buildCompactCompletionEvent treats every non-empty value as a reason and renders "Compaction failed: Compaction failed", while its empty-string branch is the intended generic fallback.
Suggested fix:
compactState.active.failure = reasonAdd the no-compact_error case to the end-to-end claudeRemote result-reporting tests.
There was a problem hiding this comment.
Findings
- [Major] Do not retry
/compactafter its result was already received — while transcript polling is pending, a rejection from the next response read bypassesonReady, so the launcher still considers the command in flight and restores it for another attempt. That can compact an already-compacted conversation again and progressively discard context. Evidencecli/src/claude/claudeRemote.ts:373; retry contextcli/src/claude/claudeRemoteLauncher.ts:652.
Suggested fix:const responseOutcome = pendingResponseNext.then( result => ({ type: 'response' as const, result }), error => ({ type: 'response-error' as const, error }) ) // If response-error wins after the compact result, await/publish // compactCompletion first, then rethrow the stream error.
Summary
Review mode: follow-up after new commits
The three prior HAPI Bot findings are addressed. One Major retry/data-loss risk remains at the latest reviewed head.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- Current GitHub checks:
integrationfailed;testand thispr-reviewrun were pending when reviewed. Failed-job logs were not yet available because the workflow run was still in progress. - Add a regression where the stream throws after the compact
resultwhile summary lookup is pending; assert the compact outcome reachesonReadybefore the stream error propagates, and that the launcher does not restore/compact.
HAPI Bot
| if (compactCompletion) { | ||
| const winner = await Promise.race([ | ||
| compactCompletion.then((result) => ({ type: 'compact' as const, result })), | ||
| pendingResponseNext.then((result) => ({ type: 'response' as const, result })) |
There was a problem hiding this comment.
[MAJOR] Publish the compact result before propagating later stream errors
Once the compact result has arrived, the command has already executed. If pendingResponseNext rejects while the optional transcript lookup is pending, this Promise.race rejects directly; the outer catch aborts compactCompletion and never calls onReady. In the real launcher, inFlightMessage therefore remains set and restoreInFlightMessage() requeues /compact (claudeRemoteLauncher.ts:652), so a transient post-result stream failure runs compaction again and can discard additional context. The added test at claudeRemote.test.ts:1115 currently codifies this retry-triggering behavior.
Suggested fix:
const responseOutcome = pendingResponseNext.then(
result => ({ type: 'response' as const, result }),
error => ({ type: 'response-error' as const, error })
)
const winner = await Promise.race([
compactCompletion.then(result => ({ type: 'compact' as const, result })),
responseOutcome
])
if (winner.type === 'response-error') {
const completion = await compactCompletion
compactCompletion = null
await opts.onReady(
completion.completionEvent,
completion.compactSummary,
completion.contextTokens
)
throw winner.error
}Update the regression to expect one ready/completion publication before the stream error, plus a launcher-level assertion that /compact is not restored.
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits
No new issues found at the current head. The prior Major retry/data-loss finding is addressed by marking the compact result accepted before deferred transcript lookup completes, publishing the compact outcome before propagating a later stream error, and retaining the next queued prompt. Residual risk is limited to timing behavior outside the covered stream-failure and simultaneous-settlement regressions.
Testing
- Not run (automation); PR code was not executed under the review security constraints.
- GitHub test check passed.
- GitHub integration check failed in cli/src/runner/runner.integration.test.ts on runner spawn/session assertions; no direct attribution to the changed Claude/web compaction paths was found in the failure log.
HAPI Bot
Problem
/compactin a Claude session renders poorly in the web chat: two plain status lines ("Compaction started" / "Compaction completed") with no icon, no token info, and no summary — even though Claude Code writes a full compaction summary to its local transcript. On top of that, the slash-command bookkeeping leaks into the chat as a raw<local-command-stdout>Compacted </local-command-stdout>terminal block, and thecompact_boundarysystem message renders an extra "Conversation compacted" line next to the completion status.Other flavors already do better: Pi renders a dedicated compact-summary card with the summary markdown and a token delta, and Codex has a structured
context_compactedevent. This brings the Claude flavor to the same level and reuses the existingcompact-summaryevent contract end to end.Solution
compact_boundarymetadata (pre_tokens/post_tokens— the result message that follows a compact reports all-zero usage, so the boundary is the only real source):📦 Compacted (34,176 → 1,576 tokens).isCompactSummaryentry from the session transcript (a pattern this file already uses for session-init detection) and emits the existingcompact-summarysession event, so the web renders the same dedicated card Pi gets. A bounded 5s poll covers the transcript being written slightly after the result; without a summary it falls back to the token-delta line.📦 Context compacted · before → after tokens) carries the essential info. This applies to Pi's card too, for consistency.<local-command-*>bookkeeping are flagged meta so they stop leaking into the chat./compact, thecompact_boundarysystem message is no longer relayed, since its only web rendering would duplicate the completion output. Auto-compact boundaries are unaffected.Screenshots
Tests
<local-command-*>meta flagging, the boundary relay suppression, and the collapsed/expanded card rendering.bun typecheckandbun run testpass across all packages./compactrenders the started line, stores acompact-summaryevent with the transcript summary, and the card collapses/expands as expected with no duplicate status line.