[brennanb2025]

	Files	Added	Deleted	Net
Test	15	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​76	$\color{#cf222e}{\Huge{\mathbf{−}}}$​104	$\color{#cf222e}{\Huge{\mathbf{−}}}$​28
Prod	10	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​4	$\color{#cf222e}{\Huge{\mathbf{−}}}$​49	$\color{#cf222e}{\Huge{\mathbf{−}}}$​45

ELI5

Native chat still carries a saved checkpoint that no running code uses and a method that only tests call. Removing both makes chat storage easier to maintain. Ordinary chats keep their current behavior, and chats saved by older builds still open; a chat previously rejected only because its obsolete checkpoint was missing or malformed can now open normally.

What Changed

• journalCheckpoint on the saved chat lease: remove the type, validation clause and null writers. The existing load-time cleanup drops this retired field before a later save. Old null or {epoch, sequence} values continue to load, and missing or malformed values no longer make an otherwise readable chat unreadable. All other existing validation stays unchanged.
• AgentSessionRecordStore.setConversationCommand: remove the unused production method and its writer. The two tests that simulate an older build's /clear or /compact record now edit saved data and reopen the store. Three redundant cases that used the setter are removed; live command readers, clear handling and compaction behavior remain in place.

The startAnswered declaration, forwarding, teardown predicate and test fixture remain unchanged from the base branch. Its removal is handled by #28046.

Why

These two mechanisms look active, so people changing native chat have to determine whether they still matter. The checkpoint has no runtime reader, and every production writer stores null; the command setter has no production callers. Removing the inactive paths avoids carrying misleading contracts forward.

For the saved field, the common pattern is to discard retired data during loading. This extends Orca's existing retired-lease-field cleanup rather than adding a migration or another storage mechanism.

Differences, labelled:

• Intended: a chat whose only invalid or missing value is its retired checkpoint used to be set aside as unreadable. It now loads and can restore history and resume through the existing ownership checks. Existing validation of other fields is unchanged.
• Intended: an older Orca build rejects a row saved without the checkpoint because its validator requires the field. Downgrades are unsupported. Mixed client and server versions remain supported: leases stay on the execution host and are not exchanged over the wire.

Linked Issue

None (maintenance cleanup). The separate startAnswered removal is handled by #28046.

Visual Proof

Real Electron QA on the gate-admitted M4Air passed on be9be12bcbb0454e3bce48406612a1edac56579f: native Claude and Codex chats created on main retained their history, resumed, recalled their previous markers, and sent normally after upgrading. Chat UI was enabled through Settings on main and retained on the branch; all accepted real turns used managed hooks. Malformed retired checkpoint values are covered by the disk regression test.

Codex on main:

Codex after upgrading and resuming:

Claude on main:

Claude after upgrading and resuming:

Testing

• Automated tests added/updated
• Saved-data coverage in agent-session-record-store.test.ts checks old null, object and malformed checkpoint values, readability after reopening, and removal of the retired field on the next write.
• Updated tests in structured-conversation-command.test.ts and structured-conversation-compaction.test.ts reopen persisted old-build command records and check that sending and command behavior still work.
• Before scope narrowing, all 14 touched test files were run by name. The review loop also ran the saved-data and old-command contracts. After scope narrowing, structured-agent-session-restart-witnesses.test.ts and structured-agent-session-restart-resume.test.ts passed: 2 files, 43 tests, through the machine queue with the explicit Vitest configuration.
• CI is the lint and typecheck gate. No standalone local lint or typecheck is run.
• No platform-specific code changes. Saved chats remain on the execution host, including WSL and remote SSH hosts; no lease field crosses the client/server boundary. Folder workspaces and git worktrees use the same record path.

Review

Full path: this changes saved chat records and their loading, so it does not qualify for the light path. Three review rounds, the readiness checklist, a unanimous design-panel KEEP ruling, both-agent real Electron QA, and the full PR workflow completed on the final head.

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Security: no new credentials or access paths. Cross-platform and remote SSH: retirement happens on the execution host using the existing loader. Mobile and mixed client/server versions: no wire change. Compatibility: old saved chats remain readable; saved-data downgrade compatibility is unsupported. Performance: no new work added.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Main/branch real-agent screenshots attached
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• Full PR workflow, including static analysis, tests, packaging, and the aggregate gate, passes on the current head; no local standalone lint/typecheck

[coderabbitai]

📝 Walkthrough

📝 Walkthrough

The session lease contract no longer includes journalCheckpoint. Loading drops that legacy field, and persisted lease validation no longer checks it. The conversation-command store setter and commit function were removed. Tests now inject selected command records into persisted state during restart or reopening. Several earlier recovery tests were removed, and the compaction test was added to the SQLite runtime test list.

Priority: ⬇️ Low

Estimated code review effort:

Merge Risk: 🟡 Moderate · up to be9be

Chats saved by this version may become unreadable after a downgrade. Preserve the older reader’s stored-field contract before merging unless that compatibility break is explicitly accepted.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the two primary changes: removing the unused native chat journal checkpoint and setConversationCommand.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The pull request description is complete and follows the required template. It explains the change, rationale, compatibility impact, testing, review scope, visual proof, and checklist status. The auth…

Full details: Description check

Explanation

The pull request description is complete and follows the required template. It explains the change, rationale, compatibility impact, testing, review scope, visual proof, and checklist status. The author explicitly states that no linked issue exists, which is acceptable only if the author is a maintainer under the template's special case.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: f0b1aae7-8110-4424-b511-f672a5835fd6

📥 Commits

Reviewing files that changed from the base of the PR and between 912cb06 and be9be12.

📒 Files selected for processing: 25

• config/scripts/vitest-sqlite-runtime-files.mjs
• src/main/native-chat/agent-session-record-test-fixture.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-crash-turn-end.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-forget-status.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-queued-gate.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-test-harness.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts
• src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts
• src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts
• src/main/runtime/agent-session-conversation-command-record.ts
• src/main/runtime/agent-session-record-founding.ts
• src/main/runtime/agent-session-record-rows.ts
• src/main/runtime/agent-session-record-store.test.ts
• src/main/runtime/agent-session-record-store.ts
• src/main/runtime/agent-session-reservation-admission.ts
• src/main/runtime/structured-agent-launch-parity.test.ts
• src/main/runtime/structured-worker-identity.test.ts
• src/shared/agent-session-lease-adjudication.test.ts
• src/shared/agent-session-record.test-fixture.ts
• src/shared/agent-session-record.ts
• tests/e2e/cross-version-wire/agent-session-handle-replacement-downgrade.unit.test.ts
• tests/e2e/cross-version-wire/cross-version-registered-agents.unit.test.ts

💤 Files with no reviewable changes: 15

• src/main/runtime/structured-agent-launch-parity.test.ts
• src/shared/agent-session-lease-adjudication.test.ts
• src/main/runtime/structured-worker-identity.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-crash-turn-end.test.ts
• src/main/runtime/agent-session-reservation-admission.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-restart-resume-test-harness.ts
• src/main/native-chat/agent-session-record-test-fixture.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts
• src/main/runtime/agent-session-record-store.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-forget-status.test.ts
• src/shared/agent-session-record.test-fixture.ts
• src/main/runtime/agent-session-conversation-command-record.ts
• src/main/runtime/agent-session-record-founding.ts
• src/shared/agent-session-record.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

[brennanb2025]

Native chat still required a saved checkpoint that no running code used. A chat with only that obsolete value missing or malformed could be set aside as unreadable. This change removes that requirement and the unused generic conversation-command writer. Ordinary chats keep their history and resume/send behavior.

The full-path review completed three rounds, architecture and performance reviews, the readiness checklist, and a unanimous three-model KEEP ruling. Fixed in place: restored the overlapping hook removal to leave it with #28046, repaired historical decoder fixtures, and added the SQLite test route. The removed queue and compaction tests duplicate surviving contracts; the two historical command tests now seed disk records and reopen the store.

The loader retains every previous non-checkpoint check, strips only explicitly named retired lease fields, and still requires ownership reconciliation. Accepting a malformed retired checkpoint is intended. Old builds reject subsequently saved rows without the checkpoint; saved-data downgrade is unsupported, while mixed client/server contracts remain unchanged. A pre-existing optional fence-floor validation defect is tracked separately in STA-11204.

Verified on be9be12bcbb0454e3bce48406612a1edac56579f: targeted regressions and the full PR workflow, including its aggregate gate, pass. Real Electron QA on the gate-admitted M4Air also passed: native Claude and Codex chats created on main retained history, resumed, recalled their earlier markers, showed working→done, and accepted another draft for sending. The old checkpoint was absent after branch load/write while the other saved chat fields remained intact. Chat UI was enabled once on main; accepted real turns used managed hooks and isolated homes. The completed driver runs were accepted after a shared-machine config-write attribution check.

Deferred QA observations: the existing first-restart session-search introduction appeared; workspace sidebar order changed; Codex's unreported effort label disappeared. Their relevant code is unchanged. Saved options and actual turn contexts show no configured model/effort loss; the precise initial label/sidebar trigger remains unproven. The review found no PR regression behind these observations. Manual QA did not cover malformed rows, legacy command records, Windows/SSH, or mixed peers; those rely on automated contracts and source review. Two early backup/hook configuration hash baselines were incomplete and are recorded in the local QA report.

For Brennan's review:

Codex on main:

Codex after upgrading and resuming:

Claude on main:

Claude after upgrading and resuming:

Human judgment points:

• Retired-field removal and validation-before-load.
• Old null/object/malformed checkpoint disk regression.
• Reopened historical compaction contract retained.