Commit Graph
861 Commits
Author SHA1 Message Date
Sergey KozyrenkoandClaude Opus 4.8 162c84d99a test(e2e): let a cassette entry pin the request's query string
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>
2026-07-24 19:34:09 +07:00
Sergey KozyrenkoandClaude Opus 4.8 fadde5ad5a fix(e2e): composite text colour onto its own ground before measuring contrast
`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>
2026-07-24 19:34:09 +07:00
Sergey KozyrenkoandClaude Opus 4.8 dbfdbe0e8d test(e2e): pair the terminal message with the type the backend emits
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>
2026-07-24 19:34:08 +07:00
Sergey KozyrenkoandClaude Opus 4.8 c906539a3d test(e2e): give review agents a sandbox instead of the live checkout
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>
2026-07-24 19:21:00 +07:00
Sergey KozyrenkoandClaude Opus 4.8 0a8ca104e0 test(e2e): cover the flag-outranks-sequence rule in the cassette matcher
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>
2026-07-24 19:04:48 +07:00
Sergey KozyrenkoandClaude Opus 4.8 9d4f5912f4 test(e2e): assert uncaught page errors everywhere, and narrow two over-broad claims
`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>
2026-07-24 18:58:55 +07:00
Sergey KozyrenkoandClaude Opus 4.8 fc15ec8c1b fix(ci): close four gaps in the workflow gates
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>
2026-07-24 18:43:50 +07:00
Sergey KozyrenkoandClaude Opus 4.8 585df2a953 test(e2e): sanction palette tokens per variant, and sweep populated providers
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>
2026-07-24 18:40:24 +07:00
Sergey KozyrenkoandClaude Opus 4.8 a51e36fdc7 test(e2e): serve default providers the id the backend actually sends
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>
2026-07-24 18:40:24 +07:00
Sergey KozyrenkoandClaude Opus 4.8 c8314c4213 fix(e2e): stop the mock LLM dying on a null element in tools
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>
2026-07-24 18:40:24 +07:00
Sergey KozyrenkoandClaude Opus 4.8 07fa2d3a31 docs: correct two detail-navigation claims the code does not make
`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>
2026-07-24 13:26:10 +07:00
Sergey KozyrenkoandClaude Opus 4.8 cec08d075b test(e2e): close three gates that could not fail
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>
2026-07-24 13:13:29 +07:00
Sergey KozyrenkoandClaude Opus 4.8 5cec81b860 test(e2e): give the visual gate an absolute pixel budget
`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>
2026-07-24 13:13:16 +07:00
Sergey KozyrenkoandClaude Opus 4.8 7b1ecfdc77 fix(ci): stop unrelated labels from cancelling the stand run, and gate it to non-forks
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>
2026-07-24 13:13:15 +07:00
Sergey KozyrenkoandClaude Opus 4.8 3b948d68ea test(e2e): prove the pager route by its history trail, not by sampling
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>
2026-07-24 13:00:06 +07:00
Sergey KozyrenkoandClaude Opus 4.8 c45fbd401f test(e2e): pin the pager-switch stream teardown on the subscriber set
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>
2026-07-24 12:59:58 +07:00
Sergey KozyrenkoandClaude Opus 4.8 d414ec0bb6 test(e2e): gate per-tab scans on the panel's own content
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>
2026-07-24 12:59:48 +07:00
Sergey KozyrenkoandClaude Opus 4.8 7907ee5f60 fix(e2e): raise the flow-run test budget above the sum of its step timeouts
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>
2026-07-24 02:42:41 +07:00
Sergey KozyrenkoandClaude Opus 4.8 3e4316e53c fix(e2e): don't crash the mock LLM on a non-array tools field
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>
2026-07-24 02:41:58 +07:00
Sergey KozyrenkoandClaude Opus 4.8 ecfb2029ce fix(e2e): require E2E_BASE_URL for the local tier, like stand
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>
2026-07-24 02:41:02 +07:00
Sergey KozyrenkoandClaude Opus 4.8 aec236d32e fix(ci): gate the stand run on the label just added, not the whole label set
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>
2026-07-24 02:37:15 +07:00
Sergey KozyrenkoandClaude Opus 4.8 b919b7a839 test(e2e): assert the overview panel is unmounted, not merely hidden
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>
2026-07-24 02:36:00 +07:00
Sergey KozyrenkoandClaude Opus 4.8 e9c1c56b8b test(e2e): gate the reconnect test on resubscribe, not just the socket reconnect
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>
2026-07-24 02:28:52 +07:00
Sergey KozyrenkoandClaude Opus 4.8 b6e5564f49 fix(e2e): write a trace on mock-tier failure so the debug path exists
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>
2026-07-24 02:23:56 +07:00
Sergey KozyrenkoandClaude Opus 4.8 8a6eecd6e6 test(e2e): sweep tab panels in the palette gate, not just the default view
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>
2026-07-24 01:22:58 +07:00
Sergey KozyrenkoandClaude Opus 4.8 05e79902d8 fix(ci): read the visual container's Playwright version from the image, not npx
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>
2026-07-24 01:06:40 +07:00
Sergey KozyrenkoandClaude Opus 4.8 184c2b6b8c fix(ci): redact stand secrets from the e2e report before the public upload
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>
2026-07-24 01:04:32 +07:00
Sergey KozyrenkoandClaude Opus 4.8 0d68d1d622 test(e2e): cover the authz-denial redirect + fix vacuous header-order asserts
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>
2026-07-23 16:19:03 +07:00
Sergey KozyrenkoandClaude Opus 4.8 2de6386cb5 fix(templates): don't disable the detail actions on a background refetch
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>
2026-07-23 13:52:15 +07:00
Sergey KozyrenkoandClaude Opus 4.8 f0860a7858 fix(ui): don't treat an authz denial or a partial error as a missing record
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>
2026-07-23 13:52:02 +07:00
Sergey KozyrenkoandClaude Opus 4.8 3d5fc75f7b fix(ui): guard the detail loading branch too, not only the error branch
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>
2026-07-23 02:14:53 +07:00
Sergey KozyrenkoandClaude Opus 4.8 1308f2d010 fix(settings): don't blank a working settings view on a background refetch
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>
2026-07-23 01:05:14 +07:00
Sergey KozyrenkoandClaude Opus 4.8 66d9c835b7 test(e2e): wait for Submit before clicking in the real-flow spec
On a cold Tier-2 stack the new-flow form stays invalid — and Submit disabled —
until the providers query lands, so clicking it straight away burned the whole
240s test timeout waiting for a disabled control. Wait for it to enable first.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-23 00:21:52 +07:00
Sergey KozyrenkoandClaude Opus 4.8 28ab3d2e61 fix(ui): render a retryable error on a detail load failure, not "not found"
The template and knowledge detail pages dropped `error` from their query and
inferred "this record does not exist" from the absence of data. A real load
failure — a network drop, a 5xx, a cold-cache backend error on a deep link —
therefore rendered "Template not found" / bounced to the list with a toast,
offering no way back in short of retyping the URL. Only a genuine 404 should
do that; a transient failure should keep the user on the route behind Retry.

Both now split the two outcomes the way flow already does: a real error →
in-page ErrorState + Retry; a settled-empty result or a not-found error →
the existing redirect/not-found card. The `no rows`/`not found` predicate that
flow-provider kept privately becomes the shared `lib/errors.ts#isNotFoundError`
now that three call sites need it, and flow-provider moves onto it.

Proven by a runtime repro, not by reading: knowledge.test.tsx asserts the
in-page error + no redirect on a real failure and the redirect on a genuine
not-found — reverting the fix drops it to a failure. errors.test.ts pins the
predicate's two sides. e2e repros on both detail routes drive it through the
production bundle for CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-23 00:21:33 +07:00
Sergey KozyrenkoandClaude Opus 4.8 f95c48b062 refactor(knowledges): give knowledgeId back to the route param
`knowledgeId` means the route param everywhere else — the route is declared as
`knowledges/:knowledgeId`, and both knowledge.tsx and the route-title resolver
destructure it under that name. This header had taken the name for the loaded
document's id and left the route param as `routeKnowledgeId`, so the one file
that reads both used the project's vocabulary backwards.

The document's id needed no binding of its own: `handleRenameSave` two functions
above already guards on `knowledge` and passes `knowledge.id`, so `handleDelete`
now does the same and the rename input keys off `knowledge?.id`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 18:23:00 +07:00
Sergey KozyrenkoandClaude Opus 4.8 e51f686e5c refactor: drop the header-order comment and the rest of the narration
The comment on AppHeaderActions claimed the component pins itself to the right.
It does not — the sibling content block takes `flex-1` and pushes it there — and
the rule it stated was an instruction to call sites, not a fact about the twelve
characters below it. The convention now lives in docs/list_detail_pages.md, next
to the detail-page recipe that needs it.

Two more went the same way, by making the code carry the point instead: the
knowledge header's two ids are now `documentId` and `routeKnowledgeId`, so
there is nothing to warn about, and the separator that doubles up is gated on a
named `hasViewRow`. In the pager spec the injected delay is held by an assertion
that counts the samples taken before the sibling appears — removing the delay
now fails the test instead of quietly emptying it.

Across the e2e specs, six comments that only explained why a test exists are
gone and five more are down to one line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 18:16:04 +07:00
Sergey KozyrenkoandClaude Opus 4.8 f24b75f350 refactor: cut the comments that only explain the author
Seven blocks went: three justified a test's existence or the way an assertion
was written, one repeated the fact stated two hunks above it, one restated a
guard that a unit test now enforces, and two paraphrased the identifier sitting
next to them. The five that stay each name a consequence that is invisible from
the line: a delay whose removal makes the surrounding assertion vacuous, the
leftward-growth contract of a shared header primitive, a route id that must not
be taken from the entity beside it, a separator that doubles for callers without
a mode toggle, and a flag read by a subscription gate forty lines away.

The counter's width reservation gets a unit test over three set sizes instead of
prose, so its scaling is pinned by the suite rather than asserted in a comment.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 17:55:35 +07:00
Sergey KozyrenkoandClaude Opus 4.8 46457026e8 fix(ui): repair what the always-rendered header exposed on the other two pages
Three defects the review found, all in surfaces the flow page does not share.

The knowledge pager took its current id from the loaded document, so now that
the cluster no longer unmounts it sat there reading "–/N" with both arrows
dead for the whole document fetch — and again after every step. It takes the
route's id, like the flow and template pages already did.

The knowledge actions menu drew a doubled divider while loading: the View row
between the two groups only exists when a mode toggle is passed, which the
loading shell does not do.

The isLoading prop threaded down to that header could not change any output —
the only caller that passed it also passed a null document, so the flag it fed
was already true. Removed rather than left as a signal that looks live.

Also: the template not-found card reuses the page header, which since the
redesign offered a Save aimed at a form that is not on that screen and a pager
for an id absent from the list. The actions are gone from that branch; the
loading branch keeps them, which is the point of the convention.

The two flow baselines are regenerated: they were captured mid-series, before
the counter's width reservation moved off the button, and the visual gate could
not see the drift — the diff sits seven times under its ratio. The template
header's order now has a spec assertion; that route has no baseline at all.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 17:43:35 +07:00
Sergey KozyrenkoandClaude Opus 4.8 15319ef817 fix(ui): reserve the counter width on the label, not on the button
The reservation was computed as digits-of-total × 2 + 1 and applied to the
button, which is border-box: 18px of padding and border ate it, so the
counter still grew from 65 to 83px across a digit boundary and Previous
still slid 18px out from under the cursor — measured, both before and after.

It now reserves the width of the widest label the set can produce, on the
label itself, and the button holds 83px through every position. Building
that label instead of deriving its length also drops the arithmetic that
made the intent unreadable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 16:21:38 +07:00
Sergey KozyrenkoandClaude Opus 4.8 ee6ed49d48 fix(ui): keep the header actions in fixed slots instead of a pager prop
Reverts the `pager` slot on AppHeaderActions. The block is right-pinned, so
ordering alone gives the guarantee the slot was reaching for: put the controls
that come and go at the start of the children and everything after them keeps
its position. Flow header, right to left: actions menu, pager, favourite,
report — the report being the one that waits on the task list.

The controls that are always meaningful for a route are now always rendered
and disabled from an explicit loading flag rather than unmounted when the
entity object is falsy. Stepping used to collapse the whole cluster to a lone
star for the length of the fetch, and the pager — which needs the sibling list,
not the current entity — went with it, so a second step meant waiting.
Templates and knowledge get the same treatment; knowledge had no loading
signal at all, so one is threaded down from the page.

Two side effects of dropping the entity gates: on phones the flows row and the
favourite toggle survive an unloaded list (they were nested behind it), and the
position counter reserves the widest label its total can produce, so stepping
across a digit boundary no longer slides Previous out from under the cursor.

The pixel baselines cannot pin any of this — the cluster is far below the
visual project's diff ratio — so the order is asserted in the spec instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 16:05:36 +07:00
Sergey KozyrenkoandClaude Opus 4.8 5b4ded1e3c fix(flows): stop the pager bouncing the user back to the flows list
Stepping to a sibling flow landed on /flows. The detail page inferred "this
flow does not exist" from three absences — not loading, no flow, no error —
and an Apollo variables change satisfies all three: it reports networkStatus
setVariables, not loading, while the new flow's data is still undefined.
Instrumenting history showed both hops: replaceState to /flows/2861, then
straight back out to /flows.

The provider now publishes a positive isFlowMissing (the query settled with
no flow, or failed as a not-found) and the redirect reads only that, so
retuning the loading flag cannot silently break navigation again — which is
how this shipped. isLoading itself becomes "in flight with nothing to show",
which also keeps the Retry button on a failed load from ejecting the user.

Nothing in the suite pressed Prev or Next, so the new spec does: it samples
the DOM through a delayed fetch, proving the URL never passes through the
list and the pager stays mounted while the sibling loads.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 16:05:22 +07:00
Sergey KozyrenkoandClaude Opus 4.8 3d04fcfff2 test(e2e): fail on a colour written outside the variant set
The contrast gate mounts probes from cva output, so a colour written as a
raw utility in a page — the shape of the badge defect that shipped — was
outside it by construction. This walks the rendered DOM of every swept
route instead: each badge and button must draw its colour from the variant
set or from a semantic token.

It immediately found one, a hard-coded blue hover on the file manager's
expand-all control, waived by its exact node string until the design pass
takes it (the colour change moves baselined pixels).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 14:06:20 +07:00
Sergey KozyrenkoandClaude Opus 4.8 dfff92bef6 docs: correct two claims in the list/detail guide
The intro left /settings/prompts out of the list pages built on these
pieces and implied every one of them has a detail page; api-tokens edits
in place. The removed/renamed table pointed at stale mentions of the
deleted writer hook in two files that no longer contain any.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 14:06:09 +07:00
Sergey KozyrenkoandClaude Opus 4.8 ecb94b25e7 test(e2e): assert stand routes on data, not only on client-side state
Every assertion was client-side — the URL, a route-derived breadcrumb and
pageerror, which a production React build does not emit for a failed
query — so all five passed against a backend erroring on every request.
Verified against the live stand: healthy, 5/5 pass; with GraphQL forced to
error, the URL and breadcrumb assertions still pass and only the new one
fails. Each route now proves its query resolved, accepting the empty state
as well since a stand may hold no rows.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 14:06:09 +07:00
Sergey KozyrenkoandClaude Opus 4.8 04cc821bd6 test(e2e): stop the flows entries owning pages no route renders
Both flow entries claimed the whole src/pages/flows dir, which also holds
the report and create pages. A diff touching only the report page scoped
the run to two routes that cannot render it — the same ownership class
already fixed elsewhere in the manifest. Each entry now names its own
page file, so those two pages fall through to the conservative full run
and the detail/list diffs scope more precisely than before.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 14:06:09 +07:00
Sergey KozyrenkoandClaude Opus 4.8 d58f840702 test(e2e): probe every editor colour token, keyed off the stylesheet
Two of the five --editor-* tokens were probed and nothing made the list
rot-proof, unlike the badge matrix. The remaining three hang off element
selectors rather than classes, so the probe mount now takes a tag per
probe; the accent and code pairs clear AA in both themes. A new assertion
reads the token declarations out of the stylesheet, so a newly declared
token has to be probed or exempted.

The mount guard moved from "composites to transparent" to "still wears the
surface's colour", which every token rule overrides — the transparent form
could not cover an element-selector probe with no chip.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 14:05:55 +07:00
Sergey KozyrenkoandClaude Opus 4.8 908d35f8e4 test(e2e): make every dashboard period query serve its own dataset
Three of the four by-period queries returned one dataset regardless of the
period, so the period switch could only be asserted on the token chart —
the other three cards would have rendered identically had the app never
rewired them. Each now has week and month variants keyed on the period
variable, and the spec asserts the swap across all four cards plus the
execution breakdown, which carries no dates and so differs by flow.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 14:05:55 +07:00
Sergey KozyrenkoandClaude Opus 4.8 054f46b0ed test(e2e): gate the reconnect delta and give the dedup assertion a source
The post-reconnect flow(5) response was ungated, so any spec that fetched
flow 5 twice consumed it and rendered a message that never streamed. It now
serves only after a drop, behind a flag `dropAndReconnect` raises.

The no-duplicate assertion beside it had nothing that could produce a
duplicate: every id reached the page exactly once by construction. The
resubscribe now replays the id the refetch already delivered — the real
server behaviour the client dedups — followed by a sentinel that proves
the replay arrived rather than merely being awaited.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 14:05:42 +07:00
Sergey KozyrenkoandClaude Opus 4.8 f6bcff8672 test(e2e): scan the flow tab panels and the populated provider cards
The a11y sweep only ever saw each route's default view. Radix unmounts
inactive tab panels, so seven of the flow-detail tabs were never scanned,
and /settings/providers was scanned exclusively in its empty state — the
provider cards, where the badge-contrast defect lived, went unlooked at.

The manifest entry also claimed to own the file-manager, dashboard and
resources dirs while seeding empty collections, so the swept form of the
route rendered none of them. It now runs on the populated cassette, which
required the six per-flow stats queries the Dashboard tab issues.

Scanning the panels surfaced five real defects (unnamed progressbar,
unnamed icon buttons, under-size targets, screenshot-title contrast,
unfocusable scroll regions); each is waived by rule and node so the rest
of the panel still fails on anything new.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 13:41:35 +07:00
Sergey KozyrenkoandClaude Opus 4.8 ce1fc7049d test(e2e): cover the two detail routes whose editor loads server content
/settings/prompts/:promptId and /templates/:templateId had no test on any tier,
and they are where MarkdownEditorField loads content from the server — the
prompts list spec only expands a row into a <pre>, and the templates spec only
exercises create mode. The editor's one shipped crash reproduced solely in a
production build, which is exactly what the mock tier runs.

Each route now loads a non-trivial body (headings, list, fenced command, table,
and the {{.Var}} / {{PLACEHOLDER}} atoms the backend parses) and asserts both
halves: the raw view matches the loaded source byte-exact, and after an edit in
the rich editor every atom survives its serialization.

Also close the hole that let them stay uncovered: route builders are functions,
so the manifest's static path walk never saw them. Every builder must now
declare where it is covered or why it is not, and the check fails when a new one
appears undeclared.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 13:18:20 +07:00