Skip to content

fix(v2): validate context patches by provenance instead of blanket AiMessage.make (#455) - #470

Closed
ranxianglei wants to merge 2 commits into
2026-09-17_v2-repeated-system-textfrom
2026-09-29_v2-media-validation
Closed

ranxianglei wants to merge 2 commits into
2026-09-17_v2-repeated-system-textfrom
2026-09-29_v2-media-validation

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Fixes #455

What

validateFinalMessages in lib/v2/projection/patch.ts re-ran the public AiMessage.make(...) over every assembled outgoing message before accepting a V2 context patch. On @opencode/ai >= 2.0.18, MediaPart.media is an Asset-instance nominal type (Schema.declare((value) => value instanceof Asset)) that only the host's own module copy satisfies — so ACP's copy rejected the host's own media instances and rolled back the whole patch for any attachment-bearing message (accepted === false, invalid-schema). ACP pruning / protected-content retention / compression blocks were silently dropped and the turn proceeded unpruned.

Fix

Validate by provenance, not by nominal schema:

  • Build an authored set at the call site: replacements ∪ insertions — exactly the messages ACP mutated or created (the only path by which ACP touches a message).
  • validateFinalMessages skips host-owned messages from nominal re-validation (trusted-valid, carried through untouched).
  • For ACP-authored messages, dispatch on authorship class:
    • Fully ACP-composed insertion (isAcpOwnedId) → full AiMessage.make (all parts are ACP text; safe).
    • ACP-cloned host message → new delta check validateAcpAuthoredParts: every part whose id is prt_dcp_text_* / prt_dcp_summary_* must be well-formed text; provider parts (media/metadata/tool results) are ignored.

New exported helpers isAcpAuthoredPartId + validateAcpAuthoredParts; validateFinalMessages is now exported for unit tests. The curated ../lib/v2/projection barrel deliberately does not re-export them, so the public surface is unchanged.

Behavior change (old → new)

Old New
Host message with attachment re-validated via AiMessage.make → rejected invalid-schema on 2.0.18+ → whole patch rolled back skipped from nominal validation → accepted; attachment survives by object identity
ACP-composed insertion (summary/text notice) AiMessage.make unchanged (AiMessage.make)
ACP edit of a host message AiMessage.make over the clone (trips on provider parts) delta check on ACP-authored parts only
Protective invariants (duplicate-message-id, duplicate-call-id, invalid-tool-pair, invalid-order) fire unchanged, still fire (re-tested T1.5 / T1.5b)

This restores precisely the invariant blanket validation was written for — ACP never authors a malformed part — without rejecting host-owned data.

Tests

tests/v2-context-patch.test.ts (+6, now 35 in file). The cross-module-instance instanceof Asset rejection cannot be reproduced naturally in one process, so a spy on the shared Message.make simulates it version-independently (throws when handed a message containing the foreign media part by identity):

  • T1.1 replay attachment fixture under simulated 2.0.18 rejection → accepted === true; make never handed the host media message.
  • T1.2 attachment reaches finalMessages by object identity.
  • T1.3 an ACP prune (text → empty) of the same message still accepted; attachment survives.
  • T1.4 delta-check unit (provider parts ignored; malformed ACP part → invalid-schema; id discriminator).
  • T1.5 protective invariants still reject: duplicate id → duplicate-message-id; orphaned tool result / removed-but-present call → invalid-tool-pair.
  • T1.5b order shift of retained sources → invalid-order.

Regression proof: reintroducing the blanket loop makes T1.1/T1.2/T1.3 fail; restoring the fix makes them pass.

Full suite: 1422/1423 pass. The single failure is pre-existing and unrelated — tests/soft-block.test.ts hardcodes /tmp/opencode-dcp-dangerous-${pid} and mkdirSyncs at module load, which crashes under a read-only /tmp sandbox regardless of this change (fails identically on the base branch here). Candidate follow-up: fs.mkdtemp(os.tmpdir()).

Base branch note

Targets 2026-09-17_v2-repeated-system-text (the V2 integration line), not master — the entire V2 port (~1400 commits) is WIP and not yet on master, so a master-targeted PR would show the whole V2 subsystem as new files. This fix lives entirely in lib/v2/. Please redirect the base if you have a different V2 integration target.

…Message.make (#455)

validateFinalMessages re-ran public AiMessage.make over every assembled outgoing message. On @opencode/ai >= 2.0.18 media is an Asset-instance nominal type only the host's own module copy satisfies, so ACP's copy rejected valid host data and rolled back the whole patch for any attachment-bearing message (accepted=false, invalid-schema).

Gate validation on message provenance: skip host-owned messages from nominal re-validation (trusted-valid); run full AiMessage.make only for ACP-composed insertions (isAcpOwnedId) and a new delta check otherwise. The delta check validates only ACP-authored parts (prt_dcp_text_*/prt_dcp_summary_*), requiring well-formed text; provider parts ignored.

Protective invariants unchanged: duplicate-message-id, duplicate-call-id, invalid-tool-pair, invalid-order still fire (re-tested). Tests T1.1-T1.5b simulate the 2.0.18 media rejection version-independently via a spy on Message.make; verified to fail when blanket validation is reintroduced.

Closes #455
- reuse isAcpAuthoredPartId() in the transformed-side provenance gate instead of duplicating the prt_dcp_text_/prt_dcp_summary_ prefix check (DRY; behavior-identical, adds undefined-id safety)

- correct the validateFinalMessages doc comment: the curated barrel is lib/v2/projection.ts (a file), not '../lib/v2/projection'

No behavior change.
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 结论:PR #470 不应合并——它修的是已退役的 V2 移植代码。按 #442 决议,本仓库对 OpenCode 2.x 统一走 billion-context、lib/v2/* 不再推进。我已将其关闭(非合并),标注由 billion-context 取代。

核对到的事实

  • 本 PR 是 #455 的修复:lib/v2/projection/patch.ts 的 validateFinalMessages 对每条组装出的消息整体跑 AiMessage.make(...);在 @opencode/ai >= 2.0.18 上,MediaPart.media 是跨模块副本无法满足的 instanceof Asset nominal 类型,于是宿主自带的附件消息被判 invalid-schema,整段补丁回滚(accepted === false)。
  • 改动范围全部落在退役的 V2 树上,不触碰 master / V1 路径:
    • lib/v2/projection/patch.ts (+79/−15)
    • tests/v2-context-patch.test.ts (+258/−1)
    • devlog/2026-09-29_v2-media-validation/{REQ,DESIGN,WORKLOG}.md
  • base 分支 2026-09-17_v2-repeated-system-text 仍存在;head commit 的 CI 状态为 pending(未出结果)。

为什么不该合并(路由依据)

流程偏差(如实指出)
本 PR 创建于 09-29 13:01Z,比 #442 决议(09-19)晚了约 10 天。也就是说初次处理 #455 时没有先命中 #442 的路由判断,直接在本仓库开工并开了 PR,直到你 13:44Z 才把方向拨到 billion-context。这是初次处理漏检了路由——后续对同类 V2 报告应先查 #442 再动手,而不是先实现。

处置与待办


中文摘要:PR #470 修的是已退役的 V2 移植代码(lib/v2/*),按 #442「本仓库只留 1.x、V2 走 billion-context」的决议不应合并,我已关闭并标注由 billion-context 取代;同时指出该 PR 在 #442 决议后约 10 天才开、属初次处理漏检路由,另留 #429/#430 与 billion-context 转投两项等你拍板。

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