From 031f0c7ddfd828289ad0bb824592a69ef6dbedd6 Mon Sep 17 00:00:00 2001 From: Jan Date: Mon, 5 Oct 2026 11:27:23 +0200 Subject: [PATCH] Supersede versioned cache paths on append Every preset bump writes a new versioned @fetch dest, and append kept the old one: instructions and skills.paths stacked opencode-planify-german 0.2.1 through 0.4.0, so opencode loaded every version's rules and skill. An append entry under the cache dir now shares an identity with any entry that differs only in its X.Y.Z token, and supersedes it in place like a name@spec plugin does. Paths outside the cache dir keep plain append. --- AGENTS.md | 9 +++-- src/batch.ts | 2 +- src/merge.ts | 62 +++++++++++++++++++++++++------ test/merge.test.ts | 93 +++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 142 insertions(+), 24 deletions(-) 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