diff --git a/scripts/lib/install-lifecycle.js b/scripts/lib/install-lifecycle.js index ffbcd4901..1ef50b03c 100644 --- a/scripts/lib/install-lifecycle.js +++ b/scripts/lib/install-lifecycle.js @@ -2003,12 +2003,12 @@ function uninstallInstalledStates(options = {}) { const { isScriptBoundary, retainScriptBoundary } = require('./install/opencode-script-boundary'); // Remove owned script bodies first. Keep their CommonJS scope while any // retained or user-created JavaScript still relies on that boundary. - const orderedOperations = record.adapter.target === 'opencode' - ? [...operations.filter(operation => !isScriptBoundary(operation, record.targetRoot)), - ...operations.filter(operation => isScriptBoundary(operation, record.targetRoot))] - : operations; + const orderedOperations = [ + ...operations.filter(operation => !isScriptBoundary(operation, record.targetRoot)), + ...operations.filter(operation => isScriptBoundary(operation, record.targetRoot)), + ]; for (const operation of orderedOperations) { - if (record.adapter.target === 'opencode' && retainScriptBoundary(operation, record.targetRoot)) { + if (retainScriptBoundary(operation, record.targetRoot)) { retainedPaths.push(operation.destinationPath); continue; } diff --git a/scripts/lib/install-targets/claude-home.js b/scripts/lib/install-targets/claude-home.js index 2de35ec58..3edc41249 100644 --- a/scripts/lib/install-targets/claude-home.js +++ b/scripts/lib/install-targets/claude-home.js @@ -65,7 +65,7 @@ module.exports = createInstallTargetAdapter({ homeDir: input.homeDir, }; - return modules.flatMap(module => { + const operations = modules.flatMap(module => { const paths = Array.isArray(module.paths) ? module.paths : []; return paths .filter(p => !isForeignPlatformPath(p, adapter.target) && !adapter.excludesSourcePath(p)) @@ -96,5 +96,14 @@ module.exports = createInstallTargetAdapter({ return [adapter.createScaffoldOperation(module.id, sourceRelativePath, planningInput)]; }); }); + if (!operations.some(operation => operation.moduleId === 'commands-core' + && normalizeRelativePath(operation.sourceRelativePath).startsWith('scripts/'))) return operations; + return [...operations, createRemappedOperation( + adapter, + 'commands-core', + 'manifests/install-assets/commonjs-scripts-package.json', + path.join(adapter.resolveRoot(planningInput), 'scripts', 'package.json'), + { strategy: 'preserve-relative-path' } + )]; }, }); diff --git a/scripts/lib/install-targets/claude-project.js b/scripts/lib/install-targets/claude-project.js index afd413716..79623bcd3 100644 --- a/scripts/lib/install-targets/claude-project.js +++ b/scripts/lib/install-targets/claude-project.js @@ -64,9 +64,9 @@ module.exports = createInstallTargetAdapter({ homeDir: input.homeDir, }; - return modules.flatMap(module => { + const operations = modules.flatMap(module => { const paths = Array.isArray(module.paths) ? module.paths : []; - const operations = paths + const moduleOperations = paths .filter(p => !isForeignPlatformPath(p, 'claude')) .flatMap(sourceRelativePath => { if ( @@ -96,7 +96,7 @@ module.exports = createInstallTargetAdapter({ }); if (module.id !== 'hooks-runtime') { - return operations; + return moduleOperations; } return [ @@ -107,8 +107,19 @@ module.exports = createInstallTargetAdapter({ path.join(adapter.resolveRoot(planningInput), 'scripts', directory, 'package.json'), { strategy: 'preserve-relative-path' } )), - ...operations, + ...moduleOperations, ]; }); + if (!operations.some(operation => operation.moduleId === 'commands-core' + && normalizeRelativePath(operation.sourceRelativePath).startsWith('scripts/'))) return operations; + // Standalone command entries and their helpers need the same scope even + // when hooks are excluded. Reuse the shared boundary asset and owner. + return [...operations, createRemappedOperation( + adapter, + 'commands-core', + 'manifests/install-assets/commonjs-scripts-package.json', + path.join(adapter.resolveRoot(planningInput), 'scripts', 'package.json'), + { strategy: 'preserve-relative-path' } + )]; }, }); diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index fddd3298f..ff49ddf53 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -58,7 +58,7 @@ const { } = require('./stale-operations-reconciliation'); const { buildInstallIndex, rewriteRelativeLinks } = require('./link-rewrite'); const { transformInstallContent } = require('./content-transform'); -const { assertScriptBoundary } = require('./opencode-script-boundary'); +const { assertScriptBoundary, isScriptBoundary } = require('./opencode-script-boundary'); function isMarkdownPath(filePath) { return /\.(md|mdx|markdown)$/i.test(String(filePath || '')); @@ -297,7 +297,7 @@ function readPreviousInstallState(plan) { } function assertOpenCodeScriptBoundary(plan, writtenDestinations = new Set()) { - if (plan.adapter?.target !== 'opencode') return; + if (!plan.operations.some(operation => isScriptBoundary(operation, plan.targetRoot))) return; assertSafeInstallOperation(plan, { destinationPath: plan.installStatePath }); assertScriptBoundary(plan, { readFile: destinationPath => readInstalledFileNoFollow(plan, { destinationPath }), @@ -742,6 +742,8 @@ function previewInstallPlan(plan) { function applyInstallPlan(plan, dependencies = {}) { assertHookConsentReady(plan); + // Reject conflicts before locks can create any target directories/files. + assertOpenCodeScriptBoundary(plan); if (plan.adapter?.target === 'opencode') { assertSafeInstallOperation(plan, { destinationPath: plan.installStatePath }); return withOpenCodeInstallLocks( diff --git a/scripts/lib/install/opencode-script-boundary.js b/scripts/lib/install/opencode-script-boundary.js index 99f15e759..48dcb24dc 100644 --- a/scripts/lib/install/opencode-script-boundary.js +++ b/scripts/lib/install/opencode-script-boundary.js @@ -30,16 +30,59 @@ function scopedJavaScriptFiles(directory, { nested = false, depth = 0 } = {}) { return files; } +function assertCommonJSMetadata(content, file) { + if (content === null) { + if (fs.lstatSync(file, { throwIfNoEntry: false })) { + throw new Error(`Refusing CommonJS runtime migration: invalid user module boundary at ${file}.`); + } + return; + } + let metadata; + try { metadata = JSON.parse(content.toString('utf8')); } + catch { throw new Error(`Refusing CommonJS runtime migration: invalid user module boundary at ${file}.`); } + if (!metadata || typeof metadata !== 'object' || Array.isArray(metadata) + || (metadata.type !== undefined && metadata.type !== 'commonjs')) { + throw new Error(`Refusing CommonJS runtime migration: preserve the existing user module boundary at ${file}.`); + } +} + +function assertSelectedScriptScopes(plan, readFile, scriptsRoot) { + const inspected = new Set(); + for (const operation of plan.operations) { + if (operation.kind !== 'copy-file' || !operation.destinationPath.endsWith('.js')) continue; + const relative = path.relative(scriptsRoot, operation.destinationPath); + if (relative.startsWith(`..${path.sep}`) || relative === '..' || path.isAbsolute(relative)) continue; + // Check every selected entry/helper, including nested library boundaries + // which the unrelated-user-script scope walk intentionally skips. + let directory = path.dirname(operation.destinationPath); + while (key(directory) !== key(scriptsRoot)) { + const file = path.join(directory, 'package.json'); + if (!inspected.has(key(file))) { + assertCommonJSMetadata(readFile(file), file); + inspected.add(key(file)); + } + directory = path.dirname(directory); + } + } +} + function assertScriptBoundary(plan, { readFile, previousOperations = [], writtenDestinations = new Set(), expectedContent }) { - if (plan.adapter?.target !== 'opencode' - || !plan.operations.some(operation => isScriptBoundary(operation, plan.targetRoot))) return; + if (!plan.operations.some(operation => isScriptBoundary(operation, plan.targetRoot))) return; const marker = markerPath(plan.targetRoot); + assertSelectedScriptScopes(plan, readFile, path.dirname(marker)); const content = readFile(marker); + assertCommonJSMetadata(content, marker); if (content !== null) { - const metadata = JSON.parse(content.toString('utf8')); - if (!metadata || typeof metadata !== 'object' || Array.isArray(metadata) - || (metadata.type !== undefined && metadata.type !== 'commonjs')) { - throw new Error(`Refusing OpenCode CommonJS runtime migration: preserve the existing user module boundary at ${marker}.`); + const ownedMarker = previousOperations.find(operation => operation.ownership === 'managed' + && isScriptBoundary(operation, plan.targetRoot)); + // A reinstall bridge may omit digests after preflight validated the old + // receipt. At publication the fixed boundary asset still proves its bytes. + const expectedMarker = ownedMarker && !ownedMarker.contentSha256 + ? expectedContent(plan.operations.find(operation => isScriptBoundary(operation, plan.targetRoot))) : null; + const markerDigest = ownedMarker?.contentSha256 || (expectedMarker + ? crypto.createHash('sha256').update(expectedMarker).digest('hex') : null); + if (ownedMarker && crypto.createHash('sha256').update(content).digest('hex') !== markerDigest) { + throw new Error(`Refusing CommonJS runtime migration: preserve the edited user module boundary at ${marker}.`); } // Existing user-owned CommonJS metadata is skipped by the ownership guard; // retaining its bytes does not change anyone's module interpretation. @@ -58,7 +101,7 @@ function assertScriptBoundary(plan, { readFile, previousOperations = [], written const expectedDigest = expected === null ? operation?.contentSha256 : crypto.createHash('sha256').update(expected).digest('hex'); if (!expectedDigest || digest !== expectedDigest) { - throw new Error(`Refusing OpenCode CommonJS runtime migration: unowned or edited JavaScript at ${file} would change module scope. Give user scripts their own package.json scope before retrying.`); + throw new Error(`Refusing CommonJS runtime migration: unowned or edited JavaScript at ${file} would change module scope. Give user scripts their own package.json scope before retrying.`); } } } diff --git a/tests/scripts/command-runtime.test.js b/tests/scripts/command-runtime.test.js index 765f971ba..0918fd7a3 100644 --- a/tests/scripts/command-runtime.test.js +++ b/tests/scripts/command-runtime.test.js @@ -15,9 +15,10 @@ let failed = 0; function snapshot(root) { if (!fs.existsSync(root)) return {}; return Object.fromEntries(fs.readdirSync(root, { recursive: true, withFileTypes: true }) - .filter(entry => entry.isFile()) .map(entry => { const file = path.join(entry.parentPath || entry.path, entry.name); - return [path.relative(root, file), require('crypto').createHash('sha256').update(fs.readFileSync(file)).digest('hex')]; }).sort()); + const value = entry.isDirectory() ? '[directory]' : entry.isSymbolicLink() + ? `[symlink:${fs.readlinkSync(file)}]` : require('crypto').createHash('sha256').update(fs.readFileSync(file)).digest('hex'); + return [path.relative(root, file), value]; }).sort()); } function test(name, fn, target = 'claude-project') { const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-command-runtime-'))); @@ -50,9 +51,11 @@ function test(name, fn, target = 'claude-project') { ok(result); return result.stdout.trim(); }; const noPayload = () => { - for (const file of ['settings.json', '.mcp.json', 'hooks/hooks.json', 'scripts/hooks']) { + for (const file of ['.mcp.json', 'hooks/hooks.json', 'scripts/hooks']) { assert.ok(!fs.existsSync(path.join(root, file)), `Unexpected ${file}`); } + const settings = path.join(root, 'settings.json'); + if (fs.existsSync(settings)) assert.ok(!JSON.parse(fs.readFileSync(settings, 'utf8')).hooks); assert.ok(!state().operations.some(op => ['hooks-runtime', 'mcp-configs'].includes(op.moduleId))); }; try { fn({ dir, home, project, root, scripts, marker, write, install, uninstall, plan, @@ -154,4 +157,40 @@ test('publication rechecks nested helper package conflict', f => { } }), /module boundary/); assert.ok(!fs.existsSync(f.marker)); }); +test('uninstall retains owned boundary for newly added user JS until it is removed', f => { + f.write(path.join(f.project, 'package.json'), '{"type":"module"}\n'); + f.ok(f.install()); const file = path.join(f.scripts, 'user.js'); f.write(file, 'module.exports = 47;\n'); + const partial = JSON.parse(f.uninstall().stdout); + assert.strictEqual(partial.results[0].status, 'partial'); + assert.strictEqual(partial.summary.errorCount, 0); + assert.ok(partial.results[0].retainedPaths.includes(f.marker)); + assert.ok(fs.existsSync(f.marker)); assert.strictEqual(f.probe(file), '47'); + fs.unlinkSync(file); f.ok(f.uninstall()); assert.ok(!fs.existsSync(f.marker)); assert.ok(!fs.existsSync(f.statePath)); +}); +for (const scope of ['lib/package.json', 'lib/skill-evolution/package.json', 'ci/package.json']) { + test(`existing CommonJS parent still rejects incompatible selected ${scope}`, f => { + f.write(f.marker, '{"type":"commonjs","user":true}\n'); + f.write(path.join(f.scripts, scope), '{"type":"module"}\n'); const before = snapshot(f.dir); + assert.notStrictEqual(f.install().status, 0); assert.deepStrictEqual(snapshot(f.dir), before); + }); +} +test('edited owned CommonJS package bytes are preserved by rejection', f => { + f.ok(f.install()); f.write(f.marker, '{"type":"commonjs","userAdded":true}\n'); + const before = snapshot(f.dir); const result = f.install(); assert.notStrictEqual(result.status, 0); + assert.match(result.stderr + result.stdout, /edited.*module boundary/); assert.deepStrictEqual(snapshot(f.dir), before); +}); +for (const scope of ['package.json', 'lib/package.json']) { + test(`symlinked ${scope} refuses before mutation`, f => { + const file = path.join(f.scripts, scope); const outside = path.join(f.dir, 'user-package.json'); + f.write(outside, '{"type":"commonjs"}\n'); fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.symlinkSync(outside, file); const before = snapshot(f.dir); + assert.notStrictEqual(f.install().status, 0); assert.deepStrictEqual(snapshot(f.dir), before); + }); +} +for (const scope of ['package.json', 'lib/package.json']) { + test(`non-file ${scope} refuses before mutation`, f => { + fs.mkdirSync(path.join(f.scripts, scope), { recursive: true }); const before = snapshot(f.dir); + assert.notStrictEqual(f.install().status, 0); assert.deepStrictEqual(snapshot(f.dir), before); + }); +} console.log(`Passed: ${passed}`); console.log(`Failed: ${failed}`); process.exitCode = failed ? 1 : 0;