fix: V2 provider checkpoints must not claim correlated tool messages (#456)

devlog/2026-09-17_v2-checkpoint-provenance-guard/DESIGN.md

@@ -0,0 +1,55 @@
+# DESIGN - V2 provider-checkpoint provenance guard
+
+- Task ID: `2026-09-17_v2-checkpoint-provenance-guard`
+- Date: 2026-09-17
+
+## 1. Problem Shape
+
+ACP's V2 layer normalizes a **projected** (public) history and correlates it with the **outgoing** (model-aware, lowered) request messages via provenance entries. Two views legitimately diverge across native provider compaction checkpoints in OpenCode V2:
+
+```
+public context (projected):  [checkpoint, nextUser]
+outgoing after compatible model decode:      [decodedCheckpoint, nextUserLowered]
+outgoing after incompatible model switch:    [originalUser, originalAssistant, nextUserLowered]
+direct-tool view (host.ts passes []):        []
+```
+
+Before this change, `claimCheckpointRanges` claimed **every unclaimed outgoing index** between the previous source's last claimed index and the next source's first claimed index as checkpoint content. That is a positional inference: it works only when the window contains exactly the decoded checkpoint message itself.
+
+## 2. Decision
+
+**Claim by exact uniqueness, never by position.** Collect the unclaimed candidates in the window:
+
+| Candidates | Meaning | Action |
+|------------|---------|--------|
+| exactly 1 | decoded checkpoint message | claim it (compatible case, unchanged behavior) |
+| >1 | re-expanded originals from an incompatible model switch | leave all uncorrelated |
+| 0 | checkpoint absent from the outgoing view (incompatible switch or direct-tool view) | leave uncorrelated |
+
+Uniqueness here is positional, not content-based — see the first item in §5 for the single-re-expanded-original residual.
+
+Consequences of "uncorrelated":
+
+1. **Outgoing side**: each uncorrelated host message becomes its own `V2OutgoingProvenance` entry with `opaque: true` and `opaqueMessage = <message>` (`normalize.ts`, existing logic). The patcher rejects any non-object-identical replacement of such messages and keeps empty ones alive, so re-expanded originals survive into the final request byte-for-byte and remain individually identifiable in provenance.
+2. **Normalized side**: `buildProviderCheckpoint` now renders the entry from source compaction data when it has no outgoing indices — `lowerCompactionText(source)` (existing `<conversation-checkpoint>` envelope with summary + recent context) under origin key `source:{i}:checkpoint:source`, opaque text origin/part. The direct-tool path therefore sees the checkpoint content instead of zero parts.
+3. **Patch path safety** (verified in `patch.ts`): a normalized message whose entry has `outgoingMessageIndices: []` is skipped on removal without rejection; its opaque origins have no outgoing pointers so no content edits can be mapped; and because it carries a known normalized ID it can never become an ACP insertion — the fallback-rendered checkpoint is never injected into outgoing requests.
+4. **Disclosure**: an unsupported window is disclosed structurally — `providerCheckpoint: true` with `outgoingMessageIndices: []` — and documented on `V2ProvenanceEntry.providerCheckpoint`. No rejection is raised: the projection remains valid and usable, which matches the issue's "safe explicit fallback" acceptance option.
+5. **Restoration safety** (`restore.ts`, found during dual-agent review): once the shared engine compresses the normalized checkpoint away, `restoreMissingV2OpaqueSources` would otherwise reject the whole patch for an uncorrelated entry ("no exact lowered correlation") from `lib/v2/context.ts:187` — silently disabling every ACP edit for the rest of such sessions (a new failure mode this fix must not introduce). A provider checkpoint with empty indices is now skipped instead of restored: the outgoing request already carries its information as re-expanded host-owned originals, and fabricating a message the host never sent is worse than omitting one. Non-checkpoint opaque sources keep the fail-closed rejection.
+
+## 3. Trade-offs Considered
+
+- **Host-provided model-aware snapshot** (issue's first acceptance option): cleanest long-term, but requires new host API surface in OpenCode V2; not available to ACP today. Rejected for this iteration; the fallback covers both views without host changes.
+- **Reject (>1 candidates) instead of fallback render**: fail-closed, but would break every incompatible-switch session outright; the divergence is benign once ownership stops being inferred, so degrading gracefully is safer.
+- **Claiming extras conservatively (old behavior)**: keeps the checkpoint correlated even when provider-added messages sit in the window, but swallows host-owned content into opaque checkpoint territory — the exact misclassification this fix removes. Note the behavioral change: with provider-added extras present, the checkpoint renders from source data rather than decoded outgoing content. Acceptable because the entry stays opaque/protected/non-removable either way.
+
+## 4. Invariants Preserved
+
+- Compatible single-decoded-checkpoint windows correlate exactly as before (regression-locked by test).
+- Opaque provider checkpoints remain protected and non-removable (`patch.ts` guard unchanged).
+- No persisted-state format, internal-tag, or public-API signature changes.
+
+## 5. Known Residual Limitations
+
+- **Single re-expanded original is ambiguous** (pre-existing, not a regression): if the checkpoint covered exactly one transcript message and the incompatible switch re-expands just that one, the window holds exactly one candidate and ACP claims it as the decoded checkpoint — identical to pre-fix behavior. Content-preserving either way (the entry stays opaque/protected); hardening would require cross-checking against `source.providerContext`, which is empty for most checkpoints.
+- **An earlier unclaimed checkpoint widens the next window**: windows are computed over all drafts in source order. If an earlier checkpoint is incompatible (claims nothing), its re-expanded originals sit inside the *next* checkpoint's candidate scan, pushing its count above 1 and leaving a genuinely compatible decoded checkpoint uncorrelated (rendered from source data instead of the decoded message). Fail-safe and content-preserving; do not "simplify" the windowing without re-reading this.
+- **Multi-message decoded checkpoints are now uncorrelated**: pre-fix, a checkpoint lowering to multiple outgoing messages had all of them claimed; post-fix (>1 candidates) none are claimed. Content is still preserved via `lowerCompactionText`; correlation precision degrades only for providers that decode a checkpoint into more than one message.

devlog/2026-09-17_v2-checkpoint-provenance-guard/REQ.md

@@ -0,0 +1,61 @@
+# REQ - V2 provider-checkpoint provenance guard
+
+- Task ID: `2026-09-17_v2-checkpoint-provenance-guard`
+- Home Repo: `opencode-acp`
+- Created: 2026-09-17
+- Status: InProgress
+- Priority: P1
+- Owner: ranxianglei
+- References: https://github.com/ranxianglei/opencode-acp/issues/425
+
+## 1. Background & Problem Statement
+
+- **Context**: OpenCode V2 serves two different histories for the same session: the public context (`session.context`, used by ACP direct tools) always starts at the latest native compaction checkpoint, while the model-aware runner history excludes an incompatible provider checkpoint and re-expands the original transcript underneath it.
+- **Current behavior (symptom)**:
+  1. `claimCheckpointRanges` claims every unclaimed outgoing message in the positional window around a provider checkpoint. After an incompatible model switch that window contains the re-expanded originals, so they are misclassified as opaque checkpoint content — neither original ID is individually addressable in provenance.
+  2. When the checkpoint is absent from the outgoing view entirely (direct-tool path passes `outgoing = []`), `buildProviderCheckpoint` loops over zero indices and normalizes the checkpoint to zero parts, so the model loses the checkpoint's summary/recent context on the direct path.
+- **Expected behavior**: Checkpoint ownership must be established only by exact identity, never inferred from array position. Uncorrelated outgoing messages stay individual opaque host entries. The checkpoint remains visible in the normalized view from source compaction data even when it has no outgoing counterpart. Unsupported windows are disclosed via the entry shape (`providerCheckpoint: true` + empty `outgoingMessageIndices`).
+- **Impact**: Public context and runner history diverge silently; ACP could treat host-owned re-expanded messages as removable/checkpoint-owned, and direct tools see a checkpoint with no content.
+
+## 2. Reproduction (if applicable)
+
+- **Environment**:
+  - Node: 22
+  - OS/Arch: linux
+- **Minimal reproduction steps** (isolated adapter repro from issue floor 2, official v2.0.3 `toLLMMessages`):
+  1) Public context: `[nativeCheckpoint, nextUser]`; outgoing request: `[originalUser, originalAssistant, nextUser]` after an incompatible model switch.
+  2) Normalize → checkpoint entry claimed outgoing `[0, 1]` as opaque; original IDs unaddressable.
+  3) Direct-tool view (`outgoing = []`) → checkpoint normalized to zero parts.
+- **Relevant configuration**: none (V2 projection defaults).
+
+## 3. Constraints & Non-Goals
+
+- **Constraints**:
+  - Backward compatibility: no persisted-state or internal-tag changes; existing compatible-checkpoint behavior (single decoded message claimed) must be unchanged.
+  - Performance requirements: no new allocations beyond the candidate collection per checkpoint entry.
+  - Resource limits: fail-closed philosophy preserved — never guess ownership; when ambiguous, keep everything opaque/uncorrelated.
+- **Non-Goals** (explicitly out of scope):
+  - Switching ACP to a host-provided model-aware source snapshot (would require new host API surface; the safe explicit fallback chosen here covers both views).
+  - Fixing the pre-existing direct-view rejection (`Patchable origin ... has no exact lowered outgoing match`) that occurs for any user/assistant source with empty outgoing — recorded as a finding in issue #425, not fixed here.
+  - Fixing `tests/soft-block.test.ts` hardcoding `/tmp/opencode-dcp-dangerous-*` (fails only where `/tmp` is read-only; environmental).
+
+## 4. Acceptance Criteria (must be testable)
+
+- **Correctness**:
+  - [x] Compatible native checkpoint (exactly one unclaimed candidate in window) is still claimed and rendered from outgoing content; patch round-trip keeps it object-identical.
+  - [x] Incompatible model switch with re-expansion (>1 candidates): checkpoint stays uncorrelated (`outgoingMessageIndices: []`), renders from source compaction data; each re-expanded original remains its own opaque host entry; no-op patch keeps all outgoing messages object-identical and injects nothing.
+  - [x] Direct-tool view (`outgoing = []`) (0 candidates): checkpoint renders summary + recent context instead of zero parts.
+  - [x] Unsupported window disclosed through entry shape + documented on `V2ProvenanceEntry.providerCheckpoint`.
+- **Performance / Stability**:
+  - [x] No new failure modes: all previous v2 projection/patch tests still pass.
+- **Regression**:
+  - [x] New/modified test cases added to test suite and passing (`tests/v2-message-projection.test.ts`); bug-catching tests verified to FAIL at HEAD without the fix.
+
+## 5. Proposed Approach (optional)
+
+- **Affected modules & entry files**:
+  - `lib/v2/projection/normalize.ts` — `claimCheckpointRanges`, `buildProviderCheckpoint`
+  - `lib/v2/projection/types.ts` — disclosure doc comment on `V2ProvenanceEntry.providerCheckpoint`
+  - `tests/v2-message-projection.test.ts` — 3 new tests
+- **Risks**: Windows where the provider adds extra messages between the checkpoint and the next correlated source now leave the checkpoint uncorrelated (rendered from source data) instead of claiming extras. Acceptable: the entry stays opaque/protected/non-removable either way, and extras remain individually addressable.
+- **Rollback strategy**: Revert the single commit; no state migration involved.

devlog/2026-09-17_v2-checkpoint-provenance-guard/WORKLOG.md

@@ -0,0 +1,47 @@
+# WORKLOG - V2 provider-checkpoint provenance guard
+
+- Task ID: `2026-09-17_v2-checkpoint-provenance-guard`
+- Branch: `2026-09-17_v2-checkpoint-provenance-guard`
+- Updated: 2026-09-17
+
+## 1. Commits
+
+| Commit | Description |
+|--------|-------------|
+| (this devlog's single commit) | fix: stop inferring V2 provider-checkpoint ownership from array position; render uncorrelated checkpoints from source data |
+
+## 2. Key Files
+
+| File | Change |
+|------|--------|
+| `lib/v2/projection/normalize.ts` | `claimCheckpointRanges`: collect unclaimed candidates in the window and claim only when exactly one exists (>1 = re-expanded originals after incompatible model switch; 0 = checkpoint absent from outgoing view). `buildProviderCheckpoint`: when `outgoingMessageIndices` is empty, render `lowerCompactionText(source)` under key `source:{i}:checkpoint:source` as an opaque origin/part so the direct-tool path keeps the checkpoint's summary + recent context. |
+| `lib/v2/projection/types.ts` | Doc comment on `V2ProvenanceEntry.providerCheckpoint` documenting the unsupported-window disclosure semantics, including the single-re-expanded-original residual. |
+| `lib/v2/projection/restore.ts` | Review-found blocker fix: `appendRestored` skips a provider checkpoint with empty `outgoingMessageIndices` instead of rejecting the whole patch (the outgoing request already carries its information as re-expanded host-owned originals). Non-checkpoint opaque sources keep the fail-closed rejection. |
+| `tests/v2-message-projection.test.ts` | 3 new tests + `switchCheckpoint` / `stringValue` helpers; review nits applied (`protected === true` pin; direct-view `valid: false` + rejection-code pin). |
+| `tests/v2-context-patch.test.ts` | 1 new regression test: uncorrelated provider checkpoint compressed away → restoration accepted, originals survive by identity, full patch pipeline green. |
+
+## 3. Test Results
+
+- `npm run typecheck` — pass.
+- `node --import tsx --test tests/v2-message-projection.test.ts tests/v2-context-patch.test.ts` — 33 pass / 0 fail (was 29 before this change).
+- Full suite `npm test` — 1414/1415 pass; the single failure is `tests/soft-block.test.ts` with `EACCES: permission denied, mkdir '/tmp/opencode-dcp-dangerous-<pid>'` — environmental (`/tmp` read-only in this sandbox), pre-existing at HEAD, unrelated to this change. Passes on CI where `/tmp` is writable.
+- Bug-catching verification (per §5.7.3 methodology): with the normalize.ts fix stashed, both `keeps re-expanded originals uncorrelated...` and `renders the provider checkpoint from source data...` FAIL at HEAD; with the fix applied they pass. The compatible-view test passes in both states by design (regression lock). The restore regression test fails against the unfixed `restore.ts` with "no exact lowered correlation".
+- `npm run build` — success.
+
+## 3b. Dual-Agent Review (AGENTS.md §5.3 + §5.6)
+
+Two independent agents reviewed commit 71458d18:
+
+- **Test reviewer**: APPROVE-WITH-NITS. Empirically confirmed bug-catching (test 1 fails pre-fix at `outgoingMessageIndices` deepEqual `[0,1]`; test 3 fails pre-fix on missing `source:0:checkpoint:source` part; test 2 passes in both states). Nits applied: pin `protected === true`; pin the disclosed `valid: false` direct-view state. Residual documented gap: no end-to-end decompress-through-both-views coverage exists anywhere in the suite (V1 engine-level decompress tests do not traverse V2 projection/views) — tracked as a follow-up item in the #425 thread, out of scope here.
+- **Code reviewer**: REQUEST-CHANGES → **blocker fixed in follow-up commit**. Cross-module interaction: after an incompatible switch the uncorrelated checkpoint entry is opaque + non-removable, so once the engine compresses it away, `restoreMissingV2OpaqueSources` rejected every subsequent patch ("no exact lowered correlation") and `lib/v2/context.ts:187-200` preserved the raw provider request — ACP edits silently disabled for the rest of such sessions (new failure mode, violating REQ AC#4). Fix: skip (not reject) provider checkpoints with empty indices in `restore.ts`, plus regression test. Nits applied: `types.ts` comment precision; DESIGN.md §5 residual-limitation documentation (single re-expanded original ambiguity, earlier-unclaimed-checkpoint window widening, multi-message decoded checkpoints now uncorrelated).
+
+## 4. Findings Recorded (issue #425 thread)
+
+1. **Pre-existing**: the direct-tool view (`normalizeV2ProjectedHistory(projected, [], ...)`, `lib/v2/host.ts:45,52`) reports `valid: false` with rejection `Patchable origin source:N:part:M has no exact lowered outgoing match` for any plain user/assistant source, because patchable text origins get no lowered pointer when there is no outgoing message. Verified identical at HEAD without this diff. Functional impact today is nil (`host.ts` consumes `.messages`, which are fully built regardless of validity), but the invalid flag is misleading. Needs its own issue from a human.
+2. **Environmental**: `tests/soft-block.test.ts:10` hardcodes `/tmp/opencode-dcp-dangerous-${process.pid}`; fails with EACCES wherever `/tmp` is read-only.
+
+## 5. Lessons Learned
+
+- Position-based ownership inference is unsafe whenever two histories can diverge (public vs model-aware). Exact identity only; ambiguity → keep everything opaque.
+- Running repo-wide `npm run format` with a Prettier version that does not reproduce the committed style (local prettier 3.9.5 wraps at a different width than the committed tree) pollutes the working tree with hundreds of unrelated files. Verify `git status --porcelain` scope immediately after formatting; here the whole tree had to be reverted and the three intended files restored from backup.
+- Always verify new bug-catching tests fail against the unfixed code before trusting them.