Port Nous Portal subscription credits

[Finesssee]

Summary

• Add the Nous Portal subscription provider from upstream 0.61.0.
• Read Hermes Agent credentials from the configured profile without refreshing or writing them; support NOUS_PORTAL_ACCESS_TOKEN as an explicit environment override.
• Enforce HTTPS and trusted nousresearch.com routing for stored portal origins.
• Parse monthly subscription, rollover, purchased, and total usable credits with renewal and account identity details.
• Register Nous across the Rust provider catalog, factory, CLI aliases, OAuth source policy, tray/dashboard catalogs, and provider icons.
• Bound streamed portal responses incrementally so oversized chunked responses cannot be buffered without a limit.

Validation

• cargo test --manifest-path rust/Cargo.toml nous -- --nocapture — 11 passed
• cargo test --manifest-path rust/Cargo.toml --lib --quiet — 1,937 passed, 1 ignored
• cargo test --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml commands::tests --quiet — 94 passed
• cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings — passed
• cargo clippy --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml --all-targets -- -D warnings — passed
• cargo fmt --all and git diff --check — passed

Frontend Vitest and fresh Tauri UI/CUA proof were not run because this checkout has no apps/desktop-tauri/node_modules; installing dependencies would consume the storage budget during the port batch. Hosted CI should provide the frontend validation.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 02823452-63b1-483a-a565-7f3bda3ffae1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.}

[Finesssee]

Thermo-Nuclear Review: PR #569 — Port Nous Portal subscription credits

Verdict: REQUEST CHANGES

Structural regressions

1. The credential_pool selection logic is an embedded policy engine inside a provider module. select_pool_credential implements a three-level comparator (agent expiry → access expiry → priority, with tie-breaking on lower priority winning) as a hand-rolled Option<(StoredCredential, i64, i64, i64)> tuple-mutation loop with a nested is_none_or closure containing a three-way boolean expression. This is the single densest function in the series: a comparator written as mutating should_replace booleans is exactly the "reframe the state model so conditionals disappear" case. A code-judo move: derive a max_by_key-style comparison from a (agent_expiry, access_expiry, Reverse(priority)) tuple — entries.iter().filter_map(...).max_by_key(|(s, e, a, p)| (*agent, *access, Reverse(*priority))) — and the nested conditional collapses to one ordering tuple. The current form is a bug magnet (note the tuple is (StoredCredential, agent, access, priority) while should_replace destructures as (_, old_agent, old_access, old_priority) — one swapped field from a silent mis-selection).

2. is_truthy is a JS-flavored truthiness function in Rust. Value::Number → != 0.0, non-empty string → true, arrays/objects → true. The portal reports an error via a truthy root field, so {"error": []} or {"error": "0"}... wait, "0" is a non-empty string → truthy → error reported. And {"error": 0} → not truthy → ignored. This is a magic-behavior boundary: the actual API presumably returns a string or object error; define exactly which shapes mean "error present" (e.g. not null or false or empty) instead of importing JS semantics. Port Muse Code subscription usage #568 chose the opposite policy (optional_bool — strict, errors on wrong type). Two opposite truthiness policies introduced by the same series in sibling files is unmanaged design drift.

3. Fourth bounded-body reader copy (read_bounded_body/append_bounded_body), byte-identical to Port Muse Code subscription usage #568's. Two identical private copies introduced in sibling PRs of the same series must be one shared helper — see series notes.

Missed simplification opportunities (code-judo)

1. parse_response is 200 lines with five field-ladders, each doing number(optional_object(...).and_then(|v| v.get("...")), "dotted.field.name")?. Same finding as Port Muse Code subscription usage #568: a fn field(obj, name) helper removing double-naming, plus the two-lane fallback pattern (subscription.credits_remaining or paid_service_access.subscription_credits_remaining, same for purchased credits) repeats twice — one first_number([&sub, &access], "credits_remaining") walker deletes both ladders.

2. resolve_credential_from's candidate loop with saw_file + expired: Option<Credential> flags mirrors Port Muse Code subscription usage #568's resolver shape but re-rolls its own state. Both resolvers share the skeleton (explicit → env → files in precedence order → distinct error per terminal state). A shared "credential source chain" helper (like Port Hugging Face billing usage #567 correctly reuses for resolve_api_key) would give all token-file providers the same fail-closed ladder with per-provider extraction.

3. format_usd here has a < 100.0 two-decimal switch — the third private currency formatter in the series (Port Venice web subscription credits #565 format_credits, Port Hugging Face billing usage #567 same, here with a different rounding edge). Three rounding policies for "display dollars" is three opinions; one shared formatter.

4. envs().collect::<HashMap>() per fetch — identical to Port Muse Code subscription usage #568's finding, same fix.

Spaghetti / branching complexity

• parse_auth_file is a three-shape fallback (providers.nous → credential_pool.nous[] → bare root) — three auth-file shapes in one parser. Understandable (Hermes evolved), but the three-way shape sniffing is a compatibility ladder with no comment pinning which shape is current vs. legacy. One sentence each prevents archaeology.
• normalized_https_url's six-condition rejection chain (scheme, host, username, password, query, fragment, path) is explicit and readable — good. But note it duplicates intent with base-branch validated_https_url (which already exists in providers/mod.rs and rejects encoded-host evasion that this one misses, e.g. %2f). The local copy is weaker than the canonical one for the same job — direct skill-rule-6 violation: reuse the canonical helper, add the trust-domain check on top.

Boundary / abstraction / type problems

1. is_trusted_portal_host allow-lists nousresearch.com + subdomains for stored origins, but env overrides (NOUS_PORTAL_BASE_URL, HERMES_PORTAL_BASE_URL) bypass the trust check entirely (test pins https://localhost:1234 accepted from env). The trust boundary is therefore "whoever can set the env var wins" — fine for a local desktop app, but the asymmetry (stored → allow-listed, env → anything) should be stated in the comment; as written, a reader assumes the allow-list is the security boundary when it is only half of it.

2. Credential vs StoredCredential split is honest (URL + expiry normalization vs. raw file shape) — good. No objection. But expires_at: Option<DateTime> being None → unwrap_or(0) in the pool comparator means "no expiry" sorts lowest priority — i.e. a credential with unknown expiry loses to any credentialed expiry, even a stale one. If intended (unknown expiry is risky), comment it; if not, it's a silent mis-ranking.

3. parse_response mixes two response dialects (subscription.* vs paid_service_access.*) with fallback chains and an "all-none → parse error" gate. The gate is the right invariant, stated once — good. But remaining falling back across dialects while monthly doesn't (monthly only from subscription) means the monthly_credits: 0 test path silently downgrades to informational with only total_usable — behavior covered by tests, accepted.

File-size / decomposition concerns

• rust/src/providers/nous/mod.rs = 962 lines — ~40 lines under the 1k threshold, and this PR is the largest of the series. ~450 of those lines are tests, so ~510 lines of logic, but the file now contains: credential resolution (env + file + pool + JWT expiry), URL trust policy, HTTP fetch, response parsing, and display projection. That's five responsibilities at 962/1000. Any follow-up (adding a refresh path, a second endpoint) crosses the bar. Recommend decomposing now while the seam is fresh: credentials.rs (resolve + pool + JWT) and mod.rs (provider + parse + project) is the obvious two-file split and costs nothing behaviorally.
• format_usd max-0.0 clamp then two-tier decimals — covered above.

Lower-priority notes

• format_month_day uses %b + day-of-month — locale-dependent rendering ("Sep 18") baked into a non-i18n path; consistent with rest of app, fine.
• status_error maps UNAUTHORIZED → OAuthExpired (distinct variant from Port Hugging Face billing usage #567's AuthRequired for the same status). If OAuthExpired triggers a distinct UI affordance (re-login prompt), that's right for a refreshable Hermes login — but the series now has three different 401 mappings across providers. Decide the convention once.
• jwt_expiry re-implements base64url payload decoding that codex_accounts::credentials::jwt_payload already provides (canonical, handles padding; this one tries NO_PAD then padded). One canonical JWT helper should serve both; the fallback-to-padded branch is speculative.

Series note (applies to #565–#569 collectively): this batch is five independent ports sharing: (a) private bounded-body readers ×2 identical, (b) private credential resolvers ×3 same skeleton, (c) private number formatters ×3 different rounding, (d) private URL validators ×2 with the local one weaker than the canonical, (e) per-provider usageSourcePolicy.ts + HAS_DASHBOARD registry churn, (f) provider.rs all()-count test bumps that break pairwise merges. The single highest-value follow-up: extract providers/http.rs (bounded read + status classification) and providers/credentials.rs (env/file resolution ladder + JWT expiry + quote trimming), then port each provider to them — deleting roughly 300 lines of copy-paste across the batch.