feat(runtime): ctx.team client for sandboxed agent teams#137
feat(runtime): ctx.team client for sandboxed agent teams#137khaliqgant wants to merge 2 commits into
Conversation
Implements the workforce slice of the ctx.team v1 spec (cloud/specs/ricky-ctx-team-v1.md, AgentWorkforce/cloud#1030): - TeamContext / TeamHandle / TeamSpawnArgs / TeamResult / TeamStatus types on WorkforceCtx (types.ts) per spec §6.4. - ctx.team in cloud-defaults.ts as an HTTP client over the cloud team endpoint, mirroring how ctx.workflow is wired (#136). Gated on WORKFORCE_WORKSPACE_TOKEN + WORKFORCE_CLOUD_BASE_URL; undefined otherwise. - spawn() POSTs to /api/v1/workspaces/{ws}/agents/{parentId}/team, attach() reconstructs a handle by teamId, completion() polls GET …/teams/{teamId} to terminal, cancel() POSTs …/cancel. Depends on the cloud endpoint (separate PR, must ship first). Tests: runtime 63/63 (13 new for ctx.team), typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 8 minutes and 36 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds team spawn, attach, and lifecycle management to the cloud-backed runtime context. It defines team type contracts, integrates team client creation into the runtime defaults factory, implements spawn/cancel/status/completion operations with cloud HTTP endpoints and polling logic including transient error retry, and provides comprehensive test coverage for all team operations. ChangesTeam Cloud Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🤖 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 `@packages/runtime/src/cloud-defaults.test.ts`:
- Around line 345-355: The helper withFetch replaces globalThis.fetch for the
whole process which is brittle; instead change tests that use withFetch to mock
fetch per-test using node:test's mock.module + dynamic import or inject a fetch
implementation into the module under test. Locate uses of withFetch and the
function withFetch itself, remove the global swap, and rewrite each test to call
mock.module(() => { globalThis.fetch is not mutated }) or import the module
under a mock context and pass a fake fetch via dependency injection (e.g.,
passFetch or setFetch API) so the fetch override scope is limited to that test
module and no process-global mutation occurs.
In `@packages/runtime/src/types.ts`:
- Around line 167-214: The WorkforceCtx interface is missing the optional team
property, so add an optional team?: TeamContext field to WorkforceCtx (matching
the existing workflow: WorkflowContext pattern) so handlers can access ctx.team;
locate the WorkforceCtx declaration and insert the comment/field "team?:
TeamContext" (using the TeamContext type already defined) adjacent to workflow
for consistency.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ece8705f-abf3-4272-8c30-4322447cf403
📒 Files selected for processing (3)
packages/runtime/src/cloud-defaults.test.tspackages/runtime/src/cloud-defaults.tspackages/runtime/src/types.ts
| async function withFetch( | ||
| fetchImpl: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response> | Response, | ||
| run: () => Promise<void> | ||
| ): Promise<void> { | ||
| const originalFetch = globalThis.fetch; | ||
| globalThis.fetch = fetchImpl as typeof fetch; | ||
| try { | ||
| await run(); | ||
| } finally { | ||
| globalThis.fetch = originalFetch; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js node:test, what are the default concurrency semantics for top-level tests in a single file, and what is the recommended way to safely mock globalThis.fetch per test?
💡 Result:
Default concurrency semantics for top-level tests in a single file (node:test) 1) Top-level tests (in a file) are not run in parallel by default. When process-level test isolation is disabled (the default), Node loads each test file into the same runner process, and “top level tests are executed with a concurrency of one.”[1] 2) Subtests are sequential unless you opt into parallelism. The node:test test option “concurrency” defaults to false, meaning “only one test runs at a time.” If concurrency is unspecified, subtests inherit this value from their parent; the default therefore keeps them sequential within the suite unless you set concurrency explicitly.[2][1] (How this changes with process-level isolation) If process-level test isolation is enabled, each test file runs in a separate child process; the number of child processes in parallel is controlled by --test-concurrency.[2] Recommended way to safely mock globalThis.fetch per test Use node:test’s built-in per-test mocking utilities (module mocks), rather than overwriting globalThis.fetch in shared global state. Specifically, register the mock before importing the module that uses fetch, and then dynamically import the module under test. The node:test docs/example show using mock.module(...) and then doing await import(...) after the mock is registered, ensuring each test gets its own mocked implementation in the right order.[3] Why: because when files share a context (no process isolation), tests can modify global state and affect other tests.[1] Mocking fetch via node:test’s module-mock mechanism avoids relying on a shared globalThis.fetch across concurrent tests. Practical pattern (sketch) - Inside your test callback: - call mock.module(...) to replace the module (or export) that provides fetch usage - then dynamically import the module under test If you truly must set globalThis.fetch, ensure you restore it in the same test (e.g., within afterEach / finally), because global mutation is inherently shared when isolation is off.[1]
Citations:
- 1: https://nodejs.org/api/test.html
- 2: https://nodejs.org/docs/v22.11.0/api/test.html
- 3: https://dev.to/gabrielanhaia/mocking-esm-in-2026-vitest-bun-and-nodes-mockmodule-hep
Avoid shared global fetch swapping—use per-test mocking instead.
In packages/runtime/src/cloud-defaults.test.ts (withFetch, lines 345-355), globalThis.fetch is replaced for the duration of run. node:test executes top-level tests in a single file with concurrency=1 by default, so same-file overlap is unlikely—but this is still brittle if test/process concurrency settings change or if other tests in the same process also touch globalThis.fetch. Prefer node:test’s per-test module mocking (e.g., mock.module(...) + dynamic import) or dependency injection so tests don’t rely on process-global mutation.
🤖 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 `@packages/runtime/src/cloud-defaults.test.ts` around lines 345 - 355, The
helper withFetch replaces globalThis.fetch for the whole process which is
brittle; instead change tests that use withFetch to mock fetch per-test using
node:test's mock.module + dynamic import or inject a fetch implementation into
the module under test. Locate uses of withFetch and the function withFetch
itself, remove the global swap, and rewrite each test to call mock.module(() =>
{ globalThis.fetch is not mutated }) or import the module under a mock context
and pass a fake fetch via dependency injection (e.g., passFetch or setFetch API)
so the fetch override scope is limited to that test module and no process-global
mutation occurs.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Addresses CodeRabbit on #137: - Critical: WorkforceCtx didn't declare `team?: TeamContext`, so handler authors got a type error on ctx.team even though cloud-defaults wires it at runtime. Added the field. - Minor: documented why withFetch's global-fetch swap is race-free (node:test top-level tests are sequential by default; finally restores). runtime 63/63, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Workforce slice of the ctx.team v1 spec (
cloud/specs/ricky-ctx-team-v1.md, AgentWorkforce/cloud#1030). Generated by Ricky, then validated + scoped by hand.TeamContext/TeamHandle/TeamSpawnArgs/TeamResult/TeamStatustypes onWorkforceCtx(types.ts), matching spec §6.4.ctx.teamincloud-defaults.tsas an HTTP client mirroringctx.workflow(Wire cloud workflow and integration runtime defaults #136). Gated onWORKFORCE_WORKSPACE_TOKEN+WORKFORCE_CLOUD_BASE_URL;undefinedotherwise.spawn()→ POST…/agents/{parentId}/team;attach(teamId)rebuilds a handle;completion()pollsGET …/teams/{teamId}to terminal;cancel()→ POST…/cancel.Scope / what this is NOT
This is the client only. The server endpoint, DB tables, sandbox provisioning, relayfile subtree, and token scoping live in the cloud substrate PR. This PR is inert until that merges.
Test plan
pnpm --filter @agentworkforce/runtime typecheck— cleanpnpm --filter @agentworkforce/runtime test— 63/63 (13 new for ctx.team)🤖 Generated with Claude Code