From 1b8cdb4462d868eb4365be6066c1f46c699b0347 Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Sat, 19 Sep 2026 19:24:27 +1000 Subject: [PATCH] =?UTF-8?q?feat(loop):=20a=20stopped=20run=20can=20be=20co?= =?UTF-8?q?ntinued=20=E2=80=94=20same=20objective,=20a=20new=20ceiling,=20?= =?UTF-8?q?briefed=20on=20what=20already=20landed=20=E2=80=94=20refs=20#80?= =?UTF-8?q?6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice (ii) of #806: "grant more iterations and continue". Items 1, 2 and 3(b) still wait on the owner decision recorded on the issue; this is the half that needed none. POST /v1/instances/:id/loop/:runId/continue carries a stopped run's objective onto a NEW run. It is not a resumed invocation: a finished agent_loop_runs row has no live Workflow to grant steps to, and #523's option (b) — checkpoint the OBJECTIVE, not the transcript — means there is no transcript to reanimate. The continuity is the resume note, composed at the start of the new run from what the old one left on the record. The route is thin on purpose. driver.start() already picks the repo, opens or reuses the session, runs admission and takes the single-flight claim; assembling any of that here would be a second, quieter copy of the start path. What it decides is only: which runs may be continued (RESUMABLE_STOP_REASONS, refusing everything else with a sentence naming the next step instead), what the new run starts with (the stopped run's objective, its repo via its session, its own step cap unless the caller grants more, clamped by the same #477 account ceiling), and a FRESH budget — #184's rule that every autonomous entry point admits separately, or one admission funds an unbounded chain. The lookback is the whole difference from a restart. #806 item 4 asks that this work "even hours later", and RESUME_NOTE_LOOKBACK_MS is six hours — so a Continue pressed the next morning would have started a run briefed on nothing at all. A continue passes CONTINUE_RESUME_LOOKBACK_MS (30 days) through LoopStartInput → the workflow params → pendingCodingResumeNote → lastUnfinishedRunForRepo. It moves the FLOOR only: the predecessor is still the most recent FINISHED run on the repo, judged afterwards, so a verdict in between still ends the note's job. That is also why the note is NOT pinned to the continued run's id — pinning would skip that check and brief the new run on a checkpoint another run has already consumed. Console: a Continue button beside the ending label, offered only after an ending with no verdict (canContinueRun, pure and tested, with a test that reads the worker's own RESUMABLE_STOP_REASONS so the two lists cannot drift silently). MCP: continue_instance_run, beside start_instance_loop. Not part of #613's sweep — the route is new here, and shipping the console button without a tool would have ADDED a gap to the ratchet #613 is driving to zero. Parity holds at 29 recorded gaps, unchanged. Server 0.1.38 → 0.1.39 with its SURFACE_LOCK entry; 196 → 197 registrations. Verified: 42 new tests (24 route, 13 console-lib, 3 real-schema lookback, 1 driver threading, 1 workflow wiring). Mutation-checked — dropping the lookback at any of its three links, the cross-instance guard, the unfinished-run guard or the ceiling default each turns tests red. Full suite 11,793/11,793; pnpm -r typecheck; biome; docs:drift; all 13 check-*.mjs. Not verified: no live run. That needs a real run stopped for a resumable reason, followed by Continue — agent_trace on the successor shows whether the note fired. Co-Authored-By: Claude Opus 5 (1M context) --- docs/mcp-instance-runtime.md | 4 +- platform-docs/mcp.md | 6 +- .../proagentstore-mcp-operator/SKILL.md | 2 +- .../proagentstore-mcp-operator/SKILL.md | 2 +- scripts/check-file-size.mjs | 10 +- server.json | 2 +- skills/proagentstore-mcp-operator/SKILL.md | 2 +- store/.well-known/mcp-server.json | 2 +- store/about/index.html | 2 +- store/console/src/lib/loopContinue.test.ts | 46 ++++ store/console/src/lib/loopContinue.ts | 45 ++++ store/console/src/tabs/LoopRunsSection.tsx | 44 +++- store/llms-full.txt | 2 +- store/openapi.yaml | 71 ++++++ workers/api/src/lib/agent-loop-store.ts | 27 ++- .../src/lib/coding-resume-note-repo.test.ts | 43 ++++ .../api/src/lib/coding-resume-note.test.ts | 11 +- workers/api/src/lib/coding-resume-note.ts | 9 +- workers/api/src/lib/loop-drivers.test.ts | 19 ++ workers/api/src/lib/loop-drivers.ts | 11 + workers/api/src/lib/pipeline-budget.test.ts | 6 + .../src/routes/loop-continue-routes.test.ts | 225 ++++++++++++++++++ .../api/src/routes/loop-continue-routes.ts | 142 +++++++++++ workers/api/src/routes/tools.ts | 2 + .../src/workflows/coding-session-params.ts | 10 + workers/api/src/workflows/coding-session.ts | 2 +- workers/mcp/AGENTS.md | 2 +- workers/mcp/CLAUDE.md | 8 +- workers/mcp/README.md | 3 +- workers/mcp/src/instance-tools/composition.ts | 38 +++ .../mcp/src/instance-tools/contract.test.ts | 1 + workers/mcp/src/server-version.ts | 2 +- workers/mcp/src/surface-lock.ts | 20 ++ workers/mcp/src/tool-count.ts | 4 +- workers/mcp/src/tool-metadata.ts | 8 +- workers/mcp/src/write-readback.test.ts | 3 + 36 files changed, 805 insertions(+), 31 deletions(-) create mode 100644 store/console/src/lib/loopContinue.test.ts create mode 100644 store/console/src/lib/loopContinue.ts create mode 100644 workers/api/src/routes/loop-continue-routes.test.ts create mode 100644 workers/api/src/routes/loop-continue-routes.ts diff --git a/docs/mcp-instance-runtime.md b/docs/mcp-instance-runtime.md index 583473f1..afe57065 100644 --- a/docs/mcp-instance-runtime.md +++ b/docs/mcp-instance-runtime.md @@ -112,7 +112,7 @@ That response means the instance runtime path is working and correctly refusing ## MCP Tool Groups -The server currently has 196 tool registrations across `workers/mcp/src`. +The server currently has 197 tool registrations across `workers/mcp/src`. Some are capability-gated and appear only for users with matching agent surfaces, so `tools/list` on your own connection is the authoritative surface — 23 of those registrations are gated. The `/health` marker reports the same total from @@ -184,7 +184,7 @@ Status: active Verified live: -- MCP exposed its tool set (26 at the time of this 2026-06-11 record; 196 tool registrations in source today — see MCP Tool Groups above). +- MCP exposed its tool set (26 at the time of this 2026-06-11 record; 197 tool registrations in source today — see MCP Tool Groups above). - Required instance tools were present. - `subscribe_agent` returned the existing active instance. - `add_instance_knowledge` saved a document to that private instance. diff --git a/platform-docs/mcp.md b/platform-docs/mcp.md index bbf084c6..685b1c6f 100644 --- a/platform-docs/mcp.md +++ b/platform-docs/mcp.md @@ -136,7 +136,7 @@ Confirm before destructive actions. ## What `initialize` Answers -- `serverInfo.version`: `0.1.38` +- `serverInfo.version`: `0.1.39` That is the same value the published MCP-registry manifest (`server.json`) carries, and both are read from one constant — `MCP_SERVER_VERSION` in `workers/mcp/src/server-version.ts` — @@ -249,7 +249,7 @@ The two published hints are **derived, not hand-maintained per tool**. `workers/mcp/src/tool-metadata.ts` classifies every tool `read` / `write` / `runtime` / `destructive` in one table, and `annotationsFor()` maps that classification onto the two hints. The classification is then derived **back out of the handlers** by `index.test.ts`, -which drives all 196 tools under two different scope sets and reads the required scope out +which drives all 197 tools under two different scope sets and reads the required scope out of each refusal — so a tool announced read-only that enforces a write gate fails the build rather than reaching a host. `conformance.test.ts` asserts the same thing against a real `tools/list` response. @@ -412,7 +412,7 @@ More recipes, with real argument names, are in ## Tool Surface -The server registers **196 tools**. 172 are always present. The remaining 24 are gated to +The server registers **197 tools**. 173 are always present. The remaining 24 are gated to the console surfaces of the connected user's own subscribed agents, so the surface is per-connection: diff --git a/plugins/claude/proagentstore/skills/proagentstore-mcp-operator/SKILL.md b/plugins/claude/proagentstore/skills/proagentstore-mcp-operator/SKILL.md index 4be2f5a4..d5a77f68 100644 --- a/plugins/claude/proagentstore/skills/proagentstore-mcp-operator/SKILL.md +++ b/plugins/claude/proagentstore/skills/proagentstore-mcp-operator/SKILL.md @@ -83,7 +83,7 @@ the private runtime is working. The instance is correctly refusing to spend plat ## Tool Surface -The server registers 196 tools; call `tools/list` for the current set. The tool table in +The server registers 197 tools; call `tools/list` for the current set. The tool table in `workers/mcp/README.md` (or the published `platform-docs/mcp.md`) lists every tool with its scope, dry-run support, and confirmation value. The always-on `platform_guide` tool returns a plain-text map of the most commonly used ones — call it at the start of a session to orient diff --git a/plugins/codex/proagentstore/skills/proagentstore-mcp-operator/SKILL.md b/plugins/codex/proagentstore/skills/proagentstore-mcp-operator/SKILL.md index 4be2f5a4..d5a77f68 100644 --- a/plugins/codex/proagentstore/skills/proagentstore-mcp-operator/SKILL.md +++ b/plugins/codex/proagentstore/skills/proagentstore-mcp-operator/SKILL.md @@ -83,7 +83,7 @@ the private runtime is working. The instance is correctly refusing to spend plat ## Tool Surface -The server registers 196 tools; call `tools/list` for the current set. The tool table in +The server registers 197 tools; call `tools/list` for the current set. The tool table in `workers/mcp/README.md` (or the published `platform-docs/mcp.md`) lists every tool with its scope, dry-run support, and confirmation value. The always-on `platform_guide` tool returns a plain-text map of the most commonly used ones — call it at the start of a session to orient diff --git a/scripts/check-file-size.mjs b/scripts/check-file-size.mjs index 6854113c..b0b41288 100644 --- a/scripts/check-file-size.mjs +++ b/scripts/check-file-size.mjs @@ -1296,7 +1296,11 @@ const PINS = { // which has no checkout to repair. The brief the flag stands for, and every sentence about it, // live in `lib/repo-sync-gate.ts`; what is here is the one fact only this route knows, which is // that an objective is required EXCEPT on a repair run. - "workers/api/src/routes/tools.ts": 1259, + // +1 at #806: `registerLoopContinueRoutes(toolRoutes)` and its import — `POST + // /:id/loop/:runId/continue`, which carries a stopped run's objective onto a fresh run with a + // new ceiling. The handler is NOT here: it is `routes/loop-continue-routes.ts`, for the same + // reason the queue's is not, which is what keeps this at +1 rather than +100. + "workers/api/src/routes/tools.ts": 1261, // First entry at #477: Usage.tsx crossed 800 lines as BudgetPanel expanded to cover per-tree // run knobs (perTreeCostMicros, perTreeDelegations, perTreeMaxDepth, loopMaxIterations) and // their edit fields. The page is one coherent screen — usage data + the limits that bound it — @@ -1708,7 +1712,9 @@ const PINS = { // why — most of it the boundary that was measured and left alone — plus these three. Same price // as the first split's note directly above. // +5 at #814: the coding-session raise above (four lines of why) and this one. - "scripts/check-file-size.mjs": 1792, + // +6 at #806: the tools.ts raise above (four lines of why, naming where the handler went) + // and these two. + "scripts/check-file-size.mjs": 1798, }; /** diff --git a/server.json b/server.json index c3ed7943..f375b7ad 100644 --- a/server.json +++ b/server.json @@ -3,7 +3,7 @@ "name": "io.github.ProAgentStore/platform", "title": "ProAgentStore", "description": "MCP-first control plane for ProAgentStore agents and private instances.", - "version": "0.1.38", + "version": "0.1.39", "repository": { "url": "https://github.com/ProAgentStore/platform", "source": "github" diff --git a/skills/proagentstore-mcp-operator/SKILL.md b/skills/proagentstore-mcp-operator/SKILL.md index 4be2f5a4..d5a77f68 100644 --- a/skills/proagentstore-mcp-operator/SKILL.md +++ b/skills/proagentstore-mcp-operator/SKILL.md @@ -83,7 +83,7 @@ the private runtime is working. The instance is correctly refusing to spend plat ## Tool Surface -The server registers 196 tools; call `tools/list` for the current set. The tool table in +The server registers 197 tools; call `tools/list` for the current set. The tool table in `workers/mcp/README.md` (or the published `platform-docs/mcp.md`) lists every tool with its scope, dry-run support, and confirmation value. The always-on `platform_guide` tool returns a plain-text map of the most commonly used ones — call it at the start of a session to orient diff --git a/store/.well-known/mcp-server.json b/store/.well-known/mcp-server.json index c3ed7943..f375b7ad 100644 --- a/store/.well-known/mcp-server.json +++ b/store/.well-known/mcp-server.json @@ -3,7 +3,7 @@ "name": "io.github.ProAgentStore/platform", "title": "ProAgentStore", "description": "MCP-first control plane for ProAgentStore agents and private instances.", - "version": "0.1.38", + "version": "0.1.39", "repository": { "url": "https://github.com/ProAgentStore/platform", "source": "github" diff --git a/store/about/index.html b/store/about/index.html index 59fd12f6..4c1268ea 100644 --- a/store/about/index.html +++ b/store/about/index.html @@ -176,7 +176,7 @@

Data isolation

Developer tools

CLI — npm i -g @proagentstore/cli then pags up: one runner serves ALL your agents over a WebSocket relay — no tunnel binary, no config. Plus pags login, pags init, pags check, pags publish.

-

MCP Server — manage agents from Claude Code, Cursor, or VS Code. 196 tools across creator operations, public trials, private instance runtime, coding, and error observability (list_errors).

+

MCP Server — manage agents from Claude Code, Cursor, or VS Code. 197 tools across creator operations, public trials, private instance runtime, coding, and error observability (list_errors).

Skills — install the same ProAgentStore MCP workflow in Codex and Claude Code from the skills catalog.

Webhook ingestion — Zapier, Make, n8n can POST documents directly into an instance's knowledge base.

API key vault — store your OpenAI, Anthropic, Google AI keys once. Encrypted AES-256-GCM. Agents access via proxy.

diff --git a/store/console/src/lib/loopContinue.test.ts b/store/console/src/lib/loopContinue.test.ts new file mode 100644 index 00000000..b1c4f69a --- /dev/null +++ b/store/console/src/lib/loopContinue.test.ts @@ -0,0 +1,46 @@ +/** + * Which ended runs offer Continue (#806), and the one assertion that is not about this file. + * + * The last test reads the WORKER's own `RESUMABLE_STOP_REASONS` and requires the two lists to + * match. The module header explains why a copy is acceptable and which direction of drift is + * survivable; this is what keeps the drift from happening silently anyway. It is a source read + * rather than an import because the console and the worker are separate builds — importing worker + * code into a console test would bring `Env` and the D1 types with it. + */ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { canContinueRun, CONTINUABLE_STOP_REASONS } from "./loopContinue"; + +describe("canContinueRun", () => { + it.each(["interrupted", "max_iterations", "engine_limit", "provider_credit"])("offers Continue after %s", (stopReason) => { + expect(canContinueRun({ status: "failed", stopReason })).toBe(true); + }); + + it.each(["done", "failed", "cancelled", "escalated", "no_progress", "budget"])("does not offer it after %s", (stopReason) => { + expect(canContinueRun({ status: "failed", stopReason })).toBe(false); + }); + + it("never offers it on a run that is still going", () => { + // Belt and braces: a running run carries no stop reason today, but a Continue button + // underneath a live Stop button is the one outcome that must be impossible by construction. + expect(canContinueRun({ status: "running", stopReason: null })).toBe(false); + expect(canContinueRun({ status: "running", stopReason: "max_iterations" })).toBe(false); + }); + + it("says no to a run it knows nothing about", () => { + expect(canContinueRun(null)).toBe(false); + expect(canContinueRun(undefined)).toBe(false); + expect(canContinueRun({ status: "failed" })).toBe(false); + }); +}); + +describe("the copy of the server's list", () => { + it("matches RESUMABLE_STOP_REASONS in the API worker", () => { + const source = readFileSync(new URL("../../../../workers/api/src/lib/agent-loop-store.ts", import.meta.url).pathname, "utf8"); + const declared = /export const RESUMABLE_STOP_REASONS = \[([^\]]*)\]/.exec(source); + expect(declared).not.toBeNull(); + const server = [...(declared as RegExpExecArray)[1].matchAll(/"([a-z_]+)"/g)].map((m) => m[1]); + expect(server.length).toBeGreaterThan(0); + expect([...CONTINUABLE_STOP_REASONS]).toEqual(server); + }); +}); diff --git a/store/console/src/lib/loopContinue.ts b/store/console/src/lib/loopContinue.ts new file mode 100644 index 00000000..e8a0bdce --- /dev/null +++ b/store/console/src/lib/loopContinue.ts @@ -0,0 +1,45 @@ +/** + * Which ended runs offer a Continue button (#806 item 3(c)). + * + * ── The server is the authority, and this is a filter + * + * `RESUMABLE_STOP_REASONS` lives in `workers/api/src/lib/agent-loop-store.ts` and is where the + * decision is made: `POST /:id/loop/:runId/continue` refuses anything else with a 409 that names + * what to do instead. This list exists only so the button is not OFFERED on a run the server will + * refuse — a control that always errors is worse than no control. + * + * So the two lists can drift, and the drift is survivable in one direction only, which is why it is + * worth stating: a reason added to the server and not here costs a button that could have been + * shown; a reason added here and not to the server costs a click that comes back as an explained + * 409. Neither can start work the server would not have started. The console cannot import the + * worker's copy — different build, no shared package — and vendoring is this repo's answer to that + * (see the workspace CLAUDE.md), so a copy with its direction of failure written down beats an + * import that does not exist. + * + * ── Why "ended" is checked separately from the reason + * + * A run still going has no `stopReason` at all, so the reason test alone would be enough today. It + * is checked anyway because the two facts answer different questions — "is this over" and "was it + * over WITHOUT a verdict" — and a run that reports a reason while still running (a cooperative + * cancel settling, a park) must not grow a Continue button underneath its Stop button. + */ + +/** The endings the server will continue. Kept in the order `RESUMABLE_STOP_REASONS` lists them. */ +export const CONTINUABLE_STOP_REASONS = ["interrupted", "max_iterations", "engine_limit", "provider_credit"] as const; + +/** The fields of a loop run that decide whether Continue is offered (`LoopRunView` has more). */ +export interface LoopRunContinueLike { + status: string; + stopReason?: string | null; +} + +/** + * Is this run one the owner can carry on from? + * + * Deliberately not "should we show a Continue button" — the caller decides that, because the same + * answer is wanted in two places that render differently. This is the fact, not the layout. + */ +export function canContinueRun(run: LoopRunContinueLike | null | undefined): boolean { + if (!run || run.status === "running") return false; + return (CONTINUABLE_STOP_REASONS as readonly string[]).includes(run.stopReason ?? ""); +} diff --git a/store/console/src/tabs/LoopRunsSection.tsx b/store/console/src/tabs/LoopRunsSection.tsx index c1f42dcf..d778a067 100644 --- a/store/console/src/tabs/LoopRunsSection.tsx +++ b/store/console/src/tabs/LoopRunsSection.tsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useState } from "react"; import { api } from "@proagentstore/sdk/client"; import { loopStopControl, type LoopPhase } from "../lib/loopStopState"; +import { canContinueRun } from "../lib/loopContinue"; import { activityLabel, isOpen, runActivity, type RunHealth } from "../lib/workInFlight"; import Button from "../components/Button"; import Card from "../components/Card"; @@ -140,6 +141,28 @@ export default function LoopRunsSection({ instanceId }: { instanceId: string }) setBusy(false); }; + /** + * Carry a stopped run's objective onto a fresh run (#806 item 3(c)). + * + * No "how many more iterations" prompt here on purpose: the server defaults to the stopped + * run's own ceiling, so an empty body means "another run of the same size", which is the + * reading that cannot surprise an account's spend. A number belongs with the other loop + * controls, not behind a button whose whole appeal is that it takes one click. + */ + const resume = async (runId: string) => { + setBusy(true); + setMsg(""); + try { + await api(`/v1/instances/${instanceId}/loop/${runId}/continue`, { method: "POST", body: "{}" }); + await load(); + } catch (e) { + // The server's refusals NAME what to do instead ("read its outcome first", "run `pags + // up`"), so they are shown verbatim rather than replaced with a generic failure. + setMsg(e instanceof Error ? e.message : String(e)); + } + setBusy(false); + }; + if (runs.length === 0) return null; return ( @@ -166,8 +189,25 @@ export default function LoopRunsSection({ instanceId }: { instanceId: string }) {ctl.actionLabel} ) : ( - - {REASON_LABEL[r.stopReason ?? ""] ?? r.status} + + + {REASON_LABEL[r.stopReason ?? ""] ?? r.status} + + {/* + * Offered only after an ending with no verdict (#806). The reason label stays + * beside it: "Hit the step limit · Continue" is the sentence the owner needs, + * and a button alone would not say what it is continuing FROM. + */} + {canContinueRun(r) && ( + + )} )} diff --git a/store/llms-full.txt b/store/llms-full.txt index 15b44a3c..38932bb4 100644 --- a/store/llms-full.txt +++ b/store/llms-full.txt @@ -156,7 +156,7 @@ That response means the private instance path is working and correctly refusing ## Main MCP Tool Groups -The server registers 196 tools; 172 are always present and 24 are gated to the `apply`, `repo`, and `coding` surfaces. The lists below are the commonly used subset, not the full surface — call `tools/list` for the authoritative set, and see the tool table in `workers/mcp/README.md` for scopes, `dry_run` support, and confirmation values. +The server registers 197 tools; 173 are always present and 24 are gated to the `apply`, `repo`, and `coding` surfaces. The lists below are the commonly used subset, not the full surface — call `tools/list` for the authoritative set, and see the tool table in `workers/mcp/README.md` for scopes, `dry_run` support, and confirmation values. Creator tools: diff --git a/store/openapi.yaml b/store/openapi.yaml index 11cfb2cc..c9ad27d7 100644 --- a/store/openapi.yaml +++ b/store/openapi.yaml @@ -8019,6 +8019,77 @@ paths: application/json: schema: { $ref: "#/components/schemas/Error" } + /v1/instances/{instanceId}/loop/{runId}/continue: + post: + summary: Continue a stopped loop run + description: | + Carries a STOPPED run's objective onto a NEW run, briefed on what the stopped one + already landed. It is not a resumed invocation: a finished run has no live workflow to + grant steps to, and the platform checkpoints the OBJECTIVE rather than the transcript. + The continuity is the resume note, composed at the start of the new run from the acts + the old one left on the record and the state of the working tree. + + Only for a run that ended WITHOUT reaching a verdict on its objective — `interrupted`, + `max_iterations`, `engine_limit`, `provider_credit`. Anything else is a 409 naming what + to do instead, because each is a different next step: a finished run wants a new + objective, a failed one wants its outcome read first, and a cancelled one was a human's + own decision that continuing would quietly undo. + + The new run reuses the stopped run's repository and, unless `maxIterations` says + otherwise, its step cap. It opens its OWN budget — the stopped run's is not inherited, + so a single admission cannot fund an unbounded chain of continues — and it reaches + further back for its predecessor than an ordinary start does, so continuing the next + morning still produces a briefing. + operationId: continueLoopRun + tags: [Tools] + parameters: + - { $ref: "#/components/parameters/InstanceId" } + - { name: runId, in: path, required: true, schema: { type: string }, description: The stopped run to continue. } + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + maxIterations: + type: integer + description: > + Steps the new run may take. Omit to reuse the stopped run's own cap. Clamped + to the account ceiling, so a continue is not a way around it. + budget: + type: object + properties: + costMicros: { type: integer } + delegations: { type: integer } + maxDepth: { type: integer } + responses: + "201": + description: The new run + content: + application/json: + schema: + type: object + properties: + runId: { type: string } + driver: { type: string } + budgetId: { type: string } + maxIterations: { type: integer } + status: { type: string, example: running } + continuedFromRunId: + type: string + description: The stopped run this one carries the objective of. + "401": { $ref: "#/components/responses/Unauthorized" } + "404": { $ref: "#/components/responses/NotFound" } + "409": + description: > + The run is still going, or it reached a verdict and is not continuable — the message + names which, and what to do instead. Also the driver's own refusals (no runner, an + unusable checkout, the repo already being worked on). + content: + application/json: + schema: { $ref: "#/components/schemas/Error" } + /v1/instances/{instanceId}/loop-presets: get: summary: Read this instance's loop presets diff --git a/workers/api/src/lib/agent-loop-store.ts b/workers/api/src/lib/agent-loop-store.ts index 4cea172e..a8ac91d7 100644 --- a/workers/api/src/lib/agent-loop-store.ts +++ b/workers/api/src/lib/agent-loop-store.ts @@ -256,6 +256,24 @@ export async function listLoopRuns(env: Env, userId: string, instanceId: string, */ export const RESUME_NOTE_LOOKBACK_MS = 6 * 60 * 60 * 1000; +/** + * The same bound, widened, for a run the owner asked to CONTINUE (#806 item 4). + * + * Six hours is the right default because an ordinary start has no reason to think the last run is + * related to it, and archaeology is the likelier read. A continue is the opposite case: a human + * looked at one specific stopped run and said "carry on with that one", which is exactly the + * evidence the default has to do without. #806's fourth requirement is that this works "even hours + * later — the point is a human can check back on a stalled run on their own schedule", and under + * the default a Continue pressed the next morning starts a run briefed on nothing at all, which is + * the whole thing the button was for. + * + * Widening is safe because it does NOT widen what may be said. The predecessor is still the single + * most recent FINISHED run on the repo and is still judged afterwards, so a run that reached a + * verdict in between still ends the note's job ({@link lastUnfinishedRunForRepo}) — thirty days + * buys reach back to the run the owner is pointing at, and no additional claim. + */ +export const CONTINUE_RESUME_LOOKBACK_MS = 30 * 24 * 60 * 60 * 1000; + /** * The stop reasons after which a successor is told what already landed (#523 item 4, #806). * @@ -317,6 +335,12 @@ export function isResumableStopReason(reason: string | null | undefined): reason * Ordered and bounded on `started_at` so it rides `idx_agent_loop_runs_instance` directly; see * {@link RESUME_NOTE_LOOKBACK_MS} for why the floor is not optional. `r.*`, not `*`: the join * brings `coding_sessions`' own `status` and `started_at`, which would overwrite the run's. + * + * `lookbackMs` overrides that floor for the one caller who has evidence the default cannot have — + * a human pointing at a specific stopped run and pressing Continue ({@link + * CONTINUE_RESUME_LOOKBACK_MS}). It moves the FLOOR only. Which run is chosen, and whether it may + * be spoken about at all, are decided below and are unaffected by how far back we were willing to + * look for it. */ export async function lastUnfinishedRunForRepo( env: Env, @@ -324,6 +348,7 @@ export async function lastUnfinishedRunForRepo( instanceId: string, sessionId: string, now: number = Date.now(), + lookbackMs: number = RESUME_NOTE_LOOKBACK_MS, ): Promise<(LoopRunView & { stopReason: ResumableStopReason }) | null> { const row = await env.DB.prepare( `SELECT r.* FROM agent_loop_runs r @@ -334,7 +359,7 @@ export async function lastUnfinishedRunForRepo( AND r.started_at >= ?4 ORDER BY r.started_at DESC LIMIT 1`, ) - .bind(userId, instanceId, sessionId, now - RESUME_NOTE_LOOKBACK_MS) + .bind(userId, instanceId, sessionId, now - lookbackMs) .first(); // The predecessor exists but reached a verdict — nothing to hand forward. See the header. if (!row || !isResumableStopReason(row.stop_reason)) return null; diff --git a/workers/api/src/lib/coding-resume-note-repo.test.ts b/workers/api/src/lib/coding-resume-note-repo.test.ts index 122b0436..e85acfa2 100644 --- a/workers/api/src/lib/coding-resume-note-repo.test.ts +++ b/workers/api/src/lib/coding-resume-note-repo.test.ts @@ -12,6 +12,7 @@ */ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { realSchemaD1, seedTenant, type RealSchemaD1 } from "./d1-sqlite.js"; +import { CONTINUE_RESUME_LOOKBACK_MS } from "./agent-loop-store.js"; import { pendingCodingResumeNote } from "./coding-resume-note.js"; import type { Env } from "../types.js"; @@ -142,3 +143,45 @@ describe("the successor of a run-opened session is on a NEW session (#806)", () expect(await pendingCodingResumeNote(env, { userId: "u1", instanceId: "inst-1", sessionId: "s2" }, NOW + 7 * 60 * MIN)).toBeNull(); }); }); + +/** + * The floor a CONTINUE moves, and the rule it does not (#806 item 3(c), item 4). + * + * The default six hours is right for an ordinary start, which has no reason to think the last run + * is related to it. A continue has the one thing that default cannot have: a human looked at a + * specific stopped run and asked for it. #806 item 4 is explicit that this must work "even hours + * later", and the test above is what that requirement fails against today. + */ +describe("a CONTINUE reaches further back for its predecessor", () => { + const NEXT_MORNING = NOW + 14 * 60 * MIN; + + it("briefs a run continued the next morning, which the default floor would not", async () => { + session("s2", "repo-r", "active"); + const params = { userId: "u1", instanceId: "inst-1", sessionId: "s2" }; + expect(await pendingCodingResumeNote(env, params, NEXT_MORNING), "the default floor should still refuse this").toBeNull(); + const note = await pendingCodingResumeNote(env, { ...params, lookbackMs: CONTINUE_RESUME_LOOKBACK_MS }, NEXT_MORNING); + expect(note, "the widened lookback never reached the query").not.toBeNull(); + expect(note).toContain("a previous run on this repository used up its step limit"); + expect(note).toContain("pushed directly to the trunk origin main"); + }); + + it("widens the SEARCH and nothing else — a verdict in between still ends the note's job", async () => { + // The rule that makes pinning-to-a-run-id wrong (see `loop-continue-routes.ts`). Run B + // consumed A's checkpoint and finished; a continue pressed the next morning must not be + // re-briefed on work that is already on the trunk, however far back it is willing to look. + session("s2", "repo-r", "ended"); + run("run-b", "s2", { startedAt: NOW - 20 * MIN, finishedAt: NOW - 5 * MIN, stopReason: "done" }); + session("s4", "repo-r", "active"); + const note = await pendingCodingResumeNote(env, { userId: "u1", instanceId: "inst-1", sessionId: "s4", lookbackMs: CONTINUE_RESUME_LOOKBACK_MS }, NEXT_MORNING); + expect(note).toBeNull(); + }); + + it("does not widen it for anyone who did not ask — the default is unchanged", async () => { + // Byte-for-byte the pre-#806-slice-(ii) behaviour for every ordinary start: the note exists + // inside six hours and does not exist outside it, with no `lookbackMs` passed at all. + session("s2", "repo-r", "active"); + const params = { userId: "u1", instanceId: "inst-1", sessionId: "s2" }; + expect(await pendingCodingResumeNote(env, params, NOW)).toContain("used up its step limit"); + expect(await pendingCodingResumeNote(env, params, NOW + 7 * 60 * MIN)).toBeNull(); + }); +}); diff --git a/workers/api/src/lib/coding-resume-note.test.ts b/workers/api/src/lib/coding-resume-note.test.ts index f3d9996f..ae43ab01 100644 --- a/workers/api/src/lib/coding-resume-note.test.ts +++ b/workers/api/src/lib/coding-resume-note.test.ts @@ -291,7 +291,16 @@ describe("the wiring — the defect a unit test of this module cannot see", () = it("asks for the note, in its own durable step", () => { expect(source).toContain('step.do("resume-note"'); - expect(source).toContain("pendingCodingResumeNote(env, { userId, instanceId, sessionId, uncommittedFiles: repair ? 0 : (repoState?.changedFiles ?? 0) })"); + expect(source).toContain("pendingCodingResumeNote(env, { userId, instanceId, sessionId, uncommittedFiles: repair ? 0 : (repoState?.changedFiles ?? 0), lookbackMs: event.payload.resumeLookbackMs })"); + }); + + it("passes the run's own lookback through, so a CONTINUE reaches its predecessor (#806)", () => { + // The last link in the chain, and the only one no unit test can reach: the route widens the + // lookback, the driver puts it on the workflow params, and if this call drops it the + // continue is silently an ordinary restart — briefing nothing, with nothing going red. + // `event.payload`, because the workflow destructures only some of its params and + // `resumeLookbackMs` is not among them. + expect(source).toContain("lookbackMs: event.payload.resumeLookbackMs"); }); it("hands it the start-of-run tree count — and none to a repair run, whose brief already carries it (#806, #804)", () => { diff --git a/workers/api/src/lib/coding-resume-note.ts b/workers/api/src/lib/coding-resume-note.ts index e5364cf6..4b0f6ed9 100644 --- a/workers/api/src/lib/coding-resume-note.ts +++ b/workers/api/src/lib/coding-resume-note.ts @@ -182,14 +182,19 @@ export function codingResumeNote(acts: ReadonlyArray, endedBy: Resumabl * Never throws: this sits on the start path of every coding run, and a resume note is an * IMPROVEMENT to a run that is otherwise fine. A failed read here must cost the briefing, not the * run — so the caller gets `null` and the run starts exactly as it did before #523. + * + * `lookbackMs` is how far back the predecessor may be, and is passed only by a CONTINUE (#806 item + * 4): the owner named one stopped run, possibly the morning after it stopped, and the default + * six-hour floor would have briefed that run's successor on nothing. It widens the SEARCH and + * nothing else — every rule about what the note may claim is downstream of it and untouched. */ export async function pendingCodingResumeNote( env: Env, - params: { userId: string; instanceId: string; sessionId: string; uncommittedFiles?: number }, + params: { userId: string; instanceId: string; sessionId: string; uncommittedFiles?: number; lookbackMs?: number }, now: number = Date.now(), ): Promise { try { - const prev = await lastUnfinishedRunForRepo(env, params.userId, params.instanceId, params.sessionId, now); + const prev = await lastUnfinishedRunForRepo(env, params.userId, params.instanceId, params.sessionId, now, params.lookbackMs); if (!prev) return null; // That run's OWN session over the interval it drove it (#809) — not `params.sessionId`, which since // #806 is usually a different, later session of the same repo. diff --git a/workers/api/src/lib/loop-drivers.test.ts b/workers/api/src/lib/loop-drivers.test.ts index 7aeb8eb4..0f04b3d4 100644 --- a/workers/api/src/lib/loop-drivers.test.ts +++ b/workers/api/src/lib/loop-drivers.test.ts @@ -140,6 +140,25 @@ describe("every driver opens an agent_loop_runs row — the fact that makes ONE expect((b.created[0].params.goal as { repairCheckout?: boolean }).repairCheckout).toBeUndefined(); }); + it("the coding driver threads a CONTINUE's widened resume lookback to the Pilot (#806)", async () => { + // The note is composed inside the workflow, so a lookback the route held and did not pass + // would reach nothing — the continue would silently be an ordinary restart, and the only + // visible difference is a briefing that never appears. + const stub = () => + stubEnv({ + repos: [{ id: "r1", name: "fws/platform", instance_id: "i1", user_id: "u1", clone_status: "ready" }], + session: { id: "s1", client_type: "claude", status: "active" }, + }); + const a = stub(); + await loopDriverFor(caps("CODING_SESSION")).start({ env: a.env, ...base, resumeLookbackMs: 777 }); + expect(a.created[0].params.resumeLookbackMs).toBe(777); + const b = stub(); + await loopDriverFor(caps("CODING_SESSION")).start({ env: b.env, ...base }); + // Absent, not zero: the workflow falls back to the six-hour default, which is every + // ordinary start and must stay byte-identical. + expect(b.created[0].params.resumeLookbackMs).toBeUndefined(); + }); + it("the coding driver does, and threads the SAME run id into the Pilot", async () => { const { env, sql, created } = stubEnv({ repos: [{ id: "r1", name: "fws/platform", instance_id: "i1", user_id: "u1", clone_status: "ready" }], diff --git a/workers/api/src/lib/loop-drivers.ts b/workers/api/src/lib/loop-drivers.ts index 2ceb1927..ecf354b0 100644 --- a/workers/api/src/lib/loop-drivers.ts +++ b/workers/api/src/lib/loop-drivers.ts @@ -67,6 +67,15 @@ export interface LoopStartInput { * Only the coding driver knows what a checkout is; the route refuses it for any other. */ repairCheckout?: boolean; + /** + * How far back this run's resume note may look for the run it continues (#806 item 4). + * + * Set only by `POST /:id/loop/:runId/continue`, where a human named one stopped run and the + * default six-hour floor would brief the successor on nothing. Threaded rather than decided + * here because only the workflow composes the note, and only the route knows a human chose. + * The chat driver ignores it: a chat loop has no repository and no resume note to widen. + */ + resumeLookbackMs?: number; } export type LoopStartResult = @@ -332,6 +341,8 @@ const codingDriver: LoopDriver = { // licenses the Pilot to close it. A session the user opened by hand outlives the // run — taking it away is what made delegation single-use. sessionOpenedByRun: opened, + // #806: a CONTINUE reaches further back for its predecessor than an ordinary start. + resumeLookbackMs: input.resumeLookbackMs, }, }); // The last of the Pilot's entry points (#556). Everything the Workflow subsequently drives diff --git a/workers/api/src/lib/pipeline-budget.test.ts b/workers/api/src/lib/pipeline-budget.test.ts index e8121c79..fe9e4716 100644 --- a/workers/api/src/lib/pipeline-budget.test.ts +++ b/workers/api/src/lib/pipeline-budget.test.ts @@ -91,6 +91,12 @@ describe("BUDGET_OPENING_TOOLS", () => { // `startJobApply` — every application is a root and every application is unattended, // so unlike the Pilot there is no human-is-watching path to leave unpooled (#516). "routes/instances-apply.ts", + // `POST …/loop/:runId/continue` (#806) — carrying a stopped run's objective onto a new + // run. A NEW pool, deliberately: the stopped run's is spent, and inheriting an unspent + // one would let a single admission fund an unbounded chain of continues — the same + // argument the objective queue's drain makes above. A root, so depth 0; the button is + // the owner's. + "routes/loop-continue-routes.ts", ].sort()); }); }); diff --git a/workers/api/src/routes/loop-continue-routes.test.ts b/workers/api/src/routes/loop-continue-routes.test.ts new file mode 100644 index 00000000..59ae4bf8 --- /dev/null +++ b/workers/api/src/routes/loop-continue-routes.test.ts @@ -0,0 +1,225 @@ +/** + * `POST /:id/loop/:runId/continue` (#806 item 3(c)) — the guards, and the two things the route + * hands downstream that nothing else can check. + * + * ── What is worth testing here, and what is not + * + * Not the start. `driver.start()` is mocked, because picking the repo, opening the session, taking + * the single-flight claim and refusing an unusable checkout are `loop-drivers.ts`'s job and have + * their own tests. Re-asserting them through this route would test the mock. + * + * What only this route decides is: WHICH runs may be continued, what the new run is started WITH + * (objective, ceiling, repo, a fresh budget), and that the resume-note lookback is widened — the + * last of which is the entire difference between a continue and a plain restart, and is invisible + * in the response. So it is asserted on the `start` call. + */ +import { Hono } from "hono"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { HttpError } from "../lib/auth.js"; +import { CONTINUE_RESUME_LOOKBACK_MS } from "../lib/agent-loop-store.js"; +import type { Env } from "../types.js"; + +const getLoopRun = vi.fn(); +const requireOwnedInstance = vi.fn(); +const getSession = vi.fn(); +const openBudget = vi.fn(); +const resolveAccountCeilings = vi.fn(); +const capabilitiesForInstance = vi.fn(); +const start = vi.fn(); + +vi.mock("../lib/auth.js", async () => { + const actual = await vi.importActual("../lib/auth.js"); + return { ...actual, requireUser: async () => ({ uid: "u1" }) }; +}); +vi.mock("../lib/agent-loop-store.js", async () => { + // The real `isResumableStopReason` and the real lookback constant: which stop reasons are + // continuable is exactly what these tests are about, and a mocked predicate would let the + // route and `RESUMABLE_STOP_REASONS` drift apart without a single test going red. + const actual = await vi.importActual("../lib/agent-loop-store.js"); + return { ...actual, getLoopRun: (...a: unknown[]) => getLoopRun(...a) }; +}); +vi.mock("../lib/coding-store.js", () => ({ getSession: (...a: unknown[]) => getSession(...a) })); +vi.mock("../lib/delegation-budget-store.js", () => ({ + openBudget: (...a: unknown[]) => openBudget(...a), + resolveAccountCeilings: (...a: unknown[]) => resolveAccountCeilings(...a), +})); +vi.mock("../lib/agent-capabilities.js", () => ({ capabilitiesForInstance: (...a: unknown[]) => capabilitiesForInstance(...a) })); +vi.mock("../lib/loop-drivers.js", () => ({ loopDriverFor: () => ({ id: "coding", label: "x", start }) })); +vi.mock("./instances-runtime.js", () => ({ requireOwnedInstance: (...a: unknown[]) => requireOwnedInstance(...a) })); + +const { registerLoopContinueRoutes } = await import("./loop-continue-routes.js"); + +function app() { + const router = new Hono<{ Bindings: Env }>(); + registerLoopContinueRoutes(router); + router.onError((e, c) => (e instanceof HttpError ? c.json({ error: e.message }, e.status as 404) : c.json({ error: String(e) }, 500))); + return router; +} + +/** A run that stopped at its step limit — #806's own case, and the happy path everywhere below. */ +const stoppedRun = { + runId: "run-1", + instanceId: "i1", + objective: "finish the migration", + status: "failed", + stopReason: "max_iterations", + iteration: 20, + maxIterations: 20, + startedAt: 1_000, + finishedAt: 2_000, + sessionId: "csess_a", +}; + +const post = (path: string, body?: unknown) => + app().request(path, { method: "POST", body: body === undefined ? undefined : JSON.stringify(body), headers: { "content-type": "application/json" } }, {} as Env); + +beforeEach(() => { + vi.clearAllMocks(); + requireOwnedInstance.mockResolvedValue(undefined); + getLoopRun.mockResolvedValue({ ...stoppedRun }); + getSession.mockResolvedValue({ id: "csess_a", repoId: "repo-7" }); + openBudget.mockResolvedValue({ id: "budget-new" }); + resolveAccountCeilings.mockResolvedValue({ loopMaxIterations: 50 }); + capabilitiesForInstance.mockResolvedValue({ workflow: "CODING_SESSION" }); + start.mockResolvedValue({ ok: true, runId: "run-2", driver: "coding" }); +}); + +describe("the happy path", () => { + it("starts a new run on the stopped one's objective and answers 201", async () => { + const res = await post("/i1/loop/run-1/continue", {}); + expect(res.status).toBe(201); + expect(await res.json()).toEqual({ + runId: "run-2", + driver: "coding", + budgetId: "budget-new", + maxIterations: 20, + status: "running", + // The lineage is reported so a caller can tell a continue's run from a fresh one. + continuedFromRunId: "run-1", + }); + expect(start.mock.calls[0][0]).toMatchObject({ instanceId: "i1", userId: "u1", objective: "finish the migration", depth: 0 }); + }); + + it("widens the resume-note lookback — the whole difference from a restart", async () => { + await post("/i1/loop/run-1/continue", {}); + expect(start.mock.calls[0][0].resumeLookbackMs).toBe(CONTINUE_RESUME_LOOKBACK_MS); + // Not merely "a number": the default floor is six hours, and a continue pressed the next + // morning under it would brief its successor on nothing, which is #806 item 4. + expect(CONTINUE_RESUME_LOOKBACK_MS).toBeGreaterThan(24 * 60 * 60 * 1000); + }); + + it("continues on the stopped run's own repo, not the driver's first one", async () => { + await post("/i1/loop/run-1/continue", {}); + expect(getSession).toHaveBeenCalledWith({}, "i1", "u1", "csess_a"); + expect(start.mock.calls[0][0].repoId).toBe("repo-7"); + }); + + it("opens a FRESH budget rather than inheriting the stopped run's", async () => { + await post("/i1/loop/run-1/continue", { budget: { costMicros: 5 } }); + expect(openBudget).toHaveBeenCalledWith({}, "u1", "i1", { costMicros: 5 }); + expect(start.mock.calls[0][0].budgetId).toBe("budget-new"); + }); + + it("passes no repo for a chat run, which has no session", async () => { + getLoopRun.mockResolvedValue({ ...stoppedRun, sessionId: null }); + await post("/i1/loop/run-1/continue", {}); + expect(getSession).not.toHaveBeenCalled(); + expect(start.mock.calls[0][0].repoId).toBeUndefined(); + }); +}); + +describe("the iteration ceiling", () => { + it("defaults to the stopped run's own ceiling when the caller names no number", async () => { + await post("/i1/loop/run-1/continue", {}); + expect(start.mock.calls[0][0].maxIterations).toBe(20); + }); + + it("takes the caller's number when they grant more", async () => { + const res = await post("/i1/loop/run-1/continue", { maxIterations: 40 }); + expect(start.mock.calls[0][0].maxIterations).toBe(40); + expect((await res.json() as { maxIterations: number }).maxIterations).toBe(40); + }); + + it("clamps to the account ceiling — a continue is not a way around #477", async () => { + resolveAccountCeilings.mockResolvedValue({ loopMaxIterations: 25 }); + await post("/i1/loop/run-1/continue", { maxIterations: 900 }); + expect(start.mock.calls[0][0].maxIterations).toBe(25); + }); +}); + +describe("which runs may be continued", () => { + it("404s when there is no such run", async () => { + getLoopRun.mockResolvedValue(null); + const res = await post("/i1/loop/nope/continue", {}); + expect(res.status).toBe(404); + expect(start).not.toHaveBeenCalled(); + }); + + it("404s when the run belongs to another instance of the same owner", async () => { + // `getLoopRun` is user-scoped but not instance-scoped, so without this guard the new run + // would open on whichever instance was in the URL. + getLoopRun.mockResolvedValue({ ...stoppedRun, instanceId: "i2" }); + const res = await post("/i1/loop/run-1/continue", {}); + expect(res.status).toBe(404); + expect(start).not.toHaveBeenCalled(); + }); + + it("409s while the run is still going, and says so", async () => { + getLoopRun.mockResolvedValue({ ...stoppedRun, status: "running", stopReason: null, finishedAt: null }); + const res = await post("/i1/loop/run-1/continue", {}); + expect(res.status).toBe(409); + expect((await res.json() as { error: string }).error).toContain("still going"); + expect(start).not.toHaveBeenCalled(); + }); + + it("409s on a run that is cancelling but has not stopped yet", async () => { + // `cancelRequested` is cooperative: the run keeps going until the top of its next + // iteration. Reading it as stopped would start a second run against the same session, + // which is the single-flight collision #208 exists to prevent. + getLoopRun.mockResolvedValue({ ...stoppedRun, status: "running", stopReason: "max_iterations", finishedAt: null, cancelRequested: true }); + const res = await post("/i1/loop/run-1/continue", {}); + expect(res.status).toBe(409); + // Refused for being UNFINISHED, not for its reason — a run carrying a continuable reason + // while still in flight is exactly the case the reason test alone would wave through. + expect((await res.json() as { error: string }).error).toContain("still going"); + expect(start).not.toHaveBeenCalled(); + }); + + it.each([ + ["done", "new one"], + ["failed", "read its outcome"], + ["cancelled", "clean start"], + ["escalated", "handoff"], + ["no_progress", "repeating"], + ["budget", "spend limit"], + ])("409s on a %s run, naming what to do instead", async (reason, phrase) => { + getLoopRun.mockResolvedValue({ ...stoppedRun, stopReason: reason }); + const res = await post("/i1/loop/run-1/continue", {}); + expect(res.status).toBe(409); + expect((await res.json() as { error: string }).error).toContain(phrase); + expect(start).not.toHaveBeenCalled(); + }); + + it.each(["interrupted", "max_iterations", "engine_limit", "provider_credit"])("continues a %s run", async (reason) => { + getLoopRun.mockResolvedValue({ ...stoppedRun, stopReason: reason }); + expect((await post("/i1/loop/run-1/continue", {})).status).toBe(201); + }); + + it("is owner-scoped before it reads the run", async () => { + requireOwnedInstance.mockRejectedValue(new HttpError(404, "instance not found")); + const res = await post("/i1/loop/run-1/continue", {}); + expect(res.status).toBe(404); + expect(getLoopRun).not.toHaveBeenCalled(); + }); +}); + +describe("when the driver refuses", () => { + it("passes the driver's own status and sentence through", async () => { + // The refusal names the real blocker — a runner that is off, a checkout that failed + // admission. Replacing it with a generic "could not continue" is what #271 was about. + start.mockResolvedValue({ ok: false, status: 409, error: "repo-7 is already being worked on — wait for the current run to finish, or stop it first with stop_work." }); + const res = await post("/i1/loop/run-1/continue", {}); + expect(res.status).toBe(409); + expect((await res.json() as { error: string }).error).toContain("already being worked on"); + }); +}); diff --git a/workers/api/src/routes/loop-continue-routes.ts b/workers/api/src/routes/loop-continue-routes.ts new file mode 100644 index 00000000..1f8e990f --- /dev/null +++ b/workers/api/src/routes/loop-continue-routes.ts @@ -0,0 +1,142 @@ +// Continue a run that stopped without reaching a verdict (#806 item 3(c)). +// +// A helper module for the same reason `loop-queue-routes.ts` is one: `routes/tools.ts` is the +// largest route file in the tree and pinned by the #302 ratchet, so what lands there is the two +// lines that register this. +// +// ── What "continue" is, and what it is not +// +// It is a NEW run, carrying the stopped run's objective onto the same repository, with the +// iteration ceiling the owner chose. It is not a resumed invocation: a finished `agent_loop_runs` +// row has no live Workflow to grant steps to, and #523's recorded choice — option (b), checkpoint +// the OBJECTIVE, not the transcript — means there is no transcript to reanimate even in principle. +// The continuity the owner actually gets is the resume note (`lib/coding-resume-note.ts`), which is +// composed at the start of the new run from what the old one left on the record. +// +// That is why this route is thin, and why it must be: every guarantee it appears to offer is +// already owned by something else. `driver.start()` picks the repo, opens or reuses the session, +// runs the admission checks and takes the single-flight claim; the workflow composes the briefing. +// A continue that assembled any of that itself would be a second, quieter copy of the start path — +// which is precisely how `delegate-instance.ts`'s duplicate lost `onBehalfOf` (see +// `LoopStartInput.onBehalfOf`). +// +// ── Why the note is not pinned to the run being continued +// +// The obvious design is to hand the new run the id of the run it continues and brief it on THAT. +// It is wrong in the one case that matters. `lastUnfinishedRunForRepo` deliberately takes the most +// recent finished run on the repo and only THEN asks what ended it, so that a run which reached a +// verdict in between ends the note's job (its header sets this out in full). Pinning would skip +// that check and brief the new run on a checkpoint another run has already consumed — re-doing work +// that is on the trunk, which is #523's opening complaint pointed the other way. So the continue +// widens the SEARCH ({@link CONTINUE_RESUME_LOOKBACK_MS}) and changes nothing about the choosing. +// In the ordinary case the run the owner pressed the button on IS the most recent finished run on +// its repo, and it is the one that gets quoted. +// +// ── The budget is new +// +// Not the stopped run's. #184's rule is that every autonomous entry point admits separately, and +// `POST /:id/loop` says so twice in its own queue branch. Inheriting the spent pool of the run that +// just stopped would be a continue that cannot pay for itself, and inheriting an unspent one would +// let a single admission fund an unbounded chain of continues. + +import type { Hono } from "hono"; +import { HttpError, requireUser } from "../lib/auth.js"; +import { CONTINUE_RESUME_LOOKBACK_MS, getLoopRun, isResumableStopReason } from "../lib/agent-loop-store.js"; +import { sanitizeMaxIterations } from "../lib/agent-loop.js"; +import { capabilitiesForInstance } from "../lib/agent-capabilities.js"; +import { getSession } from "../lib/coding-store.js"; +import { loopDriverFor } from "../lib/loop-drivers.js"; +import { openBudget, resolveAccountCeilings } from "../lib/delegation-budget-store.js"; +import { requireOwnedInstance } from "./instances-runtime.js"; +import type { Env } from "../types.js"; + +/** + * Why a run cannot be continued, in the owner's words rather than the enum's. + * + * Each of these is a DIFFERENT next step, which is the whole reason they are not one sentence: a + * `done` run needs a new objective, a `failed` one needs the failure read first, a `cancelled` one + * was the owner's own decision and continuing it silently would undo that decision. The set that + * IS continuable is `RESUMABLE_STOP_REASONS`, and it is not restated here — one list, one place. + */ +const REFUSAL: Record = { + done: "that run finished its objective — start a new one rather than continuing it", + failed: "that run failed and said why; read its outcome first, because repeating its steps may be exactly wrong", + cancelled: "you stopped that run — start a fresh one, which is the clean start continuing would quietly skip", + escalated: "that run asked a human a question and is answered through its own handoff, not by continuing it", + no_progress: "that run was repeating itself; continuing it would repeat it again — change the objective instead", + budget: "that run hit its spend limit; raise the limit before starting more work on it", +}; + +export function registerLoopContinueRoutes(router: Hono<{ Bindings: Env }>): void { + /** + * Carry a stopped run's objective onto a fresh run with a new iteration ceiling. + * + * 409 rather than 400 throughout: the request is well-formed and the run exists, but its state + * says no. A caller that reads 400 for this goes looking for a bug in its own body. + */ + router.post("/:id/loop/:runId/continue", async (c) => { + const session = await requireUser(c); + const instanceId = c.req.param("id"); + await requireOwnedInstance(c.env, instanceId, session.uid); + const runId = c.req.param("runId"); + + // `getLoopRun` is user-scoped but not instance-scoped, so the instance is checked here — + // otherwise an owner with two agents could continue one agent's run through the other's + // URL and the new run would open on the wrong instance entirely. The same guard, for the + // same reason, is on `GET /:id/loop/:runId`'s sibling in `tool-registry.ts`. + const run = await getLoopRun(c.env, session.uid, runId); + if (!run || run.instanceId !== instanceId) throw new HttpError(404, "loop run not found"); + + // STILL GOING is a different fact from "cannot be continued", and conflating them sends the + // owner to the wrong control. `finishedAt`, not `status`: a run with `cancelRequested` set + // is still running until it reaches the top of its next iteration, and starting a second + // run against the same session in that window is the single-flight collision #208 is about. + if (run.finishedAt === null) throw new HttpError(409, "that run is still going — stop it first, then continue it"); + if (!isResumableStopReason(run.stopReason)) { + throw new HttpError(409, REFUSAL[run.stopReason ?? ""] ?? "that run reached a verdict on its objective and cannot be continued"); + } + + const body = (await c.req.json().catch(() => ({}))) as { + maxIterations?: number; + budget?: { costMicros?: number; delegations?: number; maxDepth?: number }; + }; + + // The stopped run's own ceiling is the DEFAULT, not a floor to add to. "Grant more + // iterations" is what the owner asks for by naming a number; pressing Continue with an + // empty body asks for another run of the same size, which is the conservative reading and + // the one that cannot surprise an account's spend. Clamped by the same per-account ceiling + // `POST /:id/loop` uses (#477) — a continue must not be a way around it. + const maxIterations = sanitizeMaxIterations(body.maxIterations ?? run.maxIterations, (await resolveAccountCeilings(c.env, session.uid)).loopMaxIterations); + + // The repo the stopped run was on, so a multi-repo Coder continues the right checkout + // rather than `repos[0]` (#374). Null for a chat run, which has no session and no repo — + // the chat driver ignores `repoId`, so a chat loop continues correctly by doing nothing + // special here. A session that has since been deleted also lands on undefined, which + // degrades to "you pick" rather than to a failure. + const repoId = run.sessionId ? (await getSession(c.env, instanceId, session.uid, run.sessionId).catch(() => null))?.repoId : undefined; + + const budget = await openBudget(c.env, session.uid, instanceId, body.budget); + const caps = await capabilitiesForInstance(c.env, instanceId, session.uid).catch(() => null); + const driver = loopDriverFor(caps); + const started = await driver.start({ + env: c.env, + instanceId, + userId: session.uid, + objective: run.objective, + maxIterations, + repoId, + budgetId: budget.id, + depth: 0, + // The one thing that makes this a continue rather than a restart (#806 item 4). + resumeLookbackMs: CONTINUE_RESUME_LOOKBACK_MS, + }); + if (!started.ok) throw new HttpError(started.status, started.error); + // `continuedFromRunId` is in the RESPONSE and not on the row: nothing reads it back, and a + // column claiming a lineage the resume note may not have honoured (see the header on why + // the note is not pinned) would be a stored fact that can be false. + return c.json( + { runId: started.runId, driver: started.driver, budgetId: budget.id, maxIterations, status: "running", continuedFromRunId: run.runId }, + 201, + ); + }); +} diff --git a/workers/api/src/routes/tools.ts b/workers/api/src/routes/tools.ts index ce6168c4..0f35c5a9 100644 --- a/workers/api/src/routes/tools.ts +++ b/workers/api/src/routes/tools.ts @@ -43,6 +43,7 @@ import { loopDriverFor } from "../lib/loop-drivers.js"; import { REPAIR_RUN_OBJECTIVE } from "../lib/repo-sync-gate.js"; import { enqueueObjective } from "../lib/objective-queue.js"; import { registerLoopQueueRoutes } from "./loop-queue-routes.js"; +import { registerLoopContinueRoutes } from "./loop-continue-routes.js"; import { readLoopPresets, writeLoopPresets } from "../lib/loop-presets-store.js"; import { capabilitiesForInstance } from "../lib/agent-capabilities.js"; import { sanitizeMaxIterations } from "../lib/agent-loop.js"; @@ -1149,6 +1150,7 @@ toolRoutes.post("/:id/loop", async (c) => { // Registered HERE, above `GET /:id/loop/:runId` — Hono matches in order, and `/loop/queue` would // otherwise be read as a run id (#788). See `routes/loop-queue-routes.ts`. registerLoopQueueRoutes(toolRoutes); +registerLoopContinueRoutes(toolRoutes); /** * Loop presets (#234) — the named objectives the loop form offers, per instance. diff --git a/workers/api/src/workflows/coding-session-params.ts b/workers/api/src/workflows/coding-session-params.ts index 5e04a940..b181a6b8 100644 --- a/workers/api/src/workflows/coding-session-params.ts +++ b/workers/api/src/workflows/coding-session-params.ts @@ -83,4 +83,14 @@ export interface CodingSessionParams { * is not recoverable. */ sessionOpenedByRun?: boolean; + /** + * How far back this run's resume note may look for the run it is about (#806 item 4). + * + * Absent keeps `RESUME_NOTE_LOOKBACK_MS`, which is every ordinary start. Present only on a + * CONTINUE, where the owner pointed at one stopped run and the default floor would have found + * nothing to brief the successor with — see `CONTINUE_RESUME_LOOKBACK_MS`. It is a parameter of + * the run rather than of the route because the note is composed HERE, at the start of the run, + * and a value the route held would never reach the only code that reads it. + */ + resumeLookbackMs?: number; } diff --git a/workers/api/src/workflows/coding-session.ts b/workers/api/src/workflows/coding-session.ts index c4d48a23..17d700c7 100644 --- a/workers/api/src/workflows/coding-session.ts +++ b/workers/api/src/workflows/coding-session.ts @@ -742,7 +742,7 @@ export class CodingSessionWorkflow extends WorkflowEntrypoint (await pendingCodingResumeNote(env, { userId, instanceId, sessionId, uncommittedFiles: repair ? 0 : (repoState?.changedFiles ?? 0) })) ?? null)) as string | null; + const resumeNote = (await step.do("resume-note", async () => (await pendingCodingResumeNote(env, { userId, instanceId, sessionId, uncommittedFiles: repair ? 0 : (repoState?.changedFiles ?? 0), lookbackMs: event.payload.resumeLookbackMs })) ?? null)) as string | null; if (resumeNote) goal.resumeNote = resumeNote; await step.do("tl-start", async () => { // …and in the UNIFIED trace, which is the surface `agent_trace` and every MCP debug diff --git a/workers/mcp/AGENTS.md b/workers/mcp/AGENTS.md index 06dae9c2..37df9cea 100644 --- a/workers/mcp/AGENTS.md +++ b/workers/mcp/AGENTS.md @@ -12,7 +12,7 @@ Connection methods and the full tool table: [`README.md`](./README.md). unless the user explicitly overrides this. 2. **Discover the surface before using it.** The tool list is versioned and - *per-connection*: of the 196 tool registrations, 24 are gated to the console surfaces + *per-connection*: of the 197 tool registrations, 24 are gated to the console surfaces of the agents the connected user actually subscribes to (`apply`, `repo`, `coding`). A tool you used last week may be absent today, and a tool present for one user is absent for another. Call `tools/list` first; never assume a name exists. diff --git a/workers/mcp/CLAUDE.md b/workers/mcp/CLAUDE.md index 83569f51..1ed332bb 100644 --- a/workers/mcp/CLAUDE.md +++ b/workers/mcp/CLAUDE.md @@ -86,7 +86,7 @@ src/ │ voice settings (read / customise / use-my-defaults, #613) ├── triggers.ts 7 tools — webhook / cron / connector-sync triggers, plus the action │ vocabulary and the draft preview the console's form is built from (#613) - ├── composition.ts 15 tools — supervision (#183), connections (#182) with the delivery outbox, + ├── composition.ts 16 tools — supervision (#183), connections (#182) with the delivery outbox, │ replay and delete that complete them (#613), loops, loop presets ├── account.ts 13 tools — whoami, billing, usage, keys, email, profile, budget limits, │ notifications, account preferences (#613) @@ -104,13 +104,13 @@ src/ surfaces:["coding"]) + 5 loop tools (3 run + 2 objective queue) ``` -**196 tool registrations** (`.tool(` in the files above): 21 in `index.ts`, 13 in +**197 tool registrations** (`.tool(` in the files above): 21 in `index.ts`, 13 in `coding-tools.ts` — all of them behind the `groups.has("coding")` gate — 14 in -`storage-tools.ts`, and 148 across `instance-tools/`. 172 are always registered; 24 are +`storage-tools.ts`, and 149 across `instance-tools/`. 173 are always registered; 24 are surface-gated (apply=4, repo=3, coding=17). Those four numbers ADD UP to the headline, and that is the point of stating them: 21 + 13 -+ 14 + 148 = 196. They said 88 until #602, which made the paragraph sum to 132 — a total the ++ 14 + 149 = 197. They said 88 until #602, which made the paragraph sum to 132 — a total the same sentence contradicted two clauses earlier; and they said 31 + 13 + 93 = 140 under a headline of 141 until #696 re-counted them; and said 21 + 12 + 13 + 100 = 146 until #739 added two always-on settings tools; and said 21 + 12 + 13 + 103 = 149 until #772 added diff --git a/workers/mcp/README.md b/workers/mcp/README.md index 60298366..d2175252 100644 --- a/workers/mcp/README.md +++ b/workers/mcp/README.md @@ -146,7 +146,7 @@ implementation. ## Tools -**196 tool registrations.** 172 are always registered; 24 are gated to the console +**197 tool registrations.** 173 are always registered; 24 are gated to the console surfaces of the connected user's subscribed agents (`apply`, `repo`, `coding`), so a Repo Chat user never sees `apply_to_job`. @@ -371,6 +371,7 @@ immediately instead of a whole transcript. |---|---|---|---|---| | `start_instance_loop` | Give an agent an objective and let it run on the server, budget-bounded | write | yes | | | `check_instance_loop` | Status, steps taken, stop reason (omit `run_id` to list runs) | read | | | +| `continue_instance_run` | Carry a STOPPED run's objective onto a fresh run, briefed on what it already landed. Only after an ending with no verdict; reuses its repo and step cap, opens its own budget (#806) | write | yes | | | `get_instance_loop_presets` | The saved objectives the loop form offers, and whose list it is (`instance` / `agent` / `default`) (#613) | read | | | | `set_instance_loop_presets` | Replace the instance's own presets — the whole list; `[]` goes back to inheriting. An out-of-limit list is refused, not trimmed | write | yes | | | `stop_instance_loop` | Cooperative stop — the in-flight step finishes | write | no ([why](#tools-with-no-dry-run)) | | diff --git a/workers/mcp/src/instance-tools/composition.ts b/workers/mcp/src/instance-tools/composition.ts index 8b9acb66..c9ceb880 100644 --- a/workers/mcp/src/instance-tools/composition.ts +++ b/workers/mcp/src/instance-tools/composition.ts @@ -396,6 +396,44 @@ export function registerCompositionTools(server: McpServer, ctx: InstanceToolsCt }, ); + server.tool( + "continue_instance_run", + "Carry a STOPPED run's objective onto a fresh run, briefed on what the stopped one already landed. Only for a run that ended WITHOUT a verdict — `interrupted`, `max_iterations`, `engine_limit`, `provider_credit`; anything else is refused, naming what to do instead. It reuses the stopped run's repository and step cap (pass `max_iterations` to grant more), opens its own budget, and reaches further back for its predecessor than an ordinary start, so continuing the next morning still works. Read a run first with check_instance_loop.", + { + token: z.string().optional().describe("PAGS session token. Omit when connected with browser sign-in."), + instance_id: z.string(), + run_id: z.string().describe("The stopped run to continue, from check_instance_loop."), + max_iterations: z.coerce.number().optional().describe("Steps the new run may take. Omit to reuse the stopped run's own cap (max 50)."), + dry_run: z.boolean().optional().describe("Report the run that would be started, without starting it."), + }, + async ({ token, instance_id, run_id, max_iterations, dry_run }) => { + const sessionToken = tokenFor(token); + if (!sessionToken) return authRequired(); + const input = { instance_id, run_id, max_iterations }; + const denied = await requirePermission(safetyFor(token), "write", "continue_instance_run", input); + if (denied) return denied; + if (dry_run) { + // Same reasoning as `start_instance_loop`'s preview, and one fact more: a continue + // SPENDS AGAIN. A caller that thought it was resuming a paid-for run rather than + // starting another one has no other way to find out before the money goes. + return dryRun(safetyFor(token), "continue_instance_run", "start a fresh run on a stopped run's objective", input, { + endpoint: `/v1/instances/${instance_id}/loop/${run_id}/continue`, + method: "POST", + effect: `${instance_id} would start a NEW run on ${run_id}'s objective${max_iterations === undefined ? ", with that run's own step cap" : `, for up to ${max_iterations} steps`}. The stopped run is not reanimated.`, + spend: "A new budget is opened — the stopped run's is not inherited. Each step spends the instance's own AI budget.", + }); + } + const data = await authedCall( + `/v1/instances/${encodeURIComponent(instance_id)}/loop/${encodeURIComponent(run_id)}/continue`, + sessionToken, + { method: "POST", body: JSON.stringify({ maxIterations: max_iterations }) }, + env, + ); + if (!(data as { error?: string }).error) await audit(safetyFor(token), { tool: "continue_instance_run", action: "completed", input, result: { ok: true } }); + return jsonText(data); + }, + ); + // ── Loop presets (#613) ───────────────────────────────────────────────────── // // The objectives an owner curated for the loop form (#234). Without these, a caller starting a diff --git a/workers/mcp/src/instance-tools/contract.test.ts b/workers/mcp/src/instance-tools/contract.test.ts index ee053d00..ccec43cf 100644 --- a/workers/mcp/src/instance-tools/contract.test.ts +++ b/workers/mcp/src/instance-tools/contract.test.ts @@ -446,6 +446,7 @@ const TABLE: Record = { "commands_enabled,disabled_commands,dry_run,instance_id,keep_awake,language,max_dictation_ms,provider,sensitivity,silence_ms,speed,stt_mode,stt_model,token,tts_max_chars,vocabulary", ], start_instance_loop: ["composition", "write", null, "envelope", "dry_run,instance_id,max_iterations,objective,token"], + continue_instance_run: ["composition", "write", null, "envelope", "dry_run,instance_id,max_iterations,run_id,token"], stop_instance_loop: ["composition", "write", null, null, "instance_id,run_id,token"], subscribe_agent: ["base", "write", null, "envelope", "agent_id,dry_run,idempotency_key,token"], system_status: ["coding", "none", null, null, "instance_id,token"], diff --git a/workers/mcp/src/server-version.ts b/workers/mcp/src/server-version.ts index 268c2f70..3b4c6446 100644 --- a/workers/mcp/src/server-version.ts +++ b/workers/mcp/src/server-version.ts @@ -88,4 +88,4 @@ */ /** Advertised in `serverInfo.version`, and restated in `server.json` and `platform-docs/mcp.md`. */ -export const MCP_SERVER_VERSION = "0.1.38"; +export const MCP_SERVER_VERSION = "0.1.39"; diff --git a/workers/mcp/src/surface-lock.ts b/workers/mcp/src/surface-lock.ts index cdbbc8f4..261e4467 100644 --- a/workers/mcp/src/surface-lock.ts +++ b/workers/mcp/src/surface-lock.ts @@ -609,4 +609,24 @@ export const SURFACE_LOCK: Record = { // // Appended, never edited in place: 0.1.37 is published. "0.1.38": "sha256:f774e627cf44e400a8e56348ac77296fb5099bcb45fd3f671f8742d7f7e29956", + // 0.1.39 (#806, continue a stopped run): one new tool NAME, ALWAYS-ON — 196 registrations + // become 197, `MCP_TOOL_ALWAYS_ON` 172 → 173, `MCP_TOOL_GATED` stays 24. + // + // * `continue_instance_run` (annotated `runtime`, asserts `write`, dry_run) — POST + // /v1/instances/:id/loop/:runId/continue in `instance-tools/composition.ts`, beside + // `start_instance_loop`, whose two classes split the same way for the same reason: the + // annotation says this spends something out there, the gate is the one a default grant + // holds. A caller able to START a run must be able to continue one, or the narrower scope + // would be a distinction the route itself does not make. + // + // NOT part of #613's parity sweep: the route is new in the same change, and shipping the + // console button without the tool would have ADDED a gap to the ratchet #613 is driving to + // zero. + // + // The `dry_run` preview names the SPEND twice over, because the word "continue" invites the + // wrong model: this opens a NEW budget and does not reanimate the stopped run's. + // `SERVER_INSTRUCTIONS` did not move. + // + // Appended, never edited in place: 0.1.38 is published. + "0.1.39": "sha256:aa427b83ad091801d567616620814e37f95b9be48dde4a6f19f459c138113360", }; diff --git a/workers/mcp/src/tool-count.ts b/workers/mcp/src/tool-count.ts index 371295bc..2d841e41 100644 --- a/workers/mcp/src/tool-count.ts +++ b/workers/mcp/src/tool-count.ts @@ -20,10 +20,10 @@ */ /** Every tool the server can register, with all surfaces gated on. */ -export const MCP_TOOL_COUNT = 196; +export const MCP_TOOL_COUNT = 197; /** Registered for every connection, whatever the user is subscribed to. */ -export const MCP_TOOL_ALWAYS_ON = 172; +export const MCP_TOOL_ALWAYS_ON = 173; /** Registered only when the user has an agent with the matching console surface * (`apply`, `repo`, `coding`) — so a Repo Chat user never sees `apply_to_job`. */ diff --git a/workers/mcp/src/tool-metadata.ts b/workers/mcp/src/tool-metadata.ts index 1edefc19..bbbeb70f 100644 --- a/workers/mcp/src/tool-metadata.ts +++ b/workers/mcp/src/tool-metadata.ts @@ -314,6 +314,8 @@ export const TOOL_RISK: Record = { // STRICTER than its gate (`write`): starting a loop hands the agent an objective and // lets it act unattended until it stops. start_instance_loop: "runtime", + // A continue STARTS a run, so it carries the same scope as starting one (#806). + continue_instance_run: "runtime", trigger_agent_deploy: "runtime", // ── destructive: deletes, overwrites, or commits an irreversible external action. @@ -469,7 +471,11 @@ export const MCP_RISK_COUNTS: Record = { // `status: complete`. read: 92, write: 61, - runtime: 23, + // +1 runtime at #806: `continue_instance_run`. `runtime` rather than `write` for the reason + // `start_instance_loop` is — it starts an autonomous run that spends on its own — and the + // two must agree, because a caller holding the scope to start one holding a narrower one to + // continue it would be a distinction the route itself does not make. + runtime: 24, destructive: 20, }; diff --git a/workers/mcp/src/write-readback.test.ts b/workers/mcp/src/write-readback.test.ts index b9f58dee..b0e8f3a3 100644 --- a/workers/mcp/src/write-readback.test.ts +++ b/workers/mcp/src/write-readback.test.ts @@ -323,6 +323,9 @@ const READBACK: Record = { // ── loops ── "start_instance_loop.objective": "check_instance_loop", "start_instance_loop.max_iterations": "check_instance_loop", + // The NEW run's cap, read back on the run the call returns — not on the stopped one it + // continues, whose own row is unchanged by a continue (#806). + "continue_instance_run.max_iterations": "check_instance_loop", "set_instance_loop_presets.presets": "get_instance_loop_presets", // When `queue_if_busy` parks it instead of starting it, the SAME objective reads back through // `coding_loop_queue` until it becomes a run — at which point this reader is the right one