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

CLInpm 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