fix(onboarding): load pasted custom-provider credentials

[sj0618]

What

• Add an explicit apiKeyStored: true credential-source marker for custom models whose key lives in GJC credential storage.
• Make env-backed provider replacement remove stale stored credentials, then reload the active credential cache for both env and literal setup.
• Cover strict validation, secure persistence, literal-to-env replacement, immediate availability, restart loading, docs, changelog, and the generated docs index.

Why

Fixes #3738.

The wizard already stored pasted API keys outside models.yml, but the generated provider entry did not declare that credential source. Static config validation therefore rejected the wizard output.

A force replacement from a pasted key to apiKeyEnv also left the stored key active. Because stored credentials outrank environment values, the old secret continued to win. Env setup now removes that stored credential, and the running session reloads the canonical store before refreshing models.

Hand-authored custom models with no apiKey, apiKeyEnv, or stored-key marker remain invalid.

Testing

• bun test packages/coding-agent/test/provider-onboarding.test.ts packages/coding-agent/test/provider-onboarding-wizard.test.ts (29 passed)
• bun test packages/coding-agent/test/provider-onboarding.test.ts packages/coding-agent/test/provider-onboarding-wizard.test.ts packages/coding-agent/test/model-registry.test.ts --test-name-pattern "stored API key|declared credential source|provider onboarding" (32 passed)
• bun test packages/coding-agent/test/docs-index-lazy.test.ts (5 passed)
• bun --cwd=packages/coding-agent run check

GJC verdict

gajae.pr-review-verdict.v1 needs-human sha256:f55cd7fb4779f31748ef5bb293ef6407650f5655 reviewer:human evidence:needs-independent-review

---

• Target branch is dev
• bun check passes
• Tested locally
• CHANGELOG updated (if user-facing)
• Verdict above matches the exact PR head, not an earlier commit

[Yeachan-Heo]

Verdict: REQUEST_CHANGES

Exact-head review completed against the current PR head and current dev base.

• PR head: 0581f2c0009fb05fbd2c714a6cc2032e83f9feb1
• Current dev base: 2b806cc7809aa828323b04e4e86cbe242decdff7
• Full 10-file diff reviewed, including the credential marker, persistence, reload, selector flow, error handling, documentation, generated docs index, and tests.

Blocking finding: stale stored credential survives switching the same provider to env auth

addApiCompatibleProvider() now persists pasted literal keys in the canonical AuthStorage database, but the apiKeyEnv branch does not remove an existing stored key. On a force overwrite of an already-configured provider, changing from a pasted literal key to an environment-backed key therefore leaves the old database credential active:

1. Configure provider p with a pasted key. The new code stores it with AuthStorage.set() and writes apiKeyStored: true.
2. Run the wizard again for p with apiKeyEnv: P_KEY and force: true.
3. The replacement models.yml contains apiKeyEnv and no apiKeyStored, but the old p API-key row remains in AuthStorage.
4. AuthStorage.getApiKey() resolves stored API keys before environment variables, so requests continue using the old pasted secret rather than P_KEY.

This is a credential-source/config-persistence mismatch and can silently send requests with a revoked or unintended secret. The reverse transition (env to literal) uses replaceAuthCredentialsForProvider() and replaces the stored rows, so the asymmetry is specifically in the env branch. Please clear/remove the provider's stored credentials when replacing a literal setup with an env-backed setup, and add a force-overwrite regression test asserting the selected key is the environment key and the old stored row is gone.

#3738 overlap

The original #3738 reproducer remains correctly rejected: auth: apiKey without apiKey, apiKeyEnv, or apiKeyStored: true still fails validation. The new marker fixes the wizard-generated literal-key path, but it does not address the stale-credential overwrite defect above.

Verification

Current-head GitHub CI is green: the status query reports no non-passing checks; completed checks are SUCCESS and only inapplicable jobs are SKIPPED. No source files were modified and no merge was performed during this review.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[sj0618]

Addressed the blocking literal-to-env credential transition finding on the latest dev base.

• 03f04a414: env-backed setup now removes the provider stored credential; the core force-overwrite regression verifies a fresh store has no old key and resolves the environment value.
• f55cd7fb4: the live wizard now reloads auth storage for env and literal setup; the integration test seeds a stale in-session key and verifies the environment value wins immediately.
• Local verification: 29 onboarding tests passed, 32 focused onboarding/model-registry tests passed, and the coding-agent package check passed.

Please rerun the exact-head review against f55cd7fb4779f31748ef5bb293ef6407650f5655 with base be3940aa72f08bfce43b4e5a72b5d6f3260e3ba5.

[Yeachan-Heo]

Signed exact-head verdict — REQUEST_CHANGES

Reviewed against the live GitHub state on 2026-08-05:

• PR head: f55cd7fb4779f31748ef5bb293ef6407650f5655
• recorded PR base: be3940aa72f08bfce43b4e5a72b5d6f3260e3ba5 (dev at the last rebase)
• current dev: 732856b3ccb3fade6e9fbc17908a4fbca5a7682f
• integration state: draft, dirty, 6 commits ahead / 56 behind, not rebaseable
• exact-head CI: Dev CI and Public site sync completed successfully: 18 successful jobs, 6 intentional skips, 0 failures, 0 pending

That CI is valid evidence for this exact head on its old base. It is not current-dev integration evidence.

Contributor ledger

The contributor record is substantial: GitHub search returns 51 authored PRs (25 merged, 5 open, 21 closed without merge) plus 11 issues; the contributors endpoint credits 24 repository contributions. On this PR, the contributor responded to the prior stale-credential blocker within five minutes, added normal-path persistence/reload coverage, kept pasted secrets out of models.yml, updated the public schema/docs/generated index, and supplied green exact-head CI. Those strengths deserve explicit credit. They do not close the following auth/data-integrity blockers.

Blocking findings

1. P1 — credential/config replacement is not failure-atomic. In the exact-head provider-onboarding.ts hunk, AuthStorage.remove() or set() completes before writeModelsConfig(). If the YAML replacement fails, stored→env deletes the working credential while the old apiKeyStored: true config remains; env→stored can replace the credential while the old config remains. The YAML write is atomic by itself, but the SQLite/YAML transition has no compensation or durable recovery. Add injected write-failure and retry/interruption coverage for both directions.

2. P1 — the wizard writes through the wrong credential authority in broker mode. addApiCompatibleProvider() hardcodes AuthStorage.create(getAgentDbPath()). Broker-backed sessions use RemoteAuthCredentialStore; reloading that live store fetches the broker snapshot, not the local database the helper just changed. A pasted-key wizard run can therefore persist apiKeyStored: true while the active/broker credential store has no key. Inject and mutate the session's active, broker-aware credential backend; add broker-backed setup and restart coverage.

3. P1 — env replacement deletes the whole provider credential set. AuthStorage.remove(providerId) disables every stored credential for the resolved provider, not only the wizard-owned stale API key. Onboarding accepts arbitrary normalized provider IDs and does not reserve bundled IDs or aliases, so a custom-provider collision can erase unrelated OAuth or multi-account credentials. Namespace/reject collisions or track a specific onboarding-owned credential and replace only that record; test bundled IDs, aliases, OAuth rows, and multiple credentials.

4. P1 — cache activation is incomplete across call sites. The new authStorage.reload() exists only in the TUI wizard. Both /provider add handlers call the same mutation helper and then only ModelRegistry.refresh(), which does not reload AuthStorage. A force switch to env auth can therefore leave the active session using the stale stored key until restart. Centralize mutation + reload/activation on the active store and cover every caller.

5. P1 — current-dev integration is unresolved. The exact patch no longer applies cleanly to model-registry.ts, docs/models.md, CHANGELOG.md, or docs-index.generated.ts. Current dev already merged #3757's actionable credential-source diagnostic; the rebase must preserve it while adding the stored-key form, then regenerate the embedded docs index. Fresh current-head CI is required after the rebase.

P2 follow-up: after durable writes succeed, reload/refresh/notification failures are caught and reported as Provider setup failed. Separate durable commit from activation failure and provide a non-mutating activation retry so users are not told to repeat an operation that already committed.

Required re-review boundary

Rebase onto current dev, repair the credential ownership/atomicity design, route all mutations through the active broker-aware store, cover every call site and failure phase, retain the current #3757 diagnostic, regenerate public artifacts, and obtain fresh exact-head CI. A new exact-head hostile review is required; the old green runs cannot be carried forward as approval.

—
Signed: Yeachan-Heo · repository owner · hostile exact-head review · 2026-08-05