ci: skip the opposite surface's single-surface tests

[iamwhatever]

Problem

Every PR runs the full CI matrix regardless of what it touches. A frontend-only diff runs all 8 backend shards plus 4 Windows shards; a backend-only diff runs the whole vitest suite. Most of those tests cannot be affected by the diff at all (#1556).

Why it matters

Wasted compute and slower feedback on every PR, plus noise that buries the checks that actually matter, and needless fork-approval toil.

Fix (symptoms → root cause → change)

Symptom: irrelevant tests run on single-surface PRs. Root cause: jobs have no notion of which surface a diff touches.

The obvious fix — gate a whole suite on changed paths — is unsafe in this repo, and that is the crux. A minority of tests are cross-surface parity guards: they live in one suite but assert against the other surface's source. Examples: backend test_redaction_mirror_parity.py reads website/src/utils/sanitize.ts (credential-redaction parity); website/electron/test/external-scheme.test.js reads src/kiro_crew/computer_use/permissions.py. Skipping one of those is exactly how a drift bug ships green, and an allowlist of "paths that make a suite skippable" has to be complete to be safe — it never is.

So the logic is inverted. scripts/ci-surface-tests.py enumerates only the files it can positively prove are single-surface; everything else — every parity guard, every file it cannot classify, every file added tomorrow — stays in the must-run set. A heuristic miss therefore costs CI time, never a skipped guard. It fails closed on an unreadable file, and every failure path in the workflow falls back to running the full suite.

A new changes job computes only_backend / only_frontend once. backend is a catch-all filter (['**', '!website/**', '!.github/**', '!scripts/**']), so every changed file lands in exactly one bucket and "only_X" literally means only X changed — a diff touching two buckets, or meta paths, leaves both flags false and the full matrix runs. On a single-surface diff the opposite suite narrows:

Diff	Effect
frontend-only	backend-test + backend-test-windows run 223 of 1033 files
backend-only	frontend-test runs 81 of 758 spec files
anything else	full matrix, unchanged

The catch-all requires dorny/paths-filter >= v4.0.3 for the some-with-excludes quantifier (v4.0.2 rejects the input and throws), so the pin moves v4.0.2 → v4.0.3 ceb8a2b8.

The scanned roots matter more than the classifier. An unenumerated root is worse than an unclassified file: the reduced run passes explicit paths, so a root that is never walked never runs at all, instead of falling back to "keep running". The selector therefore walks every root in setup.cfg testpaths (test, transfer, src/kiro_crew/apps/builtins) and every root in vitest's test.include (website/src, website/integration) — and the test suite asserts that, so adding a testpath or vitest include without adding it here fails loudly.

electron-test, frontend-lint, backend-lint and the sandbox job stay always-on: they are whole-project gates or hold cross-surface guards, and subsetting them buys little.

Coverage Gate keeps its fail-closed contract as an explicit invariant. A narrowed run collects no coverage (a subset's coverage is not comparable to the repo floor), so Coverage Combine is skipped on a frontend-only diff, and the gate asserts: backend-test and frontend-test must always succeed (only their scope narrows — backend-test is checked directly so a failed reduced run cannot hide behind a by-design skipped Combine); backend coverage is required unless the run was frontend-only (where Combine must be exactly skipped); empty surface flags — the detection job never reported — fail closed.

Tests

test/test_ci_surface_tests.py (new, 25 cases) pins the deny-by-default contract:

• 18 known parity guards can never be classified single-surface — and it asserts they exist, so a rename forces the list to be updated instead of silently becoming a green skip.
• Every setup.cfg testpath and every vitest include root is a scanned root (the two checks that catch the "root never walked" class).
• Both frontend escape styles are detected: '../../..' string literals and path.resolve(__dirname, '..', '..', '..') segment lists.
• An unreadable file is treated as cross-surface (fails closed).

Also verified locally: flake8 / isort / mypy clean, YAML valid, and the reduced 223-file selection collects cleanly under pytest.

Manual verification

N/A — CI workflow change; correctness is exercised by CI on this PR. This PR touches .github/** → meta → it runs the full matrix on itself, which is the conservative branch. The narrowing paths are covered by the unit tests above.

Known limitations

• The classifier reads each file's text, not its import graph, so an indirect cross-surface dependency (a test importing a helper that reads the other surface) is a structural residual risk. Both local reviewers probed for one and found no live instance; the always-run default means a miss costs CI time rather than a lost guard.
• The ci-full-run label forces the full matrix but takes effect on the next push — this workflow does not subscribe to the labeled event, because that would re-run everything on every bot label. Documented in-code at the FULL_RUN step.
• On the rare selector-fallback path (frontend-only diff and the selector errors), the backend shards run with coverage but Combine is skipped, so the 70% backend floor is not enforced for that run. Harmless — a frontend-only diff cannot move backend coverage — and the gate stays fail-closed.

Attribution

Supersedes #1560. Original authorship by @RohanK6 is preserved on the commit; the implementation was reworked from the original path-gating approach to the deny-by-default selector after path-allowlisting proved unsafe in review (fixes folded in as committer).

Closes #1556

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of f8bd1a5889f18a4902c9db3c4a5ea8824aa184fd and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] f8bd1a5

False positive or not applicable? A repository writer can comment:
/ai-review override gpt f8bd1a5889f18a4902c9db3c4a5ea8824aa184fd: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of f8bd1a5889f18a4902c9db3c4a5ea8824aa184fd — updated in place on each push; does not block merge.

Design-Verdict: PASS

Deny-by-default selection with fail-closed fallbacks inverts the unsafe allowlist approach correctly; residual risks are acknowledged, tested, and cost only CI time.

Suggestions

• docs/ci/ci-and-reviews.md's job table now misdescribes CI (no changes job, no narrowing, no conditional coverage-gate contract); update it so the documented matrix matches the shipped one.

[DESIGN-REVIEWED] f8bd1a5

[github-actions]

Opus 5 Review — ✅ no blocking findings

Reviewed f8bd1a5889f18a4902c9db3c4a5ea8824aa184fd — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] f8bd1a5

Verdict parsed from the review's SHA-scoped output markers for commit f8bd1a5889f18a4902c9db3c4a5ea8824aa184fd.

False positive or not applicable? A repository writer can comment:
/ai-review override fable f8bd1a5889f18a4902c9db3c4a5ea8824aa184fd: <one-sentence reason>

[iamwhatever]

Prior reviewed SHA 73e95c88b → now 427af11a2. Local GPT + Opus mirror findings addressed:

• fixed — pull-requests: read (High): dorny/paths-filter uses the REST API on pull_request events; added permissions: {contents: read, pull-requests: read} scoped to the changes job (job-level replaces top-level, so both restated).
• fixed — E2E gating (High): e2e now runs on run_backend == 'true' || run_frontend == 'true' — it exercises a backend persistence invariant via setup.py test_e2e, so a backend/test/**-only change no longer skips it.
• fixed — setup.py (High): added to the backend filter.
• fixed — transfer/** (Medium): added (it is a setup.cfg testpaths root).
• fixed — config-baseline.json / error-code-baseline.json (Medium): added to the backend filter.
• accepted (documented, not code-changed) — ci-full-run needs types: [labeled] to fire at apply-time (Medium): intentionally NOT added — subscribing to labeled would re-run the full matrix on every bot label (readiness/fork), defeating the PR's compute saving. The hatch works on the next push; the FULL_RUN step comment now documents this.

[iamwhatever]

Prior reviewed SHA 427af11a2 → now caee5b9e3. Opus blocking finding (backend filter omitted repo-root / cross-surface inputs the backend suite asserts on):

• fixed — added to the backend filter: website/** (~45 backend guard tests under test/ read website/, incl. test_redaction_mirror_parity.py:59 → website/src/utils/sanitize.ts credential-redaction parity and test_dashboard_security_headers.py), plus docker/** (test_docker_entrypoint.py:41), packaging/**, cli.sh, install.sh (installer/CLI contract tests).
• rebutted → added as inert precaution — conftest.py: there is no root conftest.py (only test/conftest.py, already covered by test/**), and the cited test/test_host_service_guard.py does not exist; added the pattern anyway as a harmless guard for a future root conftest.
• trade-off, follow-up filed — including website/** in backend means frontend-only PRs now also run the backend suite. A follow-up will split the ~45 cross-surface guards into a dedicated always-on job so frontend-only PRs can skip the full backend suite again without losing the guards.

[iamwhatever]

Rebased onto current main (was 19 commits behind). The previous Automated Rule Check failure was a stale-base artifact: it diffed against an advanced base and mis-attributed main's now-removed claude-haiku-4.5 defaults in src/kiro_crew/** to this PR, which only touches .github/workflows/ci.yml. The rebase resolves it; the workflow content is unchanged from the prior revision.

[iamwhatever]

Reworked to the A′ shape (prior SHA caee5b9e → now 42ac0b32): backend jobs are now ungated / always run (the backend suite is a repo-wide guard suite, so a backend path-allowlist was inherently leaky — this closes that entire finding class), and only frontend-lint/frontend-test/electron-test gate on run_frontend.

• fixed (Opus BLOCKING, frontend cross-surface leak): frontend jobs contain guards that read backend paths, so a backend-only change would skip them. Added the exact reads to the frontend filter: src/kiro_crew/apps/builtins/**, src/kiro_crew/connections/registry.json, skills/feature-request/SKILL.md, test/fixtures/theme_css_corpus.json.
• Coverage Gate simplified: backend coverage always required; frontend required-or-exactly-skipped by run_frontend; empty flag fails closed.
• The earlier Automated Rule Check failure was stale-base drift (mis-attributed main's model-defaults) — resolved by rebasing onto current main.
• A follow-up will add a dedicated always-on cross-surface job so the backend suite can also be gated without the enumerated frontend paths being load-bearing.

[iamwhatever]

Synced onto current main (8bf09b3; was 7 behind). Note #1013 also edited ci.yml — the rebase auto-merged cleanly and I re-validated my additions survived intact: the changes job (permissions, only_backend/only_frontend outputs, paths-filter v4.0.3 + some-with-excludes), the scope steps on backend-test/backend-test-windows/frontend-test, and the Coverage Gate invariant. 25 selector tests + flake8 green locally. The prior base failures (sandbox fixture, ja catalog parity) are fixed on this base. Fresh CI running.

[iamwhatever]

BLOCKING — ci.yml reduced runs bypass the test-suite exclusion — FIXED (prev reviewed 8bf09b30e → f8bd1a588).

Confirmed reachable exactly as described: scripts/ci-surface-tests.py --surface backend legitimately emits .../auto_improvement/tests/test_dogfood_learnings.py in the cross-surface must-run list, and the reduced Linux/Windows commands ran pytest … "${TARGETS[@]}" with none of the 11 --deselect flags the full commands carry — so on a frontend-only diff that POSIX/git-dependent file would run and fail the shards.

Root cause is drift: the deselect list was duplicated across the full backend + full Windows commands, and the two reduced commands silently diverged. Rather than add a 3rd/4th copy that can drift again, I hoisted the list to one workflow-level env: BACKEND_DESELECTS (folded to a single space-separated line) and reference it from all four invocations (full + reduced, Linux + Windows).

Safety verified locally: (1) a --deselect of a path a given run did not collect is a harmless no-op (pytest rc=0, full set still collected), so the reduced runs carry the full list safely; (2) the unquoted expansion word-splits into correct separate args (no deselect path contains a space); (3) full-suite behavior is byte-identical (same 11 flags); (4) the namespace-sandbox job that deliberately runs these files is untouched. YAML valid, 25/25 selector tests + flake8 green.