From 99ac4af7a872aa095ed931db9788d5de45ab320f Mon Sep 17 00:00:00 2001 From: Alberto Arroyo Raygada Date: Mon, 13 Jul 2026 21:41:46 -0500 Subject: [PATCH 1/2] =?UTF-8?q?fix(rulesets):=20HXA-03=20stays=20native=20?= =?UTF-8?q?=E2=80=94=20remove=20misrouted=20enforce=20block=20(GT-516)=20(?= =?UTF-8?q?#185)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HXA-03 ("Infrastructure IMPLEMENTS Core ports") is a POSITIVE/structural assertion. dependency-cruiser is a forbidden-dependency tool and cannot verify a positive "X implements Y" claim, so an enforce block routing HXA-03 through it is semantically wrong: it can only ever degrade to a native fallback or, with a from/to config, emit false positives. Per GT-516's own design (and closure note), HXA-03 takes the documented per-rule NATIVE path rather than an enforce block (HXA-06 keeps its enforce block, which already degrades to a native fallback). - Remove the enforce block from HXA-03 so it evaluates on the NativeEvaluator path. - Update the ADR-0002 pilot test: enforcer-routed = HXA-01/02/04/05/06/07 (6); compiled = HXA-01/02/04/05/07 (5, unchanged); fallback = HXA-06 (1). Low-risk: GT-516's real cross-runtime execution is GT-512-gated, so nothing runs this enforce block live yet. Verified core-domain green (npx tsc && npx jest: 1018 passing) on this exact change. Co-authored-by: Claude Opus 4.8 --- .../enforcement/policy-compiler.spec.ts | 18 +++++++++++------- .../adr-0002-hexagonal-architecture.rules.json | 8 +------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/packages/core-domain/src/application/validators/enforcement/policy-compiler.spec.ts b/src/packages/core-domain/src/application/validators/enforcement/policy-compiler.spec.ts index 6952356a..25b0888b 100644 --- a/src/packages/core-domain/src/application/validators/enforcement/policy-compiler.spec.ts +++ b/src/packages/core-domain/src/application/validators/enforcement/policy-compiler.spec.ts @@ -170,22 +170,26 @@ describe('PolicyCompiler — compileRuleset partitions and never wholesale-fails }); describe('PolicyCompiler ⇄ ADR-0002 pilot (GT-516 AC1: HXA rules compile/fallback)', () => { - it('routes every HXA-01..07 rule through the enforcer and partitions compile vs per-rule fallback', () => { + it('routes the import-graph-expressible HXA rules through the enforcer and partitions compile vs per-rule fallback', () => { const ruleset = JSON.parse(readFileSync(findUp('src/rulesets/adr/adr-0002-hexagonal-architecture.rules.json'), 'utf8')); const rules: CompilableRule[] = ruleset.rules; const enforced = rules.filter((r) => r.enforce?.engine === 'enforcer'); - // The pilot populates an enforce block on all seven HXA rules. - expect(enforced.map((r) => r.id).sort()).toEqual(['HXA-01', 'HXA-02', 'HXA-03', 'HXA-04', 'HXA-05', 'HXA-06', 'HXA-07']); + // HXA-03 ("Infrastructure IMPLEMENTS Core ports") is a POSITIVE/structural assertion. + // dependency-cruiser is a forbidden-dependency tool and cannot express "X implements Y", + // so HXA-03 carries NO enforce block and is left on the native engine — routing it through + // the enforcer would only ever degrade to a native fallback (or, with a config, false-positive). + // The remaining six HXA rules are enforcer-routed. + expect(enforced.map((r) => r.id).sort()).toEqual(['HXA-01', 'HXA-02', 'HXA-04', 'HXA-05', 'HXA-06', 'HXA-07']); const { compiled, fallbacks } = compileRuleset(rules); // The five import-graph-expressible rules lower to a dependency-cruiser check. expect(compiled.map((c) => c.ruleId).sort()).toEqual(['HXA-01', 'HXA-02', 'HXA-04', 'HXA-05', 'HXA-07']); - // The two structural/positive rules (implements-ports, AOP-in-infra-only) are not - // import-graph-expressible → they take the documented per-rule NATIVE fallback rather - // than emitting an all-matching (false-positive-prone) rule, and never fail the run. - expect(fallbacks.map((f) => f.ruleId).sort()).toEqual(['HXA-03', 'HXA-06']); + // HXA-06 (AOP-in-infra-only) is enforcer-routed but not import-graph-expressible → it takes the + // documented per-rule NATIVE fallback rather than emitting an all-matching (false-positive-prone) + // rule, and never fails the run. (HXA-03 is native-only and never enters the enforcer partition.) + expect(fallbacks.map((f) => f.ruleId).sort()).toEqual(['HXA-06']); for (const f of fallbacks) { expect(f.fallback).toBe('native'); expect(f.tool).toBe('dependency-cruiser'); diff --git a/src/rulesets/adr/adr-0002-hexagonal-architecture.rules.json b/src/rulesets/adr/adr-0002-hexagonal-architecture.rules.json index b4d2692a..8ba371a1 100644 --- a/src/rulesets/adr/adr-0002-hexagonal-architecture.rules.json +++ b/src/rulesets/adr/adr-0002-hexagonal-architecture.rules.json @@ -63,13 +63,7 @@ "rationale": "ADR-0002 §Decision: Infrastructure is the adapter layer. It implements the ports defined by Core.", "validationQuery": "Infrastructure files implement interfaces defined in Core layer. No domain logic lives here.", "blocking": true, - "layer": "Infrastructure", - "enforce": { - "engine": "enforcer", - "tool": "dependency-cruiser", - "toolRuleId": "hxa-03-infra-implements-ports", - "runtime": "node" - } + "layer": "Infrastructure" }, { "id": "HXA-04", From d4fc4ad04e889002b669ad5594c9339a04c0ef96 Mon Sep 17 00:00:00 2001 From: Alberto Arroyo Raygada Date: Mon, 13 Jul 2026 21:42:07 -0500 Subject: [PATCH 2/2] fix(agent-runtime): route structural-review skill via canonical 'evidence' kind (GT-535) (#184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `code-quality-structural-review` skill declared `evaluationKinds: ['code-quality']`, but 'code-quality' is a quality-signal DIMENSION (the tag on the Evidence StructuralReviewProvider emits), NOT a member of the core-domain `EvaluationKind` union. `buildEvaluationContext` filters declared kinds against a KNOWN_KINDS allowlist, so 'code-quality' was silently dropped and the context fell back to `['gate']` — the skill was declared but never routed as the quality-signal evaluation it is, leaving the GT-535 provider unreachable. Fix (map onto an existing supported kind, not forge a phantom one): - default-skills.ts: `evaluationKinds: ['code-quality']` -> `['evidence']`, the canonical kind for declared quality-signal Evidence (ADR-0111 / GT-533). The 'code-quality' dimension stays where it belongs — on the emitted Evidence. - context-mapper.ts: sync the KNOWN_KINDS allowlist to the full canonical `EvaluationKind` union (add 'design', 'phase-artifacts') with a maintenance note, so the same "declared-but-dropped" class can't recur for those kinds. - structural-review.spec.ts: add a routing guard proving the skill forwards 'evidence' through buildEvaluationContext (not the 'gate' fallback), and that a dimension-shaped kind still collapses to ['gate']. Kind routing only; the concrete IStructuralReviewer LLM adapter is a separate follow-on. agent-runtime jest green (17 suites / 121 tests), tsc --noEmit clean. Co-authored-by: Claude Opus 4.8 --- .../src/__tests__/structural-review.spec.ts | 39 +++++++++++++++++++ .../src/adapters/skills/default-skills.ts | 8 +++- .../src/application/context-mapper.ts | 9 +++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/packages/agent-runtime/src/__tests__/structural-review.spec.ts b/src/packages/agent-runtime/src/__tests__/structural-review.spec.ts index af50dbb2..4d360b10 100644 --- a/src/packages/agent-runtime/src/__tests__/structural-review.spec.ts +++ b/src/packages/agent-runtime/src/__tests__/structural-review.spec.ts @@ -32,6 +32,10 @@ import { compareSeverity, } from '../domain/rubrics/structural-review-rubric'; import type { CollectionContext, CollectionTarget } from '../domain/ports/quality-signal-provider.port'; +import { DEFAULT_SKILLS } from '../adapters/skills/default-skills'; +import { buildEvaluationContext } from '../application/context-mapper'; +import { parseAgentRuntimeRequest } from '../domain/contracts/agent-runtime-request'; +import type { SkillDescriptor } from '../domain/contracts/capability'; /** A deterministic stub reviewer standing in for the probabilistic LLM/agent. */ function stubReviewer(findings: readonly RawStructuralFinding[]): IStructuralReviewer { @@ -208,3 +212,38 @@ describe('Structural Quality Gate (deterministic severity → decision)', () => expect(decideForSeverity('medium', DEFAULT_STRUCTURAL_GATE_POLICY)).toBe('warn'); }); }); + +/** + * Kind routing (GT-535) — the structural-review skill must forward a canonical + * EvaluationKind through buildEvaluationContext, NOT be dropped to the 'gate' + * fallback. Regression guard: 'code-quality' is a quality-signal DIMENSION, not a + * kind; the skill declares the canonical 'evidence' kind so the provider stays + * reachable when the IStructuralReviewer adapter lands. + */ +describe('structural-review skill kind routing (GT-535)', () => { + const skill = DEFAULT_SKILLS.find((s) => s.id === 'code-quality-structural-review'); + const req = parseAgentRuntimeRequest({ tenant: 't-1', intent: 'structural_review' }); + + it('declares a canonical evaluation kind (not the code-quality dimension)', () => { + expect(skill).toBeDefined(); + expect(skill?.evaluationKinds).toEqual(['evidence']); + // 'code-quality' is the Evidence DIMENSION, never an EvaluationKind. + expect(skill?.evaluationKinds).not.toContain('code-quality'); + }); + + it('forwards the declared kind through buildEvaluationContext (not the gate fallback)', () => { + const ctx = buildEvaluationContext(req, skill as SkillDescriptor); + expect(ctx.kinds).toEqual(['evidence']); + // Guard the regression: an unknown/dimension-shaped kind would collapse to ['gate']. + expect(ctx.kinds).not.toEqual(['gate']); + }); + + it('drops a non-canonical kind to the gate fallback (documents the filter)', () => { + const bogus: SkillDescriptor = { + ...(skill as SkillDescriptor), + evaluationKinds: ['code-quality'], + }; + const ctx = buildEvaluationContext(req, bogus); + expect(ctx.kinds).toEqual(['gate']); + }); +}); diff --git a/src/packages/agent-runtime/src/adapters/skills/default-skills.ts b/src/packages/agent-runtime/src/adapters/skills/default-skills.ts index 8f96bbc1..40c192f7 100644 --- a/src/packages/agent-runtime/src/adapters/skills/default-skills.ts +++ b/src/packages/agent-runtime/src/adapters/skills/default-skills.ts @@ -93,8 +93,14 @@ export const DEFAULT_SKILLS: readonly SkillDescriptor[] = [ intents: ['structural_review', 'code_quality_review', 'review_structure'], // Orchestration runs the probabilistic reviewer behind IStructuralReviewer and // hands the normalized Evidence to the Core as a code-quality signal (GT-535). + // NOTE: 'code-quality' is the quality-signal DIMENSION the emitted Evidence is + // tagged with (StructuralReviewProvider.DEFAULT_DIMENSION), NOT an EvaluationKind. + // The canonical EvaluationKind for declared quality-signal Evidence is 'evidence' + // (ADR-0111 / GT-533). Declaring it here keeps the kind routed by + // buildEvaluationContext instead of being dropped to the 'gate' fallback, so the + // StructuralReviewProvider stays reachable when the IStructuralReviewer adapter lands. kind: 'evaluation', - evaluationKinds: ['code-quality'], + evaluationKinds: ['evidence'], permissions: ['read:repo'], requiresApproval: false, emitsTrace: true, diff --git a/src/packages/agent-runtime/src/application/context-mapper.ts b/src/packages/agent-runtime/src/application/context-mapper.ts index e0da69dd..b8b55718 100644 --- a/src/packages/agent-runtime/src/application/context-mapper.ts +++ b/src/packages/agent-runtime/src/application/context-mapper.ts @@ -35,9 +35,18 @@ export function toPhaseId(phase?: string): PhaseIdT | undefined { return (CANONICAL_PHASES as readonly string[]).includes(v) ? (v as PhaseIdT) : undefined; } +/** + * Allowlist of canonical evaluation kinds a skill may forward to the Core. It MUST + * mirror the core-domain `EvaluationKind` union (evaluation-context.ts) — a skill + * that declares a kind absent from this list is silently dropped to the 'gate' + * fallback (that is the bug GT-535 hit: 'code-quality' is a quality-signal + * DIMENSION, not a kind). Kept as a literal (not imported) because this file is a + * type-only consumer of core-domain — the hexagon carries no runtime dependency. + */ const KNOWN_KINDS: readonly string[] = [ 'gate', 'artifact', 'evidence', 'architecture', 'blueprint', 'topology', 'checkpoint', 'deployment', 'rule', 'compliance', + 'design', 'phase-artifacts', ]; /** Build a canonical EvaluationContext from a runtime request + resolved skill. */