feat: report blocked-to-exported reference repairs as non-breaking (#99)

Author: jacekradko

.changeset/repaired-reference-downgrade.md

@@ -0,0 +1,22 @@
+---
+"@clerk/break-check": minor
+---
+
+A reference _repair_ is now reported non-breaking (issue #98). When a breaking
+modification's only diff is swapping module specifiers consumers could not
+resolve (export-blocked, e.g. `@clerk/shared/_chunks/index-Cr_OtBLq` under
+`"./_chunks/*": null`, or chunk-shaped with the dependency unlocatable) for
+specifiers that provably resolve against the dependency's `exports`, the change
+is deterministically downgraded and tagged as a repaired reference in the
+report. The old reference errored (TS2307) or degraded to `any` downstream, so
+fixing it cannot break anyone. The check is fail-closed: any difference beyond
+the specifier/alias swap, or an introduced specifier that does not provably
+resolve, keeps the change breaking. Opt out with
+`downgradeRepairedReferences: false`.
+
+The AI reviewer now also receives deterministic exports-map verdicts
+(`referenceResolutions`) for every specifier a signature drops or introduces,
+instead of guessing resolvability from path shapes, and cannot escalate a
+deterministically repaired change back to breaking (recorded as
+`ai-suggested-escalation`, mirroring the downgrade refusal for unresolvable
+references).

AGENTS.md

@@ -276,6 +276,36 @@ Before declaring work done: `pnpm check` must pass, and `git diff main
   diff fires. The only case `canonicalType` collapses to nothing is a
   resolvable-chunk -> resolvable-chunk move, which is benign (a resolvable chunk
   is importable by consumers).
+- **The repair downgrade is the guard's deterministic inverse (issue #98).**
+  When a breaking modification's only diff is swapping unconsumable specifiers
+  for exported ones, `detector.ts#applyReferenceRepairs` (running right after
+  `flagUnresolvableReferences`, before the AI) flips it to non-breaking and
+  records `repairedReference: { from, to }`. The gate is
+  `findRepairedReference` (`utils/exports-resolution.ts`): every removed
+  specifier must classify `blocked` deterministically, or `unknown` +
+  chunk-shaped + `packageNotFound` (a LOCATED dependency without an `exports`
+  map never qualifies: legacy resolution serves every file, so the chunk may
+  genuinely have resolved), and must not match `resolvableSpecifiers`; every
+  introduced specifier must be a bare specifier classified `exported`
+  DETERMINISTICALLY against the dependency's actual `exports` map (the
+  downgrade clears a break, so nothing else may vouch for the after side;
+  note `classifyReference` calls a relative/absolute/malformed specifier
+  "exported" for the guard's fail-safe direction, which is why the repair
+  pass uses `classifyTransition`'s richer verdicts, not `classifyReference`);
+  and the snippets must be identical after masking each swapped
+  `import("spec").Name` unit (the alias name may change with the specifier,
+  bundlers minify chunk-internal names; only the first member access is
+  masked, deeper chains must still match). Anything else fails the masked
+  compare and stays breaking, fail-closed. A change the unresolvable guard
+  flagged is never downgraded; `downgradeRepairedReferences: false` is the
+  config opt-out. The AI cannot escalate a repaired change: the analyzer
+  records the refused verdict as `ai-suggested-escalation`, mirroring the
+  downgrade refusal for `unresolvableReference`. The same pass attaches
+  `referenceResolutions` (per-specifier exports-map verdicts, both sides) to
+  any change whose specifier sets differ, regardless of repair outcome or the
+  toggle; the per-change review JSON ships them and system-prompt rules 12/13
+  tell the model to trust those verdicts over path shapes. Keep
+  `repairedReference` and `referenceResolutions` OUT of `generateChangeId`.
 - **Action depends on the published package**: the composite Action's `npx`
   step fetches `@clerk/break-check` from npm at runtime, so consumers pin the
   repo's moving `v1` tag (`clerk/break-check@v1`). Keep the README's Actions

README.md

@@ -187,18 +187,19 @@ a machine-readable verdict without running detection (and the AI reviewer) twice
 
 ## Configuration
 
-| Option                 | Type     | Default          | Description                                                            |
-| ---------------------- | -------- | ---------------- | ---------------------------------------------------------------------- |
-| `packages`             | string[] | required         | Package paths to analyze                                               |
-| `snapshotDir`          | string   | `.api-snapshots` | Snapshot output directory                                              |
-| `mainBranch`           | string   | `main`           | Base branch name for repo-specific workflows                           |
-| `checkVersionBump`     | boolean  | `true`           | Mark insufficient version bumps in reports                             |
-| `outputFormat`         | string   | `markdown`       | Default report format                                                  |
-| `ignoreSubpaths`       | string[] | `[]`             | Subpath exports to skip (exact, or glob with `*`/`**`)                 |
-| `ignoreHashedChunks`   | boolean  | `true`           | Drop content-hashed bundler chunks matched by `./*`                    |
-| `acknowledgedChanges`  | string[] | `[]`             | Breaking changes you've verified safe (downgraded + tagged)            |
-| `resolvableSpecifiers` | string[] | `[]`             | Module-specifier globs to exempt from the unresolvable-reference guard |
-| `ai`                   | object   | unset            | AI reviewer options (see below)                                        |
+| Option                        | Type     | Default          | Description                                                                      |
+| ----------------------------- | -------- | ---------------- | -------------------------------------------------------------------------------- |
+| `packages`                    | string[] | required         | Package paths to analyze                                                         |
+| `snapshotDir`                 | string   | `.api-snapshots` | Snapshot output directory                                                        |
+| `mainBranch`                  | string   | `main`           | Base branch name for repo-specific workflows                                     |
+| `checkVersionBump`            | boolean  | `true`           | Mark insufficient version bumps in reports                                       |
+| `outputFormat`                | string   | `markdown`       | Default report format                                                            |
+| `ignoreSubpaths`              | string[] | `[]`             | Subpath exports to skip (exact, or glob with `*`/`**`)                           |
+| `ignoreHashedChunks`          | boolean  | `true`           | Drop content-hashed bundler chunks matched by `./*`                              |
+| `acknowledgedChanges`         | string[] | `[]`             | Breaking changes you've verified safe (downgraded + tagged)                      |
+| `resolvableSpecifiers`        | string[] | `[]`             | Module-specifier globs to exempt from the unresolvable-reference guard           |
+| `downgradeRepairedReferences` | boolean  | `true`           | Report a blocked-to-exported specifier swap (a reference repair) as non-breaking |
+| `ai`                          | object   | unset            | AI reviewer options (see below)                                                  |
 
 Version-bump validation (`checkVersionBump`) compares the bump between the
 baseline and current versions against the severity of the detected changes,
@@ -254,6 +255,27 @@ escalating an otherwise non-breaking modification (say a new optional parameter)
 when its type is provably export-blocked. A brand-new export is still reported as
 an addition, not a breaking change, even when its type is unresolvable.
 
+The guard's inverse is a _repair_: the PR that fixes such a regression swaps the
+blocked specifier back to a public subpath
+(`import("@clerk/shared/_chunks/index-Cr_OtBLq").Xm` becomes
+`import("@clerk/shared/types/utils").Without`). The raw text differs, so the
+differ would flag it breaking, but the old reference was never consumable
+downstream; nobody can be broken by fixing it. When every specifier a signature
+drops is export-blocked (or chunk-shaped with the dependency unlocatable), every
+specifier it introduces provably resolves against the dependency's `exports`,
+and the signature is otherwise identical once the swapped `import("...").Name`
+units are masked (the imported alias may change with the specifier; bundlers
+minify chunk-internal names), the change is downgraded to non-breaking and
+tagged as a repaired reference in the report. The downgrade is deterministic,
+runs with or without the AI reviewer, and the AI cannot escalate it back.
+Anything beyond the swap, an introduced specifier that does not provably
+resolve, or a dropped specifier that was actually public fails the check and
+the change stays breaking. Set `downgradeRepairedReferences: false` to opt out
+(for example when `skipLibCheck` consumers who saw `any` must be treated as a
+compatibility surface). Both directions surface their exports-map verdicts to
+the AI reviewer, so it judges resolvability from resolved facts rather than
+path shapes.
+
 ### AI reviewer config
 
 | Field               | Type    | Default             | Description                                                                               |
@@ -544,7 +566,7 @@ Break Check classifies each diff as one of three types.
 | Type         | Severity | What it covers                                                                                                                                                                                                                                            |
 | ------------ | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | Breaking     | Major    | Removed exports or members; required parameter added; optional parameter or property made required; a parameter's rest-ness changed; parameter or property type changed; return type changed; a member's `static`/`protected`/`abstract` modifier changed |
-| Non-breaking | Minor    | Optional parameter added; rest parameter added; required parameter or property made optional                                                                                                                                                              |
+| Non-breaking | Minor    | Optional parameter added; rest parameter added; required parameter or property made optional; a type change whose only diff is swapping an unresolvable specifier for an exported one (a reference repair, see Configuration)                             |
 | Addition     | Minor    | New exports, new interface/class members                                                                                                                                                                                                                  |
 
 The analyzer compares parameters, return types, property types, and enum values

src/analyzers/ai-analyzer.ts

@@ -235,7 +235,8 @@ Reasoning rules:
 9. Adding a *new optional* property to an object type is non-breaking, for both input and output types: existing consumers neither passed it nor relied on reading it. (This is distinct from rule 4, which is about flipping an *existing* output field to optional.)
 10. \`Pick\`, \`Omit\`, \`Partial\`, \`Required\`, and other mapped types preserve each member's optionality from the source type. Resolve the member against the referenced source definition before judging; a property newly included by a \`Pick\` (or surviving an \`Omit\`) is optional in the result whenever the source declares it optional, so do not treat it as required unless the source does.
 11. Adding a *required* property to a type is breaking only when consumers construct or assign values of that type (a parameter / input position, or an object literal they author against the type). For a type consumers only read (a return / response / output type, e.g. the resolved value of a \`Promise<T>\` return or a hook result field), adding a property is non-breaking. Determine the direction from the "Usage sites" block: a \`Promise<T>\` result, a function return, or a result field is output. If any usage is an input position, or no usage sites are shown, keep "breaking".
-12. A reference whose module specifier is not a public, exported entry point of its package is breaking regardless of structural shape: the consumer cannot resolve the module, so the type degrades to \`any\` (skipLibCheck) or fails to compile (TS2307). Treat a specifier that contains a \`/_chunks/\` segment, ends in a content-hashed bundler chunk basename (a \`-<hash>\` suffix), or names a subpath a package blocks/omits in its \`exports\` as non-resolvable. Do NOT downgrade such a change on structural-equivalence grounds (rule 2 does not apply): "the shape is identical" is true but irrelevant when the consumer never receives the type.
+12. A reference whose module specifier is not a public, exported entry point of its package is breaking regardless of structural shape: the consumer cannot resolve the module, so the type degrades to \`any\` (skipLibCheck) or fails to compile (TS2307). Treat a specifier that contains a \`/_chunks/\` segment, ends in a content-hashed bundler chunk basename (a \`-<hash>\` suffix), or names a subpath a package blocks/omits in its \`exports\` as non-resolvable. Do NOT downgrade such a change on structural-equivalence grounds (rule 2 does not apply): "the shape is identical" is true but irrelevant when the consumer never receives the type. When a change carries a \`referenceResolutions\` list, entries with \`"deterministic": true\` were resolved against the dependency's actual \`exports\` map: \`blocked\` means consumers cannot resolve the specifier, \`exported\` means it is a public entry point. Trust those over any guess from the specifier's path shape. An \`"unknown"\` verdict verified nothing (dependency not installed, no \`exports\` map, or a relative/absolute specifier); fall back to the shape heuristics above for those.
+13. The inverse swap is a repair, not a break: when a change's only difference is that a non-resolvable specifier (per rule 12) was replaced by one whose \`referenceResolutions\` verdict is \`exported\`, the old type was never consumable (it errored or degraded to \`any\`), so no well-typed consumer can be broken by the swap. Judge it "non-breaking" even though the imported alias name changed with the specifier (bundlers minify chunk-internal names, e.g. \`Xm\` for \`Without\`). A change carrying \`repairedReference\` was already verified this way deterministically; confirm it as non-breaking rather than escalating.
 
 Output protocol:
 - Always respond by calling the submit_review tool. Never reply with plain text.
@@ -538,6 +539,28 @@ export class AiChangeAnalyzer {
         continue;
       }
 
+      // The mirror guard for repaired references (issue #98): the detector's
+      // downgrade is deterministic (every dropped specifier unconsumable, every
+      // introduced one exported, signature otherwise identical), and the swap
+      // is the entire diff, so a model escalation cannot be adding information.
+      // Record the opinion without applying it; the reporter explains why.
+      const isEscalation =
+        change.type === ChangeType.NON_BREAKING &&
+        verdictType === ChangeType.BREAKING;
+      if (isEscalation && change.repairedReference) {
+        enriched.push({
+          ...change,
+          aiAnalysis: {
+            source: "ai-suggested-escalation",
+            confidence: clamp01(v.confidence),
+            rationale: v.rationale,
+            migration: undefined,
+            model: this.model,
+          },
+        });
+        continue;
+      }
+
       const overrode = verdictType !== change.type;
       if (overrode) this.overriddenCount += 1;
       const aiAnalysis: AiAnalysis = {
@@ -610,6 +633,16 @@ export class AiChangeAnalyzer {
       // referenced type definitions ride in the surface block.
       beforeSnippet: capSnippet(c.beforeSnippet),
       afterSnippet: capSnippet(c.afterSnippet),
+      // Deterministic exports-map verdicts for dropped/introduced specifiers
+      // (rules 12/13), so the model never guesses resolvability from path
+      // shapes. Only present when the specifier sets differ, so the common
+      // change pays no tokens for them.
+      ...(c.referenceResolutions
+        ? { referenceResolutions: c.referenceResolutions }
+        : {}),
+      ...(c.repairedReference
+        ? { repairedReference: c.repairedReference }
+        : {}),
     }));
 
     // Compact JSON (no indentation): this block is in the non-cached part of

src/config.ts

@@ -128,6 +128,19 @@ export const ConfigSchema = z.object({
    */
   resolvableSpecifiers: z.array(z.string()).default([]),
 
+  /**
+   * Downgrade reference *repairs* to non-breaking. A repair is the inverse of
+   * the unresolvable-reference guard: a breaking modification whose only diff
+   * is swapping specifiers consumers could not resolve (export-blocked or
+   * internal bundler chunks) for deterministically exported ones, leaving the
+   * signature otherwise identical. The before state already errored (TS2307)
+   * or degraded to `any` downstream, so the swap strictly improves
+   * resolvability. On by default; set to `false` to keep reporting repairs as
+   * breaking (e.g. when `skipLibCheck` consumers who saw `any` must be treated
+   * as a compatibility surface).
+   */
+  downgradeRepairedReferences: z.boolean().default(true),
+
   /** Optional AI analyzer configuration. */
   ai: AiConfigSchema.optional(),
 });
@@ -156,6 +169,7 @@ export function createDefaultConfig(): BreakCheckConfig {
     ignoreHashedChunks: true,
     acknowledgedChanges: [],
     resolvableSpecifiers: [],
+    downgradeRepairedReferences: true,
   };
 }