Playwright writes a -diff.png when a snapshot differs, but a missing baseline
takes handleMissing, which writes only -actual.png. The detection step looked
for -diff.png alone, so a PR that adds a manifest route without regenerating
baselines uploaded no artifact and the sticky comment carried no advisory at
all. Match -actual.png too — both names are written exclusively by the snapshot
matcher, so a non-snapshot failure (which leaves test-failed-N.png) still
reports nothing — and say in the comment that the missing case uploads actual
only.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`head.repo.fork == false` is not null-safe: a PR whose head fork was deleted
arrives with `head.repo: null`, and GitHub coerces null and false alike to 0, so
the guard passed and a labelled fork PR would enter the protected environment,
hold a reviewer, then fail on the secrets GitHub withholds from fork runs — the
exact outcome the guard's own comment says it prevents. Comparing head.repo
full_name to github.repository yields false for a live fork and for a deleted
one, and it also survives this repo ever becoming a fork itself.
Evaluated both expressions with @actions/expressions against the real workflow
file: the only row that moves is the deleted fork, true -> false.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On pull_request, `github.event.before` is the PR's previous head, so the second
push to a PR diffs a range that no longer contains the commit that edited the
schema: the detector emits changed=false, the freshness step is skipped by its
`if:`, and lint-and-test goes green on a stale src/graphql/types.ts. Every
later push that does not itself touch one of the five inputs repeats it, forks
included — which is the case the trigger was added for. Nothing else covers the
gap: tsc reads the committed file as truth, eslint ignores it, and prettier
passes on codegen's own output.
Diff from pull_request.base.sha on PR events and keep `before` for push. The
range selection moves out of the YAML into .github/scripts so it can be driven
over real git fixtures: on the three-commit fixture the old range yields
changed=false and the new one changed=true.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An audit of the comments added since the feature base flagged a batch of
justification openers ("Not a copy-paste", "Live entry not dead weight"),
"there's a test for it" cross-refs, and how-we-found-it war-stories. Cut the
narration and keep the load-bearing fact in each. One was actively wrong:
playwright.config.ts still described the `maxDiffPixelRatio` anti-aliasing
budget that 5cec81b replaced with an absolute `maxDiffPixels` — removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
280d7b9 filtered the fork-fallback PR lookup by head identity but left the
primary same-repo path taking `run.pull_requests[0]` — one commit can belong to
several same-repo PRs, so the first listed could collect another branch's
report. Apply one identity filter to both sources. workflow_run.pull_requests
carries head.repo.name but no full_name, so the repo is enforced only when
present, keeping the primary path matching on head sha + ref.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ci.yml triggered only on push, but a fork PR's commits never push to this repo,
so its lint-and-test job (eslint, vitest, and the GraphQL codegen-freshness
check) never ran for forks. e2e.yml, the only workflow a fork PR did trigger,
builds with tsc — which reads the committed types.ts and cannot see it go stale.
So a fork editing schema.graphqls without regenerating, or shipping a lint/test
failure, could merge green.
Add a pull_request trigger (lint-and-test uses no secrets and is fork-safe) and
narrow push to main + tags, which is all docker-build — gated to those refs
already — needs. A concurrency group drops superseded PR-sync runs. The
codegen-detect step already treats an empty `github.event.before` as "check
anyway", so pull_request events always run the freshness check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sticky comment computed `mockPassed` by falling back to results.json alone
whenever the jobs API was unreachable. A run killed by the global timeout writes
its results.json before the abort is observed — the un-run tests land in
`skipped`, so `unexpected` stays 0 — so that fallback posted a ✅ on a run that
never finished.
Model three states instead of a boolean: pass (zero failures AND the job
confirmed success), fail, and unknown (jobs API unreachable, so the job cannot
be confirmed). Unknown renders ⚠️ with a note to check the run, never ✅.
Verified by driving the extracted script over a timeout-shaped results.json with
a rejecting jobs API: it now yields ⚠️ unknown where it used to yield ✅.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The fork fallback took `prs[0]` from the commit's PR associations without
checking any of them is still this run's head, so a stale association could
collect another branch's report. Match on head sha, ref and repository, and skip
when nothing matches. Verified against a synthesised association list where the
stale PR is listed first: the old lookup picks it, the new one does not.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codegen freshness watched the four inputs but not the generated types.ts, so a
push editing only the generated file skipped the check and left the drift to
fail someone else's unrelated codegen push. Watch the output too.
Stand redaction split results.json on the raw secret values only. auth.setup
builds a locator name from the regex-escaped user, and the file it scrubs is
JSON, where that backslash is encoded again — so a failed stand login published
the login address in a public artifact. Verified against a synthesised
results.json: the old pass leaves `qa\\.bot@…` intact, the new one removes it.
Also covers the URL-encoded form.
The visual-diffs artifact was gated on the snapshot step failing, which a
webServer or build failure inside that step also satisfies while still leaving
results.json behind — the report then announced "snapshots differ" for an infra
failure. Gate on diff images actually existing.
Dispatching `tier: all` did nothing: e2e-stand never read its input, and
e2e.yml's matching arm could not be selected. Drop both.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Concurrency is evaluated for the whole run before the job's `if`, so a run
started by any other label joined the same group and cancelled an approved,
in-flight stand run — then skipped its own job, leaving nothing in its place.
Key the group on the label as well.
The file's header promises fork PRs get Tier 1 only, but nothing enforced it.
GitHub withholds secrets from fork `pull_request` runs, so a labelled fork PR
held reviewers for an environment approval and then failed on empty
credentials. Require a non-fork head.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The job triggers on `pull_request: [labeled]`, but the guard tested
`contains(labels.*.name, 'e2e:stand')` — the label *set*, not the label that fired
the event. So adding ANY label to a PR that already carries `e2e:stand` re-triggered
the run, and `cancel-in-progress` then killed the approved, in-flight stand run and
re-pinged the environment reviewers. Gates on `github.event.label.name` instead;
workflow_dispatch is unchanged, and the job only listens to labeled + dispatch so the
event always carries a label name.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The version guard compared @playwright/test against `npx playwright --version`, but
the pinned playwright container ships no global playwright package and the step runs
before pnpm install — so npx fetched the registry latest and compared the package
against that, not against the container. Green only while latest == the pin; the next
Playwright release fails every run telling you to bump the tag to the value it already
is, and a pin bump without a re-tag passes despite real drift.
Reads driverVersion from the image's own /ms-playwright/.docker-info instead. Verified
first-hand inside v1.61.1-noble: no global playwright, .docker-info reports 1.61.1, and
the fixed check reads pkg=container=1.61.1 with the repo mounted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The stand job runs against a URL, user and password held as repo secrets, and
Playwright's results.json embeds the resolved page URL (baseURL) in navigation and
toHaveURL error messages, plus the user in locator text, on any failing run. The
`if: always()` upload then publishes results.json as a public-repo artifact for 3
days. GitHub masks secrets in logs but never in artifacts, so a red stand run leaked
the stand URL and user. The comment beside the upload claimed results.json carried
none of those — false exactly when the upload matters.
Adds a redact step (node split/join, literal — safe for password metacharacters)
that replaces each secret with <redacted> before upload; proven locally to strip a
URL + user from a sample results.json while keeping it valid JSON.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The check ran on every push and failed with a bare diff, so a backend schema
change surfaced as an unexplained red frontend job.
Run it only when a codegen input moved — the backend schema, the operations
document, the codegen config, or the lockfile (a codegen bump can change the
output, and skipping it there would let types.ts go stale and fail someone
else's later push). When the compare range can't be resolved (new branch,
force-push, tag) it still runs. On failure it now says which command to run.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Reject a non-object JSON body in the mock-LLM (JSON.parse('null') parsed but
reading .tools then threw and killed the process).
- Pin serve-dist's PORT from the config so an ambient PORT can't move it off the
port Playwright waits on.
- Fail lint on any warning, so a Playwright test with no assertion (expect-expect
is a warning) can no longer lint clean.
- Add a CI check that regenerates src/graphql/types.ts and diffs it, so the
compiled operations can't drift from the codegen input the stand validates.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Drop the screenshot threshold to 0.02 (baselines and CI both render in the
pinned container, so there is no host rasterisation noise to absorb) so a
palette change now fails the visual gate; verified 20/20 still match.
- Gate the visual guard on a container marker set by run-visual.sh and the CI
job, not the host OS, so an --update on a Linux workstation cannot overwrite
baselines with host-font pixels.
- retries:0 on the hermetic mock tier so a retry-recovered race fails instead of
merging green; wire @quarantine via grepInvert as the escape hatch.
- Scope globalTimeout to the mock tier so a real-tier run is not aborted mid-retry.
- Verify the visual container against @playwright/test's actual version instead
of a hardcoded literal.
- Key the report's "snapshots differ" advisory on the diffs artifact (uploaded
only on a real snapshot-step failure), not the visual job conclusion; and stop
a transient jobs-API error from silently leaving a stale sticky comment.
- Drop the unconsumed blob report that doubled every uploaded trace.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The gate subscribed to `labeled` so applying `e2e:stand` could start the stand
job, but every gate job carried `if: github.event.action != 'labeled'`. A label
event then ran with e2e-mock skipped, and GitHub counts a skipped required check
as satisfied on the same head SHA — so adding any label to a PR whose e2e-mock
was red published a merge-permitting check over the red one.
Move the stand tier into its own workflow (e2e-stand.yml) that owns the
`labeled` trigger; e2e.yml now fires only on real code events (opened /
synchronize / reopened) and its gate jobs never skip. The stand workflow keeps
the protected environment + reviewer gate, and uploads results.json only.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Four independent ways the signal misled:
Adding a label to a PR skips every gate job, yet the report still ran, found no
results file and overwrote a correct green comment with a red "no results". It
now returns early when the run carries no completed mock job.
A run aborted by the global timeout still uploads a results file whose failure
count is zero, so the header could go green over a red job; the verdict now
requires the mock job's own conclusion too.
The report's concurrency key was the branch name alone, so a fork PR opened from
a branch named `main` lost its comment to upstream `main` activity. The gate's
key split `opened` from `synchronize`, leaving both runs live so the older one
could finish last.
Stand failure screenshots carried the stand URL and the login into a public
artifact — screenshots are now tier-aware like traces and videos already were.
Also guard the Tier-2 teardown: the trap is the script's last statement, so a
teardown hiccup turned a passing run red.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sticky PR comment took its header from the whole-workflow conclusion, so a
visual-snapshot diff — advisory, never a required check — rendered a red ❌ over a
"62 passed, 0 failed" mock-tier stats line, reading as a gate failure it isn't.
Track the header on the mock gate instead (✅ pass / ❌ real failure) and surface a
differing visual job as a separate ⚠️ line pointing at the diff artifact.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Real-tier (Tier 2/3) fixes, the blocking set:
- stand-smoke asserts a per-route anchor (breadcrumb title span) instead
of a shared sidebar link that 0-matches on /settings/* and double-matches
on /templates — the smoke was structurally red on 3 of 5 routes.
- stand-tier trace/video forced off (they embedded the session cookie and
the stand password in a public-repo artifact) and the stand project
selects @stand via config grep (the CI step selected zero tests).
- run-local-tier's sandbox filter is anchored to the seeded 9xxxx range
(`^/?pentagi-terminal-9[0-9]{4,}$`) so it can't force-remove a dev's
flow-9 sandbox; sandbox cleanup moved behind the keep-stack gate; compose
logs are captured before teardown.
- mock-LLM wraps the request handler (a non-JSON body crashed the process);
its transcript now issues a real terminal exec via a delegate chain and
flow-run asserts the terminal output; OpenAI-shaped error envelope.
Mock-gate rigor + coverage:
- order-insensitive deep-subset variable matching; unmatched WS subscribes
are now accounted at teardown; REST/GraphQL entries can pin a body/vars
subset; favorite-star asserts the persisted (not optimistic) value.
- new flows/create smoke (the primary journey had no mock coverage), a
graphql-ws protocol unit test against the real client, and a
route-manifest completeness test.
- affected-routes no longer under-scopes shared component dirs.
Hygiene: .mjs tools now linted/prettier'd; dead gitignore line; JSDoc/
sheet aria-label/schema-compat/trend/serve-dist/report-comment fixes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lays the Phase-3 substrate on top of the three tiers.
- schema-compat pre-flight (e2e/tools/schema-compat.mjs): introspects a
target backend's live GraphQL schema and validates every frontend
operation against it, so deploy skew (a renamed/removed field) fails
once, readably, instead of as dozens of red specs. Verified against the
live Tier-2 backend both ways: 105 operations pass, an injected bogus
field is caught with the exact location.
- trend aggregation (trend.mjs): turns a run's results.json into one JSONL
record (p50/p95 spec duration, slowest three, pass/flaky/fail) so slow
regressions are visible, not just green/red; CI appends it to a
90-day-retained artifact.
- diff-scoping (affected-routes.ts + affected.ts): maps a diff to the
manifest routes it touches via each route's owning sources — the
substrate for selective runs and for scoping the exploratory agent.
Pure mapping fn, unit-tested (backend-only → none; shared infra → all;
owned → that route).
- CI: an e2e-stand job (label-gated + a protected Environment whose
reviewers approve before secrets are exposed; schema-compat runs first),
a trend step, and an LLM-independent @stand smoke.
- docs: the stand tier, trend/affected tools, and the LLM advisory recipe
(playwright-mcp + init-agents + guardrails) — the recipe the
deterministic tiers plug into, not a bespoke bot.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
18 baselines (9 manifest routes × light/dark) compared only inside
mcr.microsoft.com/playwright:v<version>-noble, so pixels are identical on
every machine and CI; the wrapper derives the tag from the installed
@playwright/test version, making the pin drift-proof, and a CI guard
fails loudly if the workflow's container tag falls behind.
macOS hosts cannot run the visual project directly (parallel darwin
baselines) nor mount their node_modules into the container (native vite
binaries): the wrapper builds dist on the host and the container serves
it with a dependency-free static server — route mocks intercept API
calls before the network, so no proxy is needed. The xterm canvas is
masked (SwiftShader pixels are driver-dependent). Determinism proven by
back-to-back container runs. The e2e-visual CI job is advisory, never a
required check.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
specs/real/** now exercise the actual agent loop end to end: create flow
-> image/language/title -> tool-call-ID sampling -> subtask_list plan ->
done barrier -> subtask_patch refine -> report_result — with messages
streamed over the real GraphQL websocket and the flow settling in
Waiting. The mock LLM is ~150 lines of Node driven by a deterministic
first-match transcript; the custom provider env seam means zero backend
changes.
The stack is fully isolated from a developer machine:
- own compose project/network/ports (8444/5433), coexists with a dev stack
- --env-file /dev/null so the developer's .env (live keys, DOCKER_HOST,
config paths) never leaks into the e2e backend
- flow ids seeded from 90001: sandbox containers are named
pentagi-terminal-<flowId> on the shared docker daemon, so the range
keeps them clear of dev flows and makes runner cleanup unambiguous
(down -v cannot remove them — the backend spawns them outside compose)
- pentagi healthcheck via busybox wget (the alpine image has no curl),
since the base service has none and up --wait returns too early
Auth for live tiers moved to the canonical setup-project + storageState;
the localStorage seed is now mock-tier-only (a forged client session has
no cookie behind it and the first 401 wipes it). CI gains a nightly/
dispatch-only e2e-local job.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
setup-go's cache:true already caches the build and module caches keyed
on go.sum; the extra actions/cache step restored the same read-only
~/go/pkg/mod on top and failed with "Cannot open: File exists" whenever
both caches hit. Also sets the workflow's GITHUB_TOKEN to contents:read.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The default tier runs the production bundle (vite build + preview)
against a cassette-driven mock of the whole API surface — GraphQL over
HTTP, graphql-transport-ws subscriptions, and REST — so the suite needs
no backend, no keys, and no secrets, and fork PRs can run it.
- mock engine: (operationName, variables) matching with sequenced
entries and world flags (login flips /info guest->user without
call-order coupling); unmatched calls answer 501 and fail the test,
so nothing leaks through the vite preview proxy to a live backend
- ws mock follows the graphql-transport-ws contract the app's client
needs: immediate ack, nothing before ack, streams stay open, delta
cursors survive reconnects, drops use retryable close codes
- clock and timezone pinned on the mock tier: formatDate branches on
isToday/isThisYear, so unpinned cassette dates rot within a day
- cassettes are TS modules typed against the generated GraphQL types,
so schema/operation drift fails the existing tsc gate at compile time
- CI: fork-safe e2e.yml (read-only token, no secrets, no write steps)
plus e2e-report.yml posting a sticky PR comment via workflow_run,
resolving fork PRs by head SHA
- smoke specs (login redirect, form login, authenticated /flows render)
green on the mock tier; e2e/ wired into tsc, eslint, and prettier
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "Frontend - Install dependencies" step ran pnpm install --frozen-lockfile
with continue-on-error: true, swallowing the ERR_PNPM_OUTDATED_LOCKFILE that the
flag exists to raise. The rest of the frontend pipeline (prettier, lint,
type-check, test) already gates strictly, so a stale pnpm-lock.yaml would only
surface later as a confusing downstream "command not found". Drop the flag so
lockfile drift fails loudly at the install step. Backend steps left as-is.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- add frontend/.nvmrc (24.17.0) as the single source of truth for Node;
GitHub CI reads it via node-version-file, Dockerfile uses node:24.17.0-slim
- bump packageManager to pnpm@11.8.0; drop "corepack prepare pnpm@latest"
so the pnpm version derives from packageManager everywhere
- migrate pnpm onlyBuiltDependencies -> allowBuilds in pnpm-workspace.yaml
(the package.json "pnpm" field is no longer read by pnpm 11)
- add a CI step that fails if the Dockerfile Node tag drifts from .nvmrc
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Prettier / Lint / Test were `continue-on-error: true` (advisory — failures did not fail CI), the same gap that let type errors pile up. Now Prettier, Lint, Type check and Test all block the lint-and-test job (which runs on every branch push).
Prerequisite: `prettier --write` on 5 pre-existing non-conformant files (pages/login.tsx, lib/report/report-pdf.tsx, 3 *.test.tsx) so the now-blocking Prettier check passes — pure formatting, no logic change. Install stays advisory (setup step); backend checks unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vite build strips types and never type-checks, and the old build`s bare tsc (solution config, files:[]) checked nothing — so type errors reached main unnoticed (which is how 76 had accumulated). Now they fail fast everywhere.
- build: `tsc -b && vite build` — `pnpm build` (and the Docker image build, which runs `pnpm run build`) fails on any type error before bundling.
- add `typescript` script (`tsc -b`, checks both app + node project configs).
- ci.yml: blocking "Frontend - Type check" step in lint-and-test (runs on every branch push; docker-build only runs on main/tags).
Verified: a type error makes both `pnpm run typescript` and `pnpm build` exit non-zero (vite never runs); removing it restores a clean build.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sergey Kozyrenko