diff --git a/llp/0219-retired-client-assets-are-pruned.decision.md b/llp/0219-retired-client-assets-are-pruned.decision.md index 2b87aa1fd..72350f334 100644 --- a/llp/0219-retired-client-assets-are-pruned.decision.md +++ b/llp/0219-retired-client-assets-are-pruned.decision.md @@ -6,7 +6,7 @@ **Author:** Claude **Date:** 2026-08-13 **Related:** LLP 0138 (#one-materializer, #marker-undo: the module this extends and the half-record it left), LLP 0107 (#currency, #reversal: why attach re-runs and what reversal may touch), LLP 0142 (the retirement that is still installed), LLP 0212 (the retirement that motivated #726), LLP 0215 (#not-in-scope: named this gap and deferred it) -**Extended-by:** LLP 0226 (narrows #prune-on-materialize's condition three to a direct child, and splits #edited-assets-are-not-ours's "no digest" outcome into gone vs. unreadable), LLP 0284 (widens #edited-assets-are-not-ours's match from one client's record to every digest recorded for the path), LLP 0400 (adds a second kind of ownership evidence, for healing a stale record only: bytes equal to the current source; #edited-assets-are-not-ours itself is unchanged and still gates the prune) +**Extended-by:** LLP 0226 (narrows #prune-on-materialize's condition three to a direct child, and splits #edited-assets-are-not-ours's "no digest" outcome into gone vs. unreadable), LLP 0284 (widens #edited-assets-are-not-ours's match from one client's record to every digest recorded for the path), LLP 0400 (adds a second kind of ownership evidence, for healing a stale record only: bytes equal to the current source; #edited-assets-are-not-ours itself is unchanged and still gates the prune), LLP 0401 (narrows what #edited-assets-are-not-ours's digest covers to the set the copier carries, so an entry the copy cannot hold no longer registers as a user edit) > Extends [LLP 0138](./0138-client-assets-one-install.decision.md), which made > one routine own copying client assets and recorded, on the org-driven half diff --git a/llp/0226-prune-direct-children-and-unreadable-assets.decision.md b/llp/0226-prune-direct-children-and-unreadable-assets.decision.md index d54d7d3cb..d0e5023e4 100644 --- a/llp/0226-prune-direct-children-and-unreadable-assets.decision.md +++ b/llp/0226-prune-direct-children-and-unreadable-assets.decision.md @@ -108,7 +108,10 @@ silently migrate into "gone" the way it could when one `try` wrapped *inside* the tree never reaches either `try`'s error path: `hashTree` reads shape from `readdir`'s `Dirent` entries without following them, so a symlink whose target is gone hashes as an opaque entry by name, same as one -whose target exists; nothing about it is unreadable in the first place. +whose target exists; nothing about it is unreadable in the first place. (As of +[LLP 0401 #digest-covers-the-copy](./0401-the-asset-digest-covers-what-the-copier-carries.decision.md#digest-covers-the-copy) +`hashTree` skips such an entry outright rather than hashing it by name, which +leaves that conclusion standing for a stronger reason.) ## Consequences {#consequences} diff --git a/llp/0397-installed-client-assets-follow-the-update.decision.md b/llp/0397-installed-client-assets-follow-the-update.decision.md index 1ce06a9f8..a789ffd33 100644 --- a/llp/0397-installed-client-assets-follow-the-update.decision.md +++ b/llp/0397-installed-client-assets-follow-the-update.decision.md @@ -114,9 +114,12 @@ changed, deciding each one by the install ledger.** need a field the ledger does not have, and the work it saves is hashing a few dozen small files once per boot. Bounded by the ledger, not by uptime. - A source tree that holds an entry the copier skips (a symlink, a device) - never digests equal to its copy, so such an asset is re-copied on every - boot. No shipped skill has one; the cost if one did is one small copy per - boot, and the fix would be in the copier, not here. + still digests equal to its copy, because the hasher skips the same set: what + the copy does not carry, the digest does not cover. Covering it would re-copy + the asset on every boot, since the copy can never match (issue #1666). + [LLP 0401](./0401-the-asset-digest-covers-what-the-copier-carries.decision.md) + settles that and states what the narrowing gives up: such an entry added to an + installed copy no longer reads as a user edit. - `hyp skills install` keeps the role [LLP 0107 #every-attach](./0107-skills-ride-attach.decision.md#every-attach) gives it: the way to re-copy on demand, and now also the way to replace a diff --git a/llp/0401-the-asset-digest-covers-what-the-copier-carries.decision.md b/llp/0401-the-asset-digest-covers-what-the-copier-carries.decision.md new file mode 100644 index 000000000..c96d83f61 --- /dev/null +++ b/llp/0401-the-asset-digest-covers-what-the-copier-carries.decision.md @@ -0,0 +1,104 @@ +# LLP 0401: the asset digest covers exactly what the copier carries + +**Type:** Decision +**Status:** Accepted +**Systems:** Onboarding, Plugins, Daemon +**Author:** Claude +**Date:** 2026-09-11 +**Related:** LLP 0219 (#edited-assets-are-not-ours: the evidence rule this narrows), LLP 0226 (#unreadable-is-not-absent: the paragraph whose supporting sentence this makes obsolete), LLP 0284 (#digests-are-per-path: which records the gate reads), LLP 0397 (#ledger-decides: the boot refresh whose unchanged check this makes reachable), LLP 0400 (#one-write: the per-boot write this stops) +**Extends:** LLP 0219 + +> `copyDir` copies files and directories and skips every other entry; +> `hashTree` folded any other entry into the digest as an `o:` line. A +> source tree holding a symlink could therefore never digest equal to the copy +> made of it, so the boot refresh re-copied the tree and rewrote the ledger on +> every boot, forever. This settles that the digest covers exactly the set the +> copier carries, and states the edit-detection that narrowing gives up. + +## Context {#context} + +[LLP 0397 #ledger-decides](./0397-installed-client-assets-follow-the-update.decision.md#ledger-decides) +decides an installed copy by comparing `digest(source)` with the digest +recorded for the copy. That comparison presumes the two are asked the same +question. They were not: the copier's set of entries and the hasher's set were +written independently and disagreed on everything that is neither a file nor a +directory. + +For a source tree holding one such entry the disagreement is total and +permanent. `digest(source)` carries an `o:` line the copy can never carry, so +the unchanged check fails on every boot, the whole tree is re-copied, the new +digest is recorded, and the post-loop ledger write runs with byte-identical +content every time. That is the repeated per-boot work and the per-boot write +[LLP 0400 #one-write](./0400-source-equality-heals-a-stale-record.decision.md#one-write) +rejects, and it is silent except as an endless `refreshed` line +([#1666](https://github.com/hyparam/hypaware/issues/1666)). No shipped skill +source holds such an entry today, but a `local-dir` plugin source can. + +Two directions close it: teach the copier to carry what the hasher covers, or +narrow the hasher to what the copier carries. + +## Decision {#decision} + +**The digest covers exactly the set `copyDir` copies: files and directories, +and nothing else** {#digest-covers-the-copy}. + +- **The copier is the authority, not the hasher.** What the copy does not hold, + the digest does not cover. The invariant the refresh needs is one-directional: + the hash may never cover something the copy cannot carry. Narrowing the hasher + satisfies it for every entry kind at once. + +- **The copier is not taught to carry them.** Three reasons, in order of weight. + A recreated symlink is a pointer the plugin tree chose, materialized into + `~/.claude/skills`, which the client loads as skill content: the copier would + gain the ability to place a reference to anywhere on disk into a directory it + otherwise only ever writes plain files into. A fifo, socket, or device cannot + be recreated at all without reaching outside the standard library, so that + direction closes the symlink case and leaves the identical bug for the rest. + And copying a symlink changes `digest(copy)`, which invalidates every ledger + digest already recorded for such a tree and freezes those assets as + `asset_edited` with no self-repair. + +- **Recorded digests survive** {#migration}. Skipping changes the digest only of + a tree that holds an entry the copier already skipped, so every digest ever + recorded for a tree the copier wrote is unchanged. Verified against the + shipped trees before landing: `src`, `src/core/cli`, `hypaware-core/plugins-workspace`, + and every `skills/` and `agents/` directory digest byte-identically under both + versions. No migration, and no boot that reports a copy as edited because the + hasher moved under it. + +- **Edit detection narrows to the same set** {#edit-detection-narrows}. This is + the price, and it is a real narrowing of + [LLP 0219 #edited-assets-are-not-ours](./0219-retired-client-assets-are-pruned.decision.md#edited-assets-are-not-ours), + which reads the digest as the measure of "the user took this copy over". A + symlink the user drops into an installed copy no longer moves the digest, so + the copy still matches its record: the refresh overwrites it on the next + source change instead of reporting `asset_edited`, and a prune of the retired + asset removes the directory (the link with it, never its target) instead of + withholding it. A file or a directory the user adds, and any edit to the bytes + of a file already there, still move the digest and still stop both. + + Accepted because the alternative is worse in the same terms: covering the + entry buys edit detection for one entry kind and pays for it with a tree that + can never settle, is re-copied on every boot, and whose next copy overwrites + the user's entry anyway. + +## Consequences {#consequences} + +- A source tree holding a symlink settles: the boot after the copy reports it + unchanged, copies nothing, and writes no ledger. Pinned by a test that fails + on the previous hasher. +- The hasher does strictly less work per skipped entry (no `path.join`, no + `path.relative`, no `hash.update`) and the same work per entry it keeps. +- Two trees differing only in entries the copier skips now digest identically. + Every digest consumer sees that: the refresh's unchanged check, the record the + materializer writes, and the prune's ownership evidence. The prune's other + gates are untouched, including the shape check that refuses a candidate whose + file-or-directory kind contradicts its record. +- The `o:` line is gone from the hash input. Nothing ever parsed it: the digest + is opaque to every reader, compared only against another digest produced by + the same function. +- [LLP 0226 #unreadable-is-not-absent](./0226-prune-direct-children-and-unreadable-assets.decision.md#unreadable-is-not-absent) + is unaffected in what it decided, but its supporting sentence that a dangling + symlink inside the tree "hashes as an opaque entry by name" no longer + describes the code. The conclusion that sentence supports still holds, and for + a stronger reason: a skipped entry reaches neither `try`'s error path at all. diff --git a/src/core/runtime/client_asset_ledger.js b/src/core/runtime/client_asset_ledger.js index b9aa32750..7c80691bd 100644 --- a/src/core/runtime/client_asset_ledger.js +++ b/src/core/runtime/client_asset_ledger.js @@ -160,8 +160,8 @@ export async function writeClientAssetLedger(stateRoot, records) { * * Paths are hashed alongside the bytes so adding or renaming a file inside an * installed skill registers as a change; entries that are neither a file nor a - * directory (a symlink someone dropped in) contribute their name only, so they - * likewise cannot be mistaken for the tree we copied. + * directory (a symlink someone dropped in) are skipped, because `copyDir` skips + * them too and a source tree has to digest equal to the copy made of it. * * **The shape is hashed before anything else.** Without it the two branches * write into the same unframed byte stream and produce collisions across kinds: @@ -178,6 +178,8 @@ export async function writeClientAssetLedger(stateRoot, records) { * @ref LLP 0219#edited-assets-are-not-ours [implements]: a digest may only match * what we actually wrote, so file-shaped and directory-shaped content are * hashed in separate domains. + * @ref LLP 0401#edit-detection-narrows [constrained-by]: "what we wrote" is what + * the copier carries, so an entry it cannot carry does not register as an edit. */ export async function digestClientAsset(dest) { return (await inspectClientAsset(dest)).digest @@ -209,11 +211,10 @@ export async function digestClientAsset(dest) { * error, a file `readdir` just listed that a concurrent actor removes before * the following `readFile` reaches it) falls into a second, narrower `try` * that always reports `missing: false`, so a failure below `dest` can never - * be mistaken for `dest` itself being gone. A dangling symlink *inside* the - * tree never reaches either `try`'s error path at all: `hashTree` reads - * `Dirent` shape from `readdir` without following the entry, so a symlink - * whose target is gone hashes as an opaque `o:` entry by name, the same as - * one whose target exists. + * be mistaken for `dest` itself being gone. A symlink *inside* the tree never + * reaches either `try`'s error path at all: `hashTree` reads `Dirent` shape + * from `readdir` without following the entry, so it is skipped whether its + * target is gone or not. * * @param {string} dest * @returns {Promise<{ digest?: string, missing: boolean }>} `missing` is true @@ -258,13 +259,19 @@ async function hashTree(root, dir, hash) { const entries = await fs.readdir(dir, { withFileTypes: true }) entries.sort((a, b) => compareStrings(a.name, b.name)) for (const entry of entries) { + // Exactly the set `copyDir` copies: an entry it skips must not reach the + // hash either, or a source tree holding one could never digest equal to + // the copy made of it, and the refresh would re-copy it on every boot. + // @ref LLP 0401#digest-covers-the-copy [implements]: the copier is the + // authority on what a tree is, so the hash may never cover more. + if (!entry.isDirectory() && !entry.isFile()) continue const full = path.join(dir, entry.name) // The entry's shape leads its path, so a subdirectory named `x` and a file // named `x` cannot hash alike, and a file's bytes can never be read back as // the tree that would have followed a directory of the same name. - hash.update(`${entry.isDirectory() ? 'd' : entry.isFile() ? 'f' : 'o'}:${path.relative(root, full)}\n`) + hash.update(`${entry.isDirectory() ? 'd' : 'f'}:${path.relative(root, full)}\n`) if (entry.isDirectory()) await hashTree(root, full, hash) - else if (entry.isFile()) hash.update(await fs.readFile(full)) + else hash.update(await fs.readFile(full)) } } diff --git a/src/core/util/fs_copy.js b/src/core/util/fs_copy.js index 7bed48ca2..ab5025cad 100644 --- a/src/core/util/fs_copy.js +++ b/src/core/util/fs_copy.js @@ -9,9 +9,16 @@ import path from 'node:path' * skill/agent installers want (a template tree copied into a client's * config directory). * + * The predicate is not local to this file: `hashTree` mirrors it so a source + * tree and the copy made of it digest equal. Widening it here alone makes the + * digest cover less than the copy carries, and the boot refresh re-copies such + * an asset on every boot forever (issue #1666). + * * @param {string} src * @param {string} dest * @returns {Promise} + * @ref LLP 0401#digest-covers-the-copy [constrained-by]: the asset digest is + * defined as what this function carries, so the two sets move together. */ export async function copyDir(src, dest) { await fs.mkdir(dest, { recursive: true }) diff --git a/test/core/client-assets-refresh.test.js b/test/core/client-assets-refresh.test.js index 937145ab8..5b5fdf3d1 100644 --- a/test/core/client-assets-refresh.test.js +++ b/test/core/client-assets-refresh.test.js @@ -151,6 +151,32 @@ test('a changed source is re-copied and the ledger digest follows it', async () assert.deepEqual(again.refreshed, []) }) +test('a source holding a symlink settles instead of re-copying on every boot', async () => { + // `copyDir` skips a symlink, so the copy cannot hold one. Unless the digest + // skips it too, the source never digests equal to its copy: every boot fails + // the unchanged check, re-copies the tree, and rewrites the ledger with + // identical content. + const h = await makeHome() + const src = await writeSkillSource(h.home, 'alpha', 'v1') + await fs.symlink('SKILL.md', path.join(src, 'alias.md')) + const regs = registries([{ name: 'alpha', sourceDir: src }]) + await install(h, regs) + const dest = path.join(h.home, '.claude/skills/alpha') + assert.deepEqual(await fs.readdir(dest), ['SKILL.md']) + + // Backdated, so a rewrite of identical content still shows as a write. + const ledgerFile = path.join(h.stateRoot, 'client-assets.json') + const backdated = new Date(Date.now() - 60_000) + await fs.utimes(ledgerFile, backdated, backdated) + const before = await fs.stat(ledgerFile) + + const out = await refresh(h, regs) + assert.equal(out.unchanged, 1) + assert.deepEqual(out.refreshed, []) + assert.deepEqual(out.skipped, []) + assert.equal((await fs.stat(ledgerFile)).mtimeMs, before.mtimeMs) +}) + test('a copy the user edited is kept and named, even when the source moved on', async () => { const h = await makeHome() const src = await writeSkillSource(h.home, 'alpha', 'v1')