mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-20 16:47:59 +02:00
fix(pi): recognize @scope/pi-subagents in /ecc-doctor companion check
The /ecc-doctor false-negative for users who installed @tintinweb/pi-subagents (a working, separately-published subagents implementation) because matching was a bare exact string match against COMPANION_PACKAGES. Matching is now asymmetric: - exact match first (stable reported name when both present) - unscoped entry also satisfied by @scope/<bare-name> (via @ + /suffix check, not plain endsWith, so my-pi-subagents does not qualify) - scoped entry is exact-only (no silent substitution by another publisher's rpiv-todo for @juicesharp/rpiv-todo) - non-exact match emits "satisfied by: <actual>" so user sees the real impl Verification (already performed, not re-run here): - node tests/pi/pi-extension-adapter.test.js: 31/31 pass - Mutation test: the three reverts (old .has loop; delete scoped guard; plain endsWith) each fail the new test - Real settings.json with npm:@tintinweb/pi-subagents produces correct "installed pi-subagents" / "satisfied by: @tintinweb/pi-subagents" - Full npm test: 4546/4636 pass, failures byte-identical to clean main Refs: pi-subagents capability, ecc-doctor report
This commit is contained in:
+44
-3
@@ -141,6 +141,10 @@ const DISABLED_VALUES = new Set(["0", "false", "off", "none", "disabled"])
|
||||
/**
|
||||
* Optional Pi companion packages. ECC works without every one of these; they
|
||||
* are reported by `/ecc-doctor` so users can see which extras are available.
|
||||
*
|
||||
* These are capability names, not exact install specs. See
|
||||
* `findInstalledCompanion` for how an entry is matched against what Pi has
|
||||
* actually installed.
|
||||
*/
|
||||
const COMPANION_PACKAGES = [
|
||||
"pi-subagents",
|
||||
@@ -475,6 +479,41 @@ function normalizePiPackageName(entry: unknown): string | undefined {
|
||||
return versionAt > 0 ? spec.slice(0, versionAt) : spec
|
||||
}
|
||||
|
||||
/**
|
||||
* The installed package satisfying a companion entry, or undefined if none is.
|
||||
*
|
||||
* An exact name match is the ordinary case. An UNSCOPED companion entry is
|
||||
* also satisfied by a scoped package with the same bare name --
|
||||
* `@tintinweb/pi-subagents` satisfies `pi-subagents`. The subagents capability
|
||||
* is published to npm by more than one maintainer under that same bare name,
|
||||
* and a user running a scoped fork has the capability installed by any
|
||||
* meaning of the word; reporting "not installed" at them while its tools are
|
||||
* live in their session is a false negative, and the suggested
|
||||
* `pi install npm:pi-subagents` would push them into installing a second
|
||||
* extension that registers the same tool names.
|
||||
*
|
||||
* A SCOPED companion entry is matched exactly, because there the scope is
|
||||
* part of the identity the entry names, not incidental packaging.
|
||||
*/
|
||||
function findInstalledCompanion(companion: string, installed: Set<string>): string | undefined {
|
||||
if (installed.has(companion)) {
|
||||
return companion
|
||||
}
|
||||
|
||||
if (companion.startsWith("@")) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
const scopedSuffix = `/${companion}`
|
||||
for (const name of installed) {
|
||||
if (name.startsWith("@") && name.endsWith(scopedSuffix)) {
|
||||
return name
|
||||
}
|
||||
}
|
||||
|
||||
return undefined
|
||||
}
|
||||
|
||||
function countDirectories(dir: string): number {
|
||||
try {
|
||||
return fs.readdirSync(dir, { withFileTypes: true }).filter(entry => entry.isDirectory()).length
|
||||
@@ -547,10 +586,12 @@ function buildDoctorReport(ctx: ExtensionContext): string {
|
||||
|
||||
const installed = listInstalledPiPackages(ctx.cwd)
|
||||
for (const name of COMPANION_PACKAGES) {
|
||||
const present = installed.has(name)
|
||||
lines.push(` ${present ? "installed " : "not installed"} ${name}`)
|
||||
if (!present) {
|
||||
const match = findInstalledCompanion(name, installed)
|
||||
lines.push(` ${match ? "installed " : "not installed"} ${name}`)
|
||||
if (!match) {
|
||||
lines.push(` install with: pi install npm:${name}`)
|
||||
} else if (match !== name) {
|
||||
lines.push(` satisfied by: ${match}`)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -188,6 +188,32 @@ function readInstalledPackageNames(settingsFile) {
|
||||
return names
|
||||
}
|
||||
|
||||
/**
|
||||
* Mirror of the adapter's `findInstalledCompanion` (same file, same matching
|
||||
* rule) so the exact-match and unscoped-satisfied-by-scoped cases can be
|
||||
* exercised directly without importing the TypeScript source. This copy
|
||||
* proves the *behavior* below is correct, but a copy cannot detect the real
|
||||
* adapter's rule drifting out from under it. The source-text assertions in
|
||||
* the "companion package detection tolerates a scoped fork" test below read
|
||||
* the real `findInstalledCompanion` text out of `.pi/extensions/index.ts` and
|
||||
* pin its actual guards directly.
|
||||
*/
|
||||
function findInstalledCompanion(companion, installed) {
|
||||
if (installed.has(companion)) {
|
||||
return companion
|
||||
}
|
||||
if (companion.startsWith("@")) {
|
||||
return undefined
|
||||
}
|
||||
const scopedSuffix = `/${companion}`
|
||||
for (const name of installed) {
|
||||
if (name.startsWith("@") && name.endsWith(scopedSuffix)) {
|
||||
return name
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Parses the `PORTABLE_RULE_FILES` array literal out of `.pi/extensions/index.ts`
|
||||
* by text, so the real-filesystem-existence test and the `loadPortableRules`
|
||||
@@ -1135,6 +1161,146 @@ async function main() {
|
||||
}
|
||||
}],
|
||||
|
||||
["companion package detection tolerates a scoped fork (source contract): the unscoped-entry fallback exists, scoped entries stay exact, and the doctor loop reports what satisfied the entry", () => {
|
||||
const matchStart = extensionSource.indexOf("function findInstalledCompanion")
|
||||
assert.ok(
|
||||
matchStart !== -1,
|
||||
"expected .pi/extensions/index.ts to define a function named findInstalledCompanion; " +
|
||||
"a bare installed.has(name) check reports an installed scoped fork such as " +
|
||||
"@tintinweb/pi-subagents as missing, and then tells the user to run " +
|
||||
"`pi install npm:pi-subagents`, which would put a SECOND extension registering " +
|
||||
"the same tool names into their session"
|
||||
)
|
||||
const nextFunctionStart = extensionSource.indexOf("\nfunction ", matchStart + 1)
|
||||
const matchSource =
|
||||
nextFunctionStart === -1
|
||||
? extensionSource.slice(matchStart)
|
||||
: extensionSource.slice(matchStart, nextFunctionStart)
|
||||
|
||||
assert.ok(
|
||||
/if\s*\(\s*installed\.has\(\s*companion\s*\)\s*\)/.test(matchSource),
|
||||
"expected findInstalledCompanion in .pi/extensions/index.ts to check the exact name " +
|
||||
"first; an exact install is the ordinary case and must not be routed through the " +
|
||||
"scoped-fork scan"
|
||||
)
|
||||
assert.ok(
|
||||
/if\s*\(\s*companion\.startsWith\(\s*["'`]@["'`]\s*\)\s*\)\s*\{\s*return undefined/.test(
|
||||
matchSource
|
||||
),
|
||||
"expected findInstalledCompanion in .pi/extensions/index.ts to bail out for a SCOPED " +
|
||||
"companion entry before the fallback; for an entry like " +
|
||||
"@juicesharp/rpiv-todo the scope is part of the identity ECC is naming, so some " +
|
||||
"other publisher's rpiv-todo must not silently satisfy it"
|
||||
)
|
||||
assert.ok(
|
||||
/name\.startsWith\(\s*["'`]@["'`]\s*\)\s*&&\s*name\.endsWith\(\s*scopedSuffix\s*\)/.test(
|
||||
matchSource
|
||||
),
|
||||
"expected findInstalledCompanion in .pi/extensions/index.ts to match an installed " +
|
||||
"scoped package by the '@scope/' + exact bare name shape; matching on endsWith " +
|
||||
"alone would let a package named my-pi-subagents satisfy the pi-subagents entry"
|
||||
)
|
||||
|
||||
const withoutComments = stripComments(extensionSource)
|
||||
assert.ok(
|
||||
!/installed\.has\(name\)/.test(withoutComments),
|
||||
"found a bare installed.has(name) still used as executable code in " +
|
||||
".pi/extensions/index.ts; the /ecc-doctor companion loop must go through " +
|
||||
"findInstalledCompanion so a scoped fork is not reported as missing"
|
||||
)
|
||||
assert.ok(
|
||||
/satisfied by/.test(extensionSource),
|
||||
"expected the /ecc-doctor companion loop in .pi/extensions/index.ts to name the " +
|
||||
"package that satisfied an entry when it is not an exact match; reporting a " +
|
||||
"bare 'installed' for @tintinweb/pi-subagents under the pi-subagents line hides " +
|
||||
"which implementation is actually loaded, which is the first thing to know when " +
|
||||
"its behavior differs from the unscoped package's"
|
||||
)
|
||||
}],
|
||||
|
||||
["companion package matching (behavioral mirror): an unscoped entry is satisfied by a scoped fork, a scoped entry is matched exactly", () => {
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("pi-subagents", new Set(["pi-subagents"])),
|
||||
"pi-subagents",
|
||||
"expected an exactly-installed companion to be reported as itself"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("pi-subagents", new Set(["@tintinweb/pi-subagents"])),
|
||||
"@tintinweb/pi-subagents",
|
||||
"expected a scoped fork to satisfy the unscoped pi-subagents entry; the subagents " +
|
||||
"capability is published under that bare name by more than one maintainer, and a " +
|
||||
"user running the scoped one has working Agent/SubagentWorkflow tools in session " +
|
||||
"while /ecc-doctor was calling it missing"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("pi-subagents", new Set(["pi-subagents", "@tintinweb/pi-subagents"])),
|
||||
"pi-subagents",
|
||||
"expected the exact match to win when both are installed, so the reported name is " +
|
||||
"stable rather than depending on Set iteration order"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("pi-subagents", new Set(["my-pi-subagents"])),
|
||||
undefined,
|
||||
"expected an unscoped package that merely ENDS WITH the companion name to not " +
|
||||
"satisfy it; only a @scope/ prefix counts"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("pi-subagents", new Set(["@acme/my-pi-subagents"])),
|
||||
undefined,
|
||||
"expected a scoped package whose bare name merely ends with the companion name to " +
|
||||
"not satisfy it; the segment after the scope must equal the companion name"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("@juicesharp/rpiv-todo", new Set(["@juicesharp/rpiv-todo"])),
|
||||
"@juicesharp/rpiv-todo",
|
||||
"expected an exactly-installed scoped companion to be reported as itself"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("@juicesharp/rpiv-todo", new Set(["@someoneelse/rpiv-todo"])),
|
||||
undefined,
|
||||
"expected a DIFFERENT scope to not satisfy a scoped companion entry; ECC names that " +
|
||||
"scope deliberately, so relaxing this direction would report an unrelated " +
|
||||
"publisher's package as the one ECC documents"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("@juicesharp/rpiv-todo", new Set(["rpiv-todo"])),
|
||||
undefined,
|
||||
"expected an unscoped package to not satisfy a scoped companion entry"
|
||||
)
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion("pi-subagents", new Set()),
|
||||
undefined,
|
||||
"expected an empty install set to satisfy nothing"
|
||||
)
|
||||
}],
|
||||
|
||||
["every COMPANION_PACKAGES entry this repo ships is still resolvable by the matcher it is checked with", () => {
|
||||
const constStart = extensionSource.indexOf("const COMPANION_PACKAGES")
|
||||
assert.ok(
|
||||
constStart !== -1,
|
||||
"expected to find a COMPANION_PACKAGES array literal in .pi/extensions/index.ts"
|
||||
)
|
||||
const constEnd = extensionSource.indexOf("]", constStart)
|
||||
const companions = Array.from(
|
||||
extensionSource.slice(constStart, constEnd + 1).matchAll(/["'`](@?[\w./-]+)["'`]/g)
|
||||
).map(match => match[1])
|
||||
|
||||
assert.ok(
|
||||
companions.length > 0,
|
||||
"expected to parse at least one companion package name out of COMPANION_PACKAGES"
|
||||
)
|
||||
|
||||
for (const companion of companions) {
|
||||
assert.strictEqual(
|
||||
findInstalledCompanion(companion, new Set([companion])),
|
||||
companion,
|
||||
`expected the companion entry ${companion} to be recognized when it is installed ` +
|
||||
"under exactly its own name; an entry the matcher cannot resolve would be " +
|
||||
"reported as permanently missing no matter what the user installs"
|
||||
)
|
||||
}
|
||||
}],
|
||||
|
||||
// ---- Group 4: engineering-rules injection -------------------------
|
||||
|
||||
["PORTABLE_RULE_FILES lists exactly ECC's 7 Pi-portable rule files and excludes the 3 Claude-Code-only ones", () => {
|
||||
|
||||
Reference in New Issue
Block a user