feat: Agent Teams MCP (lead + teammate)#12
Merged
Conversation
Two MCP servers for coordinating multiple AI agent sessions as a team: - agent-teams-lead: spawn teams, create tasks, send messages, monitor progress - agent-teams-teammate: claim tasks, communicate, publish artifacts Inspired by Anthropic's Claude Code agent teams, built as an editor-agnostic MCP layer that works with Kiro, Cursor, Claude Code, and OpenCode.
|
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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThese changes introduce two new npm packages for an agent team orchestration system: agent-teams-lead-mcp and agent-teams-teammate-mcp. The lead package manages team creation, process spawning, task coordination, and messaging. The teammate package enables individual agent processes to participate in teams via shared store access, supporting task lifecycle management and artifact handling. Both implement MCP servers with file-based persistence, file locking for concurrent safety, and graceful shutdown handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Lead as LeadMCPServer
participant Store as TeamStore
participant Spawner as TeammateSpawner
participant Teammate as TeammateMCPServer<br/>(Process)
Lead->>Spawner: initialize spawner
Lead->>Lead: call spawnTeam(objective, configs)
Lead->>Store: call store.spawnTeam()
Store->>Store: create team, generate IDs
Store->>Store: persist to JSON
Store-->>Lead: return Team
loop for each teammate config
Lead->>Spawner: spawnTeammate(agent, mcp_servers)
Spawner->>Spawner: generate agent config
Spawner->>Spawner: generate prompt
Spawner->>Teammate: spawn process via kiro-cli
Teammate->>Teammate: load TEAMMATE_ID, TEAMMATE_NAME
Teammate->>Teammate: initialize TeammateMCPServer
Teammate-->>Spawner: process started
Spawner-->>Lead: teammate spawned
end
Note over Lead,Teammate: Teams initialized and agents running
sequenceDiagram
participant Lead as LeadMCPServer
participant Store as TeamStore
participant Teammate as TeammateMCPServer
Lead->>Lead: call createTask(title, description, deps)
Lead->>Store: store.createTask()
Store->>Store: persist task to JSON
Store-->>Lead: return Task {id, status: pending}
Teammate->>Teammate: call listTasks()
Teammate->>Store: store.listTasks({status: pending})
Store-->>Teammate: return pending tasks
Teammate->>Teammate: select task, call claimTask(taskId)
Teammate->>Store: store.claimTask(taskId, agentId)
Store->>Store: validate pending, unassigned, dependencies
Store->>Store: update task.assigned_to, status=in_progress
Store->>Store: persist update via file lock
Store-->>Teammate: return claimed Task
Teammate->>Teammate: work on task
Teammate->>Teammate: call completeTask(summary, artifacts)
Teammate->>Store: store.completeTask(taskId, params)
Store->>Store: update status=completed, add summary, artifacts
Store->>Store: persist via file lock
Store-->>Teammate: return completed Task
Lead->>Store: store.getTasks({status: completed})
Store-->>Lead: return completed tasks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 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: 7
🧹 Nitpick comments (10)
packages/agent-teams-teammate/package.json (1)
1-29: Consider adding afilesfield for cleaner npm publishing.Without a
filesfield, npm will publish all files not excluded by.npmignoreor.gitignore. Adding an explicitfilesarray ensures only necessary files are included in the published package."files": [ "dist" ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-teammate/package.json` around lines 1 - 29, The package.json for `@arvoretech/agent-teams-teammate-mcp` is missing a "files" field which causes npm to include everything not ignored; add a "files" array to package.json (e.g., include "dist" and any other runtime assets like bin or README if needed) so only necessary artifacts are published, then rebuild to ensure dist is present for the "main" entry and the CLI bin ("agent-teams-teammate-mcp") to work correctly.packages/agent-teams-lead/package.json (1)
1-12: Whitelist the published payload todist/.This package is public and both entry points resolve under
dist/, so adding afilesallowlist avoids publishingsrc/, configs, and other repo artifacts unintentionally.📦 Proposed change
{ "name": "@arvoretech/agent-teams-lead-mcp", "version": "0.1.0", "description": "MCP server for the team lead agent — spawn teammates, create tasks, coordinate work", "main": "dist/index.js", "type": "module", + "files": [ + "dist" + ], "publishConfig": { "access": "public" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-lead/package.json` around lines 1 - 12, Add a "files" allowlist to package.json to ensure only the compiled distribution is published: add a top-level "files" array containing "dist/" (and optionally "README.md" or "LICENSE" if you want them published) so the package's public entry points ("main": "dist/index.js" and the "bin" field "agent-teams-lead-mcp": "./dist/index.js") only expose dist and not src or other repo artifacts; update package.json accordingly.packages/agent-teams-lead/tsconfig.json (1)
4-5: Use a modern Node module resolver here.This package is configured as a Node-executed ESM CLI (evidenced by
"type": "module","bin"entry point, and direct execution vianode dist/index.js). TypeScript's legacyNoderesolver does not fully model modern Node ESM resolution semantics. For Node 20+, the recommended configuration ismodule: "NodeNext"paired withmoduleResolution: "NodeNext", which aligns TypeScript's type-checking with Node's actual module resolution behavior.♻️ Proposed fix
"compilerOptions": { "target": "ES2022", - "module": "ESNext", - "moduleResolution": "Node", + "module": "NodeNext", + "moduleResolution": "NodeNext",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-lead/tsconfig.json` around lines 4 - 5, Update the TypeScript module settings to use the modern Node ESM resolver: change the "module" and "moduleResolution" compilerOptions entries (currently "module": "ESNext" and "moduleResolution": "Node") to "NodeNext" so TypeScript's resolution matches Node 20+ ESM semantics for this package's ESM CLI setup.packages/agent-teams-lead/src/spawner.ts (1)
137-145: Process termination lacks graceful shutdown timeout.When stopping a teammate,
SIGTERMis sent but there's no timeout before the process reference is deleted. If the process doesn't terminate promptly, resources may leak and the config cleanup happens before the process actually exits.♻️ Add timeout and force-kill for unresponsive processes
async stopTeammate(teammateId: string): Promise<void> { const spawned = this.processes.get(teammateId); if (!spawned) return; await this.log(teammateId, "Stopping teammate process"); spawned.process.kill("SIGTERM"); + + // Wait briefly for graceful shutdown, then force kill + await new Promise<void>((resolve) => { + const timeout = setTimeout(() => { + if (!spawned.process.killed) { + spawned.process.kill("SIGKILL"); + } + resolve(); + }, 5000); + spawned.process.once("exit", () => { + clearTimeout(timeout); + resolve(); + }); + }); + this.processes.delete(teammateId); await this.cleanupAgentConfig(spawned.agentConfigPath); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-lead/src/spawner.ts` around lines 137 - 145, In stopTeammate, add a graceful shutdown wait after sending SIGTERM to ensure the child actually exits before deleting its record and cleaning config: after spawned.process.kill("SIGTERM") wait for the process 'exit' (or 'close') event with a short timeout (e.g. 5–10s); if the timeout elapses, send SIGKILL and await final exit, then remove the entry from this.processes and call cleanupAgentConfig(spawned.agentConfigPath). Use the existing spawned.process reference and ensure any listeners are cleaned up to avoid leaks.packages/agent-teams-lead/src/filelock.ts (1)
2-2: Unused import.
existsSyncis imported but never used in this file.🧹 Remove unused import
import { mkdir, rmdir, stat } from "node:fs/promises"; -import { existsSync } from "node:fs";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-lead/src/filelock.ts` at line 2, The import existsSync in packages/agent-teams-lead/src/filelock.ts is unused; remove the existsSync named import from the top-level import statement (or delete the entire import line if nothing else is imported) so the file no longer contains an unused symbol, ensuring no behavioral changes to functions like any file locking utilities in this module.packages/agent-teams-lead/src/store.ts (1)
208-217:getTasksreads from stale in-memory cache.This method returns tasks from the in-memory
this.tasksarray without refreshing from disk. In a multi-process environment (which this system is designed for—multiple teammate agents), tasks may have been updated by other processes, leading to stale reads. Consider either:
- Refreshing from disk before returning, or
- Documenting that
load()must be called to get fresh dataNote:
teamStatus()in tools.ts correctly callsload()before reading, but callers may forget this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-lead/src/store.ts` around lines 208 - 217, getTasks currently returns from the in-memory this.tasks and can return stale data; make it refresh from disk before filtering by converting getTasks to async and calling await this.load() at the start (or invoke whatever synchronous disk-refresh helper exists) so callers always see up-to-date tasks, then apply the existing status/assigned_to filters; alternatively, if you must keep it sync, update the function docstring to clearly state that callers must call load() beforehand (reference getTasks and load methods to locate the code).packages/agent-teams-lead/src/server.ts (1)
167-176: Signal handlers should properly await async shutdown.The
shutdownfunction is async, but the signal handlers call it without awaiting. Whileprocess.exit(0)insideshutdownwill terminate anyway, the exception handlers on lines 169-176 callprocess.exitsynchronously without invokingshutdown, so spawned processes may not be stopped cleanly on uncaught exceptions.Consider invoking
stopAll()in exception handlers for consistency:Proposed fix
process.on("uncaughtException", async (error) => { console.error("Uncaught exception:", error); + await this.spawner.stopAll(); process.exit(1); }); process.on("unhandledRejection", async (reason) => { console.error("Unhandled rejection:", reason); + await this.spawner.stopAll(); process.exit(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-lead/src/server.ts` around lines 167 - 176, Signal and exception handlers currently call async shutdown logic without awaiting and exception handlers call process.exit directly; update the process.on callbacks for "SIGINT" and "SIGTERM" to call and await shutdown("SIGINT"/"SIGTERM") (use an async wrapper so the await runs), and change the "uncaughtException" and "unhandledRejection" handlers to await stopAll() (or await shutdown("exception") if you prefer unified shutdown flow) before calling process.exit(1), referencing the existing shutdown and stopAll functions to ensure spawned processes are stopped cleanly.packages/agent-teams-teammate/src/filelock.ts (1)
1-68: Code duplication withpackages/agent-teams-lead/src/filelock.ts.This file is identical to the lead package's
filelock.ts. Consider extracting this into a shared utility package or a common module to avoid maintaining duplicate code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-teammate/src/filelock.ts` around lines 1 - 68, The file duplicates file lock logic found in the lead package; extract the shared implementation (functions like acquireLock, releaseLock, isLockStale, withFileLock and constants STALE_LOCK_MS, RETRY_INTERVAL_MS, MAX_WAIT_MS) into a single shared module or utility package and have both packages import that module instead of keeping identical copies; update the current file to import and re-export (or directly use) the shared withFileLock API, remove the duplicated implementation, and ensure types/exports match so callers in both agent-teams-teammate and agent-teams-lead work without code changes.packages/agent-teams-teammate/src/store.ts (1)
6-58: Consider sharing type definitions with the lead package.The
Team,Task,Message, andArtifactinterfaces are likely duplicated frompackages/agent-teams-lead/src/types.ts. Since both packages operate on the same data files, extracting these types into a shared package would ensure consistency and reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-teammate/src/store.ts` around lines 6 - 58, The Team, Task, Message, and Artifact interfaces in store.ts are duplicated; extract these type definitions into a shared module (e.g., a new package or an existing shared package) and export them so both agent-teams-teammate and agent-teams-lead import the same types. Remove the local interface declarations from packages/agent-teams-teammate/src/store.ts and replace them with imports for Team, Task, Message, and Artifact from the shared module, and update any other files that currently declare or use duplicate definitions to import the shared types as well.packages/agent-teams-lead/src/index.ts (1)
1-21: Module executes server on import — consider separating CLI entry from library exports.This file serves dual purposes: CLI entry point (lines 8-15 run on load) and library exports (lines 17-21). Importing this module for its exports (e.g.,
import { TeamStore } from 'agent-teams-lead') will also attempt to start the server.If library consumption is intended, consider moving the startup logic to a separate
cli.tsfile and keepingindex.tspurely for exports. Alternatively, guard the execution with a check.Option: Guard execution when used as library
+import { fileURLToPath } from "node:url"; import { resolve } from "node:path"; import { LeadMCPServer } from "./server.js"; const workspacePath = resolve(process.env.WORKSPACE_PATH || process.cwd()); +// Only run server when executed directly, not when imported as a module +const isMain = process.argv[1] === fileURLToPath(import.meta.url); + +if (isMain) { -try { const server = new LeadMCPServer(workspacePath); server.setupGracefulShutdown(); - await server.start(); -} catch (error) { - console.error("Failed to start Agent Teams Lead MCP Server:", error); - process.exit(1); -} + server.start().catch((error) => { + console.error("Failed to start Agent Teams Lead MCP Server:", error); + process.exit(1); + }); +} export { LeadMCPServer } from "./server.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-teams-lead/src/index.ts` around lines 1 - 21, This module currently constructs and starts LeadMCPServer on import (creating workspacePath, new LeadMCPServer(...), calling setupGracefulShutdown() and await start()), so importing exports like TeamStore will inadvertently start the server; fix by moving the startup block into a separate CLI entry (e.g., cli.ts) or guard it so it only runs when executed as a script — wrap the server startup (workspacePath creation, new LeadMCPServer(...), setupGracefulShutdown(), and start()) behind a runtime check (for example using an entry-point guard) and keep index.ts exporting LeadMCPServer, TeamStore, LeadTools and the rest only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-teams-lead/package.json`:
- Around line 22-31: The workspace lockfile is out of sync with the changed
dependency specifiers in packages/agent-teams-lead/package.json; from the
repository root run pnpm install (or pnpm -w install for workspaces) to
regenerate pnpm-lock.yaml, verify the lockfile includes the new entries for
"@modelcontextprotocol/sdk" and "zod", and commit the updated pnpm-lock.yaml so
CI's pnpm install --frozen-lockfile will succeed.
In `@packages/agent-teams-lead/src/filelock.ts`:
- Around line 42-47: The code unconditionally removes the lockPath (rmdir) after
the deadline and immediately mkdirs it, which can race with a legitimately
acquired lock; either throw an error on timeout instead of force-acquiring or,
if you must force-acquire, re-verify staleness before removing and acquiring:
check the lock's metadata (timestamp/pid) or attempt an atomic mkdir and only
rm+mkdir when a fresh re-check confirms the lock is still stale. Update the
logic around rmdir/mkdir (in filelock.ts) to perform that re-check (or to throw
on timeout) to avoid simultaneous lock holders.
In `@packages/agent-teams-lead/src/spawner.ts`:
- Around line 179-218: The flattening loop for parsed.mcpServers currently
builds flatServers entries for each upstream by only setting command, args, and
env (in the block handling upstreams), which drops other upstream fields (e.g.,
transport, url, auth required by UpstreamServerConfigSchema) and breaks HTTP
upstreams; update the upstream handling in spawner.ts so that when creating
flatServers[upstream.name] you preserve all original upstream properties (spread
the upstream object) and then override/replace only the env with the resolvedEnv
(and ensure args/command remain present if needed), so fields like transport,
url, and auth are retained for mcp-proxy upstreams.
- Around line 38-45: The hardcoded teammateMcpPath built from this.workspacePath
is fragile; replace it with a dynamic resolution and/or configurable override:
accept a config option or env var (e.g., teammateMcpPath or TEAMMATE_MCP_PATH)
in the class/constructor that currently sets this.teammateMcpPath, and if not
provided use Node package resolution (require.resolve or import.meta.resolve) to
locate the "agent-teams-teammate" package entrypoint (or resolve via
resolve-package-path logic using package.json lookup from this.workspacePath).
Update the code paths that reference this.teammateMcpPath so the code falls back
to the resolved path and throws a clear error if resolution fails.
In `@packages/agent-teams-lead/src/store.ts`:
- Around line 83-121: spawnTeam currently writes team.json, tasks.json,
messages.json and artifacts.json without synchronization; wrap the critical
section in the same locking mechanism used elsewhere by calling
this.withFileLock(...) (or the existing withFileLock helper) so all file
operations are atomic. Move/reset resetNames(), teammates creation, assigning
this.team, clearing this.tasks/messages/artifacts, this.ensureDir(), and the
four this.writeJson(...) calls inside the lock callback; continue to use
teamPath(), tasksPath(), messagesPath(), artifactsPath(), generateId(),
ensureDir, and writeJson so concurrent spawnTeam callers cannot interleave and
create inconsistent files.
In `@packages/agent-teams-teammate/src/filelock.ts`:
- Around line 41-46: The force-acquire block that does try { await
rmdir(lockPath) } catch { } followed by await mkdir(lockPath) can race and throw
an unhandled EEXIST; wrap the remove-and-create sequence in a try/catch (or
catch errors from mkdir) and handle EEXIST by treating it as a failed
force-acquire and retry/loop or re-enter the existing retry flow. Specifically,
update the block that references lockPath in the lock acquisition routine so
that mkdir errors are caught (inspect error.code === 'EEXIST') and handled
deterministically (e.g., swallow EEXIST or retry after a short backoff) to avoid
both processes believing they hold the lock.
In `@packages/agent-teams-teammate/src/server.ts`:
- Around line 286-296: The handler currently returns a success response when an
artifact is missing by calling this.ok({ error: ... }) — change it to return a
proper error via this.err(...) instead (e.g., call this.err(new Error(`Artifact
${params.artifact_id} not found`)) or pass the standard error payload used
across handlers), locating the logic around this.store.load() /
this.store.getArtifact(...) and replacing the this.ok(...) branch with a
this.err(...) call so clients receive the consistent error response format.
---
Nitpick comments:
In `@packages/agent-teams-lead/package.json`:
- Around line 1-12: Add a "files" allowlist to package.json to ensure only the
compiled distribution is published: add a top-level "files" array containing
"dist/" (and optionally "README.md" or "LICENSE" if you want them published) so
the package's public entry points ("main": "dist/index.js" and the "bin" field
"agent-teams-lead-mcp": "./dist/index.js") only expose dist and not src or other
repo artifacts; update package.json accordingly.
In `@packages/agent-teams-lead/src/filelock.ts`:
- Line 2: The import existsSync in packages/agent-teams-lead/src/filelock.ts is
unused; remove the existsSync named import from the top-level import statement
(or delete the entire import line if nothing else is imported) so the file no
longer contains an unused symbol, ensuring no behavioral changes to functions
like any file locking utilities in this module.
In `@packages/agent-teams-lead/src/index.ts`:
- Around line 1-21: This module currently constructs and starts LeadMCPServer on
import (creating workspacePath, new LeadMCPServer(...), calling
setupGracefulShutdown() and await start()), so importing exports like TeamStore
will inadvertently start the server; fix by moving the startup block into a
separate CLI entry (e.g., cli.ts) or guard it so it only runs when executed as a
script — wrap the server startup (workspacePath creation, new
LeadMCPServer(...), setupGracefulShutdown(), and start()) behind a runtime check
(for example using an entry-point guard) and keep index.ts exporting
LeadMCPServer, TeamStore, LeadTools and the rest only.
In `@packages/agent-teams-lead/src/server.ts`:
- Around line 167-176: Signal and exception handlers currently call async
shutdown logic without awaiting and exception handlers call process.exit
directly; update the process.on callbacks for "SIGINT" and "SIGTERM" to call and
await shutdown("SIGINT"/"SIGTERM") (use an async wrapper so the await runs), and
change the "uncaughtException" and "unhandledRejection" handlers to await
stopAll() (or await shutdown("exception") if you prefer unified shutdown flow)
before calling process.exit(1), referencing the existing shutdown and stopAll
functions to ensure spawned processes are stopped cleanly.
In `@packages/agent-teams-lead/src/spawner.ts`:
- Around line 137-145: In stopTeammate, add a graceful shutdown wait after
sending SIGTERM to ensure the child actually exits before deleting its record
and cleaning config: after spawned.process.kill("SIGTERM") wait for the process
'exit' (or 'close') event with a short timeout (e.g. 5–10s); if the timeout
elapses, send SIGKILL and await final exit, then remove the entry from
this.processes and call cleanupAgentConfig(spawned.agentConfigPath). Use the
existing spawned.process reference and ensure any listeners are cleaned up to
avoid leaks.
In `@packages/agent-teams-lead/src/store.ts`:
- Around line 208-217: getTasks currently returns from the in-memory this.tasks
and can return stale data; make it refresh from disk before filtering by
converting getTasks to async and calling await this.load() at the start (or
invoke whatever synchronous disk-refresh helper exists) so callers always see
up-to-date tasks, then apply the existing status/assigned_to filters;
alternatively, if you must keep it sync, update the function docstring to
clearly state that callers must call load() beforehand (reference getTasks and
load methods to locate the code).
In `@packages/agent-teams-lead/tsconfig.json`:
- Around line 4-5: Update the TypeScript module settings to use the modern Node
ESM resolver: change the "module" and "moduleResolution" compilerOptions entries
(currently "module": "ESNext" and "moduleResolution": "Node") to "NodeNext" so
TypeScript's resolution matches Node 20+ ESM semantics for this package's ESM
CLI setup.
In `@packages/agent-teams-teammate/package.json`:
- Around line 1-29: The package.json for `@arvoretech/agent-teams-teammate-mcp` is
missing a "files" field which causes npm to include everything not ignored; add
a "files" array to package.json (e.g., include "dist" and any other runtime
assets like bin or README if needed) so only necessary artifacts are published,
then rebuild to ensure dist is present for the "main" entry and the CLI bin
("agent-teams-teammate-mcp") to work correctly.
In `@packages/agent-teams-teammate/src/filelock.ts`:
- Around line 1-68: The file duplicates file lock logic found in the lead
package; extract the shared implementation (functions like acquireLock,
releaseLock, isLockStale, withFileLock and constants STALE_LOCK_MS,
RETRY_INTERVAL_MS, MAX_WAIT_MS) into a single shared module or utility package
and have both packages import that module instead of keeping identical copies;
update the current file to import and re-export (or directly use) the shared
withFileLock API, remove the duplicated implementation, and ensure types/exports
match so callers in both agent-teams-teammate and agent-teams-lead work without
code changes.
In `@packages/agent-teams-teammate/src/store.ts`:
- Around line 6-58: The Team, Task, Message, and Artifact interfaces in store.ts
are duplicated; extract these type definitions into a shared module (e.g., a new
package or an existing shared package) and export them so both
agent-teams-teammate and agent-teams-lead import the same types. Remove the
local interface declarations from packages/agent-teams-teammate/src/store.ts and
replace them with imports for Team, Task, Message, and Artifact from the shared
module, and update any other files that currently declare or use duplicate
definitions to import the shared types as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a944db00-5ad6-461c-aa0b-1a66beb7e1a8
📒 Files selected for processing (17)
packages/agent-teams-lead/package.jsonpackages/agent-teams-lead/src/filelock.tspackages/agent-teams-lead/src/index.tspackages/agent-teams-lead/src/names.tspackages/agent-teams-lead/src/schemas.tspackages/agent-teams-lead/src/server.tspackages/agent-teams-lead/src/spawner.tspackages/agent-teams-lead/src/store.tspackages/agent-teams-lead/src/tools.tspackages/agent-teams-lead/src/types.tspackages/agent-teams-lead/tsconfig.jsonpackages/agent-teams-teammate/package.jsonpackages/agent-teams-teammate/src/filelock.tspackages/agent-teams-teammate/src/index.tspackages/agent-teams-teammate/src/server.tspackages/agent-teams-teammate/src/store.tspackages/agent-teams-teammate/tsconfig.json
Comment on lines
+286
to
+296
| async (params) => { | ||
| try { | ||
| await this.store.load(); | ||
| const artifact = this.store.getArtifact(params.artifact_id); | ||
| if (!artifact) return this.ok({ error: `Artifact ${params.artifact_id} not found` }); | ||
| return this.ok(artifact as unknown as Record<string, unknown>); | ||
| } catch (error) { | ||
| return this.err(error); | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
Inconsistent error handling — uses ok() to return an error.
When the artifact is not found, line 290 returns this.ok({ error: ... }) instead of this.err(...). This means clients receive a "success" response structure containing an error field, rather than the standard error format used elsewhere.
Proposed fix
async (params) => {
try {
await this.store.load();
const artifact = this.store.getArtifact(params.artifact_id);
- if (!artifact) return this.ok({ error: `Artifact ${params.artifact_id} not found` });
+ if (!artifact) {
+ return this.err(new Error(`Artifact ${params.artifact_id} not found`));
+ }
return this.ok(artifact as unknown as Record<string, unknown>);
} catch (error) {
return this.err(error);
}
}📝 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.
Suggested change
| async (params) => { | |
| try { | |
| await this.store.load(); | |
| const artifact = this.store.getArtifact(params.artifact_id); | |
| if (!artifact) return this.ok({ error: `Artifact ${params.artifact_id} not found` }); | |
| return this.ok(artifact as unknown as Record<string, unknown>); | |
| } catch (error) { | |
| return this.err(error); | |
| } | |
| } | |
| ); | |
| async (params) => { | |
| try { | |
| await this.store.load(); | |
| const artifact = this.store.getArtifact(params.artifact_id); | |
| if (!artifact) { | |
| return this.err(new Error(`Artifact ${params.artifact_id} not found`)); | |
| } | |
| return this.ok(artifact as unknown as Record<string, unknown>); | |
| } catch (error) { | |
| return this.err(error); | |
| } | |
| } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-teams-teammate/src/server.ts` around lines 286 - 296, The
handler currently returns a success response when an artifact is missing by
calling this.ok({ error: ... }) — change it to return a proper error via
this.err(...) instead (e.g., call this.err(new Error(`Artifact
${params.artifact_id} not found`)) or pass the standard error payload used
across handlers), locating the logic around this.store.load() /
this.store.getArtifact(...) and replacing the this.ok(...) branch with a
this.err(...) call so clients receive the consistent error response format.
- filelock: re-verify staleness before force-acquiring lock on timeout, handle EEXIST on mkdir - spawner: spread full upstream object to preserve url/transport/auth fields - spawner: resolve teammateMcpPath dynamically via env var or require.resolve - store: wrap spawnTeam in withFileLock for atomic file writes
- use createRequire instead of bare require (no-undef) - avoid unused destructured vars when stripping upstream fields
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Descricao
Dois novos MCP servers para coordenar múltiplas sessões de agentes IA trabalhando como um time:
Conceito inspirado no Agent Teams do Claude Code da Anthropic, construído como uma camada MCP editor-agnostic que funciona com Kiro, Cursor, Claude Code e OpenCode.
Funcionalidades
mkdirpara evitar race conditions.agent-teams/team.logEtiquetas (Labels)
Historia Relacionada
N/A
Motivacao e Contexto
Permitir que o orquestrador coordene múltiplos agentes trabalhando em paralelo, com comunicação direta entre eles — algo que sub-agents não suportam.
Como Isso Foi Testado?
Testado manualmente com times de 3 teammates, tasks com dependências, messaging, e parallel work. Verificado fix de nomes com acento (sanitizeName).
Analise de Risco e Impacto
Pacotes novos, sem impacto em código existente.
Summary by CodeRabbit
Release Notes
New Features