diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index fd80577a..fab29389 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -55,6 +55,8 @@ npm run test:all # literally everything, environmental failures included Expect `test:browser`/`test:mobile`/`test:perf` to fail where the machine cannot provide what they need; read that as "not runnable here", not as a regression. `config/test-suites.ts` holds the globs, and both configs derive from it, so the exclusions and those runners cannot drift apart. +The browser suite also runs nightly (and on demand) in `.github/workflows/browser-suite.yml`; it is informational, not a gate. + If you add a test that binds a port, bind port 0 (`new WebServer(0, …)` + `server.boundPort`, or `listen({ port: 0 })` + `address().port`), or use `app.inject()` when no socket is needed; `test/test-ports-guard.test.ts` fails a `WebServer` built on any other port. Mobile tests (`test/mobile/**`, via `createTestServer(PORT)`) keep the fixed-port convention in `test/mobile/README.md` for now, because that helper caches servers by port. Never 3000. Tests are tmux-safe by design: under vitest, the tmux layer becomes an in-memory mock, so tests cannot touch real sessions. diff --git a/.github/workflows/browser-suite.yml b/.github/workflows/browser-suite.yml index 7581e07c..c5da3964 100644 --- a/.github/workflows/browser-suite.yml +++ b/.github/workflows/browser-suite.yml @@ -1,18 +1,18 @@ name: Browser suite # The per-push CI gate deliberately skips the Playwright-driven suite (config/test-suites.ts), -# which has twice let two PRs that conflict semantically merge green. This job runs it on a -# schedule and on demand, so a browser-only regression (the Shift+Enter keypress bug was one) -# is caught within a day instead of by a user. It is NOT a merge gate: a red run means "look", -# and it never blocks a push or a PR. +# so a browser-only regression can merge green. This job runs that suite on a schedule and on +# demand, so such a regression (the Shift+Enter keypress bug was one) is caught within a day +# instead of by a user. It is NOT a merge gate: a red run means "look", and it never blocks a +# push or a PR. # # Needs: chromium (installed below), tmux, and the live server the tests start themselves. # Not run here: test:mobile (per-machine PNG baselines), test:perf (wall-clock), and -# codex-predictive-echo (needs a real, authenticated codex binary). +# codex-predictive-echo (needs a real codex binary; it also skips itself without one). on: schedule: - - cron: '17 3 * * *' + - cron: '23 3 * * *' workflow_dispatch: permissions: @@ -48,14 +48,3 @@ jobs: - name: Run the browser suite run: npm run test:browser -- --exclude test/codex-predictive-echo.test.ts - - - name: Keep the failure artifacts - if: failure() - uses: actions/upload-artifact@v4 - with: - name: browser-suite-results - path: | - test-results/ - screenshots-echo-diag/ - if-no-files-found: ignore - retention-days: 7 diff --git a/CLAUDE.md b/CLAUDE.md index a87feccf..37f0ec3e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,7 +123,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph | Dependency doctor | `codeman doctor` (alias `check-deps`; `--json`, `--category core\|office\|other`). Probes Node/Claude CLI/tmux/LibreOffice/MS Office against `config/dependency-registry.ts`; engine is pure given an injectable `ProbeHost` | | Multi-user accounts | `codeman users add ` / `passwd ` / `list` / `rm ` (writes `~/.codeman/users.json`, mode 0600; see Multi-user mode) | -**CI**: `.github/workflows/ci.yml` (push to master/main + PRs, Node 22) runs two jobs: **(1)** `check:lockfile`, `typecheck`, `lint`, `check:frontend-syntax`, `check:browser-excludes`, `format:check`, then a **server boot smoke test** (`tsx src/index.ts web --port 3151` must answer `/api/status` within 30s); **(2)** the **unit/integration test suite** via `npm run test:ci` (`config/vitest.ci.config.ts` — excludes the browser-driven `test/mobile/**` suite, `perf-*` benchmarks, and 14 Playwright tests; globs live in `config/test-suites.ts`), followed by the **`packages/xterm-zerolag-input` package tests** (a bare `npx vitest run` in that directory; its vitest is hoisted by the root `npm ci`, so no separate install, and `npm test` at the root does NOT run them). `npm test` runs this same config, so local green == CI green. Tests are tmux-safe in CI: `TmuxManager` no-ops all shell commands under `VITEST` (see Testing). A third workflow, `wiki-sync.yml`, fires only on master pushes touching `docs/wiki/**` and mirrors that directory to the GitHub wiki (browser edits to the wiki are overwritten by the next sync, so fix pages via `docs/wiki/`). +**CI**: `.github/workflows/ci.yml` (push to master/main + PRs, Node 22) runs two jobs: **(1)** `check:lockfile`, `typecheck`, `lint`, `check:frontend-syntax`, `check:browser-excludes`, `format:check`, then a **server boot smoke test** (`tsx src/index.ts web --port 3151` must answer `/api/status` within 30s); **(2)** the **unit/integration test suite** via `npm run test:ci` (`config/vitest.ci.config.ts` — excludes the browser-driven `test/mobile/**` suite, `perf-*` benchmarks, and the Playwright tests in `BROWSER_TEST_GLOBS`; globs live in `config/test-suites.ts`), followed by the **`packages/xterm-zerolag-input` package tests** (a bare `npx vitest run` in that directory; its vitest is hoisted by the root `npm ci`, so no separate install, and `npm test` at the root does NOT run them). `npm test` runs this same config, so local green == CI green. Tests are tmux-safe in CI: `TmuxManager` no-ops all shell commands under `VITEST` (see Testing). A third workflow, `wiki-sync.yml`, fires only on master pushes touching `docs/wiki/**` and mirrors that directory to the GitHub wiki (browser edits to the wiki are overwritten by the next sync, so fix pages via `docs/wiki/`). `browser-suite.yml` runs `npm run test:browser` (minus codex-predictive-echo) nightly and on demand via `workflow_dispatch`; it is informational and never gates a push or PR. **Code style**: Prettier (`singleQuote: true`, `printWidth: 120`, `trailingComma: "es5"`) — config lives in the **`"prettier"` key of `package.json`**, not a `.prettierrc` (keeps the repo root short; editors read it natively). `.prettierignore` stays at the root because Prettier resolves it relative to cwd. ESLint flat config (`config/eslint.config.js`) allows `no-console`, warns on `@typescript-eslint/no-explicit-any`. Ignores: `app.js`, `scripts/**/*.mjs`, `src/web/public/vendor/**`, `scripts/remotion/**`. @@ -465,7 +465,7 @@ npm run test:perf # wall-clock benchmarks — need an otherwise idle machin npm run test:all # literally everything; fails ~87 tests on a clean master here, which is why it is not the default ``` -⚠️ **`npm test` cannot see those suites**, so a change touching mobile/gesture/terminal-render behaviour needs the matching runner by hand — diff its FAIL list against master rather than reading it as pass/fail. That blind spot is what let two semantically-conflicting PRs merge green (see the on-screen-keyboard note above). +⚠️ **`npm test` cannot see those suites**, so a change touching mobile/gesture/terminal-render behaviour needs the matching runner by hand — diff its FAIL list against master rather than reading it as pass/fail. That blind spot is what let two semantically-conflicting PRs merge green (see the on-screen-keyboard note above). The browser suite (not mobile or perf) also runs nightly in `.github/workflows/browser-suite.yml`, which catches a regression after it lands, not before. ⚠️ **A file filter must match the runner.** `npm test -- test/mobile/keyboard.test.ts` matches nothing and exits GREEN having run zero tests, because the gate's config excludes that path — an excluded file needs its own runner (`npm run test:mobile -- `, `npm run test:browser -- `, `npm run test:perf -- `). Vitest treats "no files matched a filter" as success, so read the file count, not just the colour. diff --git a/test/opencode-resize.test.ts b/test/opencode-resize.test.ts index e5a40161..5a2cb696 100644 --- a/test/opencode-resize.test.ts +++ b/test/opencode-resize.test.ts @@ -119,13 +119,12 @@ describe('OpenCode session initial resize', () => { ws.on('framesent', (frame) => { try { const msg = JSON.parse(String(frame.payload)); - if (msg.t === 'z') resizeCalls.push({ url: ws.url() + '#' + sessionIdForWs, cols: msg.c, rows: msg.r }); + if (msg.t === 'z') resizeCalls.push({ url: ws.url(), cols: msg.c, rows: msg.r }); } catch { /* not JSON */ } }); }); - let sessionIdForWs = ''; await page.route('**/api/sessions/*/resize', async (route) => { const request = route.request(); const body = request.postDataJSON(); @@ -147,11 +146,10 @@ describe('OpenCode session initial resize', () => { }); const data = await res.json(); // POST /api/sessions answers in the { success, data: { session } } envelope. - return data.data?.session?.id ?? data.id ?? data.session?.id; + return data.data?.session?.id; }); expect(sessionId).toBeTruthy(); - sessionIdForWs = sessionId; // Call selectSession (which is what runOpenCode does after fix) await page.evaluate(async (sid: string) => { @@ -194,7 +192,7 @@ describe('OpenCode session initial resize', () => { }); const data = await res.json(); // POST /api/sessions answers in the { success, data: { session } } envelope. - return data.data?.session?.id ?? data.id ?? data.session?.id; + return data.data?.session?.id; }); expect(sessionId).toBeTruthy(); @@ -246,7 +244,7 @@ describe('OpenCode session initial resize', () => { }); const data = await res.json(); // POST /api/sessions answers in the { success, data: { session } } envelope. - return data.data?.session?.id ?? data.id ?? data.session?.id; + return data.data?.session?.id; }); expect(sessionId).toBeTruthy(); @@ -331,7 +329,7 @@ describe('OpenCode close modal text', () => { }); const data = await res.json(); // POST /api/sessions answers in the { success, data: { session } } envelope. - return data.data?.session?.id ?? data.id ?? data.session?.id; + return data.data?.session?.id; }); expect(sessionId).toBeTruthy(); @@ -374,7 +372,7 @@ describe('OpenCode close modal text', () => { }); const data = await res.json(); // POST /api/sessions answers in the { success, data: { session } } envelope. - return data.data?.session?.id ?? data.id ?? data.session?.id; + return data.data?.session?.id; }); expect(sessionId).toBeTruthy(); diff --git a/test/perf-browser.test.ts b/test/perf-browser.test.ts index 97c65229..0c5a2c92 100644 --- a/test/perf-browser.test.ts +++ b/test/perf-browser.test.ts @@ -70,7 +70,7 @@ async function createSession(page: Page, name: string): Promise { }); const data = await res.json(); // POST /api/sessions answers in the { success, data: { session } } envelope. - return data.data?.session?.id ?? data.id ?? data.session?.id; + return data.data?.session?.id; }, name); return result as string; } diff --git a/test/split-pane-terminal.browser.test.ts b/test/split-pane-terminal.browser.test.ts index 3f032c62..5463cef4 100644 --- a/test/split-pane-terminal.browser.test.ts +++ b/test/split-pane-terminal.browser.test.ts @@ -113,9 +113,9 @@ describe('TerminalTile in a real browser', () => { // own startup can race an early write and, on this box, a startup // script issues a `clear` that erases scrollback (modern ncurses // `clear` emits \x1b[3J) if the input lands before the shell is ready. - // Codeman itself writes `clear` into a NEW shell session ~100ms after - // creating it, which can erase an early marker, so re-send until the - // marker is present in the capture rather than writing once. + // Send with useMux:false: a plain prompt otherwise goes out through + // tmux send-keys, which test mode no-ops, so the marker never reached + // the PTY. Re-sending until the capture shows it is just belt and braces. const deadline = Date.now() + 8000; for (;;) { await fetch(`/api/sessions/${id}/input`, { @@ -169,7 +169,7 @@ describe('TerminalTile in a real browser', () => { await page.evaluate(async (id) => { await fetch(`/api/sessions/${id}`, { method: 'DELETE' }); }, sessionId); - }, 20000); + }); it('gates app-level chords out of Pane B instead of forwarding their raw bytes', async () => { // Regression guard for PR #453's Ctrl+K/Alt+1/Alt+B leak: Pane B had no