docs: add Claude Code agent teams implementation plan#50
docs: add Claude Code agent teams implementation plan#50
Conversation
7-phase plan for extending DJ to support Claude Code alongside Codex, with multi-agent team orchestration, shared context, and unified approvals. Designed to run in parallel with existing Codex v2 roadmap. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
hessnd
left a comment
There was a problem hiding this comment.
PR Review: Claude Code Agent Teams Implementation Plan
Verdict: Strong direction, not yet execution-ready. The phased structure and additive-first mindset are right. But the proposal overstates independence from existing code, assumes unvalidated Claude IPC surfaces, and has a safety gap around approvals that needs closing before implementation begins.
🔴 Blockers (fix before treating as actionable)
1. Provider interface contract is underspecified for DJ's architecture
The proposed Provider interface (lines 65–72) has several gaps when checked against the actual codebase:
ReadLoop(handler func(Event))has no error channel and nocontext.Contextfor cancellation. Today'sClient.ReadLoopalready has a goroutine lifecycle problem (no explicit stop signal). The Provider contract should fix this, not inherit it.Event{Type string, Raw json.RawMessage}is stringly-typed. DJ's bridge layer (bridge_v2.go) works with typedtea.Msgvalues (ThreadStartedMsg,V2ExecApprovalMsg, etc.). Where does the canonicalEvent → tea.Msgmapping live? The doc implies the TUI owns it, but doesn't define the contract boundary.SendApproval(requestID string, approved bool)doesn't match current patterns. Today's approvals are type-specific (client_send.gohandles exec vs file approvals). A singleboolwon't carry enough info for Claude's tool approval model.- No
SendInputmethod — DJ's core interaction model is PTY-first. How does user input reach a Claude session?
Recommendation: Define whether Provider emits typed, provider-neutral event structs (preferred) or raw envelopes that TUI decodes per-provider. Pick one and spec it.
2. Need a Phase 0: Claude IPC feasibility spike
Phase 2.1 lists --output-format stream-json, Agent SDK Go bindings, and sub-agent lifecycle events as integration points, but none of these are validated. Open Question #1 and #2 (lines 455–458) acknowledge this but treat it as appendix rather than a gate.
Recommendation: Insert Phase 0 with concrete acceptance criteria: capture real claude CLI streaming output, confirm approval/tool-call event granularity, confirm sub-agent lifecycle observability, and produce a minimal mapping to one DJ view.
3. Approval safety must come before multi-agent enablement
Today handleV2ExecApproval and handleV2FileApproval in app_proto_v2.go auto-approve everything (SendApproval(..., true)). Phase 5 (approval queue) is sequenced after Phase 2 (Claude adapter). This means the first Claude integration will auto-approve all tool calls with no manual gate — a serious safety gap, especially in multi-agent mode.
Recommendation: Move approval policy config into Phase 1 or 2. Default to manual approvals when provider != codex or when team.agents is configured. Auto-approve should be opt-in per tool via allowlist.
4. Session pane integration for Claude is undefined
DJ's "Enter → session" flow spawns a PTYSession with a command (currently codex). The proposal doesn't specify whether Claude sessions are: (a) attachable PTYs, (b) rendered from streamed deltas in a virtual terminal, or (c) a new session model entirely. This is core UX, not a detail.
Recommendation: Add a "Session Pane Integration" subsection to Phase 2 with the MVP approach (e.g., "render streamed text deltas; defer full PTY integration").
5. CreateSession() sync return doesn't match Codex's async model
CreateSession(opts SessionOpts) (string, error) implies synchronous thread creation. But Codex thread creation is notification-driven (thread/started event). The CodexProvider adapter can't return a thread ID synchronously without blocking or faking it.
Recommendation: Change to async: CreateSession returns a request/correlation ID, and the actual thread ID arrives via a ThreadStarted event. Or explicitly document that CodexProvider stubs this differently.
🟡 Should-fix (accuracy, compatibility, design)
6. "Phases 1–3 are new files only" is inaccurate (lines 20, 58)
Phase 1.3 explicitly says "Update AppModel to accept Provider" — that's internal/tui/app.go. Phase 6 modifies bridge_v2.go. Phase 3 modifies config.go. These are fine to do, but the claim should be corrected so reviewers can assess real conflict risk.
7. Config backward compatibility needs a plan (lines 156–167)
The proposal introduces [codex] and [claude-code] top-level tables alongside existing [appserver] and [interactive]. Questions:
- Is
[appserver]renamed to[codex]? If so, what's the migration path? - Hyphenated TOML key
[claude-code]may be awkward with Viper's dot-path lookups. Consider[providers.claude]or[claude_code]. - What happens for existing users with only
[appserver]/[interactive]/[ui]? They should see zero behavior change.
8. Event mapping table mixes confirmed and speculative mappings (lines 133–142)
The Phase 2.3 table maps Claude events to DJ message types as if both sides are known. Neither Claude's streaming event taxonomy nor its approval granularity is confirmed. Split into "Confirmed mappings" and "Desired mappings (pending Phase 0)."
9. TokensUsed int64 needs input/output split (line 244)
The pricing config (line 340) has separate input and output rates, but ThreadState.TokensUsed is a single counter. You can't compute cost without knowing the split. Change to InputTokens + OutputTokens and document graceful degradation if a provider only reports aggregate.
10. TeamStore mutex may conflict with Bubble Tea's model (lines 254–258)
TeamStore with sync.RWMutex introduces shared mutable state accessed from multiple goroutines. DJ's current pattern routes everything through tea.Msg → single-goroutine Update(). Consider making TeamStore updates go through tea.Msg types rather than direct mutex-protected writes, matching how ThreadStore is used today.
11. ContextBus (line 277) likely duplicates tea.Msg
A pub/sub "context bus" adds concurrency surface when DJ already has a message bus (program.Send). Reframe as: "Team events are tea.Msg types; TeamStore logs them for display" — unless there's a proven need for cross-goroutine pub/sub.
🟢 Nice-to-have
- Add per-phase "Done when…" acceptance criteria (e.g., "Claude adapter can render live output in session pane," "approval queue blocks execution until user action")
- Add a migration section: "Existing users with only
[appserver]/[interactive]/[ui]see no behavior change" - Open Questions 1–4 (lines 455–461) are good but should be promoted to blocking prerequisites rather than appendix items
Suggested phase reordering
Phase 0: Claude IPC spike (validate streaming events, approvals, sub-agent lifecycle)
Phase 1: Provider interface (typed contract, no behavior change for Codex path)
Phase 2: Approval safety gate (manual-by-default for non-Codex, policy config)
Phase 3: Claude MVP single agent (session pane rendering, basic thread cards)
Phase 4: Config extensions + team topology
Phase 5: Team orchestration (after roadmap item #3)
Phase 6: Token/cost dashboard
Phase 7: Git worktree integration
What's good
- Clear dependency analysis (Phase 4 ↔ roadmap item #3)
- Additive-first approach minimizes merge conflict risk
- File impact summary is useful for planning
- Team config shape (
[[team.agents]]) is clean and extensible - Worktree integration vision (Phase 7) is well-scoped as a later phase
Overall: solid foundation for a major feature. The doc needs one revision pass to close the gaps above, then it's ready to drive implementation.
Revision pass closing all blockers and should-fix items from review: - Add Phase 0 (Claude IPC feasibility spike) as a gate before adapter work - Fix Provider interface: context.Context on ReadLoop, typed events, typed ApprovalResponse, SendInput method, async CreateSession with correlation ID - Move approval safety to Phase 2 (before Claude adapter), manual-by-default - Define session pane integration for Claude (streamed text deltas MVP) - Correct "new files only" claim with explicit modified-files lists - Add config backward compatibility plan with [providers.X] namespace - Split event mapping table into confirmed vs desired (pending Phase 0) - Split TokensUsed into InputTokens/OutputTokens for accurate cost calc - Replace mutex-based TeamStore with tea.Msg-driven updates - Replace ContextBus pub/sub with tea.Msg types via program.Send - Add per-phase "Done when" acceptance criteria - Add migration section for existing users - Promote blocking open questions to Phase 0 prerequisites Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pushed a revision pass addressing all review feedback. Here's what changed: Blockers (all resolved)
Should-fix (all resolved)
Nice-to-haves (all done)
|
7-phase plan for extending DJ to support Claude Code alongside Codex, with multi-agent team orchestration, shared context, and unified approvals. Designed to run in parallel with existing Codex v2 roadmap.