feat: save sandbox snapshot before stopping#6
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA new snapshot persistence feature is added to save sandbox state after agent sessions complete. The implementation includes documentation guidance, a new Recoup API integration function, a snapshot orchestration utility, and agent lifecycle modifications to trigger snapshots using the user's bearer token. Changes
Sequence DiagramsequenceDiagram
participant Agent as Agent Handler
participant Sandbox as Sandbox
participant SaveSnapshot as saveSnapshot()
participant API as Recoup API
Agent->>Agent: Streaming completes / Error occurs
Agent->>SaveSnapshot: saveSnapshot(sandbox, bearerToken)
SaveSnapshot->>Sandbox: snapshot()
Sandbox-->>SaveSnapshot: { snapshotId, ... }
SaveSnapshot->>API: updateAccountSnapshot(bearerToken, snapshotId)
Note over API: PATCH request with<br/>Authorization header
API-->>SaveSnapshot: 200 OK / Error logged
SaveSnapshot->>Sandbox: stop() [in finally]
Sandbox-->>Agent: Cleanup complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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: 4
🤖 Fix all issues with AI agents
In `@app/components/TerminalPage.tsx`:
- Around line 64-73: The getAccessToken function passed from TerminalPage to
TerminalComponent is unstable and causes Terminal.tsx's effect (cleanup/re-init)
to run on every parent render; wrap the hook-derived getAccessToken in
useCallback inside TerminalPage.tsx (so the reference is stable) or replace the
hook usePrivy() call with Privy's stable module export, then pass that stable
function to TerminalComponent (refer to getAccessToken, TerminalComponent,
Terminal.tsx useEffect) to prevent unnecessary terminal re-initialization.
In `@app/page.tsx`:
- Line 97: The anchor element with id "credits" uses target="_blank" and must
include rel="noopener noreferrer" to prevent the opened page from accessing
window.opener; update the <a ... id="credits"> element (the anchor with id
"credits" in app/page.tsx) to add rel="noopener noreferrer" alongside the
existing attributes to resolve the Biome warning.
In `@lib/agent/createAgentResponse.ts`:
- Around line 20-24: Wrap the call to req.json() in a try/catch inside
createAgentResponse (or surrounding handler) to catch JSON.parse errors and
return a 400 with a clear message; after parsing, validate that the extracted
messages is an array before using it (e.g., check Array.isArray(messages)), and
guard access to lastUserMessage and its parts (ensure lastUserMessage exists and
lastUserMessage.parts is an array) before logging or using .parts[0].text so
malformed requests yield a 400 instead of throwing a 500.
- Around line 59-72: The pipeTo promise from body.pipeTo(transform.writable) is
not handled, causing unhandled rejections if the stream errors; modify the code
around the TransformStream usage in createAgentResponse (the transform variable
and body.pipeTo call) to capture the promise (e.g., const pipePromise =
body.pipeTo(transform.writable)) and attach error handling to it (await it if
the surrounding function is async or add .catch(...) to consume/log the error)
while preserving the existing .finally() behavior that calls
saveSnapshot(sandbox, bearerToken) and sandbox.stop(); ensure the Response still
returns transform.readable and that any errors from pipeTo are not left
unhandled.
🧹 Nitpick comments (6)
lib/recoup-api/updateAccountSnapshot.ts (1)
1-4: Extract shared API configuration tolib/recoup-api/constants.ts.Both
RECOUP_API_URLandIS_PRODare duplicated identically inlib/recoup-api/createSandbox.tsandlib/recoup-api/updateAccountSnapshot.ts. Move these constants to a shared module to maintain the DRY principle and simplify future updates to API URLs or environment logic.lib/recoup-api/createSandbox.ts (2)
1-4: Consider extracting the duplicated API URL configuration.The
IS_PROD/RECOUP_API_URLderivation is duplicated here and inlib/recoup-api/updateAccountSnapshot.ts. Extract it into a shared constant (e.g.,lib/recoup-api/constants.ts) to keep the base URL in one place.
24-26: Silent error swallowing may hinder debugging.The catch block returns
nullwithout any logging. While the caller handlesnullgracefully, consider adding aconsole.warn(similar tocreateSnapshotSandbox.tsLine 15) so failures are observable in production logs.Proposed fix
- } catch { - return null; + } catch (err) { + console.warn("createSandbox failed:", err); + return null; }app/components/TerminalPage.tsx (1)
15-20: ESLint false positive —mountedhydration guard is a standard pattern.The
useEffect(() => setMounted(true), [])idiom is a well-known SSR hydration guard. The ESLintreact-hooks/set-state-in-effectwarning is a false positive here since this is intentional one-time synchronization, not a cascading render issue.That said, if
usePrivy().readyis guaranteed to befalseduring SSR (which is typical for client-only auth SDKs), themountedstate may be redundant with the!readycheck on Line 22. Consider whether you can simplify to justif (!ready).lib/agent/createAgentResponse.ts (2)
59-82: Consider extracting the repeated cleanup pattern.The
saveSnapshot(...).finally(() => sandbox.stop().catch(() => {}))pattern appears three times (lines 64–66, 74–76, 79–81). A small helper would reduce duplication and make the cleanup intent clearer.Example helper
+const cleanupSandbox = (sandbox: Sandbox, bearerToken: string) => + saveSnapshot(sandbox, bearerToken).finally(() => + sandbox.stop().catch(() => {}), + );Then use
cleanupSandbox(sandbox, bearerToken)in each location.
31-35:messagestyped asunknown[]— good for safety, but verify downstream compatibility.The
messagesparameter is typed asunknown[]and passed directly tocreateAgentUIStreamResponseasuiMessages. This is fine if the downstream API acceptsunknown[], but could hide type mismatches.
| return ( | ||
| <> | ||
| {children} | ||
| <TerminalData /> | ||
| <TerminalComponent | ||
| getAccessToken={getAccessToken} | ||
| agentEndpoint={agentEndpoint} | ||
| /> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate the Terminal.tsx file and examine the useEffect dependency array
find . -name "Terminal.tsx" -o -name "Terminal.ts" | head -20Repository: recoupable/bash
Length of output: 88
🏁 Script executed:
# Search for getAccessToken usage to understand its scope and dependencies
rg -n "getAccessToken" --type=ts --type=tsx -A 3 -B 3 | head -100Repository: recoupable/bash
Length of output: 86
🏁 Script executed:
# Check for usePrivy hook usage and how it's imported
rg -n "usePrivy" --type=ts --type=tsx -A 2 -B 2 | head -80Repository: recoupable/bash
Length of output: 86
🏁 Script executed:
# Look at the TerminalPage.tsx file specifically
cat -n app/components/TerminalPage.tsxRepository: recoupable/bash
Length of output: 2160
🏁 Script executed:
# Now let's examine the Terminal.tsx file, especially around line 119
cat -n app/components/Terminal.tsx | head -150Repository: recoupable/bash
Length of output: 4923
🏁 Script executed:
# Check if getAccessToken is wrapped in useCallback anywhere
rg -n "useCallback" app/components/ --type-listRepository: recoupable/bash
Length of output: 448
🏁 Script executed:
# Search for any memoization patterns around Privy usage
rg -n "useMemo|useCallback" app/components/ -A 2 -B 2Repository: recoupable/bash
Length of output: 41
🌐 Web query:
Privy usePrivy getAccessToken function reference stability memoization
💡 Result:
getAccessToken is exposed two ways:
- From React context:
const { getAccessToken } = usePrivy();
const token = await getAccessToken();This call will also refresh the token if it’s near expiry/expired. [1][2]
- As a direct import (outside the hook):
import { getAccessToken } from '@privy-io/react-auth';
const token = await getAccessToken();If you do this, you must ensure PrivyProvider has already rendered. [1]
Reference stability / memoization guidance
Privy’s docs describe behavior, but do not promise that the function identity of getAccessToken returned by usePrivy() is stable across renders. So in React, you should treat it as potentially unstable:
- If you use it inside
useEffect/useCallback, include it in the dependency array (don’t omit it assuming it’s stable). - If you need a stable callable for dependency reasons, either:
- wrap your usage in a callback that depends on
getAccessToken, or - use the direct import
getAccessToken(module-level function identity is stable), while still ensuringPrivyProvideris mounted. [1]
- wrap your usage in a callback that depends on
Also, make sure Privy is initialized (ready === true) before relying on auth state/interfaces from usePrivy(). [2]
Sources: [1] Privy Docs – Access tokens (client retrieval + direct import) • [2] Privy Docs – React setup / ready initialization guidance
🏁 Script executed:
# Check if getAccessToken is used in createAgentCommand to confirm it's actually needed in dependency array
rg -n "createAgentCommand" app/components/ -A 5 -B 2Repository: recoupable/bash
Length of output: 2235
🏁 Script executed:
# Check if there's a createAgentCommand definition to see how getAccessToken is used
fd "terminal-parts" app/components/ -type fRepository: recoupable/bash
Length of output: 229
🏁 Script executed:
# Let's look at how getAccessToken is used in agent-command.ts
cat -n app/components/terminal-parts/agent-command.ts | head -80Repository: recoupable/bash
Length of output: 2784
Memoize getAccessToken to prevent terminal re-initialization on every render.
getAccessToken from usePrivy() is passed to TerminalComponent and included in a useEffect dependency array (Terminal.tsx Line 119). Since Privy does not guarantee a stable function reference across renders, each parent re-render creates a new getAccessToken reference, triggering the effect and tearing down/recreating the terminal unnecessarily.
Wrap getAccessToken in useCallback in TerminalPage.tsx to maintain referential stability:
const getAccessToken = useCallback(() => {
const { getAccessToken } = usePrivy();
return getAccessToken();
}, []);Alternatively, use Privy's direct import (which has module-level stability) instead of the hook.
🤖 Prompt for AI Agents
In `@app/components/TerminalPage.tsx` around lines 64 - 73, The getAccessToken
function passed from TerminalPage to TerminalComponent is unstable and causes
Terminal.tsx's effect (cleanup/re-init) to run on every parent render; wrap the
hook-derived getAccessToken in useCallback inside TerminalPage.tsx (so the
reference is stable) or replace the hook usePrivy() call with Privy's stable
module export, then pass that stable function to TerminalComponent (refer to
getAccessToken, TerminalComponent, Terminal.tsx useEffect) to prevent
unnecessary terminal re-initialization.
| </noscript> | ||
| <TerminalData /> | ||
| <TerminalComponent getAccessToken={getAccessToken} /> | ||
| <a href="https://vercel.com" target="_blank" hidden id="credits"> |
There was a problem hiding this comment.
Add rel="noopener noreferrer" to the external link.
Flagged by Biome: target="_blank" without rel="noopener" allows the opened page to access window.opener. Low practical risk since the link is hidden, but trivial to fix.
Proposed fix
- <a href="https://vercel.com" target="_blank" hidden id="credits">
+ <a href="https://vercel.com" target="_blank" rel="noopener noreferrer" hidden id="credits">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://vercel.com" target="_blank" hidden id="credits"> | |
| <a href="https://vercel.com" target="_blank" rel="noopener noreferrer" hidden id="credits"> |
🧰 Tools
🪛 Biome (2.3.13)
[error] 97-97: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
🤖 Prompt for AI Agents
In `@app/page.tsx` at line 97, The anchor element with id "credits" uses
target="_blank" and must include rel="noopener noreferrer" to prevent the opened
page from accessing window.opener; update the <a ... id="credits"> element (the
anchor with id "credits" in app/page.tsx) to add rel="noopener noreferrer"
alongside the existing attributes to resolve the Biome warning.
lib/agent/createAgentResponse.ts
Outdated
| const { messages } = await req.json(); | ||
| const lastUserMessage = messages | ||
| .filter((m: { role: string }) => m.role === "user") | ||
| .pop(); | ||
| console.log("Prompt:", lastUserMessage?.parts?.[0]?.text); |
There was a problem hiding this comment.
No validation on request body; potential for unhandled errors.
req.json() can throw on malformed JSON, and messages is accessed without checking it exists or is an array. A bad request would surface as a 500 instead of a 400.
🤖 Prompt for AI Agents
In `@lib/agent/createAgentResponse.ts` around lines 20 - 24, Wrap the call to
req.json() in a try/catch inside createAgentResponse (or surrounding handler) to
catch JSON.parse errors and return a 400 with a clear message; after parsing,
validate that the extracted messages is an array before using it (e.g., check
Array.isArray(messages)), and guard access to lastUserMessage and its parts
(ensure lastUserMessage exists and lastUserMessage.parts is an array) before
logging or using .parts[0].text so malformed requests yield a 400 instead of
throwing a 500.
Replicates the snapshot-saving pattern from Recoup-Tasks so sandbox state is preserved between sessions. After the agent stream finishes, we call sandbox.snapshot() and PATCH /api/sandboxes with the snapshotId before calling sandbox.stop(). Also adds CLAUDE.md with project-specific guidance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sweetmantech
force-pushed
the
sweetmantech/myc-4181-bash-load-snapshot-via-recoup-api
branch
from
a8f1131 to
08edd1f
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous fire-and-forget pattern in .finally() was racing against Vercel's serverless function shutdown. The function would terminate before sandbox.snapshot() and the PATCH call could complete, so snapshots were never persisted. Using after() tells Vercel to keep the function alive until the snapshot save finishes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
sandbox.stop()after each agent session, so sandbox state is preserved between sessionslib/recoup-api/updateAccountSnapshot.tsto callPATCH /api/sandboxeswith the Bearer token and snapshotIdlib/sandbox/saveSnapshot.tsas SRP wrapper:sandbox.snapshot()→updateAccountSnapshot()lib/agent/createAgentResponse.tsto threadbearerTokenand callsaveSnapshotbefore eachsandbox.stop()CLAUDE.mdwith project-specific guidance for Claude CodeTest plan
npx tsc --noEmitpasses (verified locally)PATCH /api/sandboxesaccount_snapshotsrow in Supabase has updatedsnapshot_idafter session🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation