Repository navigation
fix(ci): route the code the kernel suite guards to the lane that runs it - #684
Merged
Merged
Conversation
`src/Ashlar.Tests.Kernel`'s 412 SPEC-006 admission and signing cases run only in
the readiness gate's native lanes, and those lanes are diff-conditional. Both
path lists contained `src/Ashlar.Tests.Kernel/**` but none of the code that suite
tests. So editing the tests re-ran them, while editing `GateStore.cs` or
`AdmissionGate.cs` — the code they guard — skipped them. The trigger was
inverted.
Adds the kernel project's transitive ProjectReference closure that no entry
covered, to BOTH lists at the same position in the same order:
src/Ashlar.Manifest/**
src/Ashlar.Policies.Dev/**
src/Ashlar.Certification.Contracts/**
src/Ashlar.Analyzers/**
`src/Ashlar.Policies/**` does not cover `src/Ashlar.Policies.Dev/` — the
rewritten pattern's `/` after `Policies` must consume a `/`, and the candidate
has `.Dev` there. Verified under both matchers.
`Ashlar.Analyzers` is referenced by `Ashlar.Core.Application` with
`OutputItemType="Analyzer"` and `ReferenceOutputAssembly="false"`. It is a
build-time analyzer, not code the 412 cases exercise; it is here because breaking
it breaks the kernel suite's BUILD.
Both lists are machine-enforced to be identical including ordering
(`AiPipelineCiRoutingConventionTests:49-61`, inside required cert-gate), so the
patch asserts that itself rather than trusting the edit, along with the test's
supported-glob shape, its uniqueness requirement, and its negative contract that
no glob may match `README.md`, `docs/unrelated-guide.md` or
`src/Unrelated/Nested/Other.cs`.
Cost is zero, measured rather than asserted. Across the last 40 merged PRs
(#683..#631), 20 already set `run_heavy=true` and these four entries flip
**none** of the other 20. Only three of the 40 touched these directories at all
(#645, #662 in `Manifest/Admission/`, #640 in `Policies.Dev/`) and all three
already ran heavy via `src/Ashlar.Tests.Infrastructure/**`.
Three records asserted the absence this commit removes, two of them written
yesterday in #683. Leaving them would reproduce the defect they describe:
- `ci/test-ownership.tsv` row 58 said Manifest and Policies.Dev were "in
neither"; now states the routing and that the wider 41-directory absent set
across all 22 discovered suites is still open.
- `docs/specs/SPEC-006-keys-and-signing.md` said a change confined to the code
under test does not re-run the tests. The remaining caveat — a skipped lane
counts as a pass, and the sweep emits no per-project count — is unchanged and
still true.
- `ci/test-ownership.tsv` row 47 stays literally true: `src/Ashlar.Analyzers/**`
does not match `src/Ashlar.Analyzers.Tests/`. Annotated so it does not read as
false.
- `docs/CiGateInventory.md` line 112's core-path prose, which nothing
machine-reads.
This does not fix the other half. `ValidationServiceAdapter.cs:248` still
computes `passed` from failures alone, so a project selecting zero tests passes,
and `readiness-summary` still counts a skipped lane as a pass. Those are separate
changes with real blast radius, and they are not bundled here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 30, 2026
IanFrelinger
pushed a commit
that referenced
this pull request
Sep 30, 2026
…and gate it The native readiness lanes run `ashlar ci verify`, which runs `ashlar validate`, which builds and runs every test project its recursive sweep discovers (22 on this tree). Those lanes are diff-conditional: they run only when a PR touches a glob in READINESS_PATHS. The transitive ProjectReference closure of the 22 suites is 70 projects, and 37 of their directories matched no glob. So editing src/Ashlar.Mcp.Server (built into six suites) skipped every lane that runs them, while editing their tests re-ran them. #684 closed this for the kernel suite alone. The same hole existed one level out: src/Ashlar.Compat is compiled into most of the closure through src/Directory.Build.props and Directory.Build.targets, three nested Directory.Build.props and the root .editorconfig are read implicitly, and the CLI embeds samples/templates/brick while Tests.Infrastructure copies samples/physical-atom-cert. None of those re-ran readiness either. What this does: - Adds those 44 entries to BOTH readiness lists (on.push.paths and READINESS_PATHS), at the same positions and in the same order: the 37 project directories as dir/** after the kernel test entry, and the 7 implicit and cross-directory inputs after the shared root build inputs. - Adds ReadinessClosureConventionTests (cert-gate). It calls validate's own discovery predicate (under a virtual root, because the predicate also reads the absolute path for "test"), walks ProjectReference with an XML parser so attribute order cannot hide an edge, follows analyzer-only (OutputItemType=Analyzer, ReferenceOutputAssembly=false) and conditional references on purpose, and fails naming the uncovered path, what names it, the list and a suite that needs it. - Reads references from every shared MSBuild file a closure project is evaluated from, not only from the project file: the implicit Directory.Build.props/.targets and Directory.Packages.props above it, and any file it reaches by an explicit <Import>, transitively. A ProjectReference declared in src/Directory.Build.props is a real edge of every src/ project, and a first version of this test dropped it silently: a verifier added a Condition'd reference to src/Ashlar.Lite there for the kernel suite and the gate stayed green. Such a reference is now an edge of every closure project that reads the file (conditions are not evaluated, the same over-approximation as for conditional references). A reference, or an item path with a directory, in a shared file that is not anchored by $(MSBuildThisFileDirectory) is refused, because MSBuild resolves it against each importing project and it names no single file; so is a computed or wildcard Import. - Also checks every implicit Directory.Build.*/Directory.Packages.props/.editorconfig/ .globalconfig/nuget.config/global.json above a closure project, and every path outside the project's own directory that a closure project or one of those shared MSBuild files names: in the Include of an item of ANY type except the identity types (PackageReference, PackageVersion, FrameworkReference, InternalsVisibleTo, Using and the like, and ProjectReference, which the walk covers) and a None item that is not copied, in any item's HintPath, or in an Import. The item types are a denylist on purpose. A second version scanned an allowlist of six types (Compile, EmbeddedResource, Content, AdditionalFiles, Analyzer, None), and a verifier showed the item type alone decided the verdict: a cross-directory <Protobuf> item, a Reference HintPath and an EditorConfigFiles item in src/Ashlar.Transport.Grpc (which already compiles a Protobuf item) left it green, while a Compile item at the same spot turned it red. Moving that project's .proto to a shared protos/ directory would have kept cert-gate green while later edits to the contract skipped the lanes that build its suites. Now a type nobody listed is read as naming files, and a computed path on one fails instead of being skipped. A bare file name in a shared file names a file in each importer's own directory, which that project's glob covers; a path under $(NuGetPackageRoot) is in the package cache, outside the repository. $(Property) paths and wildcard/absolute/escaping/missing references fail rather than being approximated. Floors are stated constants (20 suites, 60 projects, 6 implicit, 3 cross-directory; measured 22/70/10/6, unchanged by the denylist). - Adds its row to ci/cert-gate-assertions.md (after the runner-column row, so the "line 34" citation in TestOwnershipRunnerConventionTests stays true), updates the readiness core-path prose in docs/CiGateInventory.md, and regenerates the knowledge graph (+1 file, +11 facts). Cost, measured: of the last 40 merged PRs (#633-#685) none would newly run the heavy lanes. 21 ran them already, 19 did not, and a replication of the in-job bash matcher agreed with the observed Linux-lane conclusion on all 40. The only two that touched a newly covered path (#640, #661) were already heavy through another entry. Mutation-checked in the devtest container (net8.0): - Removing src/Ashlar.Mcp.Server/** and src/Ashlar.Compat/** from both lists reddens the two tree facts naming exactly those paths, while AiPipelineCiRoutingConventionTests stays green. - A walker that sees only Include-first references reddens the fixture fact (misses lib/B and lib/C). Reordering the attributes of Core.Application's analyzer reference leaves the logged closure identical. Breaking the discovery port fires the suite floor at 0. - The verifier's shared-file edge (src/Directory.Build.props referencing src/Ashlar.Lite for the kernel suite) plus a direct control edge (src/Ashlar.Hosting.Bundle) now reddens the tree fact naming both directories in both lists; on the previous walker only the control was named. - Reading only the project file, not following explicit imports, accepting an unanchored shared reference, and letting a wildcard Import through each redden their own fixture facts. - The verifier's allowlist probe, re-applied to src/Ashlar.Transport.Grpc.csproj (a never-true ItemGroup with a cross-directory Protobuf item, a Reference HintPath and an EditorConfigFiles item, plus a property-function read of ver-verifier/v.txt), now reddens the tree input fact with 6 problems: protos-verifier/probe.proto (Protobuf), lib-verifier/VerifierProbe.dll (Reference HintPath) and cfg-verifier/probe.editorconfig (EditorConfigFiles), in both lists. It went green on the allowlist. ver-verifier/v.txt is not named; that is the documented property-function limit below. - Restoring the six-type allowlist reddens the new fixture fact (it misses 6 of its 7 paths) and the computed-Protobuf row. Not reading HintPath reddens the fixture fact (misses lib/Probe.dll) and the computed-HintPath row. Dropping the $(NuGetPackageRoot) rule makes CopyAssemblies.targets' AssembliesToCopy list throw in the tree fact. Dropping the bare-name rule, or InternalsVisibleTo from the identity list, reddens only the fixture fact. Each mutation turned red only those facts (24 or 25 of 26 still pass). The clean tree passes 26/26 here, 70/70 together with AiPipelineCiRouting and CertGateFilterCoverage, and 1314/1314 over the whole cert-gate filter. What this does NOT fix: readiness-summary still counts a skipped lane as a pass; validate still passes a project that selects zero tests; the container lanes do not run the sweep at all; files tests read at runtime (ci/*.json, workflows, docs) are not build inputs and are not traced; files read through an MSBuild property function are not traced - $([System.IO.File]::ReadAllText) is how the root Directory.Build.props and Directory.Build.targets read VERSION, which is in both lists by hand and is not proven by this test; paths in item metadata other than HintPath (a Protobuf ProtoRoot or AdditionalImportDirs) and files a compiled input reaches on its own (a .proto import) are not traced; projects an <MSBuild> task builds inside a target are not traced; and projects `ci verify` builds that are not discovered suites are out of scope. ci/test-ownership.tsv rows 47-49, 55 and 58, docs/HowGatesGoQuiet.md, CertifierReferenceSetTests' summary and the readiness CLOSING-PLAN still describe the old routing gap; they belong to other owners and are listed for them. This proves routing, not that anything passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
The trigger was inverted.
src/Ashlar.Tests.Kernel's 412 SPEC-006 admission and signing cases runonly in the readiness gate's native lanes, and those lanes are diff-conditional. Both path lists
contained
src/Ashlar.Tests.Kernel/**and none of the code that suite tests — so editing the testsre-ran them, while editing
GateStore.csorAdmissionGate.cs, the code they guard, skipped them.Adds the kernel project's transitive
ProjectReferenceclosure that no entry covered, to bothlists at the same position in the same order:
src/Ashlar.Policies/**does not coversrc/Ashlar.Policies.Dev/— the pattern's/afterPoliciesmust consume a/, and the candidate has.Devthere. Confirmed under both matchers.Ashlar.Analyzersis referenced byAshlar.Core.ApplicationwithOutputItemType="Analyzer"andReferenceOutputAssembly="false". It is a build-time analyzer, not code the 412 cases exercise; itis here because breaking it breaks the kernel suite's build.
Cost is zero, measured
Across the last 40 merged PRs (#683..#631), 20 already set
run_heavy=true, and these four entriesflip none of the other 20. Only three of the 40 touched these directories at all — #645 and #662
in
Manifest/Admission/, #640 inPolicies.Dev/— and all three already ran heavy viasrc/Ashlar.Tests.Infrastructure/**. Heavy runs are ~10-56 min (median 35m07s, n=27) with theWindows lane on the critical path; light runs median 28s. So this buys coverage on exactly the diffs
that need it without moving the median.
The constraint that makes a correct-looking fix fail
AiPipelineCiRoutingConventionTests:49-61assertsinternalPaths.Should().Equal(push)— identicalglobs and ordering — and it is inside required
cert-gate. Editing one list, or the two in adifferent order, reddens a required check.
So the patch asserts the invariant itself rather than trusting the edit: both lists parsed back out
and compared as sequences (45 = 45, identical), uniqueness, the test's supported-glob shape
(a single trailing
/**), and its negative contract that no glob may matchREADME.md,docs/unrelated-guide.mdorsrc/Unrelated/Nested/Other.cs. Then both matchers were run againstall four target paths and the three negative probes — bash's
*crosses/and GitHub's does not,so they are checked separately.
Records this falsifies
Three claims asserted the absence this PR removes, two written yesterday in #683. Leaving them would
reproduce the defect they describe, one turn later:
ci/test-ownership.tsvrow 58 said Manifest and Policies.Dev were "in neither". Now states therouting, and that the wider absent set — 41 directories across all 22 discovered suites — is still
open.
docs/specs/SPEC-006-keys-and-signing.mdsaid a change confined to the code under test does notre-run the tests that guard it. The other caveat in that paragraph — a skipped lane counts as a
pass, and the sweep emits no per-project count — is unchanged and still true.
ci/test-ownership.tsvrow 47 stays literally true (src/Ashlar.Analyzers/**does not matchsrc/Ashlar.Analyzers.Tests/, verified under both matchers) but now reads as false, so it isannotated rather than rewritten.
docs/CiGateInventory.md:112's core-path prose, which nothing machine-reads.Verification
114 passed, 0 failed —
AiPipelineCiRoutingConventionTests,TestOwnershipRunnerConventionTests,CertGateFilterCoverageTests,GateSignatureResidualTests,GateStoreAnchorProvenanceConventionTests.scripts/knowledge-graph/verify-knowledge-graph-current.py— current.collect_workflowsrecordstrigger names, not paths, so a path-list edit cannot drift the graph. Checked by running it.
verify-workflow-context-availability.shPASS (62 files),verify-workflow-run-interpolation.pyclean (62),
verify-doc-anchors.py31/31 resolve.run-repo-gates.sh --liststill 14 againstMINIMUM_GATES=11— no scripts added.What this does not fix
ValidationServiceAdapter.cs:248still computespassedfrom failures alone, so a discoveredproject that selects zero tests still passes; and
readiness-summarystill counts a skipped laneas a pass. Both are real and both have blast radius on a required check with
enforce_admins=true,so they are not bundled here.
Worth recording from the investigation: the zero-selection guard would be a no-op ratchet today.
All 22 discovered projects contribute non-zero tests on all three platforms — verified by a full
sweep in the container (5705/5706, 10 skipped, 22 projects) and across 17 sampled master runs, where
the
; no tests selected:suffix has never once been emitted. The detector has never fired becausecommit 95f8f41 added it and removed its only trigger (
Ashlar.Agents.TestKit) in the same commit.🤖 Generated with Claude Code