From f5f5190dd07f1df5625f29eb8656bc0377653eec Mon Sep 17 00:00:00 2001 From: James Long Date: Wed, 11 Nov 2026 13:52:20 -0600 Subject: [PATCH] refactor(core): remove the unused foreground path from SubagentJob (#63223) --- packages/core/src/config/plugin/command.ts | 2 +- packages/core/src/session/execution.ts | 7 -- .../src/session/execution/subagent-report.ts | 4 +- packages/core/src/session/subagent-job.ts | 70 +++++-------------- packages/core/src/tool/plugin/subagent.ts | 2 +- packages/core/test/schedule.test.ts | 1 - packages/core/test/session-owned.test.ts | 2 - packages/core/test/session-prompt.test.ts | 1 - .../core/test/session-runner-recorded.test.ts | 1 - packages/core/test/session-runner.test.ts | 1 - packages/core/test/session-shell.test.ts | 1 - packages/core/test/tool-shell.test.ts | 1 - packages/core/test/tool-subagent.test.ts | 9 +-- packages/server/test/session-diff.test.ts | 1 - 14 files changed, 22 insertions(+), 81 deletions(-) diff --git a/packages/core/src/config/plugin/command.ts b/packages/core/src/config/plugin/command.ts index 2eaa0115b86..ab2a59f782d 100644 --- a/packages/core/src/config/plugin/command.ts +++ b/packages/core/src/config/plugin/command.ts @@ -93,7 +93,7 @@ export const Plugin = define({ agent: selected.id, description: command.description ?? name, } - yield* subagents.start(recovery, { background: true }) + yield* subagents.start(recovery) return } if (agent !== undefined) { diff --git a/packages/core/src/session/execution.ts b/packages/core/src/session/execution.ts index b73168b2017..f59066ab668 100644 --- a/packages/core/src/session/execution.ts +++ b/packages/core/src/session/execution.ts @@ -24,11 +24,6 @@ export interface Interface { readonly resume: (sessionID: SessionSchema.ID) => Effect.Effect /** Registers newly recorded work. Repeated wakeups may coalesce. */ readonly wake: (sessionID: SessionSchema.ID) => Effect.Effect - /** - * The durable sequence where the Session's current busy period started, if one is bound. - * Sequence-bounded lookups use it to find only work created by this period. - */ - readonly periodStartSeq: (sessionID: SessionSchema.ID) => Effect.Effect /** * Interrupt active work owned by this process. Idle interruption is a no-op. Resolves once * the interruption is accepted; cleanup settles asynchronously in the execution fiber. @@ -182,7 +177,6 @@ export const layer = Layer.effect( return Service.of({ active: coordinator.active, isActive: coordinator.isActive, - periodStartSeq: subagentReport.periodStartSeq, interrupt: (sessionID, options) => Effect.gen(function* () { const interrupted = yield* coordinator.interrupt(sessionID, "user") @@ -236,7 +230,6 @@ export const noopLayer = Layer.succeed( isActive: () => Effect.succeed(false), resume: () => Effect.void, wake: () => Effect.void, - periodStartSeq: () => Effect.succeed(undefined), interrupt: () => Effect.succeed(false), awaitIdle: () => Effect.void, }), diff --git a/packages/core/src/session/execution/subagent-report.ts b/packages/core/src/session/execution/subagent-report.ts index 4a2e1f6e91a..2bbb96bd46b 100644 --- a/packages/core/src/session/execution/subagent-report.ts +++ b/packages/core/src/session/execution/subagent-report.ts @@ -53,8 +53,6 @@ export const make = (deps: Deps) => { periods.delete(sessionID) } - const periodStartSeq = (sessionID: SessionSchema.ID) => Effect.succeed(periods.get(sessionID)?.startedSeq) - const resolveBody = Effect.fnUntraced(function* ( sessionID: SessionSchema.ID, outcome: Exclude, @@ -207,5 +205,5 @@ export const make = (deps: Deps) => { return { commit, deliver } }) - return { started, clear, periodStartSeq, prepare } + return { started, clear, prepare } } diff --git a/packages/core/src/session/subagent-job.ts b/packages/core/src/session/subagent-job.ts index bcea37ca8b9..43bbf4e5cbf 100644 --- a/packages/core/src/session/subagent-job.ts +++ b/packages/core/src/session/subagent-job.ts @@ -3,45 +3,26 @@ export * as SubagentJob from "./subagent-job.js" import { Context, Effect, Layer } from "effect" import { Node } from "@opencode/util/effect/app-node" import type { LayerNode } from "@opencode/util/effect/layer-node" -import { Bus } from "../bus.js" -import { Database } from "../database/database.js" import { Job } from "../job.js" -import { SessionBackground } from "./background.js" import { SessionExecution } from "./execution.js" -import type { SessionSchema } from "./schema.js" -import { SessionStore } from "./store.js" -import { SubagentCompletion } from "./subagent-completion.js" type Recovery = Extract -export type StartOptions = { - readonly background?: boolean -} - export interface Interface { - readonly outstanding: (sessionID: SessionSchema.ID) => Effect.Effect - readonly start: (recovery: Recovery, options?: StartOptions) => Effect.Effect + readonly start: (recovery: Recovery) => Effect.Effect } export class Service extends Context.Service()("@opencode/SubagentJob") {} const make = Effect.gen(function* () { const execution = yield* SessionExecution.Service - const store = yield* SessionStore.Service const jobs = yield* Job.Service - const { db } = yield* Database.Service // Jobs outlive the plugin that launched them. Fork with the global context too, rather // than retaining the caller's location. const context = yield* Effect.context() - const outstanding = (sessionID: SessionSchema.ID) => - Effect.gen(function* () { - return yield* SessionBackground.outstanding(jobs, store, sessionID, yield* execution.active) - }) - return Service.of({ - outstanding, - start: (recovery, options) => + start: (recovery) => jobs .start({ id: recovery.childSessionID, @@ -49,38 +30,21 @@ const make = Effect.gen(function* () { title: recovery.description, metadata: {}, recovery, - background: options?.background, - run: Effect.gen(function* () { - const resume = execution.resume(recovery.childSessionID).pipe( - // Only an explicit caller cancel releases the child's claim. A server shutdown - // interrupts this job but must leave the child resumable on the next start. - Effect.onInterrupt(() => - jobs - .isCancelled(recovery.childSessionID) - .pipe( - Effect.flatMap((cancelled) => - cancelled ? execution.interrupt(recovery.childSessionID) : Effect.void, - ), + background: true, + run: execution.resume(recovery.childSessionID).pipe( + // Only an explicit caller cancel releases the child's claim. A server shutdown + // interrupts this job but must leave the child resumable on the next start. + Effect.onInterrupt(() => + jobs + .isCancelled(recovery.childSessionID) + .pipe( + Effect.flatMap((cancelled) => + cancelled ? execution.interrupt(recovery.childSessionID) : Effect.void, ), - ), - ) - if (options?.background) return yield* resume.pipe(Effect.as("")) - // A joining period already owns its boundary; otherwise the child was admitted but not - // resumed, so the latest durable sequence precedes everything this period will write. - const startedSeq = - (yield* execution.periodStartSeq(recovery.childSessionID)) ?? - (yield* Bus.latestSequence(db, recovery.childSessionID)) - yield* resume - const assistant = yield* store.lastAssistantAfter({ - sessionID: recovery.childSessionID, - afterSeq: startedSeq, - }) - return SubagentCompletion.formatBody( - SubagentCompletion.text(assistant), - recovery.childSessionID, - yield* outstanding(recovery.childSessionID), - ) - }), + ), + ), + Effect.as(""), + ), }) .pipe(Effect.setContext(context)), }) @@ -91,5 +55,5 @@ export const layer = Layer.effect(Service, make) export const node: LayerNode.Provider = Node.makeGlobalNode({ service: Service, layer, - deps: [SessionExecution.node, SessionStore.node, Database.node, Job.node], + deps: [SessionExecution.node, Job.node], }) diff --git a/packages/core/src/tool/plugin/subagent.ts b/packages/core/src/tool/plugin/subagent.ts index acff3a36475..39f6e391b62 100644 --- a/packages/core/src/tool/plugin/subagent.ts +++ b/packages/core/src/tool/plugin/subagent.ts @@ -234,7 +234,7 @@ export const Plugin = { agent: agent.name, description: input.description, } - yield* subagents.start(recovery, { background: true }) + yield* subagents.start(recovery) return backgroundResult(child.id, notify) }).pipe( Effect.map((output) => ({ diff --git a/packages/core/test/schedule.test.ts b/packages/core/test/schedule.test.ts index bde767359a0..abd5f698ef4 100644 --- a/packages/core/test/schedule.test.ts +++ b/packages/core/test/schedule.test.ts @@ -63,7 +63,6 @@ const execution = Layer.succeed( isActive: () => Effect.succeed(false), resume: () => Effect.void, interrupt: () => Effect.succeed(false), - periodStartSeq: () => Effect.succeed(undefined), wake: (sessionID) => Effect.suspend(() => { wakes.push(sessionID) diff --git a/packages/core/test/session-owned.test.ts b/packages/core/test/session-owned.test.ts index b9ce1832075..0c3c996286d 100644 --- a/packages/core/test/session-owned.test.ts +++ b/packages/core/test/session-owned.test.ts @@ -105,7 +105,6 @@ const setup = Effect.fnUntraced(function* (options?: { }), awaitIdle: () => Effect.void, interrupt: () => Effect.succeed(false), - periodStartSeq: () => Effect.succeed(undefined), wake: (id) => Effect.gen(function* () { const pending = yield* SessionInbox.list(database.db, id) @@ -638,7 +637,6 @@ describe("Session-owned handles", () => { }), wake: coordinator.wake, awaitIdle: coordinator.awaitIdle, - periodStartSeq: () => Effect.succeed(undefined), interrupt: (id, options) => Effect.sync(() => void interrupts.push({ sessionID: id, options })).pipe( Effect.andThen(coordinator.interrupt(id)), diff --git a/packages/core/test/session-prompt.test.ts b/packages/core/test/session-prompt.test.ts index cd5848ac914..6c50de01f37 100644 --- a/packages/core/test/session-prompt.test.ts +++ b/packages/core/test/session-prompt.test.ts @@ -75,7 +75,6 @@ const execution = Layer.succeed( wakeCalls.push(sessionID) }), awaitIdle: () => Effect.void, - periodStartSeq: () => Effect.succeed(undefined), }), ) const mediaDirectory = "/media" diff --git a/packages/core/test/session-runner-recorded.test.ts b/packages/core/test/session-runner-recorded.test.ts index 691e7a96620..2c2d3894239 100644 --- a/packages/core/test/session-runner-recorded.test.ts +++ b/packages/core/test/session-runner-recorded.test.ts @@ -125,7 +125,6 @@ const execution = (llmClient: Layer.Layer) => wake: coordinator.wake, interrupt: (sessionID) => coordinator.interrupt(sessionID), awaitIdle: coordinator.awaitIdle, - periodStartSeq: () => Effect.succeed(undefined), }) }), ).pipe(Layer.provide(runnerLayer(llmClient)), Layer.orDie) diff --git a/packages/core/test/session-runner.test.ts b/packages/core/test/session-runner.test.ts index 1e433d06727..824986ac808 100644 --- a/packages/core/test/session-runner.test.ts +++ b/packages/core/test/session-runner.test.ts @@ -469,7 +469,6 @@ const layer = Layer.unwrap( wake: coordinator.wake, interrupt: (sessionID) => coordinator.interrupt(sessionID), awaitIdle: coordinator.awaitIdle, - periodStartSeq: () => Effect.succeed(undefined), }) }), ).pipe( diff --git a/packages/core/test/session-shell.test.ts b/packages/core/test/session-shell.test.ts index 007ac749240..8f1c92cff26 100644 --- a/packages/core/test/session-shell.test.ts +++ b/packages/core/test/session-shell.test.ts @@ -50,7 +50,6 @@ const executionLayer = Layer.effect( resume: coordinator.run, interrupt: (sessionID) => coordinator.interrupt(sessionID), awaitIdle: coordinator.awaitIdle, - periodStartSeq: () => Effect.succeed(undefined), wake: (sessionID) => Effect.sync(() => control.wakes.push(sessionID)).pipe(Effect.andThen(coordinator.wake(sessionID))), }) diff --git a/packages/core/test/tool-shell.test.ts b/packages/core/test/tool-shell.test.ts index 9e96e55e448..623823b7344 100644 --- a/packages/core/test/tool-shell.test.ts +++ b/packages/core/test/tool-shell.test.ts @@ -127,7 +127,6 @@ const executionNode = makeGlobalNode({ resume: complete, wake: () => Effect.void, interrupt: () => Effect.succeed(false), - periodStartSeq: () => Effect.succeed(undefined), awaitIdle: (id) => complete(id).pipe(Effect.exit, Effect.asVoid), }) }), diff --git a/packages/core/test/tool-subagent.test.ts b/packages/core/test/tool-subagent.test.ts index 503a2ea0f78..9eaa847f5f3 100644 --- a/packages/core/test/tool-subagent.test.ts +++ b/packages/core/test/tool-subagent.test.ts @@ -109,7 +109,6 @@ const executionNode = makeGlobalNode({ resume: complete, wake: () => Effect.void, interrupt: () => Effect.succeed(false), - periodStartSeq: () => Effect.succeed(undefined), awaitIdle: (sessionID) => complete(sessionID).pipe(Effect.exit, Effect.asVoid), }) }), @@ -1077,7 +1076,6 @@ describe("SubagentJob cancellation", () => { it.live("interrupts the child only for an explicit cancel and preserves shutdown claims", () => Effect.gen(function* () { const database = yield* Database.Service - const store = yield* SessionStore.Service const scope = yield* Scope.make() const cancelStarted = yield* Deferred.make() const shutdownStarted = yield* Deferred.make() @@ -1100,7 +1098,6 @@ describe("SubagentJob cancellation", () => { Effect.onInterrupt(() => Effect.sync(() => interrupted.add(sessionID))), ), wake: () => Effect.void, - periodStartSeq: () => Effect.succeed(0), interrupt: (sessionID) => Effect.sync(() => { interrupts.push(sessionID) @@ -1113,9 +1110,7 @@ describe("SubagentJob cancellation", () => { const context = yield* Layer.buildWithScope( SubagentJob.layer.pipe( Layer.provide(Layer.succeed(SessionExecution.Service, execution)), - Layer.provide(Layer.succeed(SessionStore.Service, store)), Layer.provide(Layer.succeed(Job.Service, jobs)), - Layer.provide(Layer.succeed(Database.Service, database)), ), scope, ) @@ -1128,7 +1123,7 @@ describe("SubagentJob cancellation", () => { description: "review", }) - yield* subagents.start(recovery("ses_job_cancel_parent", cancelChild), { background: true }) + yield* subagents.start(recovery("ses_job_cancel_parent", cancelChild)) yield* Deferred.await(cancelStarted) yield* jobs.cancel(cancelChild) expect(interrupted.has(cancelChild)).toBe(true) @@ -1136,7 +1131,7 @@ describe("SubagentJob cancellation", () => { expect(claimed.has(cancelChild)).toBe(false) // A server shutdown interrupts the job fiber but must not release the child's claim. - yield* subagents.start(recovery("ses_job_shutdown_parent", shutdownChild), { background: true }) + yield* subagents.start(recovery("ses_job_shutdown_parent", shutdownChild)) yield* Deferred.await(shutdownStarted) yield* Scope.close(scope, Exit.void) expect(interrupted.has(shutdownChild)).toBe(true) diff --git a/packages/server/test/session-diff.test.ts b/packages/server/test/session-diff.test.ts index 07c24b69e7d..bafcfa07b26 100644 --- a/packages/server/test/session-diff.test.ts +++ b/packages/server/test/session-diff.test.ts @@ -49,7 +49,6 @@ it.live("serves turn diffs by user message with range validation", () => }), interrupt: () => Effect.succeed(false), awaitIdle: () => Effect.void, - periodStartSeq: () => Effect.succeed(undefined), }) }), )