Merge pull request #2293 from modelcontextprotocol/v2/feat/2248-skills-phase3
feat: Skills extension phase 3 — CLI methods, TUI pane, resources/directory/read, and the frontmatter cross-check
This commit is contained in:
Cliff Hall
GitHub
commit
fb1dd1f280
51 changed files with 8986 additions and 379 deletions
|
|
@ -108,11 +108,12 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en
|
|||
|
||||
| Option | Description |
|
||||
| ----------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `--method <method>` | MCP method to invoke. Supports `initialize` (connect-only probe → `{serverInfo, protocolVersion, capabilities, instructions}`), `tools/list`, `tools/call`, `resources/list`, `resources/read`, `resources/templates/list`, `prompts/list`, `prompts/get`, `logging/setLevel`, plus catalog-only `servers/list` / `servers/show` (no MCP connect). Stream / session-only methods (e.g. `logging/tail`) are rejected. |
|
||||
| `--method <method>` | MCP method to invoke. Supports `initialize` (connect-only probe → `{serverInfo, protocolVersion, capabilities, instructions}`), `tools/list`, `tools/call`, `resources/list`, `resources/read`, `resources/templates/list`, `prompts/list`, `prompts/get`, `logging/setLevel`, `skills/list`, `skills/get`, `resources/directory/read`, plus catalog-only `servers/list` / `servers/show` (no MCP connect). Stream / session-only methods (e.g. `logging/tail`) are rejected. |
|
||||
| `--tool-name <name>` | Tool name (for `tools/call`). |
|
||||
| `--tool-arg <key=value>` | Tool argument; repeat for multiple. Use `key='{"json":true}'` for JSON. Values are coerced (JSON-parsed, so `count=1` becomes a number). |
|
||||
| `--tool-args-json <json>` | Tool arguments as a single JSON object (e.g. `'{"zip":"10001"}'`). Passed verbatim — no `key=value` coercion, so `"012"` stays a string. Mutually exclusive with `--tool-arg`. |
|
||||
| `--uri <uri>` | Resource URI (for `resources/read`). |
|
||||
| `--uri <uri>` | Resource URI (`resources/read`), directory URI (`resources/directory/read`), or skill URI (`skills/get`). |
|
||||
| `--cursor <cursor>` | Opaque pagination cursor for `resources/directory/read` — pass back the `nextCursor` from the previous page. The listing is not recursive and pages are not aggregated: SEP-2640 gives the cursor to the client, and descending is the caller's job. |
|
||||
| `--prompt-name <name>` | Prompt name (for `prompts/get`). |
|
||||
| `--prompt-args <key=value>` | Prompt arguments; repeat for multiple. |
|
||||
| `--log-level <level>` | Logging level for `logging/setLevel` (e.g. `debug`, `info`). |
|
||||
|
|
@ -121,6 +122,7 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en
|
|||
| `--connect-timeout <ms>` | Connection timeout in ms. Defaults to `15000` for ad-hoc `--server-url`/target runs (so a black-holed host fails fast) and to the file-level timeout for `--catalog`/`--config` runs. `0` disables the timeout. |
|
||||
| `--app-info` | Probe a tool's MCP App UI metadata without invoking it. With `--method tools/call --tool-name <name>`: prints one JSON line (`hasApp`, `resourceUri`, `csp`, `permissions`, `domain`, …) and exits `0` if the tool has an app or `2` (`no_app`) if not. With `--method tools/list`: emits NDJSON — one app-info line per tool over a single connection. |
|
||||
| `--strict` | With `--method tools/list`: report tool-schema portability problems in full (path, issue, suggested fix) on stderr, and exit `6` if any is error-severity. Without it, a one-line count is printed instead. See [Schema portability](#schema-portability---strict). |
|
||||
| `--verify` | With `--method skills/list` or `--method skills/get`: run the SEP-2640 conformance, digest and frontmatter checks over the skills returned, emit one JSON report per skill on stdout, and exit `7` if any fails. See [Skill verification](#skill-verification---verify). |
|
||||
| `--format <text\|json>` | Output format. `text` (default) pretty-prints the result. `json` emits a single JSON object on stdout (`{ "result": … }`, plus `{ "appInfo": … }` as a sibling key for App tools) with no banners, so the whole output pipes cleanly into `jq`. |
|
||||
| `--relogin` | Delete stored OAuth for this server URL from the shared store before connect; interactive login still only runs if the server requires auth. Requires an HTTP/SSE URL (rejected for stdio). Conflicts with `--stored-auth-only` / `--use-stored-auth` / `--wait-for-auth` / catalog short-circuits. |
|
||||
| `--no-revoke` | With `--relogin`, skip the [RFC 7009](https://datatracker.ietf.org/doc/html/rfc7009) revocation request that would otherwise end the grant at the authorization server when the local state is deleted. The per-server `oauth.revokeOnClear` setting is the persistent form of the same opt-out; either one is enough to skip it. See [Revoking on `--relogin`](#revoking-on---relogin). |
|
||||
|
|
@ -333,6 +335,91 @@ mcp-inspector --cli --transport http --server-url https://api.example/mcp \
|
|||
--wait-for-auth 120 --method tools/list
|
||||
```
|
||||
|
||||
#### Skill verification (`--verify`)
|
||||
|
||||
SEP-2640 puts real obligations on whoever consumes a skill: verify each fetched
|
||||
file against the digest its manifest advertised, check that the served
|
||||
`SKILL.md`'s frontmatter matches the one the listing advertised, and honour the
|
||||
per-skill limits. `--verify` runs all of them over a whole catalog and turns the
|
||||
answer into an exit code, so a server author can gate CI on it:
|
||||
|
||||
```sh
|
||||
mcp-inspector --cli <server> --method skills/list --verify
|
||||
```
|
||||
|
||||
Stdout is **NDJSON, one report per skill**, in listing order:
|
||||
|
||||
```json
|
||||
{
|
||||
"uri": "skill://tampered-notes/SKILL.md",
|
||||
"name": "tampered-notes",
|
||||
"conformance": [],
|
||||
"frontmatter": [],
|
||||
"files": [
|
||||
{ "uri": "skill://tampered-notes/SKILL.md", "status": "verified", "…": "…" },
|
||||
{ "uri": "skill://tampered-notes/notes.md", "status": "mismatch", "…": "…" }
|
||||
],
|
||||
"ok": false
|
||||
}
|
||||
```
|
||||
|
||||
Stderr gets a one-line summary, so a reader who piped stdout into `jq` still
|
||||
sees the verdict — and then, on a failing run, the ordinary
|
||||
[`ErrorEnvelope`](#exit-codes--error-envelopes) line that **every** non-zero
|
||||
exit writes. Two stderr lines on failure, one on success, which is the same
|
||||
shape `--strict` produces and is why the envelope is not suppressed here: a
|
||||
caller branching on `.code` should not have to special-case this command.
|
||||
`--method skills/get --uri <skill>` verifies exactly one skill, in the same
|
||||
shape.
|
||||
|
||||
**What fails the run.** Three outcomes, three exit codes, because "this skill is
|
||||
wrong" and "this skill could not be fully checked" are different answers:
|
||||
|
||||
| `outcome` | Exit | When |
|
||||
| --- | --- | --- |
|
||||
| `verified` | `0` | Everything was checked and everything passed. |
|
||||
| `failed` | `7` | Something SEP-2640 makes a MUST was broken — an error-severity finding, a digest or size mismatch, or an unreadable manifest file. |
|
||||
| `incomplete` | `8` | Nothing checked was wrong, but the read bounds stopped the walk before it finished. See `incomplete` in the report for the reason. |
|
||||
|
||||
**The run is bounded, and says when a bound bit.** Three limits, all reported as
|
||||
`incomplete` (`8`) rather than as a pass or a failure, because an entry that was
|
||||
not read has not been cleared of anything:
|
||||
|
||||
| Bound | Limit | Why |
|
||||
| --- | --- | --- |
|
||||
| Per skill | 512 manifest entries / 16 MiB | SEP-2640's own interoperability limits. |
|
||||
| Per skill, on the wire | 16 MiB actually served | The declared sizes are server-controlled; this one cannot be lied past. |
|
||||
| Per run | 256 skills / 64 MiB | SEP-2640 bounds a skill and deliberately does not bound a *catalog*. Every entry costs at least one `resources/read`, so without this a large listing — hostile or merely big — is unbounded work against the tool inspecting it. |
|
||||
|
||||
The run bound is this tool's, not the spec's. A skill past it is still reported,
|
||||
with its static conformance findings and an `incomplete` reason saying nothing
|
||||
about its files was checked; verify it on its own with `--method skills/get
|
||||
--uri <skill>` to get a verdict for it.
|
||||
|
||||
A **warning** never produces `7`. That distinction matters most for `resources: "dynamic"`, which is a
|
||||
*conforming* wire form for generated content: it means integrity cannot be
|
||||
verified, which is worth reporting, but failing CI for it would tell server
|
||||
authors their valid skill is broken.
|
||||
|
||||
**Three checks, three different jobs**, and the second is the one nothing else
|
||||
covers:
|
||||
|
||||
- **Conformance** — structural checks against the entry as listed (name grammar,
|
||||
the name/URI invariant, digest and size formats, manifest completeness, the
|
||||
interoperability limits).
|
||||
- **Frontmatter** — the served `SKILL.md`'s own YAML frontmatter, compared field
|
||||
by field against the frontmatter the listing advertised. A digest cannot cover
|
||||
this: it is taken over the bytes the server served, so it proves the file was
|
||||
not altered in transit and says nothing about whether the *listing* described
|
||||
it honestly. A server can advertise one description, serve another, and pass
|
||||
every digest check.
|
||||
- **Files** — each manifest entry fetched and hashed. Reads are sequential: a
|
||||
conforming manifest may declare 512 entries, and a parallel walk would open
|
||||
512 `resources/read` calls against the server under test.
|
||||
|
||||
A read failure is recorded against the file it happened on and the walk
|
||||
continues, so one unreadable file never hides the findings after it.
|
||||
|
||||
## Exit codes & error envelopes
|
||||
|
||||
Every non-zero exit maps to a stable failure class, so a programmatic caller
|
||||
|
|
@ -348,6 +435,8 @@ prose from stderr:
|
|||
| `4` | Server unreachable (DNS, connection refused, timeout, `fetch failed`). |
|
||||
| `5` | Tool error (`tools/call` returned `isError:true`, or the tool was not found). |
|
||||
| `6` | `--strict` found an error-severity tool-schema portability problem (`schema_unportable` — the schema is valid JSON Schema, just not portable). |
|
||||
| `7` | `--verify` found a SEP-2640 violation (`skills_nonconformant` — a conformance error, a digest or size mismatch, or an unreadable manifest file). |
|
||||
| `8` | `--verify` could not check the whole catalog (`skills_incomplete` — the read bounds stopped the walk). The server broke no **MUST**: the 512-entry and 16 MiB limits are `SHOULD NOT`, and hosts may support more. A job that tolerates oversized catalogs can allow `8` and still fail on `7`. |
|
||||
|
||||
On any non-zero exit the CLI also writes a single JSON line to **stderr** — the
|
||||
`ErrorEnvelope`:
|
||||
|
|
|
|||
335
clients/cli/__tests__/run-method-skills.test.ts
Normal file
335
clients/cli/__tests__/run-method-skills.test.ts
Normal file
|
|
@ -0,0 +1,335 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import { runMethod } from "../src/handlers/run-method.js";
|
||||
import { summarizeSkillVerification } from "../src/handlers/skills-verify.js";
|
||||
import { EXIT_CODES } from "../src/error-handler.js";
|
||||
import type { InspectorClient } from "@inspector/core/mcp/index.js";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import { sha256Digest } from "@inspector/core/mcp/skills.js";
|
||||
import type { SkillVerifyReport } from "@inspector/core/mcp/skillsVerification.js";
|
||||
|
||||
/**
|
||||
* The three SEP-2640 methods the CLI gained in #2248, plus `--verify`.
|
||||
*
|
||||
* The store's cursor walk and the verification checks are covered where they
|
||||
* live (`managedSkillsState.test.ts`, `skillsVerification.test.ts`); what these
|
||||
* pin is the dispatcher's own decisions — which method reaches which client
|
||||
* call, what shape leaves as a result, and when the report sets a non-zero exit
|
||||
* code.
|
||||
*/
|
||||
const SKILL_MD = "---\nname: demo\ndescription: A demo\n---\n\n# Demo\n";
|
||||
|
||||
async function cleanEntry(): Promise<SkillEntry> {
|
||||
const bytes = new TextEncoder().encode(SKILL_MD);
|
||||
return {
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: { name: "demo", description: "A demo" },
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://demo/SKILL.md",
|
||||
digest: await sha256Digest(bytes),
|
||||
size: bytes.byteLength,
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
function mockClient(overrides: Record<string, unknown> = {}): InspectorClient {
|
||||
return {
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn(),
|
||||
getStatus: vi.fn().mockReturnValue("connected"),
|
||||
getSkillsExtension: vi.fn().mockReturnValue({ directoryRead: true }),
|
||||
listSkills: vi.fn().mockResolvedValue({ skills: [] }),
|
||||
getSkill: vi.fn(),
|
||||
getSkillResult: vi.fn(),
|
||||
readResourceDirectory: vi.fn(),
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: { contents: [{ uri: "skill://demo/SKILL.md", text: SKILL_MD }] },
|
||||
}),
|
||||
...overrides,
|
||||
} as unknown as InspectorClient;
|
||||
}
|
||||
|
||||
describe("runMethod skills dispatch (#2248)", () => {
|
||||
it("returns the walked list for skills/list", async () => {
|
||||
const entry = await cleanEntry();
|
||||
const client = mockClient({
|
||||
listSkills: vi.fn().mockResolvedValue({ skills: [entry] }),
|
||||
});
|
||||
const outcome = await runMethod(client, { method: "skills/list" });
|
||||
expect(outcome).toEqual({
|
||||
kind: "result",
|
||||
result: { skills: [entry] },
|
||||
appInfo: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects skills/list with a usage exit code when the server declares no extension", async () => {
|
||||
// The store answers "no extension" with an empty list, which is right for
|
||||
// a UI that must render something and wrong for a CLI: "no skills" and
|
||||
// "does not serve skills" are answers a script has to tell apart.
|
||||
const client = mockClient({
|
||||
getSkillsExtension: vi.fn().mockReturnValue(undefined),
|
||||
});
|
||||
await expect(runMethod(client, { method: "skills/list" })).rejects.toThrow(
|
||||
/does not declare/i,
|
||||
);
|
||||
await expect(
|
||||
runMethod(client, { method: "skills/list" }),
|
||||
).rejects.toMatchObject({ exitCode: EXIT_CODES.USAGE });
|
||||
});
|
||||
|
||||
it("rejects skills/get too when the server declares no extension", async () => {
|
||||
// Declaring the extension commits a server to BOTH methods, so gating one
|
||||
// and not the other is inconsistent with the thing being checked — and an
|
||||
// undeclared server's -32601 is indistinguishable to a script from the
|
||||
// -32602 a declared server returns for a URI it does not serve.
|
||||
const client = mockClient({
|
||||
getSkillsExtension: vi.fn().mockReturnValue(undefined),
|
||||
getSkillResult: vi.fn(),
|
||||
});
|
||||
await expect(
|
||||
runMethod(client, { method: "skills/get", uri: "skill://x/SKILL.md" }),
|
||||
).rejects.toMatchObject({ exitCode: EXIT_CODES.USAGE });
|
||||
expect(client.getSkillResult).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps the { skill } envelope on skills/get", async () => {
|
||||
// The client unwraps it for callers that want the entry; a CLI whose
|
||||
// contract is "print the result" must not quietly reshape the wire form.
|
||||
const entry = await cleanEntry();
|
||||
const client = mockClient({
|
||||
getSkillResult: vi.fn().mockResolvedValue({ skill: entry }),
|
||||
});
|
||||
const outcome = await runMethod(client, {
|
||||
method: "skills/get",
|
||||
uri: entry.uri,
|
||||
});
|
||||
expect(outcome).toMatchObject({ result: { skill: entry } });
|
||||
});
|
||||
|
||||
it("prints the whole skills/get envelope, not just the entry", async () => {
|
||||
// SEP-2640 leaves it open whether this result carries `ttlMs`/`cacheScope`,
|
||||
// so a server may send them — and unwrapping to the entry discarded exactly
|
||||
// those, from a path whose contract is "print the result" (Copilot).
|
||||
const entry = await cleanEntry();
|
||||
const envelope = {
|
||||
skill: entry,
|
||||
resultType: "complete",
|
||||
ttlMs: 60,
|
||||
cacheScope: "public",
|
||||
};
|
||||
const client = mockClient({
|
||||
getSkillResult: vi.fn().mockResolvedValue(envelope),
|
||||
});
|
||||
const outcome = await runMethod(client, {
|
||||
method: "skills/get",
|
||||
uri: entry.uri,
|
||||
});
|
||||
expect(outcome).toMatchObject({ result: envelope });
|
||||
});
|
||||
|
||||
it("requires --uri for skills/get", async () => {
|
||||
await expect(
|
||||
runMethod(mockClient(), { method: "skills/get" }),
|
||||
).rejects.toThrow(/URI is required/);
|
||||
});
|
||||
|
||||
it("requires --uri for resources/directory/read", async () => {
|
||||
await expect(
|
||||
runMethod(mockClient(), { method: "resources/directory/read" }),
|
||||
).rejects.toThrow(/URI is required/);
|
||||
});
|
||||
|
||||
it("returns one page of resources/directory/read and forwards the cursor", async () => {
|
||||
// One page, not a walk: the SEP says the listing is not recursive and the
|
||||
// client descends, so aggregating here would present a subtree as a
|
||||
// directory.
|
||||
const page = { resources: [], nextCursor: "2" };
|
||||
const readResourceDirectory = vi.fn().mockResolvedValue(page);
|
||||
const client = mockClient({ readResourceDirectory });
|
||||
const outcome = await runMethod(client, {
|
||||
method: "resources/directory/read",
|
||||
uri: "skill://demo",
|
||||
cursor: "1",
|
||||
});
|
||||
expect(readResourceDirectory).toHaveBeenCalledWith(
|
||||
"skill://demo",
|
||||
"1",
|
||||
undefined,
|
||||
);
|
||||
expect(outcome).toMatchObject({ result: page });
|
||||
});
|
||||
|
||||
it("--verify emits one NDJSON report per skill with no exit code when clean", async () => {
|
||||
const entry = await cleanEntry();
|
||||
const client = mockClient({
|
||||
listSkills: vi.fn().mockResolvedValue({ skills: [entry] }),
|
||||
});
|
||||
const outcome = await runMethod(client, {
|
||||
method: "skills/list",
|
||||
verify: true,
|
||||
});
|
||||
expect(outcome.kind).toBe("ndjson");
|
||||
if (outcome.kind !== "ndjson") throw new Error("unreachable");
|
||||
expect(outcome.lines).toHaveLength(1);
|
||||
expect((outcome.lines[0] as SkillVerifyReport).ok).toBe(true);
|
||||
expect(outcome.summary).toMatch(/no conformance errors/);
|
||||
expect(outcome.exitCode).toBeUndefined();
|
||||
});
|
||||
|
||||
it("--verify sets the skills exit code when a skill fails", async () => {
|
||||
const entry = await cleanEntry();
|
||||
const client = mockClient({
|
||||
listSkills: vi.fn().mockResolvedValue({ skills: [entry] }),
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: {
|
||||
contents: [{ uri: entry.uri, text: "totally different bytes" }],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const outcome = await runMethod(client, {
|
||||
method: "skills/list",
|
||||
verify: true,
|
||||
});
|
||||
if (outcome.kind !== "ndjson") throw new Error("unreachable");
|
||||
expect(outcome.exitCode).toBe(EXIT_CODES.SKILL_NONCONFORMANT);
|
||||
// Its own code, not SCHEMA_UNPORTABLE — an unportable tool schema and a
|
||||
// tampered skill digest are different CI failures.
|
||||
expect(EXIT_CODES.SKILL_NONCONFORMANT).not.toBe(
|
||||
EXIT_CODES.SCHEMA_UNPORTABLE,
|
||||
);
|
||||
});
|
||||
|
||||
it("exits 8, not 7, when the walk was only truncated", async () => {
|
||||
// SEP-2640 states the read limits as SHOULD NOT and lets hosts support
|
||||
// more, so exiting `SKILL_NONCONFORMANT` would call a conforming server
|
||||
// nonconformant — while exiting 0 would report success for entries nobody
|
||||
// fetched (Copilot).
|
||||
const md = "---\nname: many\ndescription: Big\n---\n\n# many\n";
|
||||
const enc = new TextEncoder();
|
||||
const selfDigest = await sha256Digest(enc.encode(md));
|
||||
const bodyDigest = await sha256Digest(enc.encode("x"));
|
||||
const entry: SkillEntry = {
|
||||
uri: "skill://many/SKILL.md",
|
||||
frontmatter: { name: "many", description: "Big" },
|
||||
resources: Array.from({ length: 600 }, (_, i) =>
|
||||
i === 0
|
||||
? {
|
||||
uri: "skill://many/SKILL.md",
|
||||
digest: selfDigest,
|
||||
size: enc.encode(md).byteLength,
|
||||
}
|
||||
: { uri: `skill://many/f${i}.md`, digest: bodyDigest, size: 1 },
|
||||
),
|
||||
};
|
||||
const client = mockClient({
|
||||
listSkills: vi.fn().mockResolvedValue({ skills: [entry] }),
|
||||
readResource: vi.fn(async (uri: string) => ({
|
||||
result: {
|
||||
contents: [{ uri, text: uri.endsWith("/SKILL.md") ? md : "x" }],
|
||||
},
|
||||
})),
|
||||
});
|
||||
const outcome = await runMethod(client, {
|
||||
method: "skills/list",
|
||||
verify: true,
|
||||
});
|
||||
if (outcome.kind !== "ndjson") throw new Error("unreachable");
|
||||
expect(outcome.exitCode).toBe(EXIT_CODES.SKILL_INCOMPLETE);
|
||||
expect(EXIT_CODES.SKILL_INCOMPLETE).not.toBe(
|
||||
EXIT_CODES.SKILL_NONCONFORMANT,
|
||||
);
|
||||
});
|
||||
|
||||
it("--verify works on a single skills/get", async () => {
|
||||
const entry = await cleanEntry();
|
||||
const client = mockClient({
|
||||
getSkillResult: vi.fn().mockResolvedValue({ skill: entry }),
|
||||
});
|
||||
const outcome = await runMethod(client, {
|
||||
method: "skills/get",
|
||||
uri: entry.uri,
|
||||
verify: true,
|
||||
});
|
||||
expect(outcome.kind).toBe("ndjson");
|
||||
if (outcome.kind !== "ndjson") throw new Error("unreachable");
|
||||
expect(outcome.lines).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("summarizeSkillVerification (#2248)", () => {
|
||||
const report = (
|
||||
over: Partial<SkillVerifyReport> = {},
|
||||
): SkillVerifyReport => ({
|
||||
uri: "skill://demo/SKILL.md",
|
||||
name: "demo",
|
||||
conformance: [],
|
||||
frontmatter: [],
|
||||
files: [{ uri: "skill://demo/SKILL.md", status: "verified" }],
|
||||
ok: true,
|
||||
outcome: "verified",
|
||||
...over,
|
||||
});
|
||||
|
||||
it("reports a clean run with singular wording for one skill", () => {
|
||||
expect(summarizeSkillVerification([report()])).toBe(
|
||||
"Verified 1 skill and 1 file: no conformance errors.",
|
||||
);
|
||||
});
|
||||
|
||||
it("pluralizes for more than one", () => {
|
||||
expect(summarizeSkillVerification([report(), report()])).toBe(
|
||||
"Verified 2 skills and 2 files: no conformance errors.",
|
||||
);
|
||||
});
|
||||
|
||||
it("counts failures and digest mismatches separately", () => {
|
||||
// A skill can fail on a conformance error with no mismatched file at all,
|
||||
// so collapsing the two counts would misreport the cause.
|
||||
const failed = report({
|
||||
ok: false,
|
||||
outcome: "failed",
|
||||
files: [{ uri: "skill://demo/SKILL.md", status: "mismatch" }],
|
||||
});
|
||||
expect(summarizeSkillVerification([report(), failed])).toBe(
|
||||
"1 of 2 skills failed verification (1 digest/size mismatch across 2 files).",
|
||||
);
|
||||
});
|
||||
|
||||
it("reports a failure with no mismatched file", () => {
|
||||
const failed = report({ ok: false, outcome: "failed", files: [] });
|
||||
expect(summarizeSkillVerification([failed])).toBe(
|
||||
"1 of 1 skill failed verification (0 digest/size mismatch across 0 files).",
|
||||
);
|
||||
});
|
||||
|
||||
it("does not claim a truncated walk verified", () => {
|
||||
// An `incomplete` report keeps `ok: true` — nothing checked was wrong —
|
||||
// so a summary branching on `ok` printed "no conformance errors" one line
|
||||
// before the run exited SKILL_INCOMPLETE (Copilot).
|
||||
const cut = report({
|
||||
outcome: "incomplete",
|
||||
incomplete: "Stopped after 2 of 9 manifest entries.",
|
||||
});
|
||||
expect(summarizeSkillVerification([cut])).toBe(
|
||||
"Checked 1 skill and 1 file: no conformance errors in what was read." +
|
||||
" 1 of 1 skill could not be fully checked: the read bounds stopped the walk.",
|
||||
);
|
||||
});
|
||||
|
||||
it("reports a mixed catalog on both counts", () => {
|
||||
// The louder verdict must not hide the quieter one: a caller told only
|
||||
// about the failure would think the rest of the catalog was cleared.
|
||||
const failed = report({
|
||||
ok: false,
|
||||
outcome: "failed",
|
||||
files: [{ uri: "skill://demo/SKILL.md", status: "mismatch" }],
|
||||
});
|
||||
const cut = report({ outcome: "incomplete", incomplete: "Stopped." });
|
||||
expect(summarizeSkillVerification([report(), failed, cut])).toBe(
|
||||
"1 of 3 skills failed verification (1 digest/size mismatch across 3 files)." +
|
||||
" 1 of 3 skills could not be fully checked: the read bounds stopped the walk.",
|
||||
);
|
||||
});
|
||||
});
|
||||
177
clients/cli/__tests__/skills-verify-cli.test.ts
Normal file
177
clients/cli/__tests__/skills-verify-cli.test.ts
Normal file
|
|
@ -0,0 +1,177 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { runCli } from "../src/cli.js";
|
||||
import { consumeMethodOutcome } from "../src/handlers/consume-outcome.js";
|
||||
import { EXIT_CODES } from "../src/error-handler.js";
|
||||
|
||||
/**
|
||||
* `--verify`'s argument validation and its NDJSON consumption path (#2248).
|
||||
*
|
||||
* The validation sits with `--strict`'s, ahead of every short-circuit return in
|
||||
* `parseArgs`, for the same reason: the returns below it never reach
|
||||
* `runMethod`, so a check placed further down would let the flag be accepted
|
||||
* and then silently ignored.
|
||||
*/
|
||||
describe("--verify argument validation", () => {
|
||||
it("is rejected with a method other than skills/list or skills/get", async () => {
|
||||
await expect(
|
||||
runCli([
|
||||
"node",
|
||||
"cli",
|
||||
"--cli",
|
||||
"--method",
|
||||
"tools/list",
|
||||
"--verify",
|
||||
"--server-url",
|
||||
"http://127.0.0.1:1/mcp",
|
||||
]),
|
||||
).rejects.toThrow(
|
||||
"--verify requires --method skills/list or --method skills/get.",
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["servers/list", ["--method", "servers/list"]],
|
||||
["--list-stored-auth", ["--method", "servers/list", "--list-stored-auth"]],
|
||||
])(
|
||||
"is rejected on the %s short-circuit path, which never reaches the report",
|
||||
async (_label, extra) => {
|
||||
await expect(
|
||||
runCli(["node", "cli", "--cli", "--verify", ...extra]),
|
||||
).rejects.toThrow(
|
||||
"--verify requires --method skills/list or --method skills/get.",
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("is accepted with skills/get", async () => {
|
||||
// Reaches the connect and fails there — which is the point: the flag
|
||||
// itself was not what was rejected.
|
||||
await expect(
|
||||
runCli([
|
||||
"node",
|
||||
"cli",
|
||||
"--cli",
|
||||
"--method",
|
||||
"skills/get",
|
||||
"--uri",
|
||||
"skill://demo/SKILL.md",
|
||||
"--verify",
|
||||
"--server-url",
|
||||
"http://127.0.0.1:1/mcp",
|
||||
]),
|
||||
).rejects.not.toThrow(/--verify requires/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("consumeMethodOutcome NDJSON summary and exit code (#2248)", () => {
|
||||
function captureStreams() {
|
||||
let stdout = "";
|
||||
let stderr = "";
|
||||
const write = (sink: (s: string) => void) =>
|
||||
((chunk: unknown, ...rest: unknown[]) => {
|
||||
sink(typeof chunk === "string" ? chunk : String(chunk));
|
||||
const cb = rest.find((r) => typeof r === "function") as
|
||||
| (() => void)
|
||||
| undefined;
|
||||
cb?.();
|
||||
return true;
|
||||
}) as typeof process.stdout.write;
|
||||
const originalOut = process.stdout.write;
|
||||
const originalErr = process.stderr.write;
|
||||
process.stdout.write = write((s) => (stdout += s));
|
||||
process.stderr.write = write((s) => (stderr += s));
|
||||
return {
|
||||
get stdout() {
|
||||
return stdout;
|
||||
},
|
||||
get stderr() {
|
||||
return stderr;
|
||||
},
|
||||
restore() {
|
||||
process.stdout.write = originalOut;
|
||||
process.stderr.write = originalErr;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("writes the summary to stderr so it cannot contaminate the NDJSON", async () => {
|
||||
const streams = captureStreams();
|
||||
try {
|
||||
await consumeMethodOutcome(
|
||||
{ kind: "ndjson", lines: [{ ok: true }], summary: "all good" },
|
||||
{},
|
||||
);
|
||||
} finally {
|
||||
streams.restore();
|
||||
}
|
||||
expect(JSON.parse(streams.stdout.trim())).toEqual({ ok: true });
|
||||
expect(streams.stderr).toBe("all good\n");
|
||||
});
|
||||
|
||||
it("throws the exit code AFTER writing the report", async () => {
|
||||
// The report is the output a CI job reads; failing before writing it would
|
||||
// give the reader an exit code and nothing to act on.
|
||||
const streams = captureStreams();
|
||||
let thrown: unknown;
|
||||
try {
|
||||
await consumeMethodOutcome(
|
||||
{
|
||||
kind: "ndjson",
|
||||
lines: [{ ok: false }],
|
||||
summary: "one failed",
|
||||
exitCode: EXIT_CODES.SKILL_NONCONFORMANT,
|
||||
},
|
||||
{},
|
||||
);
|
||||
} catch (err) {
|
||||
thrown = err;
|
||||
} finally {
|
||||
streams.restore();
|
||||
}
|
||||
expect(streams.stdout.trim()).toBe('{"ok":false}');
|
||||
expect(thrown).toMatchObject({
|
||||
exitCode: EXIT_CODES.SKILL_NONCONFORMANT,
|
||||
envelope: { code: "skills_nonconformant" },
|
||||
});
|
||||
});
|
||||
|
||||
it("labels the envelope for an INCOMPLETE run, not a nonconformant one", async () => {
|
||||
// The envelope's `code` follows the exit code, so a caller reading one
|
||||
// never has to reconcile it against the other — and exit 8 means the
|
||||
// server broke no MUST.
|
||||
const streams = captureStreams();
|
||||
let thrown: unknown;
|
||||
try {
|
||||
await consumeMethodOutcome(
|
||||
{
|
||||
kind: "ndjson",
|
||||
lines: [{ outcome: "incomplete" }],
|
||||
summary: "not fully checked",
|
||||
exitCode: EXIT_CODES.SKILL_INCOMPLETE,
|
||||
},
|
||||
{},
|
||||
);
|
||||
} catch (err) {
|
||||
thrown = err;
|
||||
} finally {
|
||||
streams.restore();
|
||||
}
|
||||
expect(thrown).toMatchObject({
|
||||
exitCode: EXIT_CODES.SKILL_INCOMPLETE,
|
||||
envelope: { code: "skills_incomplete" },
|
||||
});
|
||||
});
|
||||
|
||||
it("leaves an --app-info NDJSON outcome unchanged", async () => {
|
||||
// No summary, no exit code — the field is additive and the older caller
|
||||
// must behave exactly as before.
|
||||
const streams = captureStreams();
|
||||
try {
|
||||
await consumeMethodOutcome({ kind: "ndjson", lines: [{ a: 1 }] }, {});
|
||||
} finally {
|
||||
streams.restore();
|
||||
}
|
||||
expect(streams.stderr).toBe("");
|
||||
expect(streams.stdout.trim()).toBe('{"a":1}');
|
||||
});
|
||||
});
|
||||
|
|
@ -679,7 +679,14 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
parseKeyValuePair,
|
||||
{},
|
||||
)
|
||||
.option("--uri <uri>", "URI of the resource (for resources/read method)")
|
||||
.option(
|
||||
"--uri <uri>",
|
||||
"URI of the resource (resources/read, resources/directory/read) or of the skill (skills/get)",
|
||||
)
|
||||
.option(
|
||||
"--cursor <cursor>",
|
||||
"Opaque pagination cursor (for resources/directory/read; pass back the nextCursor from the previous page).",
|
||||
)
|
||||
.option(
|
||||
"--prompt-name <promptName>",
|
||||
"Name of the prompt (for prompts/get method)",
|
||||
|
|
@ -743,6 +750,10 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
"--strict",
|
||||
"Report tool-schema portability problems in full (path, issue, suggested fix) on stderr, and exit 6 if any is error-severity. Use with --method tools/list. Without it, a one-line count is printed instead.",
|
||||
)
|
||||
.option(
|
||||
"--verify",
|
||||
"Run the SEP-2640 conformance and digest checks over the skills returned, emit one JSON report per skill on stdout, and exit 7 if any fails or 8 if any could not be fully checked within the read bounds. Use with --method skills/list or --method skills/get.",
|
||||
)
|
||||
.option(
|
||||
"--connect-timeout <ms>",
|
||||
`Connection timeout in ms (default ${DEFAULT_CONNECT_TIMEOUT_MS} for ad-hoc --server-url / target invocations; 0 = no timeout).`,
|
||||
|
|
@ -848,6 +859,8 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
header?: Record<string, string>;
|
||||
appInfo?: boolean;
|
||||
strict?: boolean;
|
||||
verify?: boolean;
|
||||
cursor?: string;
|
||||
connectTimeout?: number;
|
||||
format?: OutputFormat;
|
||||
toolArgsJson?: string;
|
||||
|
|
@ -919,6 +932,18 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
}
|
||||
}
|
||||
|
||||
// `--verify` is checked here for exactly the reason `--strict` is: the
|
||||
// short-circuit returns below never reach `runMethod`, so validating further
|
||||
// down would let `--verify --method servers/list` succeed while silently
|
||||
// ignoring a flag documented as skills-only.
|
||||
if (options.verify) {
|
||||
if (options.method !== "skills/list" && options.method !== "skills/get") {
|
||||
throw new Error(
|
||||
"--verify requires --method skills/list or --method skills/get.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// State-path precedence (getStateFilePath): MCP_INSPECTOR_OAUTH_STATE_PATH →
|
||||
// <MCP_STORAGE_DIR>/oauth.json → ~/.mcp-inspector/storage/oauth.json — the
|
||||
// same file the web backend writes, so tokens are shared across surfaces.
|
||||
|
|
@ -1147,6 +1172,8 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
toolMeta: options.toolMetadata,
|
||||
appInfo: options.appInfo === true,
|
||||
strict: options.strict === true,
|
||||
verify: options.verify === true,
|
||||
cursor: options.cursor,
|
||||
format: options.format,
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,29 @@ export const EXIT_CODES = {
|
|||
UNREACHABLE: 4,
|
||||
TOOL_ERROR: 5,
|
||||
SCHEMA_UNPORTABLE: 6,
|
||||
/**
|
||||
* `--verify` found a SEP-2640 violation: a conformance error, a digest or
|
||||
* size mismatch, or a manifest file that could not be read (#2248).
|
||||
*
|
||||
* Its own code rather than reusing `SCHEMA_UNPORTABLE`, for the reason that
|
||||
* one exists at all: a CI job that fails on an unportable tool schema and a
|
||||
* CI job that fails on a tampered skill digest are different jobs, and
|
||||
* collapsing them would make `if [ $? -eq 6 ]` ambiguous.
|
||||
*/
|
||||
SKILL_NONCONFORMANT: 7,
|
||||
/**
|
||||
* `--verify` could not check the whole catalog: the read bounds stopped the
|
||||
* walk before it finished (#2248).
|
||||
*
|
||||
* Distinct from `SKILL_NONCONFORMANT` because the server has broken no
|
||||
* **MUST** — SEP-2640 states the 512-entry and 16 MiB limits as SHOULD NOT,
|
||||
* with hosts free to support more — so exiting 7 would call a conforming
|
||||
* server nonconformant. It is still non-zero, because reporting success for
|
||||
* a manifest whose unread entries were never fetched is a false pass. A CI
|
||||
* job that wants to tolerate oversized catalogs can allow 8 and still fail
|
||||
* on 7.
|
||||
*/
|
||||
SKILL_INCOMPLETE: 8,
|
||||
} as const;
|
||||
|
||||
/** Machine-readable error envelope written as one JSON line on stderr. */
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { awaitableLog } from "../utils/awaitable-log.js";
|
||||
import { awaitableError, awaitableLog } from "../utils/awaitable-log.js";
|
||||
import { CliExitCodeError, EXIT_CODES } from "../error-handler.js";
|
||||
import { emitResult } from "./emit-result.js";
|
||||
import type { MethodArgs, MethodOutcome } from "./method-types.js";
|
||||
|
||||
|
|
@ -21,6 +22,22 @@ export async function consumeMethodOutcome(
|
|||
for (const line of outcome.lines) {
|
||||
await awaitableLog(JSON.stringify(line) + "\n");
|
||||
}
|
||||
// Summary on **stderr**, after the report, so it cannot contaminate the
|
||||
// NDJSON a consumer is parsing on stdout.
|
||||
if (outcome.summary) await awaitableError(`${outcome.summary}\n`);
|
||||
// Thrown rather than returned so it routes through the CLI's single exit
|
||||
// path — the report has already been written, which is why this is the
|
||||
// last thing that happens.
|
||||
if (outcome.exitCode) {
|
||||
throw new CliExitCodeError(outcome.exitCode, outcome.summary ?? "", {
|
||||
// The envelope's `code` follows the exit code, so a caller reading one
|
||||
// never has to reconcile it against the other.
|
||||
code:
|
||||
outcome.exitCode === EXIT_CODES.SKILL_INCOMPLETE
|
||||
? "skills_incomplete"
|
||||
: "skills_nonconformant",
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -34,6 +34,18 @@ export type MethodArgs = {
|
|||
taskId?: string;
|
||||
/** When true, tools/call uses callToolStream (task-augmented). */
|
||||
task?: boolean;
|
||||
/**
|
||||
* `--verify`: run the SEP-2640 conformance and digest checks over the skills
|
||||
* a `skills/list` / `skills/get` returned, emit one NDJSON report per skill,
|
||||
* and exit non-zero when any fails (#2248).
|
||||
*/
|
||||
verify?: boolean;
|
||||
/**
|
||||
* Opaque pagination cursor. Used by `resources/directory/read`, whose result
|
||||
* pages exactly as `resources/list` does — and where the caller descends the
|
||||
* tree itself, so there is no store to walk it.
|
||||
*/
|
||||
cursor?: string;
|
||||
/** roots/set payload (JSON array of {uri, name?}). */
|
||||
rootsJson?: string;
|
||||
/** prompts/complete: argument name / value / ref. */
|
||||
|
|
@ -53,7 +65,18 @@ export type McpResponse = Record<string, unknown>;
|
|||
export type MethodOutcome =
|
||||
| { kind: "result"; result: McpResponse; appInfo?: CliAppInfo }
|
||||
/** One JSON object per line (e.g. tools/list --app-info). Caller writes stdout. */
|
||||
| { kind: "ndjson"; lines: unknown[] }
|
||||
| {
|
||||
kind: "ndjson";
|
||||
lines: unknown[];
|
||||
/**
|
||||
* A line for **stderr**, written after the NDJSON. `--verify` uses it for
|
||||
* its one-line summary, so a reader who piped stdout into `jq` still sees
|
||||
* the verdict; `--app-info` sets nothing and behaves as before.
|
||||
*/
|
||||
summary?: string;
|
||||
/** Non-zero when the emitted report is itself a failure (`--verify`). */
|
||||
exitCode?: number;
|
||||
}
|
||||
| {
|
||||
kind: "stream";
|
||||
/** Human label for errors. */
|
||||
|
|
@ -76,6 +99,7 @@ export const SESSION_RPC_METHODS = [
|
|||
"resources/list",
|
||||
"resources/read",
|
||||
"resources/templates/list",
|
||||
"resources/directory/read",
|
||||
"resources/subscribe",
|
||||
"resources/unsubscribe",
|
||||
"prompts/list",
|
||||
|
|
@ -89,6 +113,8 @@ export const SESSION_RPC_METHODS = [
|
|||
"tasks/result",
|
||||
"roots/list",
|
||||
"roots/set",
|
||||
"skills/list",
|
||||
"skills/get",
|
||||
] as const;
|
||||
|
||||
export type SessionRpcMethod = (typeof SESSION_RPC_METHODS)[number];
|
||||
|
|
@ -109,6 +135,12 @@ export const ONE_SHOT_METHODS = [
|
|||
"prompts/list",
|
||||
"prompts/get",
|
||||
"logging/setLevel",
|
||||
// SEP-2640. All three are ordinary one-shot request/response calls — no
|
||||
// stream, no long-lived subscription — so they belong here alongside the
|
||||
// other list verbs rather than being reachable only from the session CLI.
|
||||
"skills/list",
|
||||
"skills/get",
|
||||
"resources/directory/read",
|
||||
] as const;
|
||||
|
||||
export type OneShotMethod = (typeof ONE_SHOT_METHODS)[number];
|
||||
|
|
|
|||
|
|
@ -6,10 +6,18 @@ import {
|
|||
ManagedResourceTemplatesState,
|
||||
ManagedPromptsState,
|
||||
ManagedRequestorTasksState,
|
||||
ManagedSkillsState,
|
||||
MessageLogState,
|
||||
} from "@inspector/core/mcp/state/index.js";
|
||||
import { SKILLS_EXTENSION_KEY } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import { CliExitCodeError, EXIT_CODES } from "../error-handler.js";
|
||||
import { collectAppInfo } from "./collect-app-info.js";
|
||||
import { summarizeSkillVerification } from "./skills-verify.js";
|
||||
import {
|
||||
allSkillsVerified,
|
||||
anySkillFailed,
|
||||
verifySkills,
|
||||
} from "@inspector/core/mcp/skillsVerification.js";
|
||||
import type {
|
||||
CliAppInfo,
|
||||
McpResponse,
|
||||
|
|
@ -17,6 +25,28 @@ import type {
|
|||
MethodOutcome,
|
||||
} from "./method-types.js";
|
||||
|
||||
/**
|
||||
* Refuse a `skills/*` call against a server that never declared the extension.
|
||||
*
|
||||
* Shared by `skills/list` and `skills/get` so the two cannot drift: declaring
|
||||
* the extension commits a server to both, so a client that gates one and not
|
||||
* the other is inconsistent with the thing it is checking. Not needed for
|
||||
* `resources/directory/read`, whose stricter `directoryRead` gate lives in
|
||||
* `InspectorClient` itself.
|
||||
*/
|
||||
function assertSkillsSupported(
|
||||
inspectorClient: InspectorClient,
|
||||
method: string,
|
||||
): void {
|
||||
if (!inspectorClient.getSkillsExtension()) {
|
||||
throw new CliExitCodeError(
|
||||
EXIT_CODES.USAGE,
|
||||
`Server does not declare the ${SKILLS_EXTENSION_KEY} extension, so ${method} is not available.`,
|
||||
{ code: "skills_unsupported" },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Run one MCP method against a connected {@link InspectorClient}.
|
||||
* Core method dispatch used by the CLI (and other Inspector Node runners).
|
||||
|
|
@ -35,6 +65,7 @@ export async function runMethod(
|
|||
null;
|
||||
let managedPromptsState: ManagedPromptsState | null = null;
|
||||
let managedTasksState: ManagedRequestorTasksState | null = null;
|
||||
let managedSkillsState: ManagedSkillsState | null = null;
|
||||
|
||||
try {
|
||||
let result: McpResponse;
|
||||
|
|
@ -283,6 +314,101 @@ export async function runMethod(
|
|||
result = (await inspectorClient.getRequestorTaskResult(
|
||||
args.taskId,
|
||||
)) as McpResponse;
|
||||
} else if (args.method === "skills/list") {
|
||||
// The store's cursor walk is reused rather than re-implemented — it
|
||||
// carries the repeated-cursor and page-cap guards, and a second copy of
|
||||
// a pagination walk is how the two come to disagree. What the CLI adds
|
||||
// is the check below: the store answers "no extension" with an empty
|
||||
// list, which is right for a UI that must render *something*, and wrong
|
||||
// for a CLI where "this server has no skills" and "this server does not
|
||||
// serve skills at all" are different answers a script has to tell apart.
|
||||
assertSkillsSupported(inspectorClient, args.method);
|
||||
managedSkillsState = new ManagedSkillsState(inspectorClient);
|
||||
const skills = await managedSkillsState.refresh(args.metadata);
|
||||
if (args.verify) {
|
||||
const reports = await verifySkills(
|
||||
inspectorClient,
|
||||
skills,
|
||||
args.metadata,
|
||||
);
|
||||
return {
|
||||
kind: "ndjson",
|
||||
lines: reports,
|
||||
summary: summarizeSkillVerification(reports),
|
||||
// Three outcomes, three exit codes: a broken MUST is 7, a walk the
|
||||
// read bounds cut short is 8, and everything checked and passing is
|
||||
// 0. Collapsing the middle case into either of the others reports
|
||||
// something untrue about the server (Copilot).
|
||||
...(allSkillsVerified(reports)
|
||||
? {}
|
||||
: {
|
||||
exitCode: anySkillFailed(reports)
|
||||
? EXIT_CODES.SKILL_NONCONFORMANT
|
||||
: EXIT_CODES.SKILL_INCOMPLETE,
|
||||
}),
|
||||
};
|
||||
}
|
||||
result = { skills };
|
||||
} else if (args.method === "skills/get") {
|
||||
if (!args.uri) {
|
||||
throw new Error(
|
||||
"URI is required for skills/get method. Use --uri to specify the skill URI.",
|
||||
);
|
||||
}
|
||||
// Same gate as `skills/list`, and for the same reason. Without it an
|
||||
// undeclared server answers `-32601`, which a script cannot tell apart
|
||||
// from the `-32602` a *declared* server returns for a skill URI it does
|
||||
// not serve — "this server has no Skills support" and "no such skill"
|
||||
// are different answers (Copilot).
|
||||
assertSkillsSupported(inspectorClient, args.method);
|
||||
// The ENVELOPE, not the unwrapped entry. `getSkill` discards every other
|
||||
// member the result carried — including the `ttlMs` / `cacheScope` that
|
||||
// SEP-2640 explicitly leaves open — and a CLI whose contract is "print
|
||||
// the result" must not drop what the server actually sent (Copilot).
|
||||
const envelope = await inspectorClient.getSkillResult(
|
||||
args.uri,
|
||||
args.metadata,
|
||||
);
|
||||
const skill = envelope.skill;
|
||||
if (args.verify) {
|
||||
const reports = await verifySkills(
|
||||
inspectorClient,
|
||||
[skill],
|
||||
args.metadata,
|
||||
);
|
||||
return {
|
||||
kind: "ndjson",
|
||||
lines: reports,
|
||||
summary: summarizeSkillVerification(reports),
|
||||
// Three outcomes, three exit codes: a broken MUST is 7, a walk the
|
||||
// read bounds cut short is 8, and everything checked and passing is
|
||||
// 0. Collapsing the middle case into either of the others reports
|
||||
// something untrue about the server (Copilot).
|
||||
...(allSkillsVerified(reports)
|
||||
? {}
|
||||
: {
|
||||
exitCode: anySkillFailed(reports)
|
||||
? EXIT_CODES.SKILL_NONCONFORMANT
|
||||
: EXIT_CODES.SKILL_INCOMPLETE,
|
||||
}),
|
||||
};
|
||||
}
|
||||
result = envelope;
|
||||
} else if (args.method === "resources/directory/read") {
|
||||
if (!args.uri) {
|
||||
throw new Error(
|
||||
"URI is required for resources/directory/read. Use --uri to specify the directory URI.",
|
||||
);
|
||||
}
|
||||
// One page, not a walk. SEP-2640 says the listing is not recursive and
|
||||
// clients descend by calling again on a child, so aggregating pages here
|
||||
// would present a subtree as a directory — and the cursor is exposed as
|
||||
// `--cursor` precisely so a script can do the descending.
|
||||
result = await inspectorClient.readResourceDirectory(
|
||||
args.uri,
|
||||
args.cursor,
|
||||
args.metadata,
|
||||
);
|
||||
} else if (args.method === "roots/list") {
|
||||
result = { roots: inspectorClient.getRoots() };
|
||||
} else if (args.method === "roots/set") {
|
||||
|
|
@ -318,6 +444,7 @@ export async function runMethod(
|
|||
managedResourcesState?.destroy();
|
||||
managedResourceTemplatesState?.destroy();
|
||||
managedPromptsState?.destroy();
|
||||
managedSkillsState?.destroy();
|
||||
managedTasksState?.destroy();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
53
clients/cli/src/handlers/skills-verify.ts
Normal file
53
clients/cli/src/handlers/skills-verify.ts
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
/**
|
||||
* `--verify`: the scriptable SEP-2640 conformance report (#2248).
|
||||
*
|
||||
* The Skills screen in the web client can verify a skill, but only by hand, one
|
||||
* file at a time, in a browser. A server author wants the same verdict in CI,
|
||||
* over the whole catalog, with an exit code — which is exactly the argument
|
||||
* `--strict` makes for the tool-schema lint, so this follows that handler's
|
||||
* shape rather than inventing a second one.
|
||||
*
|
||||
* The walk itself is `core/mcp/skillsVerification.ts`, shared with the TUI's
|
||||
* Skills pane. What is left here is presentation: the one-line stderr summary,
|
||||
* which is a CLI concern and nothing else's.
|
||||
*/
|
||||
|
||||
import type { SkillVerifyReport } from "@inspector/core/mcp/skillsVerification.js";
|
||||
|
||||
/**
|
||||
* A one-line human summary for stderr, so a reader who piped stdout to `jq`
|
||||
* still learns the verdict.
|
||||
*/
|
||||
export function summarizeSkillVerification(
|
||||
reports: readonly SkillVerifyReport[],
|
||||
): string {
|
||||
// ⚠️ Counted off `outcome`, never off `ok`. `ok` means "nothing that was
|
||||
// checked is wrong", which an `incomplete` report satisfies while the walk
|
||||
// was cut short — so branching on `ok` printed "no conformance errors" one
|
||||
// line before exiting SKILL_INCOMPLETE (Copilot).
|
||||
const failed = reports.filter((report) => report.outcome === "failed").length;
|
||||
const incomplete = reports.filter(
|
||||
(report) => report.outcome === "incomplete",
|
||||
).length;
|
||||
const files = reports.reduce((sum, report) => sum + report.files.length, 0);
|
||||
const mismatched = reports.reduce(
|
||||
(sum, report) =>
|
||||
sum + report.files.filter((file) => file.status === "mismatch").length,
|
||||
0,
|
||||
);
|
||||
const skillWord = reports.length === 1 ? "skill" : "skills";
|
||||
const fileWord = files === 1 ? "file" : "files";
|
||||
// A catalog can be both: some skills broken, others merely cut short. Say so
|
||||
// rather than letting the louder verdict hide the quieter one.
|
||||
const incompleteClause =
|
||||
incomplete === 0
|
||||
? ""
|
||||
: ` ${incomplete} of ${reports.length} ${skillWord} could not be fully checked: the read bounds stopped the walk.`;
|
||||
const headline =
|
||||
failed === 0
|
||||
? incomplete === 0
|
||||
? `Verified ${reports.length} ${skillWord} and ${files} ${fileWord}: no conformance errors.`
|
||||
: `Checked ${reports.length} ${skillWord} and ${files} ${fileWord}: no conformance errors in what was read.`
|
||||
: `${failed} of ${reports.length} ${skillWord} failed verification (${mismatched} digest/size mismatch across ${files} ${fileWord}).`;
|
||||
return `${headline}${incompleteClause}`;
|
||||
}
|
||||
|
|
@ -55,6 +55,13 @@ export default defineConfig({
|
|||
"atomically",
|
||||
"open",
|
||||
"zod",
|
||||
// Newly on `core/`'s runtime import graph as of #2248:
|
||||
// `core/mcp/skillFile.ts` parses a served SKILL.md's YAML frontmatter to
|
||||
// check it against the entry the listing advertised (SEP-2640). Already a
|
||||
// root `dependency` — it was reached from `test-servers/src` — so this
|
||||
// adds no package, but a root-declared dependency `core/` imports must be
|
||||
// named in all three `external` lists or tsup inlines it here.
|
||||
"yaml",
|
||||
// Reached through `core/` but not through this client's own code today.
|
||||
// AGENTS.md requires every root-declared package `core/` imports at runtime
|
||||
// in ALL three lists regardless, because which client reaches one is a
|
||||
|
|
|
|||
|
|
@ -76,13 +76,14 @@ The TUI provides terminal-native tabs and panes for interacting with your MCP se
|
|||
- **Resources**: Browse and read resources exposed by the server.
|
||||
- **Prompts**: List and test prompts.
|
||||
- **Tools**: View available tools and execute them with form-like inputs. A tool whose advertised schema carries a portability problem is flagged in the list — red `!` for a construct a shipping MCP client refuses, yellow `?` for one handled unevenly — and the detail pane lists each finding under **Schema Portability** with the path, the problem, and a concrete fix. The verdict comes from [`core/json/schemaLint.ts`](../../core/json/schemaLint.ts), shared with the web Tools tab and the CLI's `--strict` report, so the three cannot disagree ([#1005](https://github.com/modelcontextprotocol/inspector/issues/1005)).
|
||||
- **Skills**: Shown only when the connected server declares the SEP-2640 Skills extension (`io.modelcontextprotocol/skills`), since it is a *server* declaration and so only knowable after connecting. The list marks each skill with its structural verdict — `✓` conforms, `!` warnings only, `✗` an error — using a glyph as well as a colour, because this pane is read over ssh, in tmux and through `script(1)`. The detail pane shows the entry's URI, description, conformance findings and manifest. **Enter** verifies the selected skill: one `resources/read` per manifest file, each hashed against its advertised digest, plus the frontmatter cross-check that compares the served `SKILL.md`'s own frontmatter against the one the listing advertised. Verification is a gesture rather than a page load because SEP-2640 says hosts MUST NOT retrieve a skill's files ahead of need. The checks are the same ones the web Skills tab and the CLI's `--verify` run ([#2234](https://github.com/modelcontextprotocol/inspector/issues/2234), [#2248](https://github.com/modelcontextprotocol/inspector/issues/2248)).
|
||||
- **Protocol**: View JSON-RPC request/response/notification history (matches the web Protocol monitor).
|
||||
- **Network**: View HTTP fetch traffic for SSE / Streamable HTTP servers (matches the web Network monitor).
|
||||
- **Console**: View stdio stderr from the connected server process (matches the web Console monitor).
|
||||
|
||||
## Navigation
|
||||
|
||||
- Use the **Arrow Keys** (Left/Right) or **Tab** to switch between the main tabs (Resources, Tools, Prompts, etc.).
|
||||
- Use the **Arrow Keys** (Left/Right) or **Tab** to switch between the main tabs (Resources, Tools, Prompts, Skills, etc.).
|
||||
- Use the **Arrow Keys** (Up/Down) to scroll through lists of items.
|
||||
- Press **Enter** to select an item, execute a tool, or fetch a resource.
|
||||
- Press **Escape** or `Ctrl+C` to exit the application.
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ const h = vi.hoisted(() => {
|
|||
resources: unknown[];
|
||||
resourceTemplates: unknown[];
|
||||
prompts: unknown[];
|
||||
skills: unknown[];
|
||||
skillsExtension: { directoryRead: boolean } | undefined;
|
||||
messages: unknown[];
|
||||
fetchRequests: unknown[];
|
||||
stderrLogs: unknown[];
|
||||
|
|
@ -39,6 +41,8 @@ const h = vi.hoisted(() => {
|
|||
resources: [],
|
||||
resourceTemplates: [],
|
||||
prompts: [],
|
||||
skills: [],
|
||||
skillsExtension: undefined as { directoryRead: boolean } | undefined,
|
||||
messages: [],
|
||||
fetchRequests: [],
|
||||
stderrLogs: [],
|
||||
|
|
@ -160,6 +164,10 @@ const h = vi.hoisted(() => {
|
|||
| "sse"
|
||||
| "streamable-http",
|
||||
);
|
||||
// The Skills tab is gated on a SERVER declaration, so the default here is
|
||||
// "not declared" — the tab is hidden unless a test opts in by pointing
|
||||
// `ctrl.skillsExtension` at a declaration.
|
||||
getSkillsExtension = vi.fn(() => ctrl.skillsExtension);
|
||||
authenticate = (...a: Parameters<InspectorClient["authenticate"]>) =>
|
||||
clientSpies.authenticate(...a);
|
||||
clearOAuthTokens = (
|
||||
|
|
@ -232,6 +240,11 @@ const h = vi.hoisted(() => {
|
|||
resourceTemplates: ctrl.resourceTemplates,
|
||||
})),
|
||||
useManagedPrompts: vi.fn(() => ({ prompts: ctrl.prompts })),
|
||||
useManagedSkills: vi.fn(() => ({
|
||||
skills: ctrl.skills,
|
||||
pageCount: ctrl.skills.length > 0 ? 1 : 0,
|
||||
error: null,
|
||||
})),
|
||||
useMessageLog: vi.fn(() => ({ messages: ctrl.messages })),
|
||||
useFetchRequestLog: vi.fn(() => ({ fetchRequests: ctrl.fetchRequests })),
|
||||
useStderrLog: vi.fn(() => ({ stderrLogs: ctrl.stderrLogs })),
|
||||
|
|
@ -246,6 +259,7 @@ vi.mock("@inspector/core/mcp/state/index.js", () => ({
|
|||
ManagedResourcesState: h.FakeManager,
|
||||
ManagedResourceTemplatesState: h.FakeManager,
|
||||
ManagedPromptsState: h.FakeManager,
|
||||
ManagedSkillsState: h.FakeManager,
|
||||
MessageLogState: h.FakeManager,
|
||||
FetchRequestLogState: h.FakeManager,
|
||||
StderrLogState: h.FakeManager,
|
||||
|
|
@ -271,6 +285,9 @@ vi.mock("@inspector/core/react/useManagedResources.js", () => ({
|
|||
vi.mock("@inspector/core/react/useManagedResourceTemplates.js", () => ({
|
||||
useManagedResourceTemplates: h.useManagedResourceTemplates,
|
||||
}));
|
||||
vi.mock("@inspector/core/react/useManagedSkills.js", () => ({
|
||||
useManagedSkills: h.useManagedSkills,
|
||||
}));
|
||||
vi.mock("@inspector/core/react/useManagedPrompts.js", () => ({
|
||||
useManagedPrompts: h.useManagedPrompts,
|
||||
}));
|
||||
|
|
@ -664,6 +681,8 @@ beforeEach(() => {
|
|||
resources: [],
|
||||
resourceTemplates: [],
|
||||
prompts: [],
|
||||
skills: [],
|
||||
skillsExtension: undefined as { directoryRead: boolean } | undefined,
|
||||
messages: [],
|
||||
fetchRequests: [],
|
||||
stderrLogs: [],
|
||||
|
|
@ -751,6 +770,69 @@ describe("App (foundation)", () => {
|
|||
expect(h.connect).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("hides the Skills tab until the server declares the extension", async () => {
|
||||
// A *server*-declared extension (SEP-2640), so unlike the transport-derived
|
||||
// tabs it is only knowable after connecting — and showing it against a
|
||||
// server that never declared it would send `skills/list` to a server that
|
||||
// answers -32601 (#2248).
|
||||
h.ctrl.status = "connected";
|
||||
const r = await mount(oneStdio());
|
||||
await expectFrame(r, "Tools");
|
||||
expect(r.lastFrame() ?? "").not.toContain("Skills");
|
||||
});
|
||||
|
||||
it("shows the Skills tab, with its count, once the extension is declared", async () => {
|
||||
h.ctrl.status = "connected";
|
||||
h.ctrl.skillsExtension = { directoryRead: false };
|
||||
h.ctrl.skills = [
|
||||
{
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: { name: "demo", description: "d" },
|
||||
resources: [],
|
||||
},
|
||||
];
|
||||
const r = await mount(oneStdio());
|
||||
await expectFrame(r, "Skills (1)");
|
||||
});
|
||||
|
||||
it("opens the Skills tab with its 'k' accelerator", async () => {
|
||||
// `k`, not `s` — the accelerator has to appear in the label and stay
|
||||
// unique; see `tabsConfig.ts`.
|
||||
h.ctrl.status = "connected";
|
||||
h.ctrl.skillsExtension = { directoryRead: true };
|
||||
const r = await mount(oneStdio());
|
||||
await expectFrame(r, "Skills");
|
||||
r.stdin.write("k");
|
||||
await expectFrame(r, "Select a skill to view details");
|
||||
});
|
||||
|
||||
it("leaves the Skills tab when the selected server does not serve it", async () => {
|
||||
// The tab disappears from the bar when the gate goes false, but `activeTab`
|
||||
// is independent of the bar — so without this the render branch keeps
|
||||
// showing the pane for a server that never declared the extension, and the
|
||||
// user is stranded on content they cannot navigate back to (Copilot).
|
||||
h.ctrl.status = "connected";
|
||||
h.ctrl.skillsExtension = { directoryRead: false };
|
||||
const r = await mount(oneStdio());
|
||||
await expectFrame(r, "Skills");
|
||||
r.stdin.write("k");
|
||||
await expectFrame(r, "Select a skill to view details");
|
||||
|
||||
// The server stops declaring it — the shape of switching to one without
|
||||
// the extension, since the declaration is read off the live client.
|
||||
h.ctrl.skillsExtension = undefined;
|
||||
r.rerender(
|
||||
<App
|
||||
mcpServers={oneStdio()}
|
||||
clientConfig={emptyClientConfig}
|
||||
callbackUrlConfig={callbackUrlConfig}
|
||||
/>,
|
||||
);
|
||||
await tick();
|
||||
await expectFrame(r, "Server Configuration");
|
||||
expect(r.lastFrame() ?? "").not.toContain("Select a skill to view details");
|
||||
});
|
||||
|
||||
it("disconnects with 'd' when connected", async () => {
|
||||
h.ctrl.status = "connected";
|
||||
const { stdin } = await mount(oneStdio());
|
||||
|
|
|
|||
872
clients/tui/__tests__/SkillsTab.test.tsx
Normal file
872
clients/tui/__tests__/SkillsTab.test.tsx
Normal file
|
|
@ -0,0 +1,872 @@
|
|||
import React from "react";
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { render } from "./helpers/renderTui";
|
||||
import type { InspectorClient } from "@inspector/core/mcp/index.js";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import { AuthRecoveryRequiredError } from "@inspector/core/auth/challenge.js";
|
||||
import { sha256Digest, textToBytes } from "@inspector/core/mcp/skills.js";
|
||||
|
||||
// MUST mock ink-scroll-view: the real ScrollView renders a placeholder minimap
|
||||
// in the non-TTY test env and never mounts its children.
|
||||
vi.mock("ink-scroll-view", () => import("./helpers/inkScrollViewMock.js"));
|
||||
|
||||
import { SkillsTab } from "../src/components/SkillsTab.js";
|
||||
|
||||
const tick = async () => {
|
||||
for (let i = 0; i < 8; i++)
|
||||
await new Promise((resolve) => setTimeout(resolve, 4));
|
||||
};
|
||||
|
||||
const ESC = String.fromCharCode(27);
|
||||
const UP = `${ESC}[A`;
|
||||
const DOWN = `${ESC}[B`;
|
||||
const PAGE_UP = `${ESC}[5~`;
|
||||
const PAGE_DOWN = `${ESC}[6~`;
|
||||
const ENTER = "\r";
|
||||
|
||||
const SKILL_MD = "---\nname: clean\ndescription: A clean skill\n---\n\n# C\n";
|
||||
// sha256 of SKILL_MD, so the clean fixture actually verifies.
|
||||
const CLEAN_DIGEST =
|
||||
"sha256:0000000000000000000000000000000000000000000000000000000000000000";
|
||||
|
||||
const clean: SkillEntry = {
|
||||
uri: "skill://clean/SKILL.md",
|
||||
frontmatter: { name: "clean", description: "A clean skill" },
|
||||
resources: [
|
||||
{ uri: "skill://clean/SKILL.md", digest: CLEAN_DIGEST, size: 51 },
|
||||
],
|
||||
};
|
||||
// A `name-path-mismatch`: the one structural invariant SEP-2640 states
|
||||
// outright, so this row must carry the error mark.
|
||||
const broken: SkillEntry = {
|
||||
uri: "skill://wrong-folder/SKILL.md",
|
||||
frontmatter: { name: "right-name", description: "Mismatched" },
|
||||
resources: [
|
||||
{ uri: "skill://wrong-folder/SKILL.md", digest: CLEAN_DIGEST, size: 1 },
|
||||
],
|
||||
};
|
||||
// Legal but unverifiable — a WARNING, which must read differently from an error.
|
||||
const dynamic: SkillEntry = {
|
||||
uri: "skill://gen/SKILL.md",
|
||||
frontmatter: { name: "gen", description: "Generated" },
|
||||
resources: "dynamic",
|
||||
};
|
||||
const noSize: SkillEntry = {
|
||||
uri: "skill://nosize/SKILL.md",
|
||||
frontmatter: { name: "nosize", description: "No declared size" },
|
||||
resources: [{ uri: "skill://nosize/SKILL.md", digest: CLEAN_DIGEST }],
|
||||
};
|
||||
|
||||
const skills = [clean, broken, dynamic, noSize];
|
||||
|
||||
function mockClient(
|
||||
readResource: unknown = vi.fn().mockResolvedValue({
|
||||
result: { contents: [{ uri: "skill://clean/SKILL.md", text: SKILL_MD }] },
|
||||
}),
|
||||
): InspectorClient {
|
||||
return { readResource } as unknown as InspectorClient;
|
||||
}
|
||||
|
||||
describe("SkillsTab (#2248)", () => {
|
||||
it("renders the empty state when there are no skills", () => {
|
||||
const { lastFrame } = render(
|
||||
<SkillsTab
|
||||
skills={[]}
|
||||
pageCount={0}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Skills (0)");
|
||||
expect(frame).toContain("No skills available");
|
||||
expect(frame).toContain("Select a skill to view details");
|
||||
});
|
||||
|
||||
it("shows the page count only when the walk took more than one page", () => {
|
||||
const one = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
expect(one.lastFrame() ?? "").toContain("Skills (4)");
|
||||
expect(one.lastFrame() ?? "").not.toContain("pages");
|
||||
const many = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={3}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
expect(many.lastFrame() ?? "").toContain("3 pages");
|
||||
});
|
||||
|
||||
it("renders the list error in place of the list", () => {
|
||||
const { lastFrame } = render(
|
||||
<SkillsTab
|
||||
skills={[]}
|
||||
pageCount={0}
|
||||
loadError={new Error("walk failed")}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame() ?? "").toContain("walk failed");
|
||||
});
|
||||
|
||||
it("marks each row with its static conformance verdict", () => {
|
||||
// The mark is a glyph, not only a colour: this pane is read over ssh, in
|
||||
// tmux and through `script(1)`, where colour may not survive.
|
||||
const { lastFrame } = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("✓ clean");
|
||||
// `skillDisplayName` prefers the declared name over the URI segment.
|
||||
expect(frame).toContain("✗ right-name");
|
||||
expect(frame).toContain("! gen");
|
||||
});
|
||||
|
||||
it("does not claim the listing conforms while verification is failing", async () => {
|
||||
// The two verdicts sat in one pane and contradicted each other: the static
|
||||
// checks pass on `clean` (its advertised digest is well-formed), while the
|
||||
// bytes do not hash to it. The heading now names what it actually covers.
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Verification FAILED");
|
||||
expect(frame).toContain("Listing checks: no structural issues");
|
||||
expect(frame).not.toContain("conforms");
|
||||
});
|
||||
|
||||
it("shows the selected skill's URI, description, findings and manifest", () => {
|
||||
const { lastFrame } = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("skill://clean/SKILL.md");
|
||||
expect(frame).toContain("A clean skill");
|
||||
// Named for what it covers: the static checks against the listing.
|
||||
expect(frame).toContain("Listing checks: no structural issues");
|
||||
expect(frame).toContain("Manifest (1)");
|
||||
expect(frame).toContain("SKILL.md");
|
||||
expect(frame).toContain("(51 B)");
|
||||
expect(frame).toContain("[Enter to verify digests and frontmatter]");
|
||||
});
|
||||
|
||||
it("renders a dynamic skill's manifest as unadvertised rather than empty", async () => {
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(DOWN);
|
||||
await tick();
|
||||
stdin.write(DOWN);
|
||||
await tick();
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain('"dynamic" — no files advertised');
|
||||
expect(frame).toContain("integrity cannot be verified");
|
||||
});
|
||||
|
||||
it("omits the size caption when the manifest declares none", async () => {
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
for (let i = 0; i < 3; i++) {
|
||||
stdin.write(DOWN);
|
||||
await tick();
|
||||
}
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Manifest (1)");
|
||||
expect(frame).not.toContain(" B)");
|
||||
});
|
||||
|
||||
it("moves selection with the arrow keys and stops at both boundaries", async () => {
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(UP);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("▶ ✓ clean");
|
||||
stdin.write(DOWN);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("▶ ✗ right-name");
|
||||
for (let i = 0; i < 5; i++) {
|
||||
stdin.write(DOWN);
|
||||
await tick();
|
||||
}
|
||||
// `nosize` omits a required `size`, so its row carries the error mark too
|
||||
// — the mark tracks the checks, not the position.
|
||||
expect(lastFrame() ?? "").toContain("▶ ✗ nosize");
|
||||
// …and back up from the bottom, which is the other direction of the same
|
||||
// guard: the top boundary above never exercises the move itself.
|
||||
stdin.write(UP);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("▶ ! gen");
|
||||
});
|
||||
|
||||
it("scrolls the details pane without moving the selection", async () => {
|
||||
const scrollBy = vi.fn();
|
||||
const { stdin } = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="details"
|
||||
/>,
|
||||
);
|
||||
stdin.write(UP);
|
||||
stdin.write(DOWN);
|
||||
stdin.write(PAGE_UP);
|
||||
stdin.write(PAGE_DOWN);
|
||||
await tick();
|
||||
// Nothing to assert on the mock beyond not crashing and not moving the
|
||||
// selection — the ScrollView handle is stubbed by the shared mock.
|
||||
expect(scrollBy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("ignores input entirely when a modal is open", async () => {
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={skills}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
modalOpen
|
||||
/>,
|
||||
);
|
||||
stdin.write(DOWN);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("▶ ✓ clean");
|
||||
});
|
||||
|
||||
it("verifies the selected skill on Enter and reports the outcome", async () => {
|
||||
const readResource = vi.fn().mockResolvedValue({
|
||||
result: { contents: [{ uri: "skill://clean/SKILL.md", text: SKILL_MD }] },
|
||||
});
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient(readResource)}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(readResource).toHaveBeenCalled();
|
||||
const frame = lastFrame() ?? "";
|
||||
// The fixture's advertised digest is all zeroes, so this is a mismatch —
|
||||
// which is the outcome worth showing loudly.
|
||||
expect(frame).toContain("Verification FAILED");
|
||||
expect(frame).toContain("✗ SKILL.md");
|
||||
});
|
||||
|
||||
it("surfaces the frontmatter cross-check after verifying", async () => {
|
||||
const lying: SkillEntry = {
|
||||
...clean,
|
||||
frontmatter: { name: "clean", description: "Something else entirely" },
|
||||
};
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[lying]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Frontmatter cross-check:");
|
||||
expect(frame).toContain("Something else entirely");
|
||||
});
|
||||
|
||||
it("reports an ordinary read failure as a failed verdict, not a crash", async () => {
|
||||
// `verifySkills` records a plain read failure per file rather than
|
||||
// throwing, so the pane shows the verdict rather than the error banner.
|
||||
const readResource = vi.fn().mockRejectedValue(new Error("network down"));
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient(readResource)}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("Verification FAILED");
|
||||
expect(lastFrame() ?? "").toContain("network down");
|
||||
});
|
||||
|
||||
it("hands an auth-recovery error to the callback instead of rendering it", async () => {
|
||||
// The one error `verifySkills` re-throws: the session's authorization
|
||||
// expired, and this callback is how the TUI offers to fix it. Rendered as
|
||||
// a message instead, the user would be told the file could not be read and
|
||||
// given no way to recover.
|
||||
const err = new AuthRecoveryRequiredError(
|
||||
new URL("https://auth.example/authorize"),
|
||||
{ reason: "expired" } as never,
|
||||
);
|
||||
const onAuthRecoveryRequired = vi.fn();
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient(vi.fn().mockRejectedValue(err))}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
onAuthRecoveryRequired={onAuthRecoveryRequired}
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(onAuthRecoveryRequired).toHaveBeenCalledWith(err);
|
||||
expect(lastFrame() ?? "").not.toContain("Verification FAILED");
|
||||
});
|
||||
|
||||
it("shows the read failure's own reason under the file it happened on", async () => {
|
||||
// A client missing `readResource` entirely fails every read; the walk
|
||||
// records the reason per file rather than aborting, so the diagnosis lands
|
||||
// beside the file it belongs to.
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={{} as unknown as InspectorClient}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("is not a function");
|
||||
});
|
||||
|
||||
it("does nothing on Enter with no connected client", async () => {
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain(
|
||||
"[Enter to verify digests and frontmatter]",
|
||||
);
|
||||
});
|
||||
|
||||
it("reports a verified skill and re-verifies on a second Enter", async () => {
|
||||
// The digest is computed from the very bytes the fake read returns, so the
|
||||
// pass is real rather than a constant that happens to match.
|
||||
const digest = await sha256Digest(textToBytes(SKILL_MD));
|
||||
const verifiable: SkillEntry = {
|
||||
...clean,
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://clean/SKILL.md",
|
||||
digest,
|
||||
size: textToBytes(SKILL_MD).byteLength,
|
||||
},
|
||||
],
|
||||
};
|
||||
const readResource = vi.fn().mockResolvedValue({
|
||||
result: { contents: [{ uri: "skill://clean/SKILL.md", text: SKILL_MD }] },
|
||||
});
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[verifiable]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient(readResource)}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("Verified — Enter to re-verify");
|
||||
expect(lastFrame() ?? "").toContain("✓ SKILL.md");
|
||||
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(readResource).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("shows a verifying state and ignores Enter while one is in flight", async () => {
|
||||
// The guard is what stops a held Enter from opening a second walk over the
|
||||
// same manifest on top of the first.
|
||||
let release: ((value: unknown) => void) | undefined;
|
||||
const readResource = vi.fn(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
release = resolve;
|
||||
}),
|
||||
);
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient(readResource)}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("[Verifying…]");
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(readResource).toHaveBeenCalledTimes(1);
|
||||
release?.({
|
||||
result: { contents: [{ uri: "skill://clean/SKILL.md", text: SKILL_MD }] },
|
||||
});
|
||||
await tick();
|
||||
});
|
||||
|
||||
it("drops a verdict when the entry changes under the same URI", async () => {
|
||||
// A refresh can replace the manifest or the frontmatter without the URI
|
||||
// moving. A URI-keyed verdict would then present hashes and findings
|
||||
// computed for the PREVIOUS snapshot as if they described the new one.
|
||||
const digest = await sha256Digest(textToBytes(SKILL_MD));
|
||||
const verifiable: SkillEntry = {
|
||||
...clean,
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://clean/SKILL.md",
|
||||
digest,
|
||||
size: textToBytes(SKILL_MD).byteLength,
|
||||
},
|
||||
],
|
||||
};
|
||||
const { lastFrame, stdin, rerender } = render(
|
||||
<SkillsTab
|
||||
skills={[verifiable]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("Verified — Enter to re-verify");
|
||||
|
||||
// Same URI, different manifest — the old verdict must not carry over.
|
||||
rerender(
|
||||
<SkillsTab
|
||||
skills={[
|
||||
{
|
||||
...verifiable,
|
||||
resources: [
|
||||
{ uri: "skill://clean/SKILL.md", digest: CLEAN_DIGEST, size: 9 },
|
||||
],
|
||||
},
|
||||
]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain(
|
||||
"[Enter to verify digests and frontmatter]",
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps a verdict across a reorder that leaves the entry unchanged", async () => {
|
||||
// The reason the key is the entry rather than the list index: moving a
|
||||
// skill down the list must not discard a verdict the user paid for.
|
||||
const digest = await sha256Digest(textToBytes(SKILL_MD));
|
||||
const verifiable: SkillEntry = {
|
||||
...clean,
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://clean/SKILL.md",
|
||||
digest,
|
||||
size: textToBytes(SKILL_MD).byteLength,
|
||||
},
|
||||
],
|
||||
};
|
||||
const { lastFrame, stdin, rerender } = render(
|
||||
<SkillsTab
|
||||
skills={[verifiable]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("Verified — Enter to re-verify");
|
||||
rerender(
|
||||
<SkillsTab
|
||||
skills={[verifiable, dynamic]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("Verified — Enter to re-verify");
|
||||
});
|
||||
|
||||
it("falls back to the whole URI when a manifest entry has no path separator", async () => {
|
||||
const odd: SkillEntry = {
|
||||
uri: "skill://odd/SKILL.md",
|
||||
frontmatter: { name: "odd", description: "d" },
|
||||
resources: [{ uri: "urn:opaque", digest: CLEAN_DIGEST, size: 1 }],
|
||||
};
|
||||
const { lastFrame } = render(
|
||||
<SkillsTab
|
||||
skills={[odd]}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame() ?? "").toContain("urn:opaque");
|
||||
});
|
||||
|
||||
it("renders both rows when a listing repeats a URI", () => {
|
||||
// A malformed listing can carry the same skill twice, and this pane exists
|
||||
// to show BOTH — a URI-keyed row would collide them and let React drop or
|
||||
// reuse one (Copilot).
|
||||
const dup: SkillEntry = {
|
||||
uri: "skill://twice/SKILL.md",
|
||||
frontmatter: { name: "twice", description: "Listed twice" },
|
||||
resources: [],
|
||||
};
|
||||
const { lastFrame } = render(
|
||||
<SkillsTab
|
||||
skills={[dup, dup]}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Skills (2)");
|
||||
expect(frame.match(/twice/g)?.length).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
|
||||
it("keys a row by its index when the entry carries no URI", () => {
|
||||
// A URI-less entry is a `malformed-uri` finding this pane reports, so it
|
||||
// must still render a addressable row rather than colliding React keys.
|
||||
const nameless = {
|
||||
uri: "",
|
||||
frontmatter: { name: "nameless", description: "d" },
|
||||
resources: [],
|
||||
} as SkillEntry;
|
||||
const { lastFrame } = render(
|
||||
<SkillsTab
|
||||
skills={[nameless]}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame() ?? "").toContain("nameless");
|
||||
});
|
||||
|
||||
it("reports a name collision on both entries, as a warning", async () => {
|
||||
// A catalog-level fact `checkSkillConformance` structurally cannot see —
|
||||
// and a warning, because the server did nothing wrong: the obligation is
|
||||
// on the consumer to tell two same-named skills apart.
|
||||
const acme: SkillEntry = {
|
||||
uri: "skill://acme/reports/SKILL.md",
|
||||
frontmatter: { name: "reports", description: "Acme ledger" },
|
||||
resources: [
|
||||
{ uri: "skill://acme/reports/SKILL.md", digest: CLEAN_DIGEST, size: 1 },
|
||||
],
|
||||
};
|
||||
const globex: SkillEntry = {
|
||||
uri: "skill://globex/reports/SKILL.md",
|
||||
frontmatter: { name: "reports", description: "Globex ledger" },
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://globex/reports/SKILL.md",
|
||||
digest: CLEAN_DIGEST,
|
||||
size: 1,
|
||||
},
|
||||
],
|
||||
};
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[acme, globex]}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={160}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
const frame = lastFrame() ?? "";
|
||||
// Both rows carry the warning mark, not the error one.
|
||||
expect(frame.match(/! reports/g)).toHaveLength(2);
|
||||
expect(frame).not.toContain("✗ reports");
|
||||
expect(frame).toContain("also declares the name");
|
||||
// The detail pane names the OTHER skill, which is the disambiguation.
|
||||
expect(frame).toContain("skill://globex/reports/SKILL.md");
|
||||
|
||||
stdin.write(DOWN);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain("skill://acme/reports/SKILL.md");
|
||||
});
|
||||
|
||||
it("shows the digests for a mismatch, not just the failed mark", async () => {
|
||||
// `verifySkillResource` sets `reason` for a SIZE mismatch but not a digest
|
||||
// one, so a pane rendering only `reason` left a bare `✗` with no diagnosis
|
||||
// — a failed verification the reader cannot act on (Copilot).
|
||||
// The declared size must be RIGHT, or the cheaper size cross-check
|
||||
// short-circuits before hashing and reports its own `reason` instead —
|
||||
// which is the path that already rendered.
|
||||
const digestOnly: SkillEntry = {
|
||||
...clean,
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://clean/SKILL.md",
|
||||
digest: CLEAN_DIGEST,
|
||||
size: textToBytes(SKILL_MD).byteLength,
|
||||
},
|
||||
],
|
||||
};
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[digestOnly]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={160}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Verification FAILED");
|
||||
// Truncated to keep the line inside a narrow pane; the CLI report carries
|
||||
// the digests in full.
|
||||
expect(frame).toMatch(/expected sha256:0+…/);
|
||||
expect(frame).toMatch(/got sha256:[0-9a-f]+…/);
|
||||
});
|
||||
|
||||
it("shows the reason for a size mismatch, which carries no digest", async () => {
|
||||
// The other arm: a length disagreement fails before the hash, so there is
|
||||
// no actual digest to print and the reason is the whole diagnosis.
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient()}
|
||||
width={160}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
expect(lastFrame() ?? "").toContain(
|
||||
"Manifest declares 51 bytes but the fetched file is 52.",
|
||||
);
|
||||
});
|
||||
|
||||
it("says a verification was INCOMPLETE rather than merely failed", async () => {
|
||||
// `verifySkills` sets `incomplete` so a consumer can tell "not fully
|
||||
// checked" from a real failure; printing only "Verification FAILED" threw
|
||||
// that distinction away, and the entries beyond the cap stayed marked `·`
|
||||
// with nothing explaining why (Copilot).
|
||||
//
|
||||
// Truncation is triggered by the BYTE budget rather than the 512-entry one
|
||||
// so the manifest stays four rows long: a 512-row pane pushes the status
|
||||
// line off the frame, which would make this assert the test's viewport
|
||||
// rather than the pane's behaviour.
|
||||
//
|
||||
// ⚠️ Every digest and size here is HONEST, so the only thing wrong with
|
||||
// this skill is the unfinished walk. An earlier version understated the
|
||||
// sizes, which is itself a size mismatch — the report was `failed` and the
|
||||
// test passed only because the status line branched on `ok` before
|
||||
// `incomplete`, the very bug this pins (Copilot).
|
||||
const big = "x".repeat(6 * 1024 * 1024);
|
||||
const bigDigest = await sha256Digest(textToBytes(big));
|
||||
const fatMd = "---\nname: fat\ndescription: Four big files\n---\n\n# F\n";
|
||||
const fat: SkillEntry = {
|
||||
uri: "skill://fat/SKILL.md",
|
||||
frontmatter: { name: "fat", description: "Four big files" },
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://fat/SKILL.md",
|
||||
digest: await sha256Digest(textToBytes(fatMd)),
|
||||
size: textToBytes(fatMd).byteLength,
|
||||
},
|
||||
// Three 6 MiB files: the third crosses the 16 MiB budget, so the
|
||||
// manifest is cut before it and one entry is never fetched.
|
||||
...Array.from({ length: 3 }, (_, i) => ({
|
||||
uri: `skill://fat/f${i + 1}.md`,
|
||||
digest: bigDigest,
|
||||
size: textToBytes(big).byteLength,
|
||||
})),
|
||||
],
|
||||
};
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[fat]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient(
|
||||
vi.fn().mockImplementation(async (uri: string) => ({
|
||||
result: {
|
||||
contents: [
|
||||
{
|
||||
uri,
|
||||
text: uri === "skill://fat/SKILL.md" ? fatMd : big,
|
||||
},
|
||||
],
|
||||
},
|
||||
})),
|
||||
)}
|
||||
width={160}
|
||||
height={40}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
await tick();
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Incomplete:");
|
||||
expect(frame).toContain("interoperability limits");
|
||||
expect(frame).toContain("Verification INCOMPLETE");
|
||||
expect(frame).not.toContain("Verification FAILED");
|
||||
});
|
||||
|
||||
it("shows a read failure the manifest does not cover", async () => {
|
||||
// A dynamic skill has no manifest rows, so the synthetic read-error row
|
||||
// `verifySkills` records for its own SKILL.md was rendered nowhere and the
|
||||
// pane said only "Verification FAILED".
|
||||
const { lastFrame, stdin } = render(
|
||||
<SkillsTab
|
||||
skills={[dynamic]}
|
||||
pageCount={1}
|
||||
inspectorClient={mockClient(
|
||||
vi.fn().mockRejectedValue(new Error("upstream gone")),
|
||||
)}
|
||||
width={160}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
stdin.write(ENTER);
|
||||
await tick();
|
||||
const frame = lastFrame() ?? "";
|
||||
expect(frame).toContain("Read failures:");
|
||||
expect(frame).toContain("SKILL.md");
|
||||
expect(frame).toContain("upstream gone");
|
||||
expect(frame).toContain("Verification FAILED");
|
||||
});
|
||||
|
||||
it("shows the details footer only when the details pane is focused", () => {
|
||||
const unfocused = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="list"
|
||||
/>,
|
||||
);
|
||||
expect(unfocused.lastFrame() ?? "").not.toContain("Enter to verify\n");
|
||||
const focused = render(
|
||||
<SkillsTab
|
||||
skills={[clean]}
|
||||
pageCount={1}
|
||||
inspectorClient={null}
|
||||
width={140}
|
||||
height={30}
|
||||
focusedPane="details"
|
||||
/>,
|
||||
);
|
||||
expect(focused.lastFrame() ?? "").toContain(
|
||||
"↑/↓ to scroll, Enter to verify",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -50,6 +50,38 @@ describe("Tabs", () => {
|
|||
expect(lastFrame() ?? "").toContain("Network");
|
||||
});
|
||||
|
||||
it("hides the skills tab by default and shows it when showSkills is true", () => {
|
||||
// A *server-declared* extension (SEP-2640), unlike the transport-derived
|
||||
// gates above — it is only knowable after connecting, so the default has
|
||||
// to be hidden.
|
||||
const hidden = render(
|
||||
<Tabs activeTab="info" onTabChange={noop} width={140} />,
|
||||
);
|
||||
expect(hidden.lastFrame() ?? "").not.toContain("Skills");
|
||||
const shown = render(
|
||||
<Tabs
|
||||
activeTab="info"
|
||||
onTabChange={noop}
|
||||
width={140}
|
||||
showSkills={true}
|
||||
/>,
|
||||
);
|
||||
expect(shown.lastFrame() ?? "").toContain("Skills");
|
||||
});
|
||||
|
||||
it("renders a count on the skills tab", () => {
|
||||
const { lastFrame } = render(
|
||||
<Tabs
|
||||
activeTab="skills"
|
||||
onTabChange={noop}
|
||||
width={140}
|
||||
showSkills={true}
|
||||
counts={{ skills: 4 }}
|
||||
/>,
|
||||
);
|
||||
expect(lastFrame() ?? "").toContain("Skills (4)");
|
||||
});
|
||||
|
||||
it("marks the active tab with the ▶ marker", () => {
|
||||
const { lastFrame } = render(
|
||||
<Tabs activeTab="tools" onTabChange={noop} width={120} />,
|
||||
|
|
|
|||
101
clients/tui/__tests__/tabsConfig.test.ts
Normal file
101
clients/tui/__tests__/tabsConfig.test.ts
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
tabBarRows,
|
||||
tabs,
|
||||
visibleTabs,
|
||||
type TabType,
|
||||
} from "../src/components/tabsConfig.js";
|
||||
|
||||
describe("tab accelerators", () => {
|
||||
it("are unique and appear in their own label", () => {
|
||||
const seen = new Set<string>();
|
||||
for (const tab of tabs) {
|
||||
expect(tab.label.toLowerCase()).toContain(tab.accelerator);
|
||||
expect(seen.has(tab.accelerator)).toBe(false);
|
||||
seen.add(tab.accelerator);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("visibleTabs", () => {
|
||||
it("drops every optional tab when nothing is supported", () => {
|
||||
const ids = visibleTabs({
|
||||
showAuth: false,
|
||||
showLogging: false,
|
||||
showRequests: false,
|
||||
showSkills: false,
|
||||
}).map((t) => t.id);
|
||||
expect(ids).not.toContain("auth");
|
||||
expect(ids).not.toContain("logging");
|
||||
expect(ids).not.toContain("requests");
|
||||
expect(ids).not.toContain("skills");
|
||||
// The unconditional ones remain.
|
||||
expect(ids).toContain("info");
|
||||
expect(ids).toContain("tools");
|
||||
});
|
||||
|
||||
it("keeps each optional tab when its flag is set", () => {
|
||||
const ids = visibleTabs({
|
||||
showAuth: true,
|
||||
showLogging: true,
|
||||
showRequests: true,
|
||||
showSkills: true,
|
||||
}).map((t) => t.id);
|
||||
expect(ids).toEqual(tabs.map((t) => t.id));
|
||||
});
|
||||
});
|
||||
|
||||
describe("tabBarRows (#2248)", () => {
|
||||
/** A stdio, OAuth-capable, Skills-serving server: the widest ordinary bar. */
|
||||
const stdioSkills = visibleTabs({
|
||||
showAuth: true,
|
||||
showLogging: true,
|
||||
showRequests: false,
|
||||
showSkills: true,
|
||||
});
|
||||
const counts: Partial<Record<TabType, number>> = {
|
||||
resources: 0,
|
||||
prompts: 0,
|
||||
skills: 8,
|
||||
tools: 1,
|
||||
messages: 11,
|
||||
logging: 3,
|
||||
};
|
||||
|
||||
it("wraps that bar at 80 columns", () => {
|
||||
// The regression this exists for: adding Skills pushed the bar past a
|
||||
// default terminal, while `App` assumed one row and sized every pane below
|
||||
// it one row too tall.
|
||||
expect(tabBarRows(stdioSkills, counts, 80)).toBeGreaterThan(1);
|
||||
});
|
||||
|
||||
it("needs only one row when the bar fits", () => {
|
||||
expect(tabBarRows(stdioSkills, counts, 400)).toBe(1);
|
||||
});
|
||||
|
||||
it("never reports fewer rows as the terminal narrows", () => {
|
||||
// Monotonicity is the property that matters: a narrower terminal can only
|
||||
// need the same number of rows or more, so a pane sized from this can
|
||||
// never grow into the bar.
|
||||
let previous = 1;
|
||||
for (const width of [400, 200, 132, 100, 80, 60, 40, 20]) {
|
||||
const rows = tabBarRows(stdioSkills, counts, width);
|
||||
expect(rows).toBeGreaterThanOrEqual(previous);
|
||||
previous = rows;
|
||||
}
|
||||
});
|
||||
|
||||
it("gives a tab wider than the row its own row rather than looping", () => {
|
||||
expect(tabBarRows(stdioSkills, counts, 1)).toBe(stdioSkills.length);
|
||||
});
|
||||
|
||||
it("counts the count suffixes, which are what tip it over", () => {
|
||||
const withCounts = tabBarRows(stdioSkills, counts, 100);
|
||||
const without = tabBarRows(stdioSkills, {}, 100);
|
||||
expect(withCounts).toBeGreaterThanOrEqual(without);
|
||||
});
|
||||
|
||||
it("returns one row for an empty bar", () => {
|
||||
expect(tabBarRows([], {}, 80)).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -27,6 +27,7 @@ import {
|
|||
ManagedResourcesState,
|
||||
ManagedResourceTemplatesState,
|
||||
ManagedPromptsState,
|
||||
ManagedSkillsState,
|
||||
MessageLogState,
|
||||
FetchRequestLogState,
|
||||
StderrLogState,
|
||||
|
|
@ -40,6 +41,7 @@ import { useManagedTools } from "@inspector/core/react/useManagedTools.js";
|
|||
import { useManagedResources } from "@inspector/core/react/useManagedResources.js";
|
||||
import { useManagedResourceTemplates } from "@inspector/core/react/useManagedResourceTemplates.js";
|
||||
import { useManagedPrompts } from "@inspector/core/react/useManagedPrompts.js";
|
||||
import { useManagedSkills } from "@inspector/core/react/useManagedSkills.js";
|
||||
import { useMessageLog } from "@inspector/core/react/useMessageLog.js";
|
||||
import { useFetchRequestLog } from "@inspector/core/react/useFetchRequestLog.js";
|
||||
import { useStderrLog } from "@inspector/core/react/useStderrLog.js";
|
||||
|
|
@ -79,6 +81,8 @@ import { InfoTab } from "./components/InfoTab.js";
|
|||
import { AuthTab } from "./components/AuthTab.js";
|
||||
import { ResourcesTab } from "./components/ResourcesTab.js";
|
||||
import { PromptsTab } from "./components/PromptsTab.js";
|
||||
import { tabBarRows, visibleTabs } from "./components/tabsConfig.js";
|
||||
import { SkillsTab } from "./components/SkillsTab.js";
|
||||
import { ToolsTab } from "./components/ToolsTab.js";
|
||||
import { NotificationsTab } from "./components/NotificationsTab.js";
|
||||
import { HistoryTab } from "./components/HistoryTab.js";
|
||||
|
|
@ -153,6 +157,7 @@ function App({
|
|||
info?: number;
|
||||
resources?: number;
|
||||
prompts?: number;
|
||||
skills?: number;
|
||||
tools?: number;
|
||||
messages?: number;
|
||||
requests?: number;
|
||||
|
|
@ -244,6 +249,9 @@ function App({
|
|||
const [managedPromptsStates, setManagedPromptsStates] = useState<
|
||||
Record<string, ManagedPromptsState>
|
||||
>({});
|
||||
const [managedSkillsStates, setManagedSkillsStates] = useState<
|
||||
Record<string, ManagedSkillsState>
|
||||
>({});
|
||||
const [messageLogStates, setMessageLogStates] = useState<
|
||||
Record<string, MessageLogState>
|
||||
>({});
|
||||
|
|
@ -293,6 +301,7 @@ function App({
|
|||
ManagedResourceTemplatesState
|
||||
> = {};
|
||||
const newManagedPromptsStates: Record<string, ManagedPromptsState> = {};
|
||||
const newManagedSkillsStates: Record<string, ManagedSkillsState> = {};
|
||||
const newMessageLogStates: Record<string, MessageLogState> = {};
|
||||
const newFetchRequestLogStates: Record<string, FetchRequestLogState> = {};
|
||||
const newStderrLogStates: Record<string, StderrLogState> = {};
|
||||
|
|
@ -367,6 +376,7 @@ function App({
|
|||
newManagedResourceTemplatesStates[serverName] =
|
||||
new ManagedResourceTemplatesState(client);
|
||||
newManagedPromptsStates[serverName] = new ManagedPromptsState(client);
|
||||
newManagedSkillsStates[serverName] = new ManagedSkillsState(client);
|
||||
newMessageLogStates[serverName] = new MessageLogState(client);
|
||||
newFetchRequestLogStates[serverName] = new FetchRequestLogState(client);
|
||||
newStderrLogStates[serverName] = new StderrLogState(client);
|
||||
|
|
@ -387,6 +397,10 @@ function App({
|
|||
...prev,
|
||||
...newManagedPromptsStates,
|
||||
}));
|
||||
setManagedSkillsStates((prev) => ({
|
||||
...prev,
|
||||
...newManagedSkillsStates,
|
||||
}));
|
||||
setMessageLogStates((prev) => ({ ...prev, ...newMessageLogStates }));
|
||||
setFetchRequestLogStates((prev) => ({
|
||||
...prev,
|
||||
|
|
@ -420,6 +434,9 @@ function App({
|
|||
Object.values(managedPromptsStates).forEach((manager) => {
|
||||
manager.destroy();
|
||||
});
|
||||
Object.values(managedSkillsStates).forEach((manager) => {
|
||||
manager.destroy();
|
||||
});
|
||||
Object.values(messageLogStates).forEach((manager) => {
|
||||
manager.destroy();
|
||||
});
|
||||
|
|
@ -441,6 +458,7 @@ function App({
|
|||
managedResourcesStates,
|
||||
managedResourceTemplatesStates,
|
||||
managedPromptsStates,
|
||||
managedSkillsStates,
|
||||
messageLogStates,
|
||||
fetchRequestLogStates,
|
||||
stderrLogStates,
|
||||
|
|
@ -586,10 +604,50 @@ function App({
|
|||
selectedInspectorClient,
|
||||
selectedManagedResourceTemplatesState,
|
||||
);
|
||||
const selectedManagedSkillsState = useMemo(
|
||||
() =>
|
||||
selectedServer && managedSkillsStates[selectedServer]
|
||||
? managedSkillsStates[selectedServer]
|
||||
: null,
|
||||
[selectedServer, managedSkillsStates],
|
||||
);
|
||||
const { prompts: managedPrompts } = useManagedPrompts(
|
||||
selectedInspectorClient,
|
||||
selectedManagedPromptsState,
|
||||
);
|
||||
const {
|
||||
skills: managedSkills,
|
||||
pageCount: managedSkillsPageCount,
|
||||
error: managedSkillsError,
|
||||
} = useManagedSkills(selectedInspectorClient, selectedManagedSkillsState);
|
||||
// A *server-declared* extension, so it is only knowable after connecting —
|
||||
// unlike the transport-derived `showLoggingTab` / `showRequestsTab` above.
|
||||
const showSkillsTab =
|
||||
!!selectedServer &&
|
||||
!!selectedInspectorClient?.getSkillsExtension() &&
|
||||
inspectorStatus === "connected";
|
||||
|
||||
// Switch away from the Skills tab when the selected server does not serve it.
|
||||
//
|
||||
// The same handling the Auth tab gets above, and needed for the same reason:
|
||||
// the tab disappears from the bar when the gate goes false, but `activeTab`
|
||||
// is independent of the bar, so the render branch would keep showing the pane
|
||||
// for a server that never declared the extension — content the user can see
|
||||
// but can no longer navigate back to (Copilot).
|
||||
//
|
||||
// Gated on `connected` rather than on the extension alone: the declaration is
|
||||
// only knowable after the handshake, so resetting while a reconnect is in
|
||||
// flight would bounce the user off the tab they were reading and not return
|
||||
// them to it.
|
||||
useEffect(() => {
|
||||
if (
|
||||
activeTab === "skills" &&
|
||||
inspectorStatus === "connected" &&
|
||||
!showSkillsTab
|
||||
) {
|
||||
setActiveTab("info");
|
||||
}
|
||||
}, [activeTab, inspectorStatus, showSkillsTab]);
|
||||
|
||||
// Connect — on 401 or mid-session auth recovery, run OAuth then retry.
|
||||
type TuiOAuthRunResult =
|
||||
|
|
@ -1347,6 +1405,7 @@ function App({
|
|||
setTabCounts({
|
||||
resources: managedResources.length || 0,
|
||||
prompts: managedPrompts.length || 0,
|
||||
skills: managedSkills.length || 0,
|
||||
tools: managedTools.length || 0,
|
||||
messages: inspectorMessages.length || 0,
|
||||
requests: inspectorFetchRequests.length || 0,
|
||||
|
|
@ -1356,6 +1415,7 @@ function App({
|
|||
selectedServer,
|
||||
managedResources,
|
||||
managedPrompts,
|
||||
managedSkills,
|
||||
managedTools,
|
||||
inspectorMessages,
|
||||
inspectorFetchRequests,
|
||||
|
|
@ -1430,6 +1490,7 @@ function App({
|
|||
if (tab.id === "auth" && !showAuthTab) return false;
|
||||
if (tab.id === "logging" && !showLoggingTab) return false;
|
||||
if (tab.id === "requests" && !showRequestsTab) return false;
|
||||
if (tab.id === "skills" && !showSkillsTab) return false;
|
||||
return true;
|
||||
})
|
||||
.map((tab: { id: TabType; label: string; accelerator: string }) => [
|
||||
|
|
@ -1517,6 +1578,7 @@ function App({
|
|||
"auth",
|
||||
"resources",
|
||||
"prompts",
|
||||
"skills",
|
||||
"tools",
|
||||
"messages",
|
||||
"requests",
|
||||
|
|
@ -1526,6 +1588,7 @@ function App({
|
|||
if (t === "auth" && !showAuthTab) return false;
|
||||
if (t === "logging" && !showLoggingTab) return false;
|
||||
if (t === "requests" && !showRequestsTab) return false;
|
||||
if (t === "skills" && !showSkillsTab) return false;
|
||||
return true;
|
||||
});
|
||||
const currentIndex = tabs.indexOf(activeTab);
|
||||
|
|
@ -1559,14 +1622,37 @@ function App({
|
|||
|
||||
// Calculate layout dimensions
|
||||
const headerHeight = 1;
|
||||
const tabsHeight = 1;
|
||||
const serverListWidth = Math.floor(dimensions.width * 0.3);
|
||||
const contentWidth = dimensions.width - serverListWidth;
|
||||
// Derived, not assumed. The bar wraps once the visible tabs exceed the
|
||||
// terminal width — which a stdio server with Skills does at any ordinary
|
||||
// width — and a hard-coded 1 sized every pane below it one row too tall,
|
||||
// clipping the bottom of the TUI (Copilot).
|
||||
const tabsHeight = tabBarRows(
|
||||
visibleTabs({
|
||||
showAuth: !!(
|
||||
selectedServer &&
|
||||
selectedServerConfig &&
|
||||
isOAuthCapableServerConfig(selectedServerConfig)
|
||||
),
|
||||
showLogging:
|
||||
!!selectedServer &&
|
||||
inspectorClients[selectedServer]?.getServerType() === "stdio",
|
||||
showRequests:
|
||||
!!selectedServer &&
|
||||
(inspectorClients[selectedServer]?.getServerType() === "sse" ||
|
||||
inspectorClients[selectedServer]?.getServerType() ===
|
||||
"streamable-http"),
|
||||
showSkills: showSkillsTab,
|
||||
}),
|
||||
tabCounts,
|
||||
contentWidth,
|
||||
);
|
||||
// Server details will be flexible - calculate remaining space for content
|
||||
const availableHeight = dimensions.height - headerHeight - tabsHeight;
|
||||
// Reserve space for server details (will grow as needed, but we'll use flexGrow)
|
||||
const serverDetailsMinHeight = 3;
|
||||
const contentHeight = availableHeight - serverDetailsMinHeight;
|
||||
const serverListWidth = Math.floor(dimensions.width * 0.3);
|
||||
const contentWidth = dimensions.width - serverListWidth;
|
||||
|
||||
const getStatusColor = (status: string) => {
|
||||
switch (status) {
|
||||
|
|
@ -1763,6 +1849,7 @@ function App({
|
|||
? inspectorClients[selectedServer].getServerType() === "stdio"
|
||||
: false
|
||||
}
|
||||
showSkills={showSkillsTab}
|
||||
showRequests={
|
||||
selectedServer && inspectorClients[selectedServer]
|
||||
? (() => {
|
||||
|
|
@ -1967,6 +2054,34 @@ function App({
|
|||
)
|
||||
}
|
||||
/>
|
||||
) : activeTab === "skills" &&
|
||||
currentServerState?.status === "connected" &&
|
||||
selectedInspectorClient ? (
|
||||
<SkillsTab
|
||||
key={`skills-${selectedServer}`}
|
||||
skills={managedSkills}
|
||||
pageCount={managedSkillsPageCount}
|
||||
loadError={managedSkillsError}
|
||||
inspectorClient={selectedInspectorClient}
|
||||
width={contentWidth}
|
||||
height={contentHeight}
|
||||
focusedPane={
|
||||
focus === "tabContentDetails"
|
||||
? "details"
|
||||
: focus === "tabContentList"
|
||||
? "list"
|
||||
: null
|
||||
}
|
||||
onAuthRecoveryRequired={onAuthRecoveryRequired}
|
||||
modalOpen={
|
||||
!!(
|
||||
toolTestModal ||
|
||||
resourceTestModal ||
|
||||
promptTestModal ||
|
||||
detailsModal
|
||||
)
|
||||
}
|
||||
/>
|
||||
) : activeTab === "prompts" &&
|
||||
currentServerState?.status === "connected" &&
|
||||
selectedInspectorClient ? (
|
||||
|
|
|
|||
576
clients/tui/src/components/SkillsTab.tsx
Normal file
576
clients/tui/src/components/SkillsTab.tsx
Normal file
|
|
@ -0,0 +1,576 @@
|
|||
/**
|
||||
* The TUI's Skills pane — the SEP-2640 catalog in a terminal (#2248).
|
||||
*
|
||||
* **Why the TUI owns a pane rather than reusing the web screen's logic.** It
|
||||
* does reuse everything that decides an answer: `checkSkillConformance`,
|
||||
* `checkSkillNameCollisions` and `verifySkills` all live in `core/` and are
|
||||
* driven identically here, so a verdict cannot differ depending on which client
|
||||
* you asked. What is local is presentation, and the terminal's constraints are
|
||||
* genuinely different — two panes in 80 columns, no colour to rely on, and a
|
||||
* keyboard rather than a pointer.
|
||||
*
|
||||
* ⚠️ **Severity is carried by a glyph as well as a colour** (`✓` / `!` / `✗`).
|
||||
* This pane is read over ssh, inside tmux, and piped through `script(1)`, where
|
||||
* colour may not survive; a row whose only signal was `red` would then be
|
||||
* indistinguishable from a clean one.
|
||||
*
|
||||
* The pane is shown only when the connected server declares the extension —
|
||||
* that gate, and the reset that leaves the tab when it goes false, live in
|
||||
* `App.tsx` because they are navigation concerns rather than this pane's.
|
||||
*/
|
||||
import React, { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { Box, Text, useInput, type Key } from "ink";
|
||||
import { ScrollView, type ScrollViewRef } from "ink-scroll-view";
|
||||
import type { InspectorClient } from "@inspector/core/mcp/index.js";
|
||||
import { AuthRecoveryRequiredError } from "@inspector/core/auth/challenge.js";
|
||||
import {
|
||||
checkSkillConformance,
|
||||
checkSkillNameCollisions,
|
||||
skillDisplayName,
|
||||
skillEntryKey,
|
||||
skillUriIdentity,
|
||||
type SkillIssue,
|
||||
} from "@inspector/core/mcp/skills.js";
|
||||
import {
|
||||
DYNAMIC_RESOURCES,
|
||||
type SkillEntry,
|
||||
} from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import {
|
||||
verifySkills,
|
||||
type SkillFileReport,
|
||||
type SkillVerifyReport,
|
||||
} from "@inspector/core/mcp/skillsVerification.js";
|
||||
import { useSelectableList } from "../hooks/useSelectableList.js";
|
||||
|
||||
interface SkillsTabProps {
|
||||
skills: SkillEntry[];
|
||||
/** Pages the last `skills/list` walk took; shown so pagination is visible. */
|
||||
pageCount: number;
|
||||
/** A failed list walk, rendered in place of the list. */
|
||||
loadError?: Error | null;
|
||||
inspectorClient: InspectorClient | null;
|
||||
width: number;
|
||||
height: number;
|
||||
focusedPane?: "list" | "details" | null;
|
||||
onAuthRecoveryRequired?: (error: AuthRecoveryRequiredError) => void;
|
||||
modalOpen?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* The character that leads a finding line, by severity. A terminal pane cannot
|
||||
* lean on colour alone — the Inspector is run over ssh, in tmux, and piped
|
||||
* through `script(1)` — so severity is carried by a glyph as well as a colour.
|
||||
*/
|
||||
const ISSUE_MARK: Record<SkillIssue["severity"], string> = {
|
||||
error: "✗",
|
||||
warning: "!",
|
||||
};
|
||||
|
||||
const ISSUE_COLOR: Record<SkillIssue["severity"], string> = {
|
||||
error: "red",
|
||||
warning: "yellow",
|
||||
};
|
||||
|
||||
/** Per-file verification glyph, same reasoning as {@link ISSUE_MARK}. */
|
||||
const FILE_MARK: Record<string, string> = {
|
||||
verified: "✓",
|
||||
mismatch: "✗",
|
||||
unverifiable: "?",
|
||||
error: "✗",
|
||||
"read-error": "✗",
|
||||
};
|
||||
|
||||
const FILE_COLOR: Record<string, string> = {
|
||||
verified: "green",
|
||||
mismatch: "red",
|
||||
unverifiable: "yellow",
|
||||
error: "red",
|
||||
"read-error": "red",
|
||||
};
|
||||
|
||||
/**
|
||||
* The status line for each of the three verification outcomes.
|
||||
*
|
||||
* A `Record` over the union rather than a chain of ternaries, so adding a
|
||||
* fourth outcome is a type error here instead of a silently missing label.
|
||||
*/
|
||||
const VERIFY_STATUS: Record<SkillVerifyReport["outcome"], string> = {
|
||||
verified: "[Verified — Enter to re-verify]",
|
||||
incomplete: "[Verification INCOMPLETE — Enter to re-verify]",
|
||||
failed: "[Verification FAILED — Enter to re-verify]",
|
||||
};
|
||||
|
||||
/**
|
||||
* The explanation printed under a failed file row.
|
||||
*
|
||||
* `verifySkillResource` sets `reason` for a SIZE mismatch but not for a digest
|
||||
* one — that carries `expectedDigest` / `actualDigest` instead — so a pane that
|
||||
* rendered only `reason` showed a bare `✗ notes.md` and never said why, leaving
|
||||
* the failure unactionable (Copilot). Digests are truncated because the pane is
|
||||
* 40-odd columns wide and the first bytes are enough to see that two differ;
|
||||
* the CLI report carries them in full.
|
||||
*/
|
||||
function failureDetail(file: SkillFileReport): string | undefined {
|
||||
if (file.reason) return file.reason;
|
||||
if (file.status !== "mismatch") return undefined;
|
||||
const short = (d: string | undefined) => (d ? `${d.slice(0, 23)}…` : "—");
|
||||
return `expected ${short(file.expectedDigest)}, got ${short(file.actualDigest)}`;
|
||||
}
|
||||
|
||||
/** The file name a manifest URI ends in, for a list that must fit 40 columns. */
|
||||
function fileNameOf(uri: string): string {
|
||||
const cut = uri.lastIndexOf("/");
|
||||
return cut === -1 ? uri : uri.slice(cut + 1);
|
||||
}
|
||||
|
||||
/**
|
||||
* The Skills pane (SEP-2640, #2248): the catalog on the left, and on the right
|
||||
* the selected skill's frontmatter, its conformance findings, and its manifest.
|
||||
*
|
||||
* **Enter verifies.** The static checks run on every render — they are a pure
|
||||
* walk over a list already in memory — but digest verification needs the bytes,
|
||||
* so it is one `resources/read` per manifest entry and must be asked for. That
|
||||
* split is the same one the web screen makes and the same one SEP-2640 makes:
|
||||
* hosts MUST NOT retrieve a skill's files ahead of need.
|
||||
*/
|
||||
export function SkillsTab({
|
||||
skills,
|
||||
pageCount,
|
||||
loadError = null,
|
||||
inspectorClient,
|
||||
width,
|
||||
height,
|
||||
focusedPane = null,
|
||||
onAuthRecoveryRequired,
|
||||
modalOpen = false,
|
||||
}: SkillsTabProps) {
|
||||
const visibleCount = Math.max(1, height - 7);
|
||||
const { selectedIndex, firstVisible, setSelection } = useSelectableList(
|
||||
skills.length,
|
||||
visibleCount,
|
||||
{ resetWhen: skills },
|
||||
);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [verifying, setVerifying] = useState(false);
|
||||
/**
|
||||
* The last verification, keyed by the **entry it was computed against**.
|
||||
*
|
||||
* Keyed rather than cleared on selection change, so moving off a skill and
|
||||
* back does not silently discard a verdict the user just paid a round trip
|
||||
* for. Keyed by a serialization of the entry rather than by its index, so a
|
||||
* refresh that reorders the list cannot show one skill's verdict under
|
||||
* another's name — and rather than by its URI alone, because a refresh can
|
||||
* replace the manifest or the frontmatter *under the same URI*, and a
|
||||
* URI-keyed verdict would then present hashes and findings computed for the
|
||||
* previous snapshot as if they described the new one (Copilot).
|
||||
*
|
||||
* The same key the web screen uses, for the same reason: re-verifying after a
|
||||
* metadata-only refresh is the cheap direction to be wrong in; showing a
|
||||
* verdict computed against a different entry is not.
|
||||
*/
|
||||
const [report, setReport] = useState<{
|
||||
key: string;
|
||||
result: SkillVerifyReport;
|
||||
} | null>(null);
|
||||
const scrollViewRef = useRef<ScrollViewRef>(null);
|
||||
|
||||
const selectedSkill = skills[selectedIndex] ?? null;
|
||||
|
||||
const runVerify = useCallback(
|
||||
(skill: SkillEntry) => {
|
||||
if (!inspectorClient || verifying) return;
|
||||
setVerifying(true);
|
||||
setError(null);
|
||||
// The IIFE catches everything it can throw, so there is no rejection for
|
||||
// this key handler — which cannot await — to own.
|
||||
void (async () => {
|
||||
try {
|
||||
const [result] = await verifySkills(inspectorClient, [skill]);
|
||||
setReport({ key: skillEntryKey(skill), result });
|
||||
} catch (err) {
|
||||
if (err instanceof AuthRecoveryRequiredError) {
|
||||
onAuthRecoveryRequired?.(err);
|
||||
return;
|
||||
}
|
||||
/* v8 ignore start -- `verifySkills` records an ordinary read failure
|
||||
against the file it happened on and keeps walking, and it re-throws
|
||||
exactly one error, handled directly above. So nothing the call
|
||||
graph can produce reaches here; this is the guard that keeps a
|
||||
future change from becoming an unhandled rejection instead of a
|
||||
visible message. Exercising it would mean faking a throw the walk
|
||||
cannot make, which tests the fake rather than the code. */
|
||||
setError(
|
||||
err instanceof Error ? err.message : "Failed to verify skill",
|
||||
);
|
||||
/* v8 ignore stop */
|
||||
} finally {
|
||||
setVerifying(false);
|
||||
}
|
||||
})();
|
||||
},
|
||||
[inspectorClient, onAuthRecoveryRequired, verifying],
|
||||
);
|
||||
|
||||
useInput(
|
||||
(input: string, key: Key) => {
|
||||
if (key.return && selectedSkill && inspectorClient) {
|
||||
runVerify(selectedSkill);
|
||||
return;
|
||||
}
|
||||
if (focusedPane === "list") {
|
||||
if (key.upArrow && selectedIndex > 0) {
|
||||
setSelection(selectedIndex - 1);
|
||||
} else if (key.downArrow && selectedIndex < skills.length - 1) {
|
||||
setSelection(selectedIndex + 1);
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (focusedPane === "details") {
|
||||
if (key.upArrow) {
|
||||
scrollViewRef.current?.scrollBy(-1);
|
||||
} else if (key.downArrow) {
|
||||
scrollViewRef.current?.scrollBy(1);
|
||||
} else if (key.pageUp) {
|
||||
const viewportHeight =
|
||||
scrollViewRef.current?.getViewportHeight() || 1;
|
||||
scrollViewRef.current?.scrollBy(-viewportHeight);
|
||||
} else if (key.pageDown) {
|
||||
const viewportHeight =
|
||||
scrollViewRef.current?.getViewportHeight() || 1;
|
||||
scrollViewRef.current?.scrollBy(viewportHeight);
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
isActive:
|
||||
!modalOpen && (focusedPane === "list" || focusedPane === "details"),
|
||||
},
|
||||
);
|
||||
|
||||
// Reset scroll when selection changes. A genuine synchronization with an
|
||||
// external system (the ScrollView's imperative handle), not state derived
|
||||
// from a prop — so an effect is the right tool here.
|
||||
useEffect(() => {
|
||||
scrollViewRef.current?.scrollTo(0);
|
||||
}, [selectedIndex]);
|
||||
|
||||
const listWidth = Math.floor(width * 0.4);
|
||||
const detailWidth = width - listWidth;
|
||||
// A name collision is a property of the LISTING, not of an entry, so it is
|
||||
// computed once here and merged into each entry's own findings — which is
|
||||
// what carries it into the row marks below as well as the detail pane.
|
||||
const collisions = checkSkillNameCollisions(skills);
|
||||
const findingsFor = (skill: SkillEntry): SkillIssue[] => {
|
||||
const collision = collisions.get(skillUriIdentity(skill.uri));
|
||||
return [...checkSkillConformance(skill), ...(collision ? [collision] : [])];
|
||||
};
|
||||
const issues = selectedSkill ? findingsFor(selectedSkill) : [];
|
||||
const activeReport =
|
||||
selectedSkill && report?.key === skillEntryKey(selectedSkill)
|
||||
? report.result
|
||||
: null;
|
||||
const manifest =
|
||||
selectedSkill && selectedSkill.resources !== DYNAMIC_RESOURCES
|
||||
? selectedSkill.resources
|
||||
: [];
|
||||
// Compared on normalized identity, like every other URI comparison here, so a
|
||||
// manifest entry written in an equivalent form is not reported twice.
|
||||
const manifestIdentities = new Set(
|
||||
manifest.map((resource) => skillUriIdentity(resource.uri)),
|
||||
);
|
||||
const extraReportFiles = (activeReport?.files ?? []).filter(
|
||||
(file) => !manifestIdentities.has(skillUriIdentity(file.uri)),
|
||||
);
|
||||
|
||||
return (
|
||||
<Box flexDirection="row" width={width} height={height}>
|
||||
<Box
|
||||
width={listWidth}
|
||||
height={height}
|
||||
borderStyle="single"
|
||||
borderTop={false}
|
||||
borderBottom={false}
|
||||
borderLeft={false}
|
||||
borderRight={true}
|
||||
flexDirection="column"
|
||||
paddingX={1}
|
||||
>
|
||||
<Box paddingY={1}>
|
||||
<Text
|
||||
bold
|
||||
backgroundColor={focusedPane === "list" ? "yellow" : undefined}
|
||||
>
|
||||
Skills ({skills.length}
|
||||
{pageCount > 1 ? `, ${pageCount} pages` : ""})
|
||||
</Text>
|
||||
</Box>
|
||||
{loadError ? (
|
||||
<Box paddingY={1}>
|
||||
<Text color="red">{loadError.message}</Text>
|
||||
</Box>
|
||||
) : skills.length === 0 ? (
|
||||
<Box paddingY={1}>
|
||||
<Text dimColor>No skills available</Text>
|
||||
</Box>
|
||||
) : (
|
||||
<Box
|
||||
flexDirection="column"
|
||||
height={visibleCount}
|
||||
overflow="hidden"
|
||||
flexShrink={0}
|
||||
>
|
||||
{skills
|
||||
.slice(firstVisible, firstVisible + visibleCount)
|
||||
.map((skill, i) => {
|
||||
const index = firstVisible + i;
|
||||
const isSelected = index === selectedIndex;
|
||||
// The per-row mark is the static conformance verdict, which
|
||||
// costs nothing — it is what makes a bad skill visible in the
|
||||
// list rather than only after selecting it.
|
||||
const rowIssues = findingsFor(skill);
|
||||
const worst = rowIssues.some((it) => it.severity === "error")
|
||||
? "error"
|
||||
: rowIssues.length > 0
|
||||
? "warning"
|
||||
: null;
|
||||
return (
|
||||
// Index-keyed like the manifest and finding rows, and for
|
||||
// the same reason: a malformed listing can repeat a URI, and
|
||||
// this pane exists to show BOTH entries — a URI key would
|
||||
// collide them and let React drop or reuse the wrong row
|
||||
// (Copilot).
|
||||
<Box
|
||||
key={`${index}:${skill.uri}`}
|
||||
paddingY={0}
|
||||
flexShrink={0}
|
||||
>
|
||||
<Text>
|
||||
{isSelected ? "▶ " : " "}
|
||||
{worst ? (
|
||||
<Text color={ISSUE_COLOR[worst]}>
|
||||
{ISSUE_MARK[worst]}{" "}
|
||||
</Text>
|
||||
) : (
|
||||
<Text color="green">✓ </Text>
|
||||
)}
|
||||
{skillDisplayName(skill)}
|
||||
</Text>
|
||||
</Box>
|
||||
);
|
||||
})}
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
|
||||
<Box
|
||||
width={detailWidth}
|
||||
height={height}
|
||||
paddingX={1}
|
||||
flexDirection="column"
|
||||
overflow="hidden"
|
||||
>
|
||||
{selectedSkill ? (
|
||||
<>
|
||||
<Box flexShrink={0} paddingTop={1}>
|
||||
<Text
|
||||
bold
|
||||
backgroundColor={
|
||||
focusedPane === "details" ? "yellow" : undefined
|
||||
}
|
||||
{...(focusedPane === "details" ? {} : { color: "cyan" })}
|
||||
>
|
||||
{skillDisplayName(selectedSkill)}
|
||||
</Text>
|
||||
</Box>
|
||||
|
||||
<ScrollView ref={scrollViewRef} height={height - 5}>
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text dimColor>{selectedSkill.uri}</Text>
|
||||
</Box>
|
||||
{selectedSkill.frontmatter.description && (
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text dimColor>{selectedSkill.frontmatter.description}</Text>
|
||||
</Box>
|
||||
)}
|
||||
|
||||
{/* Named for the checks it actually covers. An unqualified
|
||||
"conforms" sat directly above "Verification FAILED" in the
|
||||
same pane and flatly contradicted it — these are the static
|
||||
checks against the LISTING, and passing them says nothing
|
||||
about the bytes the server serves (Copilot). Same wording
|
||||
problem, and the same fix, as the web screen's "No structural
|
||||
issues". */}
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text bold>
|
||||
Listing checks
|
||||
{issues.length === 0 ? ": no structural issues" : ":"}
|
||||
</Text>
|
||||
</Box>
|
||||
{issues.map((issue, idx) => (
|
||||
<Box key={`issue-${idx}`} paddingLeft={2} flexShrink={0}>
|
||||
<Text color={ISSUE_COLOR[issue.severity]}>
|
||||
{ISSUE_MARK[issue.severity]} {issue.message}
|
||||
</Text>
|
||||
</Box>
|
||||
))}
|
||||
|
||||
{/* ABOVE the manifest, because it explains the list that
|
||||
follows: only the first N rows were fetched, and the rest
|
||||
stay marked `·` because nobody looked at them. Below a
|
||||
512-row manifest it would be off-screen, which is the same as
|
||||
absent. `verifySkills` sets `incomplete` precisely so a
|
||||
consumer can tell "not fully checked" from a real failure
|
||||
(Copilot). */}
|
||||
{activeReport?.incomplete && (
|
||||
<>
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text bold color="yellow">
|
||||
Incomplete:
|
||||
</Text>
|
||||
</Box>
|
||||
<Box paddingLeft={2} flexShrink={0}>
|
||||
<Text color="yellow">{activeReport.incomplete}</Text>
|
||||
</Box>
|
||||
</>
|
||||
)}
|
||||
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text bold>
|
||||
Manifest
|
||||
{selectedSkill.resources === DYNAMIC_RESOURCES
|
||||
? ': "dynamic" — no files advertised'
|
||||
: ` (${manifest.length})`}
|
||||
</Text>
|
||||
</Box>
|
||||
{manifest.map((resource, idx) => {
|
||||
// Matched on normalized identity, like the membership test
|
||||
// just above — a raw comparison misses a report row recorded
|
||||
// under an equivalent spelling, while `extraReportFiles`
|
||||
// suppresses it as already covered, and the verdict renders
|
||||
// nowhere (Copilot).
|
||||
const fileReport = activeReport?.files.find(
|
||||
(file) =>
|
||||
skillUriIdentity(file.uri) ===
|
||||
skillUriIdentity(resource.uri),
|
||||
);
|
||||
return (
|
||||
<Box
|
||||
key={`file-${idx}`}
|
||||
paddingLeft={2}
|
||||
flexShrink={0}
|
||||
flexDirection="column"
|
||||
>
|
||||
<Text>
|
||||
{fileReport ? (
|
||||
<Text color={FILE_COLOR[fileReport.status] ?? "white"}>
|
||||
{FILE_MARK[fileReport.status] ?? "?"}{" "}
|
||||
</Text>
|
||||
) : (
|
||||
<Text dimColor>· </Text>
|
||||
)}
|
||||
{fileNameOf(resource.uri)}
|
||||
{resource.size !== undefined ? (
|
||||
<Text dimColor> ({resource.size} B)</Text>
|
||||
) : null}
|
||||
</Text>
|
||||
{fileReport && failureDetail(fileReport) && (
|
||||
<Box paddingLeft={4} flexShrink={0}>
|
||||
<Text color="red">{failureDetail(fileReport)}</Text>
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
);
|
||||
})}
|
||||
|
||||
{/* A report can carry a file the MANIFEST does not — a dynamic
|
||||
skill has no rows at all, yet a failed read of its own
|
||||
SKILL.md is recorded so the failure is visible. Rendering only
|
||||
manifest rows left "Verification FAILED" with no diagnosis
|
||||
anywhere on screen (Copilot). */}
|
||||
{extraReportFiles.length > 0 && (
|
||||
<>
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text bold>Read failures:</Text>
|
||||
</Box>
|
||||
{extraReportFiles.map((file, idx) => (
|
||||
<Box
|
||||
key={`extra-${idx}`}
|
||||
paddingLeft={2}
|
||||
flexShrink={0}
|
||||
flexDirection="column"
|
||||
>
|
||||
<Text>
|
||||
<Text color={FILE_COLOR[file.status] ?? "white"}>
|
||||
{FILE_MARK[file.status] ?? "?"}{" "}
|
||||
</Text>
|
||||
{fileNameOf(file.uri)}
|
||||
</Text>
|
||||
{failureDetail(file) && (
|
||||
<Box paddingLeft={4} flexShrink={0}>
|
||||
<Text color="red">{failureDetail(file)}</Text>
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
))}
|
||||
</>
|
||||
)}
|
||||
|
||||
{activeReport && activeReport.frontmatter.length > 0 && (
|
||||
<>
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text bold>Frontmatter cross-check:</Text>
|
||||
</Box>
|
||||
{activeReport.frontmatter.map((issue, idx) => (
|
||||
<Box key={`fm-${idx}`} paddingLeft={2} flexShrink={0}>
|
||||
<Text color={ISSUE_COLOR[issue.severity]}>
|
||||
{ISSUE_MARK[issue.severity]} {issue.message}
|
||||
</Text>
|
||||
</Box>
|
||||
))}
|
||||
</>
|
||||
)}
|
||||
|
||||
{error && (
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text color="red">{error}</Text>
|
||||
</Box>
|
||||
)}
|
||||
|
||||
<Box marginTop={1} flexShrink={0}>
|
||||
<Text dimColor>
|
||||
{verifying
|
||||
? "[Verifying…]"
|
||||
: activeReport
|
||||
? // ⚠️ Switched on `outcome`, not on `ok`. `ok` stays
|
||||
// true for an `incomplete` report — nothing checked
|
||||
// was wrong — so an `ok`-first branch printed
|
||||
// "Verified" for a walk the read bounds cut short and
|
||||
// the INCOMPLETE arm was unreachable (Copilot).
|
||||
VERIFY_STATUS[activeReport.outcome]
|
||||
: "[Enter to verify digests and frontmatter]"}
|
||||
</Text>
|
||||
</Box>
|
||||
</ScrollView>
|
||||
|
||||
{focusedPane === "details" && (
|
||||
<Box
|
||||
flexShrink={0}
|
||||
height={1}
|
||||
justifyContent="center"
|
||||
backgroundColor="gray"
|
||||
>
|
||||
<Text bold color="white">
|
||||
↑/↓ to scroll, Enter to verify
|
||||
</Text>
|
||||
</Box>
|
||||
)}
|
||||
</>
|
||||
) : (
|
||||
<Box paddingY={1} flexShrink={0}>
|
||||
<Text dimColor>Select a skill to view details</Text>
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import React from "react";
|
||||
import { Box, Text } from "ink";
|
||||
import { type TabType, tabs } from "./tabsConfig.js";
|
||||
import { type TabType, visibleTabs as visibleTabsFor } from "./tabsConfig.js";
|
||||
|
||||
/**
|
||||
* Split a tab label so the accelerator letter can be underlined wherever it
|
||||
|
|
@ -30,6 +30,7 @@ interface TabsProps {
|
|||
auth?: number;
|
||||
resources?: number;
|
||||
prompts?: number;
|
||||
skills?: number;
|
||||
tools?: number;
|
||||
messages?: number;
|
||||
requests?: number;
|
||||
|
|
@ -39,6 +40,13 @@ interface TabsProps {
|
|||
showAuth?: boolean;
|
||||
showLogging?: boolean;
|
||||
showRequests?: boolean;
|
||||
/**
|
||||
* The Skills tab is shown only when the connected server declared the
|
||||
* SEP-2640 Skills extension — unlike Auth/Logging/Requests, which key off the
|
||||
* transport, this one keys off a *server* declaration, so it can only be
|
||||
* known after connecting.
|
||||
*/
|
||||
showSkills?: boolean;
|
||||
}
|
||||
|
||||
export function Tabs({
|
||||
|
|
@ -49,17 +57,16 @@ export function Tabs({
|
|||
showAuth = true,
|
||||
showLogging = true,
|
||||
showRequests = false,
|
||||
showSkills = false,
|
||||
}: TabsProps) {
|
||||
let visibleTabs = tabs;
|
||||
if (!showAuth) {
|
||||
visibleTabs = visibleTabs.filter((tab) => tab.id !== "auth");
|
||||
}
|
||||
if (!showLogging) {
|
||||
visibleTabs = visibleTabs.filter((tab) => tab.id !== "logging");
|
||||
}
|
||||
if (!showRequests) {
|
||||
visibleTabs = visibleTabs.filter((tab) => tab.id !== "requests");
|
||||
}
|
||||
// Shared with `App`, which sizes the pane below this bar from the same list —
|
||||
// see `tabBarRows`.
|
||||
const visibleTabs = visibleTabsFor({
|
||||
showAuth,
|
||||
showLogging,
|
||||
showRequests,
|
||||
showSkills,
|
||||
});
|
||||
|
||||
return (
|
||||
<Box
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ export type TabType =
|
|||
| "auth"
|
||||
| "resources"
|
||||
| "prompts"
|
||||
| "skills"
|
||||
| "tools"
|
||||
| "messages"
|
||||
| "requests"
|
||||
|
|
@ -23,8 +24,83 @@ export const tabs: { id: TabType; label: string; accelerator: string }[] = [
|
|||
{ id: "auth", label: "Auth", accelerator: "a" },
|
||||
{ id: "resources", label: "Resources", accelerator: "r" },
|
||||
{ id: "prompts", label: "Prompts", accelerator: "m" },
|
||||
// `k`, not `s`: `s` is not in conflict today, but the accelerator has to
|
||||
// appear in the label and be unique, and `S`kills against a future `S`ampling
|
||||
// or `S`ettings is the collision this rule anticipates. `k` is the earliest
|
||||
// remaining letter in the word after `s` and `i` (Info).
|
||||
{ id: "skills", label: "Skills", accelerator: "k" },
|
||||
{ id: "tools", label: "Tools", accelerator: "t" },
|
||||
{ id: "messages", label: "Protocol", accelerator: "p" },
|
||||
{ id: "requests", label: "Network", accelerator: "n" },
|
||||
{ id: "logging", label: "Console", accelerator: "o" },
|
||||
];
|
||||
|
||||
/** Which optional tabs the connected server supports. */
|
||||
export interface TabVisibility {
|
||||
showAuth: boolean;
|
||||
showLogging: boolean;
|
||||
showRequests: boolean;
|
||||
showSkills: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* The tabs actually rendered for a given server.
|
||||
*
|
||||
* One source for the answer, shared by `Tabs` (which draws them) and `App`
|
||||
* (which must size the pane below them). Duplicating the filter is how the two
|
||||
* come to disagree about how tall the bar is.
|
||||
*/
|
||||
export function visibleTabs(v: TabVisibility): typeof tabs {
|
||||
return tabs.filter((tab) => {
|
||||
if (tab.id === "auth") return v.showAuth;
|
||||
if (tab.id === "logging") return v.showLogging;
|
||||
if (tab.id === "requests") return v.showRequests;
|
||||
if (tab.id === "skills") return v.showSkills;
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
/** Rendered width of one tab: the 2-column marker, the label, and any count. */
|
||||
function tabWidth(
|
||||
tab: { id: TabType; label: string },
|
||||
counts: Partial<Record<TabType, number>>,
|
||||
): number {
|
||||
const count = counts[tab.id];
|
||||
return (
|
||||
2 + tab.label.length + (count === undefined ? 0 : ` (${count})`.length)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* How many terminal rows the tab bar occupies at a given width.
|
||||
*
|
||||
* ⚠️ **Not always 1.** Adding Skills pushed a stdio server's bar past 100
|
||||
* columns, so it wraps at any ordinary terminal width — and `App` hard-coded
|
||||
* `tabsHeight = 1`, sizing every content pane one row too tall and clipping the
|
||||
* bottom of the TUI (Copilot). Deriving the height from the same list `Tabs`
|
||||
* renders is what keeps the two in agreement as tabs are added.
|
||||
*
|
||||
* The bar is a `flexWrap="wrap"` row with one column of padding each side and
|
||||
* no gaps, so greedy packing by rendered width matches what Ink lays out.
|
||||
*/
|
||||
export function tabBarRows(
|
||||
visible: readonly { id: TabType; label: string }[],
|
||||
counts: Partial<Record<TabType, number>>,
|
||||
width: number,
|
||||
): number {
|
||||
const inner = Math.max(1, width - 2);
|
||||
let rows = 1;
|
||||
let used = 0;
|
||||
for (const tab of visible) {
|
||||
const w = tabWidth(tab, counts);
|
||||
// A tab wider than the whole row still occupies one of its own rather than
|
||||
// looping forever.
|
||||
if (used > 0 && used + w > inner) {
|
||||
rows += 1;
|
||||
used = w;
|
||||
} else {
|
||||
used += w;
|
||||
}
|
||||
}
|
||||
return rows;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -149,6 +149,13 @@ export default defineConfig({
|
|||
"ajv",
|
||||
"atomically",
|
||||
"zod",
|
||||
// Newly on `core/`'s runtime import graph as of #2248:
|
||||
// `core/mcp/skillFile.ts` parses a served SKILL.md's YAML frontmatter to
|
||||
// check it against the entry the listing advertised (SEP-2640). Already a
|
||||
// root `dependency` — it was reached from `test-servers/src` — so this
|
||||
// adds no package, but a root-declared dependency `core/` imports must be
|
||||
// named in all three `external` lists or tsup inlines it here.
|
||||
"yaml",
|
||||
// Reached through `core/` but not through this client's own code today.
|
||||
// AGENTS.md requires every root-declared package `core/` imports at runtime
|
||||
// in ALL three lists regardless, because which client reaches one is a
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue