Skip to content

fix(ci): route the code the kernel suite guards to the lane that runs it - #684

Merged
IanFrelinger merged 1 commit into
masterfrom
fix/route-guarded-code-to-readiness
Sep 30, 2026
Merged

IanFrelinger merged 1 commit into
masterfrom
fix/route-guarded-code-to-readiness

Conversation

@IanFrelinger

Copy link
Copy Markdown
Owner

The trigger was inverted. 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/** and 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.

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 pattern's / after
Policies must consume a /, and the candidate has .Dev there. Confirmed 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.

Cost is zero, measured

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 and #662
in Manifest/Admission/, #640 in Policies.Dev/ — and all three already ran heavy via
src/Ashlar.Tests.Infrastructure/**. Heavy runs are ~10-56 min (median 35m07s, n=27) with the
Windows 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-61 asserts internalPaths.Should().Equal(push) — identical
globs and ordering — and it is inside required cert-gate. Editing one list, or the two in a
different 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 match README.md,
docs/unrelated-guide.md or src/Unrelated/Nested/Other.cs. Then both matchers were run against
all 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.tsv row 58 said Manifest and Policies.Dev were "in neither". Now states the
    routing, and that the wider absent set — 41 directories 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 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.tsv row 47 stays literally true (src/Ashlar.Analyzers/** does not match
    src/Ashlar.Analyzers.Tests/, verified under both matchers) but now reads as false, so it is
    annotated rather than rewritten.
  • docs/CiGateInventory.md:112's core-path prose, which nothing machine-reads.

Verification

  • The five affected cert-gate convention tests, devtest container at net8.0:
    114 passed, 0 failed — AiPipelineCiRoutingConventionTests,
    TestOwnershipRunnerConventionTests, CertGateFilterCoverageTests,
    GateSignatureResidualTests, GateStoreAnchorProvenanceConventionTests.
  • scripts/knowledge-graph/verify-knowledge-graph-current.py — current. collect_workflows records
    trigger names, not paths, so a path-list edit cannot drift the graph. Checked by running it.
  • verify-workflow-context-availability.sh PASS (62 files), verify-workflow-run-interpolation.py
    clean (62), verify-doc-anchors.py 31/31 resolve.
  • run-repo-gates.sh --list still 14 against MINIMUM_GATES=11 — no scripts added.
  • TSV field integrity: every non-comment row still exactly 4 tab-separated fields.

What this does not fix

ValidationServiceAdapter.cs:248 still computes passed from failures alone, so a discovered
project that selects zero tests still passes; and readiness-summary still counts a skipped lane
as 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 because
commit 95f8f41 added it and removed its only trigger (Ashlar.Agents.TestKit) in the same commit.

🤖 Generated with Claude Code

`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>
@IanFrelinger
IanFrelinger merged commit 76395ff into master Sep 30, 2026
19 checks passed
@IanFrelinger
IanFrelinger deleted the fix/route-guarded-code-to-readiness branch September 30, 2026 12:11
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant