From 7f021f9aaa26dd6ea1943dd3a9785f5621a6f70e Mon Sep 17 00:00:00 2001
From: Masaru Nemoto <16267290+nemolize@users.noreply.github.com>
Date: Thu, 11 Jun 2026 11:17:46 +0900
Subject: [PATCH] docs(rules): fix live tool-verify procedure for Workers
 vitest env gap

The Workers vitest pool isolate does not expose Node's process.env, so a
test/_verify_*.test.js spec injecting a live PAT 401s. Switch the procedure
to a standalone scripts/_verify_*.mjs run via `pnpm dlx tsx`, document the
gotcha, and add an error-path approach for write tools (completed-run 409,
no-trigger-workflow 422) to avoid real CI mutations. Discovered while
verifying the #8 Phase-5 write tools.
---
 .claude/rules/tool-e2e-handler-capture.md | 78 ++++++++++++++++-------
 1 file changed, 56 insertions(+), 22 deletions(-)

diff --git a/.claude/rules/tool-e2e-handler-capture.md b/.claude/rules/tool-e2e-handler-capture.md
index bf6704f..f884173 100644
--- a/.claude/rules/tool-e2e-handler-capture.md
+++ b/.claude/rules/tool-e2e-handler-capture.md
@@ -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
 
@@ -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.