Skip to content

🤖 refactor: decompose src/node/ god services — Phase 0+1+2a+3a #2270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
ammar-agent wants to merge 14 commits into
base: main
Choose a base branch
from
Open

🤖 refactor: decompose src/node/ god services — Phase 0+1+2a+3a #2270

ammar-agent wants to merge 14 commits into from

Conversation

@ammar-agent
Copy link
Collaborator

@ammar-agent ammar-agent commented Feb 8, 2026 •
edited
Loading

Summary

Systematic decomposition of the three largest src/node/ service files, reducing god-object complexity and improving SoC. Part of the multi-phase cleanup plan following AIService decomposition (PR #2243).

Background

After merging AIService decomposition, the next targets are the remaining god objects:

  • workspaceService.ts (3,488 LoC → 2,607)
  • agentSession.ts (2,303 LoC → 1,356)
  • taskService.ts (2,680 LoC → 2,469)

Implementation

Phase 0: Cross-Cutting Cleanup

  • Error coercion standardization: Replaced 214 occurrences of error instanceof Error ? error.message : String(error) with getErrorMessage() across 78 files
  • Dead export removal: Removed 6 unused exports, deleted agentPresets.ts entirely
  • Test setup cleanup: Replaced 30-line stale manual spread in tests/ipc/setup.ts with services.toORPCContext()

Phase 1: WorkspaceService Decomposition

  • 1a: Extracted subagentArtifactArchival.ts (467 LoC) — pure functions for archiving child session artifacts
  • 1b: Extracted PostCompactionService (325 LoC) — plan files, tracked paths, exclusions, debounced metadata emission
  • 1c: Extracted workspaceAISettings.ts (231 LoC) — 7 pure functions for normalizing/persisting AI settings
  • 1e: Extracted guardStreamingAllowed helper — deduplicates ~50 lines of identical guard checks between sendMessage and resumeStream

Phase 2: AgentSession Decomposition

  • 2a: Extracted ContextExceededRetryHandler (806 LoC) — entire retry logic for context limits with 3 strategies + 8 state fields
  • 2b: Extracted PostCompactionAttachmentBuilder (207 LoC) — post-compaction context assembly (plan refs, TODOs, edited-file diffs). Also deduplicates two near-identical 50-line attachment assembly blocks
  • 2c: Extracted SnapshotMaterializer (199 LoC) — stateless @file mention and agent skill snapshot functions
  • 2d: Decomposed sendMessage god function — extracted handleEditTruncation and validateModelAndFiles as private methods, reducing sendMessage from 310 to ~219 lines

Phase 3: TaskService Decomposition

  • 3a: Extracted TaskHierarchyIndex (322 LoC) — 15 pure functions for building/querying ancestor/descendant hierarchy

LoC Impact

File Before After Δ
workspaceService.ts 3,488 2,607 -881
agentSession.ts 2,303 1,356 -947
taskService.ts 2,680 2,469 -211
Total removed -2,039
New focused modules 2,557 7 files

Validation

  • All 3,540 tests pass
  • make typecheck clean
  • make lint clean (1 pre-existing error in code_execution.ts, unrelated)
  • Rebased cleanly on latest main

Risks

  • Low: All extractions are pure delegation or function moves — no behavioral changes. Test coverage validates that the decomposition is transparent to callers.
  • Phase 3c (TaskWaiterManager) was skipped due to deep state coupling with TaskService lifecycle methods — extraction would add forwarding overhead without sufficient readability benefit.

Generated with mux • Model: anthropic:claude-opus-4-6 • Thinking: xhigh • Cost: $293.89

@ammar-agent
refactor: adopt getErrorMessage() utility across 78 files
e27cf5d 
Replace 214 inline `error instanceof Error ? error.message : String(error)`
patterns with the existing `getErrorMessage()` utility from
`src/common/utils/errors.ts`.

This eliminates the most pervasive DRY violation in the codebase (172+
occurrences across 30+ files). Heaviest adopters:
- workspaceService.ts: 32 replacements
- projectService.ts: 13 replacements
- orpc/router.ts: 8 replacements
- historyService.ts: 8 replacements
@ammar-agent
refactor: remove 6 dead exports
4bfea32
@ammar-agent
refactor: replace stale manual spread in tests/ipc/setup.ts
99b8f73 
Replace ~30-line manual service field spread with services.toORPCContext(),
which was added specifically to centralize this mapping. Remove the now-unused
ORPCContext type import.
@ammar-agent
refactor: extract subagent artifact archival to dedicated module
13ed905 
Move archiveChildSessionArtifactsIntoParentSessionDir and its supporting
helper functions (isErrnoWithCode, isPathInsideDir, copyFileBestEffort,
copyDirIfMissingBestEffort, coerceUpdatedAtMs, rollUpAncestorWorkspaceIds)
from workspaceService.ts to a new subagentArtifactArchival.ts module.

This reduces workspaceService.ts by ~450 lines with no logic changes.
The class method that calls archiveChildSessionArtifactsIntoParentSessionDir
now imports it from the new module.
@ammar-agent
refactor: extract workspace AI settings to dedicated module
b7df920 
Extract 7 workspace AI settings methods from WorkspaceService into
standalone functions in src/node/services/workspaceAISettings.ts:

- normalizeWorkspaceAISettings: validate + normalize model/thinking
- normalizeSendMessageAgentId: normalize agentId in options
- extractWorkspaceAISettingsFromSendOptions: extract AI settings from send opts
- maybePersistAISettingsFromOptions: best-effort settings persistence
- persistWorkspaceAISettingsForAgent: persist settings to config + emit
- updateAgentAISettings: validate + persist AI settings

WorkspaceService retains the same public API (updateModeAISettings,
updateAgentAISettings) and private helpers, all delegating to the new
module. An EmitMetadataFn callback bridges the session/event emission.

Zero logic changes — pure extraction.
@ammar-agent
refactor: extract TaskHierarchyIndex from TaskService
ff5102a 
Extract all task hierarchy/indexing query methods from taskService.ts
into a new taskHierarchyIndex.ts module. These are pure functions
operating on config data, making them the cleanest extraction target
with zero circular dependency risk.

Extracted functions:
- buildAgentTaskIndex, listAgentTaskWorkspaces (index building)
- listDescendantAgentTaskIdsFromIndex, isDescendantAgentTaskUsingParentById,
  listAncestorWorkspaceIdsUsingParentById (tree traversal)
- getTaskDepth, getTaskDepthFromParentById (depth helpers)
- countActiveAgentTasks, hasActiveDescendantAgentTasks,
  listActiveDescendantAgentTaskIds, listDescendantAgentTasks (active queries)
- getAgentTaskStatus (status query)

TaskService's public API remains unchanged — methods now delegate to
the standalone functions. countActiveAgentTasks takes isForegroundAwaiting
and isStreaming callbacks to avoid service-level dependencies.
@ammar-agent
refactor: extract PostCompactionService from WorkspaceService
03ee7cf 
Move 7 post-compaction state management methods into dedicated
PostCompactionService class:
- getPostCompactionState, getPostCompactionExclusions, setPostCompactionExclusion
- getPersistedPostCompactionDiffPaths, deletePlanFilesForWorkspace
- schedulePostCompactionMetadataRefresh, emitPostCompactionMetadata

WorkspaceService delegates to the new service via composition.
~230 lines moved to focused module.
@ammar-agent
refactor: extract ContextExceededRetryHandler from AgentSession
0f4b928 
Extract all context-exceeded retry logic (~600 lines + 8 state fields)
from AgentSession into a dedicated ContextExceededRetryHandler class.

The handler owns:
- 8 state fields (retry attempt sets, active stream context/flags)
- 11 methods: handleStreamError, three retry strategies
  (compaction, post-compaction, hard restart exec subagent),
  and supporting helpers

AgentSession interacts with the handler via:
- initStreamState/markStreamHadDelta/etc. for state updates
- handleStreamError as the error-handling entry point
- Callbacks (emitChatEvent, clearQueue, streamWithHistory, etc.)
  for handler->session communication

Zero logic changes - pure extraction with preserved test compatibility.
All 31 agentSession tests pass including postCompactionRetry suite.
@ammar-agent
test: update WorkspaceService tests for extracted PostCompaction + AI…
9d5620a 
…Settings

- Debounce test patches PostCompactionService directly instead of
  WorkspaceService delegation wrappers
- AI settings tests use saveConfig mock (the real side effect) instead
  of monkey-patching the now-standalone persistWorkspaceAISettingsForAgent
@ammar-agent ammar-agent force-pushed the refactor/node-cleanup-phase0 branch from 1fd2812 to 9d5620a Compare February 9, 2026 01:42
@ammar-agent
refactor: extract guardStreamingAllowed helper in WorkspaceService
2a3fcda 
Deduplicates ~50 lines of identical guard checks between sendMessage
and resumeStream (renaming/removing/missing/queued state checks).
@ammar-agent
refactor: extract SnapshotMaterializer from AgentSession
95f8591 
Move materializeFileAtMentionsSnapshot and materializeAgentSkillSnapshot
to src/node/services/snapshotMaterializer.ts as stateless free functions.
AgentSession delegates with explicit dependency parameters.

agentSession.ts: 1680 → 1527 LoC (-153)
@ammar-agent
refactor: extract PostCompactionAttachmentBuilder from AgentSession
0cdd951 
Move post-compaction attachment assembly (plan refs, TODOs, edited-file
diffs, turn-counter state) to src/node/services/postCompactionAttachments.ts.
Also deduplicates two near-identical 50-line attachment assembly blocks
into a shared buildAttachments() method.

agentSession.ts: 1527 → 1323 LoC (-204)
@ammar-agent
refactor: decompose sendMessage in AgentSession
5cd4ac2 
Extract handleEditTruncation (~80 LoC) and validateModelAndFiles (~45 LoC)
as private methods, reducing sendMessage from 310 to ~219 lines. The method
now reads as a high-level orchestrator with clearly named sub-steps.
@ammar-agent
fix: convert ToolBridge import to type-only after removing preGenerat…
4d29253 
…eMuxTypes

Removing preGenerateMuxTypes (dead export) left ToolBridge with only
type-level usage, triggering consistent-type-imports lint rule.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ammar-agent