diff --git a/AGENTS.md b/AGENTS.md index 4c2c9f3..70ae17b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,9 +83,12 @@ entries and opencode loads the plugin twice. An incoming `name@spec` entry therefore replaces every existing entry with the same package name, in place (`specName` in `src/merge.ts`), and stacked configs collapse on next install. `[name@spec, {options}]` tuples count as the same package as the plain string. -Non-`name@spec` entries (`{{cache}}` fetch dests, prompted dirs, git URLs with -credentials) stay plain append — a heuristic there would delete unrelated -entries. `remove` is untouched: it still deletes only exact matches. +Versioned `{{cache}}` fetch dests supersede too: same path with the `X.Y.Z` +token blanked = same entry (`cacheFamily`). Only under the cache dir — we own +it. Prompted dirs, hand-added paths, git URLs with credentials stay plain +append — a heuristic there would delete unrelated entries. Renaming a dest +family (`planify-*` → `opencode-planify-german-*`) still stacks once. +`remove` is untouched: it still deletes only exact matches. ## Merge stays additive diff --git a/src/batch.ts b/src/batch.ts index 21fcd67..b432813 100644 --- a/src/batch.ts +++ b/src/batch.ts @@ -251,7 +251,7 @@ export async function runBatch(opts: RunBatchOpts): Promise { ? new Set(Object.keys(beforeThisModule)) : new Set(); - const { next, stats } = applyAtPath(working, fullPath, fullBody, m.meta.mode); + const { next, stats } = applyAtPath(working, fullPath, fullBody, m.meta.mode, { cacheDir }); working = next; let preservedBatch = 0; diff --git a/src/merge.ts b/src/merge.ts index 7302093..d047da9 100644 --- a/src/merge.ts +++ b/src/merge.ts @@ -11,7 +11,10 @@ // and missing incoming entries are appended. Idempotent. // An incoming `name@spec` entry supersedes every existing // entry naming the same package, rather than stacking a -// second version of it alongside the first. +// second version of it alongside the first. Same for a +// versioned path under the cache dir (a `@fetch` dest). + +import { sep } from 'node:path'; export type MergeMode = 'replace' | 'merge' | 'merge-overwrite' | 'append'; @@ -38,14 +41,15 @@ export function applyAtPath( root: Json, dottedPath: string, value: Json, - mode: MergeMode = 'replace' + mode: MergeMode = 'replace', + opts: { cacheDir?: string } = {} ): { next: JsonObject; stats: ApplyStats } { const segments = parsePath(dottedPath); const next: JsonObject = root === undefined || root === null ? {} : (structuredClone(root) as JsonObject); if (segments.length === 0) { - const { value: merged, stats } = combine(next, value, mode); + const { value: merged, stats } = combine(next, value, mode, opts.cacheDir); return { next: merged as JsonObject, stats }; } @@ -57,7 +61,7 @@ export function applyAtPath( } const last = segments[segments.length - 1]; - const { value: combined, stats } = combine(cursor[last], value, mode); + const { value: combined, stats } = combine(cursor[last], value, mode, opts.cacheDir); cursor[last] = combined; return { next, stats }; } @@ -66,9 +70,9 @@ export function applyAtPath( // `@scope/pkg@1.2.3`, `superpowers@git+https://…#v6.3.0`. opencode loads every // entry in the array, so two specs naming the same package load that plugin // twice at two versions; append mode uses this to supersede instead of stack. -// Anything that is not a `name@spec` string returns null and keeps the plain -// append behaviour: a `{{cache}}` fetch destination, a prompted directory, or a -// git URL carrying credentials (`git+https://user@host/…`), whose trailing `@` +// Anything that is not a `name@spec` string returns null: a `{{cache}}` fetch +// destination (see cacheFamily), a prompted directory, or a git URL carrying +// credentials (`git+https://user@host/…`), whose trailing `@` // would otherwise split in the wrong place. // opencode also accepts `[name@spec, {options}]` for a plugin with options; the // spec inside names the same package, so a tuple and a plain string supersede @@ -84,7 +88,43 @@ function specName(value: Json): string | null { return name; } -function combine(existing: Json, incoming: Json, mode: MergeMode): { value: Json; stats: ApplyStats } { +// The family of a versioned `@fetch` destination: `/pkg-rules-0.4.0.md` +// and `/pkg-rules-0.3.2.md` share one. AGENTS.md mandates a versioned +// dest filename, so without this every preset bump stacks another +// `instructions` or `skills.paths` entry and opencode loads both. Only paths +// under the cache dir qualify: opencode-presets owns it, so the version +// heuristic cannot reach a prompted directory or a hand-added path that merely +// looks versioned. +// The pre-release suffix is limited to known tags: a generic `-[\w.]+` would eat +// the extension (`-0.5.0-rc.1.md` vs `-0.4.0.md` stop matching) and fold +// `tool-1.0.0-linux.tar` and `tool-1.0.0-darwin.tar` into one family. +const VERSION_TOKEN = /\d+\.\d+\.\d+(?:-(?:alpha|beta|rc|pre|next|dev)(?:\.?\d+)?)?/; + +function cacheFamily(value: Json, cacheDir: string | undefined): string | null { + if (!cacheDir || typeof value !== 'string') return null; + const root = cacheDir.endsWith(sep) || cacheDir.endsWith('/') ? cacheDir.slice(0, -1) : cacheDir; + // `{{cache}}/x` is expanded by plain substitution, so on Windows the + // separator after the cache dir is `/`, not `sep`. + if (!value.startsWith(root) || (value[root.length] !== '/' && value[root.length] !== sep)) return null; + const tail = value.slice(root.length + 1); + if (!VERSION_TOKEN.test(tail)) return null; + return root + '/' + tail.replace(VERSION_TOKEN, ''); +} + +// Entries sharing an identity are versions of one thing; append keeps one. +function identity(value: Json, cacheDir: string | undefined): string | null { + const pkg = specName(value); + if (pkg !== null) return 'pkg:' + pkg; + const family = cacheFamily(value, cacheDir); + return family === null ? null : 'cache:' + family; +} + +function combine( + existing: Json, + incoming: Json, + mode: MergeMode, + cacheDir?: string +): { value: Json; stats: ApplyStats } { const stats: ApplyStats = { mode, added: 0, preserved: 0, overwritten: 0, superseded: 0, replaced: false }; if (mode === 'append') { @@ -96,13 +136,13 @@ function combine(existing: Json, incoming: Json, mode: MergeMode): { value: Json } const target: Json[] = Array.isArray(existing) ? [...existing] : []; for (const v of incoming) { - const name = specName(v); + const name = identity(v, cacheDir); // The first entry naming the same package anchors the position. Look it // up before the deep-equality check: a config holding both `pkg@0.8.1` // and `pkg@0.9.0` must still collapse when `pkg@0.9.0` is installed, and // an equality-first check would call that a preserved no-op and leave the // stale entry behind. - const at = name === null ? -1 : target.findIndex(existingValue => specName(existingValue) === name); + const at = name === null ? -1 : target.findIndex(existingValue => identity(existingValue, cacheDir) === name); if (at === -1) { if (target.some(existingValue => deepEqual(existingValue, v))) { stats.preserved++; @@ -122,7 +162,7 @@ function combine(existing: Json, incoming: Json, mode: MergeMode): { value: Json stats.superseded++; } for (let i = target.length - 1; i > at; i--) { - if (specName(target[i]) === name) { + if (identity(target[i], cacheDir) === name) { target.splice(i, 1); stats.superseded++; } diff --git a/test/merge.test.ts b/test/merge.test.ts index 910a6f7..a5374ac 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -1,5 +1,6 @@ import { test, describe } from 'node:test'; import assert from 'node:assert/strict'; +import { join, sep } from 'node:path'; import { applyAtPath, removeAtPath, getAtPath } from '../src/merge.js'; describe('applyAtPath — replace mode', () => { @@ -175,15 +176,89 @@ describe('applyAtPath — append mode', () => { assert.equal(stats.superseded, 1); }); - // Only `name@spec` entries carry a package identity. A fetched skill path or - // a prompted directory must keep the plain additive behaviour, or unrelated - // entries sharing a prefix would silently delete each other. - test('leaves non-package entries to plain append', () => { - const root = { skill: ['{{cache}}/planify-skills-0.3.2'] }; - const { next, stats } = applyAtPath(root, 'skill', ['{{cache}}/planify-skills-0.4.0'], 'append'); - assert.deepEqual((next as any).skill, ['{{cache}}/planify-skills-0.3.2', '{{cache}}/planify-skills-0.4.0']); - assert.equal(stats.added, 1); - assert.equal(stats.superseded, 0); + // A versioned `@fetch` dest is a new path on every bump; opencode loads every + // `instructions` and `skills.paths` entry, so the old one must go. + describe('versioned cache paths', () => { + const cacheDir = join(sep, 'home', 'u', '.cache', 'opencode-presets'); + const at = (p: string) => join(cacheDir, p); + const opts = { cacheDir }; + + test('supersede an older version in place', () => { + const root = { skills: { paths: ['/home/u/diagram-design', at('planify-skills-0.3.2')] } }; + const { next, stats } = applyAtPath(root, 'skills.paths', [at('planify-skills-0.4.0')], 'append', opts); + assert.deepEqual((next as any).skills.paths, ['/home/u/diagram-design', at('planify-skills-0.4.0')]); + assert.equal(stats.added, 0); + assert.equal(stats.superseded, 1); + }); + + test('collapse a config that already stacked several versions', () => { + const root = { instructions: [at('planify-rules-0.3.1.md'), '/x/AGENTS.md', at('planify-rules-0.3.2.md')] }; + const { next } = applyAtPath(root, 'instructions', [at('planify-rules-0.4.0.md')], 'append', opts); + assert.deepEqual((next as any).instructions, [at('planify-rules-0.4.0.md'), '/x/AGENTS.md']); + }); + + test('reinstalling the same version stays a preserved no-op', () => { + const root = { instructions: [at('planify-rules-0.4.0.md')] }; + const { next, stats } = applyAtPath(root, 'instructions', [at('planify-rules-0.4.0.md')], 'append', opts); + assert.deepEqual((next as any).instructions, [at('planify-rules-0.4.0.md')]); + assert.equal(stats.preserved, 1); + assert.equal(stats.superseded, 0); + }); + + test('different families in the cache dir do not touch each other', () => { + const root = { p: [at('planify-rules-0.4.0.md')] }; + const { next } = applyAtPath(root, 'p', [at('planify-skills-0.4.0')], 'append', opts); + assert.deepEqual((next as any).p, [at('planify-rules-0.4.0.md'), at('planify-skills-0.4.0')]); + }); + + // Outside the cache dir a path is the user's (a prompted dir, a hand-added + // entry); a version-looking name there is no evidence it is ours to delete. + test('leave versioned paths outside the cache dir to plain append', () => { + const root = { p: ['/home/u/tools-1.0.0'] }; + const { next, stats } = applyAtPath(root, 'p', ['/home/u/tools-2.0.0'], 'append', opts); + assert.deepEqual((next as any).p, ['/home/u/tools-1.0.0', '/home/u/tools-2.0.0']); + assert.equal(stats.superseded, 0); + }); + + test('leave unversioned cache paths to plain append', () => { + const root = { p: [at('rules-a.md')] }; + const { next } = applyAtPath(root, 'p', [at('rules-b.md')], 'append', opts); + assert.deepEqual((next as any).p, [at('rules-a.md'), at('rules-b.md')]); + }); + + test('a sibling dir sharing the cache dir as a name prefix is not the cache', () => { + const root = { p: [join(cacheDir + '-old', 'x-1.0.0')] }; + const { next } = applyAtPath(root, 'p', [join(cacheDir + '-old', 'x-2.0.0')], 'append', opts); + assert.equal((next as any).p.length, 2); + }); + + test('a pre-release keeps the extension and supersedes both ways', () => { + const up = applyAtPath({ p: [at('rules-0.4.0.md')] }, 'p', [at('rules-0.5.0-rc.1.md')], 'append', opts); + assert.deepEqual((up.next as any).p, [at('rules-0.5.0-rc.1.md')]); + const down = applyAtPath(up.next, 'p', [at('rules-0.5.0.md')], 'append', opts); + assert.deepEqual((down.next as any).p, [at('rules-0.5.0.md')]); + }); + + test('same version, different platform suffix, are different entries', () => { + const root = { p: [at('tool-1.0.0-linux.tar')] }; + const { next, stats } = applyAtPath(root, 'p', [at('tool-1.0.0-darwin.tar')], 'append', opts); + assert.deepEqual((next as any).p, [at('tool-1.0.0-linux.tar'), at('tool-1.0.0-darwin.tar')]); + assert.equal(stats.superseded, 0); + }); + + // `{{cache}}/x` is plain substitution, so the separator after the cache + // dir is always `/`, whatever the platform separator is. + test('a `/` after the cache dir matches regardless of platform', () => { + const root = { p: [cacheDir + '/rules-0.3.2.md'] }; + const { next } = applyAtPath(root, 'p', [cacheDir + '/rules-0.4.0.md'], 'append', opts); + assert.deepEqual((next as any).p, [cacheDir + '/rules-0.4.0.md']); + }); + + test('without a cacheDir, paths keep plain append', () => { + const root = { p: [at('planify-skills-0.3.2')] }; + const { next } = applyAtPath(root, 'p', [at('planify-skills-0.4.0')], 'append'); + assert.equal((next as any).p.length, 2); + }); }); // `git+https://user@host/...` has a trailing `@` that does not split a