Code review · P3 Task 20, runs list, run detail, Cancel and the Playwright smoke¶

review-code on autopilot/P3-t20, base b0e2edf, 2026-10-31. The step is P1 Task 20, built in the local-only order of inbox #80 (a), before Tasks 7 and 12 to 16, with the rules of inbox #90. It adds:

packages/spectio-server/src/spectio_server/api/models.py: response models for the read routes and :cancel; runs_read.py uses them and a one-ID task cursor; app.py pins info.version to 0.0.1; static.py answers HEAD and says --ignore-scripts;
web/src/: live.tsx (one stream per tab), home/HomePage.tsx, runs/RunsPage.tsx, runs/RunDetailPage.tsx (with Cancel behind a modal confirm), runs/stateWord.ts, runs/RunState.tsx, shell/crumb.ts, and the typed api.ts, with Vitest tests;
tests/e2e/skeleton.spec.ts and tests/e2e/serve-local.sh, and the CI job e2e-local in .github/workflows/ci.yml, outside ci-ok;
the live shots app-home, app-runs, app-run-detail and app-run-detail-phone;
tests: test_openapi.py, test_read.py, test_web.py (server), packages/spectio/tests/test_serve_local.py (new);
inbox #90.

The reviewers were code-reviewer and security-reviewer, independent and in parallel. Format: README.md. The rules are owned by the ui-spec notes of 2026-10-31 in §4.2, §4.3 and §4.4, the api §5 note of Task 18, dev-env §6.1, §11 and §15.3 req. 17, all written by the docs commit.

Round	Head	code-reviewer	security-reviewer
1	`0cceb8f` (the step, with the smoke and the shots)	approve (5 minors, 1 simplify)	approve (5 minors)
2	`e6dc116` (focus after Cancel, sequenced lists, Load more rows kept, `stop()` stops the heads)	approve (3 minors, 1 simplify)	changes (1 major)
3	`2f30646` (`heads()` parses `head.pid` strictly)	approve (4 minors)	approve (1 minor)
simplify	`4a70188`	`code-simplifier`: changed, −2 +1

The reports were relayed by the coordinator in summary. Finding IDs restart in each round, so the same ID can name different findings; this record writes the round after each ID: CR-P3T20-05 (r1), (r2) and (r3) are three findings.

Test-first. The worker reports that the round-2 fix of SEC-P3T20-02 was written as a test first and seen to fail: test_serve_local.py writes a real head's line, a forged 1 1 <start time of PID 1> line, a line from another boot, a loose line and a symlink, and expects only the real PID. doc-writer re-checked it: run against the heads() of e6dc116, both tests fail and that heads() names ['1'] for the forged line (P3-M4 Evidence). The round-1 fixes of e6dc116 each came with a test (Cancel asks first … returns focus, an older response that lands last …, a stream frame refreshes the first page and keeps the rows Load more added, an older refresh that lands last …).

Gate at 4a70188, from the worktree, re-run by doc-writer:

uv run --locked pytest -q             # 511 passed, 15 deselected
uv run --locked ruff check .          # All checks passed!
uv run --locked ruff format --check . # 82 files already formatted
uv run --locked mypy packages         # Success: no issues found in 81 source files
uv run --locked lint-imports          # Contracts: 4 kept, 0 broken.
uv lock --check                       # Resolved 55 packages
cd web && npx vitest run              # 10 files, 63 tests passed
cd web && npx tsc --noEmit            # no output, exit 0
cd web && npm run build               # dist/index.html and two hashed assets
# smoke, against tests/e2e/serve-local.sh with Nextflow 26.10.4:
SPECTIO_E2E_ENGINES=1 npx --no-install playwright test skeleton.spec.ts --project desktop --project phone
                                      # 16 passed, 6 skipped (the three test.fixme on both projects)

CODE  step=P3-t20  review=approve b/M/m=0/0/12  security=approve b/M/m=0/1/6  rounds=3  simplify=changed -2/+1  tests=pass

The counts add up the three rounds per reviewer. The two simplify findings (CR-P3T20-02 r1, CR-P3T20-08 r2) are not counted.

Findings¶

Id	Severity	Finding	Status
CR-P3T20-01 (r1)	minor	After a confirmed Cancel, focus was lost: the Modal returned it to Cancel run, which the `cancelling` state removes, so focus fell to `<body>`.	fixed in `e6dc116`: the run's `h1` (`tabIndex=-1`) takes focus; the Cancel test asserts it
CR-P3T20-02 (r1)	simplify	`cancelButton` (`useRef`) was passed as `ref` and never read; Mantine's `returnFocus` does the work.	fixed in `4a70188` by `code-simplifier`
CR-P3T20-03 (r1)	minor	The runs list fetches were neither cancelled nor sequenced: an older response could land last and show stale rows under the pressed filter.	fixed in `e6dc116`: one `AbortController` per query and a load counter; Home drops an older refresh the same way; a test each
CR-P3T20-04 (r1)	minor	Past 200 rows added by Load more, a frame refetched `limit = min(200, shown)` and replaced the list: rows vanished and `next` reset.	fixed in `e6dc116`: a frame refetches the first page and merges it, keeping the older rows and their cursor; test
CR-P3T20-05 (r1)	minor	An unrelated edit in `test_stream.py` (a top-level `MAX_CLIENTS` import, an unused fixture dropped).	kept: it is the FU-P3-21 item CR-P3T18-05 (r2), which Task 20 owns
CR-P3T20-06 (r1)	minor	`routes.tsx` and `NotFound`'s comment disagree on how `/runs/%E0` is routed.	open: follow-up FU-P3-24
SEC-P3T20-01 (r1)	minor	Ctrl-C stopped `spectio serve` but not the heads, which run in their own sessions; the EXIT trap then removed the runs root under them.	fixed in `e6dc116`: `stop()` sends `SIGTERM` to each live head's group, waits 10 s, and keeps the directory if one still runs
SEC-P3T20-02 (r1)	minor	A response that drifts from its model raises `ResponseValidationError`; its text lists each input, uvicorn logs it, and CI uploads that log as an artifact for 14 days. Low today: the store's CHECK constraints and `test_model_enums_match_the_store` hold the enums.	open: follow-up FU-P3-20
SEC-P3T20-03 (r1)	minor	`e2e-local` installs Nextflow with `nf-core/setup-nextflow`, pinned by version only, not checked against `NEXTFLOW_SHA256` (dev-env §1).	open: follow-up FU-P3-24, before `e2e-local` joins `ci-ok`; recorded in the dev-env §11 "Not pinned" bullet
SEC-P3T20-04 (r1)	minor	The banner understates the exposure: any local user can open the address and launch as the person running the script. On a shared host the script ends with "no answer after 20 s" without the reason.	open: follow-up FU-P3-24
SEC-P3T20-05 (r1)	minor	Host residue of the shot recipe: the account `alice` (uid 30033, password locked) and `/g/demo-lab`, plus a stray `tests/e2e/node_modules` in the main checkout. The recipe has no teardown.	open: for the host's owner; a teardown step in the shot script, FU-P3-22
CR-P3T20-05 (r2)	minor	With "latest request wins", a response is dropped whenever a newer request has started; a server slower than 1 s per response, with frames arriving, never updates the list or Home.	open: follow-up FU-P3-24
CR-P3T20-06 (r2)	minor	The new Home test swallows a failed wait with `.catch(() => undefined)`.	open: follow-up FU-P3-24 (repeated as CR-P3T20-06 r3)
CR-P3T20-07 (r2)	minor	`the header names the run…` checks the crumb and `document.title` right after `findByRole`; both may come in a later render. It failed once under load.	open: follow-up FU-P3-24
CR-P3T20-08 (r2)	simplify	`useRef(new AbortController())` builds a throwaway controller on every render.	left by `code-simplifier`: the fix needs a non-null assertion or guards in three places; follow-up FU-P3-24
SEC-P3T20-02 (r2)	major	`heads()` trusted `head.pid` loosely before `stop()` signalled the group it names: no strict parse, PID 1 not refused, the group checked in the file only, no boot ID. A `1 1 <start time of PID 1>` line made `stop()` run `kill -TERM -- -1`. §15.3 req. 10 to 12.	fixed in `2f30646`: no symlink, one line of the `_PID_LINE` grammar, `pid > 1`, `pid == pgid`, this boot, and in `/proc` not a zombie, group = pid, same start time; `test_serve_local.py`. The file is not opened with `O_NOFOLLOW` (a separate check), and the host is checked for format only
CR-P3T20-05 (r3)	minor	After a frame, rows that Load more added are kept as they were: an active run past the first 50 keeps its old state word until reload.	open: follow-up FU-P3-24
CR-P3T20-06 (r3)	minor	The Home race test's first `findByRole` always times out after 1 s and its error is thrown away: a hidden sleep that asserts nothing.	open: follow-up FU-P3-24
CR-P3T20-07 (r3)	minor	The comment above `heads()` still claims the runner's strictness; `heads()` reads one line, has no `read_small` checks and does not compare the host. Behaviour is safe for `stop()`.	open: follow-up FU-P3-24
CR-P3T20-08 (r3)	minor	`heads()` is a second copy, in shell, of the runner's grammar; the test writes lines by hand, so nothing ties it to `headscript.py`.	open: follow-up FU-P3-24
SEC-P3T20-03 (r3)	minor	`heads()` does not tie a PID to its run: a forged `head.pid` with a real start time can name any group leader of the user on this boot (a login shell, tmux). No privilege gain. A PID can also be reused between `heads()` and the kill.	open: follow-up FU-P3-24

Fixes by round¶

Commit	Findings
`e6dc116` fix(web,e2e): focus after Cancel, sequenced runs lists, frame refresh keeps Load more rows, serve-local stops heads	CR-P3T20-01 (r1), CR-P3T20-03 (r1), CR-P3T20-04 (r1), SEC-P3T20-01 (r1)
`2f30646` fix(e2e): serve-local heads() parses head.pid strictly before stop() signals a group	SEC-P3T20-02 (r2)
`4a70188` refactor(P3-t20): simplify run detail by dropping the unread cancelButton ref	CR-P3T20-02 (r1)

code-simplifier reran the step's check command before and after its change (pytest 511 passed, ruff, mypy, lint-imports, uv lock --check) and the web checks (tsc --noEmit, vitest 63 of 63): the same results once timings are stripped.

For the Progress page¶

The Task 20 section of P3-M4 is written: P3-M4 · Task 20, with its findings and open questions.