Skip to content

feat(cli): show Claude compaction as a summary card with token delta - #1691

Open
junmo-kim wants to merge 19 commits into
tiann:mainfrom
junmo-kim:feat/claude-compact-ux
Open

feat(cli): show Claude compaction as a summary card with token delta#1691
junmo-kim wants to merge 19 commits into
tiann:mainfrom
junmo-kim:feat/claude-compact-ux

Conversation

@junmo-kim

Copy link
Copy Markdown
Contributor

Problem

/compact in 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 the compact_boundary system 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_compacted event. This brings the Claude flavor to the same level and reuses the existing compact-summary event contract end to end.

Solution

  • The completion status is now a token-delta line built from the compact_boundary metadata (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).
  • On success, the CLI reads the last isCompactSummary entry from the session transcript (a pattern this file already uses for session-init detection) and emits the existing compact-summary session 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.
  • The summary card is now collapsed by default with a chevron toggle — compaction summaries are long, and the header (📦 Context compacted · before → after tokens) carries the essential info. This applies to Pi's card too, for consistency.
  • User-role messages that are Claude Code's <local-command-*> bookkeeping are flagged meta so they stop leaking into the chat.
  • During a manual /compact, the compact_boundary system message is no longer relayed, since its only web rendering would duplicate the completion output. Auto-compact boundaries are unaffected.

Screenshots

Before After
before in-progress
Collapsed by default Expanded
collapsed expanded

Tests

  • New unit tests for the completion-event builder (success / no-summary / token fallback / failure), the <local-command-*> meta flagging, the boundary relay suppression, and the collapsed/expanded card rendering.
  • bun typecheck and bun run test pass across all packages.
  • Manually verified end to end against a live isolated hub + runner + Claude (Haiku) session: /compact renders the started line, stores a compact-summary event with the transcript summary, and the card collapses/expands as expected with no duplicate status line.

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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Later-turn /compact never enters the new summary path — command detection only runs for the initial turn, while subsequent turns are pushed directly; evidence cli/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 isCompactSummary row before the new row is flushed; evidence cli/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 integration check is failing at bun run test:cli:integration; job logs were unavailable while the workflow run remained in progress. The main test job was pending at review time.
  • Missing regression coverage: normal turn followed by /compact; existing summary followed by a delayed new summary.

HAPI Bot

Comment thread cli/src/claude/claudeRemote.ts Outdated
? `Compaction failed: ${compactFailure}`
: 'Compaction completed';
logger.debug(`[claudeRemote] ${completionEvent}`);
compactSummary = await lookupCompactSummary(compactFailure);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.text

Add a test with an old summary present at poll start and a new summary appended after one sleep.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] The transcript baseline is not established before /compact is enqueued — beginCompactCommand() starts stat() without awaiting it, and both callers push the command immediately afterward. If the result is handled before that promise resolves, compactTranscriptBaseline is still 0 (or the previous compaction’s offset), so findLatestCompactSummary() can promote an older isCompactSummary row with the new token delta. A transient stat() failure has the same stale-summary outcome because it is converted to offset 0; evidence cli/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 /compact cannot emit that stale summary.
  • Current GitHub checks at submission preparation: integration = FAILURE; test = IN_PROGRESS.

HAPI Bot

Comment thread cli/src/claude/claudeRemote.ts Outdated
if (opts.onCompletionEvent) {
opts.onCompletionEvent('📦 Compaction started');
}
void getTranscriptBytes().then((bytes) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 undefined

Please cover a resumed/second compact with an old summary present and the baseline lookup deliberately delayed.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Broad <local-command-*> filtering removes supported CLI output — setting isMeta here causes OutgoingMessageQueue to drop every tagged remote-Claude message before sendClaudeSessionMessage. The client contract intentionally renders non-compaction slash-command output such as <local-command-stdout> as a CLI-output block, so commands like /context and plugin commands can now lose their output. Evidence cli/src/claude/utils/sdkToLogConverter.ts:291; related handling cli/src/claude/utils/OutgoingMessageQueue.ts:125, docs/api/client-contract/messages.md:50.
    Suggested fix:
    // 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)
    }
    Remove the generic converter-level isMeta rule 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: test succeeded; integration failed in four untouched cli/src/runner/runner.integration.test.ts cases. 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-')) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 before minBytes. 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. Evidence cli/src/claude/utils/compactSummaryLookup.ts:70; related context cli/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. Evidence cli/src/claude/utils/sdkToLogConverter.ts:291.
    Suggested fix:
    // 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)
    Remove the converter-level rule.

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 integration failed in four untouched cli/src/runner/runner.integration.test.ts cases; a causal link to this diff was not found.
  • GitHub test was still in progress at review time.
  • Add a large-transcript/range-read regression test and a non-/compact exact-Compacted stdout 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Finish the requested byte range before parsing — FileHandle.read() may return fewer bytes than requested, but the new polling path ignores bytesRead and 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. Evidence cli/src/claude/utils/compactSummaryLookup.ts:78; related context cli/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 Compacted suppression to an active /compact turn — the converter has no command-state context, so any other slash/plugin command whose supported local-command stdout is exactly Compacted is now marked meta and hidden. Evidence cli/src/claude/utils/sdkToLogConverter.ts:293.
    Suggested fix:
    // 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)
    Remove the converter-level content-only rule.

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 test passed.
  • GitHub integration failed in four untouched cli/src/runner/runner.integration.test.ts cases; a causal link to this diff was not found.
  • Add a mocked short-read regression test and a non-/compact exact-Compacted stdout 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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())) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)
}

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Refresh the context-usage sample after compaction — the new event displays post_tokens, but it does not carry usage; 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. Evidence cli/src/claude/claudeRemoteLauncher.ts:526; related context web/src/chat/reducer.ts:177 and cli/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 Compacted suppression to the active /compact turn — 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. Evidence cli/src/claude/utils/sdkToLogConverter.ts:293.
    Suggested fix:
    // 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)
    Remove the converter-level content-only rule.

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 test passed.
  • GitHub integration failed in four untouched cli/src/runner/runner.integration.test.ts cases; a causal link to this diff was not found.
  • Add regressions for the post-compaction usage update, exact Compacted text outside /compact, and mocked short FileHandle.read() results.

HAPI Bot

}
if (compactSummary) {
logger.debug(`[remote]: Compact summary promoted (${compactSummary.summary.length} chars)`);
session.client.sendSessionEvent({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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())) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Scope the literal Compacted suppression to the active /compact turn — the converter has no command-state context, so any other slash/plugin command whose legitimate stdout is exactly Compacted is marked meta and silently removed by OutgoingMessageQueue. Evidence cli/src/claude/utils/sdkToLogConverter.ts:293.
    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 the converter-level content-only rule.

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 integration failed in bun run test:cli:integration; logs were unavailable while the workflow run remained active, so a causal link was not established.
  • GitHub test was still in progress at review time.
  • Add a regression proving exact Compacted stdout remains visible outside an active /compact turn.

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())) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Arm later-turn compaction only when the command is enqueued — beginCompactCommand() sets isCompactCommand before its awaited transcript stat; 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 /compact is pushed. Evidence cli/src/claude/claudeRemote.ts:321 (related stream behavior at cli/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 /compact is enqueued.

HAPI Bot

Comment thread cli/src/claude/claudeRemote.ts Outdated
// /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();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Publish the compact outcome before accepting the next turn — the new background lookup waits for onReady, and scheduleNextMessage() then consumes queued input before the summary/fallback is emitted. Because chat order uses invokedAt ?? createdAt, a delayed transcript flush can place “Context compacted” below the following user turn instead of beside the /compact result. Evidence cli/src/claude/claudeRemote.ts:487 (ready/input release at cli/src/claude/claudeRemote.ts:507 and cli/src/claude/claudeRemote.ts:516).
    Suggested fix:
    // 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()
    })()
    If the result carrier must flush immediately, split that flush into a separate callback; keep the completion/summary before the ready event.

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

Comment thread cli/src/claude/claudeRemote.ts Outdated
tokensBefore,
tokensAfter
);
await readyFlushed;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, claudeRemote returns immediately; the launcher restores /compact for retry, while the old promise can later call scheduleNextMessage() and dequeue the following queued prompt into the abandoned messages iterable. This can duplicate compaction and lose the next prompt. Evidence cli/src/claude/claudeRemote.ts:490; related retry restoration cli/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

Comment thread cli/src/claude/claudeRemote.ts Outdated
scheduleNextMessage();
})();
pendingCompactCompletions.add(completion);
void completion

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. Evidence cli/src/claude/claudeRemote.ts:503; callback work that can reject starts at cli/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: test passed; integration failed in unrelated runner spawn/session assertions. No causal link to the touched files was established.
  • Add a regression where compact summary lookup resolves and onReady rejects; assert claudeRemote rejects/relaunches and does not leave the next prompt waiting.

HAPI Bot

Comment thread cli/src/claude/claudeRemote.ts Outdated
pendingCompactCompletions.add(completion);
void completion
.finally(() => pendingCompactCompletions.delete(completion))
.catch((error) => logger.debug('[claudeRemote] compact completion callback failed', error));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 compactCompletionError

Please add a regression where summary lookup resolves and onReady rejects; the call should reject/relaunch rather than hang.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.active is armed before the prompt is pushed, but this branch claims any next result. 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. Evidence cli/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_error stores "Compaction failed"; the formatter then produces "Compaction failed: Compaction failed". Evidence cli/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: test passed; integration failed 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 result between compact enqueue and compact status/boundary, and failed compact status without compact_error.

HAPI Bot

tokensAfter,
compactCompletionAbort.signal
);
const completionEvent = compactSummary

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Comment thread cli/src/claude/claudeRemote.ts Outdated
: '';
compactFailure = reason.length > 0 ? reason : 'Compaction failed';
logger.debug(`[claudeRemote] Compaction reported as failed: ${compactFailure}`);
compactState.active.failure = reason.length > 0 ? reason : 'Compaction failed';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 = reason

Add the no-compact_error case to the end-to-end claudeRemote result-reporting tests.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Do not retry /compact after its result was already received — while transcript polling is pending, a rejection from the next response read bypasses onReady, 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. Evidence cli/src/claude/claudeRemote.ts:373; retry context cli/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: integration failed; test and this pr-review run 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 result while summary lookup is pending; assert the compact outcome reaches onReady before the stream error propagates, and that the launcher does not restore /compact.

HAPI Bot

Comment thread cli/src/claude/claudeRemote.ts Outdated
if (compactCompletion) {
const winner = await Promise.race([
compactCompletion.then((result) => ({ type: 'compact' as const, result })),
pendingResponseNext.then((result) => ({ type: 'response' as const, result }))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant