Repository navigation
Keep provider context history readable after downgrade - #26545
brennanb2025 wants to merge 4 commits into
Conversation
|
Older Orca builds could set aside a Claude or Codex chat when its saved provider history contained a fresh replacement. This PR keeps the latest context readable to those builds and preserves earlier provenance through their normal writes; it also ensures a visible owner takes priority when a downgraded build adopted an archived context elsewhere. The clear command itself is unchanged; same-conversation clear and orchestration cleanup follow separately. Three independent review rounds finished with no P0–P3 findings. Round 1 fixed a provider-session address selecting the archived owner instead of the visible owner; round 2 fixed compatibility tests that assumed the moving release baseline could never read archives; round 3 verified both closed and ran 292 passing test executions. The final main sync was conflict-free, followed by 234 focused/control/historical cases, 50 orchestration caller cases, a repeated 28-case compatibility proof, all three queued typechecks and all 22 static checks. The uploaded terminal-output screenshot is in the PR body. Final-head CI ( Actual historical modules, durable record writes, growth beside a full archive, resume-point changes, replacement of an unsaved context and subsequent branching and ownership on re-upgrade were exercised. Live providers, rendered app flows, live SSH/WSL/paired clients and hardware Windows/Linux were not run; this is storage-only test evidence. Representative durable-write/startup budgets passed, but the disclosed 2048-link timing spike remains and its shared-load/garbage-collection cause was not independently measured. No migration, typed-provider restore fallback or clear UI was added. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (19)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe change adds version-1 persistence for replaced provider-context links and updates provider-handle chain validation to apply a 256-link limit to the current context. Provider ownership lookups now use claims that distinguish archived links from visible links. The change also adds unit and cross-version tests for persistence, ownership, adoption, and historical store behavior. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No merge-blocking issue is established for provider-context history or ownership. The change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. Comment |
|
Closing: this isn't needed. This PR changed how a chat saves its list of agent sessions, so that an older Orca build could still open a chat after the in-place Instead, #26579 will be reworked onto main: on |
ELI5
A chat can need a fresh agent context while keeping its earlier history. The current storage rules reject that transition for Claude and Codex because older Orca builds would stop reading the whole chat. This prerequisite lets older builds read the latest context and lets an upgraded build recover the earlier provider history. The existing /clear behavior is unchanged in this PR; in-place clear follows separately.
What Changed
Store the latest provider context in the existing handle array, with earlier links in one optional, disjoint record-level archive. New readers reconstruct the truthful full chain; older writers preserve the archive while resuming, changing Claude's resume point, forking, or superseding a context the provider never saved. Records without replacements retain their existing bytes.
The 256-link limit applies to each context, so an older writer can grow its visible segment without invalidating the archived history on upgrade. Validation uses an iterative walk without recursive archive nesting. If an older build adopts an invisible historical context into another chat, that chat's visible claim wins on upgrade, including provider-ID address corrections; an archived claim reserves history against another adoption and cannot override the visible owner.
Why
Keeping one conversation requires separating its provider history from the context it currently resumes. A record-level archive survives old writers that rebuild the handle array, while an archive attached to the replaced link would be lost. Retaining truthful creation links avoids pretending a discarded context was resumed or forked. This also supports failed-restore replacement storage, without enabling a new Claude/Codex fallback policy here.
Differences from the common pattern:
Linked Issue
N/A — maintainer follow-up from a native chat audit
Visual Proof
Captured terminal output from the actual historical-module suite on final head
fd9234eed1f: shipped v1.4.222 and the prerequisite base each read and save the new Claude/Codex records, with the archive-aware HEAD control also passing. All 28 cases pass. This is a headless rendering of unabridged test stdout, not live provider or app UI evidence.Testing
The reliability class is agent-session.provider-ownership: the oracle is actual old read/write/adoption followed by a new reader retaining ancestry and selecting the existing visible owner. The matching gate is experimental and partial; its relevant inexpensive controls pass. Local/daemon PTY lifecycle and geometry are unaffected. Storage remains execution-host owned with no new RPC or stream opcode. Hardware Linux/Windows, live SSH/WSL/paired clients, and real providers were not run for this storage-only prerequisite.
Review
Round 1 found a P2 ownership lookup issue, fixed with focused and existing caller tests; round 2 verified it closed. Round 2 found a P2 rolling-baseline test assumption, reproduced against this implementation and fixed with actual extracted-module support detection, latest-head writes, archive-aware adoption refusal and forward controls. Fresh round 3 found no P0–P3 issues and verified both earlier findings closed; it executed 292 passing tests with matrix overlap disclosed. The final main sync was conflict-free; no authored implementation change followed the clean review.
Agent skill upstream boundary
Notes