🤖 refactor: segregate the task/workspace seam into role interfaces - #4012
🤖 refactor: segregate the task/workspace seam into role interfaces#4012ibetitsmike wants to merge 6 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7827948180
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment has been minimized.
This comment has been minimized.
1 participant
Summary
Splits the 36-method
WorkspaceHostgrab-bag on the task/workspace seam into five role interfaces named for what task-side callers do (WorkspaceTurnHost,TurnAdmissionHost,WorkspaceLifecycleHost,WorkspaceProvisioningHost,WorkspaceMetadataHost), keepsWorkspaceHostas their intersection so no call site or wiring changes, and collapses the ~180-line hand-rolled test mock onto one sharedmakeWorkspaceHostFake.Background
#3996 cut the taskService/workspaceService dependency cycle at a typed seam, but the seam stayed shallow: one interface mirroring 36 of WorkspaceService's internal mechanics. Every task test stubbed all 36 methods through a ~180-line mock in
taskService.test.ts(reached from 318 call sites), and the seam, service, and test harness churned in lockstep on every change. This builds on #3996 rather than reverting it: same seam, deeper interface. Refactor #5 from the 2026-08-29 architecture review (evidence at main @ f04e0f8).Implementation
taskWorkspaceSeam.ts: the five role interfaces group methods by caller intent (turn execution, queue/admission probes, archive/remove lifecycle, child-workspace provisioning, metadata/events). Every method signature is byte-identical to before;WorkspaceHostis nowWorkspaceTurnHost & TurnAdmissionHost & WorkspaceLifecycleHost & WorkspaceProvisioningHost & WorkspaceMetadataHost.TaskService(the only production consumer) legitimately uses all five roles, so its single constructor param stays; new narrow consumers can now depend on one role instead of the full host.taskWorkspaceSeam.testUtils.ts: adds framework-freemakeWorkspaceHostFake(overrides)beside the existingmakeAgentTaskIntegrationFake, carrying the harness's default stub semantics (granted archive hold, "keep"-style snapshot eligibility, sanitizer no-op).taskService.test.ts:createWorkspaceServiceMocksshrinks from ~180 lines to ~55 on top of the shared fake, with a mapped type overkeyof WorkspaceHostreplacing the hand-written 36-entry overrides list. Returned mock handles and the archive/remove locked-sink aliasing are preserved, so all 318 harness call sites are untouched.Net LOC delta vs main (f04e0f8)
taskWorkspaceSeam.ts): +18 (+103/-85)taskService.test.ts+taskWorkspaceSeam.testUtils.ts): -76 (+112/-188)Irreducible production additions: the five role interface declarations plus the intersection type (the point of the refactor), and the archive race-invariant docs on
ArchiveWorkspaceOptions, which review feedback correctly required keeping verbatim rather than counting as savings. Test additions aremakeWorkspaceHostFake's default bodies (moved from the harness, now reusable by any seam consumer's tests).Validation
interrupt_activeand live-activity refusal, heartbeats, tree listing, and monitor wakes. The UAT runner additionally verified the emitted JavaScript is byte-identical between base and feature for the production file.bun testoftaskService.test.ts,workspaceService.test.ts,heartbeatService.test.ts,tools/task_list.test.ts: the fail set is identical to a clean worktree at base f04e0f8 (3 pre-existing host-environment failures; none branch-attributable).Risks
Low. The production change is type-only interface restructuring with byte-identical emitted JS; regression surface is the test-harness consolidation, which preserves each mock's default behavior and aliasing semantics.
Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh• Cost:$29.31