Skip to content

fix: preserve session state on session expiration, abort, and compaction (Fixes #1054) - #1055

Open
Utkarsh-X wants to merge 1 commit into
CodebuffAI:mainfrom
Utkarsh-X:fix/preserve-session-state-on-abort-compaction
Open

fix: preserve session state on session expiration, abort, and compaction (Fixes #1054)#1055
Utkarsh-X wants to merge 1 commit into
CodebuffAI:mainfrom
Utkarsh-X:fix/preserve-session-state-on-abort-compaction

Conversation

@Utkarsh-X

Copy link
Copy Markdown

Fixes #1054

Summary

Fixes an issue where sending a message (such as "continue") after a free session expires or after interrupting with Esc causes the assistant to lose all conversation memory and restart exploration from scratch.

Root Cause

  1. Missing in-memory state update on errors / session expiration (use-send-message.ts):
    When a free session ends or fails with a gate error, the catch (error) block in use-send-message.ts calls saveChatState to persist the snapshot to disk, but does not update previousRunStateRef.current in React memory. When the user sends a follow-up prompt in the same running session, sendMessage reads the un-synced previousRunStateRef.current, causing sdk/src/run.ts to instantiate a blank initialSessionState (messageHistory = []).

  2. Abort race condition (use-send-message.ts):
    When a user interrupts with Esc, the UI input lock was released before client.run() finished resolving without synchronously assigning previousRunStateRef.current = latestRunStateSnapshot.

  3. Compaction continuation guidance (compact-history.ts, context-pruner.ts):
    When deterministic compaction runs, the history is collapsed into a synthetic summary. Because the system prompt retains # Initial Git Changes, models seeing 0 assistant turns occasionally re-ran exploratory git diff and tree commands.

Changes

  • cli/src/hooks/use-send-message.ts:
    • Assign previousRunStateRef.current = latestRunStateSnapshot and setRunState(latestRunStateSnapshot) inside registerActiveRun upon abort.
    • Assign previousRunStateRef.current = latestRunStateSnapshot and setRunState(latestRunStateSnapshot) in the catch (error) block alongside saveChatState.
  • cli/src/hooks/helpers/send-message.ts:
    • Updated the abort listener comment to reflect synchronous state preservation.
  • packages/agent-runtime/src/compact-history.ts & agents/context-pruner.ts:
    • Clarified CONTINUATION_TEXT (in 100% parity) to instruct models that initial repository exploration is already recorded in <historical_memory>.

Verification

  • bun test sdk/src/__tests__/run-cancellation.test.ts (16 passed)
  • bun test cli/src/utils/__tests__/run-state-storage.test.ts (37 passed)
  • bun test packages/agent-runtime/src/__tests__/compact-history.test.ts (31 passed)
  • packages/agent-runtime/src/__tests__/context-pruner-parity.test.ts (13 passed)
  • agents/__tests__/context-pruner.test.ts (69 passed)
  • bun run build:sdk and bun run build:freebuff both compile with exit code 0.

@codebuff-team

Copy link
Copy Markdown
Contributor

Thanks for the detailed root-cause writeup — this is a well-documented PR and the instinct (sync previousRunStateRef before disk save resolves) is reasonable. A few things to strengthen before this is portable:

  1. No new test covers the actual bug. You cite passing existing suites (run-cancellation.test.ts, run-state-storage.test.ts, etc.), but none of those appear to exercise the specific regression: send a follow-up message immediately after abort/expiration and assert messageHistory is preserved rather than reset to []. Without a repro test, this fix could regress silently later, and a maintainer can't confirm the root cause is actually what you describe versus a coincidental fix.

  2. Duplicate state assignment logic. The same previousRunStateRef.current = latestRunStateSnapshot; setRunState(latestRunStateSnapshot) pair is now inlined in two places (registerActiveRun's abort handler and the catch block). Worth factoring into a small helper to avoid drift if one branch is updated later without the other.

  3. Prompt text changes (compact-history.ts, context-pruner.ts) are the riskier part. These affect model behavior broadly, not just the abort/expire path this PR is nominally about — bundling a system-prompt tweak with a state-sync bug fix makes it harder to isolate cause/effect if either regresses. These two concerns would be easier to review (and port) as separate PRs.

  4. Confirm setRunState doesn't trigger an unwanted re-render/flash given it's now called synchronously in the abort listener as well as the catch block — worth a sentence on why that's safe.

Overall the direction is right but this needs a regression test demonstrating the fixed behavior, and ideally splitting the prompt-text change into its own PR.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Aug 20, 2026
@Utkarsh-X
Utkarsh-X force-pushed the fix/preserve-session-state-on-abort-compaction branch from 87d92b8 to f1418c3 Compare August 20, 2026 08:36
@Utkarsh-X

Copy link
Copy Markdown
Author

Thanks for the thoughtful review and constructive feedback. I've addressed all four points in the latest commit (f1418c3):

1. Regression Tests (cli/src/hooks/helpers/__tests__/send-message.test.ts)

Added a dedicated test suite verifying the exact regression path:

  • User Abort (Esc): Simulates an active turn receiving user input, assistant thought, and a completed read_files tool call. Verifies that upon abort, previousRunStateRef.current is immediately populated in memory, so createRunConfig produces a runConfig.previousRun containing the full 3-item messageHistory rather than resetting to undefined/[].
  • Session Expiration / Error: Tests the catch (error) branch (e.g. session ended banner) to ensure sending "continue" preserves context from the expired session.
  • Cross-Chat Isolation: Tests that syncRunState is a no-op if the user switches chats mid-flight (runChatIsCurrent() === false), ensuring interrupted turns never bleed into unrelated conversations.

2. Factored Duplicate State Sync Logic (cli/src/hooks/use-send-message.ts)

Consolidated the state assignment into a single closure in sendMessage scope:

const syncRunState = (state: RunState) => {
  if (!runChatIsCurrent()) return
  previousRunStateRef.current = state
  setRunState(state)
}

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

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session state is lost after free session expires or run is interrupted

2 participants