Skip to content

Conversation

@newhook
Copy link
Owner

@newhook newhook commented Feb 1, 2026

Summary

This PR refactors the codebase to establish cleaner package boundaries, consistent session management, and proper dependency injection:

  • Consolidated session management in control package - EnsureControlPlane lives in control/spawn.go, no separate session package needed
  • Added CreateWorkFromBead shared method to WorkService encapsulating common steps: expand beads and create work async
  • Moved session naming utilities (SessionNameForProject, FormatTabName) from internal/claude/runner.go to internal/project/ to break import cycles
  • Moved OrchestratorManager from internal/claude/ to internal/work/ - orchestration naturally belongs with work lifecycle management
  • Removed bare EnsureSession from zellij public API - all session creation now goes through EnsureSessionWithLayout to ensure every session has a control plane tab
  • Updated orchestrator functions to require initialized sessions rather than creating them ad-hoc, ensuring consistent state
  • Cleaned up claude package to only contain Claude-specific code (prompts, templates, runner) - moved OpenConsole, OpenClaudeSession, and SpawnPlanSession to work package
  • Fixed TUI and CLI session initialization bugs - added EnsureControlPlane calls before session-dependent operations
  • Converted all tab functions to OrchestratorManager methods - eliminates package-level functions that encouraged bypassing the interface
  • Consolidated WorkService methods - moved ImportPRAsync and DestroyWork from standalone functions to WorkService methods
  • Drastically simplified zellij package - removed unused interface methods, made types private, minimal Session interface with only 5 methods

Changes

Package Restructuring

From To What
claude/runner.go project/project.go SessionNameForProject(), FormatTabName()
claude/orchestrator.go work/orchestrator.go OrchestratorManager interface and implementation
claude/runner.go work/tabs.go OpenConsole(), OpenClaudeSession(), SpawnPlanSession(), TerminateWorkTabs()

Session Management

  • EnsureControlPlane in control/spawn.go handles all session/control plane lifecycle
  • Returns *InitResult with SessionCreated and SessionName for callers that need session info
  • Callers (cmd/work.go, cmd/run.go, TUI) are responsible for calling control.EnsureControlPlane
  • The work package no longer manages sessions - it just creates work records

WorkService Consolidation

  • CreateWorkFromBead encapsulates bead expansion and async work creation
  • Callers handle EnsureControlPlane separately (cleaner separation of concerns)
  • ImportPRAsync and DestroyWork moved to WorkService methods
  • Removed duplicate CreateWorkAsync - use CreateWorkAsyncWithOptions instead

Zellij Package Simplification

Session interface reduced to 5 methods:

  • CreateTabWithCommand - create tab running a command
  • SwitchToTab - switch to named tab
  • QueryTabNames - list all tabs
  • TabExists - check if tab exists
  • TerminateAndCloseTab - terminate process and close tab

Architecture changes:

  • Renamed Client to sessionManager (private)
  • New() returns SessionManager interface
  • Implementation moved to session struct (eliminates delegation)

Bug Fixes

  • TUI openConsole: Fixed initialization order - EnsureControlPlane now called before OpenConsole
  • TUI openClaude: Added missing EnsureControlPlane call before OpenClaudeSession
  • CLI console/claude commands: Added EnsureControlPlane calls to co work console and co work claude commands

API Changes

  • zellij.SessionManager no longer exposes EnsureSession() or CreateSession()
  • Functions like SpawnWorkOrchestrator, OpenConsole check that session exists and error if not
  • All tab operations are methods on OrchestratorManager
  • CollectIssueIDsForAutomatedWorkflow accepts beads.Reader interface

Issues Resolved

  • ac-7xrb: Refactor session and orchestrator management for cleaner package boundaries
    • ac-7xrb.1: Move SessionNameForProject and FormatTabName to project package
    • ac-7xrb.2: Move OrchestratorManager from claude to work package
    • ac-7xrb.3: Remove EnsureSession from zellij public API
    • ac-7xrb.4: Update orchestrator functions to require initialized session
    • ac-7xrb.5: Clean up claude package to only contain Claude-specific code
    • ac-7xrb.6: Fix TUI openConsole session initialization order
    • ac-7xrb.8: Fix TUI openClaude missing session initialization
    • ac-7xrb.9: Add session initialization to CLI console/claude commands

Breaking Changes

  • zellij.SessionManager.EnsureSession() and CreateSession() are no longer available
  • claude.OrchestratorManager moved to work.OrchestratorManager
  • claude.SessionNameForProject() and claude.FormatTabName() moved to project package
  • Tab management functions (OpenConsole, OpenClaudeSession, etc.) moved from claude to work package

Test Plan

  • All existing tests pass (go test ./...)
  • Manual testing of work creation and orchestration flow
  • TUI console opening works correctly with new initialization order
  • co work console and co work claude commands work with existing sessions
  • Code review completed (3 review iterations)
  • CI passing

🤖 Generated with Claude Code

newhook and others added 14 commits February 1, 2026 09:31
- Add SessionNameForProject and FormatTabName to internal/project
- Update claude package to delegate to project (maintains backward compat)
- Update control/spawn.go to use project package directly
- This breaks the import cycle between claude and control packages

Closes: ac-7xrb.1

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create internal/work/orchestrator.go with OrchestratorManager interface,
  DefaultOrchestratorManager, and orchestrator functions (SpawnWorkOrchestrator,
  EnsureWorkOrchestrator, TerminateWorkTabs)
- Remove duplicate functions from claude/runner.go
- Delete obsolete claude/orchestrator.go and claude/orchestrator_mock.go
- Update work/service.go to use local OrchestratorManager
- Update work/work.go to use local TerminateWorkTabs
- Update control/plane.go to use work.SpawnWorkOrchestrator
- Update testutil/harness.go to use work.OrchestratorManagerMock
- Update tui/tui_plan_work.go to use work.EnsureWorkOrchestrator
- Regenerate mocks

Closes: ac-7xrb.2

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updated SpawnWorkOrchestrator, OpenConsole, OpenClaudeSession, and
SpawnPlanSession to require that the zellij session already exists:

- Replace zc.EnsureSession() calls with zc.SessionExists() checks
- Return error if session doesn't exist, guiding caller to use
  control.InitializeSession or control.EnsureControlPlane
- Add documentation noting that callers must initialize the session

This ensures sessions are never created without control planes,
which was the issue when EnsureSession created bare sessions.

Closes: ac-7xrb.4

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove EnsureSession and CreateSession from SessionManager interface
- Update SpawnControlPlane to assume session already exists
- All session creation now goes through EnsureSessionWithLayout
- Regenerate zellij mock

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move OpenConsole, OpenClaudeSession, SpawnPlanSession, PlanTabName to work/tabs.go
- Remove deprecated SessionNameForProject and FormatTabName wrappers
- Update cmd/work.go and internal/tui/tui_plan_work.go to use work package
- Claude package now only contains prompt building and Claude runner code

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move EnsureControlPlane() before OpenConsole() in tui_plan_work.go
to ensure the zellij session exists before attempting to open a
console pane.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Eliminates package-level functions that encouraged bypassing the
OrchestratorManager interface. All tab operations now go through
the interface for proper dependency injection and testability.

Changes:
- Convert TerminateWorkTabs, SpawnWorkOrchestrator, EnsureWorkOrchestrator
  to DefaultOrchestratorManager methods in orchestrator.go
- Convert OpenConsole, OpenClaudeSession, SpawnPlanSession to
  DefaultOrchestratorManager methods in tabs.go
- Update TUI to use workService.OrchestratorManager for all tab ops
- Update control plane to pass DB to NewControlPlane for spawner
- Update CLI commands to create OrchestratorManager locally

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create internal/session package with Initialize, EnsureControlPlane,
  SpawnControlPlane (extracted from control/spawn.go)
- Add CreateWorkFromBead to WorkService encapsulating common work
  creation logic (expand beads, create work, init session, ensure CP)
- Update CollectIssueIDsForAutomatedWorkflow to accept beads.Reader
  interface instead of *beads.Client
- Simplify TUI executeCreateWork to use CreateWorkFromBead
- Update CLI commands (work.go, run.go, work_import_pr.go) to use
  session package directly
- control/spawn.go now delegates to session package (backward compat)

This breaks the import cycle: work -> session, control -> work (no cycle)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move ImportPRAsync from standalone function to WorkService method
- Move DestroyWork from standalone function to WorkService method
- Remove duplicate CreateWorkAsync, keep only CreateWorkAsyncWithOptions
- Update WorkDestroyer interface to not require project parameter
- Change NewControlPlane to take *project.Project instead of *db.DB

This eliminates code duplication and makes the WorkService API consistent.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The Session interface had methods that were only used internally by the
Client struct but never called from outside the zellij package:

- WriteASCII, WriteChars, SendEnter, ExecuteCommand (low-level input)
- SendCtrlC (replaced session.go usage with TerminateAndCloseTab)
- RunFloating, ToggleFloatingPanes, Run (floating pane operations)

This reduces the interface surface area and mock complexity (~430 lines
removed from zellij_mock.go).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
External callers should use the SessionManager interface, not the
concrete Client type. This enforces proper abstraction and makes
mocking easier.

- Renamed Client to client (private)
- New() now returns SessionManager instead of *Client
- Updated tests to use type assertion for internal field access

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Instead of session delegating to client methods, session now contains
the actual implementation directly. This eliminates the unnecessary
indirection where every session method just called the corresponding
client method with the session name.

- session struct now holds config values (copied from client on creation)
- All tab/pane operations implemented directly on session
- client only handles session-level operations (list, exists, create)
- Helper methods (writeASCII, sendCtrlC, etc.) are now private on session
- Added test to verify session inherits config from client

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The struct implementing SessionManager interface should be named
sessionManager, not client.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Removed methods that were never called externally:
- CreateTab (only CreateTabWithCommand is used)
- CloseTab (only used internally by TerminateAndCloseTab)
- TerminateProcess (only used internally by TerminateAndCloseTab)
- ClearAndExecute (never used)

Also removed:
- ASCIIEnter constant (was only used by removed methods)
- writeChars, sendEnter, executeCommand helpers (only used by ClearAndExecute)
- CommandDelay and SessionStartWait fields from session struct (not used)

The Session interface is now minimal: CreateTabWithCommand, SwitchToTab,
QueryTabNames, TabExists, and TerminateAndCloseTab.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@newhook newhook changed the title Refactor session and orchestrator management for cleaner package boundaries Refactor session/orchestrator management and simplify zellij package Feb 1, 2026
newhook and others added 5 commits February 1, 2026 14:44
Delete control/spawn.go and update all callers to use session package
directly. This eliminates the unnecessary indirection layer.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
EnsureControlPlane now returns (*InitResult, error) instead of just error,
eliminating the need for callers to call both functions. Also made
spawnControlPlane private since it's only used internally.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove the session package entirely by moving EnsureControlPlane into
control. The work package no longer calls EnsureControlPlane - callers
are now responsible for ensuring the control plane is running.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove prImporter struct, move methods directly to WorkService
- Remove FetchPRMetadata passthrough, callers use GitHubClient directly
- Fix CreateBeadFromPR to use BeadsReader instead of shelling out to bd CLI
- Rename AddBeadsInternal to addBeadsInternal (unexported)
- Add GitHubClient and BeadsDir fields to WorkService
- Update handler_import_pr to use WorkService instead of creating separate instances

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ommands

Fix bugs where OpenClaudeSession and OpenConsole were called without
prior session initialization. With the new session existence checks,
these would fail if the zellij session doesn't exist.

- internal/tui/tui_plan_work.go: Add EnsureControlPlane() to openClaude()
- cmd/work.go: Add EnsureControlPlane() to runWorkConsole() and runWorkClaude()

Closes: ac-7xrb.8, ac-7xrb.9

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@newhook newhook merged commit 0767f01 into main Feb 1, 2026
3 checks passed
@newhook newhook deleted the feat/refactor-session-and-orchestrator-management-for-c branch February 1, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants