Skip to content

fix(provider): validate native NaN login and retain offline models (2/2) - #1571

Merged
decode2 merged 4 commits into
mainfrom
fix/nan-provider-02-native-auth-offline
Sep 30, 2026
Merged

decode2 merged 4 commits into
mainfrom
fix/nan-provider-02-native-auth-offline

Conversation

@decode2

@decode2 decode2 commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Closes #1569

Depends on #1570. This PR targets its parent branch to keep the review at 307 changed lines. Land #1570 first, then retarget/rebase this PR to main; do not merge it into the parent feature branch.

PR type

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

Summary

  • Use Pi's native Provider API for explicit API-key login, rejecting empty/whitespace input, trimming valid input and honoring cancellation and credential precedence.
  • Publish catalogs synchronously and keep all seven documented chat models in the cold/offline fallback, without offline network requests.
  • Preserve authoritative successful key-scoped catalogs, including empty results, on same-key failures/offline refreshes; reset safely when credentials change.

Changes

File Change
extensions/nan-provider.ts Native Provider registration
lib/nan-provider.ts Validated auth, synchronous publication and complete offline baseline
tests/nan-provider.test.ts Login, precedence, publication, offline retention and clone-safety regressions
tests/runtime-harness.mjs Native registration assertions
README.md Login/setup and offline support versus account entitlement

Test plan

  • node --experimental-strip-types --test tests/nan-provider.test.ts: 20 passed.
  • pnpm run typecheck: baseline gate passed, 187 recorded diagnostics, no regressions.
  • env -u GENTLE_PI_AGENTS_CHILD -u GENTLE_PI_CONFIG_HOME pnpm test: 4,054 passed, 50 skipped, zero failures; provider-contract/runtime-harness passed.
  • git diff --check: passed.
  • Fresh independent verification used an isolated offline, frozen-lockfile dependency installation.
  • Native reliability review approved and acknowledged. One informational warning at lib/nan-provider.ts:114-118 is non-blocking and opened no correction.
  • Skipped environment-specific checks and live API/auth behavior were not rerun on this branch.

Earlier same-source validation covered eight isolated Pi 0.87.1 direct/picker login cases with synthetic credentials, seven available offline models with no network requests, and one successful authenticated nan/deepseek-v4-flash inference. The user also confirmed model selection works. Those are prior/manual results, not claims of fresh live calls in this delivery run.

The offline catalog declares documented model support, not account entitlement. A successful live catalog remains authoritative. MCP search/media bridges are not implemented.

Contributor checklist

  • Linked an approved issue with the human-selected closing reference.
  • Exactly one type:* label: type:bug.
  • Shellcheck/skill-runtime checks are not applicable: no shell scripts or skills changed.
  • Tests and documentation accompany their behavior.
  • Conventional commits, no attribution trailers.
  • No local task artifacts, personal launcher/configuration or credentials in the diff.

Chain context

Field Value
Chain First-party NaN provider
Position 2 of 2
Base feat/nan-provider-01-connection at 282cce68
Depends on #1570
Follow-up None in this model-provider integration
Review budget 268 additions + 39 deletions = 307 / 400 lines
Starts at Initial provider connection from #1570
Ends with Validated native login and seven-model offline fallback
main
  └── #1570: first-party connection
        └── 📍 This PR: native login and complete offline catalog

Includes: native authentication, publication/offline regressions, tests and documentation. Excludes: MCP tools, usage-layer changes, personal launcher/configuration and unrelated provider changes. Both slices were tested independently; reverting this slice restores the initial connection without removing it. No auto-merge is requested.

Summary by CodeRabbit

  • New Features
    • NaN sign-in now accepts /login or /login nan. API keys are trimmed before saving; blank input is rejected, and cancelling leaves the saved key unchanged. Saved keys take precedence over NAN_API_KEY.
    • Before model discovery succeeds, NaN offers seven documented chat models offline. After a successful refresh, the available models reflect the live catalog, including when it is empty. Changing credentials restores the offline model list until discovery succeeds.
    • The documented output-token limit is now 8,192.

@decode2 decode2 added the type:bug Bug fix label Sep 30, 2026
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e30b07db-4f53-424d-990d-a661869bc45a

📥 Commits

Reviewing files that changed from the base of the PR and between 664bfdd and 460341e.

📒 Files selected for processing (5)
  • README.md
  • extensions/nan-provider.ts
  • lib/nan-provider.ts
  • tests/nan-provider.test.ts
  • tests/runtime-harness.mjs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

NaN now registers as a native Pi provider with API-key login, credential resolution, host-native streaming, and a credential-scoped model catalog. Its documented offline fallback includes seven chat models. Successful discovery publishes the current catalog, including an empty result.

Changes

NaN provider integration

Layer / File(s) Summary
Native provider and authentication
lib/nan-provider.ts, extensions/nan-provider.ts, tests/nan-provider.test.ts, tests/runtime-harness.mjs, README.md
The provider uses Pi’s native registration shape. Login validates and trims submitted keys, and stored keys take precedence over NAN_API_KEY. Tests cover registration and authentication. The README documents login routes, key handling, the 8,192-token output cap, and model availability.
Model catalog and refresh
lib/nan-provider.ts, tests/nan-provider.test.ts
The offline catalog contains the seven documented chat models. Current, usable discovery results replace the catalog, including an empty result. Tests cover offline, aborted, stale, and changed-key refreshes, as well as catalog snapshots.

Priority: ⬇️ Low

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Pi
  participant NaNProvider
  participant NaNModelDiscoveryAPI
  Pi->>NaNProvider: Refresh provider models
  NaNProvider->>NaNModelDiscoveryAPI: Fetch models using current credentials
  NaNModelDiscoveryAPI-->>NaNProvider: Return discovered models
  NaNProvider-->>Pi: Publish current catalog update
Loading

Suggested reviewers: alan-thegentleman

Merge Risk: ⚪ Minimal · up to 46034

NaN becomes a native Pi provider with validated API-key login and a seven-model offline fallback. The reported checks passed and no merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 46034

Credential validation, fixed destinations, and cancellation controls limit the risk. The remaining uncertainty is whether authentication and catalog refresh consistently use the same account identity, particularly with environment-provided credentials.

Retained concerns

  • Low · architecture · inferred: The native authentication and discovery paths do not locally establish one effective credential identity. Authentication trims stored keys and falls back to NAN_API_KEY, while discovery keys its requests and retained catalog directly from context.credential. Unless the host supplies the resolved credential, environment-only authentication can diverge from discovery, and environment-key changes need not invalidate retained catalog state. The host handoff is unverified; this is a bounded ownership-contract concern, not a demonstrated authorization bypass.
Security review details

Security Blast Radius

  • inferred — The inspected discovery path exposes the active credential to the fixed NaN service and changes catalog metadata within one provider instance. A discovery response cannot supply another inference endpoint or API through this implementation. Production streaming credential enforcement remains outside the hydrated evidence.

Trust Boundaries and Controls

  • observed — Explicit login uses a secret prompt and checks cancellation before and after input. Blank credentials are rejected. Discovery rejects redirects and catches failures without logging request or response data. Tests assert that failed login preserves an existing stored credential.

Resilience and Maintainability Implications

  • observed — Offline or already-aborted refreshes make no discovery request. Online discovery propagates cancellation, applies a timeout, and removes its abort listener and timer during cleanup. Catalog snapshots clone mutable fields, preventing callers from changing another snapshot or provider's baseline.
  • inferred — The local revision guard distinguishes credential changes, not overlapping refreshes for the same credential. Without host serialization or cancellation, an older same-key response can publish after a newer response. This is an unresolved catalog-freshness limitation, not evidence of an entitlement bypass.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: native NaN login validation and retention of offline models.
Linked Issues check ✅ Passed Issue #1569 requires native API-key login, key-scoped model discovery, seven-model offline fallback, host-native streaming, tests, and setup documentation. lib/nan-provider.ts implements native logi…
Out of Scope Changes check ✅ Passed The changed files support issue #1569. The extension registration update enables native provider registration. The README update documents the new login and catalog behavior. The test and runtime-harn…
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

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

decode2 added a commit that referenced this pull request Sep 30, 2026
Refs #1569

First-party connection, documented chat discovery, tests and setup documentation. Follow-up: #1571.
@decode2
decode2 changed the base branch from feat/nan-provider-01-connection to main September 30, 2026 03:05
@decode2
decode2 merged commit 4b6b148 into main Sep 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(provider): add first-party NaN model integration

1 participant