ci: gate web/frontend PRs on tsc --noEmit + vite build - #661
Conversation
The CI workflow covers only the Python side; a frontend PR's type errors shipped to main undetected (PR #658 NB-1 -- a types.ts field-name mismatch made a table column render empty and a button never render, caught only by manual tsc/build runs). New path-filtered workflow runs npm ci, npx tsc --noEmit and npm run build in web/frontend on changes under web/. Separate file because paths: filters are trigger-level; safe because the main ruleset has no required status checks, so a skipped run cannot block a merge. No vitest step on purpose: the suite is empty and vitest run exits 1 on "No test files found", so it would fail every frontend PR rather than pass vacuously. Add npm test here when the first test lands.
padak
left a comment
There was a problem hiding this comment.
Review of #661 — ci: gate web/frontend PRs on tsc --noEmit + vite build
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This PR adds .github/workflows/frontend.yml, a new GitHub Actions workflow that runs npx tsc --noEmit and npm run build in web/frontend on any PR/push touching web/**, closing a real gap surfaced by PR #658 (a types.ts field-name mismatch that shipped undetected because CI only exercises the Python side). The change is infrastructure-only (no src/, no CLI command, no plugin-sync surface touched), so most of the command-checklist playbook (3-layer compliance, OPERATION_REGISTRY, AGENT_CONTEXT, gotchas.md, etc.) is not applicable here. Verdict: APPROVE — the workflow is well-scoped, the path-filtering rationale is independently verified against the live branch-ruleset API, the new job already ran green on this very PR, and the deliberate omission of a vitest step is well-justified (an empty suite fails outright rather than passing vacuously).
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 0
- Nits: 1
Blocking findings
(none)
Non-blocking findings
(none)
Nits
[NIT-1].github/workflows/frontend.yml:29and:34—paths: ["web/**", ...]also matchesweb/backend/**-only changes, which will schedule this frontend runner even though the job never touchesweb/backend. Given the PR explicitly scopesweb/backendas future/out-of-scope work, narrowing toweb/frontend/**(plus the workflow file itself) would avoid a wasted run on backend-only PRs until an analogous backend job is added.
Verification log
gh pr view 661 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state→ 1 file changed (.github/workflows/frontend.yml, +68/-0), state OPEN, conventional prefixci:matches a CI-config-only change ✓git rev-parse --abbrev-ref HEAD→ci/frontend-typecheck-build, matches<branch>✓ (no checkout needed)- Read
CONTRIBUTING.md(Checklist, Plugin synchronization map, Releasing a new version) andCLAUDE.mdconvention #17 — confirmed not applicable: no CLI command added/removed/renamed, noservices//commands//client/touched, nopermissions.pysurface affected. - Read
plugins/kbagent/agents/keboola-expert.md§1 — no CLI behavior changes to cross-check. gh api repos/keboola/cli/rules/branches/main→ rule types aredeletion,non_fast_forward,pull_request(review-count based) only; norequired_status_checksrule present — independently confirms the PR description's core safety claim that a skipped/path-filtered workflow run can never leave a required check pending ✓grep -n "paths" .github/workflows/*.yml→frontend.ymlis the only workflow usingpaths:filtering, confirming the PR's claim "no existing workflow in this repo usespaths:filtering" ✓grep -n "engines" web/backend/package.json→"node": ">=20";grep -n "node-version" .github/workflows/ci.yml(build-windows job) →"20"— confirms the comment's claim that Node 20 matches both floors ✓find web/frontend/src -iname "*.test.*" -o -iname "*.spec.*"→ empty — confirms "zero*.test.*/*.spec.*files" claim, justifying the deliberate omission of a vitest step ✓cat web/backend/package.json→ has its own"build": "tsc -p tsconfig.json"script but no CI job — confirms the "Out of scope: web/backend" statement is accurate (not silently glossed over) ✓grep -n "checkout@\|setup-node@" .github/workflows/*.yml→actions/checkout@v5andactions/setup-node@v6are the versions already used consistently acrossci.yml,release.yml,release-kbagent.yml,e2e.yml— new workflow matches repo convention, no stray pin ✓cache-dependency-path: web/frontend/package-lock.jsonreviewed againstdefaults.run.working-directory: web/frontend— correct:with:inputs toactions/setup-nodeare resolved relative to repo root regardless of the job'sdefaults.run.working-directory(which only affectsrun:steps), so the repo-root-relative path is correct, not a copy-paste bug ✓gh pr checks 661→ live reproduction:Type check + build (web/frontend)→pass, 33s, alongsidecheck,test (3.12),test (3.13), Windows wheel build, and Devin Review — all green on this exact commit, directly confirming the PR's own "Verification" section claim that this PR triggers its own workflow ✓gh api repos/keboola/cli/issues/661/comments→ no additional review comments beyond the check-status entries (Devin Review passed with no flagged findings) ✓make checkwas not re-run locally: the PR touches no Python source, and CI'scheck/test (3.12)/test (3.13)jobs already ran green on this exact commit (seegh pr checksabove), which is a stronger signal than a redundant local run against unrelatedHEADstate.
Open questions for the author
(none)
* chore(release): 0.90.0 Bumps pyproject.toml to 0.90.0 and adds the changelog entry covering every PR merged since v0.89.0 (#658, #662, #661, #663, #665, #666, #664, #668, #667, #623), resolves the vNEXT placeholders those PRs left behind, and adds the curated What's new reel for the release. * docs(web-server): keep the What's-new anchor stable across releases The '### What's-new popup *(since vNEXT)*' heading put the version gate in the heading itself, so resolving the placeholder to 0.90.0 changed the generated slug to 'whats-new-popup-since-0900' and broke the in-page link at line 138 -- and would have broken it again on every future release. Moved the '(since 0.90.0)' tag to the first body line: the anchor is now the stable 'whats-new-popup', the gate stays visible, and check_version_gates.py still sees it (it scans the whole file, not just headings).
Why
Follow-up to PR #658 non-blocking finding NB-1, deferred there so the CI change gets its own review instead of riding a large feature PR.
.github/workflows/ci.ymlcovers only the Python side. A frontend PR's type errors ship tomainundetected — #658 carried three commits' worth of frontend bugs caught only by manualtsc/build runs and a live browser pass, including atypes.tsfield-name mismatch (configIdvs the Queue API's actualconfig) that made a whole table column render empty and a new button never render.What
New
.github/workflows/frontend.ymlthat runs inweb/frontend:npm cinpx tsc --noEmitnpm run build(tsc -b && vite build— the same commandhatch_build.pyruns when bundling the SPA into the wheel)Path gating: no existing workflow in this repo uses
paths:filtering, so this introduces the scheme: triggers only on changes underweb/**(plus the workflow file itself), so Python-only PRs never schedule a runner. It is a separate workflow file becausepaths:filters are trigger-level, not job-level. This is safe because themainruleset has norequired_status_checksrule (verified viagh api repos/keboola/cli/rules/branches/main— the merge gate is the human review), so a skipped run can never leave a required check pending. If frontend checks ever become required, the filter needs rethinking (in-job change detection, or apaths-ignoremirror that reports success).Deliberately NO vitest step — maintainer decision requested
There are zero
*.test.*/*.spec.*files underweb/frontend/src. Notably,npx vitest runon the empty suite does not pass vacuously — it exits 1 withNo test files found, so adding it now would fail every frontend PR. Options:npm testwhen the first test lands — vitest's fail-on-empty default then also guards against the suite silently vanishing;vitest run --passWithNoTestsnow — rejected here because it would pass vacuously and hide that nothing is tested.Say the word if you prefer the other option.
Out of scope
web/backend(dev-mode BFF, TypeScript too) has no type-check gate either; it is not part of the wheel build. Can be added to this workflow later with an analogous job.Verification
npm ci,npx tsc --noEmit,npm run buildall pass inweb/frontend.paths:filter), so the run visible on this PR is the live verification.