Skip to content

fix(native-chat): an older Orca keeps a chat with a newer row kind read-only instead of deleting the rest of its history - #24477

Merged
brennanb2025 merged 5 commits into
mainfrom
brennanb2025/journal-skip-unknown-row-kinds
Oct 2, 2026
Merged

brennanb2025 merged 5 commits into
mainfrom
brennanb2025/journal-skip-unknown-row-kinds

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​342 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​341
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​85 $\color{#cf222e}{\Huge{\mathbf{−}}}$​79 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​6

ELI5

Every native chat keeps its history as a numbered list of rows, and each row has a kind ("an item", "a send", "a removal", and so on). Until now, when Orca opened a chat and found a row of a kind it had never heard of, it decided the history was damaged from that row on. It deleted that row and every row after it.

That is what happens when you run an older Orca on a chat a newer Orca wrote, if the newer Orca added a new kind of row. The older Orca throws away the end of your conversation, including ordinary messages written after the new row, then rebuilds what it can from the agent's own transcript. Anything only Orca knew (sends, Stops and Resumes) is gone for good, even after you upgrade again.

After this change, an older Orca treats an unknown kind of row the way it already treats a row from a newer format version. It keeps every row on disk and opens the chat read-only until you update. It also fixes a second way history was lost: a row whose own stored number disagrees with the number it is filed under is now treated as damage at that row, where before Orca either deleted a valid row and left a hole in the numbering, or made every later write fail.

What Changed

Before, an older Orca opening a chat that a newer Orca had written a new kind of row into:

  • said "1 journal line could not be read",
  • deleted the unknown row and everything after it,
  • rebuilt the chat from the agent's transcript where one exists. The chat stayed usable, but what the newer Orca wrote after that point was lost, including on a later upgrade.

After, the same Orca:

  • deletes nothing,
  • shows the history up to that row,
  • refuses to send with "Chats were saved by a newer Orca. Update Orca to keep using them.", keeps your unsent message with Retry, holds queued messages, and turns rewind off,
  • opens the chat normally with every row, including everything after the unknown one, once you update.

Be aware of the trade-off. Before, the chat was usable again right away, at the cost of history. Now it is frozen until you update, with nothing lost. This is the same thing a row from a newer format version already does today: Orca's turn item shipped that way. Today's Orca writes no unknown kinds, so the difference first shows up on a downgrade after some later release adds one.

A row is still damage, repaired exactly as before, when:

  • it isn't valid JSON;
  • it lacks the fields every row has (its chat history id, its number, which Orca process wrote it, and a timestamp);
  • it is a known kind with broken fields.

A second repair is fixed. Each row is stored under its number and also carries that number inside its text. When the two disagreed, opening the chat trusted the number inside the text. I confirmed main's behaviour by running the new tests with only this check reverted:

  • Inner number lower than the stored one (stored at 3, says 2): main deleted the valid row at 2 as well, showed no notice, and the next write left a hole in the numbering.
  • Inner number higher (stored at 3, says 9): main kept the bad row, showed no notice, and every later write in the chat failed with a duplicate-number error.

Now the row is treated as damaged at its stored number. It is removed with the usual "could not be read" notice, the rows before it stay, and the next write succeeds. A row of an unknown kind is never checked this way: like a newer-format row, it is kept and the chat goes read-only.

Mechanism

  • journal-row-schema.ts. Reading a row checks its format version first; a newer version is unreadable, as before. Then, if the kind is one this build does not know, and the row has the fields every row keeps, it is unreadable too: the existing read-only outcome. The fields are a non-empty epoch (which history of the chat it belongs to), an integer seq of at least 1 (its number), an integer fence (which Orca process wrote it) and a numeric ts. Without them it is still malformed, as is a known kind with broken fields.
  • Known kinds come from the row type. Each kind's own-field check sits in one table keyed by every kind the row type holds, and the set of kinds this build knows is derived from it. A kind added without a check fails to compile, instead of making this build's own chats read-only as if a newer Orca wrote them.
  • journal-open.ts. When a chat opens, a row whose inner number differs from its stored number counts as malformed at the stored number. An unreadable row still stops the read with nothing deleted. The rest, read-only state included, is the existing path, unchanged.
  • The rule for adding a kind, written at the top of journal-row-schema.ts. Builds from before this change still delete from an unknown kind. So a new kind must:
    • keep the fields every row has (changing them needs a format version bump);
    • and either ship its reader first, writing the kind only once no supported Orca lacks that reader, or be written at a bumped format version.

Follow-up, not in this PR

The Stop event (#24072, #24311) and the Resume marker still ride on a removal row with an extra field, which every Orca ignores. A Stop changes the queue and how a turn ends. An older Orca that read past it would release messages you stopped and record your cancelled turn as a failure. It can get a kind of its own in one of two ways:

  • ship its reader in one release and write it only once no supported Orca lacks that reader;
  • or write it at a bumped format version.

The cross-version test pins the current boundary: the writable baseline build still deletes an unknown kind.

Why

The alternatives:

  • Read past unknown kinds by default. This was rejected. Every kind Orca has, and every kind it plans to add, changes the queue, a turn or an item. An older Orca that read past one would keep writing from a wrong picture of the chat. An older Orca's rule is also fixed the day it ships, so an unsafe default would apply to every future kind.
  • Let the newer Orca mark a row as safe to read past. Not now. No current or planned kind could use the mark, and the common pattern has no such mark. It can come with the first kind that needs it.
  • Keep deleting and rebuilding. That loses what only Orca recorded, and the loss survives the upgrade back.
  • Only ever add kinds with a format version bump. That is already safe today. But nothing stops a kind being added without one, and this change makes that case safe as well.

Differences from the common pattern

The common pattern: a reader that meets a record type it does not know fails closed and never deletes the record. This change brings Orca in line with it.

  • Intended: a chat with an unknown row still shows everything up to that row, but refuses new writes. Orca rebuilds the queue and the turn state from the history before every write, so writing past a row it can't read isn't safe.
  • Intended: a row of an unknown kind that lacks the fields every row shares (its epoch, number, writer and time) is treated as damage and dropped with a repair notice, because it can't be told apart from corruption. The new rule in the row file's header keeps every future kind carrying those fields.
  • Intended: if you downgrade permanently, such a chat stays read-only until you update.
  • Intended: builds from before this change still delete from an unknown kind. Released builds can't be changed, so this ages out as they leave support. Until then a new kind ships its reader first or is written at a bumped format version. Orca has no written support window yet, so when that happens can't be measured until one exists.
  • Temporary: an unknown item content kind inside a known row is still treated as damage and deletes from that row on. That is the same class of bug one level down, and this change leaves it alone. It is the next follow-up: an unknown content kind should go read-only the same way.
  • Temporary: the Stop event and the Resume marker stay on removal rows until the follow-up above.

Visual Proof

N/A: no UI change. The read-only state is the existing one a newer format version already shows.

Testing

  • src/main/native-chat/agent-session-journal/journal-unknown-row-kind.test.ts (17 tests):
    • Reading a row:
      • An unknown kind with the fields every row keeps is unreadable.
      • A missing number, a number of 0, a string number, a fractional writer stamp, a string timestamp, an empty history id, an empty kind or a non-string kind is malformed.
      • A known kind with broken fields is malformed.
      • A newer format version is unreadable whatever its kind.
    • One valid row of every kind this build writes reads back exactly. The test's table is typed over every kind, so a new kind must be added to it.
    • A chat whose row 3 is a newer kind:
      • It opens read-only with the item before it, and no repair notice.
      • A write is refused (journal_read_only), and after a reopen every stored row is byte-identical.
      • With row 3 back as a kind this build reads (a stand-in for the newer Orca after an upgrade), all four items read and the next write succeeds.
    • A newer kind with a mismatched number stays read-only with nothing deleted. A newer kind with broken fields is dropped with a notice.
    • A known row stored at 3 whose text says 2 or 9 is dropped at 3 with a notice. The row at 2 survives, and the next write succeeds after a reopen.
  • Cross-version (tests/e2e/cross-version-wire/agent-session-stop-event-downgrade.unit.test.ts). For a newer kind, this build opens read-only, refuses the write and keeps every row byte-identical. The pinned writable baseline build, which predates this change, deletes it on open. The two existing Stop/Resume downgrade tests are unchanged.
  • Journal and session-wire suites: 172 files, 1636 tests, all passing. That is every test in the journal folder plus every session-wire test that touches the journal.
  • Ablations: each piece was reverted on its own and run against the unit file and the cross-version file (19 tests).
Reverted Failing
An unknown kind is malformed again (deleted, as before this change) 4 of 19, including the cross-version test
The stored-number check removed 2 of 19
One kind (tombstone) missing from the known-kind table 6 of 20 in the unit file plus the Stop/Resume rows file, including the new every-kind test. The ablation ran tests only; a missing key in the typed table is also a compile error, not run on the ablated tree.
  • Lint: oxlint on the changed files and the changed-code quality gate are clean. The scoped tc:node typecheck passes.
  • Not run: a manual app run. There is no new UI. The read-only behaviour is the existing one a newer format version already triggers, and the tests above exercise it on real SQLite. The chat history lives with the Orca that runs the agent, so SSH sessions behave the same. No wire format changes.

…d-only unless its writer declared it skippable

A row of a kind this build does not know, in a well-formed envelope, now latches the chat
read-only with every row kept, the same way a newer row version does. It is read past only
when its writer declared `ifUnknown` on the row: `skip` (a rewind drops it) or `carry` (a
rewind carries it after the rebuilt history, epoch, seq and fence restamped). Every existing
kind changes queue or turn state, so skipping by default would let an older build write from
a wrong fold.

- journal-row-kind-compatibility.ts: each kind states how older builds read it, typed over
  every row kind, so a new kind cannot be added without a declaration.
- Rewind restates the Resume and Stop as before, then carries `carry` rows in source order;
  the restatement goes back to { lifted, liveStop }.
- Replay treats a row whose body names another sequence than its stored key as malformed at
  the key, so the next write never collides with it; catch-up reads stop there too.
… kind only latches read-only

An older Orca now treats a row of a kind it does not know exactly like a row from a newer
schema version: every row stays on disk and the chat opens read-only until an update. The
writer-declared skip/carry opt-in, its in-memory placeholder, the carry through rewinds and
the per-kind registry are removed: no current or planned kind could use them, and they can
come with the first kind that may safely be read past.

Kept: an unknown kind needs the envelope every row keeps (epoch, sequence, fence, timestamp),
else it is damage as before; a row whose body names another sequence than its stored key is
malformed at the key; the epoch row's validator names its kind. The schema header states the
rule for adding a kind: keep the envelope, and either ship the reader first or bump `v`.
@brennanb2025 brennanb2025 changed the title fix(native-chat): an older Orca skips a history row kind it doesn't know instead of deleting the rest of the chat fix(native-chat): an older Orca keeps a chat with a newer row kind read-only instead of deleting the rest of its history Oct 2, 2026
@brennanb2025

brennanb2025 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Review status

The problem. When an older Orca opened a chat that a newer Orca had written, it deleted everything from the first row of a kind it didn't recognise. It treated that row as damage, removed it and every row after it, ordinary messages included, then rebuilt what it could from the agent's own transcript. Anything only Orca recorded (sends, Stops, Resumes) was gone for good, even after upgrading again. Because of this, no release could safely add a new kind of history row. That is why the Stop event and the Resume marker currently ride on removal rows with an extra field.

What changes for users. An older Orca that meets a row kind it doesn't know now keeps every row and opens the chat read-only. You can still see the history up to that row. Sending is refused with "Chats were saved by a newer Orca. Update Orca to keep using them.", and the chat comes back whole after you update. This is how Orca already handles a row from a newer format version. The trade-off: before, the chat was usable again right away at the cost of history; now nothing is lost, but the chat is frozen until you update. No current release writes an unknown kind, so nobody sees a difference until a later release adds one.

A second repair is fixed too. Each row is stored under a number and also carries that number in its own text. When the two disagreed, opening the chat trusted the text, which either deleted a valid row and left a hole in the numbering, or left a row that made every later write fail. Now the row counts as damaged at its stored number: it is removed with the usual "could not be read" notice, and writing works again.

What the review changed (the design changed twice, so the final diff is much smaller than the original):

  • The first version read past every unknown kind and kept the chat writable. Every kind Orca has or plans to add changes the queue, a turn or an item, and an older build's rule is fixed the day it ships. An older build reading past such a row would keep writing from a wrong picture of the chat; for example, it would release messages you had stopped. Replaced with the read-only default, which is the common pattern: fail closed, never delete.
  • An intermediate version let the newer Orca mark a row "safe to read past" or "safe to carry through a rewind". Removed: no current or planned kind could use it, and the common pattern has no such mark. It can come with the first kind that needs it.
  • A rewind no longer reorders the restated Stop/Resume: that change was only there to order carried rows, so it went with them.
  • The stored-number check above was found and fixed in review.
  • The list of known row kinds is now built from the row type, so a kind added without its check fails to compile. With the read-only default, a missed entry would have frozen that release's own chats. A new test reads back one row of every kind.
  • The header in journal-row-schema.ts now states the rule for adding a kind:
    • keep the fields every row has;
    • builds from before this change still delete an unknown kind;
    • so a new kind either ships its reader first and is written only once no supported build lacks it, or is written at a bumped format version.

Deferred (not fixed here):

  • An unknown item content kind inside a known row still deletes from that row on. Same class of bug one level down; next follow-up.
  • Orca has no written support window, so "no supported build predates this" can't be measured yet. This is time-sensitive: no release contains the shared chat-history database yet, so if this lands before that release, no stable or RC build that reads the shared database ever deletes this way.
  • Moving the Stop event and the Resume marker to row kinds of their own: ship the reader first, or use a bumped format version.
  • The read-only state shows no notice until a send fails (existing behaviour).
  • Corruption that turns a known kind's name into an unknown one now leaves the chat read-only instead of repairing it. This needs damage that still leaves valid JSON, so it is unlikely.
  • The opening row with the wrong kind still wipes the chat with no notice (existing behaviour).
  • Code cleanups:
    • tell "newer version" and "newer kind" apart in diagnostics;
    • a stored row with wrong column types is silently skipped instead of being treated as damage.

Verified (head 7704a6b, CI green):

  • Three review rounds plus a readiness checklist, with no open P0–P2 issues.
  • Tests: the new unit file (17 tests) and the cross-version downgrade test. In that test, this build stays read-only with every row byte-identical, while the pinned pre-change build deletes the row.
  • 1,636 journal and session-wire tests pass, and the scoped tc:node typecheck passes.
  • Each piece was reverted on its own, and each reversion turns tests red. The one exception is the opening-row kind check, which no current kind can reach.
  • No row reaches the wire; the chat history lives only on the host that runs the agent, so SSH sessions behave the same.

Not verified: no app run or live QA. This is a host-only change with no new UI, and the read-only state is the existing one a newer format version triggers. Live QA was skipped by agreement.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 2, 2026 01:16
…row union

Each kind's own-field check now lives in one table keyed by every kind JournalRow holds, and
the set of kinds this build knows is derived from that table. A kind added to the union without
a check fails to compile, rather than latching this build's own chats read-only as a newer
build's kind. A test reads one valid row of every kind.
@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Fixes how the chat journal handles unknown row types.

The PR appears safe to merge; no actionable new issue or outstanding previous finding was identified.

Summary

The PR preserves unknown journal row kinds by opening the chat read-only instead of deleting its history. It also treats a known row whose embedded sequence differs from its stored key as damaged, and adds unit and cross-version coverage.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Read stored journal row] --> B{Parse and validate}
  B -->|Known, valid kind and matching sequence| C[Fold into chat]
  B -->|Unknown kind with valid envelope| D[Keep rows; open read-only]
  B -->|Malformed row or mismatched known-row sequence| E[Repair from stored sequence]
Loading

Reviews (2) · Last reviewed commit: "refactor(native-chat): derive the journa..."

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one minor documentation nit.

Reviewed changes

  • Unknown journal row kinds now fail closed to read-only instead of deleting history. parseJournalRow reports an unknown non-empty kind that carries the shared row envelope (epoch/seq/fence/ts) as unreadable, so an older build keeps every row and refuses writes until it is updated, matching the existing newer-v path. journal-open.ts then latches readOnly with truncateFrom omitted, so nothing is deleted; the user-facing send refusal is unchanged because isJournalWrittenByNewerOrca keys off journal_read_only (journal-open-failure.ts:66).
  • A row whose body seq disagrees with its stored key is now damage at the key. startJournalRowFold truncates from the stored sequence instead of trusting the body's number, fixing a valid earlier row being deleted (inner lower) and every later write failing on a duplicate number (inner higher). Only rows parseJournalRow accepts are subject to the check, so a newer kind stays read-only first.
  • Tests. A new 16-test unit file covers the parse cases, read-only open/write-refusal/reopen/upgrade, and the seq-mismatch repair; a new cross-version test pins the pre-change baseline still deleting an unknown kind while this build keeps it.
  • Comments/types in journal-row-schema.ts, agent-session-journal-types.ts, and journal-open.ts now describe the envelope rule a new kind must keep.

I ran pnpm test src/main/native-chat/agent-session-journal (37 files / 400 tests) and the new file plus journal-row-schema.test.ts (39 tests); all pass. The readOnly block on writes was confirmed through assertJournalWritable in journal-row-writer.ts, journal-queued-messages.ts, and journal-epoch-controller.ts.

ℹ️ Nitpicks

  • src/main/native-chat/agent-session-journal/journal-write-guards.ts:21 and the journal_read_only message at line 27 still say the journal "uses a newer schema", but readOnly now also latches on an unknown row kind. Only the developer log line is affected — the user-facing refusal is keyed off journal_read_only and reads correctly — so widening the wording can wait for the next touch.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 499372c2-74c1-4d83-827e-667cc3a70a28

📥 Commits

Reviewing files that changed from the base of the PR and between 8c1670f and 7704a6b.

📒 Files selected for processing (5)
  • src/main/native-chat/agent-session-journal/journal-open.ts
  • src/main/native-chat/agent-session-journal/journal-row-schema.ts
  • src/main/native-chat/agent-session-journal/journal-unknown-row-kind.test.ts
  • src/shared/agent-session-journal-types.ts
  • tests/e2e/cross-version-wire/agent-session-stop-event-downgrade.unit.test.ts

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


📝 Walkthrough

Walkthrough

The journal parser now classifies unknown row kinds with a valid envelope as unreadable. Journal loading preserves unreadable rows, sets the journal read-only, and does not count them as malformed. The loader treats malformed rows and embedded-sequence mismatches as truncation points. Tests cover parsing, reopening, repair, append rejection, and cross-version behavior.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 7704a

An older Orca now keeps a chat containing a newer row kind read-only and preserves its history instead of deleting it. No merge-blocking issues were found.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 7704a

The change favors preserving conversation history over deleting data an older build cannot understand. Inspected write and recovery paths respect read-only mode. No introduced security vulnerability was established, but concurrent-handle behavior and some identity checks remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated trigger is a row in one session's persisted history. A party able to alter the shared journal database could target multiple chats in that state directory, but the inspected change does not give unknown rows reducer or execution authority. Remote reachability and filesystem permission enforcement were not established.

Trust Boundaries and Controls

  • observed — For an opened read-only store, inspected controls refuse ordinary appends and epoch replacement, prevent suffix deletion and repair reseeding, and skip stale-roster settlement and queued-message repair. Read-only takes precedence over destructive recovery in these paths.

Resilience and Maintainability Implications

  • observed — The read-only latch and write queue belong to individual journal instances. The inspected guards establish containment for that instance, not a database-wide latch. Exclusive ownership or coordination with a pre-existing same-session writable handle remains unverified; this limitation is not established as introduced or worsened by the PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the primary change: older Orca versions preserve chat history and enter read-only mode when they encounter a newer row kind.
Description check ✅ Passed The description is detailed and covers the user impact, implementation, rationale, limitations, visual proof, and testing. It omits the required linked issue and repository checklist sections, but the…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes (delta since f30c080b)

  • Known row kinds are now derived from the row union. journal-row-schema.ts replaces the hardcoded ROW_KINDS set with ROW_FIELD_CHECK_BY_KIND: Record<JournalRow['kind'], …> and KNOWN_ROW_KINDS = new Map(Object.entries(…)). isJournalRow looks up the per-kind check and gates on the envelope, and parseJournalRow reuses the same map for the unknown-kind test. The old explicit kind branches moved into the table unchanged, so behavior is preserved; a kind added to the union without a check now fails to compile instead of silently reading as a newer build's kind.
  • A guard test pins every kind. journal-unknown-row-kind.test.ts adds rowOfEveryKind(), typed as a mapped type over JournalRow['kind'], and asserts each parses as { ok: true, row }.

I re-ran pnpm test src/main/native-chat/agent-session-journal (37 files / 401 tests) and pnpm tc:node, both clean at 7704a6b3.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

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