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, }); }