mirror of
https://github.com/CopilotKit/CopilotKit.git
synced 2026-09-14 16:26:20 +08:00
test(skills): guard the repository facts the procedure skills name (#7051)
## What
Tests the six remaining skills on the axis that is cheap to check and
actually broken: **are the facts they state still true?**
The existing guard covers the two entry points, which point at
documentation, by resolving every docs path they name. The Inspector and
Intelligence skills point at the *repository* — an Nx target, a
dev-server port, a lab scenario id, a landing-page source file, a
Callout snippet — and nothing checked any of it.
## It was already wrong
`465abb0239` (OSS-948) deleted `open-inspector-step-angular.mdx`, and
said so explicitly:
> the Angular Open Inspector snippet — which asserted "Angular does not
mount Inspector by default" — is deleted in favour of the shared one
every other web frontend already uses
`@copilotkit/angular@0.4.0` auto-mounts a pinned
`@copilotkit/web-inspector`, so there is no install step left to link.
**That commit never touched `pane-map.md`.** So the map kept pointing at
a deleted file, and `inspector-docs` Procedure 3 kept telling agents
"Angular uses the Angular step snippet, which links the Angular
Inspector install page first."
A change to exactly the thing the skill tracks left the skill's own
source of truth stale, silently, for two weeks. Both are corrected here.
## The four assertions
Each has an unambiguous ground truth:
| assertion | ground truth |
| --- | --- |
| every `open-inspector-*.mdx` the pane map names exists |
`snippets/shared/inspector/` |
| every `docs/…mdx` page the pane map names exists | the content tree |
| every `showcase/shell-docs/src/…` file `intelligence-docs` says to
edit exists | the repo |
| the Nx target, port, scenario ids and query keys `inspector-workbench`
names resolve | `project.json`, `threads-state-lab.ts` |
## What is deliberately not asserted
**Which panes the Inspector ships.** Panes are not enumerated as data
anywhere in `packages/web-inspector`, whose entry point is a single
~14,700-line module, so matching a pane label against source proves
nothing in either direction.
A first draft of this suite tried it and **passed while the map really
was wrong**: `pane-map.md` lists "Pop-out window" under Unshipped, but
`src/lib/pop-out.ts` is imported by the package entry, carries live
`popOut` state in `index.ts`, and has its own test suite. My slug match
looked for `pop-out-window`, found nothing, and went green. That is
worse than not testing it, because a green run reads as confirmation.
So that check is gone, the reasoning is in a comment in the suite, and
**the Pop-out row is left as-is for the Inspector owner to rule on**
rather than guessed at. Making that direction testable needs a pane
registry in the package, not a cleverer regex here.
## Testing
```
$ vitest run scripts/__tests__/public-skill-drift.test.ts scripts/__tests__/sync-plugin-skills.test.ts
Test Files 2 passed (2)
Tests 20 passed (20)
```
`pnpm check:plugin-skills` → `plugin skill mirror in sync`. `pnpm
check:intelligence-env-names` → `Intelligence env var names and hosts
are canonical`. `oxfmt --check` clean (it reported two files needing
format; ran `--write`, re-checked clean, re-ran tests).
**Mutation-checked, since a check that has never failed is not a
check.** Each mutation produces exactly one failing test, and the
restored tree is clean:
| mutation | failures |
| --- | ---: |
| baseline | 0 |
| `docs/meta.json` → `docs/NOPE.json` in `intelligence-docs` | 1 |
| port `5177` → `9999` in `inspector-workbench` | 1 |
| `scenario=pro-enabled-existing` → `no-such-scenario` | 1 |
| nx target `dev:standalone` → `dev:gone` | 1 |
| `docs/inspector.mdx` → `docs/no-such-page.mdx` in the pane map | 1 |
| restored | 0 |
The Angular finding is the suite's own first catch: before the fix it
failed with exactly `[ 'open-inspector-step-angular.mdx' ]` and no false
positives. An earlier iteration also flagged `inspector.mdx` and
`react-native.mdx` — page references from a different table — which is
why the snippet pattern is anchored on the `open-inspector-` prefix.
This runs in `plugin-skills-check.yml`, which already triggers on
`skills/**`.
## Scope
This is the cheap half of testing a skill — whether it is still true.
Whether a skill measurably *helps* an agent is the other half, and that
needs the lift eval in #5689, which is parked.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Documentation**
- Updated Angular Inspector quickstart guidance to use the shared web
Inspector steps.
- Clarified that Angular automatically mounts the pinned web Inspector,
so no separate installation step is required.
- Updated the Inspector pane map to reflect the shared guidance and
revised installation notes.
- **Tests**
- Added checks to keep procedure documentation aligned with current
Inspector, Intelligence, and Workbench configuration details.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -107,3 +107,117 @@ describe("packaged skills point at pages that exist", () => {
|
||||
expect(inventories).toEqual(["skills/copilotkit-channels/sources.md"]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The procedure skills rot differently from the two entry points.
|
||||
*
|
||||
* `copilotkit` and `copilotkit-cli` point at documentation, so their failure
|
||||
* mode is a dead docs path — guarded above. The Inspector and Intelligence
|
||||
* skills instead name concrete repository facts: an Nx target, a dev-server
|
||||
* port, a lab scenario id, a landing-page source file, a Callout snippet.
|
||||
* Every one of those is a claim that can quietly stop being true, and when it
|
||||
* does the skill sends an agent somewhere that no longer exists.
|
||||
*
|
||||
* Only claims with an unambiguous ground truth are asserted. Which panes the
|
||||
* Inspector *ships* is deliberately not asserted: panes are not enumerated as
|
||||
* data anywhere in `packages/web-inspector`, whose entry point is one
|
||||
* fourteen-thousand-line module, so matching a pane label against source
|
||||
* proves nothing in either direction. A first draft of this suite tried it and
|
||||
* passed while the map really was wrong — "Pop-out window" is listed unshipped
|
||||
* although `src/lib/pop-out.ts` is imported by the package entry — which is
|
||||
* worse than not testing it, because a green run reads as confirmation.
|
||||
* Making that direction testable needs a pane registry in the package, not a
|
||||
* cleverer regex here.
|
||||
*/
|
||||
describe("procedure skills name repository facts that still hold", () => {
|
||||
/** Reads a skill file, or fails loudly rather than silently passing. */
|
||||
function skill(path: string): string {
|
||||
const full = resolve(skillsDir, path);
|
||||
if (!existsSync(full)) throw new Error(`missing skill file: ${path}`);
|
||||
return readFileSync(full, "utf8");
|
||||
}
|
||||
|
||||
it("maps every Inspector pane to a Callout snippet that exists", () => {
|
||||
const map = skill("inspector-docs/references/pane-map.md");
|
||||
// Snippet cells name bare `open-inspector-*.mdx` files, all in one
|
||||
// directory. Anchored on that prefix so the `docs/…mdx` page references in
|
||||
// the surfaces table below are not mistaken for snippets.
|
||||
const named = [
|
||||
...new Set(
|
||||
[...map.matchAll(/(?<![\w/-])(open-inspector-[a-z0-9-]*\.mdx)/g)].map(
|
||||
(m) => m[1],
|
||||
),
|
||||
),
|
||||
];
|
||||
expect(named.length).toBeGreaterThan(0);
|
||||
const dir = resolve(contentDir, "snippets/shared/inspector");
|
||||
expect(named.filter((f) => !existsSync(join(dir, f)))).toEqual([]);
|
||||
});
|
||||
|
||||
it("maps every Inspector pane to docs pages that exist", () => {
|
||||
const map = skill("inspector-docs/references/pane-map.md");
|
||||
const pages = [
|
||||
...new Set(
|
||||
[...map.matchAll(/`(docs\/[A-Za-z0-9_./-]+\.mdx)`/g)].map((m) => m[1]),
|
||||
),
|
||||
];
|
||||
expect(pages.length).toBeGreaterThan(0);
|
||||
expect(pages.filter((f) => !existsSync(resolve(contentDir, f)))).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the Intelligence landing sources it tells you to edit", () => {
|
||||
const md = skill("intelligence-docs/SKILL.md");
|
||||
const paths = [
|
||||
...new Set(
|
||||
[...md.matchAll(/`(showcase\/shell-docs\/src\/[A-Za-z0-9_./-]+)`/g)]
|
||||
.map((m) => m[1])
|
||||
// Trailing-slash entries are directory prose, not editable files.
|
||||
.filter((f) => !f.endsWith("/")),
|
||||
),
|
||||
];
|
||||
expect(paths.length).toBeGreaterThan(0);
|
||||
expect(paths.filter((f) => !existsSync(resolve(root, f)))).toEqual([]);
|
||||
});
|
||||
|
||||
it("names an Inspector workbench target, port and scenario that exist", () => {
|
||||
const md = skill("inspector-workbench/SKILL.md");
|
||||
const project = JSON.parse(
|
||||
readFileSync(
|
||||
resolve(root, "packages/web-inspector/project.json"),
|
||||
"utf8",
|
||||
),
|
||||
) as { targets?: Record<string, unknown> };
|
||||
|
||||
// The Nx target the skill tells the agent to run.
|
||||
const target = md.match(/nx run @copilotkit\/web-inspector:([\w:-]+)/)?.[1];
|
||||
expect(target, "skill names an nx target").toBeDefined();
|
||||
expect(Object.keys(project.targets ?? {})).toContain(target);
|
||||
|
||||
// The port it tells the agent to open, which that target has to pin.
|
||||
const port = md.match(/127\.0\.0\.1:(\d+)/)?.[1];
|
||||
expect(port, "skill names a port").toBeDefined();
|
||||
expect(JSON.stringify(project.targets?.[target as string])).toContain(
|
||||
`--port ${port}`,
|
||||
);
|
||||
|
||||
// The lab scenarios and query keys it tells the agent to load.
|
||||
const lab = readFileSync(
|
||||
resolve(root, "packages/web-inspector/dev/threads-state-lab.ts"),
|
||||
"utf8",
|
||||
);
|
||||
const scenarios = [
|
||||
...new Set(
|
||||
[...md.matchAll(/[?&]scenario=([a-z0-9-]+)/g)].map((m) => m[1]),
|
||||
),
|
||||
];
|
||||
expect(scenarios.length).toBeGreaterThan(0);
|
||||
expect(scenarios.filter((s) => !lab.includes(s))).toEqual([]);
|
||||
for (const key of [
|
||||
...new Set([...md.matchAll(/[?&]([a-z-]+)=1\b/g)].map((m) => m[1])),
|
||||
]) {
|
||||
expect(lab, `lab supports ?${key}=`).toContain(key);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user