From 0635d09e8311735f074ca3ef0ac44b4b16d5c1ef Mon Sep 17 00:00:00 2001 From: Bob Dickinson Date: Thu, 2 Jul 2026 17:07:25 -0700 Subject: [PATCH] Fixed CI failure (test fixture) and added local CI script --- AGENTS.md | 9 ++++---- .../helpers/oauth-client-fixtures.ts | 22 ++++++++++++++----- package.json | 3 +++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fed0eca1..1b84fa8f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -189,14 +189,15 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i ### Lint-fixed, Formatted code - ALWAYS do `npm run format` before committing — it auto-fixes any Prettier issues. `validate` runs `format:check` (the non-fixing variant) and will fail in CI on any unformatted file, so always run the auto-fixer first rather than letting `format:check` catch it. -- ALWAYS do `npm run validate` before pushing any changes — from the repo root it chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each delegates to that client's own `npm run validate` = `format:check` + `lint` + `build` + `test` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies). +- **`npm run ci` before pushing** — runs the same steps as `.github/workflows/main.yml` (minus `npm install`): `validate` → web `test:integration` → `smoke` → Storybook play-function tests (installs Playwright chromium if needed). Use this when you want CI parity; expect several minutes. **`npm run validate`** remains the fast loop during development (unit tests only — no web integration, smoke, or Storybook). +- ALWAYS do `npm run format` before committing, then **`npm run ci`** (or at minimum `npm run validate`) before pushing. From the repo root, `validate` chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each delegates to that client's own `npm run validate` = `format:check` + `lint` + `build` + `test` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies). - The one CLI nuance: `clients/cli`'s out-of-process `e2e.test.ts` spawns the built binary, so its `test` **builds first** via `pretest` (`test-servers:build && build`). To avoid building it twice, `clients/cli`'s `validate` folds that in — it is `format:check && lint && test` with **no** separate `build` step (the other clients, whose tests don't spawn their bundle, keep an explicit `build`). `validate:web`/`validate:tui`/`validate:launcher` are the uniform `format:check && lint && build && test`. - - Before pushing, also run **`npm run coverage`** — `validate` is fast and does NOT enforce the per-file gate (or, for web, run the integration project); `coverage` does both. CI does **not** run `coverage` (the gate is local-only); it runs `validate` plus a standalone web `test:integration` step. -- **`smoke` is a separate top-level target, NOT part of `validate`.** Run it (or the individual `smoke:*`) after a build/validate: `npm run validate && npm run smoke`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the web `test:integration` step, then `smoke`. Storybook is the only CI step left out (see below). + - Optionally also run **`npm run coverage`** when you want the local-only per-file ≥90 gate — CI does **not** run `coverage`; it relies on `validate` + web `test:integration` instead. +- **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). - `smoke:launcher` (`scripts/smoke-launcher.mjs`) runs the built launcher with `--help`, `--cli --help`, and `--tui --help`, asserting each exits 0 and prints that mode's usage banner (which also proves the launcher resolved and loaded the right client build). It's the cheap dispatch check before the heavier prod smokes below. - `smoke:web` (`scripts/smoke-web.mjs`) starts `mcp-inspector --web` (prod, no `--dev`) against the built `clients/web/dist` and asserts `GET /` serves the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. Prod `--web` serves from `clients/web/dist`, which ships in the published package but is absent in a fresh checkout — the runner builds it on demand (`build:client` = `vite build`) on first launch, or exits with an actionable error if that build can't run (see `clients/web/server/ensure-web-build.ts` and the launcher README). `--dev` runs Vite directly and never needs `dist`. - `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog ` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. **`smoke:tui` is local-only: it self-skips when `process.env.CI` is set**, because the Ink TUI needs a real TTY (raw mode) that headless CI lacks — so run it (via `npm run smoke`) on your own machine before pushing. Both build `test-servers/build` on demand if it's missing. -- Also run `npm run test:storybook` from `clients/web/` before pushing — it executes every story's `play` function in headless Chromium via `@vitest/browser-playwright` (~10s). CI runs this as a separate step (from `clients/web`) after `validate`; failures block merge. It is kept out of `validate` because it needs the Playwright browser binary and is much slower than the unit suite. (There is no root-level `test:storybook` aggregate — run it in the web client.) +- Storybook play-function tests (`clients/web` `test:storybook`) run in headless Chromium via `@vitest/browser-playwright` (~10s). They are part of `npm run ci` (which installs Playwright chromium first); kept out of `validate` because they need the browser binary and are slower than the unit suite. ### Typescript instructions - Use TypeScript for all new code diff --git a/clients/web/src/test/integration/helpers/oauth-client-fixtures.ts b/clients/web/src/test/integration/helpers/oauth-client-fixtures.ts index eb1b23ac..f46e4780 100644 --- a/clients/web/src/test/integration/helpers/oauth-client-fixtures.ts +++ b/clients/web/src/test/integration/helpers/oauth-client-fixtures.ts @@ -137,9 +137,8 @@ export async function createClientMetadataServer( } /** - * Helper function to programmatically complete OAuth authorization - * Makes HTTP GET request to authorization URL and extracts authorization code - * The test server's authorization endpoint auto-approves and redirects with code + * Programmatically complete OAuth against the local composable test AS. + * GET shows an HTML consent page; approve via POST (same as CLI test helper). * * @param authorizationUrl - The authorization URL from oauthAuthorizationRequired event * @returns Authorization code extracted from redirect URL @@ -147,10 +146,23 @@ export async function createClientMetadataServer( export async function completeOAuthAuthorization( authorizationUrl: URL, ): Promise { - const response = await fetch(authorizationUrl.toString(), { + let response = await fetch(authorizationUrl.toString(), { redirect: "manual", }); + if (response.status === 200) { + const body = new URLSearchParams(authorizationUrl.searchParams); + response = await fetch( + `${authorizationUrl.origin}${authorizationUrl.pathname}`, + { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body, + redirect: "manual", + }, + ); + } + if (response.status !== 302 && response.status !== 301) { throw new Error( `Expected redirect (302/301), got ${response.status}: ${await response.text()}`, @@ -162,7 +174,7 @@ export async function completeOAuthAuthorization( throw new Error("No Location header in redirect response"); } - const redirectUrlObj = new URL(redirectUrl); + const redirectUrlObj = new URL(redirectUrl, authorizationUrl.origin); const code = redirectUrlObj.searchParams.get("code"); if (!code) { throw new Error(`No authorization code in redirect URL: ${redirectUrl}`); diff --git a/package.json b/package.json index f49ae384..dd56fe20 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,9 @@ "build:tui": "cd clients/tui && npm run build", "build:web": "cd clients/web && npm run build", "build:launcher": "cd clients/launcher && npm run build", + "ci": "npm run validate && npm run ci:integration && npm run smoke && npm run ci:storybook", + "ci:integration": "cd clients/web && npm run test:integration", + "ci:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook", "validate": "npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher", "validate:cli": "cd clients/cli && npm run validate", "validate:tui": "cd clients/tui && npm run validate",