diff --git a/CHANGELOG.md b/CHANGELOG.md index d45c67b38..40510428a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to EGC are documented here. - **The memory protocol says the state store is owned by the server and what to do without `get_state`** (#1398, found through @Akisolu's follow-up checks in #1380): the protocol text still called the state files plain Markdown and pointed agents at a per-project path, so an agent whose tool had no MCP servers registered read and wrote memory on the filesystem itself, unencrypted. Every variant now says the store is owned by `egc-memory`, encrypted, one file per project and branch, never read or written directly, and that a missing `get_state` means the server is not registered: say so and point at `egc init`. The protocol version is unchanged, so the wording reaches fresh installs; the in-place upgrade of existing blocks waits for the next protocol bump (#1395). - **Existing installs receive the corrected memory protocol, and the doctor points at state files left in plain text** (#1401, closes #1395, found through @Akisolu's follow-up checks in #1380): the protocol text fixed in #1398 only reached new installs, because the in-place upgrade is keyed on the protocol version and that stayed at 5; the cognitive protocol is now v6, so the next `egc init` or `egc auto-update` rewrites the block in every context file of an install already in place (Cursor `cursor.rules` and the Codex TOML string included). `egc doctor` also gained a `State files` section: it reads the first bytes of every state file under `~/.egc/state` (the per-project directories too, never following links, never decrypting) and lists the ones without the encrypted header, with the last write of each, the first five shown and the rest counted, plus the guidance that a plain file either predates 1.1.6 or was written straight to disk by a tool with no `egc-memory` server, that the server encrypts it on its next save, and that `egc init` in that project gives the tool the server. The JSON report carries the same finding as `plaintextStateFiles`. - **The DCO check skips the merge commits GitHub itself creates** (#1413): a merge made through "Update branch" or the update-branch API is authored as the maintainer with committer GitHub, joins histories that were already checked, carries no authored content and never carries a sign-off anyone can add, so it turned the check red on a contributor's PR for a commit they did not write. Such a commit (two or more parents, committer `noreply@github.com`) is skipped; a merge a person made locally is still checked like any other commit. +- **Every install target retires the files a renamed or dropped command, prompt, rule or skill leaves behind, not just OpenCode** (#1412, generalizes #1411): when a file left the install plan, the copy an earlier install wrote used to stay in the target forever: `egc doctor` reported it as orphaned and `egc repair` pruned the state entry, but nothing removed the file, because the installer's rule is that it never deletes what the current plan does not cover. `planRetirements` now defaults to a plan-diff instead of an empty list: it compares the previous install-state's recorded `copy-file` operations against what today's plan would actually write (a directory-shaped scaffold entry still shields every file copied under it), and offers up whatever is no longer covered. The same identity check #1411 introduced decides whether a candidate is actually safe to delete (a regular file, reached through no link, byte-identical to the source EGC copied), listed by `--dry-run` under `Files to retire` and reported as `retired file`, so renaming a command or prompt now cleans up after itself on the next install or auto-update, on every target. Only a file recorded by a module the plan still selects is offered up: a targeted `--modules` install or a narrower profile leaves the files of the modules it did not select where they are. A target that writes into a second directory (Amp's plugin config root) declares it, and the apply checks each candidate against the root it belongs to. A source no longer in the repository (the file was renamed or moved in the package) is retired only when the bytes on disk match a file the plan copies today; otherwise it is left in place, listed nowhere, same as a file the person edited. OpenCode keeps its own narrower `planRetirements` (the egc-universal package cleanup, plus the permanent `opencode.json` exception), unaffected. Merge-json and hook operations have no retirement counterpart yet. - **The OpenCode install no longer dumps the egc-universal package into the config directory, which made OpenCode stop responding on every request** (#1396, reported and bisected by @Akisolu across two log sets): the `opencode` target copied the whole `.opencode/` folder of the repository, the source of the npm package, into `~/.config/opencode/`. OpenCode imports every `{tool,tools}/*.{js,ts}` and `{plugin,plugins}/*.{ts,js}` file it finds there at start, so the copied TypeScript sources failed to load and every prompt died with them; the copied `package.json` made OpenCode's own dependency install fail on every start; and the copied `opencode.json` replaced the person's global config (model, permission, plugin list) on every install and auto-update. The target now plans only the markdown OpenCode reads or that stays inert from that folder (`commands`, `instructions`, `prompts`) next to the real plugin, skills and hooks. Installs already affected are repaired by the next `egc install` or `egc auto-update`: the files an earlier install wrote from the package are read from the install-state and retired (only a regular file still byte-identical to what EGC copied, reached through no link, never a link or a directory; a file the person replaced stays; emptied directories dropped), listed by `--dry-run` under `Files to retire` and reported as `retired file`. The person's `opencode.json` is never overwritten or removed again. - **The installer migrates its own June 2026 linked skills layout instead of refusing to update it** (#1400): machines set up before 10 June 2026 have the Antigravity CLI skills as one link per skill into the Gemini home copy (`~/.gemini/skills/egc/`), and the hardened installer refused to write through them, so `egc auto-update` failed on that target on every run while `egc doctor` still reported it as OK. A link below the target root whose resolved target sits inside that managed copy is now recognised as EGC's own layout: the link is removed (never what it pointed at) and the real files are written in its place, reported as `migrated legacy link` in the result and listed by `egc install --dry-run` under `Legacy links to migrate`. A link that resolves anywhere else, or a dangling one, keeps the refusal. Tests cover the migration, the dry run, the foreign link, the dangling link, and a full apply of the `egc` target over the legacy layout; the troubleshooting page explains both cases. - **OpenCode receives the EGC servers where it reads them** (#1405, found through @Akisolu's bisect in #1396): the registration wrote a `mcpServers` block, the shape Claude-style configs use, into `~/.config/opencode/config.json`, and on Windows only when that file already existed, with a second target under `%APPDATA%` that OpenCode never reads. OpenCode resolves its config directory through xdg-basedir (`~/.config/opencode` on every platform), loads `opencode.json` and the legacy `config.json`, and reads MCP servers from the `mcp` key as `{ type: "local", command: [...] }`. `egc init` and the installers now register into `opencode.json` (or `config.json` when only that exists) under `mcp` in that shape, gate on the config directory or the `opencode` binary on every platform, retire the `mcpServers` block an older EGC left behind (entries the person added stay), and the guardian's config fallback reads the new shape. Seven registration tests and one fallback test cover the fresh file, an existing file with other servers, the legacy file, a foreign block, an invalid container, the gate per platform and `XDG_CONFIG_HOME`. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 6454fcca8..578292f17 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -19,6 +19,7 @@ This document describes the planned development direction for EGC (Extended Glob - The Token Crusher keeps the lines that say why a test failed (Jest, Vitest and Mocha markers and headers, `Received` and `Expected` diffs, Go locations and runtime frames), and the Guardian git checks went through a hardening round with a test for every form now refused: `git push` refspecs, `git config` writes in any flag order or letter case, inline `-c` and `--config-env` overrides, alias values (#1403 by @prateeekbuilds, mission #1382, nine review rounds with each case probed against `main`). - The first steps of `egc init` are check lines like the rest of the run: `cognitive protocol` with the number of tools at the current protocol and the ones installed or upgraded, `MCP registration` with the tools already registered and the ones written, `memory` with the migration count of the store just opened, and `commit-privacy filter` listing what it changed once and reporting the filter as configured afterwards; the filter planner reads the local values and lists only what is missing, so the installers stop announcing three changes on a configured repository (#1416). - A parked `session_wait` no longer depends on the file watcher alone: while a waiter is parked the mesh transport also polls the size and mtime of the store and its `-wal` every 500 ms, so a silent FSEvents stream on macOS costs half a second instead of the repoll ceiling, and the parked-waiter test stops flaking on the macOS runners (#1423, closes #1420). +- Every install target, not just OpenCode, retires the files a renamed or dropped command, prompt, rule or skill leaves behind: `planRetirements` defaults to diffing the previous install-state against what the current plan would actually write instead of an empty list, so a rename now cleans up its old destination on the next install or auto-update everywhere, through the same identity check #1411 introduced (#1412, generalizes #1411). ## v1.1.21: The Hardening Round (Released 2026-09-05) - A security hardening round lands as eighteen steps in seventeen pull requests (#1356 to #1363, #1365 to #1373): the memory server, the installer, the hooks and the dashboard get tighter defaults and stricter checks on what they accept from disk, from the network and from the tools around them, each step with its own tests and its own review. Nothing changes in the documented commands; all of it ships with the next release. diff --git a/scripts/lib/install-executor.js b/scripts/lib/install-executor.js index a19aecdb7..edc15569a 100644 --- a/scripts/lib/install-executor.js +++ b/scripts/lib/install-executor.js @@ -793,6 +793,7 @@ function createManifestInstallPlan(options = {}) { installRoot: plan.targetRoot, installStatePath: plan.installStatePath, retirements: Array.isArray(plan.retirements) ? plan.retirements : [], + managedRoots: Array.isArray(plan.managedRoots) ? plan.managedRoots : [], // The structured issues ride along untouched: the CLI's detection gate // needs the machine-readable code (ide-not-detected), not just the // flattened warning strings below. diff --git a/scripts/lib/install-lifecycle.js b/scripts/lib/install-lifecycle.js index 05ded6fcd..381372603 100644 --- a/scripts/lib/install-lifecycle.js +++ b/scripts/lib/install-lifecycle.js @@ -1257,7 +1257,15 @@ function describeUnrepairable(unrepairable) { } // Drops the orphaned entries from the state that is about to be rewritten, -// so doctor converges to OK; the installed files stay on disk. +// so doctor converges to OK; the installed files stay on disk. This is the +// "source is gone" half of cleaning up after a renamed or dropped file: a +// recorded operation whose source the reference repo no longer has cannot +// be identity-checked, so repair only forgets it here. The "source is +// still there, just not part of the plan anymore" half, the common case +// for a rename, is handled separately by planGenericRetirements +// (install-targets/helpers.js), which the next install or auto-update +// runs and which does remove the file, after the same identity check +// (#1412). function pruneOrphanedOperations(desiredPlan, orphanedInspections) { if (orphanedInspections.length === 0) return; const orphanKeys = new Set(orphanedInspections.map(entry => operationIdentityKey(entry.operation))); diff --git a/scripts/lib/install-manifests.js b/scripts/lib/install-manifests.js index e85af2584..a87e0960d 100644 --- a/scripts/lib/install-manifests.js +++ b/scripts/lib/install-manifests.js @@ -602,6 +602,7 @@ function buildInstallPlanResult({ validationIssues: scaffoldPlan ? scaffoldPlan.validationIssues : [], operations: scaffoldPlan ? scaffoldPlan.operations : [], retirements: scaffoldPlan ? (scaffoldPlan.retirements || []) : [], + managedRoots: scaffoldPlan ? (scaffoldPlan.managedRoots || []) : [], }; } diff --git a/scripts/lib/install-targets/amp-home.js b/scripts/lib/install-targets/amp-home.js index 7710ee5ea..0664d921a 100644 --- a/scripts/lib/install-targets/amp-home.js +++ b/scripts/lib/install-targets/amp-home.js @@ -93,6 +93,14 @@ module.exports = createInstallTargetAdapter({ rootSegments: ['.amp'], installStatePathSegments: ['egc', 'install-state.json'], nativeRootRelativePath: '.amp', + // Skills land under ~/.amp while the Guardian/Crusher/Mesh plugin scripts + // are copied to ~/.config/amp/plugins/ (an XDG-style location Amp's plugin + // API owns, separate from the skills root). Retirement must trust both + // roots or it can never retire the plugin scripts it itself installed + // (cubic review, #1412). + resolveManagedRoots(input, adapter) { + return [adapter.resolveRoot(input), resolveAmpConfigRoot(input.homeDir)]; + }, planOperations(input, adapter) { const configRoot = resolveAmpConfigRoot(input.homeDir); diff --git a/scripts/lib/install-targets/helpers.js b/scripts/lib/install-targets/helpers.js index bec9567a4..37af6d2a0 100644 --- a/scripts/lib/install-targets/helpers.js +++ b/scripts/lib/install-targets/helpers.js @@ -368,6 +368,211 @@ function createDefaultScaffoldOperations(input, adapter) { }); } +// What today's scaffold operations would actually write, at the +// granularity the install-state records: single files in `files`, +// whole source directories (still copied recursively, one child file +// at a time) in `dirs`. Mirrors the file/directory split +// materializeScaffoldOperation (install-executor.js) applies when it +// turns these same scaffold operations into the copy-file entries +// the state records, so a directory scaffold entry here shields every +// file under it even though no single copy-file operation names the +// directory itself. +function collectCurrentlyCoveredDestinations(operations, repoRoot) { + const files = new Set(); + const dirs = new Set(); + for (const operation of Array.isArray(operations) ? operations : []) { + const destination = typeof operation.destinationPath === 'string' ? operation.destinationPath : null; + if (!destination) continue; + const resolvedDestination = path.resolve(destination); + const source = typeof operation.sourceRelativePath === 'string' ? operation.sourceRelativePath : null; + if (!source) { + // Nothing repo-relative to check the shape of (a merge or hook + // operation may carry its payload some other way): treat the + // destination as covered rather than guess, so it is never offered + // up for retirement by mistake. + files.add(resolvedDestination); + continue; + } + let stat; + try { + stat = fs.statSync(path.join(repoRoot, ...normalizeRelativePath(source).split('/'))); + } catch { + // Source unreadable from here: same reasoning, stay conservative. + files.add(resolvedDestination); + continue; + } + (stat.isDirectory() ? dirs : files).add(resolvedDestination); + } + return { files, dirs }; +} + +function isDestinationCovered(resolved, { files, dirs }) { + if (files.has(resolved)) return true; + for (const dir of dirs) { + if (resolved === dir || resolved.startsWith(dir + path.sep)) return true; + } + return false; +} + +// The roots an adapter may legitimately write install files under. Most +// adapters write everything under their own resolveRoot(), but a few +// deliberately land copies in a second location (Amp's plugin scripts go to +// ~/.config/amp/plugins/ via resolveAmpConfigRoot(), distinct from its skills +// root ~/.amp/). A retirement candidate is only ever considered for one of +// these trusted roots, so a target with a second root must declare it here or +// its own writes can never be retired. +function resolveAdapterManagedRoots(adapter, input = {}) { + if (typeof adapter.resolveManagedRoots === 'function') { + const declared = adapter.resolveManagedRoots(input); + if (Array.isArray(declared)) { + return declared.filter(root => typeof root === 'string' && root.length > 0); + } + } + return [adapter.resolveRoot(input)]; +} + +// The managed copy-file operations an install-state records, the shape both +// the sibling check and the retirement diff read. +function recordedManagedCopies(state) { + const operations = Array.isArray(state?.operations) ? state.operations : []; + return operations.filter(operation => ( + operation.ownership === 'managed' + && operation.kind === 'copy-file' + && typeof operation.destinationPath === 'string' + && operation.destinationPath.length > 0 + )); +} + +// The marker for an install-state that exists but cannot be read or parsed: +// unlike a missing one it may still record destinations, so it must never be +// mistaken for an empty one. +const UNREADABLE_STATE = Symbol('unreadable install-state'); + +// An install-state, null when nothing sits at the path (a definite ENOENT +// and nothing else: a dangling link, a parent that cannot be inspected or +// is not a directory all count as a state that cannot be trusted), and +// UNREADABLE_STATE when something is there but cannot be read or parsed. +function readInstallStateOrNull(statePath) { + if (typeof statePath !== 'string' || statePath.length === 0) return null; + try { + fs.lstatSync(statePath); + } catch (error) { + return error.code === 'ENOENT' ? null : UNREADABLE_STATE; + } + const { readInstallState } = require('../install-state'); + try { + return readInstallState(statePath); + } catch { + return UNREADABLE_STATE; + } +} + +// Destinations a sibling adapter sharing the same trusted root still records +// as its own managed copies. codex-home, goose-home and openhands-home all +// write skills into the shared ~/.agents tree, each with its own +// install-state: if one adapter removes a skill from its plan, its retirement +// diff must not delete the file another adapter still installs. The sibling +// coverage check reads those sibling state files (passed in by registry.js +// when it plans) and refuses any candidate another adapter still owns. A +// sibling with no state file owns nothing; a sibling whose state exists but +// cannot be read may own any of them, so the answer is null: unknown +// ownership fails closed, never open. +function collectSiblingOwnedDestinations(statePaths) { + const owned = new Set(); + for (const statePath of Array.isArray(statePaths) ? statePaths : []) { + const siblingState = readInstallStateOrNull(statePath); + if (siblingState === UNREADABLE_STATE) return null; + for (const operation of recordedManagedCopies(siblingState)) { + owned.add(path.resolve(operation.destinationPath)); + } + } + return owned; +} + +// Whether a recorded destination is still a candidate once the boundaries +// apply: inside a root this adapter manages, not already offered, not owned +// by a sibling, and not covered by what today's plan writes. +function isRetirementCandidate(resolved, { managedRoots, seen, siblingOwned, covered }) { + if (!managedRoots.some(root => resolved.startsWith(root + path.sep))) return false; + if (seen.has(resolved) || siblingOwned.has(resolved)) return false; + return !isDestinationCovered(resolved, covered); +} + +// The default planRetirements body: compares the previous install-state's +// managed copy-file operations against what this plan would write today. A +// destination the state remembers EGC copied, recorded by a module this plan +// still selects, and that no scaffold operation in the plan still covers, is +// offered up for retirement. Renaming or dropping a command, prompt, rule or +// skill from the package is exactly this: the old destination stops being +// covered and is cleaned up on the next install or auto-update. +// +// The module gate is what tells a rename apart from a module that simply was +// not selected this run: a targeted --modules install, or a narrower profile, +// leaves the files of the modules it did not select exactly where they are. +// +// Only recorded copy-file operations are diffed; merge-json and hook +// operations (settings.json entries, MCP config merges) have no counterpart +// here yet, an open question left by #1412. +// +// This only decides which destinations are candidates. The identity check +// that decides whether one is actually safe to delete (a regular file, +// reached through no link, byte-identical to the source EGC copied) happens +// later in install/apply.js's isRetirableFile, the same test #1411 +// introduced for OpenCode. A source no longer in the repository (the file was +// renamed, moved or removed) passes that check only when the bytes on disk +// match a file the plan copies today; otherwise the candidate is listed +// nowhere and the file is left in place, never deleted on the strength of the +// state entry alone. +function planGenericRetirements(input, adapter) { + // The operations being diffed were planned against the package source + // root; without it identities cannot be compared, so the conservative + // answer is to retire nothing rather than to guess a directory. + const repoRoot = typeof input.repoRoot === 'string' && input.repoRoot.length > 0 ? input.repoRoot : null; + const previous = repoRoot ? readInstallStateOrNull(adapter.getInstallStatePath(input)) : null; + if (!previous || previous === UNREADABLE_STATE) return []; + + // Destinations another adapter sharing this root still manages: never a + // retirement candidate here, whatever this adapter's own coverage says. A + // sibling state that cannot be read may still own any of them, so nothing + // is retired until it can be trusted again. + const siblingOwned = collectSiblingOwnedDestinations(input.siblingStatePaths); + if (!siblingOwned) return []; + + const selectedModuleIds = new Set( + (Array.isArray(input.modules) ? input.modules : []) + .map(module => (module && typeof module.id === 'string' ? module.id : null)) + .filter(Boolean) + ); + const boundaries = { + managedRoots: resolveAdapterManagedRoots(adapter, input).map(root => path.resolve(root)), + seen: new Set(), + siblingOwned, + covered: collectCurrentlyCoveredDestinations( + Array.isArray(input.operations) ? input.operations : adapter.planOperations(input), + repoRoot + ), + }; + + const retirements = []; + for (const operation of recordedManagedCopies(previous)) { + if (!selectedModuleIds.has(operation.moduleId)) continue; + const resolved = path.resolve(operation.destinationPath); + if (!isRetirementCandidate(resolved, boundaries)) continue; + const source = normalizeRelativePath(String(operation.sourceRelativePath || '')); + if (!source) continue; + boundaries.seen.add(resolved); + retirements.push({ + destinationPath: resolved, + sourceRelativePath: source, + // The file EGC copied there, for the apply to compare against: a + // file the person replaced since is theirs and stays. + sourcePath: path.join(repoRoot, ...source.split('/')), + reason: 'file left the install plan', + }); + } + return retirements; +} + function createInstallTargetAdapter(config) { const adapter = { id: config.id, @@ -381,6 +586,19 @@ function createInstallTargetAdapter(config) { const baseRoot = resolveBaseRoot(config.kind, input); return path.join(baseRoot, ...config.rootSegments); }, + // The roots under which config.resolveManagedRoots (when declared) lets + // retirement plan writes; defaults to resolveRoot() alone. A target that + // lands install files in a second directory (Amp's plugin config root) + // declares the full list so its own writes can be retired too. + resolveManagedRoots(input = {}) { + if (typeof config.resolveManagedRoots === 'function') { + const declared = config.resolveManagedRoots(input, adapter); + if (Array.isArray(declared)) { + return declared.filter(root => typeof root === 'string' && root.length > 0); + } + } + return [adapter.resolveRoot(input)]; + }, getInstallStatePath(input = {}) { const root = adapter.resolveRoot(input); return path.join(root, ...config.installStatePathSegments); @@ -431,13 +649,16 @@ function createInstallTargetAdapter(config) { return createDefaultScaffoldOperations(input, adapter); }, // Files a previous install wrote that this plan no longer covers and - // that the target wants removed on the next apply; most targets have - // none (the installer never deletes what it did not write). + // that the target wants removed on the next apply. An adapter + // with its own rules (e.g. OpenCode's egc-universal package + // cleanup, narrower and with its own kept-files exception) + // defines config.planRetirements and is used as-is; every other + // target falls back to the generic plan-diff below (#1412). planRetirements(input = {}) { if (typeof config.planRetirements === 'function') { return config.planRetirements(input, adapter); } - return []; + return planGenericRetirements(input, adapter); }, supportsModule(module, input = {}) { if (typeof config.supportsModule === 'function') { @@ -479,5 +700,7 @@ module.exports = { normalizeModulesInput, normalizeRelativePath, planFlatSkillOperation, + planGenericRetirements, + resolveAdapterManagedRoots, resolveModulesPlan, }; diff --git a/scripts/lib/install-targets/registry.js b/scripts/lib/install-targets/registry.js index 73d5939be..5e13a5ade 100644 --- a/scripts/lib/install-targets/registry.js +++ b/scripts/lib/install-targets/registry.js @@ -1,3 +1,5 @@ +const path = require('node:path'); + const aiderProject = require('./aider-project'); const amazonqProject = require('./amazonq-project'); const amazonqHome = require('./amazonq-home'); @@ -90,6 +92,34 @@ function getInstallTargetAdapter(targetOrAdapterId) { return adapter; } +// The install-state paths of every other registered adapter that shares a +// managed root with this one. codex-home, goose-home and openhands-home all +// write skills into the same ~/.agents tree under separate state files, so +// when any one of them drops a file from its plan, retirement must check the +// siblings' states before deleting: the file may still be owned by one of +// the others (cubic review, #1412). +function collectSiblingInstallStatePaths(adapter, planningInput) { + const { resolveAdapterManagedRoots } = require('./helpers'); + const sharedRoots = new Set(resolveAdapterManagedRoots(adapter, planningInput).map(root => path.resolve(root))); + const siblingPaths = []; + + for (const sibling of ADAPTERS) { + if (sibling.id === adapter.id) continue; + try { + const sharesRoot = resolveAdapterManagedRoots(sibling, planningInput) + .map(root => path.resolve(root)) + .some(root => sharedRoots.has(root)); + if (sharesRoot) { + siblingPaths.push(sibling.getInstallStatePath(planningInput)); + } + } catch (_error) { // NOSONAR: an unplannable sibling root contributes no shared coverage + continue; + } + } + + return siblingPaths; +} + function planInstallTargetScaffold(options = {}) { const adapter = getInstallTargetAdapter(options.target); const modules = Array.isArray(options.modules) ? options.modules : []; @@ -109,11 +139,18 @@ function planInstallTargetScaffold(options = {}) { ...planningInput, modules, }); + // The generic default reuses these instead of planning a second time; + // an adapter with its own planRetirements is free to ignore the extra + // field. const retirements = adapter.planRetirements({ ...planningInput, modules, + operations, + siblingStatePaths: collectSiblingInstallStatePaths(adapter, planningInput), }); + const { resolveAdapterManagedRoots } = require('./helpers'); + return { adapter: { id: adapter.id, @@ -125,6 +162,10 @@ function planInstallTargetScaffold(options = {}) { validationIssues, operations, retirements, + // The roots the retirements above may fall under (the target root plus + // any second root the adapter declared), for the apply to check each + // candidate against the root it belongs to. + managedRoots: resolveAdapterManagedRoots(adapter, planningInput).map(root => path.resolve(root)), }; } diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index 42fe32397..a19e1c031 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -1,6 +1,7 @@ 'use strict'; const fs = require('node:fs'); +const crypto = require('node:crypto'); const os = require('node:os'); const path = require('node:path'); @@ -296,9 +297,8 @@ function refuseLinkedDestination(destinationPath, targetRoot, { migrate, dryRun // directory there is not what EGC wrote and is left alone. Directories the // removal empties are dropped too, up to the target root. function retirePlannedFiles(plan) { - const root = plan.targetRoot ? path.resolve(plan.targetRoot) : null; const retired = []; - for (const retirement of retirableFiles(plan)) { + for (const { retirement, root } of retirableEntries(plan)) { fs.unlinkSync(retirement.destinationPath); retired.push(retirement); removeEmptyParents(path.dirname(retirement.destinationPath), root); @@ -306,20 +306,46 @@ function retirePlannedFiles(plan) { return retired; } -// The retirements of a plan that would actually be removed right now: the -// same test the apply runs, so a dry run lists exactly what the apply does. -function retirableFiles(plan) { - const root = plan.targetRoot ? path.resolve(plan.targetRoot) : null; +// The roots a plan writes under: the target root, plus any second root the +// adapter declared (Amp's plugin config directory). A destination is checked +// against the root it belongs to, so the linked-ancestor walk and the +// empty-parent climb cover that root and never leave it. +function managedRootsOf(plan) { + const declared = Array.isArray(plan.managedRoots) ? plan.managedRoots : []; + const roots = [plan.targetRoot, ...declared] + .filter(root => typeof root === 'string' && root.length > 0) + .map(root => path.resolve(root)); + return [...new Set(roots)]; +} + +// The managed root a destination falls under; a destination outside every +// declared root is walked against the target root, as before. +function managedRootFor(plan, destinationPath) { + const resolved = path.resolve(destinationPath); + const root = managedRootsOf(plan).find(candidate => resolved === candidate || resolved.startsWith(candidate + path.sep)); + return root || plan.targetRoot; +} + +// The retirements of a plan that would actually be removed right now, each +// with the root it belongs to: the same test the apply runs, so a dry run +// lists exactly what the apply does. +function retirableEntries(plan) { + const roots = managedRootsOf(plan); const result = []; for (const retirement of Array.isArray(plan.retirements) ? plan.retirements : []) { const filePath = path.resolve(retirement.destinationPath); - if (!root || !filePath.startsWith(root + path.sep)) continue; - if (!isRetirableFile(filePath, root, retirement.sourcePath)) continue; - result.push({ ...retirement, destinationPath: filePath }); + const root = roots.find(candidate => filePath.startsWith(candidate + path.sep)); + if (!root) continue; + if (!isRetirableFile(filePath, root, retirement.sourcePath, plan)) continue; + result.push({ retirement: { ...retirement, destinationPath: filePath }, root }); } return result; } +function retirableFiles(plan) { + return retirableEntries(plan).map(entry => entry.retirement); +} + function isSymbolicLink(filePath) { try { return fs.lstatSync(filePath).isSymbolicLink(); @@ -328,13 +354,40 @@ function isSymbolicLink(filePath) { } } +const plannedContentHashesByPlan = new WeakMap(); + +// The content of every file the plan copies, hashed once per plan and only +// when a candidate needs it: a candidate whose recorded source is gone (the +// file was renamed or moved in the package) is still EGC's when its bytes +// match a file the plan writes today. +function plannedContentHashes(plan) { + if (plannedContentHashesByPlan.has(plan)) return plannedContentHashesByPlan.get(plan); + const hashes = new Set(); + for (const operation of Array.isArray(plan.operations) ? plan.operations : []) { + if (operation.kind !== 'copy-file' || typeof operation.sourcePath !== 'string') continue; + try { + hashes.add(sha256(fs.readFileSync(operation.sourcePath))); + } catch { + // An unreadable source vouches for nothing. + } + } + plannedContentHashesByPlan.set(plan, hashes); + return hashes; +} + +function sha256(buffer) { + return crypto.createHash('sha256').update(buffer).digest('hex'); +} + // Whether the file at filePath is the one EGC wrote and may go: a regular // file (never a link), reached through no link between the root and it (a -// linked ancestor would point the unlink outside the root), and, when the -// source EGC copied is still known, byte-identical to it. A file the person -// replaced since is theirs, and a file whose source is gone cannot be told -// apart from one, so both stay. -function isRetirableFile(filePath, root, sourcePath) { +// linked ancestor would point the unlink outside the root), and byte-identical +// to what EGC copied: the recorded source when it is still there, or, when +// that source is gone because the file was renamed or moved in the package, a +// file the plan copies today. A file the person replaced since is theirs, and +// a file whose source is gone and matches nothing the plan writes cannot be +// told apart from one, so both stay. +function isRetirableFile(filePath, root, sourcePath, plan = {}) { let stat; try { stat = fs.lstatSync(filePath); @@ -346,13 +399,22 @@ function isRetirableFile(filePath, root, sourcePath) { if (isSymbolicLink(dir)) return false; } if (!sourcePath) return false; + let content; try { - const source = fs.statSync(sourcePath); - if (!source.isFile()) return false; - return fs.readFileSync(sourcePath).equals(fs.readFileSync(filePath)); + content = fs.readFileSync(filePath); } catch { return false; } + try { + const source = fs.statSync(sourcePath); + if (!source.isFile()) return false; + return fs.readFileSync(sourcePath).equals(content); + } catch (error) { + // Only a source that is gone falls through to the content match; any + // other failure to read it keeps the file. + if (error.code !== 'ENOENT') return false; + } + return plannedContentHashes(plan).has(sha256(content)); } function removeEmptyParents(dirPath, root) { @@ -401,9 +463,14 @@ function checkedDestinations(plan) { // apply walk the same paths, so the list is what the apply will do. function findLegacyLinks(plan, { strict = false } = {}) { const migrate = []; + const targetRoot = plan.targetRoot ? path.resolve(plan.targetRoot) : null; for (const destinationPath of checkedDestinations(plan)) { + // The legacy layout (#1400) only ever lived under the target root: a + // link under a declared second root is refused outright. + const root = managedRootFor(plan, destinationPath); + const options = root === targetRoot ? { migrate, dryRun: true } : { dryRun: true }; try { - refuseLinkedDestination(destinationPath, plan.targetRoot, { migrate, dryRun: true }); + refuseLinkedDestination(destinationPath, root, options); } catch (error) { if (strict) throw error; } @@ -463,7 +530,7 @@ function applyInstallPlan(plan, { onWarning, homeDir, dbPath } = {}) { if (resolvedClaudeHooksPlan) refuseLinkedDestination(resolvedClaudeHooksPlan.hooksDestinationPath, plan.targetRoot); for (const operation of plan.operations) { - refuseLinkedDestination(operation.destinationPath, plan.targetRoot); + refuseLinkedDestination(operation.destinationPath, managedRootFor(plan, operation.destinationPath)); fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true }); @@ -528,6 +595,7 @@ function applyInstallPlan(plan, { onWarning, homeDir, dbPath } = {}) { module.exports = { applyInstallPlan, + managedRootFor, retirableFiles, retirePlannedFiles, checkedDestinations, diff --git a/tests/lib/install-apply-links.test.js b/tests/lib/install-apply-links.test.js index a2e0938af..0920db3ac 100644 --- a/tests/lib/install-apply-links.test.js +++ b/tests/lib/install-apply-links.test.js @@ -9,7 +9,7 @@ const assert = require('assert'); const fs = require('fs'); const os = require('os'); const path = require('path'); -const { checkedDestinations, findLegacyLinks, refuseLinkedDestination, removeLegacyLinks, retirePlannedFiles, writeGuardianCliMarker, writeManagedText } = require('../../scripts/lib/install/apply'); +const { checkedDestinations, findLegacyLinks, managedRootFor, refuseLinkedDestination, removeLegacyLinks, retirePlannedFiles, writeGuardianCliMarker, writeManagedText } = require('../../scripts/lib/install/apply'); const { createInstallState, writeInstallState } = require('../../scripts/lib/install-state'); @@ -185,6 +185,31 @@ function runTests() { assert.deepStrictEqual(findLegacyLinks(plan).map(entry => entry.linkPath), [path.join(home, 'egc')]); })) passed++; else failed++; + if (test('a destination under a declared second root is walked against that root: a linked ancestor there is refused before any write (#1412)', () => { + const target = path.join(dir, 'two-roots-target'); + const second = path.join(dir, 'two-roots-config'); + fs.mkdirSync(path.join(target, 'skills'), { recursive: true }); + fs.mkdirSync(path.join(second, 'plugins'), { recursive: true }); + // The plugin directory under the second root replaced by a link elsewhere. + fs.symlinkSync(outside, path.join(second, 'plugins', 'egc'), 'dir'); + const linked = path.join(second, 'plugins', 'egc', 'plugin.js'); + const plain = path.join(target, 'skills', 'SKILL.md'); + const plan = { + targetRoot: target, + managedRoots: [target, second], + installStatePath: path.join(target, 'egc', 'install-state.json'), + operations: [{ destinationPath: plain }, { destinationPath: linked }], + }; + assert.strictEqual(managedRootFor(plan, linked), second, 'the plugin write belongs to the second root'); + assert.strictEqual(managedRootFor(plan, plain), target, 'the skill write belongs to the target root'); + assert.strictEqual(managedRootFor(plan, path.join(outside, 'x.md')), target, 'a path outside every root falls back to the target root'); + assert.throws(() => refuseLinkedDestination(linked, managedRootFor(plan, linked)), /symbolic link/, 'the walk against the second root finds the linked ancestor'); + assert.doesNotThrow(() => refuseLinkedDestination(linked, target), 'the same walk against the target root alone would have missed it'); + assert.throws(() => findLegacyLinks(plan, { strict: true }), /symbolic link/, 'the preflight refuses the plan before anything is written'); + assert.deepStrictEqual(findLegacyLinks(plan), [], 'and lists nothing to migrate: the legacy layout never lived under a second root'); + assert.ok(!fs.existsSync(path.join(outside, 'plugin.js')), 'nothing was written through the link'); + })) passed++; else failed++; + if (test('a root that is itself a link is allowed', () => { const viaLink = path.join(dir, 'root-link'); assert.doesNotThrow(() => refuseLinkedDestination(path.join(viaLink, 'rules', 'plain.md'), viaLink)); @@ -283,6 +308,73 @@ function runTests() { assert.ok(fs.existsSync(root2), 'the root itself stays'); })) passed++; else failed++; + if (test('retirePlannedFiles honors a second managed root the plan declares, and still refuses anything outside every root (#1412)', () => { + const root4 = path.join(dir, 'retire-two-roots'); + const second = path.join(dir, 'retire-two-roots-config'); + const source4 = path.join(dir, 'retire-two-roots-source'); + fs.mkdirSync(path.join(root4, 'skills'), { recursive: true }); + fs.mkdirSync(path.join(second, 'plugins', 'egc'), { recursive: true }); + fs.mkdirSync(source4, { recursive: true }); + fs.writeFileSync(path.join(source4, 'plugin.js'), 'egc plugin.js'); + fs.writeFileSync(path.join(source4, 'SKILL.md'), 'egc SKILL.md'); + fs.writeFileSync(path.join(root4, 'skills', 'SKILL.md'), 'egc SKILL.md'); + fs.writeFileSync(path.join(second, 'plugins', 'egc', 'plugin.js'), 'egc plugin.js'); + fs.writeFileSync(path.join(second, 'plugins', 'keep.js'), 'keep'); + fs.writeFileSync(path.join(outside, 'elsewhere.js'), 'egc plugin.js'); + const plan = { + targetRoot: root4, + managedRoots: [root4, second], + retirements: [ + { destinationPath: path.join(root4, 'skills', 'SKILL.md'), sourcePath: path.join(source4, 'SKILL.md') }, + { destinationPath: path.join(second, 'plugins', 'egc', 'plugin.js'), sourcePath: path.join(source4, 'plugin.js') }, + { destinationPath: path.join(outside, 'elsewhere.js'), sourcePath: path.join(source4, 'plugin.js') }, + ], + }; + const retired = retirePlannedFiles(plan); + assert.deepStrictEqual( + retired.map(item => item.destinationPath).sort(), + [path.join(root4, 'skills', 'SKILL.md'), path.join(second, 'plugins', 'egc', 'plugin.js')].sort(), + 'files under either declared root go; the one outside every root does not' + ); + assert.ok(!fs.existsSync(path.join(second, 'plugins', 'egc')), 'the emptied directory under the second root is gone'); + assert.ok(fs.existsSync(path.join(second, 'plugins', 'keep.js')), 'a file that stays keeps its directory under the second root'); + assert.ok(fs.existsSync(second), 'the second root itself stays'); + assert.ok(fs.existsSync(path.join(outside, 'elsewhere.js')), 'a path outside every root is never touched'); + assert.ok(retired.every(item => !('root' in item)), 'the reported entries carry no bookkeeping field'); + })) passed++; else failed++; + + if (test('retirePlannedFiles retires a file whose recorded source is gone only when its bytes match a file the plan copies today (#1412)', () => { + const root5 = path.join(dir, 'retire-renamed'); + const source5 = path.join(dir, 'retire-renamed-source'); + fs.mkdirSync(path.join(root5, 'commands'), { recursive: true }); + fs.mkdirSync(source5, { recursive: true }); + // The package renamed old.md to new.md: the old source is gone, the new + // one carries the same bytes and is what the plan copies today. + fs.writeFileSync(path.join(source5, 'new.md'), 'egc command'); + fs.writeFileSync(path.join(root5, 'commands', 'old.md'), 'egc command'); + // A dropped file whose bytes match nothing the plan writes, and one the + // person edited after the rename: both stay. + fs.writeFileSync(path.join(root5, 'commands', 'dropped.md'), 'egc dropped'); + fs.writeFileSync(path.join(root5, 'commands', 'edited.md'), 'mine now'); + const plan = { + targetRoot: root5, + operations: [ + { kind: 'copy-file', sourcePath: path.join(source5, 'new.md'), destinationPath: path.join(root5, 'commands', 'new.md') }, + { kind: 'merge-json', sourcePath: path.join(source5, 'missing.json'), destinationPath: path.join(root5, 'x.json') }, + ], + retirements: [ + { destinationPath: path.join(root5, 'commands', 'old.md'), sourcePath: path.join(source5, 'old.md') }, + { destinationPath: path.join(root5, 'commands', 'dropped.md'), sourcePath: path.join(source5, 'dropped.md') }, + { destinationPath: path.join(root5, 'commands', 'edited.md'), sourcePath: path.join(source5, 'edited.md') }, + ], + }; + const retired = retirePlannedFiles(plan); + assert.deepStrictEqual(retired.map(item => item.destinationPath), [path.join(root5, 'commands', 'old.md')], 'only the renamed file goes'); + assert.ok(fs.existsSync(path.join(root5, 'commands', 'dropped.md')), 'a dropped file matching nothing the plan writes stays'); + assert.ok(fs.existsSync(path.join(root5, 'commands', 'edited.md')), 'a file the person edited stays'); + assert.deepStrictEqual(retirePlannedFiles({ targetRoot: root5, retirements: plan.retirements.slice(1) }), [], 'without plan operations nothing vouches for a missing source'); + })) passed++; else failed++; + if (test('retirePlannedFiles stops climbing when a parent cannot be read after the removal', () => { const root3 = path.join(dir, 'retire-sealed'); const sealed = path.join(root3, 'sealed'); diff --git a/tests/lib/install-targets.test.js b/tests/lib/install-targets.test.js index aec819105..416f7ea50 100644 --- a/tests/lib/install-targets.test.js +++ b/tests/lib/install-targets.test.js @@ -12,6 +12,10 @@ const { planInstallTargetScaffold, } = require('../../scripts/lib/install-targets/registry'); +const { + createInstallTargetAdapter, +} = require('../../scripts/lib/install-targets/helpers'); + function normalizedRelativePath(value) { return String(value || '').replace(/\\/g, '/'); } @@ -3684,6 +3688,443 @@ function runTests() { assert.ok(targets.includes('warp'), 'Should include warp target'); })) passed++; else failed++; + if (test('generic planRetirements (the default every target gets with no adapter override) retires a file that left the plan, keeps a still-planned file and a destination outside the root, and plans nothing without a previous install (#1412)', () => { + const fs = require('fs'); + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-repo-')); + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-home-')); + try { + fs.mkdirSync(path.join(repoRoot, 'commands'), { recursive: true }); + fs.writeFileSync(path.join(repoRoot, 'commands', 'kept.md'), 'kept'); + fs.writeFileSync(path.join(repoRoot, 'commands', 'old-name.md'), 'renamed away, source unchanged'); + + // A plain adapter with no planOperations/planRetirements of its own, + // the shape most real targets are (windsurf, amp, copilot, ...): it + // gets the shared defaults for both. + const adapter = createInstallTargetAdapter({ + id: 'egc-generic-retire-target', + target: 'egc-generic-retire-target', + kind: 'home', + rootSegments: ['egc-generic-retire-target'], + installStatePathSegments: ['egc', 'install-state.json'], + }); + + const targetRoot = adapter.resolveRoot({ repoRoot, homeDir }); + const installStatePath = adapter.getInstallStatePath({ repoRoot, homeDir }); + const planningInput = { repoRoot, homeDir, modules: [{ id: 'x', paths: ['commands/kept.md'] }] }; + + assert.deepStrictEqual(adapter.planRetirements(planningInput), [], 'no previous install, nothing to retire'); + + const { createInstallState, writeInstallState } = require('../../scripts/lib/install-state'); + const previous = [ + // Still named by a module path in planningInput below: stays. + ['commands/kept.md', path.join(targetRoot, 'commands', 'kept.md')], + // Renamed away: the source is still in the repo, just not + // referenced by any module path anymore. This is the common case + // (a command or prompt renamed in the package): retired. + ['commands/old-name.md', path.join(targetRoot, 'commands', 'old-name.md')], + // Dropped from the package entirely, source gone too. Still a + // *candidate* here: apply.js's identity check (#1411) is what + // actually refuses to delete it, since it cannot verify the file + // is unchanged. + ['commands/removed.md', path.join(targetRoot, 'commands', 'removed.md')], + // Outside this target's root: never a candidate, whatever the + // state says. + ['commands/escaped.md', path.join(homeDir, 'elsewhere', 'escaped.md')], + ]; + const state = createInstallState({ + adapter: { id: adapter.id }, + targetRoot, + installStatePath, + request: { profile: 'full', modules: [], legacyLanguages: [], legacyMode: false }, + resolution: { selectedModules: [], skippedModules: [] }, + operations: previous.map(([sourceRelativePath, destinationPath]) => ({ + kind: 'copy-file', + moduleId: 'x', + sourceRelativePath, + destinationPath, + strategy: 'preserve-relative-path', + ownership: 'managed', + scaffoldOnly: false, + })), + source: { repoVersion: require('../../package.json').version, repoCommit: 'abc123', manifestVersion: 1 }, + }); + writeInstallState(installStatePath, state); + + const expectedDestinations = [ + path.join(targetRoot, 'commands', 'old-name.md'), + path.join(targetRoot, 'commands', 'removed.md'), + ].sort(); + + const retirements = adapter.planRetirements(planningInput); + assert.deepStrictEqual( + retirements.map(entry => entry.destinationPath).sort(), + expectedDestinations, + 'the renamed-away and dropped files are offered up; the still-planned file and the one outside the root are not' + ); + for (const entry of retirements) { + assert.strictEqual( + entry.sourcePath, + path.join(repoRoot, ...entry.sourceRelativePath.split('/')), + 'each candidate names the file EGC copied, for apply.js to compare' + ); + } + + // registry.js passes the already-planned operations through to avoid + // planning a second time; the result must not depend on that. + const withPrecomputedOperations = adapter.planRetirements({ + ...planningInput, + operations: adapter.planOperations(planningInput), + }); + assert.deepStrictEqual( + withPrecomputedOperations.map(entry => entry.destinationPath).sort(), + expectedDestinations, + 'passing already-planned operations through gives the same result as planning them again' + ); + } finally { + fs.rmSync(repoRoot, { recursive: true, force: true }); + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('generic planRetirements treats a directory-shaped scaffold operation as covering every file under it (#1412)', () => { + const fs = require('fs'); + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-dir-repo-')); + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-dir-home-')); + try { + fs.mkdirSync(path.join(repoRoot, 'bundle', 'nested'), { recursive: true }); + fs.writeFileSync(path.join(repoRoot, 'bundle', 'a.md'), 'a'); + fs.writeFileSync(path.join(repoRoot, 'bundle', 'nested', 'b.md'), 'b'); + + const adapter = createInstallTargetAdapter({ + id: 'egc-generic-retire-dir-target', + target: 'egc-generic-retire-dir-target', + kind: 'home', + rootSegments: ['egc-generic-retire-dir-target'], + installStatePathSegments: ['egc', 'install-state.json'], + }); + + const targetRoot = adapter.resolveRoot({ repoRoot, homeDir }); + const installStatePath = adapter.getInstallStatePath({ repoRoot, homeDir }); + // A single module path naming the whole directory, the same shape a + // recursive scaffold copy plans: one 'copy-path' operation for + // 'bundle', not one per file underneath. + const planningInput = { repoRoot, homeDir, modules: [{ id: 'x', paths: ['bundle'] }] }; + + const { createInstallState, writeInstallState } = require('../../scripts/lib/install-state'); + // What an earlier install actually recorded: one copy-file entry per + // file the directory copy produced, as install-executor.js's + // materializeScaffoldOperation expands it. + const state = createInstallState({ + adapter: { id: adapter.id }, + targetRoot, + installStatePath, + request: { profile: 'full', modules: [], legacyLanguages: [], legacyMode: false }, + resolution: { selectedModules: [], skippedModules: [] }, + operations: [ + { kind: 'copy-file', moduleId: 'x', sourceRelativePath: 'bundle/a.md', destinationPath: path.join(targetRoot, 'bundle', 'a.md'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + { kind: 'copy-file', moduleId: 'x', sourceRelativePath: 'bundle/nested/b.md', destinationPath: path.join(targetRoot, 'bundle', 'nested', 'b.md'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + ], + source: { repoVersion: require('../../package.json').version, repoCommit: 'abc123', manifestVersion: 1 }, + }); + writeInstallState(installStatePath, state); + + assert.deepStrictEqual( + adapter.planRetirements(planningInput), + [], + 'both files still live inside the planned directory, so neither is offered up' + ); + } finally { + fs.rmSync(repoRoot, { recursive: true, force: true }); + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('generic planRetirements honors a declared second managed root, so an adapter that writes outside its own root can still retire its files (#1412)', () => { + const fs = require('fs'); + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-roots-repo-')); + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-roots-home-')); + try { + fs.mkdirSync(path.join(repoRoot, 'scripts'), { recursive: true }); + fs.writeFileSync(path.join(repoRoot, 'scripts', 'plugin.ts'), 'plugin'); + + // Mirrors amp-home's real shape: skills under the adapter root, plugin + // scripts under a second XDG-style root (resolveAmpConfigRoot), both + // declared via resolveManagedRoots. + const rootsTarget = path.join(homeDir, '.config', 'egc-generic-retire-roots-target'); + const adapter = createInstallTargetAdapter({ + id: 'egc-generic-retire-roots-target', + target: 'egc-generic-retire-roots-target', + kind: 'home', + rootSegments: ['egc-generic-retire-roots-target'], + installStatePathSegments: ['egc', 'install-state.json'], + resolveManagedRoots(input) { + return [path.join(input.homeDir || homeDir, 'egc-generic-retire-roots-target'), path.join(input.homeDir || homeDir, '.config', 'egc-generic-retire-roots-target')]; + }, + }); + const installStatePath = adapter.getInstallStatePath({ repoRoot, homeDir }); + const planningInput = { repoRoot, homeDir, modules: [{ id: 'x', paths: ['scripts/plugin.ts'] }] }; + + const { createInstallState, writeInstallState } = require('../../scripts/lib/install-state'); + const state = createInstallState({ + adapter: { id: adapter.id }, + targetRoot: rootsTarget, + installStatePath, + request: { profile: 'full', modules: [], legacyLanguages: [], legacyMode: false }, + resolution: { selectedModules: [], skippedModules: [] }, + operations: [ + { kind: 'copy-file', moduleId: 'x', sourceRelativePath: 'scripts/plugin.ts', destinationPath: path.join(rootsTarget, 'plugins', 'plugin.ts'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + // A destination under a root this adapter does NOT trust: stays + // untouched even though the module dropped it. + { kind: 'copy-file', moduleId: 'x', sourceRelativePath: 'scripts/plugin.ts', destinationPath: path.join(homeDir, 'untrusted', 'plugin.ts'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + ], + source: { repoVersion: require('../../package.json').version, repoCommit: 'abc123', manifestVersion: 1 }, + }); + writeInstallState(installStatePath, state); + + assert.deepStrictEqual( + adapter.planRetirements(planningInput).map(entry => entry.destinationPath), + [path.join(rootsTarget, 'plugins', 'plugin.ts')], + 'the file under the declared second root is offered up; the one under an untrusted root is not' + ); + } finally { + fs.rmSync(repoRoot, { recursive: true, force: true }); + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('generic planRetirements does not retire a file a sibling adapter sharing the same root still owns (#1412)', () => { + const fs = require('fs'); + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-sibling-repo-')); + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-sibling-home-')); + try { + fs.mkdirSync(path.join(repoRoot, 'skills', 'shared'), { recursive: true }); + fs.writeFileSync(path.join(repoRoot, 'skills', 'shared', 'SKILL.md'), 'shared skill'); + fs.mkdirSync(path.join(repoRoot, 'skills', 'kept'), { recursive: true }); + fs.writeFileSync(path.join(repoRoot, 'skills', 'kept', 'SKILL.md'), 'kept skill'); + + const adapterA = createInstallTargetAdapter({ + id: 'egc-generic-retire-sibling-a', + target: 'egc-generic-retire-sibling-a', + kind: 'home', + rootSegments: ['egc-generic-retire-shared-root'], + installStatePathSegments: ['egc', 'a-install-state.json'], + }); + const adapterB = createInstallTargetAdapter({ + id: 'egc-generic-retire-sibling-b', + target: 'egc-generic-retire-sibling-b', + kind: 'home', + rootSegments: ['egc-generic-retire-shared-root'], + installStatePathSegments: ['egc', 'b-install-state.json'], + }); + + const targetRoot = adapterA.resolveRoot({ repoRoot, homeDir }); + const siblingBStatePath = adapterB.getInstallStatePath({ repoRoot, homeDir }); + const planningInputA = { repoRoot, homeDir, modules: [{ id: 'x', paths: ['skills/kept/SKILL.md'] }] }; + + const { createInstallState, writeInstallState } = require('../../scripts/lib/install-state'); + // A's own previous state: it installed the shared skill, but its plan + // dropped it. B's sibling state still owns the same destination. + const stateA = createInstallState({ + adapter: { id: adapterA.id }, + targetRoot, + installStatePath: adapterA.getInstallStatePath({ repoRoot, homeDir }), + request: { profile: 'full', modules: [], legacyLanguages: [], legacyMode: false }, + resolution: { selectedModules: [], skippedModules: [] }, + operations: [ + { kind: 'copy-file', moduleId: 'x', sourceRelativePath: 'skills/shared/SKILL.md', destinationPath: path.join(targetRoot, 'skills', 'shared', 'SKILL.md'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + ], + source: { repoVersion: require('../../package.json').version, repoCommit: 'abc123', manifestVersion: 1 }, + }); + const stateB = { + ...createInstallState({ + adapter: { id: adapterB.id }, + targetRoot, + installStatePath: siblingBStatePath, + request: { profile: 'full', modules: [], legacyLanguages: [], legacyMode: false }, + resolution: { selectedModules: [], skippedModules: [] }, + operations: [ + { kind: 'copy-file', moduleId: 'y', sourceRelativePath: 'skills/shared/SKILL.md', destinationPath: path.join(targetRoot, 'skills', 'shared', 'SKILL.md'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + ], + source: { repoVersion: require('../../package.json').version, repoCommit: 'abc123', manifestVersion: 1 }, + }), + }; + writeInstallState(adapterA.getInstallStatePath({ repoRoot, homeDir }), stateA); + writeInstallState(siblingBStatePath, stateB); + + assert.deepStrictEqual( + adapterA.planRetirements({ + ...planningInputA, + operations: adapterA.planOperations(planningInputA), + siblingStatePaths: [siblingBStatePath], + }), + [], + 'B still owns the shared skill, so A must not retire it' + ); + + // Once B no longer records the destination, A's candidate is offered. + writeInstallState(siblingBStatePath, { + ...stateB, + operations: stateB.operations.filter(operation => operation.kind !== 'copy-file'), + }); + const retirements = adapterA.planRetirements({ + ...planningInputA, + operations: adapterA.planOperations(planningInputA), + siblingStatePaths: [siblingBStatePath], + }); + assert.deepStrictEqual( + retirements.map(entry => entry.destinationPath), + [path.join(targetRoot, 'skills', 'shared', 'SKILL.md')], + 'with no sibling ownership left, the dropped skill is offered up' + ); + + // A sibling that never installed anything has no state file: absent, + // and the candidate is still offered up. + const absentSibling = path.join(homeDir, 'nowhere', 'install-state.json'); + assert.deepStrictEqual( + adapterA.planRetirements({ ...planningInputA, operations: adapterA.planOperations(planningInputA), siblingStatePaths: [absentSibling] }).map(entry => entry.destinationPath), + [path.join(targetRoot, 'skills', 'shared', 'SKILL.md')], + 'a missing sibling state owns nothing' + ); + + // A sibling state that exists but cannot be parsed may still own the + // destination: nothing is offered up until it can be read again. + fs.writeFileSync(siblingBStatePath, '{ not json'); + assert.deepStrictEqual( + adapterA.planRetirements({ ...planningInputA, operations: adapterA.planOperations(planningInputA), siblingStatePaths: [siblingBStatePath] }), + [], + 'a malformed sibling state suppresses retirement' + ); + + // A dangling link at the sibling's state path is not an absence: the + // link is there, the state behind it is not, so nothing is offered up. + if (process.platform !== 'win32') { + const dangling = path.join(homeDir, 'dangling-install-state.json'); + fs.symlinkSync(path.join(homeDir, 'nowhere.json'), dangling); + assert.deepStrictEqual( + adapterA.planRetirements({ ...planningInputA, operations: adapterA.planOperations(planningInputA), siblingStatePaths: [dangling] }), + [], + 'a dangling link at a sibling state path suppresses retirement' + ); + } + + // Same for a state that is valid but unreadable (no permission bits), + // and for a state whose parent cannot be inspected; root reads + // everything, so the checks only mean something elsewhere. + if (process.platform !== 'win32' && typeof process.getuid === 'function' && process.getuid() !== 0) { + const sealed = path.join(homeDir, 'sealed'); + const sealedState = path.join(sealed, 'install-state.json'); + fs.mkdirSync(sealed); + writeInstallState(sealedState, stateB); + fs.chmodSync(sealed, 0o000); + try { + assert.deepStrictEqual( + adapterA.planRetirements({ ...planningInputA, operations: adapterA.planOperations(planningInputA), siblingStatePaths: [sealedState] }), + [], + 'a sibling state behind an inaccessible parent suppresses retirement' + ); + } finally { + fs.chmodSync(sealed, 0o700); + } + + writeInstallState(siblingBStatePath, stateB); + fs.chmodSync(siblingBStatePath, 0o000); + try { + assert.deepStrictEqual( + adapterA.planRetirements({ ...planningInputA, operations: adapterA.planOperations(planningInputA), siblingStatePaths: [siblingBStatePath] }), + [], + 'an unreadable sibling state suppresses retirement' + ); + } finally { + fs.chmodSync(siblingBStatePath, 0o600); + } + } + } finally { + fs.rmSync(repoRoot, { recursive: true, force: true }); + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('generic planRetirements leaves the files of a module that was not selected this run alone, and only offers up what left a module the plan still includes (#1412)', () => { + const fs = require('fs'); + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-subset-repo-')); + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-generic-retire-subset-home-')); + try { + fs.mkdirSync(path.join(repoRoot, 'commands'), { recursive: true }); + fs.mkdirSync(path.join(repoRoot, 'skills', 'tool'), { recursive: true }); + fs.writeFileSync(path.join(repoRoot, 'commands', 'kept.md'), 'kept'); + fs.writeFileSync(path.join(repoRoot, 'commands', 'old-name.md'), 'renamed away, source unchanged'); + fs.writeFileSync(path.join(repoRoot, 'skills', 'tool', 'SKILL.md'), 'a skill of another module'); + + const adapter = createInstallTargetAdapter({ + id: 'egc-generic-retire-subset-target', + target: 'egc-generic-retire-subset-target', + kind: 'home', + rootSegments: ['egc-generic-retire-subset-target'], + installStatePathSegments: ['egc', 'install-state.json'], + }); + const targetRoot = adapter.resolveRoot({ repoRoot, homeDir }); + const installStatePath = adapter.getInstallStatePath({ repoRoot, homeDir }); + + const { createInstallState, writeInstallState } = require('../../scripts/lib/install-state'); + // A full install recorded two modules; module y is not selected below. + const state = createInstallState({ + adapter: { id: adapter.id }, + targetRoot, + installStatePath, + request: { profile: 'full', modules: [], legacyLanguages: [], legacyMode: false }, + resolution: { selectedModules: [], skippedModules: [] }, + operations: [ + { kind: 'copy-file', moduleId: 'x', sourceRelativePath: 'commands/kept.md', destinationPath: path.join(targetRoot, 'commands', 'kept.md'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + { kind: 'copy-file', moduleId: 'x', sourceRelativePath: 'commands/old-name.md', destinationPath: path.join(targetRoot, 'commands', 'old-name.md'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + { kind: 'copy-file', moduleId: 'y', sourceRelativePath: 'skills/tool/SKILL.md', destinationPath: path.join(targetRoot, 'skills', 'tool', 'SKILL.md'), strategy: 'preserve-relative-path', ownership: 'managed', scaffoldOnly: false }, + ], + source: { repoVersion: require('../../package.json').version, repoCommit: 'abc123', manifestVersion: 1 }, + }); + writeInstallState(installStatePath, state); + + // A targeted install of module x only: y's skill is not in the plan, but + // it was not dropped from the package either, so it stays untouched. + const subset = adapter.planRetirements({ repoRoot, homeDir, modules: [{ id: 'x', paths: ['commands/kept.md'] }] }); + assert.deepStrictEqual( + subset.map(entry => entry.destinationPath), + [path.join(targetRoot, 'commands', 'old-name.md')], + 'only the file that left module x is offered up; module y was simply not selected' + ); + + // With both modules selected the answer is the same: y still covers its skill. + const both = adapter.planRetirements({ repoRoot, homeDir, modules: [{ id: 'x', paths: ['commands/kept.md'] }, { id: 'y', paths: ['skills/tool/SKILL.md'] }] }); + assert.deepStrictEqual(both.map(entry => entry.destinationPath), [path.join(targetRoot, 'commands', 'old-name.md')]); + + // No modules selected at all: nothing is offered up. + assert.deepStrictEqual(adapter.planRetirements({ repoRoot, homeDir, modules: [] }), []); + + // No repoRoot: identities cannot be compared, so nothing is offered up. + assert.deepStrictEqual(adapter.planRetirements({ homeDir, modules: [{ id: 'x', paths: ['commands/kept.md'] }] }), []); + } finally { + fs.rmSync(repoRoot, { recursive: true, force: true }); + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('planInstallTargetScaffold carries the managed roots of the target into the plan, for the apply to check retirements against (#1412)', () => { + const fs = require('fs'); + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-managed-roots-repo-')); + const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'egc-managed-roots-home-')); + try { + const amp = planInstallTargetScaffold({ target: 'amp-home', repoRoot, homeDir, modules: [] }); + assert.deepStrictEqual( + amp.managedRoots.map(normalizedRelativePath).sort(), + [path.join(homeDir, '.amp'), path.join(homeDir, '.config', 'amp')].map(normalizedRelativePath).sort(), + 'amp-home declares its skills root and its plugin config root' + ); + const claude = planInstallTargetScaffold({ target: 'claude-home', repoRoot, homeDir, modules: [] }); + assert.deepStrictEqual(claude.managedRoots, [claude.targetRoot], 'a target with one root declares just that root'); + } finally { + fs.rmSync(repoRoot, { recursive: true, force: true }); + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); }