GT-641/GT-642: two runtime import cycles inside the Core itself, and nothing in CI can find the next one - #311
Open
beyondnetPeru wants to merge 12 commits into
Open
GT-641/GT-642: two runtime import cycles inside the Core itself, and nothing in CI can find the next one#311beyondnetPeru wants to merge 12 commits into
beyondnetPeru wants to merge 12 commits into
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
…omain
The structural fact base added by GT-589 reports two cycles in the Core's own
sources, neither of them type-only and neither reported by anything in CI today:
services/index.ts ⇄ use-cases/initialize-project.use-case.ts
validators/blocking-criteria-validator.ts ⇄ validators/phase-gate-validator.service.ts
Both have the same shape — a module publishes a type AND constructs/re-exports
the collaborator that consumes it, so the collaborator has to import back
through it. The strongly connected components were wider than the two-node
chains: project-scaffolder.service and phase-transition.use-case sat in the
first, evidence-validator and ruleset-loader in the second.
The types move out to their own modules and every consumer imports them
directly:
application/services/use-case.types.ts InitProjectInput, InitProjectResult,
PhaseTransitionResult
application/validators/phase-gate-validator.types.ts
PhaseGateDefinition and the rest of
the gate contract
`services/index.ts` and `phase-gate-validator.service.ts` re-export them, so
every existing import path — including `@beyondnet/evolith-core-domain/
application/services` used by the CLI and the MCP server — is unchanged.
Verified by re-running the GT-589 extractor over src/packages/core-domain:
346 modules / 2 cycles before → 348 modules / 0 cycles after. core-domain
jest 129 suites / 1455 tests green, tsc build clean, CLI build clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…that cannot exist yet) GT-641 — DONE — records the two runtime import cycles the GT-589 fact base found inside core-domain and the fix: 346 modules / 2 cycles → 348 / 0, measured with the same extractor over the same tree, before and after. GT-642 — PENDING — is the guard. It is carried as its own row instead of as an unchecked criterion inside a closed one: a DONE row with an open criterion reads as finished to anyone who does not open the catalog. It is BLOCKED, not merely pending — `@beyondnet/evolith-repo-facts` exists only on `feat/gt-589-repo-facts` (PR #309), and a check on main cannot run an extractor that is not on main. Writing a second cycle detector beside the first would be the duplicated source of truth GT-589 exists to remove. The closure record for GT-641 keeps two things a green tick would have hidden: the CLI suite was red on first run for an environment reason (a fresh worktree has no built dist for agent-runtime, so jest cannot resolve it), and the three `nest build` errors in mcp-server were reproduced on a reverted tree before being attributed — they are the worktree dual-resolution problem, not this work. Guards: 08-validate-tracking 642 gaps EN/ES, 618/618 catalog sections, 583 closure records; 49-validate-gap-id-allocation 520 ids, 0 collisions, GT-641 and GT-642 newly allocated; 41-validate-evidence-commands 19 dead (budget 21), so the new commands add none; 01-validate-docs 1491 files; 04-check-bilingual-parity 1640 files. 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
To create skeletons: node .harness/scripts/generate-es-skeleton.mjs <file.md>Generated by GitHub Actions |
…T-642 rows `09-reconcile-maturity.mjs --check` was red on PR #311: the reconciliation is derived from the gap board, so adding two rows makes it stale by construction. Regenerated rather than hand-edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second link of the same derived chain: 46-validate-derived-artifact-order stops at the FIRST stale artifact, so the summary only surfaced once the maturity reconciliation ahead of it was current. Chain now replays to a fixed point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
beyondnetPeru
changed the title
…disk Found while closing GT-641: the CLI e2e suite left three TRACKED files dirty. Tracing which spec did it led to the flag, not to the test. `agents.command.ts:647` declares `-d, --dry-run` and `installAgent()` never reads it — line 218 calls `registry.installAgent(process.cwd(), ...)` unconditionally. `init` swaps in a DryRunFileSystem, `upgrade` returns before applying, `adr` threads it through; `agents` is the only declaration in the CLI with no reader. Proven by reproduction rather than by reading the code: the whole spec was run first, then `-t "dry-run"` alone against a clean tree, and both leave `rulesets/agents/agents-registry.json` plus the two files under `rulesets/agents/test-value/` modified. `test-value` is what the prompt mock answers to every question, and 6bb43cf versioned that agent into the repo's own rulesets — a test's output shipped as product data. Registered as P1 and separately from the test-hygiene symptom on purpose: pointing the spec at a temp directory would hide the defect and leave the flag lying to users. A dry run that writes is worse than no dry run, and no oracle catches it because the conformance tester compares what surfaces ANSWER, never whether the disk changed. Guards: 08-validate-tracking 643 gaps EN/ES, 619/619 catalog sections; 49-validate-gap-id-allocation 521 ids, 0 collisions; 46-validate-derived-artifact-order current and at a fixed point after regenerating the reconciliation and summary; 01-validate-docs 1491 files; 04-check-bilingual-parity 1640 files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The flag was declared at agents.command.ts:647, typed on the options interface, documented — and never read. `installAgent()` called `registry.installAgent(process.cwd(), ...)` unconditionally, so the flag a user reaches for precisely when they do not want to be trusted with the real one wrote to the working directory anyway, then reported success. The dry branch now never reaches the writer at all, rather than writing through a no-op filesystem: a branch that cannot reach the writer cannot regress when the writer grows a new call. It reports what it would have created through a new `AgentRegistryService.planInstall`, which the writer itself uses, so the report cannot drift from the layout it describes. The menu path was the same defect one level up: `showMenu()` dropped the caller's options, so `evolith agents --dry-run` with no subcommand picked install from the menu and wrote. Options are forwarded now. Deleted the three artifacts a previous run committed — `rulesets/agents/agents-registry.json` and both files under `rulesets/agents/test-value/`. `test-value` is the string the prompt mock answers to every question, and the registry held exactly that one agent: the whole file was test output shipped as product data. The e2e suite ran against the repository itself, which is why it was writing into it. It now runs in a temp directory AND asserts an unchanged disk, with a contrast case proving a real install still writes — otherwise the dry-run assertion would also pass if install had simply broken. Per-test module and directory, because a shared instance leaks commander's parsed options between runs: `agents install` after `agents install --dry-run` still saw dryRun=true, which made a working fix look broken. `did not crash` was the old oracle. It passed for as long as the defect existed. Survey of every other `--dry-run` in the CLI, recorded including the correct ones: init (swaps in DryRunFileSystem), upgrade, adr, scaffold, docs, fixtures and generate-domain all gate their writes on it. agents was the only declaration with no reader. GT-643 stays IN-PROGRESS: the cross-surface tester still has no oracle for no-effect flags as a class, so the next one is found by hand as this one was. Verified: cli unit 101 suites / 1447 tests; cli e2e 18 suites / 133 tests, and `git status` is clean after the suite for the first time. 08-validate-tracking 643 gaps EN/ES, 619/619 catalog sections; 46-validate-derived-artifact-order at a fixed point; 01-validate-docs, 04-check-bilingual-parity, 40-validate-path-literals green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… (GT-643) Deleting `src/sdk/cli/rulesets/agents/` turned two checks red, and neither failure named its own cause. Both were leaning on the residue rather than on anything authored. `47-validate-joined-paths` declares `src/sdk/cli/rulesets/opa` a generated subtree, and requires such a claim to be ANCHORED: the parent must exist in a clean checkout, so a typo inside the prefix stays red. The parent is `src/sdk/cli/rulesets` — gitignored in full (`rulesets/*`) — and it existed in CI only because three files a test had written were committed inside it. With them gone the anchor failed and the guard reported `policy.wasm` resolving to nothing, which was true and was not the cause. Proven rather than argued: over a clean checkout of HEAD~1 the path resolves true, over HEAD false. The prefix is now the whole produced subtree, anchored on `src/sdk/cli`, which is authored. Re-verified against a clean checkout: the artifact and its directory pass, while `src/sdk/cli/rulesetss/...`, `sdk/cli/rulesets/...` and an unrelated missing directory all stay red. Self-test 14/14. `rulesets-resolver.bundled.spec.ts` had one case still reading the real tree. Its own header already recorded that the bundled tree is a build artifact and that its siblings were migrated to a synthetic one "minus the committed rulesets/agents/" — that exception was the residue, and this case survived on it: the EACCES it simulates only fires when the walk reaches some `…/rulesets`, and after the deletion it reached none. It drives the synthetic tree now, and the stale header sentence is corrected. Verified under the condition CI actually has, not the local one: with `src/sdk/cli/rulesets` parked out of the tree, the CLI suite is 101 suites / 1447 tests green. Guards: 47, 46 (chain at a fixed point), 40, 42 (62 guards classified), 43 (39/39 observed red), 08 (643 gaps EN/ES, 619/619 sections), 01 (1491 files). Editing .harness/ required stepping past the unimar-core S-16 hook, with the owner's explicit approval — this repo's .harness has no upstream to propose to. 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.
Pull Request Summary
The structural fact base from #309, pointed at the repository that produced it, reported two runtime import cycles inside
core-domain. Neither is type-only, so neither is erased at compile time, and nothing that runs in CI today reports either one.Both cycles are the same defect twice: a module publishes a type AND constructs or re-exports the collaborator that consumes it, so the collaborator has to import back through it to name its own parameters.
application/services/index.tsownedInitProjectInputwhile re-exportingInitializeProjectUseCaseapplication/validators/phase-gate-validator.service.tsownedPhaseGateDefinitionwhile constructingEvidenceValidator,BlockingCriteriaValidatorandRulesetLoaderThe reported two-node chains were the smaller half of the finding. Each sits inside a four-module strongly connected component —
project-scaffolder.serviceandphase-transition.use-casein the first,evidence-validatorandruleset-loaderin the second — so a fix confined to the two files named in the chain would have moved the cycle rather than removed it. Thecomponentfield is what said so.The fix is not
import type. Erasing the back-edge would satisfy the extractor (it classifies type-only edges as erased-at-runtime) while leaving the layering inverted. The types move out instead, intouse-case.types.tsandphase-gate-validator.types.ts; both former owners re-export them, so no import path changes anywhere — including@beyondnet/evolith-core-domain/application/services, from which the CLI and the MCP server importInitProjectInput, and the 15 sites that import the gate contract from the service.Measured, before and after, over the same tree: 346 modules / 2 cycles → 348 modules / 0 cycles.
GT-642 is the open half, and it is blocked rather than pending
No guard ships with this PR and the board says so in its own row instead of leaving it implied.
@beyondnet/evolith-repo-factsexists only on the branch of #309: a check on the default branch cannot run an extractor that is not on the default branch, and writing a second cycle detector beside the first would be the duplicated source of truth that branch exists to remove. Onlycore-domainhas ever been measured —cli,mcp-server,core,agent-runtimeandsdk-clienthave not, and the barrel-owns-the-type shape is a monorepo-wide idiom.Two things kept in the record rather than smoothed over
distfor@beyondnet/evolith-agent-runtime, so jest cannot resolve it. Green after a workspace-wide build.nest buildformcp-serverreports 3TS2345duplicate-identity errors onRulesetValidatorService. Reproduced on a reverted tree before attributing it — it is the worktree dual-resolution problem (some specifiers resolve to the main checkout'sdist), pre-existing and untouched here.GT-643 — found while closing this one, and fixed
The CLI e2e suite left three TRACKED files dirty. Tracing which spec did it led to a product defect, not a test defect:
agents install --dry-runwrites to disk.agents.command.ts:647declares the flag;installAgent()never reads it and callsregistry.installAgent(process.cwd(), ...)unconditionally at line 218.init,upgradeandadrall read theirs —agentsis the only declaration in the CLI with no reader.Proven by reproduction: the whole spec first, then
-t "dry-run"alone against a clean tree; both leaverulesets/agents/agents-registry.jsonand the two files underrulesets/agents/test-value/modified.test-valueis what the prompt mock answers to every question, and6bb43cfaversioned that agent into the repository own rulesets.Fixed here. The dry branch never reaches the writer rather than writing through a no-op filesystem — a branch that cannot reach the writer cannot regress when the writer grows a new call — and reports what it would create via a new
planInstallthe writer itself uses. The menu path was the same defect one level up:showMenu()dropped the caller options, soevolith agents --dry-runwith no subcommand wrote anyway.The three committed artifacts are deleted; the registry held exactly one agent,
test-value, so the whole file was test output. The e2e suite now runs in a temp directory and asserts an unchanged disk, with a contrast case proving a real install still writes — otherwise the dry-run assertion would also pass if install had simply broken. Per-test module and directory, because a shared instance leaks commander parsed options between runs and made the working fix look broken.did not crashwas the old oracle; it passed for as long as the defect existed.Survey recorded, including the commands that were already correct:
init,upgrade,adr,scaffold,docs,fixtures,generate-domainall gate writes on the flag.agentswas the only declaration with no reader.GT-643 stays IN-PROGRESS, not DONE: the cross-surface tester still has no oracle for no-effect flags as a class, so the next one is found by hand as this one was.
Verification
core-domaincliunitclie2egit statusclean afterwards for the first timemcp-servernpm run buildtsc -bclean across the workspaceGuards run locally:
08-validate-tracking642 gaps EN/ES, 618/618 catalog sections, 583 closure records ·49-validate-gap-id-allocation520 ids, 0 collisions ·41-validate-evidence-commands19 dead (budget 21) — the new commands add none ·01-validate-docs1491 files ·04-check-bilingual-parity1640 files ·46-validate-derived-artifact-orderchain current and at a fixed point.Known-red check, and whose it is
Governance guardsis red on this PR for50-validate-gap-claim, which reports 7 contested ids across the 12 open PRs. It is a repository-wide view, not a property of this branch: the same check is already red on #306 and #309 for the same reason, and the contested ids come from the integration PR #305 overlapping #306/#307/#310. Nothing here can turn it green — that needs whoever owns those overlaps to decide which PR keeps each id.Evolith Core Quality Gates
.mdand.es.md.node .harness/scripts/ci/01-validate-docs.mjsrun locally.node .harness/scripts/ci/04-check-bilingual-parity.mjsrun locally.Linked ADRs / Issues
architecturestops ending where a grep ends #309Conventional Commits
fix(core):,chore(board):).🤖 Generated with Claude Code