fix(moss-vscode): pin fixed crash-boundary SDK + packaged-worker regression gate (MOS-166)#447
Open
abojja9 wants to merge 5 commits into
Open
fix(moss-vscode): pin fixed crash-boundary SDK + packaged-worker regression gate (MOS-166)#447abojja9 wants to merge 5 commits into
abojja9 wants to merge 5 commits into
Conversation
…r regression gate (MOS-166)
apps/moss-vscode locked @moss-dev/moss@1.3.2 -> @moss-dev/moss-core@0.19.2, inside
the pre-containment source-affected range: it ships a native addon that can abort
the extension worker on a corrupted on-disk session cache. The fix shipped in
@moss-dev/moss@1.4.1 / @moss-dev/moss-core@0.20.1.
Remediation + regression gate (one PR):
- Upgrade + exact-pin @moss-dev/moss to 1.4.1; lockfile resolves moss-core 0.20.1
and all five platform packages to the same 0.20.1.
- Worker-survival E2E (test/worker-survival.test.mjs): forks the built worker
exactly as the host does, initializes through a loopback auth stub (MOSS_AUTH_URL,
no real credentials/network), feeds a battery of deterministic, seeded torn/corrupt
caches to loadFromDisk, and asserts a catchable IPC {ok:false} error with the worker
staying alive, installing no partial state, and answering a subsequent safe request.
Runs against the real installed addon per platform via the CI matrix. Hermetic:
modelId "custom" loads no model (the native loadFromDisk deserialize boundary is
model-independent).
- Supervisor negative control (test/supervisor-negative-control.test.mjs): drives the
real MossSessionManager (bundled with a vscode stub, no production change) and aborts
its worker with a signal mid-call, proving it records the exit code/signal, rejects
pending calls as a crash, clears state, and starts a replacement worker -
distinguishing signal/abort from the catchable-throw path.
- VSIX integrity (scripts/verify-package.mjs): asserts bundled wrapper/core/platform
versions are the fixed pinned set, enforces the lockfile floor, rejects leaked test
source/fixtures/hidden test-hook exports, and executes the extracted worker against
the VSIX's own bundled .node. .vscodeignore now excludes test/, promo/, and logs.
- prepackage.mjs enforces the 0.20.1 floor; moss-vscode-ci.yml runs the tests on
macOS/Linux/Windows and gates packaging on the VSIX-integrity check.
No production behavior changed.
|
|
- Supervisor: kill('SIGKILL') is reported as signal=SIGKILL/code=null on Windows
too, so assert that uniformly instead of branching on platform.
- Worker-survival: a live worker memory-maps index.mossvec and Windows refuses to
unlink a mapped file (EPERM); make fixture temp cleanup best-effort (safeRm).
Codex reviewNo issues found. |
Follow-up on the NO-GO review of PR #447 (in-scope items only; the wrapper's Node >=20.4 vs extension >=20 mismatch is left as-is per captain escalation): - verify-package: hook-scan the native .node in ALL five packaged platform packages (not just the runner-native one) and fail if any package is missing its .node; fail if any expected JS scan target is absent instead of silently skipping it. - verify-package: reject source maps as a leak (they embed original TypeScript via sourcesContent), and stop shipping them — dropped the redundant !dist/** negation from .vscodeignore that was re-including dist/*.map past **/*.map. - Apply applyHermeticEnv() (telemetry off + isolated model cache) before both in-process baseline-generation paths (worker-survival before() and the packaged-worker check) so parent-side SDK side effects are bounded. - workerHarness: bound every IPC request with a timeout that SIGKILLs a stuck worker, so a native deadlock cannot hang the run (covers the packaged check too, which uses the harness). - Correct the architecture-coverage comments (CI executes macOS arm64, Linux x64, Windows x64; Darwin x64 and Linux arm64 are bundled and statically version/hook-checked, not executed - no hosted runners). - Remove the unrelated AGENTS.md maintenance-policy block (scope creep); keep the test/gate-documenting AGENTS.md additions.
….1 (MOS-166) Captain decision on review finding #3 (Option A): align the extension's runtime requirement with the packaged @moss-dev/moss 1.4.1 wrapper, which declares engines.node >=20.4. - package.json engines.node: >=20 -> >=20.4; moss.nodePath description now says Node.js 20.4+. - src/moss/client.ts findNodeBinary now verifies each candidate binary's Node version (probed via ELECTRON_RUN_AS_NODE=1 so VS Code's embedded Electron reports its bundled Node version) and selects only one that meets the 20.4 floor, instead of accepting any binary that merely exists. Error/log messaging references 20.4+ and reports the closest too-old match. - Exported pure helpers (MIN_WORKER_NODE_VERSION, parseNodeVersion, nodeMeetsWorkerFloor) with a focused test (test/node-floor.test.mjs) asserting the boundary; the supervisor negative control already exercises findNodeBinary end-to-end selecting a valid Node. Scoped to the Node-floor alignment; no other production change.
…etion) Three completion gaps of the approved Option A Node-floor change: - package-lock.json root engines synced to node >=20.4 to match package.json (npm install --package-lock-only; only the engines line changes). - PUBLISHING.md (shipped in the VSIX) updated 'Node 20+' -> 'Node 20.4+'. - parseNodeVersion is now anchored to a clean stable X.Y.Z (optional leading v), so a prerelease such as 20.4.0-rc.1 (or any trailing tag) no longer parses as meeting the stable >=20.4 floor; node-floor.test.mjs extended to cover prerelease/build-metadata rejection.
Contributor
There was a problem hiding this comment.
Pull request overview
Pins the VS Code extension’s packaged Moss SDK/native addon to the fixed crash-boundary versions and adds hermetic regression gates (tests + VSIX verification + CI) to prevent silently shipping a worker-aborting native addon again.
Changes:
- Exact-pin
@moss-dev/moss@1.4.1(and transitively@moss-dev/moss-core@0.20.1) and enforce a packaging floor. - Add a hermetic Node-based regression suite that forks the real built worker and validates corrupt-cache behavior and supervisor crash recovery.
- Add a VSIX integrity verifier and wire the full gate into the dedicated
moss-vscodeCI workflow.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/moss-vscode/test/worker-survival.test.mjs | E2E test that drives the built worker over IPC and asserts torn-cache errors are catchable and non-fatal. |
| apps/moss-vscode/test/support/workerHarness.mjs | Fork/IPC harness mirroring the extension host’s worker protocol and lifecycle. |
| apps/moss-vscode/test/support/vscodeStub.mjs | Minimal vscode stub to run the real supervisor in Node tests. |
| apps/moss-vscode/test/support/loadSupervisor.mjs | Bundles/imports the real supervisor with vscode aliased to the stub. |
| apps/moss-vscode/test/support/fixtures.mjs | Deterministic, seeded fixtures + corruption catalog for on-disk cache shapes. |
| apps/moss-vscode/test/support/env.mjs | Hermetic env helpers to keep tests offline and prevent model downloads. |
| apps/moss-vscode/test/support/authStub.mjs | Loopback auth stub to satisfy credential validation without network/real creds. |
| apps/moss-vscode/test/supervisor-negative-control.test.mjs | Negative control proving the supervisor handles an actual worker abort/restart path. |
| apps/moss-vscode/test/node-floor.test.mjs | Tests for Node version floor helpers used by worker binary selection. |
| apps/moss-vscode/src/moss/client.ts | Enforces a worker Node floor (>=20.4.0) via version probing during binary selection. |
| apps/moss-vscode/scripts/verify-package.mjs | VSIX integrity gate: version assertions, leak checks, hook scans, and executes extracted worker vs bundled .node. |
| apps/moss-vscode/scripts/prepackage.mjs | Packaging-time guard to refuse bundling moss-core below the fixed crash boundary. |
| apps/moss-vscode/PUBLISHING.md | Updates published guidance to the 20.4+ Node requirement. |
| apps/moss-vscode/package.json | Pins SDK dependency, adds tests, and updates engines.node + user-facing config text. |
| apps/moss-vscode/package-lock.json | Regenerated lockfile reflecting exact pinned SDK/core/platform package versions. |
| apps/moss-vscode/.vscodeignore | Ensures tests/promo/logs/scripts aren’t packaged into the VSIX. |
| AGENTS.md | Documents the moss-vscode app and its hermetic regression suite + version floors. |
| .github/workflows/moss-vscode-ci.yml | Runs the regression suite on macOS/Linux/Windows and verifies/package-gates on Ubuntu. |
Files not reviewed (1)
- apps/moss-vscode/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+64
| after(async () => { | ||
| await worker?.dispose(); | ||
| await stub?.close(); | ||
| }); |
Comment on lines
+66
to
+68
| after(async () => { | ||
| await stub?.close(); | ||
| }); |
Comment on lines
+295
to
+297
| const detail = tooOld | ||
| ? ` The closest match, ${tooOld.candidate}, is Node ${tooOld.version}, below the required ${MIN_WORKER_NODE_VERSION} (@moss-dev/moss 1.4.1).` | ||
| : ""; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MOS-166: pin fixed crash-boundary SDK + packaged-worker regression gate
apps/moss-vscodeshipped@moss-dev/moss@1.3.2→@moss-dev/moss-core@0.19.2, inside the pre-containment source-affected Node range (js-core v0.17.0–v0.20.0). That native addon can abort the extension worker/host on a corrupted on-disk session cache. The fix shipped in@moss-dev/moss@1.4.1/@moss-dev/moss-core@0.20.1. This PR remediates the shipped-vulnerable dependency and adds a regression gate so the boundary cannot silently regress, as one PR (MOSS-APP-1 from the MOS-164 inventory, §4.1 / §5).Base main SHA:
899615e455ad412d5083aab7f1b7292a015f32021. Dependency remediation + exact lock
package.json:@moss-dev/moss^1.3.2→ exact1.4.1(no caret drift for the packaged boundary).package-lock.jsonregenerated. Resolved versions:@moss-dev/moss→ 1.4.1@moss-dev/moss-core→ 0.20.1darwin-arm64,darwin-x64,linux-arm64-gnu,linux-x64-gnu,win32-x64-msvc) — every platform package resolves to the same core version.scripts/prepackage.mjsnow enforces a0.20.1floor and refuses to package a below-fix addon.2. Worker-survival E2E —
test/worker-survival.test.mjsBuilds
dist/mossWorker.jsand forks it exactly as the extension host does (IPC stdio layout,ELECTRON_RUN_AS_NODE, the same{id,method,args}protocol). Initializes through a local loopback auth stub (MOSS_AUTH_URL, no real credentials or off-box network), then feeds a battery of deterministic, seeded, privacy-safe torn/corrupt caches toloadFromDisk(truncated/torn vector sidecar, empty/missing sidecar, bad magic, count mismatch, invalid JSON). For each it asserts: a catchable IPC{ok:false}error (not an abort), the worker stays alive and IPC-connected, no partial state is installed, and it still answers a subsequent safe request. Runs against the real installed addon for the runner's platform.3. Supervisor negative control —
test/supervisor-negative-control.test.mjsDrives the real
MossSessionManager(src/moss/client.ts, bundled with avscodestub via esbuild — no production change) and deliberately aborts its worker with a signal while a call is in flight. Proves the host records the exit code/signal, rejects all pending calls as a crash, clears session state, and starts a replacement worker — explicitly distinguishing the signal/abort path from the catchable-throw path.4. VSIX integrity —
scripts/verify-package.mjsPresence checks alone are insufficient. The verifier extracts the built VSIX and asserts the bundled wrapper/core/platform versions are exactly the fixed pinned set, enforces the lockfile floor, rejects any leaked test source/fixtures/hidden test-hook or debug export (in the JS wrapper, the worker bundle, and the native
.node), and executes the extracted worker against the VSIX's own bundled.nodewith a torn cache..vscodeignorenow excludestest/,promo/, and logs — this dropped the VSIX from 129 files to 61.5. CI gate —
.github/workflows/moss-vscode-ci.ymlnpm test(worker-survival + supervisor) runs on macOS / Linux / Windows, so a boundary regression fails CI on all shipped native targets. Packaging + VSIX-integrity verification gate on Linux; packaging fails if the boundary regresses or the lock drops below the fix.Platforms exercised
.nodeexecution runs on the Linux packaging runner (packaging is Linux-only, matching the existing CI topology). Windows signal semantics are handled (a killed process reports an exit code rather than a signal).Fidelity note
The gate uses
modelId: "custom"to stay fully hermetic (no embedding-model download, loopback auth only). The nativeloadFromDiskdeserialize/validation path — the actual crash boundary — is model-independent, so this drives the identical native code path the shippedmoss-minilmsessions hit.Scope
No additional production behavior was changed. The only edits to shipped code are the dependency pin/lock and packaging scripts;
src/moss/client.tsis exercised as-is (bundled with a test-onlyvscodestub).CHANGELOG.mdwas not touched.