fix(agent-core-v2): skip known legacy wire records on restore#2211
fix(agent-core-v2): skip known legacy wire records on restore#2211ktao732084-arch wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 45c0453 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45c0453427
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * identity. Replay silently skips `LEGACY_RECORD_TYPES` — journal vocabulary | ||
| * this engine knows of but deliberately does not replay: records of retired | ||
| * features (micro compaction) and v1-only bookkeeping it recomputes live | ||
| * (context token counts). Genuinely unknown types are still reported, and | ||
| * healing rewrites preserve legacy records for readers that understand them. |
There was a problem hiding this comment.
Remove the new replay/allowlist details from this header: they narrate branch behavior and enumerate implementation decisions, so the comment will become stale whenever the legacy set or rewrite logic changes. The scoped guide requires top-of-file comments to describe only the module’s external responsibility and explicitly forbids narrating implementation steps.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L11-L15
Useful? React with 👍 / 👎.
| expect(target.wire.getModel(CounterModel)).toEqual({ value: 3 }); | ||
| }); | ||
|
|
||
| it('skips legacy records in a metadata-first journal and preserves them through the healing rewrite', async () => { |
There was a problem hiding this comment.
This single it checks both silent replay and preservation during a protocol migration, so a failure does not identify which contract regressed and the skip behavior is unnecessarily coupled to rewrite mechanics. Keep the silent-skip/replay assertion in one scenario and move the metadata upgrade plus record-preservation assertions into a separately named test.
Useful? React with 👍 / 👎.
1 participant
Related Issue
Resolve #2003
Problem
Resuming a session whose
wire.jsonlwas written by an earlier version reports unexpected errors during restore:micro_compaction.applywas persisted by the micro-compaction experiment while it was live. The feature has since been retired (removed from the v1 engine in #1317), and the v2 engine never registered an op for it, soWireService.replayRecordtreats every occurrence as an unknown record and routes it throughonUnexpectedError— once per record, which for a long session means hundreds of reports (the linked issue showsindex: 1035), even though skipping the record is exactly the intended outcome.context.update_token_countis in the same situation: v1-only bookkeeping (v1 still writes it on the import-context path) that v2 deliberately does not replay because it recomputes context size live.Reproduced against the 0.29.1 release build (v2 print mode) by adding a
micro_compaction.applyrecord to a session journal: restore prints the exact error and stack from the issue. With this patch, the same session restores silently.What changed
WireService.replayRecordnow consults a small allowlist (LEGACY_RECORD_TYPES) before reporting a skipped record. The two legacy types are still skipped, but silently; genuinely unknown types keep raisingwire.unknown_record, so the report stays meaningful for real journal corruption or version skew. The journal itself is untouched — healing rewrites preserve legacy records, since the v1 engine and the vis replay tooling still understand them.Two cases added to the existing wire-compat round-trip suite: a journal carrying both legacy types restores with no unexpected error while later records still replay, and a metadata-first 1.4 journal keeps its legacy record through the healing rewrite while its metadata upgrades to 1.5.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.