fix(task): reject sparse model fallback arrays

This commit is contained in:
panosAthDBX
2026-07-23 09:53:54 +01:00
parent 10e1ba911c
commit bfef3f7eea
4 changed files with 27 additions and 18 deletions
+7 -5
View File
@@ -26,9 +26,9 @@
## Inputs
The wire schema is shape-swapped by `task.batch` (default on). One unit of work is the task item `{ name?, agent?, task, model?, isolated? }` (`isolated` only when `task.isolation.mode` is not `none`):
The wire schema is shape-swapped by `task.batch` (default on). One unit of work is the task item `{ name?, agent?, task, model?, outputSchema?, schemaMode?, isolated? }` (`isolated` only when `task.isolation.mode` is not `none`):
- **Batch shape** (`task.batch` on): `{ context, tasks: item[] }` — one subagent per item, all run under the same fan-out rules; there is no top-level agent field. `context` is **required** shared background rendered into every spawned subagent's system prompt (`CONTEXT` section); `agent`, `model`, and `isolated` are per item, so one call may mix agent types and models.
- **Batch shape** (`task.batch` on): `{ context, tasks: item[] }` — one subagent per item, all run under the same fan-out rules; there is no top-level agent field. `context` is **required** shared background rendered into every spawned subagent's system prompt (`CONTEXT` section); `agent`, `model`, `outputSchema`, `schemaMode`, and `isolated` are per item, so one call may mix agent types, models, and output contracts.
- **Flat shape** (`task.batch` off): `{ ...item }` — exactly one spawn per call. Shared background goes into a `local://` file (e.g. `local://ctx.md`) that each spawn's `task` references; subagents share the parent's `local://` root.
| Field | Type | Required | Description |
@@ -39,13 +39,15 @@ The wire schema is shape-swapped by `task.batch` (default on). One unit of work
| `agent` | `string` | No | Agent type to run this item (e.g. `scout`). Defaults to the spawn policy's default agent (usually `task`); items in one batch call may use different agent types. Item field in batch shape, top-level in flat shape. |
| `task` | `string` | Yes | The work — complete, self-contained instructions. Empty-after-trim is rejected. Item field in batch shape, top-level in flat shape. |
| `model` | `string \| string[]` | No | Explicit non-empty model selector or non-empty fallback chain for this spawn. Optional `:reasoning` suffixes are preserved. Takes precedence over `task.agentModelOverrides` and agent frontmatter. Item field in batch shape, top-level in flat shape. |
| `outputSchema` | JSON Schema object | No | Invocation-specific structured-output contract. Takes precedence over agent frontmatter `output` and the inherited parent session schema. Item field in batch shape, top-level in flat shape. |
| `schemaMode` | `"permissive" \| "strict"` | No | Validation mode for the effective output schema. Overrides the parent session mode; defaults to `permissive`. Item field in batch shape, top-level in flat shape. |
| `isolated` | `boolean` | No | Run in an isolated workspace and return patches. Exists only when `task.isolation.mode` is not `none`; per item in batch shape, top-level in flat shape. Isolated agents are torn down at completion — not revivable. |
There is no wire label field: the one-line UI label shown in the TUI/registry is generated automatically from the `task` text by the tiny/title model (fire-and-forget), so callers never provide it.
Runtime stays permissive: the flat form is accepted even while `task.batch` is on (internal callers such as the commit flow's `analyze_files`, and stale transcripts). The model only ever sees one shape.
There is no per-call `schema` parameter. Structured output comes from the agent definition's `output` frontmatter, the inherited parent session schema, or — for ad-hoc workflows — the eval bridge's `agent(prompt, schema)`.
There is no legacy per-call `schema` parameter. Use `outputSchema` and optional `schemaMode`; when absent, structured output falls back to the agent definition's `output` frontmatter and then the inherited parent session schema.
## Outputs
@@ -75,7 +77,7 @@ 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` (a `task` per item, unique provided names), a non-empty shared `context`, and no top-level `task` alongside `tasks`; flat calls need `task`. The call is then normalized into its spawn list (`resolveSpawnItems`).
2. `execute(...)` repairs raw params (`repairTaskParams`), then validates: `schema` is always rejected; model selectors that normalize to no patterns are rejected; `tasks`/`context` are rejected unless `task.batch` is on; batch calls need a non-empty `tasks` (a `task` per item, unique provided names), a non-empty shared `context`, and no top-level `task` alongside `tasks`; flat calls need `task`. The call is then normalized into its spawn list (`resolveSpawnItems`).
3. Per-item execution split: items whose agent type declares `blocking: true` run inline; the rest become background jobs. The whole call runs sync when `async.enabled=false`, the session has no `AsyncJobManager` (orphaned host), or every item is blocking; inline spawns run through `#executeSync(...)` under the session-scoped semaphore.
4. Background execution (any non-blocking item with `async.enabled=true` and an `AsyncJobManager`):
- agent ids are allocated up front via `AgentOutputManager.allocate(...)` — each item's `name`, or a generated AdjectiveNoun name — one per spawn;
@@ -104,7 +106,7 @@ Artifacts and side channels:
- Background job — `async.enabled=true`; non-blocking spawns go through `AsyncJobManager`.
- Sync inline — `async.enabled=false`, no job manager, or the item's agent declares `blocking: true` (per item: a mixed call runs both modes).
- Batch mode (`task.batch`, default on)
- on — `{ context, tasks[] }`: one independent spawn per item, required `context` shared across the call's spawns, `agent`/`isolated` per item. Lifecycle, revival, and concurrency semantics match N parallel single calls.
- on — `{ context, tasks[] }`: one independent spawn per item, required `context` shared across the call's spawns, with `agent`, `model`, `outputSchema`, `schemaMode`, and `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 mode (`task.isolation.mode`): `none`, `auto`, `apfs`, `btrfs`, `zfs`, `reflink`, `overlayfs`, `projfs`, `block-clone`, `rcopy` (legacy `worktree`, `fuse-overlay`, `fuse-projfs` accepted for back-compat); the PAL resolves the actual backend with fallback.
- Isolation merge strategy: patch mode (capture/apply root patches) or branch mode (commit to `omp/task/<id>`, cherry-pick into parent).
+5 -2
View File
@@ -232,10 +232,13 @@ function validateShapeParams(batchEnabled: boolean, params: TaskParams): string
function hasInvalidModelSelector(model: unknown): boolean {
if (model === undefined) return false;
const selectors = typeof model === "string" ? [model] : Array.isArray(model) ? model : undefined;
const materializedSelectors = selectors ? Array.from(selectors) : [];
return (
!selectors ||
selectors.length === 0 ||
selectors.some(selector => typeof selector !== "string" || !selector.split(",").some(pattern => pattern.trim()))
materializedSelectors.length === 0 ||
materializedSelectors.some(
selector => typeof selector !== "string" || !selector.split(",").some(pattern => pattern.trim()),
)
);
}
@@ -215,17 +215,20 @@ describe("task.batch validation", () => {
expect(text).toContain("Missing `context`");
});
it("rejects an empty per-item model selector", async () => {
const text = await executeText(
{
context: "Background.",
tasks: [{ name: "Alpha", task: "Work.", model: [","] }],
},
{ "task.batch": true },
);
expect(text).toContain("Task 1 (`Alpha`) has an invalid `model`");
expect(text).toContain("non-empty array");
});
it.each([{ model: [","] }, { model: Array<string>(1) }])(
"rejects an empty per-item model selector",
async ({ model }) => {
const text = await executeText(
{
context: "Background.",
tasks: [{ name: "Alpha", task: "Work.", model }],
},
{ "task.batch": true },
);
expect(text).toContain("Task 1 (`Alpha`) has an invalid `model`");
expect(text).toContain("non-empty array");
},
);
it("rejects duplicate provided names case-insensitively", async () => {
const text = await executeText(
@@ -94,6 +94,7 @@ describe("task spawn validation", () => {
{ model: "," },
{ model: " , " },
{ model: [] },
{ model: Array<string>(1) },
{ model: ["openai-codex/gpt-5.6-sol:high", " "] },
{ model: ["openai-codex/gpt-5.6-sol:high", ","] },
])("rejects an empty model selector", async ({ model }) => {