From 02c65e988ab831542570963f72ce6d5700d47e22 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 9 Oct 2026 05:13:06 +0200 Subject: [PATCH] fix(ci): #540 landing fixes Move the nightly cron from 03:17 to 03:23 UTC. GitHub sends scheduled-run failure notices to whoever last modified the cron line, and after the merge that is the contributor, so a maintainer commit has to touch it. The docs below give no clock time, so they cannot drift from the cron. Drop the "Keep the failure artifacts" step and the blank line before it. No browser test writes test-results/ or screenshots-echo-diag/ (only the ignore files name them), and if-no-files-found: ignore made the step upload nothing without a word. The run log already carries the failure output. Reword the workflow header. Drop the claim that the skipped suite let two semantically conflicting PRs merge green: that incident came from test/mobile/keyboard.test.ts, which this job does not run. Correct the codex-predictive-echo note: the test uses a fake key in a throwaway CODEX_HOME and skips itself when codex is missing, so it needs a codex binary, not an authenticated one. opencode-resize: record WebSocket resize frames under the socket's own URL instead of appending '#' + the session id. The URL already carries /ws/sessions//terminal, and the suffix let toContain(sessionId) pass for a resize sent on any session's socket, the bug this test exists to catch. Reduce the six session-id extractions (opencode-resize and perf-browser) to data.data?.session?.id. POST /api/sessions always answers in the { success, data: { session } } envelope, and the dead fallbacks are what hid the original breakage. split-pane-terminal: restore the browser config's 60 s test timeout (the added 20000 ms override tightened it), and replace the comment that blamed Codeman's post-create clear. Under vitest the session is an echo PTY, so that clear comes back as text; the real fix is useMux:false, since a plain prompt otherwise goes through tmux send-keys, which test mode no-ops. CLAUDE.md: the CI note now says the gate excludes the Playwright tests in BROWSER_TEST_GLOBS instead of a stale count of 14, and names browser-suite.yml; the Testing warning says the browser suite runs nightly. CONTRIBUTING.md gets the same one-line pointer under Tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/CONTRIBUTING.md | 2 ++ .github/workflows/browser-suite.yml | 23 ++++++----------------- CLAUDE.md | 4 ++-- test/opencode-resize.test.ts | 14 ++++++-------- test/perf-browser.test.ts | 2 +- test/split-pane-terminal.browser.test.ts | 8 ++++---- 6 files changed, 21 insertions(+), 32 deletions(-) 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