feat(aimlapi): add passwordless client methods and response-shape guards (2/N) - #2020
Conversation
📝 WalkthroughWalkthroughThe AIMLAPI client adds passwordless onboarding, account and billing operations, abort-signal support, typed response validation, bounded response handling, secret redaction, and comprehensive Bun tests. ChangesAIMLAPI client expansion and hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 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: 1
🤖 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.ts`:
- Line 224: Standardize malformed-success responses in the auth and key flows to
throw AimlapiApiError instead of plain Error, including signup, login,
verifySignInCode, createPasswordlessAccount, and createKey. Preserve the
existing messages while supplying status 200 and the response body so callers
consistently receive the API error contract.
🪄 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: cab9c302-145f-4190-9c89-61955ace50f1
📒 Files selected for processing (2)
src/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Provider changes must follow the documented integration patterns and avoid inconsistent behavior across provider paths.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Review AI-generated code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submission.
Run multiple rounds of self-review on AI-generated code; compilation alone is insufficient to establish correctness.
Files:
src/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep pull requests focused on one issue or one clearly scoped improvement; avoid unrelated cleanup, fixes, features, or refactors in the same change.
Preserve existing repository patterns unless intentionally refactoring them, and stay within the project's existing language, runtime, dependency, and architectural direction.
Add or update tests when a change affects behavior.
Update documentation when setup, commands, or user-facing behavior changes.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files merely because they are nearby.
Keep comments useful and concise.
Run the narrowest meaningful validation command for the touched area before opening a pull request, and ensure relevant CI checks pass before merge.
Provider-change pull requests must identify affected providers, state the tested provider/model path, and document limitations or follow-up work.
Do not assign or use provider tags; provider tags are controlled and applied by maintainers.
Security reports must follow the instructions inSECURITY.md.
PR descriptions must explain what changed and why, user or developer impact, exact checks run, and include relevant issue links; UI, terminal presentation, or VS Code extension changes require screenshots.
PR authors must address CodeRabbit findings before maintainer review proceeds.
Files:
src/integrations/aimlapi/client.test.tssrc/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/integrations/aimlapi/client.test.tssrc/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/integrations/aimlapi/client.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/integrations/aimlapi/client.test.ts
🔇 Additional comments (15)
src/integrations/aimlapi/client.ts (14)
228-240: Same invalid-shape error-type inconsistency as flagged at Line 224.
264-275: Same invalid-shape error-type inconsistency as flagged at Line 224.
277-284: Same invalid-shape error-type inconsistency as flagged at Line 224.
286-301: Same invalid-shape error-type inconsistency as flagged at Line 224 (createKeythrows a plainErrorat Line 298).
1-1: LGTM!Also applies to: 38-56
58-94: LGTM!
96-157: LGTM!
159-191: LGTM!
242-253: LGTM!
255-262: LGTM!
303-317: LGTM!
319-352: LGTM!
354-438: LGTM!
440-536: LGTM!src/integrations/aimlapi/client.test.ts (1)
1-367: LGTM!
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] Redact non-success response bodies before exposing them
src/integrations/aimlapi/client.ts:507
The non-OK branch stores the raw response text inAimlapiApiError.body, bypassing the new redaction path entirely. A proxy or backend can reflect the bearer or the one-time session token in a 4xx/5xx response; the CLI handler printserror.bodydirectly, so this exposes the credential in the terminal despite the PR's redaction guarantee. Redact the body before constructing the error (and cover the non-JSON error path as well). -
[P1] Validate the nested checkout receipt before treating a payment as usable
src/integrations/aimlapi/client.ts:131
isPayResultaccepts any object containing an object-valuedcheckout, although the returned contract requires a stringproviderSessionId, a string-or-nullpayUrl, andpartnerCheckout. For example,{ checkout: { payUrl: true } }passes the guard; the existing top-up flow then treats it as a URL, silently fails to open a browser, and polls until timeout after the payment request has already been made. Validate the complete receipt (at least every field consumed or exposed byPayResult) and add malformed checkout/top-up response coverage. -
[P2] Do not leave short session tokens out of transport-error redaction
src/integrations/aimlapi/client.ts:77
The redactor deliberately skips URL path segments shorter than six characters.getSession('abc')is a valid call under this client's public contract, and a transport/read error that includes its request URL will therefore surfaceabcunchanged through the new error message. There is no token-length invariant in the type or validation to make that safe. Redact the encoded session token directly (or remove the length heuristic) and cover a short-token error case. -
[P2] Enforce the account-action enum at the response boundary
src/integrations/aimlapi/client.ts:115
isAccountCheckResultaccepts every string even thoughAccountCheckResult.actionis the closed'sign-in' | 'sign-up'union. A successful{ action: 'disabled' }response crosses the client boundary as an impossible typed value instead of raisingAimlapiApiError; the passwordless caller enabled by this change will then take an incorrect onboarding branch. Check membership in the two supported values (and validate the optional provider field) before returning the result.
5fc7e57
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] Redact verification codes from failed authentication requests
src/integrations/aimlapi/client.ts:320
verifySignInCodesends the one-time code without adding it tosecrets, while the non-2xx and non-JSON paths only redact the bearer and explicitly supplied secrets. An auth service or proxy that reflects an invalid code in its response therefore puts the active code inAimlapiApiError.body; the CLI prints that body verbatim. Pass the code (and the other credential-bearing request fields) through the redaction set and cover a reflected-error response. -
[P1] Reject unusable checkout URLs before beginning payment polling
src/integrations/aimlapi/client.ts:163
isPaymentSessiontreats any nonemptypayUrlas valid, but both existing consumers pass it toopenBrowser, which rejects malformed and non-HTTP(S) URLs. A receipt such as{ payUrl: "not-a-url" }consequently survives the new guard after the charge request, cannot be opened, and then enters the 20-minute payment poll despite no usable checkout link. Validate a non-null URL as parseable HTTP(S), with regression coverage for malformed and unsupported-scheme values. -
[P3] Complete the response guards for the exported result types
src/integrations/aimlapi/client.ts:151
The new guards only validate selected fields:isAuthResultaccepts a missing or non-numericexp, andisPartnerCheckoutSessionaccepts missing or wrong-typedpartnerName,userId,amountUsdMinor,issuedKeyId, andreturnUrl. Those payloads cross the client boundary as the exported typed results even though callers may legitimately use those fields, reintroducing the raw downstream failures that this PR aims to convert intoAimlapiApiError. Validate every required field (including finite numeric values) and add malformed-field tests. The current in-tree top-up callers do not use these omitted fields, so this is a contract-completeness issue rather than an immediate flow break.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/integrations/aimlapi/client.ts (1)
338-344: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor
expectJson: falsefor non-empty success bodies.This flag only bypasses parsing for an empty body; a successful
200 text/plainresponse still reachesJSON.parseand rejects. Returnundefinedbefore body parsing wheneverexpectJson === false, and add a regression test using a non-empty plain-text 2xx response.Proposed fix
if (!response.ok) { // ... } + if (options.expectJson === false) return undefined as T if (!text.trim()) { - if (options.expectJson === false) return undefined as T throw new AimlapiApiError(As per coding guidelines: “Add or update tests when a 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/client.ts` around lines 338 - 344, Update the response-handling logic in the client’s request method to return undefined immediately for any successful response when expectJson is false, before attempting JSON parsing, including non-empty bodies. Preserve normal parsing when expectJson is true, and add a regression test covering a non-empty text/plain 2xx response through sendSignInCode.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.
Outside diff comments:
In `@src/integrations/aimlapi/client.ts`:
- Around line 338-344: Update the response-handling logic in the client’s
request method to return undefined immediately for any successful response when
expectJson is false, before attempting JSON parsing, including non-empty bodies.
Preserve normal parsing when expectJson is true, and add a regression test
covering a non-empty text/plain 2xx response through sendSignInCode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 93120f10-4285-4d4f-aca6-637e02f95204
📒 Files selected for processing (2)
src/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Provider changes must follow the documented integration patterns and avoid inconsistent behavior across provider paths.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Review AI-generated code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submission.
Run multiple rounds of self-review on AI-generated code; compilation alone is insufficient to establish correctness.
Files:
src/integrations/aimlapi/client.tssrc/integrations/aimlapi/client.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep pull requests focused on one issue or one clearly scoped improvement; avoid unrelated cleanup, fixes, features, or refactors in the same change.
Preserve existing repository patterns unless intentionally refactoring them, and stay within the project's existing language, runtime, dependency, and architectural direction.
Add or update tests when a change affects behavior.
Update documentation when setup, commands, or user-facing behavior changes.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files merely because they are nearby.
Keep comments useful and concise.
Run the narrowest meaningful validation command for the touched area before opening a pull request, and ensure relevant CI checks pass before merge.
Provider-change pull requests must identify affected providers, state the tested provider/model path, and document limitations or follow-up work.
Do not assign or use provider tags; provider tags are controlled and applied by maintainers.
Security reports must follow the instructions inSECURITY.md.
PR descriptions must explain what changed and why, user or developer impact, exact checks run, and include relevant issue links; UI, terminal presentation, or VS Code extension changes require screenshots.
PR authors must address CodeRabbit findings before maintainer review proceeds.
Files:
src/integrations/aimlapi/client.tssrc/integrations/aimlapi/client.test.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/client.tssrc/integrations/aimlapi/client.test.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/client.tssrc/integrations/aimlapi/client.test.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/client.test.ts
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] Honor the non-JSON response opt-out for every successful code-send response
src/integrations/aimlapi/client.ts:605
sendSignInCodesetsexpectJson: false, butrequestonly returns early for that option when the successful body is empty. A valid non-empty acknowledgement such as200 text/plain: code sentis instead passed toJSON.parseand reported asAimlapiApiError, even though the code was delivered. This makes passwordless sign-in fail visibly and encourages retries that can invalidate or rate-limit the one-time code. Returnundefinedimmediately after the non-OK check wheneverexpectJsonis false, and cover a non-empty successful acknowledgement throughsendSignInCode.
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 (1)
src/integrations/aimlapi/client.test.ts (1)
473-495: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAssert caller-abort error semantics, not just rejection.
toThrow()would pass if cancellation were incorrectly wrapped asAimlapiApiErroror another transport failure. Assert that the propagated error is the expected abort error while retaining the signal-forwarding assertions.As per path instructions: tests must cover “error semantics” and abort/timeout wiring for changed runtime behavior.
Suggested assertion
- await expect(pending).rejects.toThrow() + const error = await pending.then( + () => null, + (reason: unknown) => reason, + ) + expect(error).toMatchObject({ name: 'AbortError' })🤖 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.test.ts` around lines 473 - 495, Update the test around AimlapiClient.getSession to assert the propagated rejection is the expected abort error, rather than accepting any thrown error with toThrow(). Retain the existing forwardedSignal and aborted-state assertions to continue covering abort wiring.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 170-191: Update the sendSignInCode tests to capture the fetch
arguments and assert the request uses POST to
`${endpoints.authBaseUrl}/v1/auth/sign-in/code` with the email payload `{ email:
'user@example.com' }`. Apply these contract assertions to the successful request
path while preserving the existing acknowledgement and non-2xx response checks.
---
Outside diff comments:
In `@src/integrations/aimlapi/client.test.ts`:
- Around line 473-495: Update the test around AimlapiClient.getSession to assert
the propagated rejection is the expected abort error, rather than accepting any
thrown error with toThrow(). Retain the existing forwardedSignal and
aborted-state assertions to continue covering abort wiring.
🪄 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: b17f368b-67cc-4e81-b185-572e78f10a2c
📒 Files selected for processing (2)
src/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Provider changes must follow the documented integration patterns and avoid inconsistent behavior across provider paths.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Review AI-generated code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submission.
Run multiple rounds of self-review on AI-generated code; compilation alone is insufficient to establish correctness.
Files:
src/integrations/aimlapi/client.tssrc/integrations/aimlapi/client.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep pull requests focused on one issue or one clearly scoped improvement; avoid unrelated cleanup, fixes, features, or refactors in the same change.
Preserve existing repository patterns unless intentionally refactoring them, and stay within the project's existing language, runtime, dependency, and architectural direction.
Add or update tests when a change affects behavior.
Update documentation when setup, commands, or user-facing behavior changes.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files merely because they are nearby.
Keep comments useful and concise.
Run the narrowest meaningful validation command for the touched area before opening a pull request, and ensure relevant CI checks pass before merge.
Provider-change pull requests must identify affected providers, state the tested provider/model path, and document limitations or follow-up work.
Do not assign or use provider tags; provider tags are controlled and applied by maintainers.
Security reports must follow the instructions inSECURITY.md.
PR descriptions must explain what changed and why, user or developer impact, exact checks run, and include relevant issue links; UI, terminal presentation, or VS Code extension changes require screenshots.
PR authors must address CodeRabbit findings before maintainer review proceeds.
Files:
src/integrations/aimlapi/client.tssrc/integrations/aimlapi/client.test.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/client.tssrc/integrations/aimlapi/client.test.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/client.tssrc/integrations/aimlapi/client.test.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/client.test.ts
🔇 Additional comments (1)
src/integrations/aimlapi/client.ts (1)
605-609: LGTM!
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
-
[P2] Redact JSON-escaped credentials before preserving error bodies
src/integrations/aimlapi/client.ts:79
The new redactor only substitutes a credential's raw and URL-encoded forms. A backend or proxy commonly serializes a reflected password/bearer withJSON.stringify, which escapes quotes and backslashes, so a password such asp\\qdoes not match the error body'sp\\\\qrepresentation. The non-OK path then stores that body inAimlapiApiError.body, andtopup.tsprints it during signup/login failure. Redact JSON-string-escaped forms as well (and cover special-character credentials) before surfacing the body. -
[P2] Do not bypass redaction for caller-cancelled requests
src/integrations/aimlapi/client.ts:573
When a caller aborts, both fetch and response-read error paths rethrow the transport error verbatim. A cancelledgetSession('session-secret', signal)whose transport reports its request URL therefore throws an error whose message containssession-secret, despite the new guarantee that session tokens never reach error messages. Preserve cancellation semantics while returning a redacted cancellation error (and apply the same treatment to the response-read branch). -
[P2] Process overlapping secrets longest-first
src/integrations/aimlapi/client.ts:107
Secrets are replaced in insertion order, so one credential can redact the prefix of another before the longer value is considered. For example, a reflectedabc123response fromexchange('abc', 'abc123')becomes[REDACTED]123; the suffix is then printed throughAimlapiApiError.body. Sort variants by descending length, or use a multi-secret matcher, before substituting them.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/integrations/aimlapi/client.ts (1)
330-336: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRedact account emails from reflected errors.
These flows omit the submitted email from
secrets; a backend that echoes it leaks PII throughAimlapiApiError.body, which callers may print.
src/integrations/aimlapi/client.ts#L330-L336: passsecrets: [email].src/integrations/aimlapi/client.ts#L343-L349: passsecrets: [email].src/integrations/aimlapi/client.ts#L357-L360: includecode.src/integrations/aimlapi/client.ts#L367-L371: passsecrets: [email].src/integrations/aimlapi/client.ts#L295-L306: includeinput.email.src/integrations/aimlapi/client.ts#L320-L323: includesrc/integrations/aimlapi/client.test.ts#L316-L337: add a reflected-email redaction regression test.As per coding guidelines, “Add or update tests when a change affects behavior.” As per path instructions, review “auth/token handling” 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/client.ts` around lines 330 - 336, Redact account emails from reflected AimlapiApiError data by adding each submitted email to the request secrets in checkAccount and the corresponding account flows: src/integrations/aimlapi/client.ts lines 330-336, 343-349, 367-371, 295-306, and 320-323; include email alongside code at lines 357-360. Add a regression test covering reflected-email redaction in src/integrations/aimlapi/client.test.ts lines 316-337.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.
Outside diff comments:
In `@src/integrations/aimlapi/client.ts`:
- Around line 330-336: Redact account emails from reflected AimlapiApiError data
by adding each submitted email to the request secrets in checkAccount and the
corresponding account flows: src/integrations/aimlapi/client.ts lines 330-336,
343-349, 367-371, 295-306, and 320-323; include email alongside code at lines
357-360. Add a regression test covering reflected-email redaction in
src/integrations/aimlapi/client.test.ts lines 316-337.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64145c68-d5ac-4dd1-a41b-5d312fc5ec14
📒 Files selected for processing (2)
src/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Provider changes must follow the documented integration patterns and avoid inconsistent behavior across provider paths.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Review AI-generated code for correctness, style consistency, unnecessary changes, and adherence to project architecture before submission.
Run multiple rounds of self-review on AI-generated code; compilation alone is insufficient to establish correctness.
Files:
src/integrations/aimlapi/client.test.tssrc/integrations/aimlapi/client.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep pull requests focused on one issue or one clearly scoped improvement; avoid unrelated cleanup, fixes, features, or refactors in the same change.
Preserve existing repository patterns unless intentionally refactoring them, and stay within the project's existing language, runtime, dependency, and architectural direction.
Add or update tests when a change affects behavior.
Update documentation when setup, commands, or user-facing behavior changes.
Follow the existing code style in touched files.
Prefer small, readable changes over broad rewrites.
Do not reformat unrelated files merely because they are nearby.
Keep comments useful and concise.
Run the narrowest meaningful validation command for the touched area before opening a pull request, and ensure relevant CI checks pass before merge.
Provider-change pull requests must identify affected providers, state the tested provider/model path, and document limitations or follow-up work.
Do not assign or use provider tags; provider tags are controlled and applied by maintainers.
Security reports must follow the instructions inSECURITY.md.
PR descriptions must explain what changed and why, user or developer impact, exact checks run, and include relevant issue links; UI, terminal presentation, or VS Code extension changes require screenshots.
PR authors must address CodeRabbit findings before maintainer review proceeds.
Files:
src/integrations/aimlapi/client.test.tssrc/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/integrations/aimlapi/client.test.tssrc/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/integrations/aimlapi/client.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/integrations/aimlapi/client.test.ts
🔇 Additional comments (5)
src/integrations/aimlapi/client.ts (3)
69-116: LGTM!
378-541: LGTM!
543-652: LGTM!src/integrations/aimlapi/client.test.ts (2)
43-314: LGTM!
339-586: LGTM!
4 participants
Second layer of splitting #1988 into stacked PRs (after #1995). Touches only
client.tsandclient.test.ts— nothing else in the tree changes.What this adds
The passwordless onboarding surface plus response hardening:
checkAccount,sendSignInCode,verifySignInCode,createPasswordlessAccount,createKey,getBalance,topUpByKeycontrolled
AimlapiApiErrorinstead of dereferencing a partial payload andthrowing a raw
TypeErrorrequest<T>— rejects empty /null/ non-object success bodies(
sendSignInCodeopts out viaexpectJson: false)never reach the message, and the label is the request origin, not the full URL
AbortSignalplumbed through every methodWhat this deliberately keeps
PaymentMethod,signup()andlogin()stay, andpay()accepts bothmethod(password flow) andpaymentSessionId(passwordless flow) as optionalfields.
That keeps the change additive: the current
topup.tsandProviderManager.tsxcompile and behave unchanged, so this lands withouttouching the top-up flow or the UI. The password API is removed in the follow-up
PR that migrates those callers, where it becomes a mechanical deletion.
Behaviour changes that do reach the existing flow
request<T>is shared, so the password path also picks up the hardening:undefinedcreateSession/getSession/exchangeraise anon-terminal
AimlapiApiError(status 200) rather than surfacing a partialobject — notably a repeat
exchangenow fails loudly instead of returning anundefined key
No existing endpoint returns an empty body, so this is hardening rather than a
regression.
Verification
bun run typecheck✓ ·bun run deadcode✓ ·client.test.ts14/14 — includingnew coverage for the retained
signup/logincontracts, their empty-tokenrejection, and
paywith an explicit payment method.Summary by CodeRabbit
paynow supports optionalpaymentSessionIdandautoTopUp, with optionalmethoddefaulting to card.