chore(tooling): land L17 and L18, closing oxaudit.md in full - #59
Merged
Conversation
The last two open items. DECISIONS 72 records the reasoning; taking them separately is what turned up something worth keeping. L17 needed no dependency. Section 69 deferred both items on one shared reason - each needs a new devDependency - and the audit suggested eslint-plugin- boundaries or dependency-cruiser. But the rules this repository actually has are two import bans, and @typescript-eslint/no-restricted-imports is already loaded. So the deferral reason, written for a pair, had only ever applied to one of them. Two rules: only stage-registry may value-import a concrete stage (invariant 4), and core/stages may not import from adapters, cli or gateway. allowTypeImports lets `import type` through, which is the point - a type import is erased and wires nothing. Both verified by planting the violation they should catch, because a misconfigured lint rule looks exactly like a clean codebase. Value import from a stage errors; import type from a stage passes; upward import errors. Writing the rule found that invariant 4 is not what the code does. core/validation/index.ts:13 value-imports extractConstraintDirectives from stages/cleanup/constraint-preservation, so the constraint check and the stage that preserves constraints share one extractor - a runtime dependency from core onto a concrete stage, which the invariant says there is exactly one of. Exempted at that one path with the reason at the site rather than refactored: moving the extractor changes the optimize route, which needs a corpus measurement, and a commit adding a linter should not carry one. The honest state is that the code and the invariant disagree and the code is what shipped. L18 adds @vitest/coverage-v8, which is a real dependency with no way around it. What is optional is what to do with the number, and the decision is reporting only: npm run coverage produces it, npm test does not, no threshold fails a build. A number attached to a gate becomes a target, and the failure mode is a suite that covers lines instead of behaviour - the opposite of what these tests are for. validator-guarantee.test.ts asserts that English prose passes the TypeScript validator; a coverage gate would reward deleting it. Baseline over src/**: statements 92.54% (8895/9612), branches 87.20% (3100/3555), functions 97.01% (358/369). coverage/ was already gitignored and contributes 0 entries to npm pack - checked rather than assumed, still 223 files. 99 files / 918 tests pass, typecheck and lint clean. 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
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 OX-L17 and OX-L18, the last two open items in
oxaudit.md. DECISIONS §72 records the reasoning; §69 deferred both and this reverses that.L17 needed no dependency, and the deferral reason did not survive contact
§69 held both items back on one shared reason — "Both require a new devDependency (
eslint-plugin-boundariesor dependency-cruiser;@vitest/coverage-v8)."But the rules this repository actually has are two import bans, and
@typescript-eslint/no-restricted-importsis already loaded. So the deferral reason, written for a pair, had only ever applied to one of them.Two rules:
src/core/stage-registrymay value-import a concrete stagesrc/coreandsrc/stagesmay not import fromadapters/,cli/orgateway/allowTypeImportsis load-bearing: animport typeis erased at compile time and couples nothing, so the engine naming a stage's options type is not a violation.Verified to fire, not merely to pass
A misconfigured lint rule looks exactly like a clean codebase — invariant 10 pointed at the linter. Each rule was checked by planting the violation it should catch:
stages/intocore/plannerimport typefromstages/intocore/plannergateway/session-storeintocore/plannerThe rule found that invariant 4 is not what the code does
CLAUDE.md states it flatly: "Only
stage-registryimports concrete stage implementations." Three other files import fromsrc/stages/. Two arecore/engine'simport type, which couple nothing. The third is real:core/validation/index.ts:13value-importsextractConstraintDirectivesfromstages/cleanup/constraint-preservation— so the constraint check and the stage that preserves constraints share one extractor. That is a runtime dependency from core onto a concrete stage, and the invariant says there is exactly one of those.Exempted at that one path, with the reason at the site. Moving the extractor changes the optimize route, which this repo requires a corpus measurement for, and a commit adding a linter should not carry one. The rule locks in the status quo; every new violation fails. The honest state is that the code and the invariant disagree, and the code is what shipped.
L18 is reporting, not a gate
@vitest/coverage-v8is a real dependency with no way around it. What is optional is what to do with the number.npm run coverageproduces it;npm testdoes not, and no threshold fails a build. A number attached to a gate becomes a target, and the failure mode is a suite that covers lines instead of behaviour — the opposite of what these tests are for.validator-guarantee.test.tsasserts that English prose passes the TypeScript validator; a coverage gate would reward deleting it.Baseline over
src/**: statements 92.54% (8895/9612) · branches 87.20% (3100/3555) · functions 97.01% (358/369).coverage/was already gitignored and contributes 0 entries tonpm pack— checked rather than assumed, still 223 files.What this does not establish
Neither rule says the architecture is right, only that it stopped drifting. The layering rule was clean before it existed, so it has never actually caught anything — its value is entirely prospective.
Checks
99 test files / 918 tests pass, 2 skipped. Typecheck and lint clean. No source behaviour changed; no corpus run applies.
🤖 Generated with Claude Code