Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 56 additions & 22 deletions .claude/rules/tool-e2e-handler-capture.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,61 @@
To verify a tool module (`registerXxxTools` in `src/tools/*.ts`) against the **real
GitHub API** without standing up the full Cloudflare Workers + OAuth stack, drive
the handlers directly through the same registration path the server uses, with a
**live** Octokit injected:

1. Write a throwaway vitest spec, e.g. `test/_verify_<area>_live.test.js` (the
`_verify_` prefix marks it disposable — delete it after).
2. In it: `captureHandlers()` from `test/_helpers/tools.js`, then
`registerXxxTools(server, () => new Octokit({ auth: process.env.GITHUB_PERSONAL_ACCESS_TOKEN }))`
— a **real** network Octokit, not the test stub.
3. `invoke(handlers, "<tool_name>", { owner, repo, ... })` against a repo with real
data (e.g. `nemolize/remote-mcp-github`, which has both success and failure
workflow runs). Assert on the rendered Markdown and `console.log` the body so
the output is observable.
4. Run only that file: `pnpm exec vitest run test/_verify_<area>_live.test.js`.
5. **Delete the spec when done** — it is not part of the suite (it needs network
+ a token and would fail in CI).
**live** Octokit injected. Run it as a **standalone script outside vitest**, not as
a `test/_verify_*.test.js` spec — see the gotcha below for why the vitest route does
not authenticate.

1. Write a throwaway script, e.g. `scripts/_verify_<area>_live.mjs` (the `_verify_`
prefix marks it disposable — delete it after).
2. In it: build the handler map inline (`const h = new Map(); registerXxxTools({
registerTool: (n, _c, fn) => h.set(n, fn) }, () => new Octokit({ auth:
process.env.GITHUB_PERSONAL_ACCESS_TOKEN }))`) — a **real** network Octokit, not
the test stub. (`captureHandlers` from `test/_helpers/tools.js` pulls in vitest's
`expect`, so inline the 3-line capture instead of importing it into a plain
script.)
3. Invoke a handler against a repo with real data (e.g. `nemolize/remote-mcp-github`,
which has both success and failure workflow runs), and assert / `console.log` the
rendered Markdown so the output is observable.
4. Run it with `pnpm dlx tsx scripts/_verify_<area>_live.mjs` (tsx resolves the
`.js`-extension imports inside the `.ts` source). `node --experimental-strip-types`
does **not** work — it does not remap `.js`→`.ts` specifiers, so the import of
`../src/mcp/response.js` fails with `ERR_MODULE_NOT_FOUND`.
5. **Delete the script when done** — it needs network + a token and is not part of
the suite.

This is *not* an import-and-call unit test: `registerXxxTools` is the module's
public registration boundary, and `captureHandlers` invokes the exact handler the
public registration boundary, and the inline capture invokes the exact handler the
running server registers — only the Octokit factory is swapped for a live one.

## Gotcha — sandboxed `pnpm exec vitest` fails with `listen EPERM`
### Verifying a write (mutating) tool — use the error path

For write tools (`rerun_*`, `cancel_workflow_run`, `trigger_workflow_dispatch`, …)
a real mutation against this repo is unsafe (a dispatch on `deploy.yml` is a real
production deploy; a cancel disrupts live CI). Verify the **live binding** via the
error path instead — it exercises the real Octokit method, endpoint, request body,
and `wrapTool` error surfacing with **no side effect**:

- `cancel_workflow_run` on an already-**completed** run → real `409 Cannot cancel a
workflow run that is completed`.
- `trigger_workflow_dispatch` on `ci.yml` (which has **no** `workflow_dispatch`
trigger) → real `422 Workflow does not have 'workflow_dispatch' trigger`.

## Gotcha — the Workers vitest pool does NOT expose `process.env` to the isolate

This repo's `vitest.config.mts` runs **every** spec in `@cloudflare/vitest-pool-workers`,
and the Workers isolate's `process.env` is **not** Node's process env — a handler doing
`new Octokit({ auth: process.env.GITHUB_PERSONAL_ACCESS_TOKEN })` gets `undefined` inside
the pool, so live calls fail with `401 Requires authentication`. Neither a shell-exported
PAT, adding the PAT to `secrets.required` in `wrangler.jsonc`, `CLOUDFLARE_INCLUDE_PROCESS_ENV=true`,
nor a temporary `.dev.vars` fixes it (`.dev.vars` maps to the Worker `env` binding, not
`process.env`). That is why a live-PAT verification must run as a **standalone script**
(step 4 above), where Node's real `process.env` carries the token — not as a vitest spec.

`@cloudflare/vitest-pool-workers` binds a `127.0.0.1` socket at pool startup, which
the Claude Code sandbox blocks (`Error: listen EPERM: operation not permitted
127.0.0.1`). Run the live spec with `dangerouslyDisableSandbox: true`. (This is the
network-socket axis, unrelated to `.dev.vars` — `secrets.required` does not fix it.)
(A `_verify_*.test.js` that uses only **stubbed** Octokit still works fine in the pool —
the limitation is specifically the *live-PAT* path. A sandboxed `pnpm exec vitest` also
hits a separate `listen EPERM` on the pool's `127.0.0.1` bind; run with
`dangerouslyDisableSandbox: true`. That socket axis is unrelated to `.dev.vars` /
`secrets.required`.)

## Where this sits vs the two-tier ADR

Expand All @@ -46,6 +76,10 @@ behaviour/output, not the OAuth/transport plumbing the other two tiers cover.
- Triggered when verifying a `src/tools/*.ts` module's real-API behaviour during
development (e.g. confirming a new tool returns the right Markdown against live
data) — before relying on the full manual OAuth harness, which is heavier.
- Use a throwaway `test/_verify_*.test.js`, live Octokit via PAT, sandbox-bypassed
run, then delete the spec.
- Use a throwaway **standalone** `scripts/_verify_*.mjs` (live Octokit via PAT from
Node's `process.env`), run with `pnpm dlx tsx`, then delete it. Do **not** use a
`test/_verify_*.test.js` spec for the *live-PAT* path — the Workers vitest pool
isolate has no `process.env` PAT, so it 401s (see the gotcha above).
- For a **write** tool, verify the live binding via the error path (completed-run
`409`, no-trigger-workflow `422`) rather than performing a real mutation.
- For OAuth/consent/transport coverage, use the two ADR-0002 tiers instead.