Skip to content

fix: single-writer plugin guard — never overwrite host-managed copies (#991) - #995

Merged
ranxianglei merged 2 commits into
masterfrom
2026-09-19_single-writer-plugin-guard
Sep 19, 2026
Merged

ranxianglei merged 2 commits into
masterfrom
2026-09-19_single-writer-plugin-guard

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Fixes #991

Problem

The global self-updater located any billion-context install dir via findInstallDir() and copied the new tarball over it in place. That is correct only for the copy npm itself installed. When findInstallDir resolved to a host-managed copy — a dsh profile bundle inside pnpm's .pnpm virtual store, pi's npm entry, opencode's plugin dir — the copy-over breaks the owning package manager's integrity (pnpm store corruption / hardlink damage, #953's breeding ground) and violates the principle: the thing that installed a copy is the thing that updates it.

Change

1. hostManagedInstall() classification (src/update.ts) — detects install dirs under a pnpm virtual store (<dir>/.pnpm/...) or a host agent tree (pi / opencode / dsh / kimi / omp homes, honoring their env relocations). Returns { owner, channel, hint }:

  • pnpm store → dsh plugin --profile <name> add billion-context@latest (or dsh plugin add) — the channel dsh itself drives
  • host homes → pi update / opencode's plugin manager / kimi plugin update

2. Guard enforcement — checkForUpdate skips host-managed dirs (logs the owner's channel instead); installViaTarball refuses them structurally (single-writer refusal), so direct callers cannot corrupt a store either. bili-owned dirs (npm global layout, scratch) stay updatable.

3. dsh bundles refresh through dsh's channel — self-update no longer copies over a pnpm store; it re-runs dsh plugin --profile <name> add billion-context per profile (the same command bili plugin install dsh drives), falling back to plain tarball install only when dsh is absent.

4. bili plugin update [client] — one command that brings every lane's bili presence up to date, each through its own owner: reference lanes (omp/claude/codex/kimi) follow the global install; host-managed copies report their owner's updater with a #991 note; dsh lanes refresh through dsh's plugin channel. bili plugin list shows the same per-lane channel info.

Principle (documented)

Every bili presence has exactly one writer. Mixing user commands is fine (dsh plugin add ≡ bili plugin install dsh — same channel, same records); mixing writers is what the guard forbids. Documented in README (Single-writer: who owns which copy) and AGENTS.md Key Design Decisions #6.

Tests

  • tests/update-host-guard.test.ts — classification (pnpm store, all five host homes, bili-owned stays updatable) + installViaTarball refusal leaves the store copy untouched
  • tests/plugin-update.test.ts — pluginUpdate lanes: reference lanes point at global install, pi lane reports owner by entry form, dsh lane short-circuits offline

Pre-flight

cc @ranxianglei — merged cleanly with the native-channels README section from the ework PRs; single-writer table now sits right after it.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-19_single-writer-plugin-guard (79db64f)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-995

Each push to this PR publishes a new version under the pr-995 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr995.tgz
npm install -g package

Install instructions are refreshed on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Received. This is a [PR] mirror of #995 (fix: single-writer plugin guard), so I'm treating it as review, not rework:

  1. Pulling head 2026-09-19_single-writer-plugin-guard (5ea1f47) and reviewing the diff vs current master for cleanliness and correctness (both compression-mode paths N/A here — this touches src/update.ts, src/plugin*, CLI, docs, tests).
  2. Re-running the full pre-flight locally (typecheck / test / build) on the rebased head.
  3. Checking CI status via the API.

Note: mergeable_state is currently blocked — will report whether that's CI or a conflict once checked. Full findings coming in a follow-up comment.

开始审查 PR #995:拉取分支核对 diff、本地重跑 typecheck/test/build、查 CI 状态,结论稍后回帖。

ranxianglei pushed a commit that referenced this pull request Sep 19, 2026
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Review complete for PR #995 (Fixes #991) — verified on branch 2026-09-19_single-writer-plugin-guard.

Verification (local re-run on head 5ea1f47)

  • npm run typecheck ✅
  • npm test: 1959 pass / 0 fail / 2 skipped (e2e gates) ✅ — matches the PR body
  • npm run build ✅
  • CI on 5ea1f47: all 8 check runs green (ubuntu 22/24, windows 22/24, version-guard, build-artifact, codex zero-token e2e). The branch is up to date with master (no conflict); GitHub's mergeable_state=blocked with everything green means it's waiting on human approval, not a technical gate.

Code review

  • Guard placement is correct: checkForUpdate classifies before any registry traffic (host-managed dirs are skipped with the owner's channel logged), and installViaTarball refuses structurally, so no direct caller can copy over a store copy either.
  • Classification heuristic is sound: realpath first, .pnpm segment → pnpm-owned, then host homes (pi/omp/dsh/kimi/opencode) honoring their env relocations (PI_HOME, PI_CODING_AGENT_DIR, DSH_HOME, KIMI_CODE_HOME, XDG_DATA_HOME). The test asserting a plain <home>/.local/lib/node_modules/billion-context npm-global layout stays updatable is exactly the right false-positive guard.
  • README's dsh claim checked against code: the post-self-update profile refresh is the pre-existing [dsh] unify install lanes: bili plugin install dsh should go through the bundle channel + auto-update rewrite #966 wiring (refreshDshProfileBundles at src/update.ts:600), now correctly driven only by the global copy.
  • Both new test files are offline-safe (mocked fetch, scratch homes, dsh lane short-circuits before any registry call).

Fixed directly on the PR branch (commit dabb2a9)

  • Trailing whitespace ×3 in src/plugin-install.ts (flagged by git diff --check).
  • AGENTS.md numbering collision: the new "Single-writer" item was inserted as 6. immediately above the existing 6. ("Two compression modes") — renumbered the latter to 7.. No cross-references break (the only decision cross-ref in the repo points at fix: address all code review findings (condense, routing, session-id, dead code) #2).
  • Re-verified after the push: typecheck clean, 9/9 tests green in the two new test files.

Minor observation (report-only, nothing changed): on a machine where bili exists only as dsh profile bundles (no npm-global install), no process runs the periodic update loop, so those profiles won't refresh automatically — the user must run bili plugin update dsh (or dsh plugin add billion-context@latest) explicitly. Pre-#995 that same scenario corrupted the pnpm store, so this is strictly better; flagging in case you want it documented.

Bottom line: single-writer principle implemented cleanly, both enforcement points covered, docs accurate. Mergeable once the fresh CI on dabb2a9 goes green.

中文摘要:审查了 #995 单写者插件守卫——本地复跑 typecheck/test/build 全绿、CI 8 项全过,代码与文档核对无误;已在 PR 分支直接修掉两处小问题(plugin-install.ts 3 处行尾空白、AGENTS.md 决策列表重复编号 6→7),新提交 dabb2a9 复验通过,可以合并。

This was referenced Sep 19, 2026
…, add bili plugin update

- src/update.ts: hostManagedInstall() classifies install dirs (pnpm .pnpm
  virtual store, pi/opencode/dsh/kimi/omp host homes); checkForUpdate skips
  them, installViaTarball refuses them structurally (#991)
- dsh bundles: self-update re-runs dsh's plugin channel per profile instead
  of copying over a pnpm store
- bili plugin update [client]: drives each lane through its own owner,
  prints per-lane update path; bili plugin list shows the same channel info
- tests: update-host-guard (classification + refusal), plugin-update lanes
- docs: single-writer principle in README + AGENTS.md Key Design Decisions

Fixes #991
@ranxianglei
ranxianglei force-pushed the 2026-09-19_single-writer-plugin-guard branch from dabb2a9 to 8c706c3 Compare September 19, 2026 12:24
@ranxianglei

Copy link
Copy Markdown
Owner Author

Rebased onto post-v0.1.123 master (clean, only package.json version from 0.1.122 → 0.1.123; tests 1959 pass / 0 fail).

Ship gate (per release plan): merge is held until the local validation hop succeeds — a machine running THIS guard build auto-updates 0.1.123 → 0.1.124 (no-op #1000) through the new updater code. hostManagedInstall() pre-checked on the real npm-global dir → undefined (bili-owned, no skip). Will report the hop log here before merge.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Rebase verified — with one catch: the squash dropped my two review-nit fixes, so I re-applied them. New head: 79db64f.

Rebase check

  • Head 8c706c3 sits directly on current master (cc53ca0, post-v0.1.123) — merge-base == origin/master, no conflict, single linear commit.
  • The 0.1.122 → 0.1.123 version move is master's v0.1.123 release commit, not part of this PR — the PR diff still touches neither package.json nor the lockfile (version rule intact).
  • src/ + tests/ are byte-identical between the old reviewed head (5ea1f47) and the rebased head — the rebase changed nothing but the base. Your local run (1959/0) matches mine.

Catch — fixes lost in the squash
The force-update rebuilt the branch from the pre-review state, so both nits from my earlier round were back: trailing whitespace ×3 in src/plugin-install.ts (flagged by git diff --check) and the AGENTS.md duplicate 6. list number ("Two compression modes" collided with the new "Single-writer" item). Re-applied identically as 79db64f (4 lines, docs+whitespace only, zero behavior change) and pushed.

Verification on final head 79db64f

  • Local: typecheck ✅ · npm test 1959 pass / 0 fail / 2 skipped ✅ · build ✅ · git diff --check clean
  • CI: all 8 check runs green (ubuntu 22/24, windows 22/24, version-guard, build-artifact, both codex e2e). The rebased head 8c706c3 was also fully green.

Ship gate — agreed, and it's the right shape: this PR modifies src/update.ts, which per the release spec must prove itself on a real upgrade hop before shipping. A machine running this guard build auto-updating 0.1.123 → 0.1.124 (no-op #1000) through the new updater code exercises exactly the load-bearing path, and your pre-check (hostManagedInstall() → undefined on the real npm-global dir) rules out the worst failure mode — the guard false-positiving on a bili-owned install and bricking self-update. Stand by for your hop log; once it lands and you're happy, the branch is merge-ready (CI green, base current).

中文摘要:复核了 rebase 后的 #995(基线干净、与 master 无冲突、PR 未动 version),发现 squash 丢掉了上一轮两处小修复(行尾空白、AGENTS.md 重复编号),已按原样补回为新提交 79db64f 并复验——本地 typecheck/test/build 全绿、CI 8 项全过;发布闸门(先跑 0.1.123→0.1.124 无操作升级验证新更新器代码)方案正确,等你的 hop 日志后即可合并。

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.

native模式下自动更新问题需要解决

1 participant