Add presigned URL APIs#9
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds presigned-URL support for sandboxes (models, service methods, client wrappers, tests, and README) and changes SSH service methods to operate on specific credential IDs (endpoints, signatures, tests, and an example). Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant SandboxClient as Sandbox instance
participant SandboxesSvc as SandboxesClient
participant Transport as HTTP Transport
participant API as Leap0 API
rect rgba(0,128,0,0.5)
Dev->>SandboxClient: sandbox.createPresignedUrl(port, expiresIn)
SandboxClient->>SandboxesSvc: createPresignedUrl(sandboxRef, {port, expiresIn})
SandboxesSvc->>Transport: POST /v1/sandbox/{id}/presigned-url with {port, expires_in}
Transport->>API: HTTP request
API-->>Transport: 200 JSON { id, url, token, ... }
Transport-->>SandboxesSvc: response JSON
SandboxesSvc->>SandboxesSvc: validate/normalize via presignedUrlSchema
SandboxesSvc-->>SandboxClient: PresignedUrl
SandboxClient-->>Dev: Promise<PresignedUrl>
end
sequenceDiagram
participant Dev as Developer
participant SandboxClient as Sandbox instance
participant SandboxesSvc as SandboxesClient
participant Transport as HTTP Transport
participant API as Leap0 API
rect rgba(0,0,255,0.5)
Dev->>SandboxClient: sandbox.deletePresignedUrl(presignedId)
SandboxClient->>SandboxesSvc: deletePresignedUrl(sandboxRef, presignedId)
SandboxesSvc->>SandboxesSvc: trim + encode id
SandboxesSvc->>Transport: DELETE /v1/sandbox/{id}/presigned-url/{encodedId}
Transport->>API: HTTP DELETE
API-->>Transport: 204 No Content
Transport-->>SandboxesSvc: success
SandboxesSvc-->>SandboxClient: void
SandboxClient-->>Dev: Promise<void>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/client/client-sandbox.test.ts (1)
200-200: Consider strengthening the type assertion forcreatePresignedUrl.Line 200 only checks
Promise<{ url: string }>; asserting againstPromise<PresignedUrl>would better guard the full return contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/client/client-sandbox.test.ts` at line 200, The test currently asserts that Sandbox.createPresignedUrl returns Promise<{ url: string }>; update the type assertion to check for Promise<PresignedUrl> instead so the full PresignedUrl contract is validated. Locate the assertion using ReturnType<Sandbox["createPresignedUrl"]> in tests/client/client-sandbox.test.ts and replace the expected type with Promise<PresignedUrl>, ensuring the PresignedUrl type is imported or available in the test scope.tests/services/sandboxes-client.test.ts (1)
145-148: Fix mismatched assertion type key in test payload cast.Line 145 casts to
{ expires_in_minutes: number }but the asserted payload usesexpires_in. Aligning this keeps the test contract clear.🧪 Suggested cleanup
- assert.deepEqual(jsonOf(calls[0]!) as { port: number; expires_in_minutes: number }, { + assert.deepEqual(jsonOf(calls[0]!) as { port: number; expires_in: number }, { port: 8080, expires_in: 900, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/services/sandboxes-client.test.ts` around lines 145 - 148, The test casts jsonOf(calls[0]!) to the wrong shape: it currently uses { port: number; expires_in_minutes: number } but the asserted object uses expires_in; update the cast to match the asserted payload (e.g., { port: number; expires_in: number }) or change the asserted key to expires_in_minutes so the type and runtime assertion align—locate the usage of jsonOf(calls[0]!) in tests/services/sandboxes-client.test.ts and make the cast and expected key consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 135-144: The example for createPresignedUrl(8080, 15) is ambiguous
about the expiresIn unit; update the README example and surrounding text to
explicitly state the unit (e.g., "expiresIn: 15 minutes" or "expiresIn: 15
seconds") and, if possible, name the parameter in the call (e.g., show
createPresignedUrl(port, { expiresInMinutes: 15 }) or document that the second
numeric argument to createPresignedUrl is in minutes) so readers know exactly
what the 15 represents.
In `@src/services/sandboxes.ts`:
- Around line 292-299: The presigned URL deletion uses trimmedID directly in the
path which can break for IDs with reserved chars; before interpolating,
URL-encode the ID (e.g., via encodeURIComponent) and use that encoded value in
the call to this.transport.request inside withErrorPrefix for the
`/v1/sandbox/${sandboxIdOf(sandbox)}/presigned-url/...` path so that trimmedID
is safe for use in the request URL.
---
Nitpick comments:
In `@tests/client/client-sandbox.test.ts`:
- Line 200: The test currently asserts that Sandbox.createPresignedUrl returns
Promise<{ url: string }>; update the type assertion to check for
Promise<PresignedUrl> instead so the full PresignedUrl contract is validated.
Locate the assertion using ReturnType<Sandbox["createPresignedUrl"]> in
tests/client/client-sandbox.test.ts and replace the expected type with
Promise<PresignedUrl>, ensuring the PresignedUrl type is imported or available
in the test scope.
In `@tests/services/sandboxes-client.test.ts`:
- Around line 145-148: The test casts jsonOf(calls[0]!) to the wrong shape: it
currently uses { port: number; expires_in_minutes: number } but the asserted
object uses expires_in; update the cast to match the asserted payload (e.g., {
port: number; expires_in: number }) or change the asserted key to
expires_in_minutes so the type and runtime assertion align—locate the usage of
jsonOf(calls[0]!) in tests/services/sandboxes-client.test.ts and make the cast
and expected key consistent.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 488b7c59-23c5-43b1-9c1f-89199e87bc6e
📒 Files selected for processing (8)
README.mdsrc/client/sandbox.tssrc/index.tssrc/models/index.tssrc/models/sandbox.tssrc/services/sandboxes.tstests/client/client-sandbox.test.tstests/services/sandboxes-client.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/ssh.ts`:
- Around line 38-40: The SSH service currently interpolates raw credential IDs
into request paths (e.g., in deleteAccess) which breaks for IDs containing
characters like "/", "?", or "#"; update all places that splice id into the URL
(deleteAccess and the other methods that call this.transport.request with
`/v1/sandbox/${sandboxIdOf(sandbox)}/ssh/${id}`) to encode the id with
encodeURIComponent before building the path so the URL is safe; ensure every
occurrence (the calls around the code labeled by deleteAccess and the other two
request usages) uses the encoded id consistently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d20c321e-cb0c-4538-81fe-34868cb83a35
📒 Files selected for processing (8)
README.mdexamples/ssh.tssrc/models/sandbox.tssrc/services/sandboxes.tssrc/services/ssh.tstests/client/client-sandbox.test.tstests/services/sandboxes-client.test.tstests/services/ssh.test.ts
✅ Files skipped from review due to trivial changes (2)
- README.md
- tests/services/sandboxes-client.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/services/sandboxes.ts
- tests/client/client-sandbox.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/services/ssh.ts (1)
39-41: Consider centralizing encoded SSH credential path building.The same
sandboxIdOf + encodeURIComponent + /ssh/{id}construction appears in three methods; a small private helper would reduce drift risk in future endpoint edits.Refactor sketch
export class SshClient { constructor(private readonly transport: Leap0Transport) {} + private credentialPath(sandbox: SandboxRef, id: string, suffix = ""): string { + const encodedId = encodeURIComponent(id); + return `/v1/sandbox/${sandboxIdOf(sandbox)}/ssh/${encodedId}${suffix}`; + } + async deleteAccess(sandbox: SandboxRef, id: string, options: RequestOptions = {}): Promise<void> { - const encodedID = encodeURIComponent(id); await this.transport.request( - `/v1/sandbox/${sandboxIdOf(sandbox)}/ssh/${encodedID}`, + this.credentialPath(sandbox, id), { method: "DELETE" }, options, ); } @@ ): Promise<SshValidation> { - const encodedID = encodeURIComponent(id); const data = await this.transport.requestJson<SshValidation>( - `/v1/sandbox/${sandboxIdOf(sandbox)}/ssh/${encodedID}/validate`, + this.credentialPath(sandbox, id, "/validate"), { method: "POST", body: jsonBody({ password }) }, options, ); @@ async regenerateAccess(sandbox: SandboxRef, id: string, options: RequestOptions = {}): Promise<SshAccess> { - const encodedID = encodeURIComponent(id); const data = await this.transport.requestJson<SshAccess>( - `/v1/sandbox/${sandboxIdOf(sandbox)}/ssh/${encodedID}/regen`, + this.credentialPath(sandbox, id, "/regen"), { method: "POST" }, options, );Also applies to: 62-64, 80-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/ssh.ts` around lines 39 - 41, Create a single private helper (e.g., sshEndpoint(sandbox, id) or buildSshPath) that encapsulates sandboxIdOf(sandbox), encodeURIComponent(id) and returns the full path string `/v1/sandbox/${sandboxId}/ssh/${encodedId}`, then replace the three places that call this.transport.request(`/v1/sandbox/${sandboxIdOf(sandbox)}/ssh/${encodeURIComponent(id)}`) with calls to this new helper; keep using sandboxIdOf, encodeURIComponent and this.transport.request but centralize path construction in the new function to avoid duplication and drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/services/ssh.ts`:
- Around line 39-41: Create a single private helper (e.g., sshEndpoint(sandbox,
id) or buildSshPath) that encapsulates sandboxIdOf(sandbox),
encodeURIComponent(id) and returns the full path string
`/v1/sandbox/${sandboxId}/ssh/${encodedId}`, then replace the three places that
call
this.transport.request(`/v1/sandbox/${sandboxIdOf(sandbox)}/ssh/${encodeURIComponent(id)}`)
with calls to this new helper; keep using sandboxIdOf, encodeURIComponent and
this.transport.request but centralize path construction in the new function to
avoid duplication and drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e14e9fc-c891-4004-830c-2e3f281b7544
📒 Files selected for processing (2)
src/services/ssh.tstests/services/ssh.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/services/ssh.test.ts
Summary
Summary by CodeRabbit
New Features
Documentation
Examples
Tests
Breaking Changes