diff --git a/README.md b/README.md index d5edd3a..a19e5de 100644 --- a/README.md +++ b/README.md @@ -364,9 +364,20 @@ monotonic, one space per source store): Emit `{ id, verb, params }`, get exactly one reply `{ id, ok, data }` or `{ id, ok: false, error: { code, message } }`. Verbs: `spawn` (returns `{ runId }` -immediately; result arrives via `fleet:run:ended`), `status`, `observe` (replay dump — -subscribe to the broadcast channels + dedupe by `(channel, runId, seq)` for the live tail), -`steer`, `abort`. +immediately; result arrives via `fleet:run:ended`), `schedule` (#83 — returns +`{ scheduleId, nextFire }`; schedules run lifecycles, not single delegates), `status`, +`observe` (replay dump — subscribe to the broadcast channels + dedupe by +`(channel, runId, seq)` for the live tail), `steer`, `abort`. + +`spawn` params: `agent`, `task`, `background?`, `lifecycle?` (named lifecycle — with +`background: true` routes through the bg runner; without it runs as a detached +foreground-semantics lifecycle), `modelFallback?` (per-request retry-once on a retryable +provider failure — the retry mints a fresh runId and relinks the primary's todo), `cwd?`, +`isolation?`, `maxTurns?`, `model?`, `skills?`, `readOnly?`, `track?`, `todoId?`. + +`schedule` params: `task`, `expression` (cron or interval), `lifecycle?`, `auto?`, +`isolation?`, `cwd?` — no `agent`: the scheduler runs lifecycles only. Registration emits +no run events; on fire, the normal `fleet:*` stream flows with `mode: "scheduled"`. Error codes: `E-CONTROL-DISABLED`, `E-RUN-NOT-FOUND`, `E-RUN-FINISHED`, `E-BAD-VERB`, `E-BAD-PARAMS`, `E-STEER-UNSUPPORTED`, `E-INTERNAL`. @@ -395,7 +406,7 @@ export function fleetRpc(pi: { events: { emit(c: string, d: unknown): void; on(c ### Control gate -`spawn`/`steer`/`abort` are on by default. Set `ARMORY_FLEET_RPC_CONTROL=0` (or `false`) to +`spawn`/`steer`/`abort`/`schedule` are on by default. Set `ARMORY_FLEET_RPC_CONTROL=0` (or `false`) to reject them with `E-CONTROL-DISABLED`; read-only `observe`/`status` stay available. Honest threat model: in-process extensions already have full system access through pi itself — the switch guards accidents, not adversaries. diff --git a/docs/superpowers/specs/2026-08-30-spec-rpc-lifecycle-schedule-modelfallback.md b/docs/superpowers/specs/2026-08-30-spec-rpc-lifecycle-schedule-modelfallback.md new file mode 100644 index 0000000..126eb18 --- /dev/null +++ b/docs/superpowers/specs/2026-08-30-spec-rpc-lifecycle-schedule-modelfallback.md @@ -0,0 +1,35 @@ +# SPEC — RPC spawn `lifecycle`/`modelFallback` params + `schedule` verb (#83) + +**Date:** 2026-08-30 · **Status:** approved (Option B, RECTOR) · **Parent:** SPEC-6-4 §7 deferred list +**Frozen-surface rule:** additive only — no renames, no reshapes, no new error codes. + +## Problem + +SPEC-6-4 shipped RPC `spawn` as single-delegate + background routing only. `lifecycle`, `schedule`, and `modelFallback` reject with `E-BAD-PARAMS`. The `subagent` tool supports all three; the RPC surface should reach parity for external consumers. + +## Decisions + +**D1 — `schedule` is its own verb (RECTOR, Option B).** Not a spawn param. Reasons: the scheduler is lifecycle-based (`ScheduleSpec` = `{ task, expression, lifecycle?, auto?, isolation?, cwd? }` — no `agent` field), so a spawn param would advertise an affordance that doesn't exist; and spawn's reply shape must stay uniform (`{ runId }`) rather than branching on params. + +**D2 — spawn `lifecycle` param mirrors the tool's routing:** +- `background: true` (+ optional `lifecycle`): `runBackground({ lifecycle: params.lifecycle ?? "default" })` — replaces today's hardcoded `"default"`. +- `lifecycle` WITHOUT `background`: detached foreground-semantics lifecycle run — `runLifecycle(task, lifecycle, { deps: lifecycleFullDeps, mode: "auto", entryCwd })` with `genRunId: () => runId` (the asyncRunLifecycle pre-minted-id pattern), phase spawn on the SESSION `deps.lock` (foreground pool, like the tool's fg lifecycle), phase cwd resolution unchanged (`lifecycle.cwd ?? entryCwd`). +- `mode` is always `"auto"` over RPC (the tool hardcodes it too; checkpointed lifecycles are interactive-only). + +**D3 — `modelFallback` (per-request wins over the global default, exactly like the tool's `params.modelFallback ?? deps.defaultModelFallback`):** +- bg leg: `RunBackgroundOpts.modelFallback` → `RunLifecycleOpts.modelFallback` → `asyncRunLifecycle`'s `withModelFallbackRetry(fn, opts.modelFallback ?? deps.defaultModelFallback)`. +- fg single-delegate leg: retry ONCE inline, mirroring the tool's direct path — fresh runId for the retry (the primary already journaled under the pre-minted id; reusing it would double-emit `run:started`), `todoId` relinked from the primary. Extracted as `retryForegroundOnce(primary, fallback, spawn)` (testable). +- fg lifecycle leg: `withModelFallbackRetry` on the phase spawn (same as the tool + asyncRunLifecycle). + +**D4 — schedule verb contract:** +- Request: `{ id, verb: "schedule", params: { task, expression, lifecycle?, auto?, isolation?, cwd? } }`. +- Reply: `{ ok: true, data: { scheduleId, nextFire: } }`. +- Behind the same `ARMORY_FLEET_RPC_CONTROL` gate (it is a control operation). Scheduler not configured → `E-BAD-PARAMS` (actionable "scheduler missing" message, mirroring the bg message style). `scheduler.register` throws (invalid expression) → `E-BAD-PARAMS` with the parser's message. +- No new verbs for pause/list/abort of schedules — out of scope (#83 tracks registration only). +- No run events at registration (nothing is running); on fire, the existing scheduler → `runBackground` path emits the normal `fleet:*` stream with `mode: "scheduled"` (pre-existing SPEC-5a behavior). + +**D5 — validation:** `lifecycle`/`modelFallback`/`expression`/`task` must be non-empty strings when set; `auto` boolean; `isolation` enum (`'worktree' | 'none' | 'auto'`); `cwd` resolves through the same `resolveDispatchCwd` preflight as spawn. All failures → `E-BAD-PARAMS`, validation BEFORE runId minting (no ghost runIds). + +## Non-goals + +Per-dispatch `thinkingLevel` tool param (follow-up to #78); schedule pause/list/abort verbs; `agent` on `ScheduleSpec`; external transports (SPEC-6-4 §7). diff --git a/src/engine/retry-fallback.ts b/src/engine/retry-fallback.ts index 3c82d96..5ff49b5 100644 --- a/src/engine/retry-fallback.ts +++ b/src/engine/retry-fallback.ts @@ -31,3 +31,17 @@ export function withModelFallbackRetry(spawn: SpawnFn, fallback: string | undefi return first; }; } + +/** #83 D3: the RPC foreground-semantics detached spawn's retry — mirrors the tool's direct-path + * contract (retry ONCE, retryable failures only, DISTINCT fallback model) but the retry runs + * WITHOUT the pre-minted runId: the primary already journaled run:started/ended under it, so + * the retry must mint a fresh id (a reused id would double-emit run:started). `todoId` links + * the retry to the primary's armory-todo task (same relink contract as the tool). */ +export async function retryForegroundOnce( + primary: SpawnResult, + fallback: string | undefined, + spawn: (o: { model: string; todoId?: string }) => Promise, +): Promise { + if (!fallback || primary.status !== "failed" || !primary.retryable || fallback === primary.model) return primary; + return spawn({ model: fallback, todoId: primary.todoId ?? undefined }); +} diff --git a/src/index.ts b/src/index.ts index 6f78f20..c091d48 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,7 @@ import { getAgentDir, } from "@earendil-works/pi-coding-agent"; import type { Model } from "@earendil-works/pi-ai"; -import { createSubagentTool, resolveDispatchCwd, type SubagentToolDeps } from "./tools/subagent.ts"; +import { createSubagentTool, mergeLifecycleSkills, resolveDispatchCwd, type SubagentToolDeps } from "./tools/subagent.ts"; // SPEC-6-3: /fleet uses openWorkflowPanelLoop (Task 12) instead of the raw openFleetPanel factory. import { discoverAgents } from "./registry/discovery.ts"; import { RunRegistry } from "./engine/run-registry.ts"; @@ -306,7 +306,7 @@ export default async function (pi: ExtensionAPI): Promise { cwd: isolated ? opts.worktreePath : o.cwd, runLog: deps.runLog, tierRegistry: deps.tierRegistry, modelRegistry: deps.modelRegistry, defaultThinkingLevel: deps.defaultSubagentThinking, - }), deps.defaultModelFallback), + }), opts.modelFallback ?? deps.defaultModelFallback), }; const res = await runLifecycle(task, lifecycleName, { deps: lifecycleFullDeps, mode: opts.mode, worktreePath: opts.worktreePath, baseRef: "HEAD", entryCwd: opts.entryCwd, onCheckpoint: async (p) => p.status === "failed" ? { action: "abort" } : { action: "continue" } }); return res as unknown as import("./runtime/async-runner.ts").FakeLifecycleResult; @@ -431,33 +431,84 @@ export default async function (pi: ExtensionAPI): Promise { journal: fleetJournal, parentCwd: ctx.cwd, hasAsyncRunner: true, + // #83 D4: schedule registration — scheduler.register + nextFire lookup. Throws (invalid + // expression) propagate to the verb, which maps them to E-BAD-PARAMS. + schedule: (spec) => { + if (!deps.scheduler) throw new Error("scheduling not configured in this session (scheduler missing)"); + const id = deps.scheduler.register({ + task: String(spec.task), + expression: String(spec.expression), + ...(typeof spec.lifecycle === "string" ? { lifecycle: spec.lifecycle } : {}), + ...(typeof spec.auto === "boolean" ? { auto: spec.auto } : {}), + ...(spec.isolation === "worktree" || spec.isolation === "none" || spec.isolation === "auto" ? { isolation: spec.isolation } : {}), + ...(typeof spec.cwd === "string" ? { cwd: spec.cwd } : {}), + }); + const entry = deps.scheduler.list().find((s) => s.id === id); + return { scheduleId: id, nextFire: entry?.nextFire?.toISOString() ?? null }; + }, spawn: (params: Record, runId: string) => { // Detached fire-and-forget: NEVER throws — spawnSubagent journals its own fail path // (run:ended + todo revert), so the caller's pre-minted runId always resolves to events. void (async () => { const requestedCwd = typeof params.cwd === "string" ? params.cwd : undefined; const { cwd: resolvedCwd } = resolveDispatchCwd(requestedCwd, ctx.cwd); + // #83: lifecycle + per-request modelFallback params (validated by the verb; re-narrowed here). + const rpcLifecycle = typeof params.lifecycle === "string" && params.lifecycle ? params.lifecycle : "default"; + const rpcFallback = typeof params.modelFallback === "string" && params.modelFallback ? params.modelFallback : undefined; if (params.background === true) { // RPC background: routed through the async runner (pool slot + isolation + origin), // identical to the tool's runBackground path (spec §3.2). Ghost-runId prevention: // a synchronous pre-flight failure journals run:ended failed under the caller's id. const handle = runBackground(String(params.task), { deps: deps.asyncRunner!, - lifecycle: "default", + lifecycle: rpcLifecycle, mode: "auto", isolation: params.isolation as "worktree" | "none" | "auto" | undefined, cwd: resolvedCwd ?? ctx.cwd, origin: "background", runId, + ...(rpcFallback ? { modelFallback: rpcFallback } : {}), }); if (handle.status === "failed") { deps.runLog?.append(runId, { type: "run:ended", runId, status: "failed", endedAt: Date.now(), tokenTotal: 0, error: handle.error }); } return; } + if (params.lifecycle !== undefined) { + // #83 D2: lifecycle WITHOUT background = detached foreground-semantics lifecycle run + // (tool parity: mode "auto", failed-phase checkpoint aborts, session lock on the phase + // spawn, pre-minted runId via the genRunId override — same pattern as asyncRunLifecycle). + const { runLifecycle } = await import("./lifecycle/run-lifecycle.ts"); + const { spawnSubagent } = await import("./engine/spawnSubagent.ts"); + const { withModelFallbackRetry } = await import("./engine/retry-fallback.ts"); + const lifecycleFullDeps = { + ...deps.lifecycleDeps, + genRunId: () => runId, + spawn: withModelFallbackRetry(async (o) => spawnSubagent({ + agent: o.agent, task: o.task, lifecycleTodoId: o.lifecycleTodoId, model: o.model, + skillsOverride: mergeLifecycleSkills(o.skills, Array.isArray(params.skills) ? params.skills as string[] : undefined), + backendOverride: o.backend, + registry: deps.registry, todoSync: deps.todoSync, runRegistry: deps.runRegistry, lock: deps.lock, + backendRegistry: deps.backendRegistry, parentModel: deps.parentModel, parentCwd: ctx.cwd, + runLog: deps.runLog, signal: undefined, + tierRegistry: deps.tierRegistry, modelRegistry: deps.modelRegistry, + defaultThinkingLevel: deps.defaultSubagentThinking, + maxTurns: typeof params.maxTurns === "number" ? params.maxTurns : undefined, + readOnly: params.readOnly === true, + cwd: o.cwd ?? resolvedCwd, + }), rpcFallback ?? deps.defaultModelFallback), + }; + const res = await runLifecycle(String(params.task), rpcLifecycle, { + deps: lifecycleFullDeps, mode: "auto", entryCwd: resolvedCwd, + onCheckpoint: async (p) => p.status === "failed" ? { action: "abort" } : { action: "continue" }, + }); + deps.lifecycleRuns.set(res.runId, res); // panel Lifecycle-view visibility (workflow-lambda precedent) + return; + } // Foreground-semantics detached spawn (no mode — "foreground" is the truth). const { spawnSubagent } = await import("./engine/spawnSubagent.ts"); - await spawnSubagent({ + const { retryForegroundOnce } = await import("./engine/retry-fallback.ts"); + const primary = await spawnSubagent({ agent: params.agent as string, task: params.task as string, todoId: typeof params.todoId === "string" ? params.todoId : undefined, @@ -480,6 +531,31 @@ export default async function (pi: ExtensionAPI): Promise { runId, defaultThinkingLevel: deps.defaultSubagentThinking, }); + // #83 D3: per-request fallback retry — the retry mints a FRESH runId (the primary's + // pre-minted id stays retired; a reuse would double-emit run:started) and relinks the + // primary's todo, exactly like the tool's direct path. + await retryForegroundOnce(primary, rpcFallback, (o) => spawnSubagent({ + agent: params.agent as string, + task: params.task as string, + todoId: o.todoId, + track: params.track === true, + model: o.model, + readOnly: params.readOnly === true, + skillsOverride: Array.isArray(params.skills) ? params.skills as string[] : undefined, + registry: deps.registry, + todoSync: deps.todoSync, + runRegistry: deps.runRegistry, + lock: deps.lock, + backendRegistry: deps.backendRegistry, + parentModel: deps.parentModel, + parentCwd: ctx.cwd, + runLog: deps.runLog, + maxTurns: typeof params.maxTurns === "number" ? params.maxTurns : undefined, + tierRegistry: deps.tierRegistry, + modelRegistry: deps.modelRegistry, + cwd: resolvedCwd ?? ctx.cwd, + defaultThinkingLevel: deps.defaultSubagentThinking, + })); })().catch(() => {}); }, }); diff --git a/src/rpc/rpc-server.ts b/src/rpc/rpc-server.ts index c2423cb..da0500d 100644 --- a/src/rpc/rpc-server.ts +++ b/src/rpc/rpc-server.ts @@ -40,6 +40,9 @@ export interface RpcServerDeps { * Never throws — runtime failures land via the registry + RunLog journal (spawnSubagent's own * fail path journals run:ended), so the caller's { runId } always resolves to a real run. */ spawn: (params: Record, runId: string) => void; + /** #83: schedule registration (scheduler.register under the hood). Throws on an invalid + * expression (surfaced as E-BAD-PARAMS); absent = scheduling not configured in this session. */ + schedule?: (spec: Record) => { scheduleId: string; nextFire: string | null }; } const LIST_CAP = 25; @@ -84,14 +87,15 @@ export class RpcServer { case "spawn": return gated ? this.spawnVerb(id, params) : this.controlDisabled(id); case "steer": return gated ? this.steerVerb(id, params) : this.controlDisabled(id); case "abort": return gated ? this.abortVerb(id, params) : this.controlDisabled(id); + case "schedule": return gated ? this.scheduleVerb(id, params) : this.controlDisabled(id); case "observe": return this.observeVerb(id, params); case "status": return this.statusVerb(id, params); - default: return this.err(id, "E-BAD-VERB", `unknown verb '${verb}' (known: spawn, steer, observe, abort, status)`); + default: return this.err(id, "E-BAD-VERB", `unknown verb '${verb}' (known: spawn, steer, observe, abort, status, schedule)`); } } private controlDisabled(id: string): RpcReply { - return this.err(id, "E-CONTROL-DISABLED", "fleet rpc control is disabled (ARMORY_FLEET_RPC_CONTROL is set to off; remove it or set it to 1 to enable spawn/steer/abort)"); + return this.err(id, "E-CONTROL-DISABLED", "fleet rpc control is disabled (ARMORY_FLEET_RPC_CONTROL is set to off; remove it or set it to 1 to enable spawn/steer/abort/schedule)"); } private err(id: string, code: RpcErrorCode, message: string): RpcReply { @@ -107,9 +111,9 @@ export class RpcServer { if (!p) return this.err(id, "E-BAD-PARAMS", "spawn requires params: { agent, task, ... }"); if (typeof p.agent !== "string" || !p.agent) return this.err(id, "E-BAD-PARAMS", "params.agent must be a non-empty string"); if (typeof p.task !== "string" || !p.task) return this.err(id, "E-BAD-PARAMS", "params.task must be a non-empty string"); - if (p.lifecycle !== undefined) return this.err(id, "E-BAD-PARAMS", "params.lifecycle is not supported over RPC spawn yet (single-delegate + background only — spec §7)"); - if (p.schedule !== undefined) return this.err(id, "E-BAD-PARAMS", "params.schedule is not supported over RPC spawn yet"); - if (p.modelFallback !== undefined) return this.err(id, "E-BAD-PARAMS", "params.modelFallback is not supported over RPC spawn yet"); + if (p.lifecycle !== undefined && (typeof p.lifecycle !== "string" || !p.lifecycle)) return this.err(id, "E-BAD-PARAMS", "params.lifecycle must be a non-empty string when set (#83)"); + if (p.schedule !== undefined) return this.err(id, "E-BAD-PARAMS", "params.schedule is not a spawn param — schedules run lifecycles, not single delegates; use the 'schedule' verb (#83)"); + if (p.modelFallback !== undefined && (typeof p.modelFallback !== "string" || !p.modelFallback)) return this.err(id, "E-BAD-PARAMS", "params.modelFallback must be a non-empty string when set (#83)"); if (p.cwd !== undefined && (typeof p.cwd !== "string" || p.cwd === "")) return this.err(id, "E-BAD-PARAMS", "params.cwd must be a non-empty string when set"); if (p.cwd !== undefined) { const { error } = resolveDispatchCwd(p.cwd, this.deps.parentCwd); @@ -135,6 +139,42 @@ export class RpcServer { return { id, ok: true, data: { runId } }; } + /** #83 D4: register a recurring lifecycle run. Reply shape { scheduleId, nextFire } — schedules + * are NOT runs, so no runId (spawn's uniform { runId } contract stays unbranched). */ + private scheduleVerb(id: string, params: unknown): RpcReply { + const p = this.obj(params); + if (!p) return this.err(id, "E-BAD-PARAMS", "schedule requires params: { task, expression, ... }"); + if (typeof p.task !== "string" || !p.task) return this.err(id, "E-BAD-PARAMS", "params.task must be a non-empty string"); + if (typeof p.expression !== "string" || !p.expression) return this.err(id, "E-BAD-PARAMS", "params.expression must be a non-empty string (cron or interval, e.g. '*/5 * * * *' or '30m')"); + if (p.lifecycle !== undefined && (typeof p.lifecycle !== "string" || !p.lifecycle)) return this.err(id, "E-BAD-PARAMS", "params.lifecycle must be a non-empty string when set"); + if (p.auto !== undefined && typeof p.auto !== "boolean") return this.err(id, "E-BAD-PARAMS", "params.auto must be a boolean"); + if (p.isolation !== undefined && p.isolation !== "worktree" && p.isolation !== "none" && p.isolation !== "auto") { + return this.err(id, "E-BAD-PARAMS", "params.isolation must be 'worktree' | 'none' | 'auto'"); + } + if (p.cwd !== undefined && (typeof p.cwd !== "string" || p.cwd === "")) return this.err(id, "E-BAD-PARAMS", "params.cwd must be a non-empty string when set"); + let cwd: string | undefined; + if (p.cwd !== undefined) { + const resolved = resolveDispatchCwd(p.cwd, this.deps.parentCwd); + if (resolved.error) return this.err(id, "E-BAD-PARAMS", resolved.error); + cwd = resolved.cwd; + } + if (!this.deps.schedule) { + return this.err(id, "E-BAD-PARAMS", "scheduling not configured in this session (scheduler missing)"); + } + try { + const out = this.deps.schedule({ + task: p.task, expression: p.expression, + ...(p.lifecycle !== undefined ? { lifecycle: p.lifecycle } : {}), + ...(p.auto !== undefined ? { auto: p.auto } : {}), + ...(p.isolation !== undefined ? { isolation: p.isolation } : {}), + ...(cwd !== undefined ? { cwd } : {}), + }); + return { id, ok: true, data: { scheduleId: out.scheduleId, nextFire: out.nextFire } }; + } catch (e) { + return this.err(id, "E-BAD-PARAMS", (e as Error).message || "schedule registration failed"); + } + } + private statusVerb(id: string, params: unknown): RpcReply { const p = this.obj(params) ?? {}; if (p.runId !== undefined) { diff --git a/src/runtime/async-runner.ts b/src/runtime/async-runner.ts index e9af7ae..e087e70 100644 --- a/src/runtime/async-runner.ts +++ b/src/runtime/async-runner.ts @@ -31,6 +31,9 @@ export interface RunLifecycleOpts { /** #62: the dispatch target cwd for in-place runs (isolated runs pass the worktree path). * Flows into runLifecycle's SPEC-6-5 cwd resolution (lifecycle.cwd ?? entryCwd). */ entryCwd?: string; + /** #83: per-run fallback model for the phase-spawn retry wrapper (wins over the host's + * global default). Undefined = use the host default (back-compat). */ + modelFallback?: string; } export type RunLifecycleFn = (task: string, lifecycleName: string, opts: RunLifecycleOpts) => Promise; @@ -66,6 +69,9 @@ export interface RunBackgroundOpts { runId?: string; /** v0.11.1: edit isolation for background runs. Default "auto" (worktree when cwd is a git repo, in-place otherwise). */ isolation?: Isolation; + /** #83: per-run fallback model — forwarded to the runLifecycle adapter so its phase-spawn + * retry wrapper prefers this over the host's global default. Undefined = host default. */ + modelFallback?: string; /** #62: the dispatch target cwd (undefined = session cwd, back-compat). Scopes the run: * isolation routing + worktree creation resolve against THIS cwd (via deps.worktreeFor), * and in-place runs pass it as the lifecycle entryCwd. */ @@ -120,7 +126,7 @@ function runBackgroundInPlace(runId: string, task: string, opts: RunBackgroundOp deps.journal.append(runId, ev0); emitProgress(deps, runId, { status: "running", phase: "", phaseIndex: 0, phaseTotal: 0, lifecycle: opts.lifecycle, mode: opts.mode, task }); - const res = await deps.runLifecycle(task, opts.lifecycle, { runId, worktreePath: isolated?.worktreePath, branch: isolated?.branch, mode: opts.mode, entryCwd: isolated ? isolated.worktreePath : opts.cwd, fleetMode: opts.origin ?? "background" }); + const res = await deps.runLifecycle(task, opts.lifecycle, { runId, worktreePath: isolated?.worktreePath, branch: isolated?.branch, mode: opts.mode, entryCwd: isolated ? isolated.worktreePath : opts.cwd, fleetMode: opts.origin ?? "background", ...(opts.modelFallback ? { modelFallback: opts.modelFallback } : {}) }); if (res.status === "completed") { if (isolated) { diff --git a/test/async-runner.test.mts b/test/async-runner.test.mts index aff5b9b..f65b605 100644 --- a/test/async-runner.test.mts +++ b/test/async-runner.test.mts @@ -294,3 +294,24 @@ test("RunBackgroundOpts.runId pre-mints: every isolation route names the bg run assert.equal(handle.runId, "fl-preminted-1", `route ${JSON.stringify(route)}: caller's pre-minted runId must be used verbatim`); } }); + +test("RunBackgroundOpts.modelFallback forwards to the runLifecycle adapter opts (#83)", async () => { + const repo = makeRepo(); + let seenFallback: string | boolean | undefined; + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { + seenFallback = (opts as { modelFallback?: string }).modelFallback; + return { + runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [], startedAt: 1, endedAt: 2, todoId: null, + }; + }; + try { + const { deps } = makeDeps(repo, fakeLifecycle); + const handle = runBackground("t", { deps, lifecycle: "default", mode: "auto", isolation: "none", modelFallback: "Test/fallback" }); + assert.equal(handle.status, "background"); + await new Promise((r) => setTimeout(r, 60)); + assert.equal(seenFallback, "Test/fallback", "the per-request fallback reaches the lifecycle adapter verbatim"); + } finally { + rmSync(repo, { recursive: true, force: true }); + } +}); diff --git a/test/retry-foreground-once.test.mts b/test/retry-foreground-once.test.mts new file mode 100644 index 0000000..5081f05 --- /dev/null +++ b/test/retry-foreground-once.test.mts @@ -0,0 +1,56 @@ +// test/retry-foreground-once.test.mts +// #83 D3: the RPC foreground-semantics detached spawn's fallback retry — retry ONCE, +// only on retryable failures, only with a DISTINCT fallback model, fresh runId + +// todoId relinked from the primary (mirrors the tool's direct-path retry contract). +import { test } from "node:test"; +import { strictEqual, deepStrictEqual } from "node:assert"; +import { retryForegroundOnce } from "../src/engine/retry-fallback.ts"; +import type { SpawnResult } from "../src/engine/spawnSubagent.ts"; + +const res = (over: Partial): SpawnResult => ({ + status: "failed", finalText: "", runId: "fl-primary", todoId: null, agent: "scout", + model: "Test/primary", durationMs: 1, tokenTotal: 0, retryable: true, + error: "model call ended with stopReason 'error'", ...over, +}); + +test("no fallback → primary returned untouched (single spawn call)", async () => { + let calls = 0; + const out = await retryForegroundOnce(res({}), undefined, async () => { calls++; return res({}); }); + strictEqual(calls, 0); + strictEqual(out.runId, "fl-primary"); +}); + +test("non-retryable failure → no retry (turn budget / abort / lock busy pass through)", async () => { + let calls = 0; + const primary = res({ retryable: undefined, error: "turn budget exceeded" }); + const out = await retryForegroundOnce(primary, "Test/fallback", async () => { calls++; return res({}); }); + strictEqual(calls, 0); + strictEqual(out.error, "turn budget exceeded"); +}); + +test("fallback identical to the primary model → no retry (pointless loop guard)", async () => { + let calls = 0; + await retryForegroundOnce(res({}), "Test/primary", async () => { calls++; return res({}); }); + strictEqual(calls, 0); +}); + +test("retryable failure + distinct fallback → retry fires with the fallback model + primary's todoId", async () => { + const seen: Array<{ model?: string; todoId?: string }> = []; + const retryRes = res({ runId: "fl-retry", model: "Test/fallback", status: "completed", finalText: "done", retryable: undefined, error: undefined }); + const out = await retryForegroundOnce(res({ todoId: "td-1" }), "Test/fallback", async (o) => { + seen.push({ model: o?.model, todoId: o?.todoId }); + return retryRes; + }); + deepStrictEqual(seen, [{ model: "Test/fallback", todoId: "td-1" }], "retry inherits the primary's linked todo"); + strictEqual(out.runId, "fl-retry", "the retry's OWN result wins (fresh runId — the primary's id stays retired)"); + strictEqual(out.status, "completed"); +}); + +test("primary without a todo → retry runs unlinked", async () => { + const seen: Array<{ todoId?: string }> = []; + await retryForegroundOnce(res({ todoId: null }), "Test/fallback", async (o) => { + seen.push({ todoId: o?.todoId }); + return res({ runId: "fl-retry", model: "Test/fallback", status: "completed", finalText: "done", retryable: undefined }); + }); + deepStrictEqual(seen, [{ todoId: undefined }]); +}); diff --git a/test/rpc-server.test.mts b/test/rpc-server.test.mts index c866151..8a112ad 100644 --- a/test/rpc-server.test.mts +++ b/test/rpc-server.test.mts @@ -81,8 +81,96 @@ test("spawn: validates params, pre-mints runId, calls the detached spawn, return assert.equal(h.spawned()[0]!.params.agent, "scout"); const bad = await h.server.handle({ id: "s2", verb: "spawn", params: { agent: "", task: "go" } }); assert.equal((bad as { error: { code: string } }).error.code, "E-BAD-PARAMS"); - const life = await h.server.handle({ id: "s3", verb: "spawn", params: { agent: "a", task: "t", lifecycle: "default" } }); - assert.equal((life as { error: { code: string } }).error.code, "E-BAD-PARAMS", "lifecycle over RPC is deferred (spec §7)"); + const life = await h.server.handle({ id: "s3", verb: "spawn", params: { agent: "a", task: "t", lifecycle: "default" } }) as { ok: true; data: { runId: string } }; + assert.equal(life.ok, true, "lifecycle over RPC is accepted (#83 — spec §7 deferral lifted)"); + assert.equal(h.spawned().length, 2); + assert.equal(h.spawned()[1]!.params.lifecycle, "default", "lifecycle passes through to the detached spawn"); + } finally { rmSync(h.dir, { recursive: true, force: true }); } +}); + +test("spawn: lifecycle + modelFallback validate as non-empty strings; valid values pass through (#83)", async () => { + const h = harness({ hasAsyncRunner: true }); + try { + for (const params of [ + { agent: "a", task: "t", lifecycle: "" }, + { agent: "a", task: "t", lifecycle: 42 }, + { agent: "a", task: "t", modelFallback: "" }, + { agent: "a", task: "t", modelFallback: 7 }, + ]) { + const r = await h.server.handle({ id: "v", verb: "spawn", params }) as { error: { code: string; message: string } }; + assert.equal(r.error.code, "E-BAD-PARAMS", JSON.stringify(params)); + assert.equal(h.spawned().length, 0, "no ghost runId on validation failure"); + } + const ok = await h.server.handle({ id: "ok", verb: "spawn", params: { agent: "a", task: "t", lifecycle: "review", modelFallback: "anthropic/claude-sonnet-4", background: true } }) as { ok: true; data: { runId: string } }; + assert.equal(ok.ok, true); + assert.equal(h.spawned()[0]!.params.lifecycle, "review"); + assert.equal(h.spawned()[0]!.params.modelFallback, "anthropic/claude-sonnet-4"); + assert.equal(h.spawned()[0]!.params.background, true); + } finally { rmSync(h.dir, { recursive: true, force: true }); } +}); + +test("schedule: gated like other control verbs (#83)", async () => { + const h = harness(); + const gated = new RpcServer(h.deps, () => false); + try { + const r = await gated.handle({ id: "g", verb: "schedule", params: { task: "t", expression: "*/5 * * * *" } }); + assert.equal((r as { error: { code: string } }).error.code, "E-CONTROL-DISABLED"); + } finally { rmSync(h.dir, { recursive: true, force: true }); } +}); + +test("schedule: scheduler not configured → actionable E-BAD-PARAMS (#83)", async () => { + const h = harness(); // no `schedule` dep wired + try { + const r = await h.server.handle({ id: "n", verb: "schedule", params: { task: "t", expression: "*/5 * * * *" } }) as { error: { code: string; message: string } }; + assert.equal(r.error.code, "E-BAD-PARAMS"); + assert.match(r.error.message, /scheduler/); + } finally { rmSync(h.dir, { recursive: true, force: true }); } +}); + +test("schedule: validates task + expression + optional fields, then replies { scheduleId, nextFire } (#83)", async () => { + const registered: Array> = []; + const h = harness({ + schedule: (spec: Record) => { + registered.push(spec); + return { scheduleId: "sch-abc", nextFire: "2026-09-01T00:00:00.000Z" }; + }, + }); + try { + for (const params of [ + {}, + { task: "t" }, + { task: "", expression: "*/5 * * * *" }, + { task: "t", expression: "" }, + { task: "t", expression: "*/5 * * * *", lifecycle: "" }, + { task: "t", expression: "*/5 * * * *", auto: "yes" }, + { task: "t", expression: "*/5 * * * *", isolation: "yolo" }, + { task: "t", expression: "*/5 * * * *", cwd: "/does/not/exist" }, + ]) { + const r = await h.server.handle({ id: "v", verb: "schedule", params }) as { error: { code: string } }; + assert.equal(r.error.code, "E-BAD-PARAMS", JSON.stringify(params)); + assert.equal(registered.length, 0, "nothing registers on validation failure"); + } + const ok = await h.server.handle({ id: "s", verb: "schedule", params: { task: "sweep", expression: "*/5 * * * *", lifecycle: "review", auto: false, isolation: "none" } }) as { ok: true; data: { scheduleId: string; nextFire: string | null } }; + assert.equal(ok.ok, true); + assert.equal(ok.data.scheduleId, "sch-abc"); + assert.equal(ok.data.nextFire, "2026-09-01T00:00:00.000Z"); + assert.equal(registered.length, 1); + assert.equal(registered[0]!.task, "sweep"); + assert.equal(registered[0]!.expression, "*/5 * * * *"); + assert.equal(registered[0]!.lifecycle, "review"); + assert.equal(registered[0]!.auto, false); + assert.equal(registered[0]!.isolation, "none"); + } finally { rmSync(h.dir, { recursive: true, force: true }); } +}); + +test("schedule: scheduler.register throwing (invalid expression) → E-BAD-PARAMS with the parser message (#83)", async () => { + const h = harness({ + schedule: (_spec: Record) => { throw new Error("invalid cron expression: 'nope'"); }, + }); + try { + const r = await h.server.handle({ id: "e", verb: "schedule", params: { task: "t", expression: "nope" } }) as { error: { code: string; message: string } }; + assert.equal(r.error.code, "E-BAD-PARAMS"); + assert.match(r.error.message, /invalid cron expression/); } finally { rmSync(h.dir, { recursive: true, force: true }); } });