fix: stop inferring V2 provider-checkpoint ownership from array position - #4
Open
ranxianglei wants to merge 2 commits into
Open
ranxianglei wants to merge 2 commits into
ranxianglei wants to merge 2 commits into
Conversation
claimCheckpointRanges now claims a provider checkpoint only when exactly one unclaimed candidate exists in its window. More than one means re-expanded originals after an incompatible model switch; zero means the checkpoint is absent from the outgoing view. Both stay uncorrelated so host-owned messages remain individual opaque entries instead of being swallowed into the checkpoint entry. buildProviderCheckpoint renders lowerCompactionText(source) when no outgoing indices exist, so the direct-tool view (outgoing = []) keeps the checkpoint's summary and recent context instead of normalizing to zero parts. Unsupported windows are disclosed via providerCheckpoint + empty outgoingMessageIndices (documented on V2ProvenanceEntry). Tests: compatible native checkpoint regression lock, incompatible model switch with re-expansion (object-identity patch round-trip), direct-view rendering. Bug-catching tests verified to fail at HEAD without the fix. Refs: #425
…s compressed away
Dual-agent review of the provenance guard found a cross-module blocker: once the shared engine compresses the normalized checkpoint away, restoreMissingV2OpaqueSources rejected the whole patch for the uncorrelated entry ('no exact lowered correlation') and lib/v2/context.ts preserved the raw provider request - silently disabling every ACP edit for the rest of incompatible-switch sessions. A provider checkpoint with empty outgoingMessageIndices is now skipped instead of rejected: the outgoing request already carries its information as re-expanded host-owned originals. Non-checkpoint opaque sources keep the fail-closed rejection.
Also applies review nits: protected-flag pin, disclosed invalid direct-view pin, types.ts comment precision, DESIGN.md residual-limitation section, restore regression test. Verified the regression test fails against the unfixed restore.ts.
Refs: #425
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes ranxianglei#425
Problem
ACP assumed
ctx.session.context({sessionID})(public context) and the model-aware runner history (SessionProviderContext.provenance(model)+ checkpoint re-expansion) describe the same boundary. They diverge across native checkpoints:claimCheckpointRangesthen classified the re-expanded originals as one opaque checkpoint by array position (start = previous source's max outgoing index + 1,end = next source's min outgoing index). Direct-tool history could not address any original message, and every original became an unaddressable opaque blob.normalizeV2ProjectedHistory(projected, [])): a provider checkpoint with no outgoing values normalized to zero parts, so direct tools saw an empty checkpoint.Repro confirmed against official v2.0.3
toLLMMessages: public[nativeCheckpoint, nextUser], outgoing[originalUser, originalAssistant, nextUser]; ACP reported a valid projection whose checkpoint claimed outgoing[0,1]as opaque.Fix
claimCheckpointRanges(lib/v2/projection/normalize.ts) now collects the unclaimed candidates inside the positional window and claims a provider checkpoint only when exactly one candidate exists. Two or more candidates mean re-expanded originals from an incompatible model switch; zero means the checkpoint is absent from this outgoing view. In both cases nothing is claimed: the originals stay individually addressable as ordinary sources, and the host's own opaque entries remain byte-identical (verified throughapplyV2ContextPatchround-trip).buildProviderCheckpointrenders the checkpoint from source data when it has no outgoing correlation:lowerCompactionText(source)under origin keysource:{i}:checkpoint:source, marked opaque, so direct tools see the real summary/recent content instead of zero parts. Because the rendered message carries a known normalized ID, the patcher can never inject it into outgoing requests — it is visible only in the normalized view.restore.ts(found during review): once the shared engine compresses the normalized checkpoint away,restoreMissingV2OpaqueSourceswould reject the whole patch for the uncorrelated entry ("no exact lowered correlation") andlib/v2/context.tswould preserve the raw provider request — silently disabling every ACP edit for the rest of such sessions. A provider checkpoint with emptyoutgoingMessageIndicesis now skipped instead of rejected: the outgoing request already carries its information as re-expanded host-owned originals. Non-checkpoint opaque sources keep the fail-closed rejection.V2ProvenanceEntry.providerCheckpointdoc comment documents the unsupported-window disclosure and the single-re-expanded-original residual; DESIGN.md §5 records all known residual limitations.No version bump, no publish, no merge. No changes to V1 code paths, persisted state formats, or internal tags.
Review
Two independent agent reviews (AGENTS.md §5.3 + §5.6). The code reviewer found the
restore.tsblocker above (new failure mode in exactly the scenario this PR fixes) — fixed in the second commit with a regression test verified to fail against the unfixed code. All other findings were nits, applied.Verification
tests/soft-block.test.tshardcodingmkdirSync('/tmp/opencode-dcp-dangerous-<pid>')at import time, which crashes on this sandbox's read-only/tmpmount (environmental, pre-existing, passes in CI — same failure noted in PR fix: recognize failed V2 ACP tool results as failures #3).devlog/2026-09-17_v2-checkpoint-provenance-guard/) complete.Related finding (reported on the issue thread, not fixed here)
The direct-tool view rejects any user/assistant history with
valid: false("Patchable origin source:N:part:M has no exact lowered outgoing match") even though.messagesare fully built and consumed bylib/v2/host.ts. Pre-existing on master, unrelated to this change.