Skip to content

Commit 8fe907f

Browse files
authored
🤖 refactor: split createUnderTaskTreeLifecycleLock, part 3: sanitize, announce and send (#7704)
## Summary Part 3 of 5: splits sanitize, announce and the launch send out of `TaskService.createUnderTaskTreeLifecycleLock` in place, with no behaviour change. The method drops from CC 88 to CC 78. On top of part 2. 94 changed lines. The moved code keeps its indentation, so git sees few changes. ## Steps in this part - `sanitizeTaskLaunchCheckout` still runs before the metadata emit. - `announceAndInitTask` emits the metadata, reads the secrets, then calls `registerExternalBackgroundInit`. - `sendTaskLaunchPrompt` makes the send admission with no await before it, in the same continuation as `registerExternalBackgroundInit`. ## Rules every part keeps - The method keeps `await using _lock = await this.mutex.acquire()`. Inside that scope, a bare `return promise` releases the global mutex before the promise settles. So every call from the method to a step is `await` or `return await`, and no step acquires the mutex, which is not reentrant. - The steps form a chain: each step calls the next one directly after its own last await. So no collaborator call runs a microtask later than in the inline code. Only the result of the method resolves later, at approximately 2 microtasks per chain level. The mutex therefore releases that much later, at the same logical point. - `make crap-check` against the parent commit reports 0 violations, with and without coverage. Every new function scores CC 8 or less. - `bun test` on the taskService, ledger, workspaceTurnManager, workflows, tools and workspaceService suites: 4,968 pass, 0 fail. These suites include the 27 tests from #7701. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$182.90`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=182.90 -->
1 parent 7d2256c commit 8fe907f

1 file changed

Lines changed: 69 additions & 25 deletions

File tree

‎src/node/services/taskService.ts‎

Lines changed: 69 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,15 @@ interface TaskCreatePlan extends TaskCreatePlacement {
899899
readonly createdAt: string;
900900
}
901901

902+
/** A direct launch's materialized checkout, read by the steps after its entry is persisted. */
903+
interface TaskLaunchCheckout {
904+
readonly initLogger: InitLogger;
905+
readonly workspacePath: string;
906+
readonly trunkBranch: string;
907+
readonly forkedRuntimeConfig: RuntimeConfig;
908+
readonly runtimeForTaskWorkspace: Runtime;
909+
}
910+
902911
/**
903912
* A direct Task.create launch's progress. The launch writes it as it advances and its failure
904913
* path (failTaskLaunch) reads it, so it is one mutable record: a copy would read stale values.
@@ -7529,10 +7538,20 @@ export class TaskService implements AgentTaskIntegration {
75297538
}
75307539
return materialized;
75317540
}
7532-
const { initLogger, workspacePath, trunkBranch, forkedRuntimeConfig, runtimeForTaskWorkspace } =
7533-
materialized.data;
7541+
return await this.sanitizeTaskLaunchCheckout(plan, launch, materialized.data);
7542+
}
75347543

7535-
if (!useSharedWorkspace) {
7544+
/**
7545+
* The direct launch after its entry is persisted. Sanitizes the materialized checkout, then
7546+
* chains to announceAndInitTask and sendTaskLaunchPrompt: each step starts the next in the same
7547+
* tick as its own last await, so no call runs later than it did inline.
7548+
*/
7549+
private async sanitizeTaskLaunchCheckout(
7550+
plan: TaskCreatePlan,
7551+
launch: TaskLaunchState,
7552+
checkout: TaskLaunchCheckout
7553+
): Promise<Result<TaskCreateResult, string>> {
7554+
if (!plan.useSharedWorkspace) {
75367555
// SECURITY: this checkout materialized outside the host's create/fork paths, so
75377556
// registration-time plugin-override sanitization never saw it —
75387557
// a tracked stale `plugin:` enable would re-activate a same-name
@@ -7542,28 +7561,43 @@ export class TaskService implements AgentTaskIntegration {
75427561
// subscriber can send to this running-status task workspace while
75437562
// sanitization is still waiting on the override lock.
75447563
const sanitizeError = await this.workspaceService.sanitizeMaterializedTaskWorkspace(
7545-
taskId,
7546-
workspacePath,
7547-
forkedRuntimeConfig
7564+
plan.taskId,
7565+
checkout.workspacePath,
7566+
checkout.forkedRuntimeConfig
75487567
);
75497568
if (sanitizeError !== undefined) {
75507569
// The row stays published with this checkout: MCP and sends refuse it (#4674).
7551-
this.initStateManager.markCheckoutUnsanitized(taskId);
7552-
await this.failTaskLaunch(plan, launch, sanitizeError, runtimeForTaskWorkspace, {});
7553-
initLogger.logComplete(-1);
7570+
this.initStateManager.markCheckoutUnsanitized(plan.taskId);
7571+
await this.failTaskLaunch(
7572+
plan,
7573+
launch,
7574+
sanitizeError,
7575+
checkout.runtimeForTaskWorkspace,
7576+
{}
7577+
);
7578+
checkout.initLogger.logComplete(-1);
75547579
return Err(sanitizeError);
75557580
}
75567581
}
7582+
return this.announceAndInitTask(plan, launch, checkout);
7583+
}
75577584

7585+
/** Announces the launched task's workspace and kicks its init, then sends the prompt. */
7586+
private async announceAndInitTask(
7587+
plan: TaskCreatePlan,
7588+
launch: TaskLaunchState,
7589+
checkout: TaskLaunchCheckout
7590+
): Promise<Result<TaskCreateResult, string>> {
7591+
const { taskId } = plan;
75587592
// Emit metadata update so the UI sees the workspace immediately.
75597593
await this.emitWorkspaceMetadata(taskId);
75607594

75617595
// Kick init (best-effort, async). Shared-workspace (isolation: "none") tasks reuse the parent's
75627596
// already-initialized checkout, so re-running init would redundantly (and possibly disruptively)
75637597
// mutate the live parent workspace — skip it entirely.
7564-
if (!useSharedWorkspace) {
7598+
if (!plan.useSharedWorkspace) {
75657599
const secrets = await secretsToRecord(
7566-
this.secretsStore.getEffectiveSecrets(parentMeta.projectPath)
7600+
this.secretsStore.getEffectiveSecrets(plan.parentMeta.projectPath)
75677601
);
75687602
// Registered (not just fired) with the host's abort-and-settlement mechanism:
75697603
// a model-driven archive of this task workspace must be able to cancel the init and
@@ -7574,25 +7608,35 @@ export class TaskService implements AgentTaskIntegration {
75747608
taskId,
75757609
initAbortController,
75767610
runBackgroundInit(
7577-
runtimeForTaskWorkspace,
7611+
checkout.runtimeForTaskWorkspace,
75787612
{
7579-
projectPath: parentMeta.projectPath,
7580-
branchName: workspaceName,
7581-
trunkBranch,
7582-
workspacePath,
7583-
initLogger,
7613+
projectPath: plan.parentMeta.projectPath,
7614+
branchName: plan.workspaceName,
7615+
trunkBranch: checkout.trunkBranch,
7616+
workspacePath: checkout.workspacePath,
7617+
initLogger: checkout.initLogger,
75847618
env: secrets,
75857619
abortSignal: initAbortController.signal,
7586-
skipInitHook,
7620+
skipInitHook: plan.skipInitHook,
75877621
trusted:
7588-
this.config.loadConfigOrDefault().projects.get(configProjectPath)?.trusted ?? false,
7622+
this.config.loadConfigOrDefault().projects.get(plan.configProjectPath)?.trusted ??
7623+
false,
75897624
},
75907625
taskId,
75917626
workspaceUseLeasesFor(this.config)
75927627
)
75937628
);
75947629
}
7630+
return this.sendTaskLaunchPrompt(plan, launch, checkout);
7631+
}
75957632

7633+
/** Admits the launch's prompt under the attempt it persisted and sends it. */
7634+
private async sendTaskLaunchPrompt(
7635+
plan: TaskCreatePlan,
7636+
launch: TaskLaunchState,
7637+
checkout: TaskLaunchCheckout
7638+
): Promise<Result<TaskCreateResult, string>> {
7639+
const { taskId, taskModelString, effectiveThinkingLevel } = plan;
75967640
// Start immediately (counts towards parallel limit). Launch fence, as in startReservedAgentTask:
75977641
// the obligation is bound under the owned attempt before dispatch and the token doubles as the
75987642
// send's staleness probe. A guarded send is never a user resume, so WorkspaceService skips
@@ -7617,12 +7661,12 @@ export class TaskService implements AgentTaskIntegration {
76177661
: await this.workspaceService
76187662
.sendMessage(
76197663
taskId,
7620-
prompt,
7664+
plan.prompt,
76217665
{
76227666
model: taskModelString,
7623-
agentId,
7667+
agentId: plan.agentId,
76247668
thinkingLevel: effectiveThinkingLevel,
7625-
reasoningMode: effectiveReasoningMode,
7669+
reasoningMode: plan.effectiveReasoningMode,
76267670
},
76277671
{
76287672
acceptanceOrigin: "automatic",
@@ -7637,8 +7681,8 @@ export class TaskService implements AgentTaskIntegration {
76377681
typeof sendResult.error === "string"
76387682
? sendResult.error
76397683
: formatSendMessageError(sendResult.error).message;
7640-
await this.failTaskLaunch(plan, launch, message, runtimeForTaskWorkspace, {
7641-
preservePhysicalWorkspace: useSharedWorkspace,
7684+
await this.failTaskLaunch(plan, launch, message, checkout.runtimeForTaskWorkspace, {
7685+
preservePhysicalWorkspace: plan.useSharedWorkspace,
76427686
});
76437687
return Err(message);
76447688
}
@@ -7649,7 +7693,7 @@ export class TaskService implements AgentTaskIntegration {
76497693
status: "running",
76507694
modelString: taskModelString,
76517695
thinkingLevel: effectiveThinkingLevel,
7652-
desktopOwnerWorkspaceId: taskDesktopOwnerWorkspaceId ?? taskId,
7696+
desktopOwnerWorkspaceId: plan.taskDesktopOwnerWorkspaceId ?? taskId,
76537697
});
76547698
}
76557699

0 commit comments

Comments
 (0)