diff --git a/docs/pi-links-relocation.md b/docs/pi-links-relocation.md new file mode 100644 index 0000000..01cb2ca --- /dev/null +++ b/docs/pi-links-relocation.md @@ -0,0 +1,19 @@ +# Explicit repository relocation + +The link manager normally rejects symlinks outside the current checkout. After a repository move, explicitly authorize the previous root rather than weakening that default: + +```bash +node scripts/pi-links.mjs sync --dry-run --migrate-from /absolute/old/root +node scripts/pi-links.mjs sync --migrate-from /absolute/old/root +node scripts/pi-links.mjs check +``` + +Inspect the dry run before applying. Migration accepts only a separate absolute old root and only repairs manifest-declared destinations whose resolved link target exactly equals the old root plus the current source's repository-relative path. Dangling old targets are supported. Regular files, directories, unrelated targets, and mismatched relative paths remain conflicts. Replacement rechecks the target immediately before unlinking; this is not an atomic filesystem compare-and-swap. + +The option does not claim stale/unlisted links or old local package registrations. It is rejected for `prune`; normal pruning still requires its own preview and approval. Linked-worktree ownership is a separate issue, not solved by this relocation option. + +## Validated host migration + +Moved exposure from `/srv/data/code/jb-pi/harness-tools` to `/home/jb/code/harness-tools`: 38 global links repaired (35 skills and three instruction files), two project-local skill links created. Normal `check` passed with 40 links and zero local package entries. A bounded scan of harness configuration directories found no remaining symlinks with that old absolute target prefix. All 12 link-manager tests passed, including relocation, dry-run, collision preservation, invalid-input, and no-prune coverage. + +Browser routing was also updated in `skills/tools/browser-harness/SKILL.md`: fast-browser is the default for supported local tests, while browser-harness requires explicit opt-in and must never bypass failed actions or share a fast-browser-owned session. Run `/reload` to refresh skill exposure. diff --git a/scripts/pi-links.mjs b/scripts/pi-links.mjs index 251d0da..ff7d602 100755 --- a/scripts/pi-links.mjs +++ b/scripts/pi-links.mjs @@ -26,11 +26,12 @@ const COMMANDS = new Set(["check", "sync", "prune"]); function usage() { return ( - `Usage: node scripts/pi-links.mjs [--dry-run]\n\n` + + `Usage: node scripts/pi-links.mjs [--dry-run] [--migrate-from /old/repo]\n\n` + ` check Report missing, stale, or conflicting exposure without writing\n` + ` sync Create or repair manifest-declared links and local package entries\n` + ` prune Remove repository-owned links and local package entries not in the manifest\n` + - ` --dry-run Print sync/prune changes without applying them` + ` --dry-run Print sync/prune changes without applying them\n` + + ` --migrate-from Explicit old root for exact manifest-link repairs (check/sync only)` ); } @@ -384,7 +385,7 @@ async function readLocalPackages(settingsFile) { .filter((entry) => entry.resolved); } -async function inspectDesiredLink(link, repoRoot) { +async function inspectDesiredLink(link, repoRoot, migrateFrom) { const stat = await statOrUndefined(link.destination); if (!stat) return { type: "create-link", link }; if (!stat.isSymbolicLink()) { @@ -402,7 +403,10 @@ async function inspectDesiredLink(link, repoRoot) { ); if ((await canonical(resolvedTarget)) === (await canonical(link.source))) return undefined; - if (isInside(repoRoot, resolvedTarget)) + const expectedOldTarget = migrateFrom + ? path.join(migrateFrom, path.relative(repoRoot, link.source)) + : undefined; + if (isInside(repoRoot, resolvedTarget) || resolvedTarget === expectedOldTarget) return { type: "replace-link", link, previousTarget: resolvedTarget }; return { type: "conflict", @@ -451,6 +455,14 @@ async function discoverStaleLinks(state, desiredDestinations) { export async function buildPlan(options = {}) { const state = await resolveDesiredState(options); + let migrateFrom; + if (options.migrateFrom !== undefined) { + if (typeof options.migrateFrom !== "string" || !path.isAbsolute(options.migrateFrom)) + throw new Error("migrate-from must be an absolute old repository root"); + migrateFrom = path.resolve(options.migrateFrom); + if (isInside(migrateFrom, state.repoRoot) || isInside(state.repoRoot, migrateFrom)) + throw new Error("migrate-from must be separate from the current repository root"); + } const desiredDestinations = new Set( state.links.map((link) => link.destination), ); @@ -461,7 +473,7 @@ export async function buildPlan(options = {}) { const conflicts = []; for (const link of state.links) { - const result = await inspectDesiredLink(link, state.repoRoot); + const result = await inspectDesiredLink(link, state.repoRoot, migrateFrom); if (result?.type === "conflict") conflicts.push(result); else if (result) operations.push(result); } @@ -545,7 +557,13 @@ function selectOperations(command, operations) { async function createOrReplaceLink(operation) { const { link } = operation; await mkdir(path.dirname(link.destination), { recursive: true }); - if (operation.type === "replace-link") await unlink(link.destination); + if (operation.type === "replace-link") { + const current = await statOrUndefined(link.destination); + if (!current?.isSymbolicLink() || + path.resolve(path.dirname(link.destination), await readlink(link.destination)) !== operation.previousTarget) + throw new Error(`${link.destination}: destination changed after planning; refusing replacement`); + await unlink(link.destination); + } const target = link.relative ? path.relative(path.dirname(link.destination), link.source) : link.source; @@ -576,6 +594,9 @@ async function applyOperations(operations) { } export async function run(command, options = {}) { + if (!COMMANDS.has(command)) throw new Error("unknown link-manager command"); + if (command === "prune" && options.migrateFrom !== undefined) + throw new Error("migrate-from is only supported for check/sync; it never authorizes pruning"); const plan = await buildPlan(options); for (const conflict of plan.conflicts) console.error(`CONFLICT ${conflict.path}: ${conflict.reason}`); @@ -606,16 +627,23 @@ export async function run(command, options = {}) { return 0; } -async function main(argv = process.argv.slice(2)) { - const command = argv.find((arg) => !arg.startsWith("-")); - const unknown = argv.filter( - (arg) => arg.startsWith("-") && arg !== "--dry-run", - ); - if (!COMMANDS.has(command) || unknown.length > 0) { +export async function main(argv = process.argv.slice(2)) { + const [command, ...args] = argv; + const options = {}; + let invalid = !COMMANDS.has(command); + for (let index = 0; index < args.length; index++) { + const arg = args[index]; + if (arg === "--dry-run" && options.dryRun === undefined) options.dryRun = true; + else if (arg === "--migrate-from" && options.migrateFrom === undefined && + args[index + 1] && !args[index + 1].startsWith("--")) + options.migrateFrom = args[++index]; + else invalid = true; + } + if (invalid) { console.error(usage()); return 2; } - return run(command, { dryRun: argv.includes("--dry-run") }); + return run(command, options); } if ( diff --git a/scripts/pi-links.test.mjs b/scripts/pi-links.test.mjs index 9a56e49..37c186c 100644 --- a/scripts/pi-links.test.mjs +++ b/scripts/pi-links.test.mjs @@ -11,7 +11,7 @@ import os from "node:os"; import path from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; -import { buildPlan, run } from "./pi-links.mjs"; +import { buildPlan, main, run } from "./pi-links.mjs"; const repositoryRoot = path.resolve( path.dirname(fileURLToPath(import.meta.url)), @@ -295,6 +295,68 @@ test("normal exposure and package evidence retain the Slice 0 security boundary" ); }); +test("explicit relocation repairs only exact manifest links, including dangling targets", async () => { + const options = await fixture(); + const oldRoot = path.join(options.root, "old-repo"); + const initial = await buildPlan(options); + for (const link of initial.state.links) { + await mkdir(path.dirname(link.destination), { recursive: true }); + const oldTarget = path.join(oldRoot, path.relative(options.repoRoot, link.source)); + await symlink(link.relative ? path.relative(path.dirname(link.destination), oldTarget) : oldTarget, link.destination); + } + assert.equal((await buildPlan(options)).conflicts.length, initial.state.links.length); + const migration = { ...options, migrateFrom: oldRoot }; + const plan = await buildPlan(migration); + assert.equal(plan.conflicts.length, 0); + assert.equal(plan.operations.length, initial.state.links.length); + assert.ok(plan.operations.every(op => op.type === "replace-link")); + assert.equal(await run("sync", { ...migration, dryRun: true }), 0); + assert.equal((await buildPlan(options)).conflicts.length, initial.state.links.length); + assert.equal(await run("sync", migration), 0); + assert.equal(await run("check", options), 0); +}); + +test("relocation preserves wrong-path, unrelated, prefix-lookalike and regular-file collisions", async () => { + for (const kind of ["wrong-path", "unrelated", "prefix", "file"]) { + const options = await fixture(); + const oldRoot = path.join(options.root, "old-repo"); + const destination = path.join(options.homeDir, ".agents", "skills", "global-skill"); + await mkdir(path.dirname(destination), { recursive: true }); + const target = kind === "wrong-path" ? path.join(oldRoot, "skills/wrong") + : kind === "prefix" ? `${oldRoot}-other/skills/group/global-skill` + : path.join(options.root, "unrelated"); + if (kind === "file") await writeFile(destination, "keep"); + else await symlink(target, destination); + const migration = { ...options, migrateFrom: oldRoot }; + assert.equal((await buildPlan(migration)).conflicts.length, 1); + assert.equal(await run("sync", migration), 2); + assert.equal(kind === "file" ? await readFile(destination, "utf8") : await readlink(destination), kind === "file" ? "keep" : target); + } +}); + +test("relocation never claims stale old-root links or local packages for pruning", async () => { + const options = await fixture(); + const oldRoot = path.join(options.root, "old-repo"); + await run("sync", options); + const stale = path.join(options.homeDir, ".agents", "skills", "old-excluded"); + await symlink(path.join(oldRoot, "skills/old-excluded"), stale); + const settings = JSON.parse(await readFile(options.settingsFile, "utf8")); + settings.packages.push(path.join(oldRoot, "extensions/old/index.ts")); + await writeJson(options.settingsFile, settings); + const plan = await buildPlan({ ...options, migrateFrom: oldRoot }); + assert.deepEqual(plan.operations, []); + await assert.rejects(() => run("prune", { ...options, migrateFrom: oldRoot }), /never authorizes pruning/); + assert.equal(await readlink(stale), path.join(oldRoot, "skills/old-excluded")); +}); + +test("migration rejects invalid roots and malformed CLI options", async () => { + const options = await fixture(); + for (const migrateFrom of ["relative", "", options.repoRoot, options.root, path.join(options.repoRoot, "nested")]) + await assert.rejects(() => buildPlan({ ...options, migrateFrom }), /migrate-from/); + for (const args of [["sync", "--migrate-from"], ["sync", "unexpected"], ["sync", "--migrate-from", "/old", "--migrate-from", "/other"]]) + assert.equal(await main(args), 2); +}); + test("duplicate declared skill names fail before planning changes", async () => { const options = await fixture(); await writeSkill( diff --git a/skills/tools/browser-harness/SKILL.md b/skills/tools/browser-harness/SKILL.md index 676b71f..fece6f8 100644 --- a/skills/tools/browser-harness/SKILL.md +++ b/skills/tools/browser-harness/SKILL.md @@ -1,10 +1,20 @@ --- name: browser-harness -description: "Always use browser-harness for any web interaction: automation, scraping, testing, or site/app work." +description: "Use browser-harness only when explicitly requested for direct CDP browser work or capabilities outside fast-browser's supported scope. Use fast-browser by default for supported local browser tests and regression scenarios." --- # browser-harness +## Browser tool routing + +- Use **fast-browser** by default for supported local browser tests and regression scenarios: isolated sessions, bounded click/fill, reads/assertions, navigation/tabs/popups, and private failure evidence. In Pi, use its `fast_browser_*` tools and skill; use its scenario CLI for repeatable tests. +- Use **browser-harness only by explicit user opt-in** for direct CDP work or capabilities outside fast-browser's scope. If fast-browser is unavailable or lacks a required capability, explain the limitation and ask before switching. +- A failed, timed-out, or unverifiable fast-browser action is **not** permission to retry through browser-harness. Inspect state and resolve the outcome first; do not bypass origin restrictions, profile locks, or other safeguards. +- Never let both tools control the same browser session, endpoint, or profile. Close the owned session before changing tools and use a separately owned browser. Do not attach this CLI to a fast-browser-managed session. +- Other task-specific routing (such as Firecrawl for web research or Orca for its embedded browser) remains in effect. This skill does not claim all web work. + +The instructions below apply only after browser-harness has been explicitly selected. + Direct browser control via CDP. For task-specific edits, use `agent-workspace/agent_helpers.py`. For setup, install, or connection problems, read https://github.com/browser-use/browser-harness/blob/main/install.md. ## When Not to Use diff --git a/skills/tools/manage-pi-links/SKILL.md b/skills/tools/manage-pi-links/SKILL.md index ab472b5..df196ec 100644 --- a/skills/tools/manage-pi-links/SKILL.md +++ b/skills/tools/manage-pi-links/SKILL.md @@ -19,7 +19,7 @@ Treat [`config/pi-links.json`](../../../config/pi-links.json) as the source of t 1. Run commands from the repository root. 2. Read the manifest entry and the resource's `SKILL.md` or `index.ts` when diagnosing a mismatch. 3. Resolve manifest validation errors before applying changes. Require unique skill names and existing entry points. -4. Preserve collisions. The manager refuses regular files, directories, and links whose targets are outside this repository; report these for a human decision instead of replacing them manually. +4. Preserve collisions. The manager refuses regular files, directories, and links whose targets are outside this repository; report these for a human decision instead of replacing them manually. For an explicitly approved repository relocation, use `sync --dry-run --migrate-from /absolute/old/root`, inspect the repairs, then `sync --migrate-from /absolute/old/root`. This only repairs manifest-declared destinations whose old target exactly matches the same repository-relative source under that old root. It does not migrate stale/unlisted links or local packages, and cannot be used with `prune`. Finish with a normal `check` without the migration option. 5. Apply the authorized operation through `scripts/pi-links.mjs`. 6. Run `./scripts/pi-links.mjs check` again. Completion requires exit code zero. 7. Tell the user to run `/reload` when skill or extension exposure changed in the active Pi session.