Skip to content

fix: close final OpenCode V2 lifecycle, projection, and artifact verification gaps #415

Description

@drexb-ops

Context

Follow-up independent reviews of the OpenCode V2 migration (#395) found several remaining correctness and verification gaps after the initial audit fixes.

Runtime defects

  • A V2 mutating tool can resume after plugin deactivation, mutate/persist live session state, and only then have its wrapper return a shutdown result. Command notice emission has a similar late-deactivation window.
  • Repeated V2 patch application accepts previously produced part objects even when they were reordered or moved between messages, and removal without exact source/output correlation can target a newer same-ID message.
  • V2 projected messages use empty provider/model strings, so nullish-coalescing fallback prevents provider/model-specific compression overrides from using the active state model.
  • Legacy fork state normalizes parent message-ref maps but not four-digit CompressionBlock.startId/endId boundaries.
  • Proxy tool/command reload retry is not compensating when only one domain reload succeeds.

Verification and harness defects

  • The V2 local-wrapper fallback can mask an unrelated tarball activation error unless the exact OpenCode 2.0.3 directory-only diagnostic is required.
  • The installed E2E root check is lexical, allowing .. traversal, and the fixed root is unsafe for concurrent runs.
  • The installed matrix lacks the mandatory nudge-triggered growth/compression/baseline cycle.
  • verify-package.mjs imports workspace self-references rather than an actual staged/installed tarball.
  • Some V2 public-record fixtures and full state commit assertions remain incomplete.

Acceptance

  • Fence or roll back all mutating V2 tool/command effects across unload.
  • Validate repeated patches by expected message identity, position, and order.
  • Restore model overrides and legacy fork boundaries.
  • Make proxy transition reloads compensating/retryable.
  • Harden the isolated E2E root and accept wrapper fallback only for the exact known V2.0.3 limitation.
  • Add the installed nudge growth cycle with exact state assertions.
  • Import all package entrypoints from the packed artifact in package verification.
  • Use schema-valid V2 fixtures and complete state clone/commit coverage.
  • Update the migration devlog and rerun exact V1 1.18.29/V2 2.0.3 E2E plus dual independent reviews.

Related: #395, #404, #407.

Activity

  1. ranxianglei commented on Sep 16, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    [bot] 🏷

    Triage complete — I verified every item against the codebase before touching anything. Bottom line: 9 of the 10 items describe code that does not exist in this repository. The OpenCode V2 migration was deliberately moved out of opencode-acp — see #395 floors 7–11 (@Dog: work goes to billion-context#735 launcher mode; floor 11: "本仓库维持 V1-only,不再做 V2 移植"). The audited codebase is almost certainly ranxianglei/billion-context.

    Item-by-item evidence (opencode-acp @ master 06efd39; open PRs #408/#409/#412/#413 diffs checked too — none contains V2 code)

    # Claim Verdict
    R1 V2 mutating tool late-deactivation window ❌ No V2 tools exist. index.ts is V1-only (import type { Plugin } from "@opencode-ai/plugin"); no @opencode/plugin dependency anywhere (deps: tokenizer/sdk/jsonc-parser/zod)
    R2 Repeated V2 patch application accepts reordered parts ❌ No patch-application code exists (only apply_patch param parsing in lib/protected-patterns.ts, unrelated)
    R3 V2 projected messages use empty provider/model strings ❌ No projection layer exists
    R4 Legacy fork state normalizes parent ref maps but not 4-digit CompressionBlock.startId/endId ✅ Real — details below
    R5 Proxy tool/command reload retry not compensating ❌ No such mechanism; the only reload sites are prompts.reload() (prompt store) and wrapper-dir parsing in lib/update.ts
    V1 V2 local-wrapper fallback masks tarball activation errors ❌ Zero references to "2.0.3" anywhere in the repo; no wrapper-fallback logic
    V2 Installed E2E root check is lexical / fixed root unsafe ❌ No "installed" E2E variant; scripts/e2e/run-e2e.sh:29 derives REPO_ROOT via $(cd … && pwd) (physical path, no .. traversal surface)
    V3 Installed matrix lacks nudge growth cycle ❌ No installed matrix; the Docker E2E already covers the mandatory nudge-triggered growth/compress/baseline cycle per AGENTS.md §5.7.2 (scenarios 06/08/09/10/11)
    V4 verify-package.mjs imports workspace self-references ⚠️ Partially true as phrased: validateRuntimeImportGraph() walks imports from workspace index.ts instead of importing entrypoints from the packed artifact — but this package has exactly one entrypoint (./dist/index.js; ./server aliases the same file), so the multi-entrypoint motivation doesn't apply here. Revisit if a second entrypoint ever lands
    V5 V2 public-record fixtures incomplete ❌ No V2 fixtures exist

    Where the audited code actually lives: ranxianglei/billion-context — src/agent/opencode-v2.ts (header comment: "probed live on 2.0.1 + 2.0.3"), tests/launcher.test.ts (parseOpencodeMajor("opencode v2.0.3")), plus active V2 audit threads there (#840 dual-mode isolation audit, #859 v2.0.4 TUI review, #850 /acp status under V2, #842 native-route gating). Please re-point or cross-reference items R1–R3, R5, V1–V3, V5 there. Per the anti-loop rule I won't file new issues myself.

    The one applicable item — verified real

    R4: 4-digit CompressionBlock.startId/endId are never migrated.

    • Own-session load: lib/state/state.ts:429-440 migrates 4→5-digit refs only inside messageIds.byRawId/byRef; block boundaries are untouched.
    • Fork transfer (PR fix: reconcile compaction restart and custom-storage fork recovery #409): normalizeParentMessageIds() (lib/state/rebuild.ts:164) normalizes only the parent ref map; block translation at rebuild.ts:275-284 spreads ...block, so legacy m0001-style boundaries survive into fork state while every other ref is 5-digit.

    Impact assessment (layer analysis): low severity, cosmetic/dedup only. Boundary resolution never goes through byRef (resolution uses raw IDs + blockId), so nothing breaks functionally. Consumers: hideConsumedCompressCalls range-key dedup (lib/compress/hide-consumed.ts:84 — mixed digit widths fail to match → consumed compress calls stay visible, minor context duplication), export/recap display, persistence pass-through. Only pre-1.1.0 legacy state is affected.

    Action taken / planned

    Proceeding with the single applicable fix (clear recommendation, no alternatives worth weighing): migrate block startId/endId at both sites — load-time block normalization (lib/state/utils.ts) and fork-transfer translation (lib/state/rebuild.ts) — plus red/green regression tests, devlog, dual-agent review, and one PR referencing this issue. The remaining 9 items need your call on re-pointing to billion-context; per duplicate-screening rules I'm not closing anything here.


    中文摘要:逐条核验后确认本 issue 的 10 项中 9 项描述的是本仓库不存在的代码(OpenCode V2 迁移按 #395 决议已移交 billion-context#735,本仓库维持 V1-only),已附逐项证据;唯一真实命中的一条(旧版 4 位 CompressionBlock.startId/endId 边界未随消息引用一起迁移,位于 state.ts 加载路径与 PR #409 的 fork 转移路径)已验证属实、影响低(仅去重键/展示层),正在修复并出 PR。

  2. ranxianglei commented on Sep 16, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    [bot] 🏷 Follow-up to the triage above: item R4 is fixed in PR #416 — #416

    What changed (branch 2026-09-17_legacy-block-ref-migration, commits 1a68306 + c2651ca):

    • New migrateMessageRef() helper in lib/message-ids.ts (4-digit → 5-digit; non-ref values pass through unchanged)
    • Persisted block startId/endId migrated at own-session load (loadPruneMessagesState) and at fork transfer (restoreForkCompressionState) — legacy state self-heals on next session init and re-saves normalized
    • Review-found follow-on: hideConsumedCompressCalls range keys are now width-normalized on both sides. Without this, migrating only the block side would have regressed batch filtering for legacy pre-1.1.0 sessions — pre-fix both sides were consistently 4-digit, and immutable historical tool inputs stay 4-digit forever. Non-string boundary values interpolate as-is (legacy records may omit them despite the string type) instead of throwing.
    • 6 new tests incl. a RED-confirmed regression test proving migrated 5-digit block keys still match legacy 4-digit tool inputs in batch filtering

    Verification: typecheck + build pass; full suite 1273 tests, 1271 pass, 2 fail — the 2 failures reproduce identically on clean master (pre-existing sandbox environment incompatibility, unrelated to this change).

    Dual-agent review (§5.3 code + §5.6 tests, per project spec): code review requested changes (the rangeKey regression above + a missing typeof guard on raw persisted JSON) — all fixed with red/green proof in c2651ca; test review approved with one minor item accepted as follow-up. Full record in the PR comments.

    The other nine items in this issue still target V2 migration code that lives in ranxianglei/billion-context (see the triage table above) — nothing further to do in this repo for them.

    Per project rules, merging is a human-only operation — please review and merge PR #416 when ready.

  3. drexb-ops commented on Sep 16, 2026

    @drexb-ops
    ContributorAuthor

    Final independent reviews found additional gaps to resolve before closing this issue:

    • V2 speculative quality-gate rejection loses qualityGateRetryPending, breaking the next acknowledgeRisk retry.
    • Patch replay/removal still has shifted-index and incomplete same-ID content-correlation cases, plus quadratic/unbounded fingerprint work.
    • decompress.toFile can follow a symlink outside the validated directory.
    • V2 timing starts can survive unload when no after-event arrives.
    • Normal persisted-state restart does not apply legacy block-boundary normalization.
    • Permission diagnostics need complete sanitized structural hashes rather than raw state/tool payloads.
    • The V2 wrapper fallback must reject additional activation errors, not merely find the known directory diagnostic.
    • Installed nudge proof needs exact baseline/shown-token/block transitions including the historical nothing-to-compress branch.
    • Package verification must use an allowlisted environment and stronger symlink/cleanup ownership checks.

    These findings will be fixed and re-reviewed in the same migration branch.

  4. ranxianglei commented on Sep 16, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    [bot] 🏷 Audited all nine findings against this repo before touching anything. Verdicts with evidence:

    Applies to this repo (1):

    • decompress.toFile symlink escape — confirmed real. lib/compress/decompress.ts:304–334 validates purely lexically (path.resolve + path.relative, no symlink resolution) and then writes with writeFile, which follows links: a symlink inside an allowed root can redirect the write outside it. Fix designed as three fail-closed layers (lexical containment → physical containment by realpath-ing the deepest existing ancestor against physical allowed roots → refuse final-component symlinks, plus O_NOFOLLOW on the POSIX open to close the check-then-write window), extracted into a testable module with injected allowed-dirs. Status: implementation is partially staged locally but NOT committed — see blocker below. I will not ship unverified code.

    V2 / billion-context scope (4):

    1. Speculative quality-gate losing qualityGateRetryPending — V1 has no speculative mode; verified lib/compress/range.ts:269–283, 358–361: the flag is set immediately before the rejection throw, and the error path restores the pre-call value correctly.
    2. Patch replay/removal shifted-index, same-ID correlation, quadratic fingerprint work — V1 has no patch-application system; the closest analog (mapForkIds in lib/state/rebuild.ts:155–189) is a single pass over ref entries (linear in content, fork-path only).
    3. Timing starts surviving unload — V1 has no unload/dispose lifecycle handlers at all (zero matches in lib/).
    4. Wrapper fallback rejecting additional activation errors — their harness.

    Not applicable to this repo (4):
    5. Restart missing legacy boundary normalization — V1 has exactly two loadSessionState call sites, both covered by PR #416 (own-session load normalizes via loadPruneMessagesState, proven by its integration test; fork transfer via the rebuild.ts guard); slash commands never read state files directly.
    6. Permission diagnostics raw payloads — compress-permission.ts and host-permissions.ts contain zero logging calls; there are no diagnostics to sanitize.
    8. Installed nudge proof exact transitions — already met here per §5.7: unit tests assert exact lastPerMessageNudgeTokens/lastNudgeShownTokens transitions across multi-turn growth cycles with production preserveRecentMessages > 0 (the post-#207 requirements), and e2e scenarios 06/08/09/10/13 exercise the nudge flows incl. baseline-set assertions in verify.ts. If you want exact-value e2e assertions added in this repo too, say so explicitly — they're brittle (depend on fake-LLM token accounting) and beyond the spec, so I'd rather agree first than silently add them.
    9. Package verification env allowlist/symlink ownership — scripts/verify-package.mjs imports node builtins only; no env manipulation or symlink/cleanup logic exists here to harden.

    Blocker: mid-work the workspace volume ran out of space — every shell command and file write now fails with ENOSPC: no space left on device. I tried clearing scratch and re-probing; still full. Nothing is lost (the staged fix is small and fully specified above), but I'm stopping rather than committing unverified changes. I'll resume and land the toFile hardening with tests as soon as space is available.

    PR #416 (the R4 boundary-migration fix) remains open and CI-clean, awaiting your merge.

  5. ranxianglei commented on Sep 16, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    [bot] 🏷 Status: audit of the nine new findings is complete, but I'm currently blocked from finishing by a workspace disk failure (ENOSPC: no space left on device — every command, file write, and reply post fails with it; tried clearing scratch, still full).

    Verdicts with evidence:

    Applies to this repo (1): decompress.toFile symlink escape — confirmed real: lib/compress/decompress.ts:304–334 validates lexically only (path.resolve + path.relative, no symlink resolution) then writes via writeFile, which follows links — a symlink inside an allowed root redirects the write outside. Fix designed (three fail-closed layers: lexical containment → physical containment via realpath of deepest existing ancestor vs physical allowed roots → refuse final-component symlinks + O_NOFOLLOW open) and partially staged locally, but not committed — I won't ship unverified code. Will land with tests as soon as space is available.

    V2 / billion-context scope (4): speculative quality-gate flag loss (V1 has no speculative mode; verified flag set immediately before throw and restored on error at lib/compress/range.ts:269–283, 358–361); patch replay/removal shifted-index/same-ID/quadratic-fingerprint items (V1 has no patch system; closest analog mapForkIds in lib/state/rebuild.ts:155–189 is linear, fork-path only); timing-starts-survive-unload (V1 has no unload/dispose lifecycle); wrapper-fallback activation errors (their harness).

    Not applicable here (4): restart boundary normalization (V1 has exactly two loadSessionState call sites, both covered by PR #416; commands never read state files directly); permission diagnostics (zero logging calls in compress-permission.ts / host-permissions.ts); installed nudge proof (already met per §5.7 — exact baseline/shown-token unit assertions incl. production preserveRecentMessages > 0, plus e2e scenarios 06/08/09/10/13; if you want exact-value e2e assertions added here too, confirm first — they're brittle and beyond spec); package verification env/symlink hardening (scripts/verify-package.mjs uses node builtins only, no env/symlink logic to harden).

    Please restore workspace disk space; I'll resume immediately and post the detailed triage record + the toFile fix PR update when unblocked. PR #416 remains open and CI-clean, awaiting your merge.

  6. drexb-ops commented on Sep 16, 2026

    @drexb-ops
    ContributorAuthor

    Fixed on branch 2026-09-15_opencode-v2 in f392fb8 (worklog finalization 7e8706d).

    Closure evidence:

    • npm test: 1,412/1,412
    • focused corrective suites: 151/151
    • typecheck, build, real-tarball package verifier: pass (253 entries)
    • exact installed hosts: V1 1.18.29 and V2 2.0.3; full matrix pass in 102s
    • protected/no-target nudge regression plus pinned baselines 15 → 11370 → 23059; exactly two blocks/emissions
    • exclusive V2 fallback classification, complete redacted permission projections, allowlisted verifier environments, identity-safe cleanup
    • two independent complete source/test reviews report no remaining P0/P1/P2 findings
    • package version remains 1.18.1; no publish or merge performed

    Repository-wide Prettier still reports 423 inherited files outside this work; every changed file passes targeted Prettier.

  7. ranxianglei commented on Sep 17, 2026

    @ranxianglei
    Owner

    继续

  8. ranxianglei commented on Sep 17, 2026

    @ranxianglei
    Owner
  9. ranxianglei commented on Sep 17, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    继续完成 — this repo's one in-scope finding (the decompress.toFile symlink escape from the floor-5 audit) is now fully implemented, dual-reviewed, and CI-green.

    What landed on PR #416 (branch 2026-09-17_legacy-block-ref-migration, new commits 2f54a147 + b0f67a0a on top of the R4 legacy-ref fix):

    1. resolveSafeToFileTarget() (lib/compress/tofile-target.ts) — three fail-closed layers: lexical containment → physical containment via realpath of the deepest existing ancestor (rejoined with the missing tail) → lstat refusal of a final-component symlink; the write itself opens with O_NOFOLLOW/0o600. Verified against 15 adversarial scenarios (traversal, /tmp-evil prefix-sibling, intermediate/final symlinks, null bytes, case variants, empty input).
    2. Round-2b fixes (first review pass): root-containment logic corrected every→some — with the two disjoint default roots, every legitimate target was being rejected; Node 22 compat (FileHandle.write instead of Node ≥ 23-only writeFile).
    3. Round-2c fixes after two independent agents reviewed the final state (§5.3 code + §5.6 tests, both APPROVE WITH MINORS, all actionable findings addressed):
      • confirmation message now reports the resolved path, not the raw input (relative inputs would have pointed the model at the wrong file);
      • runtime write failures (EACCES/ENOSPC) return consistent Error: toFile write failed: … strings instead of escaping as rejected promises;
      • residual intermediate-dir TOCTOU documented as an explicit known limitation in the JSDoc (needs direct FS write access to an allowed root plus a race — outside the prompt-injection threat model; Node has no public openat);
      • new E2E integration suite (tests/decompress-tofile-e2e.test.ts) covering the previously untested write path against the real default roots: a symlink pointing at $HOME is rejected with the escaped file asserted absent, and a non-canonical input is written to and reported as its resolved path (RED against the pre-fix message); plus missing-root fail-closed and ELOOP cycle unit tests.

    Verified: typecheck clean · prettier clean · targeted suites 14/14 · full suite 1287 tests / 1285 pass / 2 fail (the same 2 pre-existing sandbox failures as clean master — /tmp is read-only in daemon sandboxes) · local build OK · GitHub CI all green on b0f67a0a (pr-validation, test ×2, build, e2e, artifact). The test reviewer independently confirmed the regression tests go RED against the pre-fix code (scratch copy of the exact buggy expression, repo untouched).

    The other nine floor-5 items remain out of scope for this V1-only repo — noted your pointer to ranxianglei/billion-context#809 as where that discussion belongs.

    PR #416 is ready for human merge (per project rules I can't merge it myself).

    中文摘要:修复了 decompress.toFile 的符号链接逃逸漏洞(此前仅词法校验、写入会跟随链接),经两轮独立双-agent 审查并补齐全部可执行意见,本地与 GitHub CI(含 Docker E2E)全部通过,可以合并。

  10. ranxianglei commented on Sep 17, 2026

    @ranxianglei
    Owner

    继续

  11. ranxianglei commented on Sep 19, 2026

    @ranxianglei
    Owner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions