From 844c8dbdfe54639f3e91fe349518db2fc671239d Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 11 Jun 2026 16:06:30 +0200 Subject: [PATCH] fix(coding-agent): restored task sync mode when async.enabled was false - Updated TaskTool to skip `session.asyncJobManager` and run `task` spawns inline whenever `async.enabled` is false. - Set `async.enabled` default to `true` and updated task prompts/settings text to reflect async-versus-sync behavior. - Adjusted task batching tests to cover both async background execution and synchronous batched execution when async is disabled. --- docs/tools/job.md | 2 +- docs/tools/task.md | 24 ++++---- packages/coding-agent/CHANGELOG.md | 9 ++- .../src/config/settings-schema.ts | 6 +- .../coding-agent/src/prompts/tools/task.md | 9 ++- packages/coding-agent/src/task/index.ts | 25 ++++---- .../coding-agent/test/task/task-batch.test.ts | 59 ++++++++++++++++--- .../test/tools/task-async-fallback.test.ts | 4 +- 8 files changed, 98 insertions(+), 40 deletions(-) diff --git a/docs/tools/job.md b/docs/tools/job.md index e42d68e2c..de3ad1389 100644 --- a/docs/tools/job.md +++ b/docs/tools/job.md @@ -44,7 +44,7 @@ Read-only snapshot path: - Calling `job` with `list: true` returns a markdown summary of every job spawned by the calling agent (running + completed within retention) without waiting. ## Flow -1. `JobTool` is registered unconditionally in `packages/coding-agent/src/tools/index.ts`; there is no `async.enabled` gate (the `task` tool always schedules background jobs). +1. `JobTool` is registered unconditionally in `packages/coding-agent/src/tools/index.ts`; there is no `async.enabled` gate (the manager may still carry bash or task jobs from before a setting change). 2. `execute(...)` fetches `session.asyncJobManager`. If absent, it returns `Async execution is disabled; no background jobs are available.` 3. `cancel` ids are processed first: - `manager.getJob(id)` missing → `not_found`. diff --git a/docs/tools/task.md b/docs/tools/task.md index cbcedd3cd..83fc51020 100644 --- a/docs/tools/task.md +++ b/docs/tools/task.md @@ -1,6 +1,6 @@ # task -> Spawn subagents to work in the background — one per call, or a `tasks[]` batch per call (`task.batch`, default on). +> Spawn subagents — one per call, or a `tasks[]` batch per call (`task.batch`, default on). With `async.enabled=true`, spawns run in the background; otherwise the call blocks until they finish. ## Source - Entry: `packages/coding-agent/src/task/index.ts` @@ -28,7 +28,7 @@ The wire schema is shape-swapped by `task.batch` (default on). One unit of work is the task item `{ id?, description?, assignment, isolated? }` (`isolated` only when `task.isolation.mode` is not `none`): -- **Batch shape** (`task.batch` on): `{ agent, context, tasks: item[] }` — one subagent per item, all spawned in parallel as independent background jobs. `context` is **required** shared background rendered into every spawned subagent's system prompt (`CONTEXT` section); `isolated` is per item. +- **Batch shape** (`task.batch` on): `{ agent, context, tasks: item[] }` — one subagent per item, all run under the same fan-out rules. `context` is **required** shared background rendered into every spawned subagent's system prompt (`CONTEXT` section); `isolated` is per item. - **Flat shape** (`task.batch` off): `{ agent, ...item }` — exactly one spawn per call. Shared background goes into a `local://` file (e.g. `local://ctx.md`) that each assignment references; subagents share the parent's `local://` root. | Field | Type | Required | Description | @@ -49,12 +49,12 @@ There is no per-call `schema` parameter. Structured output comes from the agent The tool returns one text block plus `details: TaskToolDetails`. -Immediate (async) response — the normal case: +Background response (`async.enabled=true`): - `content`: `` Spawned agent `` (job ``). The result will be delivered when it yields. ... `` plus a coordination hint (`irc` DM when enabled, otherwise `job`). A batch call instead returns `` Spawned N background agents using . ... `` with a per-agent `- `` (job ``)` listing. - `details`: `{ projectAgentsDir: null, results: [], totalDurationMs: 0, progress: [], async: { state: "running", jobId, type: "task" } }`. A batch call keeps one shared `progress[]` snapshot; `async.jobId` is the first started job and `async.state` aggregates ("running" until every job settles, "failed" if any spawn failed). - Live progress keeps streaming into the same tool block via `onUpdate(...)`; each final result arrives later as an async-result injection into the parent conversation. The delivery text appends a follow-up hint: `` is now idle — message it via `irc` to follow up; transcript at history:// `` (aborted variant points at the transcript only). -Settled (sync-fallback or job-body) response: +Settled response (`async.enabled=false`, no job manager, blocking agent, or async job body): - `content`: summary rendered from `packages/coding-agent/src/prompts/tools/task-summary.md` with a preview capped at 5000 chars; `agent://` holds the full output. A sync batch concatenates the per-spawn summaries. - `details.results`: one `SingleResult` per spawn; `usage`, `outputPaths` populated (aggregated across spawns for a sync batch). @@ -74,8 +74,8 @@ Artifacts and side channels: ## Flow 1. `TaskTool.create(...)` discovers agents once per cwd through a process-level memo (`discoverAgentsForCreate`) to render the dynamic prompt description. 2. `execute(...)` repairs raw params (`repairTaskParams`), then validates: `schema` is always rejected; `tasks`/`context` are rejected unless `task.batch` is on; batch calls need a non-empty `tasks` (per-item assignments, unique provided ids), a non-empty shared `context`, and no top-level `assignment`; flat calls need `assignment`. The call is then normalized into its spawn list (`resolveSpawnItems`). -3. Sync fallback only when the session has no `AsyncJobManager` (orphaned host) or the selected agent definition declares `blocking: true`; the call then runs `#executeSync(...)` inline under the session-scoped semaphore. -4. Otherwise execution is always async: +3. Sync execution runs when `async.enabled=false`, the session has no `AsyncJobManager` (orphaned host), or the selected agent definition declares `blocking: true`; the call then runs every spawn through `#executeSync(...)` inline under the session-scoped semaphore. +4. Background execution runs only when `async.enabled=true` and the session has an `AsyncJobManager`: - agent ids are allocated up front via `AgentOutputManager.allocate(item.id || generateTaskName())`, one per spawn; - one `type: "task"` job per spawn is registered with `session.asyncJobManager` (`id` = agent id, `queued: true`, `ownerId` = caller agent id) and the tool returns immediately; - each job body acquires the session-scoped `Semaphore` (one per `TaskTool` instance, sized from `task.maxConcurrency` at first use), marks the job running, runs `#executeSync(...)` with that spawn's params, and reports progress through the shared `buildAsyncDetails`/`onUpdate`; @@ -98,10 +98,10 @@ Artifacts and side channels: ## Modes / Variants - Execution mode - - Always-async background job — default; spawns go through `AsyncJobManager`. - - Sync inline fallback — only when no job manager exists or the agent definition has `blocking: true`. + - Background job — `async.enabled=true`; spawns go through `AsyncJobManager`. + - Sync inline — `async.enabled=false`, no job manager, or `blocking: true` agent. - Batch mode (`task.batch`, default on) - - on — `{ agent, context, tasks[] }`: one independent background job per item, required `context` shared across the call's spawns, `isolated` per item. Lifecycle, revival, and concurrency semantics are identical to N parallel single calls. + - on — `{ agent, context, tasks[] }`: one independent spawn per item, required `context` shared across the call's spawns, `isolated` per item. Lifecycle, revival, and concurrency semantics match N parallel single calls. - off — single spawn per call; `tasks`/`context` are rejected and removed from the schema. - Isolation backend: `none`, `worktree`, `fuse-overlay`, `fuse-projfs`. - Isolation merge strategy: patch mode (capture/apply root patches) or branch mode (commit to `omp/task/`, cherry-pick into parent). @@ -119,13 +119,13 @@ Artifacts and side channels: - Git operations for baseline capture, patch apply, worktrees, branches, stash, cherry-pick, commits. - Session state (transcript, memory, jobs, checkpoints, registries) - Creates child `AgentSession` instances with isolated settings snapshots; finished sessions stay registered in the process-global `AgentRegistry` as `idle`/`parked` until process teardown or explicit release. - - Registers one async job per call in `session.asyncJobManager`; completion is injected into the parent as an async-result message. + - With `async.enabled=true`, registers one async job per spawn in `session.asyncJobManager`; completion is injected into the parent as an async-result message. - Arms idle-TTL timers in `AgentLifecycleManager` (unref'd; they never hold the process open). - Emits `task:subagent:event`, `task:subagent:progress`, and `task:subagent:lifecycle` on the parent event bus. - Allocates session-scoped output ids through `AgentOutputManager` so `agent://` stays unique across invocations. - Shares the parent `local://` root and `ArtifactManager` with subagents. - Background work / cancellation - - `job cancel` (or parent tool-call abort) cancels the job; a hard-aborted run lands `aborted` and is torn down. + - `job cancel` (or parent tool-call abort) cancels background jobs; parent tool-call abort cancels sync runs through the call signal. A hard-aborted run lands `aborted` and is torn down. - Missing-`yield` recovery sends up to three internal reminder prompts to the child session. ## Limits & Caps @@ -154,7 +154,7 @@ Artifacts and side channels: - `agent://` resolution errors are model-visible when another tool reads them: no session, no artifacts dir, missing id, conflicting extraction syntax, or invalid JSON for extraction. ## Notes -- Parallelism is parallel `task` calls in one assistant message — or, with `task.batch`, a `tasks[]` batch in one call; either way the session-scoped semaphore bounds the fan-out and each spawn is an independent background job. +- Parallelism is parallel `task` calls in one assistant message — or, with `task.batch`, a `tasks[]` batch in one call; either way the session-scoped semaphore bounds the fan-out. With `async.enabled=true`, each spawn is an independent background job. - Shared background convention without batch mode: write it once to a `local://` file and reference that path in each assignment — subagents share the parent's `local://` root. With `task.batch`, the required `context` parameter carries the shared background directly into each spawn's system prompt. - Prefer messaging an existing agent (`irc`) over a fresh spawn for follow-up work: it already holds the relevant context. `irc` op:"list" shows idle/parked candidates; messaging a parked agent revives it. `history://` shows what an agent has done. - `irc` availability is derived, not configured (`isIrcEnabled` in `packages/coding-agent/src/tools/irc.ts`): it exists exactly when there is someone to message — the session can spawn subagents, or it is a subagent itself. Messaging is the only follow-up path to a finished subagent, so task without irc would strand idle agents. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f882f3deb..340b819bc 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,12 +12,15 @@ - Added the `statusLine.transparent` appearance setting (default off): when enabled, the status line skips the theme's `statusLineBg` fill and powerline end caps so the bar inherits the terminal's default background — useful in Ghostty and other terminals whose theme background does not match the theme's hardcoded status-line color ([#2306](https://github.com/can1357/oh-my-pi/issues/2306)) +### Changed + +- `async.enabled` now defaults to `true`, keeping background task execution and async bash available out of the box; disable it to force blocking subtasks ([#2301](https://github.com/can1357/oh-my-pi/issues/2301)). + ### Fixed +- Restored `async.enabled=false` as the task tool's blocking mode, so subagents no longer become background jobs when async execution is disabled ([#2301](https://github.com/can1357/oh-my-pi/issues/2301)). +- Fixed MCP OAuth authorization and token requests to include the required `resource` indicator for the target MCP server. - Fixed the `/tree` selector and HTML session export dropping the inherited `│` gutter for chain rows under a last-sibling branch, so the conversation flow under a `└─` branch stays visually anchored to its parent message ([#2298](https://github.com/can1357/oh-my-pi/issues/2298)). - -### Fixed - - Fixed Anthropic web search requests to include `metadata.user_id`, matching the main Messages path: API-key requests forward the active session id verbatim, OAuth requests build the Claude-Code-shaped `{session_id, account_uuid?, device_id}` JSON envelope so gateways see consistent attribution ([#2295](https://github.com/can1357/oh-my-pi/issues/2295)). ## [15.11.0] - 2026-06-10 diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 50af9ae13..581b850bc 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -2512,11 +2512,11 @@ export const SETTINGS_SCHEMA = { // Async jobs "async.enabled": { type: "boolean", - default: false, + default: true, ui: { tab: "tools", label: "Async Execution", - description: "Enable async bash commands", + description: "Enable async bash commands and background task execution", }, }, @@ -2769,7 +2769,7 @@ export const SETTINGS_SCHEMA = { tab: "tasks", label: "Batch Task Calls", description: - "Switch the task tool to its batch shape: one call carries { agent, context, tasks[] } — one subagent per item (with per-item isolation) and a required shared context prepended to every assignment. Each spawn still runs as an independent background agent with the normal idle/parked lifecycle. Disable to restore the flat single-spawn schema.", + "Switch the task tool to its batch shape: one call carries { agent, context, tasks[] } — one subagent per item (with per-item isolation) and a required shared context prepended to every assignment. With async.enabled=true, each spawn runs as an independent background agent with the normal idle/parked lifecycle; otherwise the call blocks for merged results. Disable to restore the flat single-spawn schema.", }, }, diff --git a/packages/coding-agent/src/prompts/tools/task.md b/packages/coding-agent/src/prompts/tools/task.md index ab9a299f9..3cd136a36 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -1,10 +1,15 @@ -{{#if batchEnabled}}Spawns subagents to work in the background — one per `tasks[]` item; a single spawn is a one-item batch.{{else}}Spawns ONE subagent per call to work in the background.{{/if}} +{{#if asyncEnabled}}{{#if batchEnabled}}Spawns subagents to work in the background — one per `tasks[]` item; a single spawn is a one-item batch.{{else}}Spawns ONE subagent per call to work in the background.{{/if}} - Spawning is non-blocking: the call returns immediately with the agent id{{#if batchEnabled}}s{{/if}} and job id{{#if batchEnabled}}s{{/if}}; each result is delivered automatically when that agent yields. - Parallelism = {{#if batchEnabled}}`tasks[]` items in one call, and/or multiple `task` calls in one assistant message{{else}}multiple `task` calls in one assistant message{{/if}}. Concurrency is bounded at {{MAX_CONCURRENCY}} running subagents per session. - If genuinely blocked on a result, wait with `job poll`; otherwise keep working. `job cancel` terminates a task and **cannot carry a message** — only for stalled/abandoned work. +{{else}}{{#if batchEnabled}}Runs subagents synchronously — one per `tasks[]` item; a single spawn is a one-item batch.{{else}}Runs ONE subagent synchronously per call.{{/if}} + +- Spawning is blocking: the call returns only after the agent{{#if batchEnabled}}s{{/if}} finish; results arrive inline. +- Parallelism = {{#if batchEnabled}}`tasks[]` items in one call, and/or multiple `task` calls in one assistant message{{else}}multiple `task` calls in one assistant message{{/if}}. Concurrency is bounded at {{MAX_CONCURRENCY}} running subagents per session. +{{/if}} {{#if ircEnabled}} -- Coordinate with running agents via `irc` using their ids. Agents reach you and their siblings live the same way. +- Coordinate with agents via `irc` using their ids. Agents reach you and their siblings live the same way. {{/if}} diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 990e08107..e3a191226 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -9,7 +9,7 @@ * Supports: * - Single agent spawn per call (parallelism = parallel task calls) * - Batch spawning + shared context per call when `task.batch` is enabled - * - Non-blocking execution via the session's AsyncJobManager + * - Background execution through AsyncJobManager when `async.enabled` is enabled * - Progress tracking via JSON events * - Session artifacts for debugging */ @@ -190,6 +190,7 @@ function renderDescription( isolationEnabled: boolean, disabledAgents: string[], batchEnabled: boolean, + asyncEnabled: boolean, ircEnabled: boolean, parentSpawns: string, ): string { @@ -217,6 +218,7 @@ function renderDescription( MAX_CONCURRENCY: maxConcurrency, isolationEnabled, batchEnabled, + asyncEnabled, ircEnabled, }); } @@ -374,8 +376,8 @@ function discoverAgentsForCreate(cwd: string): Promise { * Task tool - Delegate tasks to specialized agents. * * Each call spawns one subagent — or, with `task.batch`, one per `tasks[]` - * item. Spawning is non-blocking: the call registers AsyncJobManager jobs and - * returns immediately; each result is delivered when that agent yields. + * item. When `async.enabled` is on, spawns run as AsyncJobManager jobs; when + * disabled, the tool blocks until every spawn finishes. */ export class TaskTool implements AgentTool { readonly name = "task"; @@ -411,7 +413,7 @@ export class TaskTool implements AgentTool agent.name === params.agent); - const manager = this.session.asyncJobManager; - if (!manager || selectedAgent?.blocking === true) { - // Sync fallback: orphaned host that never wired a job manager, or an - // agent definition that declares `blocking: true`. The session-scoped - // semaphore still bounds fan-out across parallel task calls. - if (!manager) { + const asyncEnabled = this.session.settings.get("async.enabled"); + const manager = asyncEnabled ? this.session.asyncJobManager : undefined; + if (!asyncEnabled || !manager || selectedAgent?.blocking === true) { + // Sync fallback: async execution disabled, orphaned host that never + // wired a job manager, or an agent definition that declares + // `blocking: true`. The session-scoped semaphore still bounds fan-out + // across parallel task calls. + if (asyncEnabled && !manager) { logger.warn("task: no AsyncJobManager registered; falling back to sync execution"); } return this.#executeSyncFanout(toolCallId, params, spawnItems, signal, onUpdate); diff --git a/packages/coding-agent/test/task/task-batch.test.ts b/packages/coding-agent/test/task/task-batch.test.ts index 6e7ba6d6f..393de8f32 100644 --- a/packages/coding-agent/test/task/task-batch.test.ts +++ b/packages/coding-agent/test/task/task-batch.test.ts @@ -9,9 +9,10 @@ * 2. Shape validation rejects `schema` always, `tasks`/`context` while batch * is disabled, top-level `assignment` in batch calls, empty/invalid items, * duplicate ids, and a missing shared `context`. - * 3. A batch call registers one background job per item; every spawn receives - * the shared `context`, and each job delivers its own follow-up hint. The - * flat form stays accepted at runtime for internal callers. + * 3. With `async.enabled=true`, a batch call registers one background job per + * item; with `async.enabled=false`, it blocks and returns merged results. + * Both modes forward the shared `context`; the flat form stays accepted at + * runtime for internal callers. */ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import { toolWireSchema } from "@oh-my-pi/pi-ai/utils/schema"; @@ -133,9 +134,14 @@ describe("task.batch schema gating", () => { mockDiscovery(); const off = await TaskTool.create(createSession({ settings: { "task.batch": false } })); - expect(off.description).toContain("Spawns ONE subagent per call"); + expect(off.description).toContain("Spawns ONE subagent per call to work in the background"); expect(off.description).not.toContain("`context`: shared background"); + const offSync = await TaskTool.create( + createSession({ settings: { "async.enabled": false, "task.batch": false } }), + ); + expect(offSync.description).toContain("Runs ONE subagent synchronously per call"); + const on = await TaskTool.create(createSession({ settings: { "task.batch": true } })); expect(on.description).toContain("`tasks`: tasks to spawn"); expect(on.description).toContain("`context`: shared background"); @@ -244,7 +250,9 @@ describe("task.batch spawning", () => { }); const manager = createManager(); - const tool = await TaskTool.create(createSession({ manager, settings: { "task.batch": true } })); + const tool = await TaskTool.create( + createSession({ manager, settings: { "async.enabled": true, "task.batch": true } }), + ); const result = await tool.execute("tc-batch", { agent: "task", @@ -290,7 +298,9 @@ describe("task.batch spawning", () => { }); const manager = createManager(); - const tool = await TaskTool.create(createSession({ manager, settings: { "task.batch": true } })); + const tool = await TaskTool.create( + createSession({ manager, settings: { "async.enabled": true, "task.batch": true } }), + ); const result = await tool.execute("tc-single", { agent: "task", @@ -312,7 +322,9 @@ describe("task.batch spawning", () => { vi.spyOn(executorModule, "runSubprocess").mockImplementation(async options => makeResult(options.id ?? "?")); const manager = createManager(); - const tool = await TaskTool.create(createSession({ manager, settings: { "task.batch": true } })); + const tool = await TaskTool.create( + createSession({ manager, settings: { "async.enabled": true, "task.batch": true } }), + ); const result = await tool.execute("tc-flat", { agent: "task", @@ -325,4 +337,37 @@ describe("task.batch spawning", () => { await job.promise; expect(job.status).toBe("completed"); }); + + it("blocks batch execution when async.enabled is false even with a job manager", async () => { + mockDiscovery(); + const seen: Array<{ id?: string; context?: string; assignment?: string }> = []; + vi.spyOn(executorModule, "runSubprocess").mockImplementation(async options => { + seen.push({ id: options.id, context: options.context, assignment: options.assignment }); + return makeResult(options.id ?? "?"); + }); + + const manager = createManager(); + const tool = await TaskTool.create( + createSession({ manager, settings: { "async.enabled": false, "task.batch": true } }), + ); + + const result = await tool.execute("tc-sync-batch", { + agent: "task", + context: "# Goal\nShared synchronous context.", + tasks: [ + { id: "Alpha", assignment: "Do A." }, + { id: "Beta", assignment: "Do B." }, + ], + } as TaskParams); + + expect(getFirstText(result)).toContain("All done."); + expect(result.details?.async).toBeUndefined(); + expect(result.details?.results.map(item => item.id).sort()).toEqual(["Alpha", "Beta"]); + expect(manager.getJob("Alpha")).toBeUndefined(); + expect(manager.getJob("Beta")).toBeUndefined(); + expect(seen.map(spawn => spawn.context)).toEqual([ + "# Goal\nShared synchronous context.", + "# Goal\nShared synchronous context.", + ]); + }); }); diff --git a/packages/coding-agent/test/tools/task-async-fallback.test.ts b/packages/coding-agent/test/tools/task-async-fallback.test.ts index b6a9e9d58..39b8f2372 100644 --- a/packages/coding-agent/test/tools/task-async-fallback.test.ts +++ b/packages/coding-agent/test/tools/task-async-fallback.test.ts @@ -47,8 +47,8 @@ describe("task.async-fallback", () => { }); discoverSpy.mockResolvedValue({ agents: [], projectAgentsDir: null }); - // createSession never wires `asyncJobManager`, which is the fallback trigger. - const tool = await TaskTool.create(createSession()); + // Enable async so the missing `asyncJobManager` is the fallback trigger. + const tool = await TaskTool.create(createSession({ "async.enabled": true })); const result = await tool.execute("tool-1", { agent: "task",