feat(aimlapi): harden passwordless onboarding and top-up recovery - #1988
Lookoff-AIMLAPI wants to merge 17 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (6)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
⚙️ CodeRabbit configuration file
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
src/components/**/*.tsx📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe AIMLAPI integration now supports passwordless onboarding, existing-key validation and reuse, resumable card top-ups, safer HTTP handling, guided ProviderManager screens, canonical endpoint attribution, centralized CLI wiring, and updated tests and documentation. ChangesAIMLAPI client and onboarding
Resumable top-up flow
Guided provider setup
Provider metadata and CLI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/integrations/aimlapi/config.ts (1)
110-136: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEnforce the return URL safety contract.
These builders accept malformed or non-HTTP(S) environment values unchanged, despite Line 129 requiring an ordinary HTTPS page. Parse the bases with
URL, reject credentials and unsafe schemes, and test invalid overrides before sending them into checkout requests.As per path instructions, provider endpoint and outbound HTTP behavior must receive high scrutiny; the PR objective requires unsafe URLs to be rejected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/aimlapi/config.ts` around lines 110 - 136, Update buildPartnerCheckoutReturnUrls and buildPartnerReturnUrl to parse candidate base URLs with URL, accepting only HTTP(S) schemes without username or password credentials. Reject malformed or unsafe AIMLAPI_RETURN_URL and checkout bases before constructing or sending checkout requests, falling back or returning an empty result according to the existing behavior, and add coverage for invalid overrides.Source: Path instructions
src/integrations/aimlapi/prompt.ts (1)
17-33: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd focused tests for the new prompt behavior.
Cover trimmed answers, empty-answer defaulting, and
rl.close()on success/failure. The top-up suite mockspromptText, so it does not exercise this implementation.As per coding guidelines, “Add or update tests when a code change affects behavior.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/aimlapi/prompt.ts` around lines 17 - 33, Add focused tests for promptText covering whitespace-trimmed answers, empty answers falling back to defaultValue, and rl.close() being called on both successful resolution and failure. Mock the readline interface and assert the implementation’s behavior directly rather than relying on the top-up suite’s promptText mock.Source: Coding guidelines
src/main.tsx (1)
4020-4046: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd a Commander regression test for the changed CLI contract.
Exercise
aimlapi topupparsing and assert that--code,--auto-top-up,--no-open, and the new model default reachaimlapiTopup; also verify removed--methodusage is rejected. Existing top-up tests bypass this entrypoint entirely.As per coding guidelines, “Add or update tests when a code change affects behavior.” As per path instructions, entrypoint changes must be reviewed for release safety.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.tsx` around lines 4020 - 4046, Add a Commander-level regression test for the `aimlapi topup` action that invokes the actual CLI entrypoint and verifies `--code`, `--auto-top-up`, `--no-open`, and the default `--model` are passed to `aimlapiTopup`. Also assert that the removed `--method` option is rejected, keeping existing handler-level tests unchanged.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ProviderManager.tsx`:
- Around line 2567-2579: Update the aimlapi first-run environment-key path in
ProviderManager to validate authorization and balance before calling
persistAimlapiKey or setting the ready-state flags; reject invalid or
low-balance keys without reusing credentials from another provider or model. In
src/components/ProviderManager.test.tsx lines 1127-1157, test the exact
aimlapi/model path and assert validation occurs, including invalid-key and
insufficient-balance cases.
In `@src/integrations/aimlapi/client.ts`:
- Around line 295-312: Sanitize request URLs before constructing errors in the
request flow around readResponseText and the network catch, ensuring getSession,
pay, and exchange never expose checkout session tokens from path segments. Reuse
the existing exact-token redaction or origin-only request-label mechanism used
by the UI, apply it to all relevant HTTP and network error messages including
the handling around lines 317-339, and add a regression test verifying active
tokens are absent from failures.
- Around line 160-164: Update getBalance to validate the successful response
payload against the complete BalanceResult shape before returning it, including
field presence and expected types. Reject 200 responses that are empty or
malformed rather than allowing the cast result to reach callers or persist the
API key, while preserving normal valid-response handling.
In `@src/integrations/aimlapi/onboarding.ts`:
- Around line 57-68: Update the balance handling in the onboarding flow around
client.getBalance so any non-abort failure preserves the issued key but returns
an explicit unknown/error balance state rather than lowBalance: false. Propagate
that state through the returned onboarding result, and ensure ProviderManager
does not persist the key as ready unless the balance check succeeds and confirms
the balance.
In `@src/integrations/aimlapi/topup.test.ts`:
- Around line 14-16: Restore the real exports for browser.js,
providerProfile.js, and prompt.js in an afterAll hook after the top-up test
suite completes. Re-register each module’s non-mocked implementations so the
process-global mock.module overrides from topUpAimlapiByApiKey and related
imports do not affect later tests.
In `@src/integrations/aimlapi/topup.ts`:
- Around line 141-153: Update the top-up flow around provisionAimlapiKey to
persist and reload both the checkout paymentSessionId and resumeSessionToken
across CLI invocations. Replace the unconditional randomUUID payment session
with the persisted value when available, and wire the onSession callback to save
the session before any payment request begins, preserving these identifiers for
ambiguous-response retries.
- Around line 438-445: Update both polling loops in the top-up flow to use the
shared retry predicate for rate-limit errors, including HTTP 429, before
terminal-session handling. Ensure retryable 429 responses sleep and retry with
the existing session token, without invoking onSession(''); add a regression
test confirming the same session is retained and retried.
In `@src/integrations/aimlapi/validation.ts`:
- Around line 7-20: Update parseAimlapiAmountUsd to reject positive amounts with
more than two decimal places before converting dollars to minor units, rather
than allowing Math.round to move them across minimum or maximum limits. Preserve
existing validation and error behavior for other invalid amounts, and add
boundary tests covering inputs such as 19.999 and 10000.004.
In `@src/integrations/runtimeMetadata.ts`:
- Around line 51-64: Refactor the header resolution logic around the existing
baseUrl check so it branches early: preserve headers unchanged when the base URL
is non-canonical, and only filter catalog headers and inject the resolved
partner header for the canonical or missing-base-URL case. Remove the redundant
filtering and re-addition loop while preserving the existing PARTNER_HEADER_NAME
override behavior.
---
Outside diff comments:
In `@src/integrations/aimlapi/config.ts`:
- Around line 110-136: Update buildPartnerCheckoutReturnUrls and
buildPartnerReturnUrl to parse candidate base URLs with URL, accepting only
HTTP(S) schemes without username or password credentials. Reject malformed or
unsafe AIMLAPI_RETURN_URL and checkout bases before constructing or sending
checkout requests, falling back or returning an empty result according to the
existing behavior, and add coverage for invalid overrides.
In `@src/integrations/aimlapi/prompt.ts`:
- Around line 17-33: Add focused tests for promptText covering
whitespace-trimmed answers, empty answers falling back to defaultValue, and
rl.close() being called on both successful resolution and failure. Mock the
readline interface and assert the implementation’s behavior directly rather than
relying on the top-up suite’s promptText mock.
In `@src/main.tsx`:
- Around line 4020-4046: Add a Commander-level regression test for the `aimlapi
topup` action that invokes the actual CLI entrypoint and verifies `--code`,
`--auto-top-up`, `--no-open`, and the default `--model` are passed to
`aimlapiTopup`. Also assert that the removed `--method` option is rejected,
keeping existing handler-level tests unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12787270-fbb9-4d30-9572-48784b98e656
⛔ Files ignored due to path filters (1)
src/integrations/generated/integrationManifest.generated.tsis excluded by!**/*.generated.*,!**/generated/**,!src/integrations/generated/**
📒 Files selected for processing (31)
README.mddocs/aimlapi-setup.mdsrc/cli/handlers/aimlapi.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/messages.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/validation.tssrc/integrations/artifactGenerator.test.tssrc/integrations/artifactGenerator.tssrc/integrations/discoveryService.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/runtimeMetadata.tssrc/main.tsxsrc/services/api/client.test.tssrc/utils/providerDiscovery.test.tssrc/utils/providerProfiles.test.tssrc/utils/providerProfiles.tssrc/utils/status.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests (24.11.x): feat(aimlapi): harden passwordless onboarding and top-up recovery
Conclusion: failure
##[group]src/integrations/index.test.ts:
(pass) loaded registry validation > registry is valid after loading all descriptors [4.00ms]
(pass) loaded registry validation > MiniMax has shared brand and model descriptors wired to its route catalog
(pass) loaded registry validation > route catalogs do not duplicate defaultModel with catalog default flags
(pass) loaded registry validation > dynamic route catalogs rely entirely on discovery
80 | .filter(entry => !descriptorOptionalEntries.has(`${gateway.id}:${entry.id}`))
81 | .filter(entry => !entry.modelDescriptorId)
82 | .map(entry => `${gateway.id}:${entry.id}`),
83 | )
84 |
85 | expect(missingDescriptors).toEqual([])
^
error: expect(received).toEqual(expected)
- []
+ [
+ "aimlapi:aimlapi-claude-sonnet-5",
+ "aimlapi:aimlapi-gemini-3.5-flash",
+ "aimlapi:aimlapi-gpt-5.5",
+ "aimlapi:aimlapi-qwen-3.7-max",
+ "aimlapi:aimlapi-deepseek-v4",
+ ]
- Expected - 1
+ Received + 7
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/index.test.ts:85:32)
##[error]- []
GitHub Actions: PR Checks / 0_smoke-and-tests (24.11.x).txt: feat(aimlapi): harden passwordless onboarding and top-up recovery
Conclusion: failure
##[group]src/integrations/index.test.ts:
(pass) loaded registry validation > registry is valid after loading all descriptors [4.00ms]
(pass) loaded registry validation > MiniMax has shared brand and model descriptors wired to its route catalog
(pass) loaded registry validation > route catalogs do not duplicate defaultModel with catalog default flags
(pass) loaded registry validation > dynamic route catalogs rely entirely on discovery
80 | .filter(entry => !descriptorOptionalEntries.has(`${gateway.id}:${entry.id}`))
81 | .filter(entry => !entry.modelDescriptorId)
82 | .map(entry => `${gateway.id}:${entry.id}`),
83 | )
84 |
85 | expect(missingDescriptors).toEqual([])
^
error: expect(received).toEqual(expected)
- []
+ [
+ "aimlapi:aimlapi-claude-sonnet-5",
+ "aimlapi:aimlapi-gemini-3.5-flash",
+ "aimlapi:aimlapi-gpt-5.5",
+ "aimlapi:aimlapi-qwen-3.7-max",
+ "aimlapi:aimlapi-deepseek-v4",
+ ]
- Expected - 1
+ Received + 7
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/index.test.ts:85:32)
##[error]- []
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/integrations/aimlapi/validation.tssrc/integrations/aimlapi/messages.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/cli/handlers/aimlapi.tssrc/integrations/artifactGenerator.test.tssrc/services/api/client.test.tssrc/utils/status.test.tssrc/utils/providerDiscovery.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/integrations/routeMetadata.test.tssrc/utils/providerProfiles.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/client.tssrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.tssrc/components/ProviderManager.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/integrations/aimlapi/validation.tsREADME.mdsrc/integrations/aimlapi/messages.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/cli/handlers/aimlapi.tssrc/integrations/artifactGenerator.test.tssrc/services/api/client.test.tssrc/utils/status.test.tssrc/utils/providerDiscovery.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/integrations/routeMetadata.test.tssrc/utils/providerProfiles.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/onboarding.tsdocs/aimlapi-setup.mdsrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/client.tssrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.tssrc/components/ProviderManager.tsx
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/integrations/aimlapi/validation.tsREADME.mdsrc/integrations/aimlapi/messages.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/cli/handlers/aimlapi.tssrc/integrations/artifactGenerator.test.tssrc/services/api/client.test.tssrc/utils/status.test.tssrc/utils/providerDiscovery.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/integrations/routeMetadata.test.tssrc/utils/providerProfiles.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/onboarding.tsdocs/aimlapi-setup.mdsrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/client.tssrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.tssrc/components/ProviderManager.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/integrations/aimlapi/validation.tssrc/integrations/aimlapi/messages.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/cli/handlers/aimlapi.tssrc/integrations/artifactGenerator.test.tssrc/services/api/client.test.tssrc/utils/status.test.tssrc/utils/providerDiscovery.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/integrations/routeMetadata.test.tssrc/utils/providerProfiles.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/client.tssrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.tssrc/components/ProviderManager.tsx
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/integrations/aimlapi/validation.tssrc/integrations/aimlapi/messages.tssrc/integrations/artifactGenerator.test.tssrc/services/api/client.test.tssrc/utils/providerDiscovery.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/integrations/routeMetadata.test.tssrc/utils/providerProfiles.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.mddocs/aimlapi-setup.md
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/ConsoleOAuthFlow.test.tsxsrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/components/ConsoleOAuthFlow.test.tsxsrc/integrations/artifactGenerator.test.tssrc/services/api/client.test.tssrc/utils/status.test.tssrc/utils/providerDiscovery.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/runtimeMetadata.test.tssrc/utils/providerProfiles.test.tssrc/integrations/routeMetadata.test.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/client.test.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsx
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
🪛 ast-grep (0.44.1)
src/integrations/aimlapi/topup.ts
[warning] 172-172: Avoid logging sensitive data
Context: console.log( key ${chalk.dim(maskKey(provisioned.apiKey))} (id ${provisioned.apiKeyId}))
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🪛 GitHub Actions: PR Checks / 1_smoke-and-tests (22).txt
src/components/ProviderManager.test.tsx
[error] 116-116: Test failed: Timed out waiting for ProviderManager test condition. Error thrown from waitForCondition at ProviderManager.test.tsx:116.
🪛 GitHub Actions: PR Checks / smoke-and-tests (22)
src/components/ProviderManager.test.tsx
[error] 116-116: Timed out waiting for ProviderManager test condition. Error thrown at waitForCondition after Bun.sleep polling.
[error] 111-1117: ProviderManager test failed: Timed out waiting for ProviderManager test condition (at waitForCondition; invoked from async test at line ~1117).
🪛 GitHub Check: smoke-and-tests (22)
src/integrations/aimlapi/onboarding.test.ts
[failure] 112-112: error:
Expected promise that rejects
Received promise that resolved: Promise { }
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/onboarding.test.ts:112:73)
[failure] 73-73: error: expect(received).toEqual(expected)
{
"action": "new-account",
- "sessionToken": "new-session",
- "sessionToken": "session_test",
}
- Expected - 1
-
Received + 1
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/onboarding.test.ts:73:64)
[failure] 46-46: error: expect(received).toEqual(expected)
{
- "action": "code-sent",
- "action": "new-account",
- "sessionToken": "session_test",
}
- Expected - 1
-
Received + 2
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/onboarding.test.ts:46:65)
src/integrations/aimlapi/topup.test.ts
[failure] 48-48: error: expect(received).toBe(expected)
Expected: false
Received: true
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:48:52)
[failure] 38-38: error: expect(received).toThrow(expected)
Expected substring: "Minimum top-up is $20"
Received message: "Invalid top-up amount."
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:38:48)
src/components/ProviderManager.test.tsx
[failure] 510-510: error: Unexpected AI/ML API by-key top-up in test
at <anonymous> (/home/runner/work/openclaude/openclaude/src/components/ProviderManager.test.tsx:510:19)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:128:9)
[failure] 510-510: error: Unexpected AI/ML API by-key top-up in test
at <anonymous> (/home/runner/work/openclaude/openclaude/src/components/ProviderManager.test.tsx:510:19)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:102:9)
[failure] 510-510: error: Unexpected AI/ML API by-key top-up in test
at <anonymous> (/home/runner/work/openclaude/openclaude/src/components/ProviderManager.test.tsx:510:19)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:75:24)
🪛 GitHub Check: smoke-and-tests (24.11.x)
src/integrations/aimlapi/onboarding.test.ts
[failure] 112-112: error:
Expected promise that rejects
Received promise that resolved: Promise { }
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/onboarding.test.ts:112:73)
[failure] 73-73: error: expect(received).toEqual(expected)
{
"action": "new-account",
- "sessionToken": "new-session",
- "sessionToken": "session_test",
}
- Expected - 1
-
Received + 1
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/onboarding.test.ts:73:64)
[failure] 46-46: error: expect(received).toEqual(expected)
{
- "action": "code-sent",
- "action": "new-account",
- "sessionToken": "session_test",
}
- Expected - 1
-
Received + 2
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/onboarding.test.ts:46:65)
src/integrations/aimlapi/topup.test.ts
[failure] 159-159: error: expect(received).toThrow(expected)
Expected substring: "Session was already exchanged"
Received message: "Unexpected AI/ML API top-up in test"
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:159:13)
[failure] 48-48: error: expect(received).toBe(expected)
Expected: false
Received: true
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:48:52)
[failure] 38-38: error: expect(received).toThrow(expected)
Expected substring: "Minimum top-up is $20"
Received message: "Invalid top-up amount."
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:38:48)
src/components/ProviderManager.test.tsx
[failure] 510-510: error: Unexpected AI/ML API by-key top-up in test
at <anonymous> (/home/runner/work/openclaude/openclaude/src/components/ProviderManager.test.tsx:510:19)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:128:9)
[failure] 510-510: error: Unexpected AI/ML API by-key top-up in test
at <anonymous> (/home/runner/work/openclaude/openclaude/src/components/ProviderManager.test.tsx:510:19)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:102:9)
[failure] 510-510: error: Unexpected AI/ML API by-key top-up in test
at <anonymous> (/home/runner/work/openclaude/openclaude/src/components/ProviderManager.test.tsx:510:19)
at <anonymous> (/home/runner/work/openclaude/openclaude/src/integrations/aimlapi/topup.test.ts:75:24)
🪛 OpenGrep (1.25.0)
src/integrations/aimlapi/validation.ts
[ERROR] 25-25: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🪛 React Doctor (0.7.6)
src/components/ProviderManager.tsx
[warning] 2572-2572: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2582-2582: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2636-2636: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2639-2639: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2730-2730: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 2813-2813: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::node.lowerReorderableExpression) Expression type MemberExpression cannot be safely reordered
(todo)
[error] 2867-2867: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[warning] 2871-2871: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[error] 2895-2895: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[warning] 2903-2903: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::node.lowerReorderableExpression) Expression type MemberExpression cannot be safely reordered
(todo)
[warning] 2948-2948: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 2999-2999: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 3068-3068: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 3374-3374: Your users can see & submit the wrong data when this list reorders or filters, so use a stable id like key={item.id}, not the array index "index".
Use a stable id from the item, like key={item.id} or key={item.slug}. Index keys break when the list reorders or filters.
(no-array-index-as-key)
🔇 Additional comments (29)
src/integrations/gateways/aimlapi.ts (1)
3-3: LGTM!Also applies to: 61-64, 77-91, 118-122
src/integrations/artifactGenerator.ts (1)
237-252: LGTM!src/integrations/artifactGenerator.test.ts (1)
72-89: LGTM!src/integrations/discoveryService.test.ts (1)
471-482: LGTM!Also applies to: 546-553
src/integrations/runtimeMetadata.ts (1)
30-49: LGTM!Also applies to: 320-321, 338-342
src/integrations/runtimeMetadata.test.ts (1)
165-193: LGTM!src/integrations/routeMetadata.test.ts (1)
280-280: LGTM!Also applies to: 559-559
src/utils/providerDiscovery.test.ts (1)
119-121: LGTM!src/utils/providerProfiles.test.ts (2)
1034-1053: LGTM!
2426-2428: 🗄️ Data Integrity & IntegrationNo issue here:
anthropic/claude-sonnet-5is the intended AIMLAPI default model.> Likely an incorrect or invalid review comment.src/utils/providerProfiles.ts (1)
997-1004: LGTM!src/utils/status.test.ts (1)
146-146: LGTM!docs/aimlapi-setup.md (1)
7-49: LGTM!Also applies to: 78-79
src/integrations/aimlapi/client.ts (1)
1-1: LGTM!Also applies to: 38-158, 167-294, 313-316, 324-325
src/integrations/aimlapi/config.ts (1)
4-100: LGTM!src/integrations/aimlapi/onboarding.ts (1)
1-56: LGTM!src/integrations/aimlapi/index.ts (1)
4-21: LGTM!src/integrations/aimlapi/client.test.ts (1)
1-157: LGTM!src/integrations/aimlapi/onboarding.test.ts (1)
1-115: LGTM!src/components/ProviderManager.tsx (1)
1-5: LGTM!Also applies to: 52-65, 101-109, 137-143, 804-1007, 1644-1646, 1748-1792, 1839-1942, 2195-2329, 2619-3220, 3240-3408, 3843-3863
src/components/ProviderManager.test.tsx (2)
10-10: LGTM!Also applies to: 35-37, 120-130, 273-275, 351-354, 998-1126, 1158-1589
469-511: 🎯 Functional CorrectnessNo leak here
mock.restore()inafterEachresets the AIMLAPI mock, so it doesn't spill intoonboarding.test.tsortopup.test.ts.> Likely an incorrect or invalid review comment.src/integrations/aimlapi/validation.ts (1)
1-5: LGTM!Also applies to: 23-32
src/integrations/aimlapi/messages.ts (1)
1-28: LGTM!src/integrations/aimlapi/prompt.ts (1)
7-14: LGTM!src/integrations/aimlapi/config.test.ts (1)
1-79: LGTM!src/integrations/aimlapi/topup.ts (1)
1-140: LGTM!Also applies to: 154-437, 446-478, 487-491
src/cli/handlers/aimlapi.ts (1)
4-4: LGTM!Also applies to: 17-24
src/components/ConsoleOAuthFlow.test.tsx (1)
138-138: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/integrations/aimlapi/prompt.ts (1)
25-40: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not echo the sign-in code in the terminal.
The top-up flow now enters the six-digit authentication code through this normal readline prompt, exposing the active credential on screen. Restore a hidden-input prompt or add a masking option, and use it specifically for the code path with focused tests.
As per path instructions, auth/token handling requires high scrutiny. As per coding guidelines, changed provider behavior must test the exact provider path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/aimlapi/prompt.ts` around lines 25 - 40, Update promptText and its callers to support hidden or masked input, and use that mode specifically when requesting the six-digit authentication code in the top-up sign-in flow. Preserve normal visible behavior for other prompts, ensure the code is never echoed to the terminal, and add focused tests covering the provider’s authentication-code path.Sources: Coding guidelines, Path instructions
src/integrations/aimlapi/client.ts (1)
151-204: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate every passwordless response before using it.
checkAccountaccepts any JSON asAccountCheckResult, so{}silently selects account creation downstream.verifySignInCode,createPasswordlessAccount, andcreateKeycan also dereference malformed/null payloads. Add runtime guards for the exact action, token, key, and ID shapes, plus malformed-200 regression tests.As per path instructions, provider responses require high scrutiny; the PR objective requires malformed responses to be rejected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/aimlapi/client.ts` around lines 151 - 204, Harden the response handling in checkAccount, verifySignInCode, createPasswordlessAccount, and createKey with runtime validation before reading or returning fields: require the expected account action shape, non-empty string tokens, non-empty string keys, and required ID fields according to their response contracts. Reject null, non-object, and otherwise malformed successful payloads with clear errors, and add regression tests covering malformed 200 responses for each endpoint.Source: Path instructions
src/components/ProviderManager.tsx (1)
3007-3010: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCheck cancellation before storing the issued credentials.
An aborted verification can still resolve and write the cancelled account’s session token and API key into the next flow. Move the abort check before all result-derived state updates and add a late-resolution cancellation regression test.
Proposed fix
const result = await completeAimlapiCodeSignIn( aimlapiTopupEmail, trimmedCode, controller.signal, aimlapiInferenceBaseUrl, ) + if (controller.signal.aborted) return setAimlapiSessionToken(result.sessionToken) setAimlapiIssuedKey(result.apiKey) setAimlapiIssuedKeyId(result.apiKeyId) - if (controller.signal.aborted) returnAs per path instructions, “Block on credential reuse mistakes.” As per coding guidelines, “Add or update tests when a code change affects behavior.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ProviderManager.tsx` around lines 3007 - 3010, In the verification flow around setAimlapiSessionToken, setAimlapiIssuedKey, and setAimlapiIssuedKeyId, check controller.signal.aborted before applying any result-derived credential state and return immediately when cancelled. Add a regression test covering a verification that resolves after cancellation, asserting no session token, API key, or API key ID is stored.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/aimlapi/config.ts`:
- Around line 135-147: Update safeHttpBaseUrl to reject URL search and hash
components, and allow http only for explicitly permitted loopback development
origins while requiring https for all remote hosts; continue rejecting
credentials and invalid protocols. Add focused tests covering remote HTTP URLs,
query strings, and fragments, while preserving normalization of accepted URLs.
In `@src/integrations/runtimeMetadata.ts`:
- Around line 52-59: Replace the local header-filtering and partner ID injection
loop in the surrounding runtime metadata flow with the exported
withResolvedPartnerHeader helper. Import and call withResolvedPartnerHeader
using the existing config headers, preserving the returned configuration and
avoiding duplicate case-insensitive resolution logic.
---
Outside diff comments:
In `@src/components/ProviderManager.tsx`:
- Around line 3007-3010: In the verification flow around setAimlapiSessionToken,
setAimlapiIssuedKey, and setAimlapiIssuedKeyId, check controller.signal.aborted
before applying any result-derived credential state and return immediately when
cancelled. Add a regression test covering a verification that resolves after
cancellation, asserting no session token, API key, or API key ID is stored.
In `@src/integrations/aimlapi/client.ts`:
- Around line 151-204: Harden the response handling in checkAccount,
verifySignInCode, createPasswordlessAccount, and createKey with runtime
validation before reading or returning fields: require the expected account
action shape, non-empty string tokens, non-empty string keys, and required ID
fields according to their response contracts. Reject null, non-object, and
otherwise malformed successful payloads with clear errors, and add regression
tests covering malformed 200 responses for each endpoint.
In `@src/integrations/aimlapi/prompt.ts`:
- Around line 25-40: Update promptText and its callers to support hidden or
masked input, and use that mode specifically when requesting the six-digit
authentication code in the top-up sign-in flow. Preserve normal visible behavior
for other prompts, ensure the code is never echoed to the terminal, and add
focused tests covering the provider’s authentication-code path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bf3d2a78-7692-47fb-a12d-b88be5e3abdb
📒 Files selected for processing (21)
src/cli/aimlapiCommand.test.tssrc/cli/aimlapiCommand.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topupDependencies.tssrc/integrations/aimlapi/validation.tssrc/integrations/gateways/aimlapi.tssrc/integrations/models/claude.tssrc/integrations/runtimeMetadata.tssrc/main.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/integrations/aimlapi/topupDependencies.tssrc/cli/aimlapiCommand.tssrc/integrations/models/claude.tssrc/cli/aimlapiCommand.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/validation.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/config.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/integrations/aimlapi/topupDependencies.tssrc/cli/aimlapiCommand.tssrc/integrations/models/claude.tssrc/cli/aimlapiCommand.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/validation.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/config.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/integrations/aimlapi/topupDependencies.tssrc/cli/aimlapiCommand.tssrc/integrations/models/claude.tssrc/cli/aimlapiCommand.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/validation.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/config.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/integrations/aimlapi/topupDependencies.tssrc/cli/aimlapiCommand.tssrc/integrations/models/claude.tssrc/cli/aimlapiCommand.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/validation.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/main.tsxsrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/config.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/integrations/aimlapi/topupDependencies.tssrc/integrations/models/claude.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/validation.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/cli/aimlapiCommand.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/onboarding.test.tssrc/components/ProviderManager.test.tsx
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsx
🪛 OpenGrep (1.25.0)
src/integrations/aimlapi/validation.ts
[ERROR] 14-14: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🪛 React Doctor (0.7.6)
src/components/ProviderManager.tsx
[warning] 2572-2572: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
🔇 Additional comments (18)
src/integrations/gateways/aimlapi.ts (1)
118-122: LGTM!src/integrations/models/claude.ts (1)
4-21: LGTM!src/integrations/aimlapi/validation.ts (1)
9-17: The prior strict-decimal validation finding remains partially addressed.
2e1and0x14both become$20and bypass this regex. Validate the entire trimmed input with^\d+(?:\.\d{1,2})?$before callingNumber(), and add those regression cases.Source: Coding guidelines
src/integrations/aimlapi/topup.test.ts (2)
5-5: Restore thetopupDependencies.jsmodule mock after this suite.The previous process-global
mock.module()leakage concern remains; it is now centralized in one dependency module but still lacks teardown.Source: Path instructions
36-37: LGTM!Also applies to: 343-365
src/integrations/aimlapi/client.ts (1)
56-101: LGTM!Also applies to: 207-220, 333-405
src/integrations/aimlapi/config.test.ts (1)
62-81: LGTM!src/integrations/aimlapi/prompt.test.ts (1)
1-63: LGTM!src/integrations/aimlapi/topup.ts (1)
19-33: LGTM!Also applies to: 205-205, 366-366, 402-504
src/integrations/aimlapi/topupDependencies.ts (1)
1-10: LGTM!src/cli/aimlapiCommand.ts (1)
1-57: LGTM!src/integrations/aimlapi/client.test.ts (1)
138-180: LGTM!src/integrations/aimlapi/onboarding.ts (1)
14-29: LGTM!Also applies to: 31-48, 50-79
src/integrations/aimlapi/onboarding.test.ts (1)
53-53: LGTM!Also applies to: 65-85
src/main.tsx (1)
39-39: LGTM!Also applies to: 4016-4018
src/cli/aimlapiCommand.test.ts (1)
1-49: LGTM!src/components/providerManagerAimlapi.ts (1)
1-38: LGTM!src/components/ProviderManager.test.tsx (1)
317-528: LGTM!Also applies to: 1116-1220, 1288-1388, 1631-1634
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/aimlapi/config.ts`:
- Around line 140-145: Update the loopback detection in the URL validation logic
to replace the broad url.hostname.startsWith('127.') check with a strict
four-octet IPv4 pattern, so domains such as 127.0.0.1.evil.com are not treated
as loopback while Node-normalized IPv4 shorthands remain supported.
In `@src/integrations/aimlapi/topup.test.ts`:
- Around line 9-13: Add an afterAll cleanup in the topup test suite that
restores the original topupDependencies.js module after the openBrowser,
saveProfileFile, and promptText mock is used, preventing the global Bun module
mock from leaking into subsequent tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 525c3a12-686c-4749-85c7-b7bddf9b4fca
📒 Files selected for processing (7)
src/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/runtimeMetadata.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/topup.test.ts
🔇 Additional comments (5)
src/integrations/runtimeMetadata.ts (1)
33-53: LGTM!src/integrations/aimlapi/config.test.ts (1)
68-89: LGTM!src/integrations/aimlapi/topup.ts (1)
145-182: LGTM!src/integrations/aimlapi/topupState.ts (1)
77-92: LGTM!src/integrations/aimlapi/topupState.test.ts (1)
41-75: LGTM!
|
Hi @jatmn! I’ve finished addressing the automated review feedback, and the PR should be ready now. When you have a chance, I’d really appreciate your review. Thanks! |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
This PR has drifted beyond its passwordless onboarding and top-up recovery scope: the global Claude model descriptor and AI/ML API fallback-catalog rollout should be split into a separate PR before this is ready.
Findings
-
[P2] Split the global model-catalog changes from the onboarding/payment work
src/integrations/models/claude.ts:3
This PR is framed as passwordless onboarding and top-up recovery, but it also introduces a globalclaude-sonnet-5descriptor and replaces AI/ML API's curated fallback catalog with five new model entries. The Claude descriptor is mechanically required by the new default-model string, but it changes shared runtime model metadata; the additional fallback catalog changes are independently consumable catalog work. Keeping this PR focused on onboarding/recovery (or separating the model/default/catalog rollout into its own reviewed PR) makes the provider/payment behavior and the model-metadata changes independently testable and revertible. -
[P1] Persist the interactive checkout recovery state
src/components/ProviderManager.tsx:963
The provider-manager path keepsresumeSessionTokenandpaymentSessionIdonly in React state. If the user completes the external checkout and OpenClaude exits before the polling/exchange branch finishes, restarting loses the paid session and starts a new checkout with a new UUID. That leaves the original payment unrecoverable from OpenClaude and can invite a duplicate charge. Persist and reload the same intent state used by the CLI flow before opening checkout, then clear it only after the provider profile has been saved. -
[P1] Clear or recover an already-exchanged persisted CLI session
src/integrations/aimlapi/topup.ts:423
A process interruption after the backend exchange succeeds but beforesaveProfileFile()/clearAimlapiTopupState()completes leavesresumeSessionTokenon disk. Every later identical CLI retry loads that token, seesexchanged, and throws without clearing it, so the flow remains permanently stuck on the same terminal session until the user manually removes~/.claude/aimlapi-topup.json. Clear the stale state on this terminal path and provide a recoverable route for the newly issued key rather than retaining an unretryable payment intent. -
[P2] Recognize the documented
OPENAI_API_KEYfallback in the existing-account flow
src/components/ProviderManager.tsx:2767
The route acceptsOPENAI_API_KEYas the AI/ML API fallback credential, and the updated setup guide documents that behavior, but the provider-manager lookup considers onlyAIMLAPI_API_KEY. A working environment-only AI/ML API configuration therefore bypasses the promised existing-configuration validation/top-up path and forces a fresh setup. Use the same credential-precedence helper (or at least the documented fallback) here and in the first-run branch. -
[P2] Do not echo the emailed sign-in code into terminal scrollback
src/integrations/aimlapi/prompt.ts:25
The CLI collects the passwordless sign-in code with visiblereadline.question, so an active authentication code is printed in the terminal transcript. The prior flow masked its password input; the replacement should likewise use a non-echoing prompt for the code so shared terminals, recorded sessions, and scrollback do not expose it. -
[P2] Serialize creation of a persisted payment intent
src/integrations/aimlapi/topup.ts:158
Two concurrentopenclaude aimlapi topupinvocations can both observe no state, generate differentpaymentSessionIdvalues, and overwrite the one state file before each independently creates and pays a checkout session. Atomic file replacement protects individual writes but does not make the read-create-save sequence exclusive, so the backend cannot deduplicate the two distinct idempotency keys. Lock or atomically claim the intent before generating the ID and creating the checkout.
|
@jatmn, thanks for the review! I’ve addressed the feedback and pushed the fixes. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/ProviderManager.tsx (2)
3334-3344: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winMask the six-digit sign-in code.
The CLI masks this authentication credential, but the Ink input renders it in clear text.
Proposed fix
<TextInput value={aimlapiSignInCode} onChange={setAimlapiSignInCode} onSubmit={verifyAimlapiSignInCode} + mask="*" focus={true}Add a focused UI assertion that entered code digits are absent from rendered output. As per coding guidelines, add or update tests when a TSX behavior change affects behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ProviderManager.tsx` around lines 3334 - 3344, Update the TextInput used for aimlapiSignInCode in ProviderManager to render entered digits masked while preserving editing, cursor, and submission behavior. Add a focused UI test asserting that entered sign-in code digits are absent from the rendered output.Source: Coding guidelines
2580-2592: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not auto-send fallback credentials to overridden AIMLAPI endpoints.
resolveRouteCredentialValuecan returnOPENAI_API_KEY, while validation usesnextDraft.baseUrlor the configured inference URL. With a noncanonicalAIMLAPI_INFERENCE_URL, this silently sends a generic provider credential to that host.Only enable automatic env-key reuse for canonical AIMLAPI URLs; require explicit key entry for custom endpoints. Add a negative custom-endpoint regression test.
As per path instructions, block credential reuse mistakes and unintended network reach.
Also applies to: 2791-2829
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ProviderManager.tsx` around lines 2580 - 2592, The first-run AIMLAPI credential reuse in the flow containing resolveRouteCredentialValue and validateAndPersistAimlapiKey must only run when nextDraft.baseUrl is a canonical AIMLAPI endpoint; for custom or overridden inference URLs, skip setAimlapiIssuedKey, setAimlapiExistingUsesEnv, and automatic validation/persistence so explicit key entry is required. Apply the same guard to the corresponding logic around the additional referenced flow and add a regression test confirming no fallback credential is sent to a custom endpoint.Source: Path instructions
src/integrations/aimlapi/topupState.ts (1)
130-165: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftProtect every checkout-state mutation with the same lock.
saveAimlapiTopupStateandclearAimlapiTopupStaterun outsidewithStateLock; the latter is an explicit TOCTOU check/delete. Concurrent CLI/UI flows can overwrite or delete a newer intent, losing the only resumable payment identifiers.Use internal unlocked read/write helpers inside
claim, then make public save/clear lock and verify the current intent/payment ID before mutating. Add concurrent stale-writer and clear-vs-claim tests.As per path instructions, review global/config state isolation and payment retry state with high scrutiny.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/aimlapi/topupState.ts` around lines 130 - 165, Protect all checkout-state mutations with the same withStateLock used by claimAimlapiTopupState. Introduce internal unlocked read/write helpers for claim’s locked flow, then make saveAimlapiTopupState and clearAimlapiTopupState acquire the lock; before writing or deleting, verify the stored intent and paymentSessionId still match the requested state to prevent stale writers and TOCTOU deletion. Add concurrency coverage for stale writers and clear racing with claim, while preserving isolated global/config and payment-retry state.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/aimlapi/topup.ts`:
- Around line 432-435: Preserve terminal exchanged-session state so ambiguous
retries remain blocked and cannot create duplicate charges. In
src/integrations/aimlapi/topup.ts:432-435, retain the exchanged receipt instead
of calling onSession with an empty value; at 453-455, keep the session and
issued-key metadata after exchange polling settles; and at 503-505, preserve
payment identity when polling observes exchanged. Update
src/integrations/aimlapi/topup.test.ts:126-176 and 274-326 to assert retained
terminal state and blocked identical retries rather than state deletion or
onSession('').
---
Outside diff comments:
In `@src/components/ProviderManager.tsx`:
- Around line 3334-3344: Update the TextInput used for aimlapiSignInCode in
ProviderManager to render entered digits masked while preserving editing,
cursor, and submission behavior. Add a focused UI test asserting that entered
sign-in code digits are absent from the rendered output.
- Around line 2580-2592: The first-run AIMLAPI credential reuse in the flow
containing resolveRouteCredentialValue and validateAndPersistAimlapiKey must
only run when nextDraft.baseUrl is a canonical AIMLAPI endpoint; for custom or
overridden inference URLs, skip setAimlapiIssuedKey, setAimlapiExistingUsesEnv,
and automatic validation/persistence so explicit key entry is required. Apply
the same guard to the corresponding logic around the additional referenced flow
and add a regression test confirming no fallback credential is sent to a custom
endpoint.
In `@src/integrations/aimlapi/topupState.ts`:
- Around line 130-165: Protect all checkout-state mutations with the same
withStateLock used by claimAimlapiTopupState. Introduce internal unlocked
read/write helpers for claim’s locked flow, then make saveAimlapiTopupState and
clearAimlapiTopupState acquire the lock; before writing or deleting, verify the
stored intent and paymentSessionId still match the requested state to prevent
stale writers and TOCTOU deletion. Add concurrency coverage for stale writers
and clear racing with claim, while preserving isolated global/config and
payment-retry state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1916e90f-37f8-4be0-b1bd-de7cb7be0094
📒 Files selected for processing (13)
src/cli/aimlapiCommand.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/discoveryService.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/cli/aimlapiCommand.tssrc/integrations/index.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/gateways/aimlapi.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/prompt.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/cli/aimlapiCommand.tssrc/integrations/index.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/gateways/aimlapi.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/prompt.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/cli/aimlapiCommand.tssrc/integrations/index.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/gateways/aimlapi.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/prompt.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/cli/aimlapiCommand.tssrc/integrations/index.tssrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/gateways/aimlapi.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/prompt.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsxsrc/integrations/aimlapi/topup.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/integrations/index.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/gateways/aimlapi.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/integrations/aimlapi/topupState.test.tssrc/integrations/discoveryService.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsx
🔇 Additional comments (14)
src/integrations/aimlapi/topupState.test.ts (1)
78-85: LGTM!src/integrations/discoveryService.test.ts (2)
476-478: LGTM!
542-543: LGTM!src/integrations/aimlapi/prompt.ts (1)
8-8: LGTM!Also applies to: 17-20, 30-60
src/integrations/aimlapi/prompt.test.ts (1)
2-2: LGTM!Also applies to: 66-85
src/integrations/aimlapi/topup.test.ts (1)
8-8: LGTM!Also applies to: 178-268, 329-534
src/integrations/aimlapi/topup.ts (1)
20-20: LGTM!Also applies to: 112-119, 142-142, 163-175, 389-431, 436-440, 442-452, 456-486, 488-502, 506-525
src/integrations/aimlapi/topupState.ts (1)
3-9: LGTM!Also applies to: 17-24, 38-84, 95-113
src/components/ProviderManager.tsx (1)
1-1: LGTM!Also applies to: 48-72, 975-1019, 1657-1666, 1760-1804, 1851-1954, 2207-2341, 2579-2579, 2593-2603, 2633-2731, 2733-2790, 2832-3279, 3299-3333, 3345-3465, 3900-3920
src/components/providerManagerAimlapi.ts (1)
10-25: LGTM!Also applies to: 50-57
src/components/ProviderManager.test.tsx (1)
37-38: LGTM!Also applies to: 319-390, 481-556, 1174-1227, 1648-1767
src/integrations/gateways/aimlapi.ts (1)
59-85: LGTM!Also applies to: 118-123
src/integrations/index.ts (1)
144-144: LGTM!src/cli/aimlapiCommand.ts (1)
4-4: LGTM!Also applies to: 18-58
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Do not forward the canonical AIMLAPI key to custom endpoints
src/utils/providerProfiles.ts:996
isAimlapiProfileis true for a profile whose provider isaimlapieven when its base URL is a user-controlled proxy. The new ambientAIMLAPI_API_KEYfallback is then copied intoOPENAI_API_KEY, so activating a keyless profile at (for example)https://proxy.example.com/v1sends the canonical AIMLAPI credential to that host. Gate this fallback on the canonical inference URL, as the new guided-flow validation already does. -
[P1] Wait for a resumed key-bound top-up that is still exchanging
src/integrations/aimlapi/topup.ts:356
A resumed by-key session inexchangingis assigned theexchangephase, but this function only waits forpayandpolland has no exchange step. It therefore immediately returns the API key; the ProviderManager subsequently marks the top-up successful even though the billing operation is not settled. Poll this state to its terminal result before reporting success. -
[P2] Claim/reuse the checkout before minting another existing-account key
src/integrations/aimlapi/topup.ts:145
The CLI creates an API key immediately after every code sign-in, before it validates--amountor loads the retained checkout state. An invalid amount permanently creates an unused key, and every retry after an interrupted checkout creates another key while merely resuming the same payment session. Validate/claim the intent first and persist or reuse the issued key with that intent so retry safety covers credentials as well as charges. -
[P2] Reject unsupported account actions in the CLI path
src/integrations/aimlapi/topup.ts:135
Any successfulcheckAccountresponse other than exactlysign-inis treated as a new account, so a malformed or newly introduced action proceeds to passwordless account creation and checkout. The UI onboarding path already explicitly accepts onlysign-inandsign-up; apply the same fail-closed switch here. -
[P2] Make stale-lock recovery ownership-safe
src/integrations/aimlapi/topupState.ts:73
If a lock holder pauses for more than 30 seconds, a second process can delete its lock and acquire a replacement. When the original process resumes, its unconditional cleanup at line 93 deletes the replacement lock, allowing a third process into the critical section while the second is mutating the checkout state. Use an ownership token (and only unlink a lock still owned by this holder) so state/payment recovery remains serialized. -
[P2] Restore the shared test-module mocks instead of leaking them to later suites
src/services/awaySummary.test.ts:30
These module-scope partial mocks are never restored, despite Bun's module-mock registry being process-global. Later smoke tests can receive only these stubs rather than the realclaudeor session-memory modules. The same regression is introduced for the analytics barrel insrc/utils/diff.test.ts:5and the providers module insrc/services/compact/compact.test.ts:30; scope the mocks under the existing serialized lifecycle and restore the complete real modules before other tests import them. -
[P2] Do not delete the shared
/tmp/taskpath in test cleanup
src/services/compact/compact.test.ts:791
This suite now returns a literal/tmp/taskfrom its mock and unconditionally unlinks that path inafterAll, although it never creates or owns it. A concurrent OpenClaude/test process can legitimately own that file or symlink and have it removed silently. Restore a unique test-owned temporary path and clean up only that path.
b3ae306 to
b9c0fb6
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ProviderManager.tsx (1)
2593-2605: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute topped-up environment keys through the paid completion path.
After an env-key top-up, this branch revalidates and persists with
doneKind: 'ready', so the paid flow shows “Everything is ready” instead of top-up confirmation. HandleaimlapiExistingUsesEnvbefore env auto-detection, allowingpersistExistingAimlapi()to useaimlapiTopupPaidRef.Proposed fix
if (draftProvider === 'aimlapi') { + if (aimlapiExistingProfileId || aimlapiExistingUsesEnv) { + persistExistingAimlapi(nextDraft.model) + return + } const envKey = resolveRouteCredentialValue({ routeId: 'aimlapi', baseUrl: nextDraft.baseUrl, })?.trim() ... - if (aimlapiExistingProfileId || aimlapiExistingUsesEnv) { - persistExistingAimlapi(nextDraft.model) - return - } }Add a focused env-key top-up completion test. As per coding guidelines, test the exact provider/model path changed when possible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ProviderManager.tsx` around lines 2593 - 2605, Update the first-run AimlAPI key handling around validateAndPersistAimlapiKey so aimlapiExistingUsesEnv is handled before env-key auto-detection, routing topped-up keys through persistExistingAimlapi and its aimlapiTopupPaidRef paid-completion path instead of the ready flow. Preserve normal environment-key detection for non-top-up cases, and add a focused test covering the affected provider/model completion path.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/aimlapi/topup.ts`:
- Around line 153-158: Update the terminal checkout reset in the
resumeSessionToken flow to clear only checkout/session identifiers while
preserving the existing apiKey and apiKeyId. Ensure cancellation, expiry, and
failure paths reuse the retained credential on the next CLI run, and update the
associated terminal-session tests to verify key preservation and reuse.
In `@src/integrations/aimlapi/topupState.ts`:
- Around line 221-239: The claimAimlapiTopupState function must not replace an
existing live checkout when matchesIntent returns false. Update the
withStateLock flow to preserve state per intent or reject the new claim with an
appropriate error, ensuring the original paymentSessionId, resumeSessionToken,
and API key remain recoverable for retries.
- Around line 94-109: Replace the stale-lock recovery in the lock acquisition
flow around statSync, renameSync, and rmSync so it never removes a lock based
solely on an earlier stale pathname observation. Use ownership-safe fencing and
revalidation, or fail closed when ownership cannot be proven; preserve exclusive
checkout-state mutation. Add a two-process regression test that exercises
release-and-reacquire between stale detection and recovery, verifying the live
replacement lock is not stolen.
In `@src/services/compact/compact.test.ts`:
- Around line 20-22: Replace the single TASK_OUTPUT_TEST_PATH file constant with
a unique temporary directory, while retaining taskId in each mocked task-output
filename. Update the mock return to build paths under that directory and adjust
cleanup to remove the directory recursively, ensuring concurrent tasks and tests
remain isolated.
---
Outside diff comments:
In `@src/components/ProviderManager.tsx`:
- Around line 2593-2605: Update the first-run AimlAPI key handling around
validateAndPersistAimlapiKey so aimlapiExistingUsesEnv is handled before env-key
auto-detection, routing topped-up keys through persistExistingAimlapi and its
aimlapiTopupPaidRef paid-completion path instead of the ready flow. Preserve
normal environment-key detection for non-top-up cases, and add a focused test
covering the affected provider/model completion path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9af3ced-c377-4acb-9a3e-77abdc7caf42
📒 Files selected for processing (11)
src/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/services/awaySummary.test.tssrc/services/compact/compact.test.tssrc/utils/diff.test.tssrc/utils/providerProfiles.test.tssrc/utils/providerProfiles.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/services/awaySummary.test.tssrc/utils/diff.test.tssrc/utils/providerProfiles.tssrc/services/compact/compact.test.tssrc/integrations/aimlapi/topupState.test.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/services/awaySummary.test.tssrc/utils/diff.test.tssrc/utils/providerProfiles.tssrc/services/compact/compact.test.tssrc/integrations/aimlapi/topupState.test.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsx
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/awaySummary.test.tssrc/utils/diff.test.tssrc/utils/providerProfiles.tssrc/services/compact/compact.test.tssrc/integrations/aimlapi/topupState.test.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/services/awaySummary.test.tssrc/utils/diff.test.tssrc/utils/providerProfiles.tssrc/services/compact/compact.test.tssrc/integrations/aimlapi/topupState.test.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/awaySummary.test.tssrc/utils/diff.test.tssrc/services/compact/compact.test.tssrc/integrations/aimlapi/topupState.test.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsx
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/utils/providerProfiles.tssrc/integrations/aimlapi/topupState.test.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topup.test.ts
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/ProviderManager.tsxsrc/components/ProviderManager.test.tsx
🪛 React Doctor (0.7.6)
src/components/ProviderManager.tsx
[error] 2902-2902: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
🔇 Additional comments (11)
src/utils/providerProfiles.ts (1)
36-36: LGTM!Also applies to: 999-1012
src/services/awaySummary.test.ts (1)
30-60: LGTM!src/utils/diff.test.ts (1)
1-19: LGTM!src/services/compact/compact.test.ts (1)
12-22: 🎯 Functional Correctness
randomUUIDis already imported; no change needed.> Likely an incorrect or invalid review comment.src/integrations/aimlapi/topup.ts (1)
28-38: LGTM!Also applies to: 49-152, 159-614
src/integrations/aimlapi/topupState.ts (1)
29-93: LGTM!Also applies to: 110-220, 240-256
src/integrations/aimlapi/topup.test.ts (1)
1-478: LGTM!Also applies to: 556-630
src/integrations/aimlapi/topupState.test.ts (1)
1-120: LGTM!src/components/ProviderManager.tsx (1)
1-2592: LGTM!Also applies to: 2606-4062
src/components/ProviderManager.test.tsx (1)
1-1804: LGTM!src/utils/providerProfiles.test.ts (1)
1013-1032: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/aimlapi-setup.md`:
- Around line 7-22: Update the interactive AI/ML API setup instructions to use
the exact current UI labels: identify the provider as “aimlapi.com” and replace
the existing configuration choices with “Continue with your saved API key” and
“Set up a new key or switch account.” Keep the surrounding setup flow accurate
and unchanged.
In `@src/components/ProviderManager.tsx`:
- Around line 3468-3473: Update renderAimlapiDone so parseAimlapiAmountUsd is
called only when a top-up completed successfully; when funding is skipped after
an invalid amount, render the existing “Everything is ready” state without
parsing. Add a focused regression covering invalid amount → back → skip and
verify the ready screen renders successfully.
In `@src/integrations/aimlapi/client.test.ts`:
- Around line 132-201: Extend src/integrations/aimlapi/client.test.ts:132-201
with an AbortController test that cancels an in-flight client request, verifies
prompt rejection/termination, and confirms the request receives the controller
signal. Extend src/integrations/aimlapi/topup.test.ts:556-600 with a polling
cancellation test that aborts after a pending poll response, then verifies no
additional GET occurs and the retained session is not cleared.
In `@src/integrations/aimlapi/client.ts`:
- Around line 151-205: Add endpoint-specific validation before returning or
processing successful responses from checkAccount, verifySignInCode,
createPasswordlessAccount, createKey, and the related session, pay, and exchange
methods. Reject null, non-object, and structurally incomplete payloads before
property access, ensuring getSession cannot clear retained payment identity or
permit an ambiguous retry and token/key methods return controlled validation
errors. Reuse the existing balance-validation approach, preserve
request/retry/proxy behavior, and add or update tests covering empty and null
success responses for each affected endpoint.
In `@src/integrations/aimlapi/onboarding.ts`:
- Around line 4-7: Prevent mixed AIMLAPI environments by validating the complete
auth, billing, and inference endpoint bundle before client creation in
clientForInferenceBaseUrl; reject inference-only custom overrides rather than
combining them with production endpoints. In src/components/ProviderManager.tsx
lines 3172-3185, keep guided provisioning disabled unless a validated full
environment is configured. In docs/aimlapi-setup.md lines 78-79, document the
required complete endpoint bundle and partial-override restriction, and add
focused tests verifying that AIMLAPI_INFERENCE_URL alone cannot create,
validate, or persist a production-issued key.
In `@src/integrations/aimlapi/topup.test.ts`:
- Around line 21-26: Expand the originalEnv snapshot in topup.test.ts to include
AIMLAPI_PARTNER_ID, AIMLAPI_VERIFICATION_BASE_URL, AIMLAPI_RETURN_URL,
AIMLAPI_EMAIL, and AIMLAPI_CODE alongside the existing variables. Ensure the
suite’s environment restoration logic uses this complete snapshot so every
AIMLAPI variable consumed by the tests is restored.
In `@src/utils/providerProfiles.ts`:
- Around line 998-1012: Update the canonical URL guard in the AIMLAPI profile
environment setup to handle an undefined profile.baseUrl before calling
isCanonicalAimlapiInferenceBaseUrl. Treat a falsy baseUrl as the implicit
canonical endpoint, while continuing to validate explicitly provided URLs
through the existing helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 755498f3-3424-45e0-aa20-b7f8bf29046b
⛔ Files ignored due to path filters (1)
src/integrations/generated/integrationManifest.generated.tsis excluded by!**/*.generated.*,!**/generated/**,!src/integrations/generated/**
📒 Files selected for processing (39)
README.mddocs/aimlapi-setup.mdsrc/cli/aimlapiCommand.test.tssrc/cli/aimlapiCommand.tssrc/cli/handlers/aimlapi.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/components/providerManagerAimlapi.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/config.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/messages.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.tssrc/integrations/aimlapi/topupDependencies.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/validation.tssrc/integrations/artifactGenerator.test.tssrc/integrations/artifactGenerator.tssrc/integrations/discoveryService.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/index.tssrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/runtimeMetadata.tssrc/main.tsxsrc/services/api/client.test.tssrc/utils/providerDiscovery.test.tssrc/utils/providerProfiles.test.tssrc/utils/providerProfiles.tssrc/utils/status.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests (22): feat(aimlapi): harden passwordless onboarding and top-up recovery
Conclusion: failure
tner id override is shared with the inference header
(pass) canonical endpoint check excludes proxies
##[endgroup]
##[group]src/integrations/aimlapi/client.test.ts:
(pass) passwordless onboarding methods use the current backend contracts [1.00ms]
(pass) pay only sends autoTopUp when it is enabled [1.00ms]
(pass) topUpByKey uses the v2 billing endpoint and API key bearer
(pass) typed requests reject an empty successful response
(pass) getBalance rejects malformed successful payloads [1.00ms]
(pass) session tokens are excluded from HTTP and network errors [1.00ms]
(pass) response bodies are capped before decoding or surfacing errors [2.00ms]
(pass) token-producing methods reject an empty token [1.00ms]
##[endgroup]
##[group]src/integrations/aimlapi/topupState.test.ts:
(pass) top-up state round-trips only for the same checkout intent [5.00ms]
(pass) top-up state is cleared only by its matching intent [2.00ms]
(pass) claiming the same checkout intent reuses one payment id
(pass) stale writers cannot overwrite a newly claimed checkout [1.00ms]
(pass) stale clear cannot delete a replacement checkout [1.00ms]
##[endgroup]
##[group]src/integrations/aimlapi/topup.test.ts:
(pass) parseAimlapiAmountUsd enforces checkout bounds [1.00ms]
(pass) isValidAimlapiEmail rejects incomplete domains
�[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;�
AI/ML API top-up - https://app.example.test
AI/ML API top-up - https://app.example.test
[OK] Balance topped up and provider configured.
key **** (id key_id)
base URL https://api.aimlapi.com/v1
model anthropic/claude-sonnet-5
profile profile.json
(pass) CLI retries reuse the persisted checkout session and payment id [5.00ms]
AI/ML API top...
GitHub Actions: PR Checks / 1_smoke-and-tests (22).txt: feat(aimlapi): harden passwordless onboarding and top-up recovery
Conclusion: failure
quest translation > defaults untyped MCP tool properties to string for Codex strict mode (issue `#1114`)
(pass) Codex request translation > drops orphan required keys when Ruflo MCP schema has no properties
(pass) Codex request translation > infers object type for untyped schemas with nested properties
(pass) Codex request translation > infers array type for untyped schemas with items
(pass) Codex request translation > infers type from enum values when type is missing
(pass) Codex request translation > leaves combinator-only schemas untyped to preserve alternatives [1.00ms]
(pass) Codex request translation > converts plain string user message into Codex input_text chunk type
(pass) Codex request translation > converts plain string user message into standard text chunk type when forceTextChunks=true
(pass) Codex request translation > preserves wrapped string message content
(pass) Codex request translation > converts assistant tool use and user tool result into Responses items [2.00ms]
(pass) Codex request translation > renders tool_reference blocks from ToolSearch results as readable text
(pass) Codex request translation > keeps the ToolSearch tool in the Responses tools list
(pass) Codex request translation > converts completed Codex tool response into Anthropic message
(pass) Codex request translation > strips <think> tag block from completed Codex text responses
(pass) Codex request translation > strips unterminated <think> tag at block boundary in Codex completed response
(pass) Codex request translation > recovers Codex web search text and sources from sparse completed response
(pass) Codex request translation > falls back to a non-empty Codex web search result message
(pass) Codex request translation > surfaces Codex web search failure reason with a message
(pass) Codex request translation > surfaces Codex web search failure reason nested under action.error
(pass) Codex request translation > handles Codex web search failure with no reason a...
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/integrations/index.tssrc/integrations/aimlapi/messages.tssrc/integrations/aimlapi/topupDependencies.tssrc/cli/handlers/aimlapi.tssrc/services/api/client.test.tssrc/integrations/artifactGenerator.test.tssrc/utils/providerDiscovery.test.tssrc/cli/aimlapiCommand.test.tssrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/utils/status.test.tssrc/integrations/aimlapi/validation.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/integrations/discoveryService.test.tssrc/components/providerManagerAimlapi.tssrc/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/cli/aimlapiCommand.tssrc/integrations/gateways/aimlapi.tssrc/main.tsxsrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/topup.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/integrations/index.tssrc/integrations/aimlapi/messages.tssrc/integrations/aimlapi/topupDependencies.tssrc/cli/handlers/aimlapi.tssrc/services/api/client.test.tssrc/integrations/artifactGenerator.test.tssrc/utils/providerDiscovery.test.tssrc/cli/aimlapiCommand.test.tsREADME.mdsrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/utils/status.test.tssrc/integrations/aimlapi/validation.tssrc/components/ConsoleOAuthFlow.test.tsxdocs/aimlapi-setup.mdsrc/integrations/discoveryService.test.tssrc/components/providerManagerAimlapi.tssrc/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/cli/aimlapiCommand.tssrc/integrations/gateways/aimlapi.tssrc/main.tsxsrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/topup.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/integrations/index.tssrc/integrations/aimlapi/messages.tssrc/integrations/aimlapi/topupDependencies.tssrc/cli/handlers/aimlapi.tssrc/services/api/client.test.tssrc/integrations/artifactGenerator.test.tssrc/utils/providerDiscovery.test.tssrc/cli/aimlapiCommand.test.tsREADME.mdsrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/utils/status.test.tssrc/integrations/aimlapi/validation.tssrc/components/ConsoleOAuthFlow.test.tsxdocs/aimlapi-setup.mdsrc/integrations/discoveryService.test.tssrc/components/providerManagerAimlapi.tssrc/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/cli/aimlapiCommand.tssrc/integrations/gateways/aimlapi.tssrc/main.tsxsrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/topup.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/integrations/index.tssrc/integrations/aimlapi/messages.tssrc/integrations/aimlapi/topupDependencies.tssrc/cli/handlers/aimlapi.tssrc/services/api/client.test.tssrc/integrations/artifactGenerator.test.tssrc/utils/providerDiscovery.test.tssrc/cli/aimlapiCommand.test.tssrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/utils/status.test.tssrc/integrations/aimlapi/validation.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/integrations/discoveryService.test.tssrc/components/providerManagerAimlapi.tssrc/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/cli/aimlapiCommand.tssrc/integrations/gateways/aimlapi.tssrc/main.tsxsrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/topup.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/integrations/index.tssrc/integrations/aimlapi/messages.tssrc/integrations/aimlapi/topupDependencies.tssrc/services/api/client.test.tssrc/integrations/artifactGenerator.test.tssrc/utils/providerDiscovery.test.tssrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/runtimeMetadata.tssrc/integrations/aimlapi/prompt.tssrc/integrations/aimlapi/config.test.tssrc/integrations/artifactGenerator.tssrc/integrations/aimlapi/validation.tssrc/integrations/discoveryService.test.tssrc/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/integrations/gateways/aimlapi.tssrc/integrations/aimlapi/index.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/onboarding.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/config.tssrc/integrations/aimlapi/topupState.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/topup.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/client.test.tssrc/integrations/artifactGenerator.test.tssrc/utils/providerDiscovery.test.tssrc/cli/aimlapiCommand.test.tssrc/integrations/routeMetadata.test.tssrc/integrations/runtimeMetadata.test.tssrc/integrations/aimlapi/prompt.test.tssrc/integrations/aimlapi/config.test.tssrc/utils/status.test.tssrc/components/ConsoleOAuthFlow.test.tsxsrc/integrations/discoveryService.test.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/onboarding.test.tssrc/integrations/aimlapi/topupState.test.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.test.tsx
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
README.mddocs/aimlapi-setup.md
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/ConsoleOAuthFlow.test.tsxsrc/components/ProviderManager.test.tsxsrc/components/ProviderManager.tsx
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
src/main.tsx
🪛 ast-grep (0.44.1)
src/integrations/aimlapi/topup.ts
[warning] 239-239: Avoid logging sensitive data
Context: console.log( key ${chalk.dim(maskKey(provisioned.apiKey))} (id ${provisioned.apiKeyId}))
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🪛 GitHub Actions: PR Checks / 2_smoke-and-tests (24.11.x).txt
src/components/ProviderManager.test.tsx
[error] 117-117: Timed out waiting for ProviderManager test condition. Error thrown from waitForCondition: 'Timed out waiting for ProviderManager test condition'.
[error] 576-576: waitForFrameOutput failed due to upstream timeout in ProviderManager test condition.
[error] 1771-1771: ProviderManager test run failed due to timeout while waiting for the test condition.
🪛 GitHub Actions: PR Checks / smoke-and-tests (24.11.x)
src/components/ProviderManager.test.tsx
[error] 117-117: Timed out waiting for ProviderManager test condition. Thrown: "Timed out waiting for ProviderManager test condition"
[error] 576-576: Test helper failed: await waitForFrameOutput due to timeout in waitForCondition.
[error] 1771-1771: ProviderManager test crashed due to unhandled timeout while waiting for the expected condition.
🪛 OpenGrep (1.25.0)
src/integrations/aimlapi/validation.ts
[ERROR] 14-14: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 30-30: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🪛 React Doctor (0.7.6)
src/components/ProviderManager.tsx
[warning] 2600-2600: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2609-2609: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2663-2663: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2666-2666: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 2758-2758: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 2847-2847: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::node.lowerReorderableExpression) Expression type MemberExpression cannot be safely reordered
(todo)
[error] 2902-2902: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[error] 2905-2905: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[warning] 2909-2909: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[error] 2933-2933: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[warning] 2941-2941: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::node.lowerReorderableExpression) Expression type MemberExpression cannot be safely reordered
(todo)
[warning] 2987-2987: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 3038-3038: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 3145-3145: This component misses React Compiler's automatic memoization & re-renders more than it should. Rewrite the flagged code so the compiler can optimize it.
Todo: (BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause
(todo)
[warning] 3393-3393: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 3395-3395: This component misses React Compiler's automatic memoization & re-renders more than it should: Cannot access refs during render. Rewrite the flagged code so the compiler can optimize it.
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the current property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
(refs)
[warning] 3456-3456: Your users can see & submit the wrong data when this list reorders or filters, so use a stable id like key={item.id}, not the array index "index".
Use a stable id from the item, like key={item.id} or key={item.slug}. Index keys break when the list reorders or filters.
(no-array-index-as-key)
🔇 Additional comments (31)
src/services/api/client.test.ts (1)
817-817: LGTM!src/integrations/gateways/aimlapi.ts (1)
3-3: LGTM!Also applies to: 61-64, 77-91, 119-122
src/integrations/artifactGenerator.ts (1)
237-252: LGTM!src/utils/providerDiscovery.test.ts (1)
121-121: LGTM!src/utils/status.test.ts (1)
146-146: LGTM!src/cli/aimlapiCommand.test.ts (1)
1-50: LGTM!src/cli/aimlapiCommand.ts (1)
1-59: LGTM!src/cli/handlers/aimlapi.ts (1)
4-4: LGTM!Also applies to: 17-24
src/main.tsx (1)
39-39: LGTM!Also applies to: 4018-4018
src/integrations/discoveryService.test.ts (1)
471-478: LGTM!Also applies to: 542-545
src/integrations/artifactGenerator.test.ts (1)
72-90: LGTM!src/integrations/index.ts (1)
144-144: LGTM!src/integrations/routeMetadata.test.ts (1)
280-280: LGTM!Also applies to: 559-559
src/integrations/runtimeMetadata.test.ts (1)
165-193: LGTM!src/integrations/runtimeMetadata.ts (1)
30-63: LGTM!Also applies to: 318-319, 336-340
src/utils/providerProfiles.test.ts (1)
1013-1034: LGTM!Also applies to: 1036-1054, 2428-2430
src/integrations/aimlapi/validation.ts (2)
9-18: The fractional-cent guard remains bypassable.
19.999e0does not matchdecimal, then rounds to 2,000 cents and passes. The earlier finding still applies: validate the complete input syntax before callingNumber.
28-37: LGTM!src/integrations/aimlapi/topup.ts (2)
152-158: The previous credential-preservation blocker remains.
persistSession('')still deletes the entire state. If sign-in already storedapiKeyandapiKeyId, a terminal checkout causes the next run to mint another key instead of rotating only the checkout identifiers.
1-151: LGTM!Also applies to: 159-614
src/integrations/aimlapi/client.ts (1)
1-1: LGTM!Also applies to: 38-135, 207-221, 323-405
src/integrations/aimlapi/config.ts (1)
4-30: LGTM!Also applies to: 42-46, 53-100, 110-157
src/integrations/aimlapi/client.test.ts (1)
1-130: LGTM!src/integrations/aimlapi/config.test.ts (1)
1-124: LGTM!src/integrations/aimlapi/onboarding.test.ts (1)
1-138: LGTM!src/integrations/aimlapi/prompt.test.ts (1)
1-85: LGTM!src/integrations/aimlapi/prompt.ts (1)
7-61: LGTM!src/integrations/aimlapi/topupDependencies.ts (1)
1-10: LGTM!src/integrations/aimlapi/topup.test.ts (1)
1-20: LGTM!Also applies to: 29-554, 602-630
src/integrations/aimlapi/topupState.ts (2)
94-109: Existing blocker remains: stale recovery can steal a replacement live lock.The stale
statSyncobservation and laterrenameSyncare not tied to the same lock instance. The owner can release and another process can acquire between them, allowing this process to rename the new live lock.
221-239: Existing blocker remains: do not replace another live checkout.An unmatched claim overwrites the only persisted payment ID, resume token, and issued key. Reject the conflicting claim or persist state per intent so the original operation remains safely resumable.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/integrations/aimlapi/client.ts (1)
173-179: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftValidate successful endpoint responses before exposing them as
T(Incomplete Fix).The previous review comment requested endpoint-specific guards for all payloads to reject null, non-object, and structurally incomplete responses. While session and balance responses were correctly addressed, the methods below still blindly trust
this.request<T>without validation or safe property access, risking NPEs on200 nullresponses and propagating invalid state. As per path instructions, review outbound HTTP behavior with high scrutiny.
src/integrations/aimlapi/client.ts#L173-L179: Add an endpoint-specific guard to ensure the response is a validAccountCheckResult.src/integrations/aimlapi/client.ts#L190-L201: Check thatresultis a non-null object before accessingresult.token.src/integrations/aimlapi/client.ts#L203-L210: Check thatresultis a non-null object before accessingresult.token.src/integrations/aimlapi/client.ts#L212-L227: Check thatresultis a non-null object before accessingresult.key.src/integrations/aimlapi/client.ts#L280-L308: Add an endpoint-specific guard to ensure the response contains validcheckoutandpartnerCheckoutobjects.src/integrations/aimlapi/client.ts#L310-L339: Add an endpoint-specific guard to ensure the response is a validTopUpByKeyResult.src/integrations/aimlapi/client.ts#L341-L350: Add an endpoint-specific guard to ensure the response is a validExchangeResult.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/aimlapi/client.ts` around lines 173 - 179, Validate all listed endpoint responses in src/integrations/aimlapi/client.ts before treating them as typed results: add endpoint-specific guards in checkAccount, the checkout flow, top-up flow, and exchange flow for valid AccountCheckResult, checkout/partnerCheckout objects, TopUpByKeyResult, and ExchangeResult; in the token/key methods, first verify result is a non-null object before accessing token or key. Apply the requested changes at ranges 173-179, 190-201, 203-210, 212-227, 280-308, 310-339, and 341-350, rejecting null, non-object, or structurally incomplete payloads rather than propagating them.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/aimlapi/client.test.ts`:
- Around line 203-222: Add AbortController regression coverage in the session
tests near “session methods reject a malformed or empty success payload” and in
the top-up polling tests: verify cancellation aborts the underlying fetch at the
transport layer and stops the polling flow at the polling layer, including the
expected rejection or termination behavior without continuing requests.
In `@src/integrations/aimlapi/client.ts`:
- Around line 102-111: Update isPartnerCheckoutSession to validate every
required string field declared by PartnerCheckoutSession, including id and
partnerId, in addition to sessionToken and status. Require each field to be a
non-empty string before returning true, while preserving the existing status
allowlist check.
---
Duplicate comments:
In `@src/integrations/aimlapi/client.ts`:
- Around line 173-179: Validate all listed endpoint responses in
src/integrations/aimlapi/client.ts before treating them as typed results: add
endpoint-specific guards in checkAccount, the checkout flow, top-up flow, and
exchange flow for valid AccountCheckResult, checkout/partnerCheckout objects,
TopUpByKeyResult, and ExchangeResult; in the token/key methods, first verify
result is a non-null object before accessing token or key. Apply the requested
changes at ranges 173-179, 190-201, 203-210, 212-227, 280-308, 310-339, and
341-350, rejecting null, non-object, or structurally incomplete payloads rather
than propagating them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71989574-ca6e-405c-b772-0045708b303c
📒 Files selected for processing (7)
docs/aimlapi-setup.mdsrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.tssrc/integrations/aimlapi/topup.test.tssrc/utils/providerProfiles.test.tssrc/utils/providerProfiles.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Runbun run typecheckandbun run typecheck:type-testsfor TypeScript changes.
Run provider tests and provider recommendation tests when changing provider behavior:bun run test:providerandbun run test:provider-recommendation.
Files:
src/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or feature and avoid mixing unrelated cleanup into the same change.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files.
Keep comments useful and concise.
Provider changes must explicitly identify affected providers, limitations, and follow-up work in the pull request description.
Do not assign or use provider tags; provider tags are controlled by maintainers.
Run the relevant validation checks locally before submitting changes; pull requests must pass CI checks.
Runbun run security:pr-scanbefore submitting a pull request.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Do not change the project's language, core runtime, or dependency stack without prior maintainer agreement.
Files:
src/utils/providerProfiles.tssrc/utils/providerProfiles.test.tsdocs/aimlapi-setup.mdsrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/providerProfiles.tssrc/utils/providerProfiles.test.tsdocs/aimlapi-setup.mdsrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/topup.test.tssrc/components/ProviderManager.tsxsrc/integrations/aimlapi/client.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/utils/providerProfiles.tssrc/utils/providerProfiles.test.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/topup.test.tssrc/integrations/aimlapi/client.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/providerProfiles.test.tssrc/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/topup.test.ts
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/aimlapi-setup.md
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/ProviderManager.tsx
🔇 Additional comments (5)
src/utils/providerProfiles.ts (1)
998-1014: LGTM!src/utils/providerProfiles.test.ts (2)
1013-1034: LGTM!
1036-1076: LGTM!src/integrations/aimlapi/topup.test.ts (1)
26-30: LGTM!src/components/ProviderManager.tsx (1)
3474-3499: LGTM!
|
First layer is up: #1995 (provider foundation — config, catalog, and the P1 ambient-key gate). |
…y flow (Twigpine#1988) Mid-port checkpoint. Rewrites the AI/ML API integration backend and CLI to the canonical passwordless, card-only design (target PR Twigpine#1988), in the current code style. NOTE: the branch does not yet compile — the GUI (ProviderManager passwordless rewire) and the aimlapi test suite still reference the removed API and are the remaining work. Done (typecheck-clean in these files): - config.ts: add payBaseUrl + verificationBaseUrl endpoints, buildPartnerReturnUrl. - topupState.ts: reduce to the Twigpine#1988 shape — intent keyed on payBaseUrl/ verificationBaseUrl (no `method`); sync lockfile-based state; drop the exchange-lease, discard/reset-command surface, async variants and fail-closed-on-corrupt (corrupt reads as absent). - client.ts: drop the password path (signup/login) and PaymentMethod/crypto; pay() is card-only. - topup.ts: rewrite to the passwordless phase-machine flow (checkAccount -> code sign-in / new-account -> provision or by-key top-up; resolveTopupSession; pollUntilPaid / pollUntilExchangeSettled / pollUntilByKeyToppedUp). Keeps the DI test seam (no cross-file mock.module). Profile env writes AIMLAPI_API_KEY mirror + CLAUDE_CODE_PROVIDER_ROUTE_ID. - onboarding.ts, messages.ts (canonical copy), validation.ts, index.ts (lean barrel), providerManagerAimlapi.ts (GUI indirection layer): new. - CLI: aimlapiCommand.ts (registerAimlapiCommand, --email/--code/--amount, no --method/reset), main.tsx wiring, handlers/aimlapi.ts (redacted errors). Mandatory attribution headers wired on EVERY aimlapi request: X-AIMLAPI-Source (agent/openclaude) + X-AIMLAPI-Partner-ID — in client.ts request() (auth/ checkout) and the config attribution path (inference/catalog).
Rebase ProviderManager.tsx on the Twigpine#1988 passwordless aimlapi flow (email -> 6-digit code -> low-balance -> top-up / paste-existing-key -> done), replacing the old password/method/Start-over flow and importing the aimlapi surface from providerManagerAimlapi.js. Re-apply the current newer-main, non-aimlapi `apiFormat: 'auto'` feature that the rebase would otherwise revert (form metadata, toDraft default, display label, startCreateFromPreset default, the API-format picker's Automatic option, and persistDraft's selectedApiFormat 'auto' -> undefined branch, kept alongside Twigpine#1988's deferNavigation/onSaved). Re-export resolveRouteCredentialValue from integrations/index for the GUI. All source now typechecks; the aimlapi test suite is the remaining work.
- topupState/topup/onboarding/aimlapiCommand tests: port Twigpine#1988's coverage, adapting the transport to `globalThis.fetch` stubbing and the profile/prompt doubles to the module's `setAimlapiTopupTestDoubles` DI seam (no process-global mock.module, which leaks across files in this repo). - client/config tests: keep the current repo's stricter versions (complete session contracts), pruning the removed password/crypto cases. - Add mandatory-attribution-header coverage on all four request classes: the client sends X-AIMLAPI-Source + X-AIMLAPI-Partner-ID on auth/checkout, and the config attribution path sends both on inference/catalog and strips them for a non-canonical proxy endpoint. - Remove the reset-based CLI handler test (reset no longer exists). Full aimlapi integration + CLI suite green (67 tests). ProviderManager GUI test is the remaining piece.
…pine#1988) Rebase ProviderManager.test.tsx on Twigpine#1988's version for the passwordless aimlapi GUI tests (email -> code -> low-balance -> top-up / paste-existing-key), which mock ./providerManagerAimlapi.js. Re-apply HEAD's newer-main apiFormat 'auto' test cases (API-mode picker, token field, OpenAI/GPT-5/MiniMax presets) since the source keeps that feature, and restore the current preset list ('LongCat') in the test's PRESET_ORDER so navigateToPreset indexes match the real presets. ProviderManager suite green (42 tests).
… flow (Twigpine#1988) Describe the passwordless /provider flow (saved-key continue, new-user email + 6-digit code, paste-existing-key, low-balance top-up) and the card-only CLI `aimlapi topup --email/--code/--amount` (no --method, no reset). Document the full endpoint override set and the two mandatory attribution headers (X-AIMLAPI-Source + X-AIMLAPI-Partner-ID) sent on every request, stripped for a non-canonical proxy endpoint.
…y flow (Twigpine#1988) Mid-port checkpoint. Rewrites the AI/ML API integration backend and CLI to the canonical passwordless, card-only design (target PR Twigpine#1988), in the current code style. NOTE: the branch does not yet compile — the GUI (ProviderManager passwordless rewire) and the aimlapi test suite still reference the removed API and are the remaining work. Done (typecheck-clean in these files): - config.ts: add payBaseUrl + verificationBaseUrl endpoints, buildPartnerReturnUrl. - topupState.ts: reduce to the Twigpine#1988 shape — intent keyed on payBaseUrl/ verificationBaseUrl (no `method`); sync lockfile-based state; drop the exchange-lease, discard/reset-command surface, async variants and fail-closed-on-corrupt (corrupt reads as absent). - client.ts: drop the password path (signup/login) and PaymentMethod/crypto; pay() is card-only. - topup.ts: rewrite to the passwordless phase-machine flow (checkAccount -> code sign-in / new-account -> provision or by-key top-up; resolveTopupSession; pollUntilPaid / pollUntilExchangeSettled / pollUntilByKeyToppedUp). Keeps the DI test seam (no cross-file mock.module). Profile env writes AIMLAPI_API_KEY mirror + CLAUDE_CODE_PROVIDER_ROUTE_ID. - onboarding.ts, messages.ts (canonical copy), validation.ts, index.ts (lean barrel), providerManagerAimlapi.ts (GUI indirection layer): new. - CLI: aimlapiCommand.ts (registerAimlapiCommand, --email/--code/--amount, no --method/reset), main.tsx wiring, handlers/aimlapi.ts (redacted errors). Mandatory attribution headers wired on EVERY aimlapi request: X-AIMLAPI-Source (agent/openclaude) + X-AIMLAPI-Partner-ID — in client.ts request() (auth/ checkout) and the config attribution path (inference/catalog).
Rebase ProviderManager.tsx on the Twigpine#1988 passwordless aimlapi flow (email -> 6-digit code -> low-balance -> top-up / paste-existing-key -> done), replacing the old password/method/Start-over flow and importing the aimlapi surface from providerManagerAimlapi.js. Re-apply the current newer-main, non-aimlapi `apiFormat: 'auto'` feature that the rebase would otherwise revert (form metadata, toDraft default, display label, startCreateFromPreset default, the API-format picker's Automatic option, and persistDraft's selectedApiFormat 'auto' -> undefined branch, kept alongside Twigpine#1988's deferNavigation/onSaved). Re-export resolveRouteCredentialValue from integrations/index for the GUI. All source now typechecks; the aimlapi test suite is the remaining work.
- topupState/topup/onboarding/aimlapiCommand tests: port Twigpine#1988's coverage, adapting the transport to `globalThis.fetch` stubbing and the profile/prompt doubles to the module's `setAimlapiTopupTestDoubles` DI seam (no process-global mock.module, which leaks across files in this repo). - client/config tests: keep the current repo's stricter versions (complete session contracts), pruning the removed password/crypto cases. - Add mandatory-attribution-header coverage on all four request classes: the client sends X-AIMLAPI-Source + X-AIMLAPI-Partner-ID on auth/checkout, and the config attribution path sends both on inference/catalog and strips them for a non-canonical proxy endpoint. - Remove the reset-based CLI handler test (reset no longer exists). Full aimlapi integration + CLI suite green (67 tests). ProviderManager GUI test is the remaining piece.
…pine#1988) Rebase ProviderManager.test.tsx on Twigpine#1988's version for the passwordless aimlapi GUI tests (email -> code -> low-balance -> top-up / paste-existing-key), which mock ./providerManagerAimlapi.js. Re-apply HEAD's newer-main apiFormat 'auto' test cases (API-mode picker, token field, OpenAI/GPT-5/MiniMax presets) since the source keeps that feature, and restore the current preset list ('LongCat') in the test's PRESET_ORDER so navigateToPreset indexes match the real presets. ProviderManager suite green (42 tests).
… flow (Twigpine#1988) Describe the passwordless /provider flow (saved-key continue, new-user email + 6-digit code, paste-existing-key, low-balance top-up) and the card-only CLI `aimlapi topup --email/--code/--amount` (no --method, no reset). Document the full endpoint override set and the two mandatory attribution headers (X-AIMLAPI-Source + X-AIMLAPI-Partner-ID) sent on every request, stripped for a non-canonical proxy endpoint.
…y flow (Twigpine#1988) Mid-port checkpoint. Rewrites the AI/ML API integration backend and CLI to the canonical passwordless, card-only design (target PR Twigpine#1988), in the current code style. NOTE: the branch does not yet compile — the GUI (ProviderManager passwordless rewire) and the aimlapi test suite still reference the removed API and are the remaining work. Done (typecheck-clean in these files): - config.ts: add payBaseUrl + verificationBaseUrl endpoints, buildPartnerReturnUrl. - topupState.ts: reduce to the Twigpine#1988 shape — intent keyed on payBaseUrl/ verificationBaseUrl (no `method`); sync lockfile-based state; drop the exchange-lease, discard/reset-command surface, async variants and fail-closed-on-corrupt (corrupt reads as absent). - client.ts: drop the password path (signup/login) and PaymentMethod/crypto; pay() is card-only. - topup.ts: rewrite to the passwordless phase-machine flow (checkAccount -> code sign-in / new-account -> provision or by-key top-up; resolveTopupSession; pollUntilPaid / pollUntilExchangeSettled / pollUntilByKeyToppedUp). Keeps the DI test seam (no cross-file mock.module). Profile env writes AIMLAPI_API_KEY mirror + CLAUDE_CODE_PROVIDER_ROUTE_ID. - onboarding.ts, messages.ts (canonical copy), validation.ts, index.ts (lean barrel), providerManagerAimlapi.ts (GUI indirection layer): new. - CLI: aimlapiCommand.ts (registerAimlapiCommand, --email/--code/--amount, no --method/reset), main.tsx wiring, handlers/aimlapi.ts (redacted errors). Mandatory attribution headers wired on EVERY aimlapi request: X-AIMLAPI-Source (agent/openclaude) + X-AIMLAPI-Partner-ID — in client.ts request() (auth/ checkout) and the config attribution path (inference/catalog).
Rebase ProviderManager.tsx on the Twigpine#1988 passwordless aimlapi flow (email -> 6-digit code -> low-balance -> top-up / paste-existing-key -> done), replacing the old password/method/Start-over flow and importing the aimlapi surface from providerManagerAimlapi.js. Re-apply the current newer-main, non-aimlapi `apiFormat: 'auto'` feature that the rebase would otherwise revert (form metadata, toDraft default, display label, startCreateFromPreset default, the API-format picker's Automatic option, and persistDraft's selectedApiFormat 'auto' -> undefined branch, kept alongside Twigpine#1988's deferNavigation/onSaved). Re-export resolveRouteCredentialValue from integrations/index for the GUI. All source now typechecks; the aimlapi test suite is the remaining work.
- topupState/topup/onboarding/aimlapiCommand tests: port Twigpine#1988's coverage, adapting the transport to `globalThis.fetch` stubbing and the profile/prompt doubles to the module's `setAimlapiTopupTestDoubles` DI seam (no process-global mock.module, which leaks across files in this repo). - client/config tests: keep the current repo's stricter versions (complete session contracts), pruning the removed password/crypto cases. - Add mandatory-attribution-header coverage on all four request classes: the client sends X-AIMLAPI-Source + X-AIMLAPI-Partner-ID on auth/checkout, and the config attribution path sends both on inference/catalog and strips them for a non-canonical proxy endpoint. - Remove the reset-based CLI handler test (reset no longer exists). Full aimlapi integration + CLI suite green (67 tests). ProviderManager GUI test is the remaining piece.
…pine#1988) Rebase ProviderManager.test.tsx on Twigpine#1988's version for the passwordless aimlapi GUI tests (email -> code -> low-balance -> top-up / paste-existing-key), which mock ./providerManagerAimlapi.js. Re-apply HEAD's newer-main apiFormat 'auto' test cases (API-mode picker, token field, OpenAI/GPT-5/MiniMax presets) since the source keeps that feature, and restore the current preset list ('LongCat') in the test's PRESET_ORDER so navigateToPreset indexes match the real presets. ProviderManager suite green (42 tests).
… flow (Twigpine#1988) Describe the passwordless /provider flow (saved-key continue, new-user email + 6-digit code, paste-existing-key, low-balance top-up) and the card-only CLI `aimlapi topup --email/--code/--amount` (no --method, no reset). Document the full endpoint override set and the two mandatory attribution headers (X-AIMLAPI-Source + X-AIMLAPI-Partner-ID) sent on every request, stripped for a non-canonical proxy endpoint.
…y flow (Twigpine#1988) Mid-port checkpoint. Rewrites the AI/ML API integration backend and CLI to the canonical passwordless, card-only design (target PR Twigpine#1988), in the current code style. NOTE: the branch does not yet compile — the GUI (ProviderManager passwordless rewire) and the aimlapi test suite still reference the removed API and are the remaining work. Done (typecheck-clean in these files): - config.ts: add payBaseUrl + verificationBaseUrl endpoints, buildPartnerReturnUrl. - topupState.ts: reduce to the Twigpine#1988 shape — intent keyed on payBaseUrl/ verificationBaseUrl (no `method`); sync lockfile-based state; drop the exchange-lease, discard/reset-command surface, async variants and fail-closed-on-corrupt (corrupt reads as absent). - client.ts: drop the password path (signup/login) and PaymentMethod/crypto; pay() is card-only. - topup.ts: rewrite to the passwordless phase-machine flow (checkAccount -> code sign-in / new-account -> provision or by-key top-up; resolveTopupSession; pollUntilPaid / pollUntilExchangeSettled / pollUntilByKeyToppedUp). Keeps the DI test seam (no cross-file mock.module). Profile env writes AIMLAPI_API_KEY mirror + CLAUDE_CODE_PROVIDER_ROUTE_ID. - onboarding.ts, messages.ts (canonical copy), validation.ts, index.ts (lean barrel), providerManagerAimlapi.ts (GUI indirection layer): new. - CLI: aimlapiCommand.ts (registerAimlapiCommand, --email/--code/--amount, no --method/reset), main.tsx wiring, handlers/aimlapi.ts (redacted errors). Mandatory attribution headers wired on EVERY aimlapi request: X-AIMLAPI-Source (agent/openclaude) + X-AIMLAPI-Partner-ID — in client.ts request() (auth/ checkout) and the config attribution path (inference/catalog).
Rebase ProviderManager.tsx on the Twigpine#1988 passwordless aimlapi flow (email -> 6-digit code -> low-balance -> top-up / paste-existing-key -> done), replacing the old password/method/Start-over flow and importing the aimlapi surface from providerManagerAimlapi.js. Re-apply the current newer-main, non-aimlapi `apiFormat: 'auto'` feature that the rebase would otherwise revert (form metadata, toDraft default, display label, startCreateFromPreset default, the API-format picker's Automatic option, and persistDraft's selectedApiFormat 'auto' -> undefined branch, kept alongside Twigpine#1988's deferNavigation/onSaved). Re-export resolveRouteCredentialValue from integrations/index for the GUI. All source now typechecks; the aimlapi test suite is the remaining work.
- topupState/topup/onboarding/aimlapiCommand tests: port Twigpine#1988's coverage, adapting the transport to `globalThis.fetch` stubbing and the profile/prompt doubles to the module's `setAimlapiTopupTestDoubles` DI seam (no process-global mock.module, which leaks across files in this repo). - client/config tests: keep the current repo's stricter versions (complete session contracts), pruning the removed password/crypto cases. - Add mandatory-attribution-header coverage on all four request classes: the client sends X-AIMLAPI-Source + X-AIMLAPI-Partner-ID on auth/checkout, and the config attribution path sends both on inference/catalog and strips them for a non-canonical proxy endpoint. - Remove the reset-based CLI handler test (reset no longer exists). Full aimlapi integration + CLI suite green (67 tests). ProviderManager GUI test is the remaining piece.
…pine#1988) Rebase ProviderManager.test.tsx on Twigpine#1988's version for the passwordless aimlapi GUI tests (email -> code -> low-balance -> top-up / paste-existing-key), which mock ./providerManagerAimlapi.js. Re-apply HEAD's newer-main apiFormat 'auto' test cases (API-mode picker, token field, OpenAI/GPT-5/MiniMax presets) since the source keeps that feature, and restore the current preset list ('LongCat') in the test's PRESET_ORDER so navigateToPreset indexes match the real presets. ProviderManager suite green (42 tests).
… flow (Twigpine#1988) Describe the passwordless /provider flow (saved-key continue, new-user email + 6-digit code, paste-existing-key, low-balance top-up) and the card-only CLI `aimlapi topup --email/--code/--amount` (no --method, no reset). Document the full endpoint override set and the two mandatory attribution headers (X-AIMLAPI-Source + X-AIMLAPI-Partner-ID) sent on every request, stripped for a non-canonical proxy endpoint.
…y flow (Twigpine#1988) Mid-port checkpoint. Rewrites the AI/ML API integration backend and CLI to the canonical passwordless, card-only design (target PR Twigpine#1988), in the current code style. NOTE: the branch does not yet compile — the GUI (ProviderManager passwordless rewire) and the aimlapi test suite still reference the removed API and are the remaining work. Done (typecheck-clean in these files): - config.ts: add payBaseUrl + verificationBaseUrl endpoints, buildPartnerReturnUrl. - topupState.ts: reduce to the Twigpine#1988 shape — intent keyed on payBaseUrl/ verificationBaseUrl (no `method`); sync lockfile-based state; drop the exchange-lease, discard/reset-command surface, async variants and fail-closed-on-corrupt (corrupt reads as absent). - client.ts: drop the password path (signup/login) and PaymentMethod/crypto; pay() is card-only. - topup.ts: rewrite to the passwordless phase-machine flow (checkAccount -> code sign-in / new-account -> provision or by-key top-up; resolveTopupSession; pollUntilPaid / pollUntilExchangeSettled / pollUntilByKeyToppedUp). Keeps the DI test seam (no cross-file mock.module). Profile env writes AIMLAPI_API_KEY mirror + CLAUDE_CODE_PROVIDER_ROUTE_ID. - onboarding.ts, messages.ts (canonical copy), validation.ts, index.ts (lean barrel), providerManagerAimlapi.ts (GUI indirection layer): new. - CLI: aimlapiCommand.ts (registerAimlapiCommand, --email/--code/--amount, no --method/reset), main.tsx wiring, handlers/aimlapi.ts (redacted errors). Mandatory attribution headers wired on EVERY aimlapi request: X-AIMLAPI-Source (agent/openclaude) + X-AIMLAPI-Partner-ID — in client.ts request() (auth/ checkout) and the config attribution path (inference/catalog).
Rebase ProviderManager.tsx on the Twigpine#1988 passwordless aimlapi flow (email -> 6-digit code -> low-balance -> top-up / paste-existing-key -> done), replacing the old password/method/Start-over flow and importing the aimlapi surface from providerManagerAimlapi.js. Re-apply the current newer-main, non-aimlapi `apiFormat: 'auto'` feature that the rebase would otherwise revert (form metadata, toDraft default, display label, startCreateFromPreset default, the API-format picker's Automatic option, and persistDraft's selectedApiFormat 'auto' -> undefined branch, kept alongside Twigpine#1988's deferNavigation/onSaved). Re-export resolveRouteCredentialValue from integrations/index for the GUI. All source now typechecks; the aimlapi test suite is the remaining work.
- topupState/topup/onboarding/aimlapiCommand tests: port Twigpine#1988's coverage, adapting the transport to `globalThis.fetch` stubbing and the profile/prompt doubles to the module's `setAimlapiTopupTestDoubles` DI seam (no process-global mock.module, which leaks across files in this repo). - client/config tests: keep the current repo's stricter versions (complete session contracts), pruning the removed password/crypto cases. - Add mandatory-attribution-header coverage on all four request classes: the client sends X-AIMLAPI-Source + X-AIMLAPI-Partner-ID on auth/checkout, and the config attribution path sends both on inference/catalog and strips them for a non-canonical proxy endpoint. - Remove the reset-based CLI handler test (reset no longer exists). Full aimlapi integration + CLI suite green (67 tests). ProviderManager GUI test is the remaining piece.
…pine#1988) Rebase ProviderManager.test.tsx on Twigpine#1988's version for the passwordless aimlapi GUI tests (email -> code -> low-balance -> top-up / paste-existing-key), which mock ./providerManagerAimlapi.js. Re-apply HEAD's newer-main apiFormat 'auto' test cases (API-mode picker, token field, OpenAI/GPT-5/MiniMax presets) since the source keeps that feature, and restore the current preset list ('LongCat') in the test's PRESET_ORDER so navigateToPreset indexes match the real presets. ProviderManager suite green (42 tests).
… flow (Twigpine#1988) Describe the passwordless /provider flow (saved-key continue, new-user email + 6-digit code, paste-existing-key, low-balance top-up) and the card-only CLI `aimlapi topup --email/--code/--amount` (no --method, no reset). Document the full endpoint override set and the two mandatory attribution headers (X-AIMLAPI-Source + X-AIMLAPI-Partner-ID) sent on every request, stripped for a non-canonical proxy endpoint.
#2032) * feat(aimlapi): add checkout state persistence and sign-in key cache * fix(aimlapi): cover lock recovery and complete the reset receipt * test(aimlapi): cover CAS result, reset receipt and sign-in key permissions * fix(aimlapi): make stale-lock recovery ownership-safe across processes * test(aimlapi): pass a file URL specifier to the lock workers * fix(aimlapi): use proper-lockfile and harden checkout-state persistence * fix(aimlapi): preserve issued keys and survive stale-lock steal races * fix(aimlapi): surface swallowed lock-retry conditions * feat(aimlapi): resume interrupted checkouts in the top-up entry points * fix(aimlapi): keep resumable checkouts through transient and paid states * fix(aimlapi): surface a lost settled-receipt write to the caller * fix(aimlapi): harden checkout resume across CLI resume, idempotency and transient errors * fix(aimlapi): resume settling checkouts and preserve records on ambiguous reads - Treat 'exchanging' as a paid/resumable status so a run interrupted between payment and receipt resumes the exchange instead of opening a second, chargeable checkout (matches pollUntilPaid). - Preserve the recorded checkout on a malformed-but-successful status read (AimlapiApiError status 200), alongside the existing transient-error path. - Print the full recovery key in the receipt-write-failed warning; a masked key is useless as a last copy. - Re-register the real client/providerProfile modules in afterAll so the test stubs cannot leak into later files (mock.restore does not undo mock.module). - Bound each lock worker's exit before draining its pipes, and loosen the held-lock timeout assertion to any errno (Windows is not always ELOCKED). * fix(aimlapi): stop mock leak, fail closed on spent sessions, clear GUI receipt - topup.test.ts: capture the real client/providerProfile modules through a cache-busting query so afterAll restores the genuine module instead of the corrupted (stub-mutated) reference. Without this the './client.js' stub bled past afterAll and failed 25 client.test.ts cases whenever it ran first. - resolveCheckoutSession: fail closed when a resumed session is already 'exchanged' but no settled receipt survived locally, matching pollUntilPaid, instead of opening a second chargeable checkout for a one-shot key that is already gone. - provisionAimlapiKey: return a clearReceipt closure; ProviderManager now calls it only after persistDraft actually saves, so a second GUI top-up opens a fresh checkout instead of short-circuiting to a stale key or throwing. - claimAimlapiTopupState: refuse to replace a stored record that still has an open resume token for a different intent, so a changed amount cannot strand a still-payable checkout and open a second one. - Mask the issued key in the CLI receipt-write-failed warning; a paid-for credential should not land in scrollback. - index.ts: note the live CLI/GUI callers and the clear-receipt obligation. - Regression coverage for exchanged fail-closed, the claim guard, and the GUI receipt clear. * fix(aimlapi): inject the topup transport instead of mocking client.js globally The prior mock.module('./client.js') stub in topup.test.ts leaked past its afterAll into client.test.ts (25 failures when this file ran first in CI, bun 1.3.13). Replace it with a local injection seam: - topup.ts exposes setAimlapiTopupTestDoubles, and both entry points create their client / write their profile through it (defaults unchanged, so production behaviour is identical). - topup.test.ts injects a stub transport through that seam and no longer calls mock.module at all, so nothing can bleed OUT to client.test.ts. - It still loads topup.js through a cache-busting ?ts= query so it stays immune to ProviderManager.test.tsx's mock.module('../integrations/aimlapi/index.js'), which mock.restore() does not undo and which would otherwise replace the shared provisionAimlapiKey binding (verified: without the query the barrel stub reaches this file and every topup test fails). * fix(aimlapi): converge racing checkouts and preserve state on ambiguous reads - Close a post-claim race: two runs of the same intent converge on one payment id, then each can open a session before either records one, and the second save overwrote the first's resume token (two payable checkouts). Add recordAimlapiCheckoutSession, a compare-and-swap that only records while the token is empty; resolveCheckoutSession adopts the winner's session and abandons the one it just opened, so pay() converges idempotently on a single charge. - Resume path now preserves the record on any ambiguous getSession error (transient, malformed-200, auth/4xx) and retires it only on a definitive 404/410 gone-session, instead of clearing on every non-transient error. - pollUntilPaid retries a malformed-but-successful (status 200) body instead of aborting, matching the resume path. - Regression coverage for the peer-records-first race, ambiguous-error preserve, 404 replace, and poll-retry-on-200. * fix(aimlapi): re-validate an adopted peer session before paying it When two racing runs converge and this one adopts the peer's recorded session, route that session through the same status classification as the initial resume: return it only while resumable, fail closed on 'exchanged', and surface a re-run error on any other terminal status instead of calling pay() on a dead session. Add regression coverage for an adopted session that is exchanged or cancelled in the race window. * test(aimlapi): make abandoned-lock recovery test deterministic The stale-lock recovery test asserted that all racing claims return the same payment id. Under a stale-lock steal two recoverers can briefly hold the lock and mint distinct ids, so that assertion was flaky in CI. A diverged claim is harmless — it is refused at its next compare-and-swap — so the invariant that actually matters is that exactly ONE checkout gets established. Drive the workers through the full claim->record flow and assert a single winner, which the single-slot store plus the record CAS guarantee deterministically. * fix(aimlapi): acquire checkout-state off the interactive thread and recover fresh orphans The interactive top-up flow acquired the checkout-state lock synchronously, parking the Ink event loop (UI, timers, SIGINT) on Atomics.wait for up to the 5s timeout, and that timeout was shorter than the 30s stale window so a lock orphaned by an interrupted holder could not be recovered on an immediate resume. - Add withStateLockAsync + async variants of the state mutators, sharing the same inner operations. It acquires with the timer-free lockSync (a sub-ms mkdir) but yields via await between retries, so the UI stays live, and its longer deadline (15s) covers the stale window so a fresh orphan is reclaimed once stale rather than timing out. Shrink the stale window to 8s (sub-ms sections never approach it) so recovery is quick. - Route topup.ts (CLI + GUI) through the async variants; provisioned.clearReceipt is now async, and ProviderManager awaits it best-effort so a cleanup failure cannot surface an error that invites a retry (a duplicate provider profile). - Regression coverage for async orphan recovery (mutation-checked: a deadline below the stale window fails to recover). Generous per-file test timeout since the now-async provisioning yields to a loaded runner's event loop. * test(aimlapi): cover the async state mutators and speed up the orphan-recovery test - Add thin contract tests for saveAimlapiTopupStateAsync (CAS accepted/rejected), recordAimlapiCheckoutSessionAsync (compare-and-swap on the empty resume token), and clearAimlapiTopupStateAsync (ownership-scoped clear); the CLI/GUI flow now routes through these and only claimAsync was exercised. - Back-date the orphaned lock to just inside the stale window so the async recovery test reclaims it in ~2s instead of burning the full 8s window. * fix(aimlapi): fail closed on corrupt state, deliver keys on receipt failure, add a reset action - Fail closed when the checkout-state file is present but unreadable/schema- invalid instead of reading it as absent: a claim would otherwise overwrite an open/paid checkout or an exchanged key and open a second chargeable one. - Never strand a one-shot exchanged key: a receipt-write that throws (lock timeout / fs / corrupt state), not just a lost CAS, is caught so the CLI still writes the profile and the GUI still returns the key; the post-delivery clear is best-effort too. - Add an explicit discard/reset escape hatch — discardAimlapiCheckoutState, an "openclaude aimlapi reset" command, and a GUI "Start over" on the top-up error screen — so a terminal checkout (whose resume token blocks a different intent) or a corrupt state file can be cleared without editing internal files. - Surface a failed GUI receipt retirement: retry a few times, then show a non-blocking warning instead of swallowing it silently. - Only chmod a config directory this flow actually created (via mkdirSync's return); never tighten a pre-existing OPENCLAUDE_CONFIG_DIR root. The state file's own 0600 mode protects the credential. - Tests for each, incl. corrupt/schema-invalid fail-closed, receipt-write-throw key delivery (CLI + GUI), discard semantics, retried/surfaced GUI cleanup, and a POSIX check that an existing config dir keeps its mode. * fix(aimlapi): surface the CLI recovery-receipt clear failure and cover the reset handler - The CLI finishCliTopup clear failure only went to logForDebugging (debug-only), invisible to the user, unlike the loud receipt-write warning and the GUI warning. Print a [warn] line pointing at "openclaude aimlapi reset" so a stranded receipt that blocks a different top-up is not hidden. - Add tests for the aimlapiReset CLI handler (discards a stored checkout / reports when there is nothing to discard). - Assert the CLI clear-failure warning is surfaced in the receipt-write-failure test (mutation-checked). - Document "openclaude aimlapi reset" and the GUI Start over recovery in docs/aimlapi-setup.md. * fix(aimlapi): protect a settled receipt from reset and bind method into the checkout identity - Discard (CLI reset / GUI Start over) no longer deletes a settled receipt — the only copy of a paid-for, one-shot key — unless forced. discard now returns 'discarded' | 'kept-settled' | 'none'; the CLI adds --force and the GUI refuses and points back at the recovering retry. - Add the payment method to AimlapiTopupIntent so a card->crypto (or reverse) restart is a different intent and cannot adopt the prior checkout's reused idempotency id on the wrong rail; covered by a changed-method resume test. - Narrow the sign-in-key cache: it is a persistence primitive for the follow-up guided passwordless flow with no in-tree consumer, so drop it from the public barrel and document the scope (kept in topupState.ts for that follow-up). - Regression + mutation coverage for the settled-receipt protection and the method-scoped intent. * fix(aimlapi): back off receipt-clear retries, expand the discard API, and test Start over - Add a 150ms backoff between the GUI clearReceipt() retries so the loop can actually ride out a lock-contention window instead of exhausting three back-to-back attempts. - Re-export AimlapiDiscardResult and add resetAimlapiCheckoutSessionAsync so barrel consumers can name the discard outcome and use a non-blocking reset, matching the other mutators. - Cover the GUI Start over recovery: it is 'r' (settings:retry) on the top-up error screen — the Settings context has no confirm:yes and Enter closes the panel; tests drive both the discard and the kept-settled refusal path. - Rename the test's stale-age constant (LOCK_AGE_WELL_PAST_STALE_MS) so it no longer reads as the source's 8s window. * fix(aimlapi): serialize the one-shot key exchange and recover receipts before login Address three checkout-state review findings: - [P1] Serialize the non-idempotent key exchange behind an exchange lease so racing same-intent processes mint and record the credential exactly once. The elected lease holder exchanges and records the settled receipt; a peer that loses the election waits for that receipt and resumes from it instead of exchanging in parallel; a lease abandoned by a crashed holder goes stale (past the client request timeout) and is reclaimed on a later attempt. The lease is released on a failed exchange so a retry is not blocked for the stale window. - [P1] Recover a settled local receipt BEFORE authenticating in both the CLI and guided entry points. A run interrupted after the one-shot exchange but before the profile write leaves the paid-for key only in that receipt; requiring a fresh login to reach it stranded the key whenever the password had changed or the auth service was down. The receipt read is side-effect free and needs no token, so it now runs first and only authenticates when a checkout must be created/resumed/exchanged. - [P2] Fix the guided-recovery key in the setup guide: Start over is bound to r, not Enter (Enter closes the settings panel). Covered by state-layer lease tests (acquire/held/stale-steal/settled/gone/ release), a two-process race asserting the exchange runs exactly once with the loser resuming the receipt, and no-auth-on-settled tests for both entry points. * test(aimlapi): harden the exchange-lease coverage Address review follow-ups on the exchange-lease tests (all test-only): - Gate the two-process race on the loser's own "waiting" status signal instead of a fixed sleep, and assert it fired, so the test deterministically exercises the held -> wait -> resume path rather than possibly reading settled directly. - Type seedPersistedState's overrides as Partial<AimlapiPersistedTopup> so a misspelled/wrong-typed seed key is a typecheck failure instead of a record that silently reroutes the test down another branch. - Cover re-acquiring your own lease (leaseOwner === owner) so the guard that keeps a caller from mistaking its own fresh lease for a live peer's — and self-blocking until the stale window — cannot regress unnoticed. * fix(aimlapi): fence a superseded profile write, protect unreadable state, resume the receipt model Address three checkout-state review findings: - [P1] Fence an in-flight checkout before it writes its key to the provider profile. If a reset (or a fresh top-up) replaces the stored slot while an abandoned flow is awaiting client.exchange(), that flow's settled-receipt CAS now misses; previously it still went on to write its stale key and could clobber the profile the new top-up created. recordSettledReceipt now reports recorded | superseded | errored, and the exchange path aborts (rejecting, and pointing the user at rotating the orphaned key) on `superseded` while still delivering on a transient `errored` so a paid-for key is never stranded. - [P1] Do not discard an unreadable/corrupt state file without --force. Such a file could be a damaged receipt holding the sole copy of a one-shot key, so discardAimlapiCheckoutState now returns `kept-unreadable` and keeps it unless forced — matching the safety promise (and existing settled-receipt protection) that reset never loses an issued key. The CLI and guided "Start over" surface the new outcome and point at `reset --force`; docs updated. - [P2] Use the settled receipt's model when a peer completed the exchange. The settled lease branch dropped lease.state.model, so a loser resuming another run's receipt configured its own --model instead of the one actually provisioned; it now propagates the receipt's model through both callers. Covered by: a superseded-mid-exchange fence test, corrupt-discard-needs-force tests (state layer + CLI + guided GUI), and a two-process race asserting the loser adopts the winner's provisioned model. All three are mutation-proven. * test(aimlapi): drop the flaky third guided Start-over drive test The guided "Start over on a kept-unreadable discard" test added a third consecutive full Ink mount+drive to ProviderManager.test.tsx. On the CI-pinned bun (1.3.13) that destabilises the Ink stdin harness (`stdin.ref is not a function`), so the 'r' keypress never reaches the handler and the awaited discard call never fires — a timeout unrelated to the code under test (it passes on local bun 1.3.14). The two-drive configuration is green on CI. The kept-unreadable behaviour stays covered where it is deterministic: the state layer (kept-unreadable unforced, discarded on --force) and the CLI handler (the `reset --force` guidance). The guided keybinding→discard→refusal plumbing is covered by the identical-structure kept-settled drive test; the kept-unreadable GUI branch is a trivial mirror of it. A comment records why the third drive is intentionally omitted. * wip(aimlapi): converge integration + CLI to the passwordless card-only flow (#1988) Mid-port checkpoint. Rewrites the AI/ML API integration backend and CLI to the canonical passwordless, card-only design (target PR #1988), in the current code style. NOTE: the branch does not yet compile — the GUI (ProviderManager passwordless rewire) and the aimlapi test suite still reference the removed API and are the remaining work. Done (typecheck-clean in these files): - config.ts: add payBaseUrl + verificationBaseUrl endpoints, buildPartnerReturnUrl. - topupState.ts: reduce to the #1988 shape — intent keyed on payBaseUrl/ verificationBaseUrl (no `method`); sync lockfile-based state; drop the exchange-lease, discard/reset-command surface, async variants and fail-closed-on-corrupt (corrupt reads as absent). - client.ts: drop the password path (signup/login) and PaymentMethod/crypto; pay() is card-only. - topup.ts: rewrite to the passwordless phase-machine flow (checkAccount -> code sign-in / new-account -> provision or by-key top-up; resolveTopupSession; pollUntilPaid / pollUntilExchangeSettled / pollUntilByKeyToppedUp). Keeps the DI test seam (no cross-file mock.module). Profile env writes AIMLAPI_API_KEY mirror + CLAUDE_CODE_PROVIDER_ROUTE_ID. - onboarding.ts, messages.ts (canonical copy), validation.ts, index.ts (lean barrel), providerManagerAimlapi.ts (GUI indirection layer): new. - CLI: aimlapiCommand.ts (registerAimlapiCommand, --email/--code/--amount, no --method/reset), main.tsx wiring, handlers/aimlapi.ts (redacted errors). Mandatory attribution headers wired on EVERY aimlapi request: X-AIMLAPI-Source (agent/openclaude) + X-AIMLAPI-Partner-ID — in client.ts request() (auth/ checkout) and the config attribution path (inference/catalog). * wip(aimlapi): port the passwordless provider-manager GUI (#1988) Rebase ProviderManager.tsx on the #1988 passwordless aimlapi flow (email -> 6-digit code -> low-balance -> top-up / paste-existing-key -> done), replacing the old password/method/Start-over flow and importing the aimlapi surface from providerManagerAimlapi.js. Re-apply the current newer-main, non-aimlapi `apiFormat: 'auto'` feature that the rebase would otherwise revert (form metadata, toDraft default, display label, startCreateFromPreset default, the API-format picker's Automatic option, and persistDraft's selectedApiFormat 'auto' -> undefined branch, kept alongside #1988's deferNavigation/onSaved). Re-export resolveRouteCredentialValue from integrations/index for the GUI. All source now typechecks; the aimlapi test suite is the remaining work. * test(aimlapi): port integration + CLI tests to the passwordless flow - topupState/topup/onboarding/aimlapiCommand tests: port #1988's coverage, adapting the transport to `globalThis.fetch` stubbing and the profile/prompt doubles to the module's `setAimlapiTopupTestDoubles` DI seam (no process-global mock.module, which leaks across files in this repo). - client/config tests: keep the current repo's stricter versions (complete session contracts), pruning the removed password/crypto cases. - Add mandatory-attribution-header coverage on all four request classes: the client sends X-AIMLAPI-Source + X-AIMLAPI-Partner-ID on auth/checkout, and the config attribution path sends both on inference/catalog and strips them for a non-canonical proxy endpoint. - Remove the reset-based CLI handler test (reset no longer exists). Full aimlapi integration + CLI suite green (67 tests). ProviderManager GUI test is the remaining piece. * test(aimlapi): port the passwordless provider-manager GUI tests (#1988) Rebase ProviderManager.test.tsx on #1988's version for the passwordless aimlapi GUI tests (email -> code -> low-balance -> top-up / paste-existing-key), which mock ./providerManagerAimlapi.js. Re-apply HEAD's newer-main apiFormat 'auto' test cases (API-mode picker, token field, OpenAI/GPT-5/MiniMax presets) since the source keeps that feature, and restore the current preset list ('LongCat') in the test's PRESET_ORDER so navigateToPreset indexes match the real presets. ProviderManager suite green (42 tests). * docs(aimlapi): rewrite the setup guide for the passwordless card-only flow (#1988) Describe the passwordless /provider flow (saved-key continue, new-user email + 6-digit code, paste-existing-key, low-balance top-up) and the card-only CLI `aimlapi topup --email/--code/--amount` (no --method, no reset). Document the full endpoint override set and the two mandatory attribution headers (X-AIMLAPI-Source + X-AIMLAPI-Partner-ID) sent on every request, stripped for a non-canonical proxy endpoint. * refactor(aimlapi): let tests inject prompt doubles into the top-up flow * fix(aimlapi): lock the partner id and complete mandatory-header coverage Lock the partner id to OpenClaude's own attribution id: drop the --partner-id CLI flag and the AIMLAPI_PARTNER_ID env override so rebate/revenue-share attribution can never be redirected. resolvePartnerId() now always returns the built-in id; the mandatory X-AIMLAPI-Partner-ID header itself is unchanged. Assert the mandatory X-AIMLAPI-Source header on the catalog/discovery and inference paths (discoveryService, bootstrap, runtimeMetadata) — the header was already sent, only the test expectations lagged. Refresh stale password-era copy in the interactive prompt and a top-up comment left over from the removed flow. * fix(aimlapi): address CodeRabbit review — settlement, key-safety, redaction - Wait for a resumed sign-in top-up to settle before returning: the account non-exchange path now mirrors the by-key flow, so a credited balance is never reported while the billing operation is still in flight. - Preserve a freshly minted sign-in key when the balance read is aborted, so an abort cannot orphan a paid credential and mint a second key on the next run. - Clear the first-run env-key adoption markers when validation fails, so a retry re-validates instead of short-circuiting into persisting the unvalidated key. - Never crash the top-up success screen on an amount parse edge — fall back to the raw entered amount after the payment has already cleared. - Show all four API-format options (visibleOptionCount 3 -> 4). - Document that guided provisioning requires the canonical inference endpoint. - Add regression tests: resumed-sign-in settlement, aborted-balance key retention, failed-env-key re-validation, and CLI error redaction. * test(aimlapi): wait for the masked code frame instead of a fixed delay The AIMLAPI code-screen assertion captured output after a fixed 25ms sleep, which is too short on a slower CI runner (Node 24) and intermittently missed the freshly rendered mask characters. Wait for the masked frame instead so the assertion is deterministic. * test(aimlapi): harden the provider-manager GUI flows against CI timing The GUI top-up flow intermittently failed on a loaded CI runner: the final keystroke on the success screen was sent in the same tick as the render, before Ink attached the input handler, so it was dropped and the flow stranded on the done screen. Add the same input settle the other steps already use. Also raise the shared waitForCondition default timeout (2s -> 5s). The predicate is polled every 10ms and returns as soon as it is met, so this only adds patience for a slow runner and never slows a passing wait — keeping the Ink-driven flows deterministic under CI load. * fix(aimlapi): gate attribution headers by trusted AI/ML API host The client sent the mandatory source/partner headers on every request, but the auth/app/pay/inference base URLs are all env-overridable — so a request pointed at a user proxy (notably the balance probe against an overridden inference URL) leaked OpenClaude's partner/source identity. Send them only when the resolved request host is aimlapi.com (production or staging, over HTTPS), mirroring the inference/catalog stripping contract in resolveAimlapiAttributionHeaders. Adds an isTrustedAimlapiRequestUrl predicate plus canonical-sends / proxy-withholds regression tests. * test(aimlapi): wait for the done screen to settle before the final keystroke Replace the fixed 25ms delay before the success-screen keystroke with an observable frame-stability wait, so a slow CI runner cannot drop the keystroke before Ink has committed the render and attached its input handler. * fix(aimlapi): durable receipts and atomic election for concurrent top-ups - Restore the atomic checkout-session election dropped during the passwordless convergence: recordAimlapiCheckoutSession is a first-writer-wins CAS, so two concurrent runs of the same intent settle on ONE payable checkout — a loser adopts the winner's token and abandons the session it just opened instead of leaving two chargeable checkouts. Wired through resolveTopupSession (the create branch elects then adopts; the resume branch notifies once) and both the CLI and GUI onSession callbacks. - Persist the settled receipt (apiKey / apiKeyId / model / settled) in the GUI BEFORE the profile write, so an interrupted or failed write resumes with the paid, one-shot exchanged key instead of stranding it (mirrors the CLI). - Clear the sign-in key cache with the just-minted key id on a sufficient-balance sign-in: persistDraft runs onSaved synchronously, so the aimlapiIssuedKeyId state setter has not applied yet — pass the id explicitly. Adds regression tests for the election (first-writer-wins + loser adoption), the settled-receipt ordering, and the sufficient-balance cache clear. * fix(aimlapi): abort on a lost election and keep the receipt write best-effort - Treat a null recordAimlapiCheckoutSession result as "the slot was cleared by a sibling that already completed this top-up" and abort, instead of silently proceeding to pay a second, unrecorded checkout (both the CLI persistSession and the GUI reportSession). Closes the residual double-charge race. - Make the GUI settled-receipt write best-effort: the payment already cleared, so a receipt-write failure (lock contention, full/read-only disk) must not divert the flow into the top-up error path — the profile write is what matters. - Align the recordAimlapiCheckoutSession test double with the real semantics: match on intent + payment id only and return null on a non-matching slot. Adds a regression test that a sibling clearing the checkout mid-flow aborts before any /pay call. The sufficient-balance sign-in test now waits for the code screen to settle before typing (the transition dropped the first keystroke). * test(aimlapi): settle after each awaited frame so keystrokes aren't dropped The provider-manager GUI tests type on the line after waitForFrameOutput matches a new screen, but Ink registers input handlers in an effect that runs after the render commits. On a loaded CI runner the first post-transition keystroke could be dropped, stranding the flow and timing out (seen intermittently on Node 22). Add a short settle after every frame match — returning the same matched frame, so no assertion changes — which lets the input handler attach before the caller types. Fixes the class instead of patching individual call sites. * fix(aimlapi): recover a settled GUI receipt and harden checkout/key edge cases - Recover a settled checkout receipt in the provider-manager GUI before provisioning: if a prior run paid + exchanged the key and saved the receipt but was interrupted before the profile write, finish that write with the retained key instead of re-entering provisioning against the now-exchanged session (which fails in resolveTopupSession and strands the paid credential). Mirrors the CLI. - Reject a non-HTTPS checkout payUrl at the client response boundary (the validator required only "openable"), so a session is never retained with an address the flow refuses later and then polls with no usable link. - Do not discard a freshly minted sign-in key when its cache write fails: copy the key into memory before persisting and make the sign-in-cache / top-up-state writes best-effort, in both the GUI and the CLI, so a lock/permission/disk failure can't force a second key on retry. - Narrow the setup guide: the canonical-inference requirement applies to new-account onboarding + key provisioning; the existing-key top-up runs against the configured endpoint. Adds regression tests for the settled-receipt recovery (no re-provision) and the non-HTTPS payUrl rejection. * fix(aimlapi): HTTPS checkout callbacks, per-email key cache, safer edges - Require a credential-free HTTPS base for the checkout return URLs (they embed the resumable session token) and for the browser return/landing URL, so a cleartext AIMLAPI_PAY_URL/AIMLAPI_RETURN_URL override can't leak the token or break the documented HTTPS return-target contract. - Store sign-in recovery keys as an email-keyed collection instead of a single global record, so a concurrent/interrupted sign-in for one account no longer evicts another's key (which forced a duplicate mint). Old single-record files migrate on read; clear stays per-email ownership-aware. - Treat post-success receipt cleanup as best-effort in both the CLI (finishProfile) and the GUI (resetAimlapiCheckoutIntent): the profile is already saved, so a lock/permission/IO failure clearing the receipt must not report failure. - Reject scientific-notation amounts: parseAimlapiAmountUsd now requires a plain decimal with at most two fractional digits, closing the "20.001e0" sub-cent bypass that silently rounded to a wrong charge. - Add the pay/verification/return env vars to the config-test snapshot so a set override can't pollute default-endpoint assertions. Adds regression tests for each. * fix(aimlapi): async checkout-state clear for the Ink flow + reject malformed bases - Clear the checkout receipt through an async lock in the provider-manager GUI: restore withStateLockAsync + clearAimlapiTopupStateAsync and fire it best-effort (unawaited) from the save callback, so a contended lock no longer blocks Ink input/timers/SIGINT after the profile is already saved. The CLI keeps the sync clear (one-shot command). - Reject a query string or fragment in the checkout/return base URLs: a base like https://pay.aimlapi.com/#x would swallow the appended /checkout?...sessionToken into the fragment, so the token never reaches the callback as a query param. - Surface a non-fatal CLI note when receipt cleanup fails (the profile is already saved and the stale receipt reconciles on the next run). Adds regression tests: async clear ownership, query/fragment rejection, and the legacy single-record sign-in-cache migration. * fix(aimlapi): reject bare ?/# delimiters in checkout and return base URLs url.search / url.hash are empty for a bare delimiter (e.g. https://pay.aimlapi.com/? or .../#), so those slipped past the query/fragment guard and still corrupted the appended /checkout?...sessionToken=... . Reject any raw ?/# in the candidate in both safeHttpsBaseUrl and requireHttpsBaseUrl, and cover the bare-delimiter cases. * fix(aimlapi): harden checkout recovery — exchange lease, retry modes, payable guard Addresses a fresh review round on the checkout state machine: - Restore the cross-process exchange lease (dropped in the passwordless convergence): the one-shot key exchange is serialized so two processes resuming the same paid sign-up session cannot both exchange and strand the credential — the lease winner exchanges, peers wait for its settled receipt. - Persist the exchange mode in the receipt so a retry that has since become sign-in still exchanges the paid session instead of minting an unrelated key and clearing the paid checkout (CLI + GUI). - Recover the checkout URL on a pending_payment resume by re-issuing the idempotent pay/top-up (the stable paymentSessionId prevents a double charge) instead of polling a session the user can never open. - Route settled-receipt recovery through persistExistingAimlapi for an existing saved profile / AIMLAPI_API_KEY top-up, so it updates the selected profile (preserveEnv) rather than minting a new one and copying the env key. - Confirm before abandoning an already-open checkout: editing amount/auto-top-up after a checkout URL was opened now requires an explicit re-submit (the old browser tab stays chargeable and no endpoint can cancel it). - Treat a credentials/query/fragment inference base as non-canonical so a `.../v1#x` override cannot be written as OPENAI_BASE_URL and break the shim. Tests: exchange-lease election + failed-exchange release, retry-exchanges-the- paid-session, idempotent URL recovery on resume, canonical-gate rejection, and the re-edit confirmation. * fix(aimlapi): repair exchange-lease liveness and the re-edit abandon guard - exchange lease: a peer that finds a live foreign lease now re-attempts on each poll instead of only watching for a settled receipt, so it resumes the moment the holder settles OR frees the lease (failed/crashed) rather than hanging the full 20-minute poll window; folds the wait into the lease loop. - exchange lease: treat a future-dated exchangeLeaseAt (backwards clock jump or an edited state file) as stale and reclaim it, instead of reading a negative age as perpetually fresh and deadlocking every peer. - re-edit guard: reset the abandon acknowledgement when a new checkout opens so a further edit to a different amount/auto-top-up is confirmed again instead of silently abandoning the freshly-opened chargeable tab; clear the opened-checkout tracking once payment settles so a later re-edit never warns about a paid tab. - tests: lease release is owner-scoped and preserves a settled receipt; a future-dated lease is reclaimed; the GUI re-edit warning re-arms after a second edit. * test(aimlapi): sync re-edit test on rendered amount; guard vacuous lease seed - re-edit GUI test: submit only once the edited amount is reflected in the rendered frame instead of after a fixed 25ms delay, so Enter is never processed against the stale amount on a slow runner. - future-dated lease test: assert the seed compare-and-swap actually persisted the lease before acquiring, so the reclaim path can never pass vacuously. - exchange lease: record a swallowed release failure via file-backed debug logging (safe on the Ink GUI path) so a lock/permission problem behind a slow takeover is diagnosable. * test(aimlapi): match the complete edited amount in the re-edit frame wait Prefix matching let "$250" match a stray "$2500" (and "$2500" match "$25000"), so a wrong-amount input regression could pass unnoticed. Pin the complete value with a negative lookahead on a trailing digit. * fix(aimlapi): make the one-shot key exchange crash-durable and per-operation Three checkout-recovery correctness fixes: - Persist the exchanged key under the CAS BEFORE returning it. The lease winner used to hand the /exchange key to the caller, which wrote the receipt only afterward; a crash in between left the checkout exchanged but its only key unpersisted, so a retry re-ran (and was rejected by) the spent one-shot exchange. exchangeKeyWithLease now records the settled receipt via recordAimlapiSettledKeyAsync (merges over the record, clears the lease) as soon as the exchange succeeds. - Use a per-operation exchange-lease owner instead of a module-global id. Two overlapping top-ups in the same process shared one owner, which the acquire treats as self and immediately reclaims, so both could POST the non-idempotent /exchange concurrently. A fresh owner per operation makes the second observe the first's lease as foreign and back off; a retry within one operation keeps its owner and still reclaims the lease it released. - Never serialize an empty apiKey/apiKeyId. The existing-key top-up path reports apiKeyId: '', which the reader rejects, making the whole settled receipt (and the paid key it records) unrecoverable. The save path now coerces an empty key/id to absent so the receipt stays readable. * fix(aimlapi): refuse to overwrite an unfinished checkout when the intent changes claimAimlapiTopupState backs a single slot, so rerunning with a different amount, auto-top-up, or endpoint used to unconditionally replace the stored record. When the prior checkout had opened a session (a resume token — possibly already paid but not yet exchanged) or held a settled key not yet written to a profile, that dropped the only handle to a paid session/key and stranded it permanently. claim now refuses a changed intent while such a record exists, with an actionable message to finish or cancel the earlier top-up first (re-running the same intent still resumes it). A never-advanced claim — empty resume token, unsettled, no key — is still replaced. The CLI surfaces the message directly; the interactive flow already clears the prior record on edit, so normal re-edits are unaffected. * fix(aimlapi): never settle a keyless receipt; keep the paid key reaching the profile Addresses a further review batch: - recordAimlapiSettledKeyAsync now refuses to mark a receipt settled (and clear the lease) when no key resolves from the call or the stored record. A keyless settled receipt would make a peer resume from a spent one-shot exchange with no credential; the record and its lease now survive so a retry can still exchange. - The CLI's pre-profile settled-receipt save is now best-effort (try/catch + a dim note), matching the earlier saves. A lock/permission/IO failure there no longer throws before finishProfile, so the paid, exchanged key still reaches the provider profile. - startCreateFromPreset drops aimlapiPersistedIntentRef on a fresh flow entry (in-memory only) so a later resetAimlapiCheckoutIntent can never clear a previous flow's on-disk receipt against a stale payment id. - Prompt copy: "Do you have an aimlapi.com key?" / "I already have an aimlapi.com key" (missing article). Tests: keyless settle is rejected and leaves the lease intact; the CLI forwards explicit --amount/--model; the settled-receipt recovery renders the top-up (not "ready") done copy. * test(aimlapi): assert the exchange lease stays held on a keyless settle attempt Tighten the keyless-settlement guard test: a "not settled" assertion also passes if the lease were wrongly cleared (a peer would then see 'acquired'). Assert the peer acquisition returns 'held' so the test pins that a keyless settle preserves the lease for a retry. * fix(aimlapi): poll the checkout token, not the auth bearer, while waiting on a resumed exchange pollUntilExchangeSettled was called with the passwordless-auth bearer instead of the partner checkout-session token, so it polled the wrong resource. A terminal error there clears the recovery receipt, stranding a paid one-shot sign-up exchange. * fix(aimlapi): keep the checkout receipt resumable through an unconfirmed amount/auto-top-up edit Editing the amount or auto-top-up cleared the persisted checkout intent and durable receipt immediately, before the abandon-ack confirmation that gates actually starting a new payment session. A user who edits and backs out (or completes the still-open browser checkout) before confirming lost the only mapping to that chargeable checkout, so a later run would open a new one instead of resuming the paid session. The reset now happens only once the user has explicitly confirmed abandonment: claimAimlapiTopupState takes an `abandonExisting` option that atomically overwrites the retained record under the same lock acquisition, instead of racing a separate async clear against a synchronous claim. * test(aimlapi): sync on the rendered email before submitting in the new receipt-resume test A fixed sleep doesn't guarantee the TextInput has processed the typed email before Enter is sent; a loaded runner can drop the submit. Wait for the typed value to actually render, matching the amount-edit sync already used later in this same test. * docs(aimlapi): describe checkout retention as durable, not session-scoped The prior wording ("retained while the provider flow remains open") undersold what topupState.ts actually does: the payment identity and any issued key are persisted to disk, so a restart resumes the same checkout too, and a prior paid+exchanged run finishes the profile write on the next run instead of re-provisioning. * fix(aimlapi): unify error-status extraction, drop dead top-up state, tighten wrappers - Extract aimlapiApiErrorStatus as the one place that reads an HTTP status off a caught error, structurally (not `instanceof AimlapiApiError`) since some callers surface a duck-typed error with a bolted-on `status` instead of the real class; use it at both call sites that previously duplicated (and disagreed on) this check. - Remove aimlapiPaymentSessionId/isAimlapiTopupRunning: both were write-only state (declared with a blank destructure slot, never read), so every setter call scheduled a render for no observable effect. - Switch providerManagerAimlapi.ts's wrappers to `...args` forwarding so an implementation gaining a parameter can't silently get it dropped by a wrapper that still names the old ones positionally. - Stop exporting pollUntilPaid from the aimlapi barrel; nothing imports it through there (topup.test.ts imports it directly from topup.js), so keep it out of the public surface. - Normalize the email key while rebuilding the sign-in key store's collection branch on read, matching the legacy single-record migration branch right above it - a hand-edited or older-build file with a mixed-case key would otherwise be invisible to loadAimlapiSignInKey and mint a duplicate key. * test(aimlapi): cover resetAimlapiCheckoutSession, by-key top-up args, and error edges - resetAimlapiCheckoutSession: refreshes the payment session while preserving a minted key, and is a no-op when there's no key to preserve. - ProviderManager: a low-balance saved key that gets topped up charges the EXISTING key via topUpAimlapiByApiKey (apiKey, non-empty paymentSessionId, empty resumeSessionToken) instead of opening a new passwordless-account checkout - previously only exercised through the default test mock, with no assertion on the call. - The three negative assertions in the top-up progress-frame check tested strings that don't exist anywhere in this GUI (CLI-only or pure invention), so they could never fail; add a check against the real failure copy so a regression that silently fails at that point is actually caught. - CLI: pin the --no-open default (false) when the flag is absent, and cover the non-Error (thrown string) branch of the handler's credential-redaction path - both previously only exercised through the Error/AimlapiApiError branches. * fix(aimlapi): close checkout-state concurrency and exchange-lease races - saveAimlapiTopupState now merges resumeSessionToken like the other retained fields instead of spreading the caller's value verbatim. A caller saves this record at points where its in-memory copy is still empty (right after sign-in, before a checkout session exists); a concurrent peer running the same intent can have already elected and recorded a real token in that window, and the unconditional spread was overwriting it with "", stranding the peer's chargeable checkout. - The exchange lease is sized for a single POST (EXCHANGE_LEASE_STALE_MS, 75s) but a resumed wait-exchange holder can sit in a read-only poll for up to POLL_TIMEOUT_MS (20 minutes) before ever reaching that POST. Without refreshing, a peer would see the lease go stale mid-wait, reclaim it, and risk a second concurrent /exchange on the same one-shot session. Add refreshAimlapiExchangeLeaseAsync and call it every poll iteration. - When a peer finishes /exchange and records the settled key WHILE this process holds the lease and is polling/exchanging, the poll seeing the session flip to 'exchanged' threw a hard failure instead of resuming from that peer's settled receipt. Re-check for a settled receipt before releasing the lease and rethrowing. - claimAimlapiTopupState's abandonExisting no longer drops an already-minted (but not yet paid) existing-account key when overwriting a retained checkout for a different amount/auto-top-up - it now merges apiKey/apiKeyId/model in, matching resetAimlapiCheckoutSession's retain-key pattern. A fully settled (paid + exchanged) credential is refused unconditionally regardless of abandonExisting, since that confirms giving up an UNPAID checkout, never an already-paid one. * fix(aimlapi): guard GUI checkout abandonment and receipt recovery - The abandon-ack gate only armed once a checkout URL surfaced (aimlapiOpenedCheckoutRef), but resolveTopupSession can already elect and persist a resumeSessionToken before that point. Backing out in that window then editing the amount hit claimAimlapiTopupState's generic refusal instead of the same confirm-to-abandon flow. Extend the gate to also cover a persisted (not yet opened) intent. - Persist an existing-account key minted at sign-in into the top-up receipt itself (mirrors the CLI), not just the separate sign-in-key cache, so a restart before settlement can resume from one self-contained record instead of depending on two files staying consistent. - reportSession's terminal branch (a cancelled/expired/dead session) always fully wiped the receipt; mirror the CLI's persistSession, which retains an already-minted key (fresh payment session, dead token dropped) and only falls back to a full clear when there's no key to keep. - Submitting the email screen unconditionally reset the whole onboarding identity, silently abandoning a chargeable checkout on an accidental Esc-back-and-resubmit of the same email. Require the same explicit confirmation an amount edit does when a resumable checkout exists. - "Set up a new key or switch account" only cleared in-memory fields, leaving a durable receipt from an earlier interrupted top-up (this mount's refs were never populated for it, since it may be from an earlier process) to hit the same refusal on the next onboarding attempt with no way to recover short of deleting the file by hand. Force the next claim to override it once. - existingAimlapiCredential() rejected saved-profile discovery whenever the AMBIENT AIMLAPI_INFERENCE_URL wasn't canonical, even for a profile that was itself saved against the canonical endpoint. Narrow the canonical requirement to what it's actually protecting: reading the ambient env key, and sending a saved key to a non-canonical endpoint (the existing per-profile check). - The post-signup success screen claimed a magic link was emailed; this flow is passwordless email-code sign-in, no magic link is ever sent. Point at the dashboard instead. * docs(aimlapi): note the interactive/CLI auto-top-up default mismatch The guided GUI flow pre-selects auto-top-up on; the CLI's --auto-top-up only enrolls when explicitly passed. Left both defaults as-is (auto-top-up is a real billing behavior, not something to flip unilaterally) and documented the asymmetry so it's not a surprise either way. * test(aimlapi): sync on the settled frame before confirming switch-account A fixed sleep doesn't prove the Select's focus actually moved to the second option; on a loaded runner the following Enter could land on "Continue with your saved API key" instead and assert against the wrong branch. Wait for the frame to stop changing, matching the settle-poll pattern already used elsewhere in this file. * fix(aimlapi): stop the exchange poll when a peer reclaims the lease The periodic lease refresh added to pollUntilExchangeSettled discarded its result (`.catch(() => false)`), so a peer reclaiming the lease mid-wait was silently ignored: the poll kept going, returned normally once the session left 'exchanging', and the caller walked straight into the non-idempotent /exchange POST with no ownership check of its own — racing whatever the peer was doing with the same one-shot session. The comment claiming this was safe ("resolves on this function's next outer retry") was simply wrong: there is no outer retry on the success path, control goes directly to the POST. Distinguish a thrown refresh (transient lock contention — best-effort, retry next iteration) from an explicit `false` result (the lease is definitively no longer ours) and bail out on the latter, so the caller's existing catch block re-checks for the peer's settled receipt (or fails the run, requiring a re-run) instead of racing it. * test(aimlapi): require the settle-wait frame to actually differ from before the keypress waitForCondition polls every 10ms; on a loaded runner two consecutive polls can both land before Ink has processed the keypress at all, so the "stable frame" check was satisfied by the unchanged PRE-keypress frame, sending Enter before focus ever moved to the second option. Snapshot the frame before the keypress and require the settled frame to differ from it, not just be internally stable. * fix(aimlapi): elect the retained key atomically, stop blocking Ink on claim - Two concurrent sign-ins for the same intent could each mint their own existing-account key before either save landed, and saveAimlapiTopupState (last-writer-wins) let whichever saved last silently overwrite the other's key on disk while both runs kept using their own in-memory copy. Elect apiKey/apiKeyId first-writer-wins (same as resumeSessionToken already is), re-check the receipt right before minting so a losing run adopts the winner's key instead of minting a second, and re-check again after a save that lost the election so the run's own in-memory key matches what's actually on disk. Apply the same first-writer-wins election to the separate GUI sign-in-key cache (saveAimlapiSignInKey), which had the identical last-writer-wins gap. - The GUI called the sync claimAimlapiTopupState directly from an event handler; its lock retry blocks the whole event loop (Atomics.wait) for up to LOCK_TIMEOUT_MS on contention, freezing Ink rendering, Esc, and SIGINT — exactly what resetAimlapiCheckoutSessionAsync already exists to avoid for the same reason. Add claimAimlapiTopupStateAsync (sharing the same claim logic via an extracted operation function) and switch the GUI to it, making startAimlapiTopup async. recordAimlapiCheckoutSession (the reportSession/onSession path) has the same sync-lock exposure but is called from a callback whose return value AimlapiProvisionOptions.onSession drives synchronous control flow in several places across both the CLI and GUI provisioning paths; making it async is a larger, riskier contract change deliberately left out of this pass. * fix(aimlapi): never pair a new key with a stale or unrelated apiKeyId saveAimlapiTopupState's apiKeyId fallback still read current.apiKeyId even when current.apiKey was empty (the id-without-a-key case) or when state carried a genuinely new apiKey with its own empty-id sentinel, letting a fresh key get silently tagged with an unrelated leftover id. Gate apiKeyId on the same winner apiKey came from instead of falling back to current independently. * fix(aimlapi): stop cross-account key leaks, lease key-minting, keep GUI CAS async - claimAimlapiTopupState's abandonExisting carried a retained apiKey into ANY differing intent, including a switch from account A to account B (the GUI's forceAbandonExisting path). A B-flow restart before the profile write could then initialize from the receipt and call the B checkout with A's credential — crediting A while B's flow saves A's key. Gate the carry-over on the intent's account/key identity (`email`) matching, not just abandonExisting. - The key-choice screen (I am a new user / I already have a key) reset the whole onboarding identity unconditionally on either choice, even when Esc had backed all the way out from the amount screen past an already-opened, still-chargeable checkout. Apply the same abandon-confirmation gate startAimlapiEmailOnboarding already uses. - POST /v1/keys (minting an existing-account key) had no cross-process serialization: two concurrent runs for the same intent could each observe no retained key and both mint, orphaning whichever key lost the first-writer-wins receipt race. Add a key-mint lease (mirroring the exchange lease's acquire/release shape) so exactly one process ever mints; a peer backs off and adopts the winner's recorded key. - The interactive flow already claimed asynchronously, but still called the synchronous saveAimlapiTopupState and recordAimlapiCheckoutSession directly from an event handler and the onSession callback — either could block the whole event loop for up to LOCK_TIMEOUT_MS under lock contention, freezing rendering, Esc, and SIGINT while a payment session is being created. Add async CAS variants and await them; this needed widening AimlapiProvisionOptions.onSession to allow returning a promise, since its return value drives resolveTopupSession's session election. - An ambient AIMLAPI_API_KEY takes the by-key route with aimlapiExistingUsesEnv, so the eventual profile intentionally stays keyless. The settled-receipt save before that write unconditionally copied the env value into aimlapi-topup.json regardless, expanding a secret's on-disk exposure surface for no recovery benefit (a restart re-reads the same env var). Keep an env-backed receipt credential-free. * fix(aimlapi): preserve the key-mint lease across unrelated CAS writes saveTopupStateOperation and recordCheckoutSessionOperation merged the exchange lease but not the key-mint lease added in the previous commit: AimlapiCheckoutState (what every caller spreads checkoutState from) carries neither lease pair, so an unrelated write - persisting the exchange flag, or electing a checkout session - silently dropped an in-flight peer's key-mint lease. A third process would then see the slot as free and mint its own key, reopening the exact double-mint race the lease exists to close. Fall back to the current lease the same way the exchange lease already does. Also: add a future-dated key-mint lease reclaim test mirroring the exchange lease's, and align the default saveAimlapiTopupStateAsync test mock with the real CAS (match on intent + payment id, keep the first writer's resumeSessionToken/apiKey) so it no longer accepts a write the real store would reject. * test(aimlapi): preserve the key-mint/exchange lease in the mocked GUI CAS writes saveAimlapiTopupStateAsync's and recordAimlapiCheckoutSessionAsync's default mocks spread { ...state } as their write's base, same gap as the real saveTopupStateOperation/recordCheckoutSessionOperation had before the previous commit: neither lease pair survived a write whose state didn't carry them (which is every real caller, since AimlapiCheckoutState exposes neither). Fixing the merge alone wasn't enough — the same two mocks' "does this write still belong to this slot" check also compared lease fields as if they were part of the intent identity, so a write that seeded a lease value failed to match the just-claimed record and silently no-op'd instead of persisting anything. Exclude both lease pairs from that comparison too, matching the real matchingStateOrNull (which only ever compares INTENT_KEYS + paymentSessionId). * fix(aimlapi): recover ambiguous key-mint/exchange outcomes before releasing leases createKey and /exchange are both non-idempotent with no server-side retrieval path, so a lost response after the request actually committed left three races: a retry could exchange (or mint) a second time and orphan the first credential, or the CLI's exchange caller would surface a generic network error instead of the accurate "already exchanged, rotate the key" guidance. exchangeKeyWithLease now distinguishes a genuinely ambiguous transport failure of the /exchange POST itself from other doExchange failures (a pre-POST bail on a reclaimed lease, or a definite rejection): only the former re-checks the session status directly, surfaces the already-exchanged error when confirmed, and otherwise leaves the lease held instead of releasing it into a race. mintExistingAccountKeyWithLease applies the same ambiguous/definite split before deciding whether to release its lease. The GUI sign-in flow had an equivalent gap one step earlier: two concurrent code-verification races could each see an empty key cache and both mint before either save elected a winner, so the loser never adopted the winner's key. completeAimlapiCodeSignIn now serializes the cache lookup and mint behind a new email-scoped lease in topupState.ts, so a losing process waits and adopts the winner's cached credential instead of minting its own. * fix(aimlapi): treat caller-aborted mutations as ambiguous and dedupe the transport helpers client.request rethrows a caller-driven abort as the raw abort error instead of wrapping it in AimlapiApiError, so the ambiguous-outcome checks added for createKey and /exchange missed it: cancelling client-side does not stop a non-idempotent POST from completing server-side, but the lease was still released as if the request definitely failed, leaving the door open to a retry racing a second mint/exchange. All three call sites (the checkout-time key-mint lease, the exchange lease, and the sign-in key-mint lease) now also hold the lease when the caller's own signal fired. Extracted the duplicated abortError/sleep/isAmbiguousTransportApiError helpers shared between topup.ts and onboarding.ts into transport.ts so the ambiguity rule can't drift between the CLI and GUI paths. Switched the GUI sign-in flow's cache save to the async, lock-yielding variant and logged its lease-release failures for parity with the other leases. * fix(aimlapi): close six checkout-state races found across the claim, lease, and recovery paths claimAimlapiTopupState's in-progress check only looked at resumeSessionToken/settled/apiKey, so a receipt claimed just before its non-idempotent POST (/v1/keys or /exchange) still looked blank and replaceable to a different intent. A competing claim could overwrite it mid-flight, leaving the in-flight request's eventual CAS save with no matching record to land in and orphaning the credential it was about to mint or exchange. The claim now also refuses (unconditionally, even under abandonExisting) while either lease is live. The sign-in key-mint lease's 75s stale window exactly matched createKey's worst-case duration (60s) plus the async lock's own timeout (15s) for the cache write that follows, with zero margin for anything else. A legitimately still-working holder could lose the lease to a peer moments before its result was cached. It's now refreshed right after createKey succeeds, giving the cache-write phase its own fresh window. ProviderManager's code-verification path called the synchronous saveAimlapiSignInKey, whose lock retry blocks the whole event loop for up to five seconds on contention — freezing Ink rendering, timers, Esc, and SIGINT right after a sign-in. Switched to the async variant, exported through providerManagerAimlapi.ts alongside the other async cache operations. The three session polling helpers typed onSession as returning void and never awaited it, even though ProviderManager's callback is async and starts receipt cleanup before returning. A terminal session (cancelled/expired/ failed, or a dead session) could let the UI reach the amount screen before the durable receipt was actually reset, so an immediate retry still saw the stale resume token and got rejected as "not yet abandoned." Both sides now await through to completion. The confirmed email-switch flow cleared the in-memory checkout intent and fired an un-awaited, error-swallowing state clear, but derived its later claim's abandonExisting only from refs that clear had just wiped — so a slow or failed clear left the user's explicit confirmation unenforced at the claim itself. It now sets the same one-shot force-abandon signal the "switch account" flow already uses for exactly this kind of on-disk, this-mount-invisible conflict. A cached sign-in key that the server had revoked was indistinguishable from one that was merely unreachable: both collapsed into balanceStatus: 'unknown', which re-cached the same dead key and sent the user to manual-key entry with no way back into the guided flow short of deleting local state. A definite 401/403 against a cached (not freshly minted) key now invalidates the stale cache entry and mints one replacement before falling back to the generic unknown-balance path; every other (ambiguous) failure still leaves the cache untouched. Extracted a shared claim/lease-liveness helper in topupState.ts and added regression coverage for each race — including two that hold a mocked createKey/reset call open to prove the competing operation actually waits instead of just asserting on the end state. * fix(aimlapi): clear the stale force-abandon flag on a fresh preset entry aimlapiForceAbandonExistingRef is armed when the user confirms abandoning a checkout during an email switch, then consumed by the next claim. If that claim never runs — the switch's own onboarding fails and the user backs all the way out to preset selection instead of retrying — the flag stayed armed. Re-entering the aimlapi preset with an unrelated email then passed abandonExisting: true on its first claim with no confirmation for that flow, silently overwriting whatever unpaid checkout was still on disk. startCreateFromPreset now resets the flag alongside the other per-flow refs it already clears on fresh entry. Also swapped a fixed 20ms sleep in the cross-intent concurrency test for a signal fired from the held-open /v1/keys handler, so the test can't flake under CI load waiting for the run to reach the point it needs to race. * fix(aimlapi): close the remaining confirmation, cleanup, and lease gaps in checkout state The API-key-choice screen's own confirm-abandon gate (Enter twice to accept "a checkout from this account is still pending") reset the onboarding identity but never armed the force-abandon signal the email-switch and switch-account flows already use. A contended or failed pre-clear left the next claim to hit the CAS's unconfirmed-conflict refusal despite the user having just confirmed abandonment through this exact screen. reportSession('')'s terminal-session handler discarded the persisted intent ref before its reset/clear attempt settled, and swallowed any failure as success. A lock timeout or I/O error then left the durable receipt exactly as it was, but with no ownership left in memory to retry cleanup or to route a later conflicting claim through the normal confirmation gate — the CAS just rejected it outright. Ownership now only drops once the transition actually commits; a failure is logged and the ref stays populated so the existing gate covers the next claim. The sign-in key-mint lease's stale window already had zero margin for its own refresh call's lock wait (up to 15s) on top of createKey's own worst case (60s) and the cache save's lock wait (another 15s) — 90s with nothing left over. Widened it to 150s and lengthened the losing side's patience to match, and stopped silently ignoring a refresh that reports lost ownership: it's now logged for diagnosability even though the save itself stays safe to attempt regardless (first-writer-wins makes a losing write a no-op). Both onSaved completion paths (persistExistingAimlapi and persistAimlapiKey) called the synchronous clearAimlapiSignInKey from Ink's synchronous save callback, whose lock retry blocks the event loop for up to five seconds on contention — freezing rendering, timers, Esc, and SIGINT right at completion, the same class of bug already fixed for the sign-in save path. Re-exported the async variant through providerManagerAimlapi.ts and switched both call sites to fire-and-forget it instead. * fix(aimlapi): stop the flow instead of risking a stranded key on a receipt-write failure /exchange (and the by-key top-up) is a one-shot operation: once it succeeds, the issued key exists on…
3 participants
Summary
Closes #1987.
Why
The guided flow added in #1886 uses the previous email/password and checkout contracts. The current backend flow is passwordless, and an existing configured key should be validated and funded without provisioning another account.
Checkout also crosses browser and terminal lifecycles. Retaining the session and payment identity prevents ambiguous retries or cancellation from creating another payment or exchanging the same payment twice.
What changed
User impact
New users can configure AI/ML API without creating a password. Existing users can sign in with an emailed code, while users with a configured key can validate and top up that key directly. Interrupted checkout can be resumed without starting duplicate payment operations.
Validation
bun test src/integrations/aimlapi— 30 passedbun test src/components/ProviderManager.test.tsx— 38 passedbun run buildbun run smokebun run typecheckbun run typecheck:type-testsbun run integrations:checkbun run security:pr-scanbun run deadcodebun run checkdid not complete locally and produced no output before it was stopped after 10 minutes. The focused suites and checks above completed successfully.Scope
This stays on the existing OpenAI-compatible provider path and adds no runtime dependencies.
Summary by CodeRabbit
New Features
anthropic/claude-sonnet-5.aimlapi topupCLI with--email,--code,--auto-top-up,--model, and--no-open(removed--method).Bug Fixes
Documentation