ci: test the frontend on every labelled PR - #315
Open
FrameAutomata wants to merge 7 commits into
Open
Conversation
FrameAutomata
added
the
ci
`npm run lint` is `prettier --check . && eslint .`, and the prettier half fails on `main`. This is `npm run format` and nothing else -- reflow only, no token changes -- kept as its own commit so it does not bury the eslint fixes that follow. Refs #316 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`npm run lint` is `prettier --check . && eslint .`; the previous commit fixed the prettier half, this fixes the eslint half. Scope note: `paths.base` is unset in svelte.config.js, so `resolve()` is identity today and none of this changes behaviour. It is NOT a claim that the app is base-path correct -- ~30 sites the rule cannot see (`redirect()` in load functions, `window.location.href` in api.ts, `window.open` in the row click handlers) still emit unresolved absolute paths. This clears the flagged sites so the gate can go green; base-path support is a separate job. ## svelte/prefer-svelte-reactivity (3, all in overview-tab) All three were false positives, not the latent staleness bugs #316 suspected. The rule does no escape or scope analysis: it flags any `new Map()` / `new URLSearchParams()` on which `set`/`delete`/`clear` is called anywhere in a `.svelte` file. All three instances are function-local scratch -- two inside `$derived.by` callbacks that return arrays, one inside a pure function returning a string -- so a reactive SvelteMap would only add proxying to paths that re-run on every search keystroke. Two sites drop the mutation instead, which is both simpler and satisfies the rule honestly: `projectOptions` builds its Map from entries, and `instanceHref` folds its conditional search param into the constructor. The third genuinely needs get-or-create and carries a disable with the reason. ## svelte/no-navigation-without-resolve (7) Four were real, and each needed a different shape: - root + connection pages: `goto('/setup')` is a static route, so it just takes `resolve()`. - dashboards `setServerScope`: was re-emitting `url.pathname` read back from `window.location`, which already carries the base path -- wrapping that in `resolve()` would prepend it twice. This function only ever runs on /dashboards, so it resolves the route id instead. The `let` split is forced: the rule follows an identifier to its declarator but not through a `+` chain. - finish-setup: `returnTo` is a concrete path from the SSO handoff, and there is no safe `resolve()` form for it -- `resolve()` substitutes `[params]`, and `resolve_route('/a[b]c', undefined)` throws `TypeError: Cannot read properties of undefined (reading 'b')` (verified against the installed kit 2.49.2). So it prefixes `base` directly and carries a disable explaining why. Note `gotoHref`/`resolveHref` in utils/navigation and utils/links have this same latent crash, which already reaches login/+page.svelte:94 and auth/callback/+page.svelte:45; that is pre-existing and left for its own change. The three `href` errors in overview-tab were false positives -- both helpers already resolved internally, but the rule only recognises a literal `resolve()` at the link site. These now use `resolveHref` from utils/links behind the `{...{ href }}` spread, the idiom six other `<a>` elements already use for a helper-built href, with the helpers returning unresolved paths so resolution happens exactly once. A plain `href={instanceHref(server)}` still errors, so this does not quietly widen what the rule accepts. Closes #316 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things this repo's own drift is traceable to. `npm run lint` was missing from the Development Commands table, which lists dev/build/check only. An agent following CLAUDE.md runs `npm run check` -- which passes on unformatted, lint-failing code -- and never learns the gate exists. That is how 11 unformatted files and 10 eslint errors accumulated. The "Adding a New Frontend Page" template taught `catch (e: any)`, which trips @typescript-eslint/no-explicit-any; #314 introduced one by following it. No page actually does this -- it is `catch (e)` 93 times and `catch (e: unknown)` 31 times, reading status and message through getErrorStatus/getErrorMessage from $lib/utils/errors. The template now shows that, matching e.g. routes/tasks/[task]/+page.svelte. Refs #316 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No workflow covered frontend/** . The only place the SPA was built was release-traceway.yml, during a release -- so a broken build surfaced where it blocks shipping instead of failing a PR. Runs check (svelte-check), test (vitest) and build, all of which pass on main today, so the gate is honest from the first run. Follows the conventions in backend.yml: label-gated on PRs, unconditional on push to main, workflow_dispatch, and the Node version read from frontend/package.json engines.node -- the same field flake.nix reads, so CI and `nix develop` cannot drift. One job rather than backend.yml's three, because these steps share a single node_modules and npm ci dominates the run. `npm run lint` is deliberately excluded and the reason is recorded in the file: it fails on pre-existing code, and the eslint fixes change routing and reactivity behaviour, so they need their own PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the step this workflow was written without, and deletes the comment block explaining its absence. The gate could not go green before the errors it checks were fixed, so this branch is now stacked on fix/316-frontend-lint (#316) rather than main -- without it `npm run lint` exits 1 here on 11 unformatted files and 10 eslint errors. GitHub retargets this to main once #316 merges. Lint runs after build rather than before: a lint failure is cheap to fix locally, a broken build is the signal worth surfacing first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FrameAutomata
force-pushed
the
ci/frontend-test-workflow
branch
from
August 27, 2026 15:52
1c26068 to
9f5a4ad
Compare
Collaborator
Author
|
Rebased onto Retargeted because the gate cannot precede the fixes: on the previous base GitHub retargets this to Lint runs after build rather than before: a lint failure is cheap to fix locally, a broken build is the signal worth surfacing first. |
FrameAutomata
added
ci
The job gained a Lint step but kept the name "Typecheck, test and build", which is what the check shows up as on a PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FrameAutomata
added
ci
checkout@v4 and setup-node@v4 both declare runs.using: node20, which the runner now force-migrates to Node 24 with a deprecation annotation. No point landing a new workflow already carrying it. The rest of the repo has the same problem on 12 actions across all 16 workflow files -- tracked in #326, which this does not attempt to fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FrameAutomata
added
ci
Collaborator
|
We should make a PR against main, stacked PRs are almost unmergable :/ |
Collaborator
Author
|
Retargeted to That makes this one PR rather than two: the lint fixes (11 prettier files, 10 eslint errors) plus the workflow that keeps them fixed. They belong together — the fixes have no independent purpose, they exist so the gate can exist. Verified locally in
One correction to my earlier comment here: I said this PR pins Generated by Claude Code |
This was referenced Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
No workflow covers
frontend/**. The only place the SPA gets built today isrelease-traceway.yml, during a release — so a broken frontend build surfaces where it blocks shipping, rather than failing a PR. That's the same argument that justifiedbackend.yml(#306).What it runs
maintodaynpm run check(svelte-check)npm run test(vitest)npm run buildAll three pass right now, so the gate is honest from the first run rather than starting red.
Conventions followed from
backend.ymlpull_request: types: [labeled]+if: ... label.name == 'ci'), unconditional on push tomain, plusworkflow_dispatchfrontend/package.jsonengines.node— the same fieldflake.nixreads (nodeMajoratflake.nix:28), so CI andnix developcan't drift, mirroringgo-version-file: backend/go.modnode_modulesandnpm cidominates the run, so splitting would pay the install cost three timesWhy
lintis excludednpm run lintisprettier --check . && eslint ., and it fails on pre-existing code:svelte/no-navigation-without-resolve, 3 ×svelte/prefer-svelte-reactivity(mutableMap/URLSearchParamsinorganization/overview-tab.svelte)Those aren't style nits — the navigation and reactivity fixes change behaviour, and the
prefer-svelte-reactivityones look like latent Svelte 5 bugs. That belongs in its own reviewed PR, not folded into CI plumbing. The reason is recorded as a comment in the workflow so nobody adds the step and gets a red gate. Filed separately as the tracking issue linked below.Verification
check,testandbuildwere each run locally in thenix develop .#frontendshell with the exact commands above. The YAML was parsed and its job structure asserted before commit. Applying thecilabel to this PR exercises the workflow itself.🤖 Generated with Claude Code