Code review · T6, Playwright end-to-end skeleton¶
review-code on autopilot/T6, base 8831408, 2026-10-28. The step adds tests/e2e/ (Playwright in TypeScript: a config whose base URL comes from an environment variable, a scenario JSON or a static stand-in page; desktop and phone projects; one smoke test; unit tests of the config) and the workflow e2e.yml. Reviewers: code-reviewer and security-reviewer, independent, in parallel. Format: README.md.
| Round | Head | code-reviewer | security-reviewer |
|---|---|---|---|
| 1 | b69fdd2 |
changes (0 blockers, 1 major, 6 minors, 1 of them simplify:) |
approve (0 blockers, 0 majors, 5 minors) |
| 2 | af7bcf0 (the round-1 fixes, delta b69fdd2..af7bcf0) |
approve (0 blockers, 0 majors, 0 minors) | approve (0 blockers, 0 majors, 1 minor: SEC-T6-05, deferred) |
| 3 | b95b740 (domcontentloaded in the smoke test, delta 7dde70b..b95b740, code only) |
approve (0 blockers, 0 majors, 0 minors) | not run (one-line test change) |
After the PR opened, the smoke test was changed to wait for domcontentloaded instead of load (remote preview assets stalled through the cloud session's proxy; see the Progress page Evidence); that one-line delta was reviewed as round 3 below. code-simplifier ran on a274636 and changed nothing (-0/+0, 11 tests pass). The e2e workflow was green on both heads: 40698975901 on b69fdd2 (10 tests) and 40699486247 on af7bcf0 (11 tests, Node v24.22.1). Test command:
cd tests/e2e && npm ci && npx --no-install playwright test
CODE step=T6 review=approve b/M/m=0/1/6 security=approve b/M/m=0/0/5 rounds=3 simplify=nochange -0/+0 tests=pass
| Id | Severity | Finding | Status |
|---|---|---|---|
| CR-T6-01 | major | e2e.yml did not run when the files its tests read changed: dev/versions.env (the pin check) and design/ui-canvas/ (the board names) |
fixed in af7bcf0: both in the push paths |
| CR-T6-02 | minor | CI ran on the runner's Node (v22.23.3), not the dev/versions.env pin |
fixed in af7bcf0: actions/setup-node, SHA-pinned as in docs.yml, with NODE_VERSION read from dev/versions.env |
| CR-T6-03 | minor | Checkout pinned at v4.4.0 (Node 20 runtime warning) | fixed in af7bcf0: v7.0.1, the pin of scenarios.yml |
| CR-T6-04 | minor | The key urls.spectio of the scenario JSON had no owner: dev/scenario.py never writes it |
applied in the spec: dev-env §5 names it, §6.1 reads it |
| CR-T6-05 | minor | The board screenshot was named RunsTable whatever the target page |
fixed in af7bcf0: taken only against the stand-in |
| CR-T6-06 | minor | Unit tests left temporary directories behind | fixed in af7bcf0: test.info().outputPath() |
| CR-T6-07 | minor | simplify: retries: 0 repeats Playwright's default |
fixed in af7bcf0: removed |
| SEC-T6-01 | minor | Traces and the report would keep credentials once the target has authentication; the error echoed the URL | fixed in af7bcf0: a base URL with user, password or query is refused and never echoed (unit test); traces only for the stand-in |
| SEC-T6-02 | minor | npx could fall back to the registry; the Chromium download was not in the §11 "Not pinned" list |
fixed: npx --no-install (af7bcf0); §11 "Not pinned" names the Chromium download and actions/setup-node |
| SEC-T6-03 | minor | CI did not use the Node pinned in §1 | fixed with CR-T6-02 |
| SEC-T6-04 | minor | The pin check did not run when the pin changed | fixed with CR-T6-01 |
| SEC-T6-05 | minor | The SAST check may not cover TypeScript | confirmed wider than T6 and deferred: a full semgrep scan lists no file under tests/ in paths.scanned (no root .semgrepignore, so semgrep's built-in ignore list applies), while tests/e2e/base-url.ts named directly is scanned with 0 findings under p/javascript and p/typescript. Recorded in §11 "Not scanned"; follow-up: a root .semgrepignore |