feat(sandbox): port skill installation from open-agents#527
feat(sandbox): port skill installation from open-agents#527sweetmantech merged 1 commit intotestfrom
Conversation
Closes the second-largest gap identified in the cutover analysis: every session sandbox now installs the platform default skills (recoup-api, artist-workspace) plus any user-configured globalSkillRefs from the session row, so agents come up with their tools loaded. Best-effort: wraps installSessionGlobalSkills in try/catch inside createSandboxHandler. A skill-install failure does NOT fail the sandbox creation request — sandbox + DB stay consistent, agent runs without skills (or whichever subset got through), and the user can recover with a follow-up request once the underlying issue is fixed. Mirrors open-agents' error handling exactly. New files: - lib/sandbox/shellEscape.ts — POSIX single-quote escaper - lib/sandbox/resolveSandboxHomeDirectory.ts — probes $HOME, falls back to /root - lib/skills/globalSkillRef.ts — Zod schema + dedup transform + normalizeGlobalSkillRefs (best-effort parse) - lib/skills/defaultGlobalSkillRefs.ts — recoup-api + artist-workspace - lib/skills/installGlobalSkills.ts — runs `npx skills add ...` per ref via sandbox.exec, throws on first failure - lib/sandbox/installSessionGlobalSkills.ts — combines defaults + user refs, no-op on empty Wired into lib/sandbox/createSandboxHandler.ts after the session-row update so the DB write happens first. TDD red -> green throughout. +22 new unit tests across 6 helper files covering: shell-escape edge cases (apostrophes, empty, metacharacters), home-directory probe success/failure/empty fallback, ref schema validation (owner/repo regex, skillName whitespace), case-insensitive dedup, normalize on invalid input, default refs constant, install per ref + dedup + failure throw, session-level wrapper merging defaults + user refs (rejects whole user array on any invalid item — matches open-agents). Plus 3 new tests on createSandboxHandler asserting installSessionGlobalSkills is invoked, that a thrown skill-install still 200s the response, and that no-sessionId paths skip the install. Suite: 2534 -> 2559 (+25 net new tests). pnpm lint:check clean. TS clean for new files (verified via tsc --noEmit; pre-existing TS errors in lib/tasks and lib/trigger remain on main, unrelated). What's still missing from the open-agents cutover gap analysis: - Org base-snapshot warm boot - Lifecycle workflow kicks (sandbox-created, status-check-overdue) - Failure-state self-healing in /status - Transient/unavailable error distinction in /reconnect Each of these blocks on workflow infra in api or is its own follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces session-scoped global skill installation during sandbox provisioning. It adds type definitions and validation for global skills, platform defaults, sandbox utilities for HOME resolution and shell escaping, installation logic with error handling, and integrates early session authorization and best-effort skill setup into the sandbox creation handler. ChangesSession Global Skills Installation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 14 files
Confidence score: 4/5
- This PR is likely safe to merge with minimal risk, since the only reported issue is a naming/export convention mismatch rather than a clear runtime defect.
- In
lib/skills/globalSkillRef.ts, the exported function name does not match the filename basename, which can reduce discoverability and make module usage patterns less consistent for maintainers. - Given severity 5/10 (with high confidence), this is a moderate maintainability concern but not an obvious merge-blocking regression by itself.
- Pay close attention to
lib/skills/globalSkillRef.ts- align the primary exported function name with the filename to avoid confusion and convention drift.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/skills/globalSkillRef.ts">
<violation number="1" location="lib/skills/globalSkillRef.ts:46">
P2: Custom agent: **Module should export a single primary function whose name matches the filename**
Exported function name does not match filename basename</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Client (User)
participant Handler as createSandboxHandler
participant Valid as validateCreateSandboxBody
participant SessionDB as Supabase Sessions
participant Sandbox as Sandbox (Fly Machines)
participant Skills as installSessionGlobalSkills
participant HomeProbe as resolveSandboxHomeDirectory
participant SkillInst as installGlobalSkills
Note over Client,SkillInst: NEW: Sandbox Creation with Skill Installation
Client->>Handler: POST /api/sandbox { sessionId?, repoUrl, ... }
Handler->>Valid: Validate body + auth
Valid-->>Handler: { body, auth }
alt sessionId provided
Handler->>SessionDB: selectSessions({ id: sessionId })
SessionDB-->>Handler: sessionRow
alt sessionRow not found
Handler-->>Client: 404 { error: "Session not found" }
else account mismatch
Handler-->>Client: 403 { error: "Forbidden" }
end
end
Handler->>Sandbox: connectSandbox()
Sandbox-->>Handler: sandbox handle
alt sessionRow exists
Handler->>SessionDB: updateSession(sessionRow.id, { sandbox_state, lifecycle_state: "active", ... })
SessionDB-->>Handler: OK
Note over Handler,SkillInst: Best-effort skill installation
alt installSessionGlobalSkills succeeds
Handler->>Skills: installSessionGlobalSkills({ sessionRow, sandbox })
Skills->>Skills: normalizeGlobalSkillRefs(sessionRow.global_skill_refs)
alt user refs invalid
Skills->>Skills: return [] (fallback to defaults only)
end
Skills->>Skills: merge DEFAULT_GLOBAL_SKILL_REFS + userRefs
Skills->>SkillInst: installGlobalSkills({ sandbox, globalSkillRefs })
SkillInst->>SkillInst: globalSkillRefsSchema.parse() (validate + dedupe)
alt empty refs list
SkillInst-->>Skills: return immediately
end
SkillInst->>HomeProbe: resolveSandboxHomeDirectory(sandbox)
HomeProbe->>Sandbox: exec('printf %s "$HOME"')
alt probe succeeds
Sandbox-->>HomeProbe: "/home/agent"
else probe fails or empty
HomeProbe->>HomeProbe: fallback to "/root"
end
HomeProbe-->>SkillInst: homeDirectory
loop For each deduped ref
SkillInst->>Sandbox: exec(`HOME=<homeDirectory> npx skills add <source> --skill <name> --agent amp -g -y --copy`)
alt command succeeds
Sandbox-->>SkillInst: { success: true }
else command fails
SkillInst->>SkillInst: throw Error with stderr
SkillInst-->>Skills: caught error propagates
end
end
SkillInst-->>Skills: done
Skills-->>Handler: done
else installSessionGlobalSkills throws
Handler->>Handler: console.error (log and continue)
end
end
Handler-->>Client: 200 { createdAt, ... }
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| * @param value - The raw JSON value (typically `row.global_skill_refs`). | ||
| * @returns Validated and deduped refs, or `[]` on any parse failure. | ||
| */ | ||
| export function normalizeGlobalSkillRefs(value: unknown): GlobalSkillRef[] { |
There was a problem hiding this comment.
P2: Custom agent: Module should export a single primary function whose name matches the filename
Exported function name does not match filename basename
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/skills/globalSkillRef.ts, line 46:
<comment>Exported function name does not match filename basename</comment>
<file context>
@@ -0,0 +1,49 @@
+ * @param value - The raw JSON value (typically `row.global_skill_refs`).
+ * @returns Validated and deduped refs, or `[]` on any parse failure.
+ */
+export function normalizeGlobalSkillRefs(value: unknown): GlobalSkillRef[] {
+ const parsed = globalSkillRefsSchema.safeParse(value);
+ return parsed.success ? parsed.data : [];
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
lib/sandbox/createSandboxHandler.ts (1)
30-128: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
createSandboxHandlerexceeds the 50-line function limit — extract phases into focused helpers.The function body runs ~98 lines across six distinct phases: body validation, session auth, sandbox provisioning, session state update, skill install, and response. Each phase is a standalone concern.
Suggested extraction:
-export async function createSandboxHandler(request: NextRequest): Promise<NextResponse> { - const validated = await validateCreateSandboxBody(request); - if (validated instanceof NextResponse) return validated; - const { body, auth } = validated; - - const sessionId = body.sessionId; - let sessionRow: Tables<"sessions"> | null = null; - if (sessionId) { - const rows = await selectSessions({ id: sessionId }); - sessionRow = rows[0] ?? null; - if (!sessionRow) { - return NextResponse.json( - { status: "error", error: "Session not found" }, - { status: 404, headers: getCorsHeaders() }, - ); - } - if (sessionRow.account_id !== auth.accountId) { - return NextResponse.json( - { status: "error", error: "Forbidden" }, - { status: 403, headers: getCorsHeaders() }, - ); - } - } - // ... rest of handler -} +// In a new file: lib/sandbox/authorizeSession.ts +export async function authorizeSession( + sessionId: string, + accountId: string, +): Promise<Tables<"sessions"> | NextResponse> { ... } + +// In a new file: lib/sandbox/persistSandboxSession.ts +export async function persistSandboxSession( + sessionRow: Tables<"sessions">, + sandbox: Sandbox, +): Promise<void> { ... } + +// Slimmed handler +export async function createSandboxHandler(request: NextRequest): Promise<NextResponse> { + const validated = await validateCreateSandboxBody(request); + if (validated instanceof NextResponse) return validated; + const { body, auth } = validated; + + let sessionRow: Tables<"sessions"> | null = null; + if (body.sessionId) { + const result = await authorizeSession(body.sessionId, auth.accountId); + if (result instanceof NextResponse) return result; + sessionRow = result; + } + // ... provision, persist, install, respond (~30 lines) +}As per coding guidelines for
lib/**/*.ts: "Keep functions under 50 lines" and "Single responsibility per function."🤖 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 `@lib/sandbox/createSandboxHandler.ts` around lines 30 - 128, The createSandboxHandler is too long—split its six phases into small helpers and make the handler just orchestrate them: extract session authorization into a function like authorizeSession(sessionId, auth) that calls selectSessions and returns sessionRow or throws NextResponse; extract sandbox provisioning into provisionSandbox({repoUrl, sandboxName}) that wraps connectSandbox and returns sandbox or throws; extract the DB update logic into updateSessionState(sessionRow, sandbox) (calling updateSession); extract skill install into installSessionSkills(sessionRow, sandbox) (wrapping installSessionGlobalSkills and doing best-effort logging); and extract response construction into buildCreateSandboxResponse(sandbox, startTime). Update createSandboxHandler to call validateCreateSandboxBody, authorizeSession, provisionSandbox, updateSessionState, installSessionSkills, and return buildCreateSandboxResponse so the handler stays under 50 lines and each helper has a single responsibility.
🤖 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 `@lib/skills/globalSkillRef.ts`:
- Around line 46-49: Move the exported function normalizeGlobalSkillRefs out of
globalSkillRef.ts into a new file named normalizeGlobalSkillRefs.ts; in the new
lib/skills/normalizeGlobalSkillRefs.ts export the normalizeGlobalSkillRefs
function (keeping its implementation the same) and import the
globalSkillRefsSchema and GlobalSkillRef type from globalSkillRef.ts; then
remove the normalizeGlobalSkillRefs export/definition from globalSkillRef.ts so
that globalSkillRef.ts only contains the schemas and types and the file name
matches its primary exports.
- Around line 7-17: The Zod error message calls on the source and skillName
schema entries are using the deprecated string shorthand (.min(1, "msg") and
.regex(..., "msg")); update these to Zod 4's object form by replacing .min(1,
"Source is required") with .min(1, { error: "Source is required" }) and .min(1,
{ error: "Skill name is required" }) and similarly replace the .regex calls for
GLOBAL_SKILL_SOURCE_PATTERN and GLOBAL_SKILL_NAME_PATTERN with
.regex(GLOBAL_SKILL_SOURCE_PATTERN, { error: "Source must be in owner/repo
format" }) and .regex(GLOBAL_SKILL_NAME_PATTERN, { error: "Skill name cannot
contain spaces" }) respectively so that the source and skillName validators use
the { error: "..." } syntax.
---
Outside diff comments:
In `@lib/sandbox/createSandboxHandler.ts`:
- Around line 30-128: The createSandboxHandler is too long—split its six phases
into small helpers and make the handler just orchestrate them: extract session
authorization into a function like authorizeSession(sessionId, auth) that calls
selectSessions and returns sessionRow or throws NextResponse; extract sandbox
provisioning into provisionSandbox({repoUrl, sandboxName}) that wraps
connectSandbox and returns sandbox or throws; extract the DB update logic into
updateSessionState(sessionRow, sandbox) (calling updateSession); extract skill
install into installSessionSkills(sessionRow, sandbox) (wrapping
installSessionGlobalSkills and doing best-effort logging); and extract response
construction into buildCreateSandboxResponse(sandbox, startTime). Update
createSandboxHandler to call validateCreateSandboxBody, authorizeSession,
provisionSandbox, updateSessionState, installSessionSkills, and return
buildCreateSandboxResponse so the handler stays under 50 lines and each helper
has a single responsibility.
🪄 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: CHILL
Plan: Pro
Run ID: f6b883c6-876c-4365-924a-97c660b74dd5
⛔ Files ignored due to path filters (7)
lib/sandbox/__tests__/createSandboxHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/installSessionGlobalSkills.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/resolveSandboxHomeDirectory.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/shellEscape.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/skills/__tests__/defaultGlobalSkillRefs.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/skills/__tests__/globalSkillRef.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/skills/__tests__/installGlobalSkills.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (7)
lib/sandbox/createSandboxHandler.tslib/sandbox/installSessionGlobalSkills.tslib/sandbox/resolveSandboxHomeDirectory.tslib/sandbox/shellEscape.tslib/skills/defaultGlobalSkillRefs.tslib/skills/globalSkillRef.tslib/skills/installGlobalSkills.ts
| source: z | ||
| .string() | ||
| .trim() | ||
| .min(1, "Source is required") | ||
| .regex(GLOBAL_SKILL_SOURCE_PATTERN, "Source must be in owner/repo format"), | ||
| skillName: z | ||
| .string() | ||
| .trim() | ||
| .min(1, "Skill name is required") | ||
| .regex(GLOBAL_SKILL_NAME_PATTERN, "Skill name cannot contain spaces"), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and check package.json for Zod version
fd -t f "package.json" -x cat {}Repository: recoupable/api
Length of output: 2645
🏁 Script executed:
# Locate the file mentioned in the review
fd "globalSkillRef.ts" -t fRepository: recoupable/api
Length of output: 87
🏁 Script executed:
# If file exists, display its contents
fd "globalSkillRef.ts" -t f -x cat -n {}Repository: recoupable/api
Length of output: 1980
🌐 Web query:
Zod 4 error message API min regex custom error documentation
💡 Result:
In Zod 4, custom error messages for string validators like.min and.regex are set using the { error: "message" } or { error: (iss) => "message" } parameter.[1][2][3] For.min: z.string.min(5, { error: "Too short!" })[1] Or with iss object: z.string.min(5, { error: (iss) => Password must have ${iss.minimum} characters or more }); // iss has.minimum,.inclusive,.code="too_small"[1][4] For.regex: z.string.regex(/^[a-z]+$/, { error: "Must contain only lowercase letters!" })[2] Note: Early Zod 4 betas had a bug where.regex ignored custom errors (issue #4098, fixed by May 2025).[5] Default.regex error: "Invalid string: must match pattern /.../" with code "invalid_format", format: "regex".[5] The iss object provides: iss.code, iss.input, iss.path, iss.inst (schema), and for min: iss.minimum; for regex: iss.pattern.[1] Official docs: https://v4.zod.dev/error-customization (primary source).[1] Migration: message -> error; errorMap -> error function.[3][4][6]
Citations:
- 1: https://v4.zod.dev/error-customization?id=global-error-customization
- 2: https://v4.zod.dev/api?id=error
- 3: https://www.zod.dev/v4?id=custom-email-regex
- 4: https://www.zod.dev/v4/changelog?id=drops-zstringip
- 5: v4: Custom error message for .regex() is ignored colinhacks/zod#4098
- 6: https://www.zod.dev/v4/changelog?id=error-customization
Update error message syntax to Zod 4 standard.
The codebase uses Zod 4.1.13, which uses the { error: "..." } form for custom error messages. Replace the deprecated string shorthand syntax (e.g., .min(1, "message")) with the object form across lines 10–11 and 15–16.
Zod 4 error API update
export const globalSkillRefSchema = z.object({
source: z
.string()
.trim()
- .min(1, "Source is required")
- .regex(GLOBAL_SKILL_SOURCE_PATTERN, "Source must be in owner/repo format"),
+ .min(1, { error: "Source is required" })
+ .regex(GLOBAL_SKILL_SOURCE_PATTERN, { error: "Source must be in owner/repo format" }),
skillName: z
.string()
.trim()
- .min(1, "Skill name is required")
- .regex(GLOBAL_SKILL_NAME_PATTERN, "Skill name cannot contain spaces"),
+ .min(1, { error: "Skill name is required" })
+ .regex(GLOBAL_SKILL_NAME_PATTERN, { error: "Skill name cannot contain spaces" }),
});📝 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.
| source: z | |
| .string() | |
| .trim() | |
| .min(1, "Source is required") | |
| .regex(GLOBAL_SKILL_SOURCE_PATTERN, "Source must be in owner/repo format"), | |
| skillName: z | |
| .string() | |
| .trim() | |
| .min(1, "Skill name is required") | |
| .regex(GLOBAL_SKILL_NAME_PATTERN, "Skill name cannot contain spaces"), | |
| }); | |
| source: z | |
| .string() | |
| .trim() | |
| .min(1, { error: "Source is required" }) | |
| .regex(GLOBAL_SKILL_SOURCE_PATTERN, { error: "Source must be in owner/repo format" }), | |
| skillName: z | |
| .string() | |
| .trim() | |
| .min(1, { error: "Skill name is required" }) | |
| .regex(GLOBAL_SKILL_NAME_PATTERN, { error: "Skill name cannot contain spaces" }), | |
| }); |
🤖 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 `@lib/skills/globalSkillRef.ts` around lines 7 - 17, The Zod error message
calls on the source and skillName schema entries are using the deprecated string
shorthand (.min(1, "msg") and .regex(..., "msg")); update these to Zod 4's
object form by replacing .min(1, "Source is required") with .min(1, { error:
"Source is required" }) and .min(1, { error: "Skill name is required" }) and
similarly replace the .regex calls for GLOBAL_SKILL_SOURCE_PATTERN and
GLOBAL_SKILL_NAME_PATTERN with .regex(GLOBAL_SKILL_SOURCE_PATTERN, { error:
"Source must be in owner/repo format" }) and .regex(GLOBAL_SKILL_NAME_PATTERN, {
error: "Skill name cannot contain spaces" }) respectively so that the source and
skillName validators use the { error: "..." } syntax.
| export function normalizeGlobalSkillRefs(value: unknown): GlobalSkillRef[] { | ||
| const parsed = globalSkillRefsSchema.safeParse(value); | ||
| return parsed.success ? parsed.data : []; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move normalizeGlobalSkillRefs to its own normalizeGlobalSkillRefs.ts file.
The coding guideline requires each file to export one primary function and the file name must match that function's name. normalizeGlobalSkillRefs is a function exported from globalSkillRef.ts; it needs its own lib/skills/normalizeGlobalSkillRefs.ts. The schemas and the GlobalSkillRef type can stay in globalSkillRef.ts.
♻️ Proposed split
lib/skills/normalizeGlobalSkillRefs.ts (new file):
+import { globalSkillRefsSchema, type GlobalSkillRef } from "@/lib/skills/globalSkillRef";
+
+/**
+ * Best-effort parse of an arbitrary `global_skill_refs` JSON value into
+ * a deduped array of valid refs. Invalid input returns `[]` instead of
+ * throwing — caller code can safely combine the result with the
+ * platform defaults.
+ */
+export function normalizeGlobalSkillRefs(value: unknown): GlobalSkillRef[] {
+ const parsed = globalSkillRefsSchema.safeParse(value);
+ return parsed.success ? parsed.data : [];
+}lib/skills/globalSkillRef.ts — remove normalizeGlobalSkillRefs:
-export function normalizeGlobalSkillRefs(value: unknown): GlobalSkillRef[] {
- const parsed = globalSkillRefsSchema.safeParse(value);
- return parsed.success ? parsed.data : [];
-}As per coding guidelines: "The file name MUST match the exported function name. If a new function is defined in a file whose name does not match, leave a review comment telling the developer to create a new file named after the function."
📝 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.
| export function normalizeGlobalSkillRefs(value: unknown): GlobalSkillRef[] { | |
| const parsed = globalSkillRefsSchema.safeParse(value); | |
| return parsed.success ? parsed.data : []; | |
| } | |
| import { globalSkillRefsSchema, type GlobalSkillRef } from "@/lib/skills/globalSkillRef"; | |
| /** | |
| * Best-effort parse of an arbitrary `global_skill_refs` JSON value into | |
| * a deduped array of valid refs. Invalid input returns `[]` instead of | |
| * throwing — caller code can safely combine the result with the | |
| * platform defaults. | |
| */ | |
| export function normalizeGlobalSkillRefs(value: unknown): GlobalSkillRef[] { | |
| const parsed = globalSkillRefsSchema.safeParse(value); | |
| return parsed.success ? parsed.data : []; | |
| } |
🤖 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 `@lib/skills/globalSkillRef.ts` around lines 46 - 49, Move the exported
function normalizeGlobalSkillRefs out of globalSkillRef.ts into a new file named
normalizeGlobalSkillRefs.ts; in the new lib/skills/normalizeGlobalSkillRefs.ts
export the normalizeGlobalSkillRefs function (keeping its implementation the
same) and import the globalSkillRefsSchema and GlobalSkillRef type from
globalSkillRef.ts; then remove the normalizeGlobalSkillRefs export/definition
from globalSkillRef.ts so that globalSkillRef.ts only contains the schemas and
types and the file name matches its primary exports.
Smoke test results — preview deploymentRan end-to-end against `https://api-git-feat-sandbox-skill-install-recoup.vercel.app\`. All paths green, with a measurable signal that skill install actually ran. ✅ Regression — negative paths
✅ Happy path — provision + skill install`POST /api/sandbox` with a fresh session: ```json The ✅ Downstream endpoints still consistent after skill install`GET /api/sandbox/status` after provision: `GET /api/sandbox/reconnect` after provision: Both happy-path lifecycle reads agree with each other and with the provision write — `lifecycleVersion: 1`, `lifecycle.state: "active"`, and `sandboxExpiresAt` is the same epoch (1778180608651) on both endpoints. Best-effort behavior — not tested live this roundThe unit test `returns 200 even when skill installation throws` covers the failure path with a mocked rejection. Inducing a real install failure live would require feeding a bogus skill source through the session row, which seemed lower-value than the latency-based proof above. Comfortable shipping. Ready to merge whenever you give the word. |
Skill installation verified — direct sandbox shellFollowup on the latency-based proof. Shelled into the running session sandbox via `vercel sandbox exec` and confirmed the skill files actually landed. Sandbox state`vercel sandbox list --scope recoup --project api` → one running sandbox bound to the smoke-test session: `sbx_bF4DuXFiXFr3DaMSegjTxc0JNJMO`. Skill files present```bash /home/vercel-sandbox/.agents/skills/recoup-api/SKILL.md Both default skills landed in the right place. `$HOME` resolved to `/home/vercel-sandbox` (not `/root` — the actual default for Vercel Sandbox cloud images), so the `resolveSandboxHomeDirectory` probe is doing its job vs the hardcoded fallback. Skill files are real, not stubs```bash name: recoup-api
|
1 participant
Summary
Closes the second-largest gap identified in the cutover analysis: every session sandbox now installs the platform default skills (
recoup-api,artist-workspace) plus any user-configuredglobalSkillRefsfrom the session row, so agents come up with their tools loaded.After this lands, the cutover gap shrinks from "agents start without skills" + 4 other regressions to just the workflow-infra-dependent ones.
Behavior
After
connectSandboxsucceeds and the session-row state is persisted,createSandboxHandlercallsinstallSessionGlobalSkills({ sessionRow, sandbox })wrapped intry/catch. The installer:row.global_skill_refs(Json from Supabase)normalizeGlobalSkillRefs(best-effort parse — invalid input falls back to[])DEFAULT_GLOBAL_SKILL_REFS(recoup-api,artist-workspace)npx skills add <source> --skill <name> --agent amp -g -y --copyonce per deduped ref viasandbox.exec, withHOME=resolved from aprintf %s "$HOME"probe (falls back to/root)A skill-install failure does not fail the sandbox creation request. Sandbox + DB stay consistent, the agent comes up without skills (or with whichever subset succeeded before the throw), and the user can recover with a follow-up request once the underlying issue is fixed. Matches open-agents' error handling exactly.
Files
lib/sandbox/shellEscape.tsnpx skills add ...lib/sandbox/resolveSandboxHomeDirectory.ts$HOME, falls back to/rootlib/skills/globalSkillRef.tsnormalizeGlobalSkillRefslib/skills/defaultGlobalSkillRefs.tsrecoup-api+artist-workspaceplatform defaultslib/skills/installGlobalSkills.tslib/sandbox/installSessionGlobalSkills.tslib/sandbox/createSandboxHandler.tsTDD
Red → green throughout. +25 net new tests (suite 2534 → 2559):
shellEscape: apostrophe escaping, empty string, shell metacharacters preservedresolveSandboxHomeDirectory: probe success / failure / empty-string fallbackglobalSkillRef: owner/repo regex, skillName whitespace rejection, case-insensitive dedup, normalize-on-invaliddefaultGlobalSkillRefs: contents + source consistencyinstallGlobalSkills: empty short-circuit, per-refexecinvocation with HOME, dedup, failure throwinstallSessionGlobalSkills: defaults-only path, defaults + valid user refs, all-user-refs-dropped on partial invalidity (matches open-agents)createSandboxHandler: installer is invoked when sessionId provided, response stays 200 when installer throws (best-effort), installer skipped when no sessionIdVerification
pnpm testpnpm lint:checktsc --noEmitfor new fileslib/tasksandlib/triggerremain on main, unrelated)Out of scope
Still missing per the original gap analysis (each blocks on workflow infra or is its own follow-up):
findOrgSnapshot+kickBuildOrgSnapshotWorkflow) — biggest remaining gap; ~75s slower cold start every session until this landsreason: "sandbox-created",reason: "status-check-overdue") — needs a workflow runner in apiTest plan
POST /api/sandboxwith a fresh session → 200, then verify the agent hasrecoup-apiandartist-workspaceskills available (check via~/.skills/listing in the sandbox or by prompting the agent to list its skills)global_skill_refsarray → those skills are also present🤖 Generated with Claude Code
Summary by cubic
Install default platform skills and any session-configured global skills into every new sandbox so agents start with tools ready. Installation is best-effort and won’t block sandbox creation (matches open-agents).
recoup-apiandartist-workspacefromrecoupable/skillsby default.global_skill_refsfrom the session, normalizes and dedupes; invalid input is ignored.npx skills add <source> --skill <name> --agent amp -g -y --copyper ref via the sandbox, withHOMEresolved (fallback/root).Written for commit b7ca6da. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements