DB workflow rationale
This history was moved out of .github/workflows/db.yml when the workflow
reached GitHub’s 500 KiB workflow-file limit. It remains the rationale for
the load-bearing filter, gate-integrity, and derived-coverage controls.
Path-filter strategy (issue #720 regression + #654 original problem):
Branch protection requires "Migrate Up + Down (PostgreSQL 16 + pgvector)"
and "Schema drift check" on every PR. We previously used a paths: filter
- companion
db-skip.ymlwithpaths-ignore:to cover both cases. That design fails on zero-file-diff PRs (e.g. #720 after its fix independently landed on main): neitherpaths:norpaths-ignore:matches when the diff is empty, so neither workflow fires and the required check stays "expected" forever.
Fix: mirror the security-scan.yml pattern. Trigger on every PR with
no paths: filter. An internal changes job uses dorny/paths-filter
to detect whether DB work is needed; heavy steps gate on the output.
ALLOW-LIST → DENY-LIST for the db filter (issue #2465):
Until #2465 the db filter was a hand-maintained ALLOW-list of ~20 globs.
A path-filtered step SKIPs rather than fails, and this job is a REQUIRED
status check, so every path outside that list produced a load-bearing green
from a job that stood up no database and ran no integration suite.
The list's own inline comments memorialised THREE separate escapes, each appended retroactively after someone noticed: internal/workflow/** (WF-28/#1906), cmd/agent-orchestrator/** + internal/temporalwf/** (#1881), and the four-package memory-recall set (#2296, whose defect "survived three PRs and a full dark deploy precisely because nothing executed it"). Three recorded escapes from one filter is not a maintenance lapse; enumeration is the wrong instrument.
It was also wrong on its own terms, and by a wide margin. This job runs ten
go test invocations (see the steps below). Their first-party dependency
closure — go list -tags integration -deps -test over all ten package
globs — is 159 of the module's 289 packages, spanning 38 internal|cmd|pkg| test trees. The allow-list named 15. Uncovered by it and covered now:
internal/gateway/** — incl. auth/bypass.go, the app.org_id GUC replant
every RLS assertion in this lane depends on
internal/rbac/, internal/auth/, internal/governance/,
internal/task/, internal/agent/, internal/dashboard/,
internal/metrics/, internal/knowledge/, internal/mcp/,
internal/bus/, internal/testsupport/, pkg/*pb/, and 20 more —
and internal/context/store/** ITSELF: only its migrations/
subdirectory was listed, never the Go SQL it migrates for.
Also absent: Makefile (which defines the migrate-up/migrate-down targets
the job invokes) and hack/verify-app-rw.sh.
So the filter is now a DENY-list: '**' plus explicit negations, evaluated
with predicate-quantifier: 'every'. New packages, new SQL, new migrations
and new integration suites are covered the day they land, instead of being
skip-green until somebody remembers. Each exclusion carries its reason.
Same idiom as wave2-smoke.yml (#2449) so the two required checks share
one auditable shape.
MEASURED COST — this widening is NOT free, unlike #2449's (2026-08-03, 150 merged PRs replayed through dorny v3's exact matcher; 120 sampled job runs for durations): trigger rate 68.7% → 93.3% of PRs (37 PRs / 24.7% newly gated) job duration 550s median when it runs, 23s when it skips runner-minutes 642 → 858 per 100 PR runs (+216, +34%) required-check critical path, repo-wide median: UNCHANGED at 550s — this job already dominated on the >50% of PRs where it ran. required-check critical path on the 37 newly-gated PRs: 262s → 550s median (+288s). Zero of the 37 had another required check at or above 550s, so every one of them gets longer. That is the price of them being gated at all; do not buy it back by narrowing.
Widening this filter also strengthens #2449/PR #2464's cost claim rather than weakening it: that PR's "critical path unchanged because Migrate Up + Down already dominates" now holds on 93% of PRs instead of 69%.
The drift filter below is deliberately left as an allow-list: Schema drift check is not a required status check, and its input set really is
small and derivable (the tool walks migration text plus the Go registry,
with no database). Two justified shapes beat one forced one.
GATE-JOB FAILURE MUST NOT SKIP-GREEN THIS CHECK (issue #2473):
#2465 (above) made the db FILTER honest. It did nothing about the changes
JOB, which was still a single point of failure: needs: changes alone means
that when changes FAILS — rather than reporting "nothing DB-relevant" —
GitHub never starts migrations, so it reports skipped, and a skipped
required check SATISFIES branch protection. Every step-level if: and every
anti-skip NEEDLE guard in this file is downstream of that and cannot see it:
the failure is one level up, at the job, so none of those steps exist to run.
Observed on 2026-07-31 (#2389, org Actions quota exhausted): every gate job
died at start, all six required checks reported skipped, and PRs read
mergeable with zero CI executed. The deny-list conversion did not help,
because the filter never got to be evaluated.
Fix on migrations below, a pair that must be read together:
1. if: ${{ always() }} at job level so it STARTS whatever changes
concluded; the job can then never report skipped.
2. A Gate integrity FIRST step failing closed unless every upstream job
reached success AND changes.db is a literal 'true'/'false'.
Without (2), (1) is worse than the original bug: a failed gate leaves
needs.changes.outputs.db EMPTY, all ~30 gated steps below evaluate
false, and this check reports green having stood up no database — the
precise outcome #2465 was filed to end.
always() not !cancelled(): the latter SKIPS on cancelled runs, and
skipped satisfies branch protection — the same defect, narrower.
This composes with, and does not replace, the Guard — the db path filter must stay a deny-list step in the changes job. That guard pins WHAT the
filter decides; the pair above pins that a decision was made AT ALL. Note
the ordering dependency the guard alone has: it lives in changes, which is
not a required context, so if someone removed if: always() here the guard
could go red without blocking anything. That is why the invariant is ALSO
asserted statically by scripts/check-required-check-gating.py.
schema-drift is deliberately NOT converted: Schema drift check is not a
required status check, so its skipping is a cost matter, not a branch
protection bypass. If it is ever promoted, apply the pair first — exactly as
the note on the drift allow-list above already says for the filter shape.
THE SECOND ALLOW-LIST IS ALSO GONE — -run (issues #2848, #2904):
The db path filter above was one enumeration. Run integration tests
carried another: a hand-maintained go test -run alternation, 31 entries,
over test/integration/context. It failed the same way — a name absent from
it did not run, and the REQUIRED check reported success anyway, with no skip,
no warning and no count.
What it concealed, all found by inspection rather than by any lane:
* 13 tests that had never executed in CI, 8 of them broken (#2849);
* TestActivityFeedV1_ClearanceMatrix, whose B_L5_noLeakFromA case
describes the exact cross-tenant read activity_feed_v1 shipped for
144 migrations (#2846);
* a whole family (#2840's) registered so late that nothing in it had ever
run on a board that was green for it;
* TestSummaryLoader_KeepsNewestN_Chronological, which did not merely
fail but PANICKED, truncating the run — repaired by #2903 (T1).
It was also the ONE line in this file that git merge conflicts on, so each
union-resolution was an opportunity to delete another PR's coverage silently.
And it bought nothing. Measured on the CI recipe (fresh pgvector:pg16, chain
222, post-T1): filtered 338 === RUN / 69.5s, unfiltered 345 / 68.8s. The
"unfiltered is slower" belief came from a run a panic had truncated.
Replacement, same instrument as the deny-list conversion — derive, do not
enumerate: test/lint/db_lane_coverage_test.go asserts that every
func Test* in that package, under BOTH build-tag states, executes in at
least one workflow invocation. It parses this file for the invocations and
AST-walks the package for the population, so it cannot agree with CI by
construction. Re-adding a -run here is not banned; it turns that check RED,
by name, for every test the filter would newly exclude.
STILL OPEN and deliberately NOT closed here: 27 tests in that package sit
behind //go:build integration and are invocable in NO lane (#2827). They
pass when run alone; they cannot yet be composed into this lane because
migration 126's down is non-total once mcp_servers holds a soft-deleted
row whose only binding is also soft-deleted (#2952). Quarantined BY NAME in
that check, which fails if the set grows or shrinks.
Refs: issue #654 (original path-filter problem), issue #720 (zero-file
diff regression), issue #2465 (allow-list skip-as-pass, this fix),
issue #2449 / PR #2464 (same defect class on Compose Smoke Test),
issue #2473 (gate-job failure skip-green — the always() + Gate integrity
pair), #2389 (the Actions quota incident that made it observed),
#2848 / #2904 (the -run allow-list deletion + derived lane-coverage check),
#2827 / #2849 / #2872 (the classes it closes or hands on).