diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 2ed1d8f1..fd80577a 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -55,7 +55,7 @@ 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. -If you add a test that binds a port, pick a unique one at 3150 or above (search the repo for `const PORT =` first). Never 3000. +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/AGENTS.md b/AGENTS.md index 49991d86..e0cf802b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,6 @@ Quick pointers: - Type check: `tsc --noEmit` · Lint: `npm run lint` · Format: `npm run format:check` - Tests: `npm test` (the CI gate, safe to run bare) or `npm test -- test/.test.ts` for one file -- Route tests use `app.inject()`; new tests needing ports must pick a unique `const PORT =` +- Route tests use `app.inject()`; new tests needing a socket bind port 0 (`new WebServer(0, …)` + `boundPort`), except mobile tests, which keep `createTestServer(PORT)` for now - Branch off `master` for all work; Conventional Commit-style messages (`fix(mobile): ...`) - Never commit secrets or local state from `~/.codeman/` diff --git a/CLAUDE.md b/CLAUDE.md index c0e3dcdb..0c3114c5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -436,7 +436,7 @@ One module per domain in `src/web/routes/` (plus a barrel; `ls src/web/routes/` - **App setting**: decide per-device vs synced first. Per-device keys go in the `displayKeys` set in settings-ui.js and must NOT be added to `SettingsUpdateSchema` (it is `.strict()`). ⚠️ Anything in `PUT /api/settings` that acts on a setting (the `toggleService` watcher calls) must resolve from **`merged`** (persisted + incoming), never from the raw request body: a partial PUT omits keys it doesn't intend to change, and `body.x ?? default` turns every omission into "apply the default" and silently resets live services. Pinned by `test/routes/system-routes-settings-partial-put.test.ts`. - **Hook event**: Add to `HookEventType`, add hook in `hooks-config.ts:generateHooksConfig()`, update `HookEventSchema` - **Mobile feature**: Add to relevant singleton, guard with `MobileDetection.isMobile()`. New header buttons must stay off phones (`test/mobile-header-buttons-policy.test.ts`). -- **New test**: Pick unique port (search `const PORT =`). Route tests use `app.inject()` (no port needed) — see `test/routes/_route-test-utils.ts`. +- **New test**: a test that constructs `WebServer` itself binds port 0 (`new WebServer(0, …)`, then `server.boundPort`; enforced by `test/test-ports-guard.test.ts`), and a raw Fastify/`ws` server listens on `port: 0` and reads `address().port`. Mobile tests (`test/mobile/**`, via `createTestServer(PORT)`) keep the fixed-port convention in `test/mobile/README.md` until the follow-up sweep, because that helper caches servers by port. Route tests use `app.inject()` (no port needed); see `test/routes/_route-test-utils.ts`. **Validation**: Zod v4 (different API from v3). Define schemas in `schemas.ts`, use `.parse()`/`.safeParse()`. @@ -475,7 +475,7 @@ Raw `npx vitest` skips the config (and with it `setup.ts`); always use `npm test **Tmux safety**: under vitest (`VITEST`), `TmuxManager` no-ops ALL shell commands (`IS_TEST_MODE` in `src/tmux-manager.ts`), docker IO is no-op'd likewise, and `Session` spawns an echo PTY (`TEST_PTY_SCRIPT`) instead of attaching tmux. `test/setup.ts` gives each file a temp `HOME`/`USERPROFILE` and strips `CODEMAN_PASSWORD`/`CODEMAN_USERNAME`, `CODEMAN_GESTURE` and `CODEMAN_INSTANCE`/`CODEMAN_DATA_DIR`/`CODEMAN_TMUX_SOCKET` (pinned by `test/test-env-isolation.test.ts`). ⚠️ `CODEMAN_DATA_DIR` overrides the temp HOME, so never drop its strip; strip `CODEMAN_INSTANCE` in the setup file, never in a hook (captured at first import). ⚠️ Delete case trees only via `safeRmHomeTree()`. ⚠️ Raw `npx vitest` without `--config` skips `setup.ts` and its isolation. → [architecture-invariants#test-isolation-tmux-docker-and-home](docs/architecture-invariants.md#test-isolation-tmux-docker-and-home) -**Ports**: Pick unique ports manually, 3150+. Search `const PORT =` before adding new tests. Never 3000 (the live instance). +**Ports**: a test that constructs `WebServer` binds port 0 and reads `server.boundPort`; `test/test-ports-guard.test.ts` fails any other port outside its shrink-only legacy list. A raw Fastify/`ws` server listens on `port: 0` and reads `address().port`. The mobile suite keeps fixed ports (3150+, per `test/mobile/README.md`) until the follow-up sweep, since `createTestServer()` caches servers by port. Never 3000 (the live instance). ⚠️ **Browser tests can pass vacuously on mobile input paths.** Two traps, both hit on 2026-07-27 while fixing the phone Enter button: **(1)** driving input with `app.sendInput('…')` writes PAST the `LocalEchoOverlay`, so `pendingText` stays empty and any overlay bug is invisible — type with `page.keyboard.type()` instead; **(2)** headless Chromium reports `MobileDetection.isTouchDevice()` **false even with `hasTouch: true`**, so `_localEchoEnabled` is off and the local-echo branch never executes. Force it (`app._localEchoEnabled = true`) or the test proves nothing. Assert on real state (`app._localEchoOverlay.pendingText`, plus `tmux -L codeman capture-pane -p -t ` for what actually reached the PTY), not on HTTP 200. diff --git a/docs/wiki/Contributing.md b/docs/wiki/Contributing.md index ae03f0ee..6872ca90 100644 --- a/docs/wiki/Contributing.md +++ b/docs/wiki/Contributing.md @@ -61,8 +61,11 @@ benchmarks for an otherwise idle machine). Expect those to fail where the machin provide what they need; that means "not runnable here", not a regression. Tests are tmux-safe by design: under vitest the tmux layer becomes an in-memory mock, so -tests cannot touch real sessions. If you add a test that binds a port, pick a unique one at -3150 or above, and never 3000. +tests cannot touch real sessions. 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. Never 3000. Mobile tests (`test/mobile/**`, +via `createTestServer(PORT)`) keep the fixed ports in `test/mobile/README.md` for now, +because that helper caches servers by port. ## Finding your way around diff --git a/test/sse-tile-grid-filter.test.ts b/test/sse-tile-grid-filter.test.ts index b9ea7dac..aead1529 100644 --- a/test/sse-tile-grid-filter.test.ts +++ b/test/sse-tile-grid-filter.test.ts @@ -1,6 +1,6 @@ /** * @fileoverview The server's side of the tile grid's SSE filter (live server, - * multi-user mode, port 3287). + * multi-user mode, ephemeral port). * * While tiles own the terminal the page subscribes with TILE_GRID_SSE_FILTER * (constants.js), an id that names no session, so the server sends it no @@ -31,8 +31,7 @@ import { createUser, invalidateUsersCache } from '../src/user-store.js'; vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true); -const PORT = 3287; -const url = (p: string) => `http://localhost:${PORT}${p}`; +const url = (p: string) => `http://localhost:${server.boundPort}${p}`; const basic = (u: string, p: string) => 'Basic ' + Buffer.from(`${u}:${p}`).toString('base64'); const alice = { Authorization: basic('alice', 'alicepass1') }; @@ -124,7 +123,7 @@ beforeAll(async () => { await createUser({ username: 'root', role: 'admin', password: 'rootpass123' }); await createUser({ username: 'alice', role: 'user', password: 'alicepass1' }); await createUser({ username: 'bob', role: 'user', password: 'bobpass1234' }); - server = new WebServer(PORT, false, true); + server = new WebServer(0, false, true); await server.start(); });