Repository navigation
refactor(framework): the tool declarations live in two copies, because an install carries only some directories #702
Description
The tool declarations that decide what this layer can measure now exist in two identical copies, because a plugin install carries only some of a plugin's directories.
What was done, and why
The diagnostic skill (02-check) needs the same three libraries the cost skill uses: readers.js, which holds the TOOLS table — the single place a tool is named, with its reader, its reason for being unreadable, and what each route can supply — plus journal.js and attribution.js.
It cannot import them. Two constraints meet:
- Plugin translation carries
skills/,agents/,commands/,rules/andhooks/. A sharedlib/at the plugin root is silently dropped on install, which is howbin/was lost before. - Each skill owns its own responsibility and never reaches into another skill's directory. That was a deliberate decision, and a test enforces it.
So the three files are copied byte for byte, and a test asserts they stay identical. 498 lines of the diagnostic skill's 778 are copy.
Why it is worth revisiting
The parity test holds today because the copies are exact. It stops holding the moment either copy needs a change the other does not — a diagnostic that wants a cheaper read, a cost report that wants a new field. At that point the test fails for a legitimate reason, and whoever is under pressure deletes it.
TOOLS is the worst thing to have twice. It is what decides which tools are covered, what each supplies, and what a person is told cannot be measured. Two copies of that answer is two answers.
Options, none obviously right
- A shared directory the install actually carries.
hooks/lib/is carried. Moving tool knowledge there makes it plugin-wide rather than skill-owned, at the cost of a skill reading from the hooks' directory. - Teach translation to carry one more directory. Cleanest to read, and it changes the plugin contract for every plugin, not just this one.
- Generate the copies rather than maintaining them. Keeps the constraint and removes the drift, but adds a build step to a plugin that deliberately has none.
- Leave it. The parity test is real and the copies are exact. This costs nothing until it costs everything at once.
Done when
- The tool declarations exist once, or the reason they cannot is written where the next person will read it before proposing a fourth option.
- Whatever holds them cannot be silently dropped by an install — proven by installing, not by reading the translator.
Where it came from
Building the diagnostic skill for #617 and #694. The duplication is not a mistake in that work; it is the cheapest correct answer under the two constraints above, and it should not be the permanent one.
Activity
blafourcade commentedon Aug 22, 2026
The debt grew, and it has now caused a defect rather than only maintenance
When this was filed, three files were copied. The diagnostic skill's
scripts/lib/now holds nine, and five of them duplicate something that exists elsewhere in the same plugin:journal.js,readers.js,attribution.js01-cost/scripts/lib/switch.jshooks/lib/repo.js'stelemetryEnabledrepo.jshooks/lib/repo.js'sgetRepoRootunrecognised.jshooks/lib/record.js's marker nameEach guard was written deliberately and each is real. That is the point: five separate guards, five separate shapes, because there is no place to put shared code that an install carries.
The defect
Someone reached across the boundary to get one constant —
require("../../../hooks/lib/record.js")for a filename. From a tree holdingskills/and nohooks/, the script died at load withMODULE_NOT_FOUND, above its own try/catch. That tree is the OpenCode install exactly: the translator delivers everyskills/**file and records hooks only as skipped.310 tests passed over it, because every one runs from the source tree where
hooks/is present. It was caught by running the script from a copied tree, which is the only thing that would have.So the cost is no longer hypothetical. The constraint pushed someone toward the obvious, wrong thing, and the test suite could not see it. The fix was a sixth copy.
What that changes about the options
The fourth option in the original list — "leave it, the parity tests are real" — is now the weakest. The guards hold what they were pointed at; they cannot hold what nobody thought to point them at, and the reflex the constraint encourages is the one that broke.
Two things worth adding to the options above:
refactor(framework): the tool declarations exist once
blafourcade commentedon Sep 2, 2026
Delivered by #706, squash-merged into
nextas 627408f.