🤖 refactor: cut the taskService/workspaceService dependency cycle at a typed seam - #3996
Merged
Conversation
Contributor
Author
|
@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.
ibetitsmike
force-pushed
the
mike/task-workspace-seam
branch
from
August 28, 2026 08:27
a6bf1b2 to
3f94aa7
Compare
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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.
Contributor
Author
|
@codex review Branch reworked per reviewer directive: the seam now pays for itself. Net diff vs main is +676/−1010 (−334). New commits dedupe seam types into taskWorkspaceSeam.ts, encode port invariants in types instead of optional chaining, and delete seam-obsoleted test scaffolding (phantom-method guard, wiring-only tests, private-spy tests, copy-pasted mock blocks, dead stubs). |
|
Codex Review: Didn't find any major issues. Swish! 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.
_Generated with `xum` • Model: `openai:gpt-5.6-sol` • Thinking: `high`_ <!-- mux-attribution: model=openai:gpt-5.6-sol thinking=high -->
Review follow-up: the four per-command handlers are only reachable through processSlashCommand now, and /plan open lost its dedicated tests in the result-based rewrite. _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
Codex P1: getDraft captured at command invocation reports that render's input, so async commands cleared newer drafts on consume and never fired restore-if-empty. _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
Codex P2: text equality cannot distinguish a retyped identical draft from the original invocation. Commands already clear through their own clear-input actions (matching trunk), so the terminal clear was additive and could only destroy mid-phase drafts. _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
Codex P2 follow-up: with the terminal consume-path clear gone, /dream and /refine left the executed command re-runnable in the composer. Emit clear-input from the handlers so commands own their composer effects. _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
…itative in background wrappers
…te; drop dead code
The seam file is now the single documented home for ArchiveWorkspaceOptions, SendMessageInternalOptions, and WorkspaceLiveActivity instead of duplicating workspaceService declarations. StreamErrorRecoveryOutcome comes from its canonical agentSession export. The AgentTaskStatus re-export shim is gone; importers use the seam. Method-level optional chaining on both typed ports is removed: the interfaces guarantee the methods, so the narrow-mock hedges and their comments no longer apply.
… suite The typed AgentTaskIntegration port makes several tests unwritable or redundant: the phantom cleanupReportedDescendantsAfterArchive guard (method never existed in production), the archive lock pass-through wiring assertion, and four private updateAgentStatus non-invocation spies whose positive registerSession behavior tests remain. Near-identical send/resume lifecycle, winding-down, auto-resume, and foreground-wait-backgrounding siblings collapse into table-driven tests preserving every case, and dead fake stubs the code under test never reads are dropped.
…d host port createWorkspaceServiceMocks loses its dead stubs (waitForIdle, deleteWorktree, updateAgentStatus never existed on WorkspaceHost), its hand-written 37-line return annotation, and return entries nothing consumes; the removeQueuedMessagesByDedupeKeyPrefix override is wired instead of the one call site mutating the built host. 23 copy-pasted createWorkspace mock blocks collapse into two shared helpers, and the redundant conditional-spread forwarding in both harnesses becomes direct pass-through since the factory already defaults absent overrides.
Drops fake overrides that restate makeAgentTaskIntegrationFake defaults, types makeWorkspaceTurnCreateMock's rest args via Parameters<WorkspaceHost['create']> instead of an unknown[] cast, and calls withTaskTreeLifecycleLock directly rather than through a bound closure.
ibetitsmike
force-pushed
the
mike/task-workspace-seam
branch
from
August 28, 2026 19:22
122fb5b to
e05dc05
Compare
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was removed
Aug 29, 2026
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was removed
Aug 29, 2026
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was removed
Aug 29, 2026
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was removed
Aug 29, 2026
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was unmerged and was not in the queue
Aug 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cuts the TaskService/WorkspaceService concrete-class dependency cycle with one-directional typed ports, then pays for the new contract by deleting the code and test scaffolding the ports make obsolete. The full branch is a net simplification: +676/−1010 (net −334) across 11 files.
Background
TaskService and WorkspaceService imported each other's concrete classes, so neither side had a stated contract: every public member of each class was implicitly reachable from the other, and tests smuggled partial fakes through
as unknown ascasts. An architecture change must shrink the codebase, so the seam had to delete more than it declares.Implementation
taskWorkspaceSeam.tsis the single home for the two ports and every type they share:WorkspaceHost(~38 methods): what TaskService may call; WorkspaceServiceimplementsit.AgentTaskIntegration(12 methods): what WorkspaceService may call; TaskServiceimplementsit, wired viasetAgentTaskIntegration(the oldsetTaskServiceis deleted).ArchiveWorkspaceOptions,SendMessageInternalOptions(formerly a 56-line inline literal),WorkspaceLiveActivity,AgentTaskStatus, and the archive untracked-path helpers moved here instead of being duplicated;StreamErrorRecoveryOutcomeis imported from its canonicalagentSessionexport. TheAgentTaskStatusre-export shim in taskService is gone; importers use the seam.integration?.method?.()) and the "test harnesses mock a narrow surface" hedges are deleted because the interfaces now guarantee the methods exist.Test shrink (where the net-negative lives)
as unknown as TaskService/WorkspaceServicecasts:makeAgentTaskIntegrationFake()(shared testUtils) and asatisfies WorkspaceHostcentral factory.cleanupReportedDescendantsAfterArchiveguard (the method never existed in production), an archive lock pass-through assertion, and four privateupdateAgentStatusnon-invocation spies whose positive behavior tests remain.test.eachtables (send/resume interrupted-task lifecycle, winding-down gate, auto-resume reset, foreground-wait backgrounding); every case is preserved.createWorkspaceServiceMockslost its dead stubs (waitForIdle,deleteWorktree,updateAgentStatusnever existed on the port), its hand-written 37-line return annotation, and unconsumed return entries; 23 copy-pastedcreateWorkspacemock blocks collapsed into two shared helpers typed viaParameters<WorkspaceHost["create"]>; redundant conditional-spread forwarding in two harnesses became direct pass-through.Validation
make static-checkgreen; typecheck is the enforcement mechanism for the seam (it caught two false "unused symbol" audit claims during development).Risks
The production diff is receiver-renames, type moves, and deleted dead plumbing; runtime control flow is unchanged (one
.bindindirection replaced by a direct guarded call). Regression risk concentrates in the test consolidation: atest.eachtranslation error could weaken a race guard. Mitigated by preserving each original case row and assertion set, and by the suites passing with an unchanged test count on the task side.Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh