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>
The doc told readers to dispatch the workflow "with tier: stand", an input
fc15ec8 removed — workflow_dispatch takes none — and claimed the protected
Environment is what stops a fork or mislabeled run from reaching secrets. It is
not: the job's `if` (label name plus a non-fork head, added in 7b1ecfd) skips
before the Environment is ever evaluated. Describe the real two-gate model: the
`if` first, then the Environment for a run that clears it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sequencing cursor embedded the whole request body, so an endpoint whose
body carries a per-call volatile field — an idempotency key, a timestamp — got
a different cursor key on every call and re-served entry one forever, never
advancing the sequence. Key on a signature of the matched entries' pins instead:
two genuinely different sequences still separate (their entries pin different
subsets), but a volatile field no longer fragments a single sequence. Unit test
covers it; it fails on the old body-keyed cursor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "shows the upload call to action" test asserted only the "No resources yet"
title — the sole empty-state surface in the suite — so dropping the drop-zone's
actionLabel kept it green. Assert the CTA button, scoped to the drop zone (its
size hint is unique) so the toolbar's Upload button is not mistaken for it.
Removing actionLabel now fails the test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The target-size waiver matched a bare `aria-label="Select `, so any control on
the page labelled "Select …" — not just the file-manager row-select buttons —
was waived. axe emits those buttons as `button[aria-label="Select <name>"]` with
no parent path, so the label is the only anchor available; requiring the button
tag at least stops a future non-button "Select …" control from slipping the
rule. /resources a11y stays green; a div/anchor "Select" is now caught.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes a stale "revisit with the design pass" comment left above the rewritten
scoped-waiver block in routes.ts, and applies the formatting the repo's own
`pnpm run prettier` wants across the a11y, palette and provider-form files —
per-file prettier checks were missing a plugin the package script loads, so a
few of this session's commits landed formatting the gate would flag.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`create --dirty` piped `git diff HEAD` without --binary and copied no untracked
files, so a regenerated PNG baseline aborted the patch (leaving a half-built
worktree) and a new spec never reached the sandbox — and because the failure
printed onto stdout, the caller captured an error string as the sandbox path.
Use --binary, copy untracked non-ignored files separately, and trap ERR to tear
the worktree down and report on stderr. Verified with a modified baseline plus a
new untracked spec: both now arrive in the sandbox.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The gate compared `[...new Set(offenders)]` against ACCEPTED, so a waiver that
names one node absorbed any number of nodes carrying the same token — a second
element reusing an already-waived off-palette class stayed green, against the
"waive one node, not a rule" contract. Compare the sorted offender list
directly: two nodes with a waived token now fail where one is accepted. All 18
scans stay green (each waived token appears exactly once today).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PALETTE_UTILITY matched only numbered hues and arbitrary values, so text-white,
text-black and bg-transparent were invisible to it: an overlay like
`cn(badgeVariants({variant:'blue'}), 'text-white')` on a badge went unmeasured.
Add white/black/transparent (with optional opacity) to the colour alternation.
No badge or button variant uses them, so all 18 scans stay green; injecting a
text-white overlay on a rendered badge now fails the gate as an off-palette node.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The only data assertion on the stand tier ran on Playwright's 5s default expect
timeout — the config overrides only the screenshot timeout — so a healthy but
slow stand backend serving the list a little late would fail the smoke falsely.
Give it a 30s budget, matching the real-backend nature of the tier.
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 settings-providers cassettes left nine of eleven default configs null and
every model catalog empty. Verified against the live backend that this is a
shape it never sends: gemini/deepseek/ollama all return a non-null default and a
populated model catalog even with zero keys configured. The empty catalogs made
the create-form's model-dropdown seeding path structurally unreachable — no spec
could reach it.
Give every type a default config and a two-entry catalog, and add a spec that
picks a type and opens an agent's model dropdown: it asserts a catalog-only
model appears there. That option is gone when the catalog is empty, so the spec
fails on the old backend-impossible shape — the coverage that was missing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The waivers were passed to all nine /flows/5 scans (base + 8 tabs), so a
Files-tab waiver silenced its rule on every other tab: a new violation of the
same shape on Assistant would stay green. Add an optional `tabs` to A11yWaiver —
omitted means route-wide (the by-design message metadata), a list means only
those tabs — and filter per scan.
The gate itself supplied the real ownership once scoped: the progressbar-name
waiver belongs to Tasks, the truncate-contrast one to Screenshots, and
table-container scrollable to Dashboard as well as Files — all previously masked.
Proven by injecting a Files-shaped table-container into the Assistant panel: it
now fails the Assistant scan where the route-wide waiver used to absorb it.
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 provider detail form was the only one of the three detail forms created
without `resetOptions: { keepDirtyValues: true }`. Its seeding effect keys on
`data`, and settingsProviders is cache-and-network + replaceWithIncoming, so a
background refetch (list→detail race, or a provider* subscription) delivers a
fresh `data` reference mid-edit and the plain reset() silently wipes whatever
the user was typing. settings-prompt and template already guard this, the
latter with a comment naming the exact hazard.
Live-reproduced on the docker branch image: an edit typed during a delayed
refetch reverted to the server value. Regression test delivers a fresh data
reference after an edit and asserts it survives — it fails without the option.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`pnpm run prettier` (which CI gates on) failed on these two files — line
wrapping only, in edits from the M5 pager-switch work. The branch tip was red
until this. No behaviour change; `prettier --check` over the {src,e2e} glob is
clean now.
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>
The REST gate matched on method and path alone, so an endpoint that carries its
payload in the query and sends no body could not be pinned — `bodySubset` has
nothing to bite on, and the path hit answered success whatever the request
named. Unmatched-call diagnostics dropped the query too, so a miss printed a
path that looked right.
Add `querySubset`, the counterpart to `bodySubset`, thread the query into the
matcher and the sequencing cursor, and report it on a miss. No spec exercises a
query-carrying endpoint yet; the unit test pins the behaviour and fails when the
new filter is removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`paint()` clears to transparent and the label was painted alone, so a
translucent text token kept its opaque base: 30% black on white measured 21:1
instead of 2.12:1 — a failing token reported as perfect contrast. Paint the
label on the same layer stack as the surface.
No probed token is translucent today, so every current reading is unchanged
(7/7 still pass); this stops the helper lying about the next one.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The cassette gave entry 503 a Terminal body under type Report — a combination
`getMessageType` never produces, since the terminal tool's output carries the
terminal type. It mattered: only a report auto-expands, so the spec's two
xterm assertions passed on a panel that was open for the wrong reason.
With the realistic pairing the message starts collapsed, so the spec now walks
the path a user does — assert one xterm, open the details, assert the second
and its content.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Verifying a claim about a gate means breaking something on purpose, and review
runs kept doing that in the working tree: probe specs left behind under src/ and
e2e/, tracked files edited mid-run, and once an `rm -rf` of a directory the run
had not created. Nothing survived to the commits, but only because each run
happened to clean up after itself.
`review-sandbox.sh` hands out a detached `git worktree` under $TMPDIR, optionally
carrying the uncommitted diff and a hardlinked node_modules so pnpm, vitest and
playwright run there. Verified by doing the damage on purpose inside one — an
edited world.ts, a stray probe spec, a deleted frontend/docs — and confirming
the real tree came back untouched.
node_modules is hardlinked rather than symlinked because pnpm treats a symlinked
modules dir as foreign and tries to purge and reinstall it, which would reach
through the link into the real tree; it aborted here only for want of a TTY.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deleting `eligible`'s second line — the one that makes a newly-enabled flagged
entry outrank the unflagged candidates — left the whole unit tier green. The
existing cases use two entries, where the consumed-index cursor lands on the
flagged entry anyway, so sequencing alone reproduced every asserted progression.
Add a three-candidate case where the two disagree: it passes as written and
fails with that line removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`pageErrorLog` was opt-in with nothing asserting it on teardown, so a spec that
never destructured the fixture collected nothing and an uncaught exception on
that surface failed nothing. Make it auto and assert on teardown. Verified by
throwing from an init script in the populated-providers a11y test, which does
not take the fixture: green before, red after. Console errors stay opt-in —
several specs drive genuine 4xx paths.
The remaining flow-detail waivers keyed on bare utility classes: `bg-primary`
matched every Progress root through `bg-primary/20`, and the other two were
similarly unanchored. Pin all three to the offending nodes, taken from a run
with the waivers disabled.
/templates and /knowledges claimed whole page dirs, so a diff touching only the
unswept detail page scoped to the list route instead of the run-everything
fallback. Name the list files; leave `src/features/knowledges` unowned, since
its only importer is that unswept route.
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>
The gate sanctioned the flat union of every badge and button variant, so any
variant's hue passed on any element — precisely the wrong-variant reuse it was
built for. Match each element's palette tokens against a single variant's set,
by equality rather than containment: a lone borrowed token is a subset of the
variant it came from, and containment waved it through (verified — an outline
badge carrying the blue variant's `text-blue-800` passed until the switch).
The route sweep also reaches /settings/providers only with the empty seed, so
the provider cards were never palette-scanned, unlike in the a11y and visual
gates. Add the same dedicated populated sweep.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The resolver builds default provider configs without an ID, so gqlgen marshals
the bare number 0 for every one of them. The cassette handed each a distinct
string id instead, which left `ProviderConfig.keyFields`'s id-0 branch — the one
that stops those un-normalisable defaults collapsing onto a single cache entry —
unexercised by any test.
Serve id 0 and the plain provider name, give each default a distinguishable
model so a collapse is observable, and pin the cache contract directly: with the
keyFields branch removed the new test reads anthropic's default as openai's.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The array guard added earlier covers the container but not its elements:
`tool.function?.name` chains off `function`, not off `tool`, so a body of
{"tools":[null]} throws a TypeError outside the request try/catch and takes the
process down mid-suite. Reproduced: the request returned nothing and the server
stopped answering; every payload now answers 200 with the process still up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`DetailNavigationToolbar` was documented as returning `null` when
`controller.itemsEmpty`. It has no such branch — it always renders the
fragment, and `itemsEmpty` is read nowhere outside the module's own tests.
Describe what it actually does: mount straight away and let the buttons show
their disabled/"–/0" state until items land.
"Prev/Next runs the same matcher the list filter uses" is also untrue. The
toolbar's `createTextMatcher` normalizes NFKD and strips combining marks;
`DataTable`'s `globalFilterFn` only lowercases. Searching `cafe` hides a `café`
row from the table while Prev/Next still steps onto it. Record the divergence
rather than the intent.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The dashboard laziness check asserted the Overview heading was unattached
straight after goto. The route is lazy, so that is satisfied by "nothing has
rendered yet": mounting the panel eagerly with forceMount still passed. Wait
for the Analytics panel first — with the gate in place forceMount now fails it.
The flow-detail a11y waivers matched `button[aria-label`, which waives
button-name and target-size for every labelled button on the route rather than
the file-manager controls that actually violate them. Anchored both on the
offending nodes; the real violations stay waived and a nameless icon button or
an undersized labelled button no longer does.
The rejected-login smoke test asserted only the disabled half of the behaviour
its name describes. Change a field and assert Sign in comes back.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`maxDiffPixelRatio: 0.01` allows 9,216 differing pixels on a 1280x720 baseline
— more than the area any single foreground token covers — so the gate could not
fail on the palette regression it was tightened for. Verified: swapping the
primary token from blue to green passed all 20 baselines before this change and
fails 6 of them after.
Both baselines and runs render in the pinned container, so the remaining noise
is glyph anti-aliasing; 200 pixels covers it with 20/20 still matching.
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>
Sampling window.location on a timer measured the host's speed: on an unloaded
machine the sibling landed before the third sample, so the guard that the
samples spanned the switch failed 3 runs in 4. Weakening it to a length check
made it vacuous instead — the sampling loop always runs its full count.
Record every pushState/replaceState the app makes and assert the exact
sequence, which is what "without passing through the list" claims. Verified by
routing the pager through the list: the trail assert names the detour.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The pager keeps FlowProvider mounted and only swaps subscription variables,
which is the one path where a superseded stream can stay open. Asserting the
leaked message is absent from the DOM cannot catch that: messageLogs is keyed
by flowId, so a frame carrying the old flow's id is written to the old flow's
cache slot and is never rendered under the new one, leaked or not.
Assert the mock's live subscriber count for the superseded stream instead, and
export the stream-key builder so the spec cannot drift from the mock's format.
Verified by holding a stale subscription open in the provider: the new assert
fails while every DOM assert stays green.
Also restores the class member ordering the linter requires.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The a11y and palette sweeps clicked a tab and scanned immediately, so the two
round-trip panels could be scanned while still skeletons. Skeletons carry no
axe or palette violations, so those scans passed on an empty panel instead of
the content they exist to check. Tabs now carry a readiness locator beside the
name and both sweeps wait for it.
Ordering is part of the same defect: the flow auto-opens the Assistant panel
when it has no message logs, so an Assistant-first sweep clicked a tab that was
already open and asserted a marker that predated the click. Dashboard leads the
left-hand pair, and the sweep asserts each panel is absent before its own click
so a future reordering that makes an iteration a no-op fails loudly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
test.setTimeout(240_000) was below the sum of the sequential step ceilings
(60+30+90+90+90+60 = 420s), so a legitimately slow-but-passing real run was killed
mid-step with a generic timeout that masked the real failure — the exact reason the
config's own globalTimeout comment gives. Raised to 450_000.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The malformed-body guard only rejects non-object top-level payloads, so a body like
{"tools":5} passed it and reached `(payload.tools ?? []).map(...)` — `.map` on a
number throws outside the try/catch and kills the process, dropping any in-flight SSE
streams and violating the guard's stated contract. Guards on Array.isArray before
mapping. Proven: the old expression throws on {"tools":5}, the new one yields "" and
still maps a real tools array.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The local tier fell back to https://localhost:8443 when E2E_BASE_URL was unset,
while stand fail-fasts. Both tiers bake the real (paid) flow-run specs against a
real backend, so a bare `E2E_TIER=local pnpm e2e` silently ran a real flow against
the developer's dev stack — a paid LLM call, a junk flow, and a sandbox
container/volume the wrapper cleanup never removes. Requires E2E_BASE_URL for local
too; run-local-tier.sh already supplies it, so the legitimate path is unaffected.
Verified: bare local now throws, local + E2E_BASE_URL loads clean.
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 lazy-mount check used toBeHidden(), which passes for a detached node AND for a
mounted-but-hidden one — so it did not actually verify the "not mounted while
Analytics is active" claim it documents; an eager mount (all overview queries
firing behind a hidden panel) would still pass. Switches to not.toBeAttached().
Confirmed green: the panel is genuinely unmounted (Radix drops the inactive tab).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
graphql-ws resubscribes active sinks in a microtask after the retry connect, so
the reconnect test gated on the wrong signal: after `retries == [false, true]` it
raised the flag immediately, and a poll landing in the gap delivered seq:2 to an
empty subscriber set — the no-replay contract then lost the frame, timing out. The
report flagged it as CI-load-dependent (0/25 local repro); the mechanism is a real
ordering gap regardless.
Adds MockWorld.subscriberCount(streamKey) and waits for the resubscribe to
re-register the sink before raising the flag. Not a repro of the flake (it does not
reproduce locally), but it closes the ordering gap the flake rides on. 4/4 green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The mock PR gate runs retries:0, but trace was 'on-first-retry' — so it never
recorded a trace on that tier, while docs/e2e.md ("Debugging a red CI run") and
the auto-posted PR comment both tell you to open trace.zip from the mock tier's
e2e-report artifact. Every red gate run dead-ended the advertised debug path.
Switches trace to 'retain-on-failure' (matching video on the same line), keeping
the stand tier at 'off' for the session-cookie privacy reason. Proven on a clean
host: a failing mock test (retries:0) now writes trace.zip + video.webm.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The palette gate scanned only each route's default view, so off-palette colours
behind a tab (which Radix unmounts while inactive) were never checked — the a11y
gate iterates tabs, this one did not, and they had drifted. The flow Files tab
carries a live off-palette node (file-manager's expand-all control,
hover:text-blue-400) that went green purely because the panel was unmounted.
Adds a per-tab scan mirroring the a11y gate, with tab-scoped waivers keyed
`${path} [${tab}]`. The file-manager control is waived on the Files tab under the
same "goes with the design pass" rationale it already carries on /resources.
Proven: the Files-tab scan passes against the exact waived offender (a non-empty
`toEqual`), so the scan reaches the panel — the old default-view scan could not.
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>
Two e2e gaps surfaced by an A/B review of this branch:
- knowledges detail gains an authz-denial case: the backend denial string
"requested permission '<perm>' not found" contains "not found", and a naive
not-found match would bounce a user who merely lacks access to the list. The
spec asserts it stays on the route behind Retry. Proven: reverting errors.ts to
the pre-fix predicate turns this red (page bounces to the list).
- template-detail and pager header-order specs used findIndex, which returns -1
for an absent label; -1 < any real index, so "Save left of Previous" passed even
if the Save/Next button had vanished. Both now assert every referenced button is
present before ordering them, so a missing-button regression fails the spec.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
isTemplatePending ORed the raw Apollo `loading` flag, gating Save/Rename/Delete.
Commit 3d5fc75 fixed the sibling *render* gate (`isLoadingTemplate && !template`)
but left this `disabled` gate one level down still keyed on raw loading, so a
background revalidation greyed out the actions on a form the user had already
edited. flow.tsx uses the entity-guarded `isFlowLoading`; template was the only
detail page reading raw loading here. The render gates above (both branches carry
`!template`) already make the form unreachable without a loaded template, so the
loading term only added a dead disabled window — dropped it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two ways a detail page redirected the user off to the list when it should not have,
both surfaced by an A/B review of this branch:
- isNotFoundError matched /not found/i, but the backend's authz failure
"requested permission '<perm>' not found" (graph/context.go) also contains
"not found". A user who merely lacked a permission was silently bounced to the
list instead of seeing the denial. Authz strings now read as real failures.
- flow-provider's isFlowMissing dropped the `!flow` guard that its two siblings
(flowLoadError and the not-found toast) apply: under errorPolicy:'all' a partial
not-found error rides alongside a flow that loaded fine, so the redirect fired on
a flow that had rendered correctly. The disjunct is gated on `!flowData?.flow`
again, extracted to a pure `deriveFlowMissing` so the regression is unit-tested.
errors.test gains the real authz strings (revert the predicate -> red);
flow-provider.test covers the partial-error-with-loaded-flow case (revert -> red).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The B1-B3/B6/B7 pass guarded the settings detail pages' error branch with
`&& !data`, but each detail page has an `if (loading)` branch that runs FIRST,
and it was left unguarded — so the fix it was meant to deliver never applied.
The queries are cache-and-network, so a background revalidation (a list→detail
navigation into a warm cache, or a post-save refetchQueries) reports loading
true with cached data present and blanks the edit form to the full-page spinner
before the guarded error branch is ever reached.
- settings-prompt.tsx / settings-provider.tsx: `if (loading)` -> `&& !data`
- template.tsx spinner: `if (!isNew && isLoadingTemplate)` -> `&& !template`,
which also realigns it with knowledge.tsx (fixed in 28ab3d2 to gate on the
entity, not raw loading) — the two had silently diverged.
- docs/list_detail_pages.md: the "canonical render gate" recipe still taught the
unguarded `if (isLoading)` it tells new pages to copy; both branches now gate.
settings-provider.test gains the loading-with-cached-data case (revert -> red);
the detail loading branch had zero coverage before. Found by the adversarial
review of the previous fix pass — the guard I applied was one line short.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The five settings surfaces guard their loading and error branches
inconsistently. Their queries are cache-and-network, so a subscription- or
mutation-driven refetch flips `loading` (and, on a failure, `error`) to true
while the cached data is still on screen. Where the guard omits `&& !data`,
that refetch replaces a populated list — or a provider/prompt edit form with
unsaved changes — with the full-page spinner or error screen for the duration
of the round-trip.
Each branch now matches the one beside it in the same file, which already
carried the guard and the comment "a failed background refetch must not blank
a working list":
- api-tokens / providers / prompts lists: `if (isLoading)` -> `&& !data`
- prompt / provider detail: `if (error)` -> `&& !data`
Proven by runtime repro, one per class: settings-provider.test asserts the
form survives an error arriving with cached data (revert -> red), and
settings-providers.test asserts the populated table survives loading:true with
cached rows (revert -> red). The other three are the identical one-line guard
against the same cache-and-network behaviour.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
