Skip to content

Commit 80e7f57

Browse files
committed
fix(optional-integration): detect absence by quoted specifier, not bare substring
The recipe classified a dynamic-import failure as "package absent" using message.includes("@r3b1s/pi-repair-layer"). A present-but-broken install (pi-repair-layer resolvable, a transitive dependency like typebox missing) throws a module-not-found error naming the transitive module while embedding pi-repair-layer only as a node_modules path segment, so the bare substring misread it as absence and silently ran tools unwrapped. Match the package name only as an imported specifier — an opening quote immediately followed by the name ('@r3b1s/pi-repair-layer, no trailing quote so both the bare-package and /pi-subpath error shapes match) — and extract the classification into an exported isRepairPackageAbsent predicate for direct unit testing. A broken install now rethrows loudly instead of falling back. Updates the recipe fixture, the tool-owner integration guide, research Claim 11, and the optional-integration spec; adds predicate unit coverage.
1 parent 6ed0c7b commit 80e7f57

10 files changed

Lines changed: 271 additions & 26 deletions

File tree

docs/research.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,19 @@ the requested module:
257257

258258
**Why it matters:** the optional-integration recipe treats an import failure
259259
as "package absent" only when the code is one of these two values **and** the
260-
message names `@r3b1s/pi-repair-layer`; anything else rethrows so a broken
261-
install (a *transitive* module missing) is not silently misread as absent.
262-
All three observed shapes pass that discrimination.
260+
message names `@r3b1s/pi-repair-layer` **as a quoted module specifier**
261+
matched as an opening quote immediately followed by the name
262+
(`'@r3b1s/pi-repair-layer`, no trailing quote so both the bare-package and
263+
`/pi`-subpath forms match); anything else rethrows so a broken install (a
264+
*transitive* module missing, e.g. `typebox`) is not silently misread as
265+
absent. The quote anchor matters because the jiti and compiled-binary shapes
266+
carry the importer path: a present-but-broken install throws
267+
`Cannot find module 'typebox/value' from '.../node_modules/@r3b1s/pi-repair-layer/...'`,
268+
which contains `@r3b1s/pi-repair-layer` only as a path segment (preceded by
269+
`/`, not a quote). A bare-substring match would misread that as absence; the
270+
quoted-specifier match rethrows it loudly. All three observed absent-package
271+
shapes still begin with quote-then-name and pass the discrimination.
272+
(Refined 2026-07-21; matcher verified by unit test against all three shapes.)
263273

264274
### Claim 12 — Git installs and other scopes do not resolve the shared npm siblings
265275

docs/tool-owner-integration.md

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -191,19 +191,29 @@ const definition: ToolDefinition<typeof parameters> = {
191191
},
192192
};
193193

194+
// Opening quote + name, no trailing quote: matches both the bare-package ESM
195+
// error (`'@r3b1s/pi-repair-layer'`) and the jiti/binary subpath error
196+
// (`'@r3b1s/pi-repair-layer/pi'`), while a `node_modules/...` path segment
197+
// (preceded by `/`, not a quote) does not read as absence.
198+
const REPAIR_PACKAGE_SPECIFIER_QUOTED = "'@r3b1s/pi-repair-layer";
199+
200+
export function isRepairPackageAbsent(error: unknown): boolean {
201+
const code = (error as { code?: unknown } | null)?.code;
202+
const message = error instanceof Error ? error.message : String(error);
203+
return (
204+
(code === "MODULE_NOT_FOUND" || code === "ERR_MODULE_NOT_FOUND") &&
205+
message.includes(REPAIR_PACKAGE_SPECIFIER_QUOTED)
206+
);
207+
}
208+
194209
async function loadRepairAdapter(): Promise<
195210
typeof adaptToolDefinition | undefined
196211
> {
197212
try {
198213
const repair = await import("@r3b1s/pi-repair-layer/pi");
199214
return repair.adaptToolDefinition;
200215
} catch (error) {
201-
const code = (error as { code?: unknown } | null)?.code;
202-
const message = error instanceof Error ? error.message : String(error);
203-
const packageAbsent =
204-
(code === "MODULE_NOT_FOUND" || code === "ERR_MODULE_NOT_FOUND") &&
205-
message.includes("@r3b1s/pi-repair-layer");
206-
if (!packageAbsent) throw error;
216+
if (!isRepairPackageAbsent(error)) throw error;
207217
return undefined;
208218
}
209219
}
@@ -225,12 +235,16 @@ The details are load-bearing:
225235
`code: "MODULE_NOT_FOUND"` (jiti's require path) or
226236
`code: "ERR_MODULE_NOT_FOUND"` (native ESM and the compiled pi binary). But
227237
a *present-but-broken* install — pi-repair-layer resolvable while one of its
228-
own transitive modules is not — throws the same codes naming the
229-
*transitive* module. Requiring the message to name
230-
`@r3b1s/pi-repair-layer` keeps a broken install loud instead of silently
231-
running unwrapped while the user believes repairs are active. Match the
232-
package name, not the `/pi` subpath: native ESM reports only
233-
`Cannot find package '@r3b1s/pi-repair-layer'`.
238+
own transitive modules (e.g. `typebox`) is not — throws the same codes
239+
naming the *transitive* module, and its message embeds
240+
`.../node_modules/@r3b1s/pi-repair-layer/...` as a path segment. Match the
241+
package name only where it appears as an imported specifier — an opening
242+
quote immediately followed by the name (`'@r3b1s/pi-repair-layer`) — so that
243+
path segment does not read as absence. No trailing quote: jiti and the
244+
compiled binary name the full subpath (`'@r3b1s/pi-repair-layer/pi'`) while
245+
native ESM names the bare package (`'@r3b1s/pi-repair-layer'`); both start
246+
with quote-then-name. This keeps a broken install loud (it rethrows) instead
247+
of silently running unwrapped while the user believes repairs are active.
234248
- **Rethrow everything else.** Any other error is a real failure, not
235249
absence.
236250
- **Emit one stderr line when falling back.** The two branches differ in
@@ -337,7 +351,11 @@ touches is deliberately small and covered by the compatibility contract in
337351
current major; documented exports follow semantic versioning.
338352
- Absence detection semantics are part of the contract: a missing package
339353
surfaces with `code` `MODULE_NOT_FOUND` or `ERR_MODULE_NOT_FOUND` and a
340-
message naming the requested module.
354+
message naming the requested module. The recipe matches that name as a
355+
quoted specifier (`'@r3b1s/pi-repair-layer`, opening quote + name), not a
356+
bare substring, so the package name appearing as a `node_modules` path
357+
segment in a present-but-broken install rethrows loudly rather than reading
358+
as absence.
341359
- Unrecognized preprocessor `kind`s are ignored — never fatal, no mutation, no
342360
claimed change — and results are still schema-validated. Repair options
343361
written against a newer minor version therefore degrade to the recognized
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-07-21
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
## Context
2+
3+
The optional-integration recipe lets a tool-owning extension depend on `@r3b1s/pi-repair-layer` optionally: it dynamically imports `@r3b1s/pi-repair-layer/pi`, and on failure discriminates "package genuinely absent" (fall back to the raw tool definition) from every other error (rethrow). The discrimination is deliberately narrow because a *present-but-broken* install — the package resolves but its own `typebox` runtime dependency does not — throws the same `MODULE_NOT_FOUND` / `ERR_MODULE_NOT_FOUND` codes, and swallowing that would silently run tools unwrapped while the user believes repairs are active.
4+
5+
The recipe discriminated with a bare substring test, `message.includes("@r3b1s/pi-repair-layer")`. The jiti/require and compiled-binary error shapes (Claim 11 in `docs/research.md`) carry the importer *path*, e.g. `Cannot find module 'typebox/value' from '.../node_modules/@r3b1s/pi-repair-layer/dist/src/pipeline.js'`. That message names the missing *transitive* module (`typebox/value`) yet contains `@r3b1s/pi-repair-layer` as a path segment — so the bare substring test misclassified it as "absent."
6+
7+
## Goals / Non-Goals
8+
9+
**Goals:**
10+
- Make the absence matcher immune to the package name appearing as a `node_modules` path segment, so a present-but-broken install rethrows (loud) instead of being misread as absence.
11+
- Express the classification as a discrete, unit-testable predicate.
12+
- Keep the recipe copy-compatible for existing consumers; no breaking change; fixture, snippet, and spec in lockstep.
13+
14+
**Non-Goals:**
15+
- Making the `/pi` entry dependency-free (bundling typebox). Evaluated and dropped — see Decision 2.
16+
- The loader shim package (Roadmap item C).
17+
- Changing repair behavior, the adapter API surface, or the pipeline internals.
18+
19+
## Decisions
20+
21+
### Decision 1: Match the package name as a quoted module specifier
22+
23+
Change the recipe's absence predicate from `message.includes("@r3b1s/pi-repair-layer")` to match the name only where it appears as an imported specifier: an opening quote immediately followed by the name — `message.includes("'@r3b1s/pi-repair-layer")`.
24+
25+
- **No trailing quote.** All three documented shapes render the missing module in single quotes, but jiti and the compiled binary name the *full subpath* (`'@r3b1s/pi-repair-layer/pi'`) while native ESM names the *bare package* (`'@r3b1s/pi-repair-layer'`). Both begin with quote-then-name; a trailing-quote match (`'@r3b1s/pi-repair-layer'`) would fail to match the subpath form and rethrow a genuine absence. So the match is opening-quote + name only.
26+
- **Why this excludes path segments.** In a path-bearing message the importer path is quoted as a whole (`from '/…/node_modules/@r3b1s/pi-repair-layer/…'`); the character after the opening quote is `/` (or a drive letter), never `@r3b1s`. So the package name inside a path segment is never immediately preceded by a quote and cannot match.
27+
- **Keep the code gate.** `(code === "MODULE_NOT_FOUND" || code === "ERR_MODULE_NOT_FOUND")` remains the first gate; the quoted-name test is the discriminator.
28+
- **Extract for testability.** Lift the predicate into an exported `isRepairPackageAbsent(error)` so a unit test can feed it synthetic error shapes. vitest cannot force a dynamic `import()` to reject with an arbitrary error (a throwing mock factory is wrapped in vitest's own diagnostic), so the pure predicate is the testable seam.
29+
30+
### Decision 2: Rely on the precise matcher; do not make the `/pi` entry dependency-free
31+
32+
The `/pi` chain has exactly one third-party runtime import, `typebox/value`. Eliminating it (so a missing transitive dep is impossible) was considered via `bundledDependencies`, but that mechanism is rejected by pnpm's isolated node-linker — `pnpm publish` errors with `ERR_PNPM_BUNDLED_DEPENDENCIES_WITHOUT_HOISTED` — and this repo both installs isolated and publishes with `pnpm publish` (and steers away from npm via `scripts/bin/npm`/`npx` shims). The workarounds each cost more than they save:
33+
34+
- **`nodeLinker: hoisted`** — one line, but switches the whole repo off pnpm's strict isolated linking (reintroducing phantom-dependency risk), against the grain of a deliberately strict setup; the hoisted reinstall was also pathologically slow in testing.
35+
- **Publish via npm** — splits the toolchain and fights the repo's pnpm-only shims.
36+
- **esbuild bundling** — robust and mechanism-independent, but adds a bundler to an intentionally `tsc`-only build.
37+
38+
Decisive point: once Decision 1 lands, a present-but-broken install (`typebox` missing) already surfaces **correctly** — the matcher returns "not absent," the recipe rethrows, and the failure is loud at extension load. That is the exact behavior the discrimination always intended. Making the entry dependency-free would only convert a loud-and-correct failure into a can't-happen; it is a robustness nicety, not a correctness requirement. Deferred to the roadmap.
39+
40+
## Risks / Trade-offs
41+
42+
- **[A genuinely broken pi-repair-layer install fails the consumer's extension load]** → this is intended and correct: it is loud and diagnosable, strictly better than silently running unwrapped. Documented in the guide's caveats.
43+
- **[Quote-form assumptions across loaders]** → the opening-quote + name match is validated against all three documented shapes (jiti, native ESM, compiled binary) by the predicate unit test; a future loader change is caught by re-verifying Claim 11.
44+
- **[Snippet/fixture divergence]** → the doc snippet and `test/fixtures/optional-consumer.ts` are edited together; the existing "keep in sync" note flags this and the smoke test exercises the fixture.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
## Why
2+
3+
The optional-integration recipe's absence detection uses a bare substring test (`message.includes("@r3b1s/pi-repair-layer")`), which false-positives when the package name appears as a `node_modules` path segment in a path-bearing error message (jiti and compiled-binary error shapes carry the importer path). A consumer integrating the recipe recently hit exactly this: a *present-but-broken* install — pi-repair-layer resolves but its `typebox` dependency does not — threw an error naming the missing transitive module while embedding pi-repair-layer only as a path segment, and the bare substring misclassified it as "absent," which would silently run tools unwrapped while the user believed repairs were active.
4+
5+
## What Changes
6+
7+
- Harden the absence-detection matcher in the canonical recipe to match the package name only as an **imported module specifier** — an opening quote immediately followed by the name (`'@r3b1s/pi-repair-layer`, no trailing quote so it matches both the bare-package ESM error and the `/pi` subpath error) — rather than a bare substring. A `node_modules/@r3b1s/pi-repair-layer/...` path segment is preceded by `/`, not a quote, so it can no longer read as absence.
8+
- Extract the classification into an exported `isRepairPackageAbsent(error)` predicate so it is directly unit-testable (a dynamic `import()` cannot be forced to reject with an arbitrary error under vitest).
9+
- With the corrected matcher, a present-but-broken install now surfaces **correctly** as a loud rethrow at load rather than a silent fallback — the discrimination the recipe always intended, now actually achieved.
10+
- Update the canonical recipe fixture (`test/fixtures/optional-consumer.ts`), the `docs/tool-owner-integration.md` snippet and caveats, and the `optional-integration` spec accordingly, keeping snippet and fixture in sync.
11+
- Add a matcher unit test proving a path-bearing transitive-missing error is not classified as absent.
12+
13+
## Capabilities
14+
15+
### New Capabilities
16+
<!-- none -->
17+
18+
### Modified Capabilities
19+
- `optional-integration`: the documented absence check matches the package name as a quoted module specifier rather than a bare substring; the classification is expressed as an exported, unit-tested predicate; and the fixture/test coverage is extended with the path-segment false-positive case.
20+
21+
## Impact
22+
23+
- **Docs:** `docs/tool-owner-integration.md` recipe snippet, the "Discriminate before falling back" caveat, and the "Absence detection semantics" stability-contract bullet; `docs/research.md` Claim 11 detection-semantics rationale.
24+
- **Tests:** `test/fixtures/optional-consumer.ts` (extracted predicate + corrected matcher), a new matcher unit test.
25+
- **Specs:** modified `optional-integration`.
26+
- **No packaging/build change.** Making the `/pi` entry dependency-free (bundling `typebox`) was evaluated and dropped: it collides with the repo's pnpm isolated node-linker (`pnpm publish` rejects `bundledDependencies`) and the alternatives (repo-wide `nodeLinker: hoisted`, or publishing via npm) run against the repo's strict pnpm-only posture. The corrected matcher makes a broken install fail loudly and correctly, so eliminating the dependency is unnecessary for correctness (recorded as a roadmap consideration, not this change).
27+
- **Consumers:** no breaking change — the recipe stays copy-compatible; the matcher change is strictly more precise.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
## MODIFIED Requirements
2+
3+
### Requirement: Documented optional integration recipe
4+
The tool-owner integration guide SHALL contain an optional-integration section presenting a complete, copyable fallback recipe by which a tool-owning extension attempts a dynamic import of the `pi` subpath and, when the package is absent, registers its unmodified tool definition. The recipe SHALL treat an import failure as "absent" only when the error code is `MODULE_NOT_FOUND` or `ERR_MODULE_NOT_FOUND` **and** the error message names `@r3b1s/pi-repair-layer` as a quoted module specifier — matched as an opening quote immediately followed by the package name (`'@r3b1s/pi-repair-layer`), with no trailing quote so it matches both the bare-package error (`'@r3b1s/pi-repair-layer'`) and the full-subpath error (`'@r3b1s/pi-repair-layer/pi'`) — rather than as a bare substring, so that the package name appearing only as a `node_modules` path segment does not classify a failure as absence; any other error SHALL be rethrown. The recipe SHALL express this classification as a discrete, exported, unit-testable predicate. The fallback branch SHALL emit a one-line stderr (or debug-channel) note identifying the extension and stating that its tools run unwrapped.
5+
6+
#### Scenario: Package absent
7+
- **WHEN** a consumer following the recipe activates in a pi install where `@r3b1s/pi-repair-layer` is not resolvable
8+
- **THEN** the extension registers its raw tool definition, emits the single fallback note, and activation succeeds
9+
10+
#### Scenario: Package present
11+
- **WHEN** the same consumer activates in an install where the package is resolvable
12+
- **THEN** the extension registers the adapted definition and no fallback note is emitted
13+
14+
#### Scenario: Broken install is not misread as absent
15+
- **WHEN** the dynamic import fails with a module-not-found error naming a different (transitive) module
16+
- **THEN** the recipe rethrows instead of silently registering the unwrapped definition
17+
18+
#### Scenario: Path-segment name does not cause a false positive
19+
- **WHEN** the classification predicate is given a module-not-found error whose message names a different missing module but includes `@r3b1s/pi-repair-layer` only as a `node_modules` path segment (e.g. `Cannot find module 'typebox/value' from '.../node_modules/@r3b1s/pi-repair-layer/...'`)
20+
- **THEN** the predicate does not classify the failure as absence and the recipe rethrows
21+
22+
### Requirement: Optional-consumer fixture and smoke coverage
23+
The repository SHALL contain an optional-consumer fixture implementing the documented recipe, and the package smoke test SHALL exercise the recipe in a clean project in both states: with the packed package installed (adapter branch taken) and without it (fallback branch taken, fallback note emitted, raw definition registered). The fixture SHALL express the absence classification as an exported predicate, and the repository SHALL contain a unit test that feeds that predicate the documented module-not-found shapes — including a path-bearing transitive-missing error — and asserts that only genuine package absence is classified as absent.
24+
25+
#### Scenario: Smoke test covers the absent branch
26+
- **WHEN** the package smoke test runs the optional-consumer fixture in a clean project without installing the package
27+
- **THEN** the fixture activates successfully, reports the fallback branch, and the run fails if the fallback note is missing or an error escapes
28+
29+
#### Scenario: Predicate rejects a path-bearing transitive-missing error
30+
- **WHEN** the unit test invokes the exported absence predicate with a synthetic module-not-found error naming a transitive module while embedding `@r3b1s/pi-repair-layer` only as a path segment
31+
- **THEN** the predicate returns that the package is not absent
32+
33+
#### Scenario: Predicate accepts the documented absent-package shapes
34+
- **WHEN** the unit test invokes the exported absence predicate with the native-ESM bare-package error and the jiti/compiled-binary full-subpath error
35+
- **THEN** the predicate returns that the package is absent for each

0 commit comments

Comments
 (0)