fix(chat/runs): install global skills in the headless run path#722
Conversation
provisionRunSession (POST /api/chat/runs, used by scheduled customer-prompt tasks) only DISCOVERED skills and never installed them — installSessionGlobalSkills ran only in createSandboxHandler (interactive POST /api/sandbox). So headless sandboxes had an empty .agents/skills/ dir, discoverSkills returned [], the agent got no `skill` tool, and it fell back to guessing API endpoints and fabricating data (chat#1822). Install global skills after connect + before discovery, best-effort (mirrors createSandboxHandler) so a failed install never blocks the run. Refs recoupable/chat#1822 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✨ 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 |
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 4/5
lib/chat/runs/provisionRunSession.tsnow exceeds the 100-line maintainability limit, which raises the chance of logic becoming harder to review and future regressions slipping in as the file grows. This PR is likely safe to merge functionally, but de-risking with a small pre-merge split (or immediate follow-up refactor) would keep complexity from compounding.
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/chat/runs/provisionRunSession.ts">
<violation number="1" location="lib/chat/runs/provisionRunSession.ts:80">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
File `provisionRunSession.ts` exceeds the 100-line limit (111 lines) after this change, violating the maintainability rule's file-length and single-responsibility constraints.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Scheduler as External Scheduler (cron)
participant API as API /chat/runs
participant Provision as provisionRunSession
participant Sandbox as Sandbox Handle (Vercel)
participant Install as installSessionGlobalSkills
participant Discover as discoverSkills
Scheduler->>API: POST /api/chat/runs (scheduled task)
API->>Provision: provisionRunSession({accountId, title})
Provision->>Provision: createSessionWithInitialChat
Provision->>Sandbox: connectSandbox(...)
Sandbox-->>Provision: sandbox handle (workingDir)
Provision->>Provision: markSessionSandboxActive(session, state)
Provision-->>Provision: updated session row
Note over Provision,Install: NEW: install global skills before discovery (fixes empty skills dir)
Provision->>Install: installSessionGlobalSkills({sessionRow: updated, sandbox})
Install->>Install: copy skills into sandbox ~/.agents/skills/
Install-->>Provision: (resolved or rejected)
alt install fails (best-effort)
Provision->>Provision: catch error, log warning, continue
end
Provision->>Discover: discoverSkills(sandbox)
Note over Discover: CHANGED: now finds installed skills (e.g., recoup-platform-api-access)
Discover-->>Provision: skill list (non-empty)
Provision-->>Provision: buildAgentTools (hasSkills=true → skill tool available)
Provision-->>API: { session, sandbox, skills }
API-->>Scheduler: 200 OK, run started with skills
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -5,6 +5,7 @@ import { getSessionSandboxName } from "@/lib/sandbox/getSessionSandboxName"; | |||
| import { resolveGitUser } from "@/lib/sandbox/resolveGitUser"; | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
File provisionRunSession.ts exceeds the 100-line limit (111 lines) after this change, violating the maintainability rule's file-length and single-responsibility constraints.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/runs/provisionRunSession.ts, line 80:
<comment>File `provisionRunSession.ts` exceeds the 100-line limit (111 lines) after this change, violating the maintainability rule's file-length and single-responsibility constraints.</comment>
<file context>
@@ -76,6 +77,18 @@ export async function provisionRunSession({
const updated = await markSessionSandboxActive(session, sandbox.getState() as Json);
if (!updated) throw new Error("Failed to activate session sandbox");
+ // Install global skills BEFORE discovery — the headless run path must
+ // provision skills, not just discover them. Without this the sandbox skills
+ // dir is empty, `discoverSkills` returns [], the agent gets no `skill` tool,
</file context>
What
Call
installSessionGlobalSkills({ sessionRow, sandbox })insideprovisionRunSession— afterconnectSandbox/markSessionSandboxActive, beforediscoverSkills. Best-effort (try/catch + log), mirroringcreateSandboxHandler.Why
Scheduled tasks (
customer-prompt-task → generateChat → POST /api/chat/runs) provision their sandbox viaprovisionRunSession, which only discovered skills and never installed them.installSessionGlobalSkills(laysrecoup-platform-api-access+ roster skills into${HOME}/.agents/skills/) ran only increateSandboxHandler(interactivePOST /api/sandbox).Headless result: empty
.agents/skills/→discoverSkillsreturns[]→buildAgentToolshasSkills=false→ agent gets noskilltool → can't loadrecoup-platform-api-access, guesses endpoints, gets 404/empty, fabricates. Root cause for recoupable/chat#1822 (0/6 audited runs loaded any skill).Tests (TDD, RED→GREEN)
New
lib/chat/runs/__tests__/provisionRunSession.test.ts: asserts install is called with{ sessionRow, sandbox }and ordered beforediscoverSkills(RED→GREEN), plus best-effort (rejected install still completes). Full domain suite green (370 passed), eslint clean, no new tsc errors in changed files.Verify on preview (Done-when)
A scheduled run's trace shows a
skilltool call loadingrecoup-platform-api-access(not curl-guessing). Gated on recoupable/skills#65 so the loaded skill points atapi.recoupable.dev.Refs recoupable/chat#1822 · merge order: skills#65 → this → api reliability · base
test🤖 Generated with Claude Code
Summary by cubic
Install global skills during headless run provisioning so scheduled runs load tools instead of guessing APIs. Fixes
recoupable/chat#1822by ensuring the agent gets the skill tool and avoiding an empty.agents/skills/directory.provisionRunSession, callinstallSessionGlobalSkillsafterconnectSandbox/markSessionSandboxActiveand beforediscoverSkills; wrapped in try/catch to stay non-blocking.recoup-platform-api-access.Written for commit 8f08e2f. Summary will update on new commits.