Skip to content

refactor: one installer for the remote services - #176

Open
viztor wants to merge 1 commit into
claude/vigilant-mendel-bctz9n-p-r3from
claude/vigilant-mendel-bctz9n-p5
Open

viztor wants to merge 1 commit into
claude/vigilant-mendel-bctz9n-p-r3from
claude/vigilant-mendel-bctz9n-p5

Conversation

@viztor

@viztor viztor commented Nov 14, 2026

Copy link
Copy Markdown
Owner

Phase P5 of docs/improvement-plan.md, stacked on #175.

What changed

Item Change
src/host/remote.ts (new) installRemoteService(ctx, Service, contract, options), the one installer for the usage, purge and store-inspection services. It keeps one service per root while some live activation holds it, the service reads the options of the most recently applied activation that is still live, and the last activation to go disposes it. Every outcome is reported through the activation's own ctx.logger, after the mount has settled. Its promises never reject, because scripts/check.ts's consumer probe does not await apply. Held at 100% coverage.
src/lifecycle.ts The three mount-once copies are replaced by calls to the installer. The ctx.provide arm (mountByProvide), which could never publish anything, is dropped, and the case labelled KNOWN_DEFECT P5 flips.
src/stream-hook.ts L1's warn-once: the first decorated turn that finds globalThis.fetch outside the plugin's live patches warns once. installFetchPatch returns the predicate, so two live activations' nested patches raise no warning.
src/usage.ts, src/usage-contract.ts Comment and wiring updates for the installer.
test/remote-service.test.ts (new), test/lifecycle.test.ts, test/stream-hook.test.ts, test/opencode-store-service.test.ts, test/support/composition.ts, test/support/src.ts Cases for the installer on a real cordis Context (latest live options, last-holder dispose, settled reports, a refused mount said by name, the fiber-less and failed-mount release), L1's guards, and a KNOWN_DEFECT P6 case for three overlapping activations, which P6 flips.
vite.config.ts The coverage floor for src/host/remote.ts, and uncovered branches named by expression instead of stale line numbers.
README.md, README.zh-CN.md One paired bullet: usage settings apply when the plugin is applied again, and disabling the plugin withdraws its services.
AGENTS.md, CONTRIBUTING.md, docs/testing.md, docs/improvement-plan.md The repo map and test docs follow the new module and stand-in; the plan's P5 row, its restack adjustment, its test matrix and pin bookkeeping record what landed.

Defects on the base this fixes, reproduced on a real cordis Context (4.0.6-alpha.2):

  • A healthy boot logged a false the usage service did not register warning, because the check ran before root.plugin(GoUsageService, …) had loaded.
  • After an update to usageBaseURL, read() still went to the first activation's base; turning usageEnabled off left the usage service present.
  • Disabling the plugin left all three services on the root, still answering.
  • A refused purge mount was reported only by cordis's own error line, and a synchronous throw in purge or store went to console.warn instead of the activation's logger.

Behaviour changes

  • One installer for the remote services: installRemoteService in src/host/remote.ts replaces the three mount-once copies (usage, purge, opencodeStore). Each service reads the latest live activation's settings, reports through ctx.logger, and disposes cleanly on reload and disable.
  • L1: the stream hook warns once on a decorated turn when globalThis.fetch is no longer the patched function. It stays silent while any live activation's patch is still in the chain.

On the title. The workflow landed this as refactor:, but the plan's P5 row retitles the phase fix: (review finding rules-1-P5-DOC-1), because the README pair announces user-visible changes and a refactor is hidden from the changelog. The plan leaves the choice to the owner at landing: either squash as fix: the remote services follow the plugin's latest activation and go with its last, or keep refactor: and drop the README pair.

Review

Three rounds, each with a correctness lens and a test lens (and a rules lens in round 1), each followed by a fix and a verify pass.

Round Found Upheld Fixed
1 23 22 (3 medium, 19 low) 22
2 8 8 (all low) 8
3 3 3 (all low) 3

Round 3's three were plan bookkeeping (which pins guard L1's predicate, and that the three-activation KNOWN_DEFECT P6 case is red on the base on its warning assertion and lands in the change commit); they were fixed with no code or test change, and the whole gate was re-run afterwards.

These reviews ran on the general model instead of the dedicated reviewer, because the dedicated reviewer was unavailable: spec:P5, review:P5:correctness#1, review:P5:rules#1, verify:P5#1.1, review:P5:correctness#2, verify:P5#2.1, review:P5:correctness#3, verify:P5#3.1.

Verification

Node Step Result
26.12.1 pnpm run check 0 errors, 12 warnings in 278 files
26.12.1 vp test run (alpha channel) 133 files passed; 2136 tests passed, 1 skipped
26.12.1 shuffle, seeds 1 and 42 each: 133 files passed; 2136 passed, 1 skipped
26.12.1 mock suite 9 files passed; 120 tests passed
26.12.1 vp test run --coverage exit 0, every per-file floor held (src/host/remote.ts at 100); global statements 98.56%, branches 96.03%, functions 98.71%, lines 98.56% against a ratchet of 98 / 95.4 / 97.9 / 98
26.12.1 pnpm run build lib/index.mjs 370.00 kB
26.12.1 scripts/check.ts lib/ current; packed tarball installed with plain npm (19 entries), host and client halves loaded; lib/client.js 136211 bytes, under the 139264 tripwire; check passed
24.22.1 shuffle seed 7; mock suite 133 files, 2136 passed, 1 skipped; 9 files, 120 passed
26.12.1 rc channel (dsh-llm-pi-ai 0.2.2-rc.2, cordis 4.1.0): unit; mock 133 files, 2136 passed, 1 skipped; 9 files, 120 passed; switching back to alpha left no package.json or lockfile residue
26.12.1 binary e2e, alpha npm cell and rc npm cell (fresh build each) each: 15 passed, 2 skipped (17); the skips are the two [HOST_LIMITATION] cases (the Go Gemini thinking budget, and the config-editor save that throws in host code)
26.12.1 re-run at landing: check, unit, mock, build, scripts/check.ts 0 errors and 12 warnings; 133 files, 2136 passed, 1 skipped; 9 files, 120 passed; build complete; check passed
— local paths and credentials over the added lines and the new files 0 hits; a planted control line matched, so the patterns do detect
— freshness scope (D23) for this diff catalog:shim, limits:shim and cli:version all answer report, so no freshness check gates this pull request

The remote base had not moved since this branch was cut, so nothing was merged in.


Generated by Claude Code

installRemoteService (src/host/remote.ts) replaces the three mount-once
copies in src/lifecycle.ts for the usage, purge and store-inspection
services. It keeps one service per root while some live activation
holds it, reads the options of the most recently applied activation
that is still live, and disposes the service with the last activation.
Every outcome is reported through the activation's own ctx.logger once
the mount has settled.

This fixes what the old copies got wrong on a real cordis Context: a
healthy boot logged a false "did not register" warning because the
check ran before the service loaded; a changed usageBaseURL or a
disabled usage service was ignored until a restart; disabling the
plugin left all three services answering; and a refused mount or a
synchronous throw was reported through cordis's own line or
console.warn instead of the activation's logger. The provide arm,
which could never publish anything, is dropped, which flips the case
labelled KNOWN_DEFECT P5.

The phase also adds L1's warn-once: the stream hook warns once on a
decorated turn when globalThis.fetch is no longer the patched function,
and stays silent while any live activation's patch is in the chain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0141fKp5k6hiaJcP5ovhGtSD
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.

2 participants