Skip to content

feat(claude): support conversation rewind via native session truncation - #1679

Open
junmo-kim wants to merge 7 commits into
tiann:mainfrom
junmo-kim:feat/claude-rewind
Open

feat(claude): support conversation rewind via native session truncation#1679
junmo-kim wants to merge 7 commits into
tiann:mainfrom
junmo-kim:feat/claude-rewind

Conversation

@junmo-kim

Copy link
Copy Markdown
Contributor

Problem

Claude sessions hard-reject Rewind (Rewind is not supported for Claude) while Codex/Pi/Grok already support it from the web UI. Claude Code itself has no in-process rewind, but v2.1.223+ ships resume-time truncation flags that can implement it across a process restart.

Approach

  • sdk wrapper: add resumeSessionAt / resumeDropsTurn query options mapping to the native --resume-session-at <uuid> / repeatable --resume-drops-turn <uuid> flags. Teach Session.consumeOneTimeFlags and the remote arg parser to treat them as one-shot flags so they never leak into later launches.
  • rewind handler:
    • Track the hub localIds of each delivered user turn (one batch = one native turn) at delivery time — no replay-user-messages overhead.
    • On rewind, parse the native transcript jsonl to resolve turn boundaries, then build a plan: keep everything through the previous turn's last entry (--resume-session-at) and drop the selected turn and everything after (repeated --resume-drops-turn, multi-turn drop supported).
    • Restart the SDK process via a new Session.requestRemoteRestart hook (abort current attempt → main loop respawns with fresh one-shot args; the restart suppresses the "Aborted by user" event).
    • Return { success: true, truncateFromLocalId } so the existing hub orchestration truncates its transcript. Known pre-mutation rejections (busy, no native history point, first message) return success:false/outcome:'rejected' instead of throwing, so the hub does not mark the session diverged.
    • Advertise rewindToMessage only when the installed binary reports >= 2.1.223 (claude --version probe at startup); fork current stays unversioned, so the two affordances have independent visibility conditions.

Semantics notes

  • Native transcripts are append-only; rewinds re-parent new turns onto the kept boundary, leaving orphaned branches in the jsonl.
  • Session id is preserved across a rewind, so no session binding changes are needed.
  • Dropping every turn including the first is rejected with a clear error.

Testing

  • Unit: transcript parsing + rewind plan resolution (multi-turn drop, boundary cases), one-shot flag consumption, version gate.
  • bun typecheck + full test suite green.
  • Manually verified end-to-end on an isolated stack (real claude binary + isolated hub/runner): 2 turns → rewind at turn 2 → transcript truncated to turn 1 → follow-up prompt answered "1" → native jsonl shows the new turn re-parented onto turn 1's assistant entry.

Add resumeSessionAt/resumeDropsTurn QueryOptions that map to the native
--resume-session-at / --resume-drops-turn CLI flags (Claude Code v2.1.223+),
and teach Session.consumeOneTimeFlags plus claudeRemote arg parsing to
handle them as one-shot flags.
Replace the hardcoded RewindConversation throw with an implementation that
resolves the selected hub message to a native turn (delivered localId
tracking + transcript jsonl parsing), builds a --resume-session-at /
--resume-drops-turn plan, restarts the SDK process with one-shot flags,
and returns truncateFromLocalId so the hub truncates its transcript.
Advertise rewindToMessage for the claude flavor.
Advertise rewindToMessage only when the installed Claude Code binary
reports >= 2.1.223 (--resume-session-at landed there); older or
undetectable binaries keep the Rewind affordance hidden instead of
failing at click time. Fork current stays unversioned.
- Clear delivered turn tracking when the native session id is dropped
  (/clear) so a later rewind cannot truncate a mismatched turn range,
  and drop the stale-metadata session id fallback in the handler.
- On an intentional restart abort (rewind), discard the in-flight batch
  instead of re-delivering it into the respawned truncated process.

@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

  • [Blocker] Rewind reports success before Claude applies the native truncation — requestRestart() only waits for the old attempt to abort. The next claudeRemote() invocation waits for a user message before it starts query(), so this handler can return success while the native JSONL is still unchanged. The hub then truncates its transcript immediately; an exit, handoff, or invalid resume flag before the next prompt leaves HAPI and Claude permanently divergent. Evidence: cli/src/claude/runClaude.ts:321; related flow cli/src/claude/claudeRemote.ts:205 and hub/src/sync/syncEngine.ts:1641.
    Suggested fix:

    const result = await claudeSession.requestRemoteRestart({
        resumeSessionAt: plan.resumeSessionAt,
        resumeDropsTurn: plan.dropsTurns,
    })
    if (!result.applied) return rejected(result.error)
    return { success: true as const, truncateFromLocalId: messageLocalId }

    The launcher should start a resume-only query immediately and resolve applied only after the new process emits system/init; spawn/init failures must return success: false.

  • [Major] Claude never publishes per-message rewind points — the web only renders Rewind when metadata.conversationHistoryPoints[localId] is true, but the new callback only appends to an in-memory array. Thus the advertised capability has no reachable UI affordance. Evidence: cli/src/claude/runClaude.ts:634; gate in web/src/components/AssistantChat/messages/UserMessage.tsx:55.
    Suggested fix:

    sessionInstance.client.updateMetadata((metadata) => ({
        ...metadata,
        conversationHistoryPoints: {
            ...metadata?.conversationHistoryPoints,
            ...Object.fromEntries(localIds.map((localId) => [localId, true as const])),
        },
    }))

    Publish this after the native turn is committed, and remove truncated points through the existing hub scrub path.

  • [Major] Local IDs are matched to native turns by unstable array position — deliveredTurnLocalIds starts at index 0 for resumed sessions and local-to-remote handoffs, and is cleared after rewind, while Claude JSONL retains all prior and orphaned branch entries. A later rewind can therefore reject a valid message or resume at an unrelated/orphaned UUID. Evidence: cli/src/claude/runClaude.ts:302; related append-only parser cli/src/claude/conversationHistory.ts:27.
    Suggested fix:

    const promptUuid = claudeSession.getMetadata()
        ?.conversationHistoryEntryIds?.[messageLocalId]
    const dropFromTurnIndex = turns.findIndex(
        (turn) => turn.promptUuid === promptUuid,
    )

    Persist the prompt UUID for each completed localId and build turns from the active parentUuid chain rather than raw file order.

Summary

Review mode: initial

Three findings: one native/HAPI divergence and data-loss risk, one missing web locator publication, and one incorrect turn-resolution strategy for resumed or repeated rewinds.

Testing

  • Not run (automation; static review only because the task forbids executing PR code).
  • GitHub at review time: integration passed, drift-gate passed, test still in progress.
  • Missing coverage: restart acknowledgment/failure before hub truncation; conversation-history point publication; resumed-session, local-handoff, and repeated-rewind branch cases.

HAPI Bot

Comment thread cli/src/claude/runClaude.ts Outdated
Comment thread cli/src/claude/runClaude.ts Outdated
Comment thread cli/src/claude/runClaude.ts Outdated
Review round 1 fixes:
- Start the respawned query immediately when rewind flags are present
  (skip waiting for a child prompt, like --fork-session) and resolve a
  rewind ack on system/init; the handler reports success only after the
  truncation is confirmed, so the hub never truncates on an unverified
  rewind. Timeouts and spawn failures return success:false instead.
- Resolve turns over the active parentUuid chain so orphaned branches
  left by earlier rewinds cannot shift turn boundaries.
- Map hub localIds to native prompt uuids via conversationHistoryEntryIds
  metadata (survives restarts, scrubbed by the hub) instead of positional
  array indexes, and publish conversationHistoryPoints at delivery time
  so the web Rewind affordance actually renders for Claude sessions.

@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

  • [Blocker] Rewind fallback timer can consume the next prompt after a failed resume — the 4-second timer is not retained or cleared in finally. If the resume process rejects and exits first, the callback still calls scheduleNextMessage() from the dead attempt. The launcher has already respawned, so this stale waiter can replace the live MessageQueue2 waiter and dequeue the next user message into an orphaned SDK stream. Evidence: cli/src/claude/claudeRemote.ts:309; related single-waiter implementation cli/src/utils/MessageQueue2.ts:669.
    Suggested fix:

    let rewindReadyTimer: ReturnType<typeof setTimeout> | null = null
    if (awaitingRewindInit) {
        rewindReadyTimer = setTimeout(markRewindReady, REWIND_READY_DELAY_MS)
    }
    try {
        // consume response
    } finally {
        if (rewindReadyTimer) clearTimeout(rewindReadyTimer)
    }
  • [Major] Unknown rewind outcomes are reported as safe rejections — the unconfirmed branch throws specifically so the hub marks history diverged, but the surrounding catch converts that throw to success: false/outcome: rejected. The hub therefore allows more history actions even though Claude may already have truncated its native transcript. Evidence: cli/src/claude/runClaude.ts:394.
    Suggested fix:

    if (result.error === "unconfirmed") {
        throw new Error("Rewind could not be confirmed; session history requires reconciliation")
    }
    // Only deterministic native rejections return rejected(...).
  • [Major] An aborted confirmation attempt is acknowledged as successfully applied — claudeRemote() swallows AbortError and returns normally, then this block calls ack(true) before checking controller.signal.aborted or exitReason. Abort RPCs are not blocked by the hub history lock, and local exit can race here, so the hub can truncate its transcript without a confirmed native mutation. Evidence: cli/src/claude/claudeRemoteLauncher.ts:584.
    Suggested fix:

    const confirmed = !controller.signal.aborted && !this.exitReason
    if (session.rewindAck) {
        const ack = session.rewindAck
        session.rewindAck = null
        ack(confirmed, confirmed ? undefined : "unconfirmed")
    }
  • [Major] Resumed sessions map new messages to the oldest native turn — committedTurnBatches starts empty for every CLI process, while turns contains the full resumed transcript. Thus the first new batch after resume is assigned to turns[0], usually causing a false “Cannot rewind the first message” or targeting the wrong UUID. Evidence: cli/src/claude/runClaude.ts:326.
    Suggested fix:

    const batches = pendingTurnBatches.splice(0)
    const start = turns.length - batches.length
    if (start < 0) return rejected("Native turn mapping is incomplete")
    batches.forEach((batch, index) => commitBatch(batch, turns[start + index]!))
  • [Major] Native locators are recorded before completion and persisted only when rewind is requested — failed attempts that are restored and retried append the same local IDs repeatedly, while a CLI restart before the first rewind loses pendingTurnBatches but preserves visible conversationHistoryPoints without conversationHistoryEntryIds. Both paths leave valid-looking Rewind actions bound to missing or incorrect prompts. Evidence: cli/src/claude/claudeRemoteLauncher.ts:506; related lazy persistence cli/src/claude/runClaude.ts:334.
    Suggested fix:

    const completedLocalIds = inFlightMessage?.items
        .flatMap((item) => item.localId ? [item.localId] : []) ?? []
    if (completedLocalIds.length > 0) {
        session.onUserTurnCompleted?.(completedLocalIds)
    }
    inFlightMessage = null

    Resolve the tail prompt UUID and persist both the point and entry ID from that completion callback.

  • [Major] Rewinding a later message inside a batched native prompt truncates the hub at the wrong boundary — every local ID in a joined batch maps to one prompt UUID, so Claude drops the whole batch, but the success response truncates from the clicked local ID. Earlier messages in that same batch remain in HAPI even though they no longer exist natively. Evidence: cli/src/claude/runClaude.ts:406; batch mapping at cli/src/claude/runClaude.ts:328.
    Suggested fix:

    const batch = committedTurnBatches.find((item) => item.promptUuid === promptUuid)
    const truncateFromLocalId = batch?.localIds[0]
    if (!truncateFromLocalId) return rejected("Native batch boundary is unavailable")
    return { success: true as const, truncateFromLocalId }

Questions

  • None.

Summary

Review mode: follow-up after new commits

Six findings remain in the latest full diff: one prompt-loss blocker plus five major rewind confirmation, locator, resume, retry, and batch-boundary correctness risks.

Testing

  • Not run (automation; static review only because the task forbids executing PR code).
  • GitHub at review time: test, drift-gate, and integration passed; pr-review in progress.
  • Missing coverage: rejected/aborted/timed-out restart lifecycle; CLI restart and resumed-session locator persistence; retry restoration; multi-message batch rewind boundaries.

HAPI Bot

Comment thread cli/src/claude/claudeRemote.ts Outdated
Comment thread cli/src/claude/runClaude.ts Outdated
Comment thread cli/src/claude/claudeRemoteLauncher.ts
Comment thread cli/src/claude/runClaude.ts Outdated
Comment thread cli/src/claude/claudeRemoteLauncher.ts Outdated
Comment thread cli/src/claude/runClaude.ts Outdated
…ound 2

- Clear the rewind ready timer in claudeRemote's finally so a rejected
  resume cannot leave a stale queue waiter that steals the next prompt
  into the dead attempt.
- Only acknowledge a rewind as applied when the confirming attempt ends
  without an abort or exit; our own teardown abort keeps the ack armed
  for the respawn, external aborts report unconfirmed (diverged).
- Let the unconfirmed timeout propagate past the rejection conversion so
  the hub marks history diverged instead of allowing further actions.
- Record rewind locators at native turn completion instead of delivery:
  crash retries cannot double-book batches, mappings survive via
  conversationHistoryEntryIds metadata, resumed sessions map by prompt
  uuid on the active chain, and joined batches truncate the hub from the
  batch's first local id.

@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] Claude-injected user entries can be mistaken for human turns — the new transcript parser accepts every string/text-block type: 'user' entry, while HAPI's existing classifier documents that Claude writes system reminders, task notifications, command caveats, and skill injections in exactly that shape. This can bind a HAPI local ID to an injected UUID and make native rewind preserve/drop a different boundary than the hub truncates. Evidence: cli/src/claude/conversationHistory.ts:22; related handling cli/src/api/apiSession.ts:43.
    Suggested fix:

    const parsed = RawJSONLinesSchema.safeParse(entry)
    if (!parsed.success || !isExternalUserMessage(parsed.data)) continue
    turns.push({ promptUuid: parsed.data.uuid, endUuid: parsed.data.uuid })

    Extract the existing classifier to a lightweight shared Claude helper if needed to avoid coupling the parser to apiSession.ts.

  • [Major] Joined-batch truncation is still wrong after a CLI restart — conversationHistoryEntryIds persists each local ID → prompt UUID, but localIdsByPromptUuid is memory-only and is never rebuilt. After restart, rewinding a later message in a joined batch falls back to that later messageLocalId; Claude drops the whole native batch while the hub leaves the earlier batch messages behind. Evidence: cli/src/claude/runClaude.ts:339; all batch members remain advertised at cli/src/claude/runClaude.ts:727.
    Suggested fix:

    const boundaryLocalId = localIds[0]!
    promptUuidByLocalId.set(boundaryLocalId, turn.promptUuid)
    localIdsByPromptUuid.set(turn.promptUuid, [boundaryLocalId])
    newEntryIds[boundaryLocalId] = turn.promptUuid
    
    conversationHistoryPoints: {
        ...metadata?.conversationHistoryPoints,
        [boundaryLocalId]: true
    }

    Persist/advertise only the canonical first local ID, or persist an explicit batch-start mapping and hydrate it on restart.

Questions

  • None.

Summary

Review mode: follow-up after new commits

Two Major correctness findings remain in the latest full diff: hidden native user entries can corrupt turn mapping, and joined-batch boundaries are still lost across CLI restarts. The prior timer, unknown-outcome, abort-confirmation, completion-timing, resumed-tail, and in-process batch-boundary findings are otherwise addressed by the new commits.

Testing

  • Not run (automation; static review only because PR code was not executed).
  • GitHub at review time: test, integration, and drift-gate passed; pr-review pending.
  • Missing coverage: injected/meta user entries inside the active native chain; joined multi-message batch rewind after CLI restart.

HAPI Bot

Comment thread cli/src/claude/conversationHistory.ts Outdated
Comment thread cli/src/claude/runClaude.ts
…daries

- Reuse HAPI's isExternalUserMessage classifier when parsing native
  turns: system reminders, task notifications and command caveats are
  written as text-bearing user entries and must not become rewind
  boundaries.
- Only the first local id of a joined batch is a rewind point (mapped,
  persisted and advertised); later members cannot become hub truncation
  boundaries that diverge from the natively dropped batch.

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

Questions

  • None.

Summary

Review mode: follow-up after new commits

No issues found in the latest full diff. The two prior Major findings are resolved at this head: native user-entry classification now reuses the existing external-user filter, and joined batches now persist/advertise only their canonical first local ID. Residual risk remains in the timing-sensitive process-restart/rewind acknowledgement path because it has no launcher-level automated coverage.

Testing

  • Not run (automation; static review only because PR content was not executed).
  • GitHub at review time: test, integration, and drift-gate passed; pr-review pending.
  • Missing coverage: end-to-end RewindConversation restart success, deterministic rejection, timeout/unknown outcome, and first prompt delivery after the rewind respawn.

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