From 542028d7f1cd76598e994f98479cc587f4259464 Mon Sep 17 00:00:00 2001 From: Agnik47 <140933190+Agnik47@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:37:27 +0530 Subject: [PATCH] fix(cli): fork the whole import closure in adapter override (#418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `adapter override /` copied one file. 510 of the 871 adapter files in this repo import a sibling helper, so for most commands the fork threw `Cannot find module .../clis//shared.js` on load, command resolution fell back to the plugin copy, and `adapter status` still reported the override as tracked — the user edited a file that never ran. Resolve the command's transitive relative-import graph and copy it with the command, preserving nested paths, with a fork-time `.base/` copy of each file. The closure is resolved before anything is written, so an adapter whose imports cannot be copied from inside the plugin directory fails with nothing left on disk. Copied files are recorded on the override, which keeps the rest of the override surface honest about them: - `adapter status` reports them as `dependency`, naming the override that imports each one, instead of `user adapter` — a label that invites deleting a file the fork needs. - `doctor` no longer reports them as untracked shadows, which would have turned every fork of a command with a sibling import into a warning telling the user to reset the fork they just made. - `plugin update` reports upstream drift in them, so a fork whose real logic lives in `./shared.js` cannot go stale unreported. - `adapter reset` removes their base copies along with the override's. A plugin command can import another command's file (linkedin's `salesnav-thread` imports `./salesnav-inbox.js`), so forking one command can place a second command's file in `clis/`. Overriding that second command now adopts the existing copy rather than refusing with "an override already exists", and never overwrites a copy the user has edited. --- docs/publish-community-plugin.mdx | 2 +- src/adapter-import-closure.test.ts | 109 +++++++++++++++++++++ src/adapter-import-closure.ts | 101 +++++++++++++++++++ src/adapter-override.test.ts | 149 ++++++++++++++++++++++++++++- src/adapter-override.ts | 113 ++++++++++++++++++++-- src/adapter-shadow.test.ts | 61 ++++++++++++ src/adapter-shadow.ts | 27 +++++- src/cli.test.ts | 58 ++++++++++- src/cli.ts | 70 +++++++++++--- src/override-provenance.ts | 45 ++++++++- src/plugin.test.ts | 40 ++++++++ src/plugin.ts | 26 ++++- 12 files changed, 773 insertions(+), 28 deletions(-) create mode 100644 src/adapter-import-closure.test.ts create mode 100644 src/adapter-import-closure.ts diff --git a/docs/publish-community-plugin.mdx b/docs/publish-community-plugin.mdx index d31bc7a8..5b8d5a58 100644 --- a/docs/publish-community-plugin.mdx +++ b/docs/publish-community-plugin.mdx @@ -26,7 +26,7 @@ The root `webcmd-plugin.json` and the README community table are generated by re ## Authoring Through Webcmd -To fork an installed command into a private copy you can edit, run `webcmd adapter override /` in either mode. Locally that copies the file into `~/.webcmd/clis` and records provenance, and `webcmd adapter path /` prints the file to edit. In hosted mode it forks the command into a tenant-private package, and `webcmd adapter source get|put /` reads and writes that source. Use `webcmd browser init /` to scaffold, `webcmd browser verify /` to validate, and `webcmd site memory` commands to retain sanitized discovery evidence. +To fork an installed command into a private copy you can edit, run `webcmd adapter override /` in either mode. Locally that copies the command — and any sibling files it imports — into `~/.webcmd/clis` and records provenance, and `webcmd adapter path /` prints the file to edit. In hosted mode it forks the command into a tenant-private package, and `webcmd adapter source get|put /` reads and writes that source. Use `webcmd browser init /` to scaffold, `webcmd browser verify /` to validate, and `webcmd site memory` commands to retain sanitized discovery evidence. ## Validation and Pull Request diff --git a/src/adapter-import-closure.test.ts b/src/adapter-import-closure.test.ts new file mode 100644 index 00000000..4abdc1ed --- /dev/null +++ b/src/adapter-import-closure.test.ts @@ -0,0 +1,109 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { collectRelativeImportClosure } from './adapter-import-closure.js'; + +let root: string; + +function write(relPath: string, source: string): string { + const filePath = path.join(root, relPath); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, source); + return filePath; +} + +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'aic-')); +}); + +afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('collectRelativeImportClosure', () => { + it('returns nothing for an adapter with no relative imports', () => { + const entry = write('search.js', "import { cli } from '@agentrhq/webcmd/registry';\n"); + expect(collectRelativeImportClosure(entry, root)).toEqual([]); + }); + + it('collects a sibling import', () => { + const entry = write('search.js', "import { parseLimit } from './shared.js';\n"); + write('shared.js', 'export const parseLimit = () => 1;\n'); + expect(collectRelativeImportClosure(entry, root)).toEqual(['shared.js']); + }); + + it('follows the graph transitively', () => { + const entry = write('timeline.js', "import { a } from './posts-core.js';\n"); + write('posts-core.js', "import { b } from './shared.js';\n"); + write('shared.js', "import { c } from './_utils.js';\n"); + write('_utils.js', 'export const c = 1;\n'); + expect(collectRelativeImportClosure(entry, root)).toEqual([ + '_utils.js', 'posts-core.js', 'shared.js', + ]); + }); + + it('preserves nested directories in the returned paths', () => { + const entry = write('publish.js', "import { p } from './_shared/private-publish.js';\n"); + write('_shared/private-publish.js', "import { r } from './runtime-info.js';\n"); + write('_shared/runtime-info.js', 'export const r = 1;\n'); + expect(collectRelativeImportClosure(entry, root)).toEqual([ + '_shared/private-publish.js', '_shared/runtime-info.js', + ]); + }); + + it('terminates on an import cycle', () => { + const entry = write('a.js', "import { b } from './b.js';\n"); + write('b.js', "import { a } from './a.js';\nexport const b = 1;\n"); + expect(collectRelativeImportClosure(entry, root)).toEqual(['b.js']); + }); + + it('recognises re-exports, side-effect imports, dynamic imports, require, and double quotes', () => { + const entry = write('mixed.js', [ + "export { helper } from './reexport.js';", + "export * from './star.js';", + "import './side-effect.js';", + "const late = await import('./dynamic.js');", + "const legacy = require('./legacy.js');", + 'import { quoted } from "./double-quoted.js";', + ].join('\n')); + for (const name of ['reexport', 'star', 'side-effect', 'dynamic', 'legacy', 'double-quoted']) { + write(`${name}.js`, 'export const x = 1;\n'); + } + expect(collectRelativeImportClosure(entry, root)).toEqual([ + 'double-quoted.js', 'dynamic.js', 'legacy.js', 'reexport.js', 'side-effect.js', 'star.js', + ]); + }); + + it('ignores bare package specifiers', () => { + const entry = write('search.js', [ + "import { cli } from '@agentrhq/webcmd/registry';", + "import * as path from 'node:path';", + "import { local } from './shared.js';", + ].join('\n')); + write('shared.js', 'export const local = 1;\n'); + expect(collectRelativeImportClosure(entry, root)).toEqual(['shared.js']); + }); + + it('skips a relative import whose target does not exist', () => { + // The plugin itself is already broken here; the override should fail the + // same way the plugin does rather than refuse to be created. + const entry = write('search.js', "import { gone } from './missing.js';\n"); + expect(collectRelativeImportClosure(entry, root)).toEqual([]); + }); + + it('refuses a specifier that escapes the plugin directory', () => { + const entry = write('plugin/search.js', "import { outside } from '../outside.js';\n"); + write('outside.js', 'export const outside = 1;\n'); + expect(() => collectRelativeImportClosure(entry, path.join(root, 'plugin'))) + .toThrow(/resolves outside the plugin directory/i); + }); + + it('refuses an escaping specifier reached transitively', () => { + const entry = write('plugin/search.js', "import { s } from './shared.js';\n"); + write('plugin/shared.js', "import { outside } from '../../escape.js';\n"); + write('escape.js', 'export const outside = 1;\n'); + expect(() => collectRelativeImportClosure(entry, path.join(root, 'plugin'))) + .toThrow(/shared\.js imports "\.\.\/\.\.\/escape\.js"/); + }); +}); diff --git a/src/adapter-import-closure.ts b/src/adapter-import-closure.ts new file mode 100644 index 00000000..0ccd901b --- /dev/null +++ b/src/adapter-import-closure.ts @@ -0,0 +1,101 @@ +/** + * Resolve the relative-import closure of an adapter file. + * + * `webcmd adapter override` forks a single command file out of an installed + * plugin. Most plugin commands are not single files: 510 of the 871 adapter + * files shipped in this repo import a sibling helper (`./shared.js`, + * `./_shared/protocol-capture.js`, ...). Copying the command alone produces + * an override that throws `Cannot find module` on load, so command resolution + * silently falls back to the plugin copy while `adapter status` still reports + * the override as tracked — the user edits a file that never runs. + * + * This module walks the transitive relative-import graph so the fork can copy + * everything the command actually needs. + */ + +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +/** + * Relative specifiers in `import`/`export`/`require` position. + * + * Covers every form the adapter corpus and hand-written private adapters use: + * import x from './a.js' export { x } from './a.js' + * import './a.js' export * from './a.js' + * await import('./a.js') require('./a.js') + * + * Comments are deliberately not stripped first. A commented-out import can + * only ever make the closure copy one file too many — harmless, since the + * copy is tracked and removed by `adapter reset`. Comment-stripping, by + * contrast, can drop a *real* import when it trips over a string, which + * silently reintroduces the broken override this module exists to prevent. + */ +const RELATIVE_SPECIFIER = /(?:\bfrom\s*|\bimport\s*|\brequire\s*)\(?\s*(['"])(\.[^'"]*)\1/g; + +/** Every relative specifier appearing in `source`, in order of appearance. */ +function readRelativeSpecifiers(source: string): string[] { + const specifiers: string[] = []; + for (const match of source.matchAll(RELATIVE_SPECIFIER)) { + const specifier = match[2]; + if (specifier) specifiers.push(specifier); + } + return specifiers; +} + +/** Path relative to `rootDir`, normalised to forward slashes for storage. */ +function toPosixRelative(rootDir: string, target: string): string { + return path.relative(rootDir, target).split(path.sep).join('/'); +} + +function escapesRoot(rootDir: string, target: string): boolean { + const relative = path.relative(rootDir, target); + return relative.startsWith('..') || path.isAbsolute(relative); +} + +/** + * Every file `entryFile` imports transitively through relative specifiers, + * as paths relative to `rootDir` and sorted for a stable provenance record. + * `entryFile` itself is not included. + * + * A specifier that resolves outside `rootDir` throws: the fork cannot be made + * loadable by copying inside the plugin directory, and a half-copied override + * is exactly the silent breakage this closure exists to prevent. A specifier + * pointing at a file that does not exist is skipped — the plugin is already + * broken in that case, and the override should fail the same way the plugin + * does rather than blame the fork. + */ +export function collectRelativeImportClosure(entryFile: string, rootDir: string): string[] { + const resolvedRoot = path.resolve(rootDir); + const resolvedEntry = path.resolve(entryFile); + const closure = new Set(); + const visited = new Set([resolvedEntry]); + const queue: string[] = [resolvedEntry]; + + while (queue.length > 0) { + const current = queue.shift()!; + let source: string; + try { + source = fs.readFileSync(current, 'utf-8'); + } catch { + continue; + } + + for (const specifier of readRelativeSpecifiers(source)) { + const target = path.resolve(path.dirname(current), specifier); + if (escapesRoot(resolvedRoot, target)) { + throw new Error( + `${toPosixRelative(resolvedRoot, current) || path.basename(current)} imports "${specifier}", ` + + `which resolves outside the plugin directory ${resolvedRoot}. ` + + 'An override can only copy files from inside the plugin, so this adapter cannot be forked as-is.', + ); + } + if (visited.has(target)) continue; + visited.add(target); + if (!fs.existsSync(target)) continue; + closure.add(toPosixRelative(resolvedRoot, target)); + queue.push(target); + } + } + + return [...closure].sort(); +} diff --git a/src/adapter-override.test.ts b/src/adapter-override.test.ts index 757b967b..d179935b 100644 --- a/src/adapter-override.test.ts +++ b/src/adapter-override.test.ts @@ -3,7 +3,13 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { createAdapterOverride } from './adapter-override.js'; -import { getBaseCopyPath, readOverrideRecords, fileSha256 } from './override-provenance.js'; +import { + getBaseCopyPath, + getBaseDependencyPath, + readOverrideRecords, + fileSha256, + removeOverrideRecords, +} from './override-provenance.js'; import type { LockEntry } from './plugin.js'; import { createProgram } from './cli.js'; @@ -94,3 +100,144 @@ describe('createAdapterOverride', () => { } }); }); + +describe('createAdapterOverride import closure', () => { + const pluginDir = () => path.join(home, '.webcmd', 'plugins', 'linkedin'); + const clisDir = () => path.join(home, '.webcmd', 'clis', 'linkedin'); + + function writePluginFile(relPath: string, source: string): string { + const filePath = path.join(pluginDir(), relPath); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, source); + return filePath; + } + + it('copies a sibling import so the override can actually load', () => { + // The bug: only search.js was copied, so the override threw + // "Cannot find module .../clis/linkedin/shared.js" on load and command + // resolution silently fell back to the plugin copy. + fs.writeFileSync(pluginFile, "import { parseLimit } from './shared.js';\n"); + writePluginFile('shared.js', 'export const parseLimit = () => 1;\n'); + + const result = createAdapterOverride('linkedin/search', { homeDir: home }); + + const copied = path.join(clisDir(), 'shared.js'); + expect(fs.existsSync(copied)).toBe(true); + expect(fs.readFileSync(copied, 'utf-8')).toBe('export const parseLimit = () => 1;\n'); + expect(result.dependencies).toEqual(['shared.js']); + }); + + it('copies the transitive closure, preserving nested directories', () => { + fs.writeFileSync(pluginFile, "import { a } from './posts-core.js';\n"); + writePluginFile('posts-core.js', "import { b } from './_shared/util.js';\n"); + writePluginFile('_shared/util.js', 'export const b = 1;\n'); + + const result = createAdapterOverride('linkedin/search', { homeDir: home }); + + expect(result.dependencies).toEqual(['_shared/util.js', 'posts-core.js']); + expect(fs.existsSync(path.join(clisDir(), 'posts-core.js'))).toBe(true); + expect(fs.existsSync(path.join(clisDir(), '_shared', 'util.js'))).toBe(true); + }); + + it('keeps a fork-time base copy of every copied file', () => { + fs.writeFileSync(pluginFile, "import { s } from './shared.js';\n"); + writePluginFile('shared.js', 'export const s = 1;\n'); + + createAdapterOverride('linkedin/search', { homeDir: home }); + + const baseCopy = getBaseDependencyPath('linkedin', 'shared.js', home); + expect(fs.readFileSync(baseCopy, 'utf-8')).toBe('export const s = 1;\n'); + }); + + it('records each copied file with the sha256 of the copy the override loads', () => { + fs.writeFileSync(pluginFile, "import { s } from './shared.js';\n"); + const sharedPlugin = writePluginFile('shared.js', 'export const s = 1;\n'); + + createAdapterOverride('linkedin/search', { homeDir: home }); + + const record = readOverrideRecords(home)['linkedin/search']!; + expect(record.dependencies).toEqual([ + { path: 'shared.js', sha256: fileSha256(sharedPlugin) }, + ]); + }); + + it('omits the dependencies key entirely for a single-file adapter', () => { + createAdapterOverride('linkedin/search', { homeDir: home }); + expect(readOverrideRecords(home)['linkedin/search']!.dependencies).toBeUndefined(); + }); + + it('removes the base copies of copied files when the override is dropped', () => { + fs.writeFileSync(pluginFile, "import { s } from './shared.js';\n"); + writePluginFile('shared.js', 'export const s = 1;\n'); + createAdapterOverride('linkedin/search', { homeDir: home }); + const baseCopy = getBaseDependencyPath('linkedin', 'shared.js', home); + expect(fs.existsSync(baseCopy)).toBe(true); + + removeOverrideRecords('linkedin', home); + + expect(fs.existsSync(baseCopy)).toBe(false); + }); + + it('never overwrites a copied file the user has already edited', () => { + fs.writeFileSync(pluginFile, "import { s } from './shared.js';\n"); + writePluginFile('shared.js', 'export const s = 1;\n'); + writePluginFile('other.js', "import { s } from './shared.js';\n"); + + createAdapterOverride('linkedin/search', { homeDir: home }); + const copied = path.join(clisDir(), 'shared.js'); + fs.writeFileSync(copied, 'export const s = 42; // my fix\n'); + + createAdapterOverride('linkedin/other', { homeDir: home }); + + expect(fs.readFileSync(copied, 'utf-8')).toBe('export const s = 42; // my fix\n'); + }); + + it('adopts a file an earlier fork copied instead of refusing as "already exists"', () => { + // linkedin/salesnav-thread imports ./salesnav-inbox.js, which is itself a + // command: forking the first puts the second command's file in clis/, and + // overriding it must not be blocked by webcmd's own copy. + fs.writeFileSync(pluginFile, "import { list } from './inbox.js';\n"); + writePluginFile('inbox.js', 'export const list = () => [];\n'); + createAdapterOverride('linkedin/search', { homeDir: home }); + + const result = createAdapterOverride('linkedin/inbox', { homeDir: home }); + + expect(result.overridePath).toBe(path.join(clisDir(), 'inbox.js')); + const record = readOverrideRecords(home)['linkedin/inbox']!; + expect(record.plugin).toBe('linkedin'); + expect(fs.existsSync(getBaseCopyPath('linkedin/inbox', home))).toBe(true); + }); + + it('adopting an existing copy does not discard edits already made to it', () => { + fs.writeFileSync(pluginFile, "import { list } from './inbox.js';\n"); + writePluginFile('inbox.js', 'export const list = () => [];\n'); + createAdapterOverride('linkedin/search', { homeDir: home }); + const copied = path.join(clisDir(), 'inbox.js'); + fs.writeFileSync(copied, 'export const list = () => [1]; // mine\n'); + + createAdapterOverride('linkedin/inbox', { homeDir: home }); + + expect(fs.readFileSync(copied, 'utf-8')).toBe('export const list = () => [1]; // mine\n'); + }); + + it('still refuses a second override of the same command', () => { + fs.writeFileSync(pluginFile, "import { s } from './shared.js';\n"); + writePluginFile('shared.js', 'export const s = 1;\n'); + createAdapterOverride('linkedin/search', { homeDir: home }); + + expect(() => createAdapterOverride('linkedin/search', { homeDir: home })) + .toThrow(/already/i); + }); + + it('writes nothing when an import cannot be copied from inside the plugin', () => { + fs.writeFileSync(pluginFile, "import { x } from '../outside.js';\n"); + fs.writeFileSync(path.join(home, '.webcmd', 'plugins', 'outside.js'), 'export const x = 1;\n'); + + expect(() => createAdapterOverride('linkedin/search', { homeDir: home })) + .toThrow(/resolves outside the plugin directory/i); + + expect(fs.existsSync(path.join(clisDir(), 'search.js'))).toBe(false); + expect(fs.existsSync(getBaseCopyPath('linkedin/search', home))).toBe(false); + expect(readOverrideRecords(home)['linkedin/search']).toBeUndefined(); + }); +}); diff --git a/src/adapter-override.ts b/src/adapter-override.ts index d14990b4..3dd231b6 100644 --- a/src/adapter-override.ts +++ b/src/adapter-override.ts @@ -4,6 +4,12 @@ * it, while keeping a `.base/` copy and a provenance record so a later * `plugin update` can tell the user upstream changed and offer a real * three-way merge base. + * + * The fork is the command's whole relative-import closure, not just the one + * file. Most plugin commands import a sibling helper, and copying the command + * alone yields an override that throws `Cannot find module` on load: command + * resolution then falls back to the plugin copy while `adapter status` still + * reports the override as tracked, so the user edits a file that never runs. */ import * as fs from 'node:fs'; @@ -12,11 +18,15 @@ import * as path from 'node:path'; import { CLI_COMMAND } from './brand.js'; import { classifyCommandOrigin } from './command-origin.js'; import { getRegistry } from './registry.js'; +import { collectRelativeImportClosure } from './adapter-import-closure.js'; import { fileSha256, getBaseCopyPath, + getBaseDependencyPath, readOverrideRecords, writeOverrideRecords, + type OverrideDependency, + type OverrideRecord, } from './override-provenance.js'; export interface AdapterOverrideResult { @@ -24,6 +34,72 @@ export interface AdapterOverrideResult { plugin: string; overridePath: string; basePath: string; + /** Files copied alongside the command so the override can load, relative to the override directory. */ + dependencies: string[]; +} + +/** + * True when `fileName` already sits in the override directory only because an + * earlier fork of a *different* command in the same plugin copied it as part + * of its import closure. + * + * This is not hypothetical: a plugin command can import another command's + * file (linkedin's `salesnav-thread` imports `./salesnav-inbox.js`), so + * forking one command can put a second command's file in `clis/`. Overriding + * that second command must then adopt the existing copy rather than refuse + * with "an override already exists" — a file webcmd itself placed there is + * not a reason to send the user to `adapter reset`. + */ +function isCopiedDependency( + records: Record, + site: string, + fileName: string, +): boolean { + return Object.values(records).some( + (record) => record.plugin === site && (record.dependencies ?? []).some((dep) => dep.path === fileName), + ); +} + +/** + * Copy every file the command imports into the override directory, and keep a + * fork-time base copy of each. + * + * An existing copy is never overwritten: it is either a file the user has + * already edited, or one an earlier fork of a sibling command placed there. + * The recorded sha256 is therefore taken from the copy the override will + * actually load — if that copy is older than upstream, reconciliation should + * say so instead of claiming the fork is current. + */ +function copyImportClosure( + closure: string[], + pluginFile: string, + overridePath: string, + site: string, + homeDir: string | undefined, +): OverrideDependency[] { + const pluginDir = path.dirname(pluginFile); + const overrideDir = path.dirname(overridePath); + const dependencies: OverrideDependency[] = []; + + for (const relPath of closure) { + const segments = relPath.split('/'); + const source = path.join(pluginDir, ...segments); + const destination = path.join(overrideDir, ...segments); + const baseDestination = getBaseDependencyPath(site, relPath, homeDir); + + if (!fs.existsSync(destination)) { + fs.mkdirSync(path.dirname(destination), { recursive: true }); + fs.copyFileSync(source, destination); + } + if (!fs.existsSync(baseDestination)) { + fs.mkdirSync(path.dirname(baseDestination), { recursive: true }); + fs.copyFileSync(destination, baseDestination); + } + + dependencies.push({ path: relPath, sha256: fileSha256(destination) }); + } + + return dependencies; } function resolveHomeDir(homeDir?: string): string { @@ -89,32 +165,53 @@ export function createAdapterOverride( } const overridePath = path.join(homeDir, '.webcmd', 'clis', site, `${command}.js`); - if (fs.existsSync(overridePath)) { + const records = readOverrideRecords(options.homeDir); + const adoptExistingCopy = isCopiedDependency(records, site, `${command}.js`); + if (fs.existsSync(overridePath) && !adoptExistingCopy) { throw new Error( `An override already exists at ${overridePath}. Run "${CLI_COMMAND} adapter reset ${site}" first if you want to start over.`, ); } + // Resolved before anything is written: an adapter whose imports cannot be + // copied must fail with nothing on disk, rather than leave behind the + // half-copied override that is the whole failure mode being fixed here. + const closure = collectRelativeImportClosure(pluginFile, path.dirname(pluginFile)); + const basePath = getBaseCopyPath(commandKey, options.homeDir); - const content = fs.readFileSync(pluginFile); fs.mkdirSync(path.dirname(overridePath), { recursive: true }); - fs.writeFileSync(overridePath, content); - fs.mkdirSync(path.dirname(basePath), { recursive: true }); - fs.writeFileSync(basePath, content); + if (!fs.existsSync(overridePath)) { + fs.copyFileSync(pluginFile, overridePath); + } + if (!fs.existsSync(basePath)) { + fs.mkdirSync(path.dirname(basePath), { recursive: true }); + fs.copyFileSync(overridePath, basePath); + } + const dependencies = copyImportClosure(closure, pluginFile, overridePath, site, options.homeDir); const commitHash = readCommitHashFor(homeDir, site); - const records = readOverrideRecords(options.homeDir); records[commandKey] = { plugin: site, commitHash, sourcePath: pluginFile, - sourceSha256: fileSha256(pluginFile), + // Hashed from the copy the override will load, not from the plugin file. + // They are the same bytes for a fresh fork; when an earlier fork already + // left this file in clis/, the copy is what the user actually runs, and + // reconciliation should compare upstream against that. + sourceSha256: fileSha256(overridePath), basePath, createdAt: new Date().toISOString(), + ...(dependencies.length > 0 ? { dependencies } : {}), }; writeOverrideRecords(records, options.homeDir); - return { commandKey, plugin: site, overridePath, basePath }; + return { + commandKey, + plugin: site, + overridePath, + basePath, + dependencies: dependencies.map((dependency) => dependency.path), + }; } diff --git a/src/adapter-shadow.test.ts b/src/adapter-shadow.test.ts index 5f9fb6fe..390bc0d7 100644 --- a/src/adapter-shadow.test.ts +++ b/src/adapter-shadow.test.ts @@ -90,6 +90,67 @@ describe('adapter shadow detection', () => { }); }); + it('does not report a file the override itself copied to stay loadable', () => { + // Forking a command copies its import closure into clis/. Reporting those + // copies as unexplained local adapters would make doctor tell the user to + // reset the override they just created. + withTempDirs(({ userClisDir, pluginsDir, homeDir }) => { + fs.mkdirSync(path.join(userClisDir, 'linkedin'), { recursive: true }); + fs.mkdirSync(path.join(pluginsDir, 'linkedin'), { recursive: true }); + for (const dir of [path.join(userClisDir, 'linkedin'), path.join(pluginsDir, 'linkedin')]) { + fs.writeFileSync(path.join(dir, 'search.js'), '', 'utf-8'); + fs.writeFileSync(path.join(dir, 'shared.js'), '', 'utf-8'); + } + fs.mkdirSync(path.join(homeDir, '.webcmd'), { recursive: true }); + fs.writeFileSync( + path.join(homeDir, '.webcmd', 'override-provenance.json'), + JSON.stringify({ + 'linkedin/search': { + plugin: 'linkedin', + commitHash: null, + sourcePath: path.join(pluginsDir, 'linkedin', 'search.js'), + sourceSha256: 'abc', + basePath: path.join(userClisDir, '.base', 'linkedin', 'search.js'), + createdAt: new Date().toISOString(), + dependencies: [{ path: 'shared.js', sha256: 'def' }], + }, + }), + 'utf-8', + ); + + expect(findShadowedUserAdapters({ userClisDir, pluginsDir, homeDir }).map((s) => s.name)) + .toEqual(['linkedin/search']); + }); + }); + + it('still reports a local file that merely shares a name with another override dependency', () => { + withTempDirs(({ userClisDir, pluginsDir, homeDir }) => { + fs.mkdirSync(path.join(userClisDir, 'twitter'), { recursive: true }); + fs.mkdirSync(path.join(pluginsDir, 'twitter'), { recursive: true }); + fs.writeFileSync(path.join(userClisDir, 'twitter', 'shared.js'), '', 'utf-8'); + fs.writeFileSync(path.join(pluginsDir, 'twitter', 'shared.js'), '', 'utf-8'); + fs.mkdirSync(path.join(homeDir, '.webcmd'), { recursive: true }); + fs.writeFileSync( + path.join(homeDir, '.webcmd', 'override-provenance.json'), + JSON.stringify({ + 'linkedin/search': { + plugin: 'linkedin', + commitHash: null, + sourcePath: path.join(pluginsDir, 'linkedin', 'search.js'), + sourceSha256: 'abc', + basePath: path.join(userClisDir, '.base', 'linkedin', 'search.js'), + createdAt: new Date().toISOString(), + dependencies: [{ path: 'shared.js', sha256: 'def' }], + }, + }), + 'utf-8', + ); + + expect(findShadowedUserAdapters({ userClisDir, pluginsDir, homeDir }).map((s) => s.name)) + .toEqual(['twitter/shared']); + }); + }); + it('yields no shadows when the plugins dir does not exist yet (no plugins installed)', () => { withTempDirs(({ userClisDir, pluginsDir, homeDir }) => { fs.mkdirSync(userClisDir, { recursive: true }); diff --git a/src/adapter-shadow.ts b/src/adapter-shadow.ts index 7399dd8a..76694019 100644 --- a/src/adapter-shadow.ts +++ b/src/adapter-shadow.ts @@ -1,7 +1,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { readOverrideRecords } from './override-provenance.js'; +import { readOverrideRecords, type OverrideRecord } from './override-provenance.js'; export type AdapterShadow = { name: string; @@ -49,6 +49,29 @@ function assertPluginsDirUsable(pluginsDir: string): void { } } +/** + * Files in `clis//` that webcmd copied there itself, as part of a + * tracked fork's import closure. + * + * These shadow a plugin file by construction — an override cannot load + * without them — so reporting them as unexplained local adapters would turn + * every fork of a command with a sibling import into a `doctor` warning + * telling the user to reset the fork they just made. `adapter reset ` + * removes them along with the override, and `plugin update` already reports + * upstream drift in them through the owning record. + */ +function copiedDependencyFiles( + provenance: Record, + site: string, +): Set { + const files = new Set(); + for (const record of Object.values(provenance)) { + if (record.plugin !== site) continue; + for (const dependency of record.dependencies ?? []) files.add(dependency.path); + } + return files; +} + export function findShadowedUserAdapters(opts: AdapterShadowOptions = {}): AdapterShadow[] { const userClisDir = opts.userClisDir ?? path.join(os.homedir(), '.webcmd', 'clis'); const pluginsDir = opts.pluginsDir ?? path.join(os.homedir(), '.webcmd', 'plugins'); @@ -61,9 +84,11 @@ export function findShadowedUserAdapters(opts: AdapterShadowOptions = {}): Adapt const site = siteEntry.name; const userSiteDir = path.join(userClisDir, site); const pluginSiteDir = path.join(pluginsDir, site); + const copiedDependencies = copiedDependencyFiles(provenance, site); for (const commandEntry of readdirOrEmpty(userSiteDir)) { if (!commandEntry.isFile() || !commandEntry.name.endsWith('.js')) continue; + if (copiedDependencies.has(commandEntry.name)) continue; const userPath = path.join(userSiteDir, commandEntry.name); const pluginPath = path.join(pluginSiteDir, commandEntry.name); if (!fs.existsSync(pluginPath)) continue; diff --git a/src/cli.test.ts b/src/cli.test.ts index 445a6ab0..c5ba29ad 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -130,6 +130,7 @@ describe('plugin update reconciliation reporting', () => { yours: '/tmp/home/.webcmd/clis/beta/search.js', upstream: '/tmp/home/.webcmd/plugins/beta/search.js', base: '/tmp/home/.webcmd/clis/.base/beta/search.js', + changedDependencies: [], }]); const discover = vi.spyOn(discoveryModule, 'discoverPlugins').mockResolvedValue(); @@ -348,12 +349,63 @@ describe('override reporting surfaces', () => { await createProgram('', userClis, pluginsDir) .parseAsync(['node', 'webcmd', 'adapter', 'status', '--format', 'json']); expect(JSON.parse(stdoutSpy.mock.calls.flat().join('\n'))).toEqual([ - { command: 'linkedin/search', kind: 'override', plugin: 'linkedin', reconciliationNeeded: true, orphaned: false, loadError: null }, - { command: 'local/run', kind: 'user', plugin: null, reconciliationNeeded: false, orphaned: false, loadError: null }, - { command: 'old/search', kind: 'override', plugin: 'old', reconciliationNeeded: false, orphaned: true, loadError: null }, + { command: 'linkedin/search', kind: 'override', plugin: 'linkedin', requiredBy: null, reconciliationNeeded: true, orphaned: false, loadError: null }, + { command: 'local/run', kind: 'user', plugin: null, requiredBy: null, reconciliationNeeded: false, orphaned: false, loadError: null }, + { command: 'old/search', kind: 'override', plugin: 'old', requiredBy: null, reconciliationNeeded: false, orphaned: true, loadError: null }, ]); }); + it('reports a file copied for an override as a dependency, not a user adapter', async () => { + // An override copies its import closure into clis/. Reported as + // `user adapter`, those copies read as junk the user can delete — which + // breaks the override that needs them. + const userClis = path.join(home, '.webcmd', 'clis'); + const pluginsDir = path.join(home, '.webcmd', 'plugins'); + const upstream = path.join(pluginsDir, 'linkedin', 'timeline.js'); + fs.mkdirSync(path.dirname(upstream), { recursive: true }); + fs.mkdirSync(path.join(userClis, 'linkedin'), { recursive: true }); + fs.writeFileSync(upstream, '// upstream\n'); + fs.writeFileSync(path.join(userClis, 'linkedin', 'timeline.js'), '// override\n'); + fs.writeFileSync(path.join(userClis, 'linkedin', 'shared.js'), '// copied helper\n'); + fs.writeFileSync(path.join(home, '.webcmd', 'override-provenance.json'), JSON.stringify({ + 'linkedin/timeline': { + plugin: 'linkedin', commitHash: null, sourcePath: upstream, sourceSha256: 'abc', + basePath: '/tmp/base.js', createdAt: '2026-08-09T00:00:00.000Z', + dependencies: [{ path: 'shared.js', sha256: 'def' }], + }, + })); + + await createProgram('', userClis, pluginsDir) + .parseAsync(['node', 'webcmd', 'adapter', 'status', '--format', 'json']); + expect(JSON.parse(stdoutSpy.mock.calls.flat().join('\n'))).toMatchObject([ + { command: 'linkedin/shared', kind: 'dependency', plugin: 'linkedin', requiredBy: 'linkedin/timeline' }, + { command: 'linkedin/timeline', kind: 'override', requiredBy: null }, + ]); + }); + + it('names the override that imports a copied file in adapter status', async () => { + const userClis = path.join(home, '.webcmd', 'clis'); + const pluginsDir = path.join(home, '.webcmd', 'plugins'); + const upstream = path.join(pluginsDir, 'linkedin', 'timeline.js'); + fs.mkdirSync(path.dirname(upstream), { recursive: true }); + fs.mkdirSync(path.join(userClis, 'linkedin'), { recursive: true }); + fs.writeFileSync(upstream, '// upstream\n'); + fs.writeFileSync(path.join(userClis, 'linkedin', 'timeline.js'), '// override\n'); + fs.writeFileSync(path.join(userClis, 'linkedin', 'shared.js'), '// copied helper\n'); + fs.writeFileSync(path.join(home, '.webcmd', 'override-provenance.json'), JSON.stringify({ + 'linkedin/timeline': { + plugin: 'linkedin', commitHash: null, sourcePath: upstream, sourceSha256: 'abc', + basePath: '/tmp/base.js', createdAt: '2026-08-09T00:00:00.000Z', + dependencies: [{ path: 'shared.js', sha256: 'def' }], + }, + })); + + await createProgram('', userClis, pluginsDir) + .parseAsync(['node', 'webcmd', 'adapter', 'status']); + expect(stdoutSpy.mock.calls.flat().join('\n')) + .toContain('copied for override: linkedin/shared (imported by linkedin/timeline)'); + }); + it('reports an empty adapter status as JSON', async () => { const userClis = path.join(home, '.webcmd', 'clis'); diff --git a/src/cli.ts b/src/cli.ts index bae75c48..eaf2d038 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -677,10 +677,16 @@ async function handleAdapterOverride(commandKey: string, _opts: unknown, command plugin: result.plugin, overridePath: result.overridePath, basePath: result.basePath, + dependencies: result.dependencies, }, () => { console.log(`✅ Override created for ${result.commandKey}`); console.log(` yours: ${result.overridePath}`); console.log(` base: ${result.basePath}`); + if (result.dependencies.length > 0) { + // The fork is more than one file. Say so, or the extra files look + // like clutter the user is free to delete. + console.log(` also copied (imported by your command): ${result.dependencies.join(', ')}`); + } console.log(); console.log(` Your copy now takes precedence over plugin "${result.plugin}".`); console.log(` "${CLI_COMMAND} plugin update" keeps updating the plugin copy, not your override,`); @@ -1479,6 +1485,12 @@ cli({ } else { console.log(` base: unavailable (merge base was deleted)`); } + if (need.changedDependencies.length > 0) { + // Named individually: the drift is in a file the user never asked to + // fork, so "your override is stale" is not actionable without saying + // which copied file moved upstream. + console.log(` also changed upstream: ${need.changedDependencies.join(', ')}`); + } } console.log(` Your override still takes precedence. Merge the upstream change, or run`); console.log(` ${CLI_COMMAND} adapter reset to drop the override.`); @@ -1880,10 +1892,20 @@ cli({ const records = readOverrideRecords(); const reconcile = new Set((await import('./plugin.js')).findOverridesNeedingReconcile().map(({ commandKey }) => commandKey)); const failures = new Map(getAdapterLoadFailures().map(failure => [failure.file, failure.error])); + // Files an override copied to stay loadable are not adapters the user + // wrote. Listing them as `user adapter` invites a reset of a file the + // fork needs, so name the override that owns each one instead. + const dependencyOwners = new Map(); + for (const [commandKey, record] of Object.entries(records)) { + for (const dependency of record.dependencies ?? []) { + dependencyOwners.set(`${record.plugin}/${dependency.path}`, commandKey); + } + } const adapters: Array<{ command: string; - kind: 'user' | 'override'; + kind: 'user' | 'override' | 'dependency'; plugin: string | null; + requiredBy: string | null; reconciliationNeeded: boolean; orphaned: boolean; loadError: string | null; @@ -1894,23 +1916,45 @@ cli({ const command = `${site}/${file.slice(0, -3)}`; const record = records[command]; const loadError = failures.get(path.join(USER_CLIS, site, file)) ?? null; - adapters.push(record - ? { - command, - kind: 'override', - plugin: record.plugin, - reconciliationNeeded: reconcile.has(command), - orphaned: !fs.existsSync(path.join(pluginsDir, record.plugin)), - loadError, - } - : { command, kind: 'user', plugin: null, reconciliationNeeded: false, orphaned: false, loadError }); + const owner = dependencyOwners.get(`${site}/${file}`); + if (record) { + adapters.push({ + command, + kind: 'override', + plugin: record.plugin, + requiredBy: null, + reconciliationNeeded: reconcile.has(command), + orphaned: !fs.existsSync(path.join(pluginsDir, record.plugin)), + loadError, + }); + } else if (owner) { + adapters.push({ + command, + kind: 'dependency', + plugin: site, + requiredBy: owner, + reconciliationNeeded: false, + orphaned: false, + loadError, + }); + } else { + adapters.push({ + command, + kind: 'user', + plugin: null, + requiredBy: null, + reconciliationNeeded: false, + orphaned: false, + loadError, + }); + } } } if (fmt !== 'table') { renderOutput(adapters, { fmt, fmtExplicit: outputFormatIsExplicit(adapterStatusCmd), - columns: ['command', 'kind', 'plugin', 'reconciliationNeeded', 'orphaned', 'loadError'], + columns: ['command', 'kind', 'plugin', 'requiredBy', 'reconciliationNeeded', 'orphaned', 'loadError'], title: `${CLI_COMMAND}/adapter-status`, source: `${CLI_COMMAND} adapter status`, }); @@ -1923,6 +1967,8 @@ cli({ const failure = adapter.loadError ? ` (failed to load: ${adapter.loadError})` : ''; if (adapter.kind === 'user') { console.log(` user adapter: ${adapter.command}${failure}`); + } else if (adapter.kind === 'dependency') { + console.log(` copied for override: ${adapter.command} (imported by ${adapter.requiredBy})${failure}`); } else if (adapter.orphaned) { console.log(` orphaned override: ${adapter.command} (plugin ${adapter.plugin} is not installed)${failure}`); } else { diff --git a/src/override-provenance.ts b/src/override-provenance.ts index 9b479207..b1e20471 100644 --- a/src/override-provenance.ts +++ b/src/override-provenance.ts @@ -16,6 +16,17 @@ import * as path from 'node:path'; import { createHash } from 'node:crypto'; import { isRecord } from './utils.js'; +/** + * A file the forked command imports, copied alongside it so the override can + * load. `path` is relative to both the plugin directory it came from and the + * override directory it was copied into, so one string locates the upstream + * file, the user's copy, and the base copy. + */ +export interface OverrideDependency { + path: string; + sha256: string; +} + export interface OverrideRecord { plugin: string; commitHash: string | null; @@ -23,6 +34,12 @@ export interface OverrideRecord { sourceSha256: string; basePath: string; createdAt: string; + /** + * Optional: records written before overrides copied their import closure + * have no dependencies key, and must keep loading rather than being + * rejected as malformed. + */ + dependencies?: OverrideDependency[]; } function resolveHomeDir(homeDir?: string): string { @@ -39,6 +56,15 @@ export function getBaseCopyPath(commandKey: string, homeDir?: string): string { return path.join(resolveHomeDir(homeDir), '.webcmd', 'clis', '.base', `${commandKey}.js`); } +/** + * Path to the fork-time base copy of a dependency copied alongside a fork. + * `relPath` is the record's dependency path, mirrored under the same + * `.base//` root the command file's base copy lives in. + */ +export function getBaseDependencyPath(site: string, relPath: string, homeDir?: string): string { + return path.join(resolveHomeDir(homeDir), '.webcmd', 'clis', '.base', site, ...relPath.split('/')); +} + /** * Read all override records. A missing store is normal and returns {}. * A malformed store throws, naming the path — silently returning {} here @@ -75,6 +101,13 @@ export function readOverrideRecords(homeDir?: string): Record; } +function isValidDependencyList(value: unknown): value is OverrideDependency[] { + return ( + Array.isArray(value) && + value.every((entry) => isRecord(entry) && typeof entry.path === 'string' && typeof entry.sha256 === 'string') + ); +} + function isValidOverrideRecord(value: unknown): value is OverrideRecord { if (!isRecord(value)) return false; return ( @@ -83,7 +116,8 @@ function isValidOverrideRecord(value: unknown): value is OverrideRecord { typeof value.sourcePath === 'string' && typeof value.sourceSha256 === 'string' && typeof value.basePath === 'string' && - typeof value.createdAt === 'string' + typeof value.createdAt === 'string' && + (value.dependencies === undefined || isValidDependencyList(value.dependencies)) ); } @@ -111,10 +145,19 @@ export function removeOverrideRecords(site: string, homeDir?: string): string[] for (const key of Object.keys(records)) { if (key !== site && !key.startsWith(prefix)) continue; removed.push(key); + const record = records[key]!; delete records[key]; try { fs.rmSync(getBaseCopyPath(key, homeDir), { force: true }); } catch {} + // The command's own base copy is not the whole fork: the files copied to + // make it loadable have base copies too, and leaving them behind would + // leak stale bytes into the next fork of the same plugin. + for (const dependency of record.dependencies ?? []) { + try { + fs.rmSync(getBaseDependencyPath(record.plugin, dependency.path, homeDir), { force: true }); + } catch {} + } } if (removed.length > 0) writeOverrideRecords(records, homeDir); diff --git a/src/plugin.test.ts b/src/plugin.test.ts index 87f739ef..b83150c4 100644 --- a/src/plugin.test.ts +++ b/src/plugin.test.ts @@ -2268,6 +2268,7 @@ describe('findOverridesNeedingReconcile', () => { yours: path.join(clisDir, 'linkedin', 'search.js'), upstream: pluginFile, base: path.join(clisDir, '.base', 'linkedin', 'search.js'), + changedDependencies: [], }]); }); @@ -2287,6 +2288,45 @@ describe('findOverridesNeedingReconcile', () => { fs.writeFileSync(pluginFile, '// upstream moved again\n', 'utf-8'); expect(pluginModule.findOverridesNeedingReconcile(['linkedin'])[0]!.base).toBeNull(); }); + + it('reports an override whose copied dependency changed upstream, naming the file', () => { + // The fork runs its own copy of shared.js. Without this, a command whose + // real logic lives in a helper could go stale forever without a word. + const helper = path.join(pluginsDir, 'linkedin', 'shared.js'); + fs.writeFileSync(helper, '// shared v1\n', 'utf-8'); + fs.writeFileSync(path.join(pluginsDir, 'linkedin', 'timeline.js'), "import './shared.js';\n", 'utf-8'); + createAdapterOverride('linkedin/timeline'); + + fs.writeFileSync(helper, '// shared v2\n', 'utf-8'); + + const needs = pluginModule.findOverridesNeedingReconcile(['linkedin']); + expect(needs.map((need) => need.commandKey)).toContain('linkedin/timeline'); + expect(needs.find((need) => need.commandKey === 'linkedin/timeline')!.changedDependencies) + .toEqual(['shared.js']); + }); + + it('reports a copied dependency that was deleted upstream', () => { + const helper = path.join(pluginsDir, 'linkedin', 'shared.js'); + fs.writeFileSync(helper, '// shared v1\n', 'utf-8'); + fs.writeFileSync(path.join(pluginsDir, 'linkedin', 'timeline.js'), "import './shared.js';\n", 'utf-8'); + createAdapterOverride('linkedin/timeline'); + + fs.rmSync(helper); + + expect(pluginModule.findOverridesNeedingReconcile(['linkedin']) + .find((need) => need.commandKey === 'linkedin/timeline')!.changedDependencies) + .toEqual(['shared.js']); + }); + + it('does NOT report an override whose copied dependency is unchanged upstream', () => { + fs.writeFileSync(path.join(pluginsDir, 'linkedin', 'shared.js'), '// shared v1\n', 'utf-8'); + fs.writeFileSync(path.join(pluginsDir, 'linkedin', 'timeline.js'), "import './shared.js';\n", 'utf-8'); + createAdapterOverride('linkedin/timeline'); + + expect(pluginModule.findOverridesNeedingReconcile(['linkedin']) + .map((need) => need.commandKey)) + .not.toContain('linkedin/timeline'); + }); }); describe('listPlugins override reporting', () => { diff --git a/src/plugin.ts b/src/plugin.ts index 2b57d54b..02b5de0b 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -1462,6 +1462,11 @@ export interface OverrideReconcileNeed { yours: string; upstream: string; base: string | null; + /** + * Files copied alongside the fork whose upstream bytes have changed since, + * relative to the plugin directory. Empty when only the command file drifted. + */ + changedDependencies: string[]; } /** @@ -1472,6 +1477,12 @@ export interface OverrideReconcileNeed { * override on every unrelated update. Comparing the file's own sha256 * against the override record's sourceSha256 only flags overrides whose * actual upstream content changed. + * + * A fork owns the files it copied to stay loadable, so upstream changes in + * those are reported too. Without that, a fork of a command whose real logic + * lives in `./shared.js` would keep running its fork-time copy of that helper + * and never be told upstream moved — the same silent staleness this check + * exists to prevent, just one import away. */ export function findOverridesNeedingReconcile(pluginNames?: string[]): OverrideReconcileNeed[] { const homeDir = getHomeDir(); @@ -1483,7 +1494,19 @@ export function findOverridesNeedingReconcile(pluginNames?: string[]): OverrideR // Plugin was uninstalled: no upstream to reconcile against. Task 7's // `adapter status` surfaces these separately as orphaned. if (!fs.existsSync(record.sourcePath)) continue; - if (fileSha256(record.sourcePath) === record.sourceSha256) continue; + + const pluginDir = path.dirname(record.sourcePath); + const changedDependencies = (record.dependencies ?? []) + .filter((dependency) => { + const upstreamPath = path.join(pluginDir, ...dependency.path.split('/')); + // A dependency deleted upstream is a change the user must see, not one to skip. + if (!fs.existsSync(upstreamPath)) return true; + return fileSha256(upstreamPath) !== dependency.sha256; + }) + .map((dependency) => dependency.path); + + const commandChanged = fileSha256(record.sourcePath) !== record.sourceSha256; + if (!commandChanged && changedDependencies.length === 0) continue; needs.push({ commandKey, @@ -1491,6 +1514,7 @@ export function findOverridesNeedingReconcile(pluginNames?: string[]): OverrideR yours: path.join(homeDir, '.webcmd', 'clis', `${commandKey}.js`), upstream: record.sourcePath, base: fs.existsSync(record.basePath) ? record.basePath : null, + changedDependencies, }); }