refactor(models): fail closed on invalid saved routing

[decode2]

Reviewer path

Review this bounded slice in this order:

1. extensions/gentle-ai.ts: verify that saved-routing apply consumes the shared status-carrying authority and returns invalid source status and path before any profile mutation.
2. tests/model-routing-authority.test.ts: verify invalid project and global sources, no fallback, zero mutator calls, unchanged profile bytes and mtime, and unchanged missing and valid behavior.
3. tests/gentle-ai.test.ts: verify the same fail-closed behavior through the production startup path.
4. Use the verification table, strict TDD evidence, and chain boundary below to confirm the slice is complete without pulling later work into this review.

Closes #396

Type

• Bug fix
• New feature
• Documentation only
• Code refactoring
• Maintenance/tooling
• Breaking change

Summary

• Makes saved-routing apply consume the shared status authority delivered by refactor(models): isolate saved-routing read authority #389 and merged refactor(models): extract saved-routing authority #398.
• Preserves invalid source status and path while stopping before any profile mutation.
• Proves invalid project and global sources never reach the profile mutator while missing, valid, inherited, and omitted behavior remains unchanged.

Current-main baseline

• main is eb8448e3c9e8d2367eefe34fc731f06655e6d539.
• The verified PR remote head is cc46e6a9c288d4233fa626d7008d4c65eb6fd802.
• The ordered remote-head parents are f9498e9d0b189196aad81117a0e00862e9017412 and eb8448e3c9e8d2367eefe34fc731f06655e6d539.
• Merged refactor(review): consume last-event closure #411 is part of the current-main baseline and supersedes the retired restart and parity dependency assumptions. This PR has no dependency on that retired work.
• PR refactor(models): fail closed on invalid saved routing #408 is open, clean, mergeable, and has exactly one label: type:refactor.

Changes

File	Change
extensions/gentle-ai.ts	Routes saved apply through the status-carrying authority and returns invalid source paths before applying profiles.
tests/model-routing-authority.test.ts	Covers invalid project and global sources, no fallback, zero mutator calls, unchanged profile bytes and mtime, and preserved valid behavior.
tests/gentle-ai.test.ts	Proves invalid saved routing reaches the existing production startup path without profile mutation.

Chain context

#381 machine-readable contract tracker
└─ #382 internal authority foundation tracker
   └─ #389 / #398 shared read authority (delivered)
      └─ 📍 #396 / PR #408 fail-closed saved apply and profile safety
         └─ #395 direct export and UI/lifecycle consumers (approved, blocked until #396 merges)
            └─ #391 canonical names and explicit target authority (approved, blocked until #395 merges)
               └─ dedicated durable-write slice (no approved issue number yet)
                  └─ dedicated materialization slice (no approved issue number yet)
                     └─ re-frozen #383, then re-frozen #384

• Start: The shared read authority delivered by refactor(models): isolate saved-routing read authority #389 and merged through refactor(models): extract saved-routing authority #398 on current main.
• End: Fail-closed saved apply and profile no-write safety only.
• Current boundary: refactor(models): isolate fail-closed saved-routing apply #396 is the current implementation slice. refactor(models): migrate routing UI and lifecycle consumers #395 remains blocked until refactor(models): isolate fail-closed saved-routing apply #396 merges, and refactor(models): enforce canonical routing names and target authority #391 remains blocked until refactor(models): migrate routing UI and lifecycle consumers #395 merges.
• Follow-up: The durable-write and materialization work must receive separate approved issues before implementation. feat(models): add headless inspect and validate contract #383 and feat(models): add apply CLI and package executable #384 remain on hold until the complete refactor(models): share safe routing persistence authority #382 foundation is finished and their contracts are re-frozen.

Out of scope

• Direct export and UI/lifecycle consumer migration in refactor(models): migrate routing UI and lifecycle consumers #395.
• Canonical routing names and explicit target authority in refactor(models): enforce canonical routing names and target authority #391.
• Durable writes and downstream materialization.
• The standalone machine-readable executable and process contract tracked by feat(models): expose a machine-readable routing contract #381.
• Parser or grammar changes and the superseded refactor(models): centralize routing persistence authority #385 candidate.
• Any implementation based on the current stale feat(models): add headless inspect and validate contract #383 or feat(models): add apply CLI and package executable #384 bodies.
• Merge approval or landing decisions.

Strict TDD evidence

• RED: With the connected mutator seam removed, the valid-authority test expected one mutator call and observed zero.
• GREEN: tests/model-routing-authority.test.ts passed 3/3 and tests/gentle-ai.test.ts passed 9/9. The full pnpm test result is recorded below.
• TRIANGULATE: Coverage includes invalid JSON, non-object and null values, invalid-global no-fallback, missing, valid, literal inherit, omission, startup reachability, profile bytes and mtime preservation, and zero invalid mutator calls.
• REFACTOR: Production callers retain the real applyModelConfigAsync default. No consumer migration or write redesign was introduced.

Verification

Check	Result
Remote head and base	cc46e6a9c288d4233fa626d7008d4c65eb6fd802 verified against main at eb8448e3c9e8d2367eefe34fc731f06655e6d539 with the ordered parents above
Authority-focused tests	3/3 pass
gentle-ai focused tests	9/9 pass
pnpm test	1034 total, 1033 pass, 1 skip, 0 fail
pnpm run check:runtime-modules	4 runtime modules match
Syntax checks	3/3 changed paths pass
git diff --check	Pass
Required CI	verify pass
Hashes and status	Clean

Review workload

• Changed files: 3.
• Additions and deletions: 211 additions + 4 deletions = 215 A+D.
• Review budget: 215 A+D, within the 400-line limit.
• No size:exception.

AI assistance

Material assistance was used.

• Tool/model: el Gentleman on Pi with delegated implementation and independent verification workers.
• Material scope: authority analysis, strict-TDD implementation, differential baseline diagnosis, test design, and PR preparation.
• Verification performed: reconstructed RED, focused GREEN, current-main verification, scope audit, runtime-module parity, syntax checks, diff validation, and full-suite verification.

Contributor checklist

• Linked approved issue refactor(models): isolate fail-closed saved-routing apply #396 with Closes #396.
• Added exactly one PR type label: type:refactor.
• Kept one bounded work unit with its behavior tests.
• Used a Conventional Commit message.
• Added no Co-Authored-By trailers.
• Material AI assistance is disclosed above.
• Stayed within 400 A+D.
• Kept refactor(models): migrate routing UI and lifecycle consumers #395, refactor(models): enforce canonical routing names and target authority #391, the dedicated write and materialization slices, and stale feat(models): add headless inspect and validate contract #383/feat(models): add apply CLI and package executable #384 implementation out of scope.

Readiness and merge boundary

Ready status is not merge approval. Normal maintainer review and merge controls still apply, and this body makes no merge claim.

Summary by CodeRabbit

• Bug Fixes

  • Invalid model-routing configurations are now rejected safely without overwriting existing agent profiles.
  • Missing, inherited, null, and valid routing settings are handled consistently.
  • Startup warnings clearly indicate when a project configuration cannot be applied.

• Tests

  • Added coverage for configuration validation, profile preservation, timestamps, agent-file updates, and startup behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fad77a78-c950-4959-b25f-2a5e83f51563

📥 Commits

Reviewing files that changed from the base of the PR and between f9498e9 and cc46e6a.

📒 Files selected for processing (2)

• extensions/gentle-ai.ts
• tests/gentle-ai.test.ts

💤 Files with no reviewable changes (1)

• extensions/gentle-ai.ts

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

applySavedModelConfig now reads model routing through the authoritative reader and supports an injected async applier. Tests cover invalid, missing, valid, null, and inherit configurations, profile preservation, agent updates, and startup warnings.

Changes

Saved routing apply

Layer / File(s)	Summary
Authority-driven model configuration apply extensions/gentle-ai.ts	applySavedModelConfig uses the authoritative model-routing reader and accepts an optional injected asynchronous configuration applier.
Apply and startup safety coverage tests/model-routing-authority.test.ts, tests/gentle-ai.test.ts	Tests verify invalid-source fail-closed behavior, unchanged profile contents and timestamps, valid and missing routing behavior, injected application, and startup warning output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to cc46e

The saved-routing change is covered by passing focused and full checks, and no actionable merge-blocking risk remains.

Suggested reviewers: alan-thegentleman

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The implementation and tests address #396 by using the authority result, preventing fallback and profile mutation for invalid routing, and preserving valid and missing behavior.
Out of Scope Changes check	✅ Passed	The changes remain within #396, covering the apply path and profile-safety regression tests without introducing later-slice functionality.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: failing closed when saved model routing is invalid.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[decode2]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Alan-TheGentleman]

Verified against current main at head cc46e6a: on main the global-invalid path already failed closed but a project-invalid models.json collapsed to a valid empty config and applied silently, and this slice makes applySavedModelConfig consume the shared status-carrying authority so exactly that case now returns invalidPath before any profile mutation. Downstream consumers session_start and ensureSddPreflight already handle invalidPath. Tests pin invalid project and global shapes with zero mutator calls and byte-identical profiles through the real mutator. Merging. The remaining silent-fallback consumers (module wrappers, export, models panel, status doctor) are #395's scope as the issue declares.