GT-589: a structural fact base, so the depth of architecture stops ending where a grep ends - #309
Merged
Merged
Conversation
release: align main — the CLI and MCP resolve the SDK that carries the security wave, 1.2.2 (GT-634)
A board row carries a STATUS but not who is working it or where, so two sessions can both read `PENDING` and both start, correctly. On 2026-07-30 that happened three times in a day. `50-validate-gap-claim` derives the claim set from OPEN PULL REQUESTS — every `GT-*` in a PR's title, body or branch name — and fails when one id is claimed by two of them, naming both with their branches and titles. DERIVED, NEVER HAND-WRITTEN, and the row asked for that on purpose: a hand-written claim goes stale in the direction that matters, because someone forgets to remove it and the next session works around a claim nobody holds. THREE CRITERIA OF FOUR, and the fourth is left open with its reason rather than stretched to fit. A claim list committed to the board would be derived from live GitHub state, so it would be stale the moment anyone opened a pull request — and GT-630's chain exists precisely to insist derived artifacts reach a fixed point. A perpetually-stale artifact inside that chain would be worse than none. Making the claim discoverable from the board needs something the board can render without committing, and that is not built. The row stays IN-PROGRESS. What it cannot see is in its own failure text, not implied: a branch with no open pull request claims nothing. Opening the PR early — draft is enough — is what makes the claim visible, which turns a convention into something with a check behind it. Anti-vacuous: a pull-request query that cannot be answered is a hard failure, not a quiet pass. A guard built because two sessions could not see each other must not stay silent when it cannot look. Zero open PRs is reported in words, because it looks identical to a broken query otherwise. Verified: 10/10 fixtures; 42-validate-guard-denominators 62 guards classified; 43-validate-guard-negative-fixtures 39/39 observed red, up from 38. Run against the real repository it reports 0 claims across 5 open PRs — checked rather than assumed: all five are Dependabot bumps naming no gap, so the zero is real and not an empty query. 08-validate-tracking (640 gaps), 01, 04, 41 within budget, 46 chain at a fixed point. Counters: 600/640, in-progress 14 -> 15, pending 22 -> 21. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat(ci): one gap, one claim — the claim guard, advancing GT-639
`.git/EVOLITH_*` is a DIRECTORY only in the primary checkout; inside a `git worktree` it is a FILE holding a `gitdir:` pointer, so every write/read to `.git/EVOLITH_*` was ENOTDIR. In pre-commit that made `set -e` abort the commit outright; in pre-push the `rm -f` failed and TARGET stayed unread — together the reason `--no-verify` had become habitual from a worktree. `git rev-parse --git-dir` resolves both layouts; both hooks now go through it via a shared `write_push_intent`/read path, per-worktree, so the push intent belongs to the checkout that was committed in rather than the repo as a whole. Also in pre-push: `assert_checkoutable` refuses up front, before repo optimization or board sync run, when the merge target branch (main/develop) is already checked out in another worktree — `git checkout "$to"` would otherwise fail mid-dispatch with the push half-done. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(ci): pre-commit/pre-push hooks assumed .git is always a directory
The engine had no queryable structural facts, so the depth of `architecture` ended where a grep ends: the OSS-enforcer seam returns a flat `Violation[]` per tool run, which answers exactly the questions the tool was configured to ask and no others. There was no module graph, no symbol graph, nothing an evaluator could interrogate. This adds `RepoFacts` — a content-hashed module graph + symbol graph — produced OUTSIDE the Core and delivered INLINE as a deterministic member of the `EvaluationContext`, the same shape ADR-0101/ADR-0080 already mandate for source files and ADR-0111 for quality signals. It reinforces statelessness rather than eroding it: the Core runs no indexer, opens no repository and retains nothing. - `core-domain/evaluation/contracts/repo-facts.ts` — the pure contract plus its queries: `canonicalizeRepoFacts` (the deterministic form a hash names), `findImportCycles` (Tarjan SCC + a concrete chain per component) and `findSymbolBoundaryCrossings` (multi-source BFS over the symbol reference graph). No filesystem, no crypto, no clock. - `EvaluationContext.repoFacts` + `ArchitectureContext.symbolBoundaries` (policy travels on the context, facts travel beside it), and `ArchitectureEvaluationResult.structuralFacts`, which echoes the extractor's `contentHash` so a verdict names the input it judged. - `createArchitectureKindEvaluator` merges the drift report with the structural queries, and now evaluates with NO workspace at all when facts are supplied — the "judge a repository the Core has never seen" path. - New package `@beyondnet/evolith-repo-facts`: the out-of-core extractor over the TypeScript compiler API (the engine `scip-typescript` itself drives), emitting SCIP-shaped symbol ids, plus a CLI and a `sha256` content hash. - Contract Schema Registry updated for both new shapes. The question the evaluator can now answer and could not before: *which forbidden SYMBOL does a boundary reach, and through which chain* — when `cli → application` and `application → infrastructure` are each legal, no single edge is a violation, so no pairwise import check can see that the CLI reaches the connection pool. Reproducibility is asserted, not argued: two extractions of the same tree produce byte-identical facts and two evaluations produce byte-identical verdicts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📊 Bilingual Coverage ImpactPR Changes
Repository Coverage
✅ Good: All EN changes have ES counterparts. Generated by GitHub Actions |
5 tasks
…to 1.1.0 `Test contracts` was red on this PR: `evaluation-context` and `evaluation-result` grew `symbolBoundaries` / `repoFacts` / `structuralFacts`, and both the published `MACHINE_CONTRACT_SET` and the producer descriptor still pinned the pre-change sha256. The pin doing its job — a schema cannot widen without the contract set saying so. Both changes are strictly ADDITIVE (the only removed line in the diff is a trailing comma), so this is a minor bump, not a major: a 1.0.0 consumer keeps validating against a 1.1.0 payload. `supportedConsumers[].contractVersion` is deliberately left at 1.0.0 — that pins what the Tracker was built against, and moving it would assert a consumer change nobody made. contracts 104/104, guard 10 "4 schema(s) hashed and resolved". Co-Authored-By: Claude Opus 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
1 participant
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.
Closes GT-589 (P1, complexity L). All three acceptance criteria met and observed.
The shape, and why it does not erode ADR-0101
New package
src/packages/repo-factsowns everything side-effectful — filesystem walk, compiler program, clock,sha256. The canonical contract lives in the Core and is pure:core-domain/src/evaluation/contracts/repo-facts.ts, no fs, no crypto, no clock.EvaluationContext.repoFacts,ArchitectureContext.symbolBoundaries(policy on the context, facts beside it),ArchitectureEvaluationResult.structuralFacts; both JSON schemas updated.The Core never calls the extractor. It receives a finished value inline — the identical seam
qualitySignals/Evidenceuse (ADR-0111) andOverlayFileSystemuses for source files (ADR-0080).contentHashis opaque provenance the Core echoes and never recomputes, exactly asProvenance.artifactHashis treated. The end-to-end spec injects a drift service that throws if touched and passessatellitePath: ''— so the evaluator provably judges a repository it has never seen, from facts alone. All queries are pure; nothing is retained.The indexer is the TypeScript compiler API — the same type-resolution engine
scip-typescriptdrives — emitting SCIP-shaped symbol ids<module>#<name>.Criterion 2: the question no current ruleset can express
Which forbidden symbol does a boundary reach, and through which exact chain?
Proof it could not before:
cli → applicationandapplication → infrastructureare each legal, and there is nocli → infrastructureimport edge anywhere in the tree (asserted in the spec). Import checks — ESLint boundaries, dependency-cruiser through the enforcer seam — are file-to-file and pairwise, so no single edge is a violation and none of them can see the composition. A symbol is also not a file.Cycle detection is deliberately not claimed as the criterion-2 question:
policy-compiler.tsalready compiles dependency-cruiserto.circular, so cycles are expressible today.findImportCyclesshipped anyway (same graph, nearly free) because it names the concrete chain instead of asserting existence — and it found two genuine runtime cycles insidecore-domainitself that nothing in CI reports. Both independently confirmed by the integrating session by reading the imports:application/services/index.tsre-exports../use-cases/initialize-project.use-case, which imports back from../services/index.validators/blocking-criteria-validator.ts↔validators/phase-gate-validator.service.tsimport each other.Those are a separate task, not fixed here.
Criterion 3: reproducibility, by diff rather than by argument
canonicalizeRepoFactsexcludes both the hash andprovenance.extractedAt, so a timestamp cannot leak into a verdict.Verification
npm run build(tsc -b, full solution)test:unitReal-scale check: extracting this repo's
core-domain/srcyields 349 modules / 933 imports / 1572 symbols / 2693 references in ~1.6s.Environment caveat, stated because it invalidates results silently: this worktree's
node_modulesoriginally resolved@beyondnet/*into the MAIN checkout. Everynode_modules(root + 6 per-package, including the nested ones holding@nestjs/cache-manager) was rebuilt with worktree-local links, verified withreadlink -f, and every number above is post-fix.dist/is load-bearing — mcp-server/core-api jest resolve core-domain through itsexportsmap — sonpm run buildran before every measurement.Deliberately left for later
scip-typescriptbinary and not anindex.scipreader. Both are second adapters behind the same output shape; documented as not-implemented in the package README.symbolBoundariesyet.🤖 Generated with Claude Code