fix: ship js-yaml as a runtime dependency - #1010
Conversation
lib/compose-utils.js requires js-yaml, but js-yaml was declared as a devDependency, so npm-shrinkwrap.json marked it "dev": true and no consumer install ever received it. Worktree Compose teardown then threw MODULE_NOT_FOUND while auto-stopping a finished cluster: Failed to auto-stop cluster <id>: Cannot find module 'js-yaml' Require stack: - .../@the-open-engine/zeroshot/lib/compose-utils.js - .../@the-open-engine/zeroshot/src/orchestrator.js Flat npm trees could mask it by hoisting js-yaml from another package; pnpm's isolated store cannot, so it surfaced there first. Move js-yaml to dependencies, update the release integrity check that asserted the devDependency placement, and load lib/compose-utils inside the teardown try blocks so best-effort cleanup stays best-effort instead of aborting cluster shutdown. Adds a guard test asserting published files only require declared runtime dependencies. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Closes #1009 |
Greptile SummaryThe PR makes
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| package.json | Moves js-yaml from development-only installation to the published runtime dependency set. |
| npm-shrinkwrap.json | Pins js-yaml 4.3.1 and argparse in the production dependency closure. |
| package-lock.json | Synchronizes the root dependency placement and resolved production dependency metadata. |
| src/isolation-manager.js | Guards Compose utility loading, emits a warning on failure, and allows worktree removal to continue. |
| src/orchestrator.js | Makes Compose teardown support best-effort and logs why teardown was skipped. |
| scripts/rust-distribution.js | Updates release integrity validation to require js-yaml as a runtime dependency. |
| tests/unit/published-runtime-dependencies.test.js | Adds a package-surface check for undeclared bare CommonJS runtime imports. |
| tests/unit/rust-release-workflow-dependencies.test.js | Updates release workflow tests for js-yaml's runtime placement. |
| tests/worktree-compose-cleanup.test.js | Verifies unloadable Compose support is diagnosed without blocking worktree cleanup. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Cluster teardown] --> B[Load compose-utils]
B -->|Success| C[Resolve worktree Compose teardown]
C -->|Scoped Compose project| D[Run docker compose down]
C -->|No eligible project| E[Skip Compose teardown]
B -->|Load or resolution error| F[Log skip reason]
D --> G[Remove worktree]
E --> G
F --> G
Reviews (2): Last reviewed commit: "fix: update js-yaml to patched release" | Re-trigger Greptile
tomdps
left a comment
There was a problem hiding this comment.
Confirmed the published-runtime regression and reviewed the fix end to end. js-yaml is now shipped at patched 4.3.1, both lockfiles remain synchronized, teardown failures are diagnosed without blocking worktree removal, and focused plus full CI—including production audit and Ubuntu/macOS install checks—passes on this exact head.
|
🎉 This PR is included in version 6.39.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Problem
Every cluster that finishes in a worktree crashes during auto-stop:
lib/compose-utils.js(built fromsrc/legacy-lib/compose-utils.ts) requiresjs-yamlat runtime, butjs-yamlwas declared as a devDependency.npm-shrinkwrap.jsontherefore marked it"dev": true, so no consumer install ever received it.Flat npm trees could mask this by hoisting
js-yamlfrom an unrelated package into top-levelnode_modules. pnpm's isolated store has no such accident, so the failure surfaced there first — but the dependency was never actually declared for any install.A second defect turned a cosmetic gap into a fatal one: both call sites
require('../lib/compose-utils')outside theirtryblocks, so a module-resolution failure escaped the "best-effort, silently ignores failures" contract and aborted cluster shutdown.Changes
js-yamlfromdevDependenciestodependencies; regeneratepackage-lock.json/npm-shrinkwrap.json(clearsdev: trueonjs-yamland itsargparsedependency).checkScriptDependencies()inscripts/rust-distribution.js, which asserted the devDependency placement and would have failed the release integrity gate.lib/compose-utilsinside the teardowntryblocks insrc/orchestrator.jsandsrc/isolation-manager.jsso compose cleanup stays best-effort. Orchestrator logs the skip reason; worktree removal still proceeds.Tests
tests/unit/published-runtime-dependencies.test.js(new): scans every published CommonJS file (respecting thefilesallow/deny list) and asserts each barerequire()resolves to a declared runtime or optional dependency. This is the guard that would have caught the bug at commit time.tests/worktree-compose-cleanup.test.js: two regression tests covering an unloadablelib/compose-utils— teardown is skipped without throwing, and worktree removal still runs.tests/unit/rust-release-workflow-dependencies.test.js: updated for the new dependency placement.Verification
3 pre-existing failures in
tests/unit/opcore-introduced-gate.test.jsreproduce identically on unmodifiedmainin this environment (localrust.function-metricsreportsunsupported_request) and are unrelated to this change.🤖 Generated with Claude Code