feat: Integrate Agent Teams, subagent memory, and worktree isolation - #296
Conversation
…eatures Design document for #295 covering Agent Teams experimental flag, 5 new hooks (TeammateIdle, TaskCreated, TaskCompleted, WorktreeCreate, WorktreeRemove), memory/isolation/mcpServers on agents, .worktreeinclude, and a porting guide for other repos. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Key changes from doc verification: - Drop WorktreeCreate/WorktreeRemove hooks (they replace default git behavior entirely, too risky for notification-only use) - Replace mcpServers scoping with disallowedTools (mcpServers is additive, not restrictive) - Document exit code semantics (exit 2 blocks operations) - Add verified input fields for each hook event - Reduce from 5 hooks to 3 (Agent Teams only) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
disallowedTools requires exact tool names (no wildcards confirmed), making it fragile across dozens of MCP tools. Agent prompts already constrain tool usage. Only audit agents change: memory: user + isolation: worktree. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…295) Enable experimental Agent Teams with event bus coordination hooks, persistent cross-session memory for audit agents, and git worktree isolation for clean audit snapshots. Changes: - Add CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1 env flag - Add 3 Agent Teams hooks (TeammateIdle, TaskCreated, TaskCompleted) that publish events to event bus for cross-agent visibility - Add memory: user + isolation: worktree to 5 audit agents - Add .worktreeinclude for copying .env files into worktrees - Add 12 new tests (syntax, graceful degradation, integration) - Update hooks/README.md with lifecycle diagram and hook details - Add docs/agent-teams-setup.md porting guide Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Summary
Clean integration of Agent Teams, subagent memory/isolation, and worktree include. The three new hooks follow established patterns with proper graceful degradation, and the design spec clearly documents why WorktreeCreate/WorktreeRemove and MCP scoping were dropped from scope.
Verdict
APPROVE — Only suggestions, no critical or important issues. Well-structured PR with thorough tests and documentation.
Automated review by Claude Code
|
|
||
| # Parse input | ||
| SESSION_ID=$(echo "$INPUT" | jq -r '.session_id // ""') | ||
| TEAMMATE_NAME=$(echo "$INPUT" | jq -r '.teammate_name // "unknown"') |
There was a problem hiding this comment.
[Suggestion] Minor inconsistency: TEAMMATE_NAME defaults to "unknown" here, but to "" in task-created.sh and task-completed.sh. The task hooks conditionally append (by: ...) only when non-empty, which is cleaner. Consider using // "" here too and conditionally appending the teammate name to the payload, matching the task hooks' pattern.
| bash "$HOOKS_DIR/teammate-idle.sh" >/dev/null 2>&1 || exit_code=$? | ||
|
|
||
| [[ $exit_code -eq 0 ]] | ||
| } |
There was a problem hiding this comment.
[Suggestion] The happy path tests verify exit code 0 but don't assert that the mock CLI was called with the correct event type, payload, or channel. The mock infrastructure could capture args (e.g., to a temp file) so tests verify the publish command receives --type "teammate_idle", --channel "repo:...", etc. This would catch payload construction bugs that exit-code-only tests miss.
| --payload "$PAYLOAD" \ | ||
| --session-id "$SESSION_ID" \ | ||
| --channel "repo:${REPO_NAME}" \ | ||
| 2>/dev/null || true |
There was a problem hiding this comment.
[Suggestion] The task_description field is available in the input JSON (documented in the README and spec) but not included in the published payload. If event bus consumers would benefit from richer context, consider adding description as a separate flag or appending a truncated version. Fine to keep concise if intentional — just flagging since the field isn't used in either task hook.
Default TEAMMATE_NAME to "" instead of "unknown", matching the conditional-append pattern used in task-created.sh and task-completed.sh. Addresses claude-review suggestion on PR #296. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Feedback AddressedImplemented
Skipped
|
There was a problem hiding this comment.
Code Review
Summary
Clean integration of Agent Teams hooks, audit agent memory/isolation, and worktree include. All three hooks follow established patterns with correct graceful degradation, || true safety on publish, and set -euo pipefail. Design decisions to drop WorktreeCreate/WorktreeRemove hooks (replace default behavior — too risky) and MCP scoping (additive, not restrictive) are well-documented in the spec.
Previously Addressed (Filtered)
- teammate-idle.sh TEAMMATE_NAME default — Implemented (conditional append matching task hooks pattern)
- Arg-capturing test assertions — Skipped (existing convention is exit-code-only)
- task_description in payload — Skipped (concise payloads for event bus readability)
All 3 items verified — no re-raised findings.
Verdict
APPROVE — No critical or important issues. Hooks are safe (always exit 0), tests cover all degradation paths, and documentation is thorough.
Automated review by Claude Code
Summary
CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1)TeammateIdle,TaskCreated,TaskCompleted) that publish events to event bus for cross-agent coordinationmemory: user+isolation: worktreeto 5 audit agents for persistent learning and clean snapshots.worktreeincludefor copying.envfiles into worktreesFixes #295
Test plan
make checkpasses (lint, test, hooks, bootstrap — 68 + 29 tests)🤖 Generated with Claude Code