From 18b1637b6371a6a6adade857e2a73654ee626fef Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:05:05 -0500 Subject: [PATCH 1/8] fix(runner): accept late Antigravity login URL during drain (#18254) ## Thinking Path > - Paperclip manages work performed by AI agents. > - The native Runner owns Antigravity subscription sign-in. > - Sign-in receives completion on stdout and the browser URL on stderr. > - The two pipes can deliver the completion result before the valid URL. > - The owner rejected that URL after it entered the completed phase. > - This pull request accepts one validated URL while the same child drains. > - A real child regression now proves the order and the failure controls. ## Linked Issues or Issue Description Refs #18188, #18189, and #18190. Related qualification work: #18345 and #18356. The duplicate search found this existing PR, so this update preserves its history and adds the real child proof here. **What happened?** A valid sign-in can fail when the authenticate response arrives before the authorization URL on the other pipe. The completed phase then rejects the valid URL. The same existing success case failed in [current follow-up CI](https://github.com/paperclipai/paperclip/actions/runs/41138618869/job/124347859445). That log has no stream-order trace, so it does not prove its inner failure branch. **Expected behavior** Both valid delivery orders must succeed. Invalid or duplicate URLs, unexpected protocol requests, failed retirement, and failed credential verification must still fail. **Steps to reproduce** 1. Run a synthetic native child that completes authenticate on stdout. 2. Wait for stdin EOF before it emits the authorization URL on stderr. EOF proves that the parent accepted the result and began retirement. 3. The valid case fails on frozen master `45a3a92e6dc52d20923b9aa34a22fee2ec5443f9`. The invalid and duplicate controls pass there. 4. Run the same regression with this PR's existing production fix. All three cases pass. **Paperclip version or commit** Existing reviewed PR parent: `646f5cdf37898ad54e1fa1cdfe39f59844da3e7c`. Frozen actual master for the real child RED: `45a3a92e6dc52d20923b9aa34a22fee2ec5443f9`. ## What Changed - Accept one fully validated authorization URL in the authentication or completed-drain phase. Keep duplicate rejection and all existing URL checks. - Retain the ten existing in-memory controls for both valid orders and eight failure paths. - Add three real child cases for valid, invalid, and duplicate late URLs. Use stdin EOF to order the streams without sleeps or a mocked retirement handler. - Check exact protocol requests, child exit, close-once behavior, Google-call counts, and persisted refresh rotation. This update changes only the existing test file; it adds no production delta to the reviewed parent. ## Verification - Frozen actual master: the new valid real child case fails, while both negative controls pass. The original eight-control parent calibration also remains recorded in this PR's history. - Existing fixed source plus the new tests: 24 passed. This includes 11 existing authentication tests, ten stream controls, and three real child cases. - Late invalid and duplicate URLs fail with zero Google calls. The valid case makes two mocked identity calls and saves the rotated synthetic refresh token. All three retire the real child and close the command lease once. - Scoped owning TypeScript `noEmit` passes for the auth module and both test files. The temporary configuration is not committed. Diff and credential-pattern checks pass. - The tests use real child pipes and private synthetic files. Google responses and the command installation are mocked. No provider account, network request, model run, app, or QA deployment was used. - Full monorepo typecheck, tests, and build were not run locally. Fresh [normal CI](https://github.com/paperclipai/paperclip/actions/runs/41140757050) passes on `1744c9f3374f78b10c2f15c42f3cf5d3af8c3119`: 56 successful checks, two skipped, and one neutral. Typecheck, build, Rust, static, Runner, server and browser lanes pass. Greptile gives that exact head a fresh 5/5 with zero unresolved threads. The PR is mergeable and clean. The earlier parent's passing CI remains historical evidence only. ## Risks The change accepts a URL while the same authenticated child drains. It still validates the exact OAuth URL and rejects a second URL. Success still requires the authenticate result, verified credentials, and proven process retirement. No provider pins, contracts, timeout bounds, retries, or public APIs change. Existing documentation already specifies these sign-in semantics; no documentation change is needed. ## Model Used OpenAI Codex, based on GPT-6, with tool use and code execution. The exact runtime model ID and context window were not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../antigravity-auth-stream-order.test.ts | 172 ++++++++++++++++++ .../src/drivers/acpx/antigravity-auth.ts | 2 +- 2 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 packages/paperclip-runner/src/drivers/acpx/antigravity-auth-stream-order.test.ts diff --git a/packages/paperclip-runner/src/drivers/acpx/antigravity-auth-stream-order.test.ts b/packages/paperclip-runner/src/drivers/acpx/antigravity-auth-stream-order.test.ts new file mode 100644 index 00000000000..e44bb0ade13 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/antigravity-auth-stream-order.test.ts @@ -0,0 +1,172 @@ +import { spawn, type ChildProcess } from "node:child_process"; +import { EventEmitter } from "node:events"; +import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { PassThrough, Writable } from "node:stream"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +const setup = vi.hoisted(() => ({ verify: vi.fn(), spawn: vi.fn(), close: vi.fn(), retire: vi.fn() })); +vi.mock("./antigravity-installation.js", () => ({ antigravityNativeDistributionSpec: () => ({ pinned: true }) })); +vi.mock("./installation-integrity.js", () => ({ verifyNativeAcpxInstallation: setup.verify })); +vi.mock("./antigravity-process.js", async importOriginal => ({ + ...await importOriginal(), retireAntigravityProcess: setup.retire, +})); +import { startAntigravityBrowserLogin } from "./antigravity-auth.js"; + +const clientId = "1071006060591-tmhssin2h21lcre235vtolojh4g403ep.apps.googleusercontent.com"; +const scopes = ["https://www.googleapis.com/auth/cloud-platform", "https://www.googleapis.com/auth/userinfo.email", "https://www.googleapis.com/auth/aicode"]; +const authorizationUrl = "https://accounts.google.com/o/oauth2/v2/auth?" + new URLSearchParams({ client_id: clientId, response_type: "code", + redirect_uri: "http://127.0.0.1:18973/", scope: scopes.join(" "), state: "fixture_state_0123456789", code_challenge: "a".repeat(43), + code_challenge_method: "S256", access_type: "offline", prompt: "consent" }); +const roots: string[] = []; +const physicalChildren: ChildProcess[] = []; +afterEach(async () => { + for (const child of physicalChildren.splice(0)) { + if (child.exitCode === null && child.signalCode === null) { + try { process.kill(-child.pid!, "SIGKILL"); } catch {} + } + } + vi.unstubAllGlobals(); vi.clearAllMocks(); + await Promise.all(roots.splice(0).map(root => rm(root, { recursive: true, force: true }))); +}); + +type Mode = "url-first" | "result-first" | "wrong-url" | "duplicate-url" | "unexpected-prompt" | "retirement-failure" | "credential-failure" | "auth-error" | "late-wrong-url" | "late-duplicate-url"; +async function fixture(mode: Mode) { + const home = await realpath(await mkdtemp(join(tmpdir(), "paperclip-antigravity-stream-test-"))); roots.push(home); + await mkdir(join(home, "antigravity-acp"), { mode: 0o700 }); + await writeFile(join(home, "antigravity-acp/acp_token.json"), JSON.stringify({ client_id: clientId, client_secret: "synthetic-client-secret", + refresh_token: "synthetic-refresh-token", token_uri: "https://oauth2.googleapis.com/token", scopes }), { mode: 0o600 }); + const stdout = new PassThrough(), stderr = new PassThrough(), requests: string[] = []; + const child = Object.assign(new EventEmitter(), { stdout, stderr, stdin: new Writable({ write(chunk, _encoding, done) { + const request = JSON.parse(String(chunk)); requests.push(request.method); + queueMicrotask(() => { + if (request.method === "initialize") { + stdout.write(JSON.stringify({ jsonrpc: "2.0", id: request.id, result: { protocolVersion: 1, + agentInfo: { name: "antigravity-acp", version: "1.4.3" }, agentCapabilities: { _meta: { antigravity: { paperclipAdmission: 1 } } }, + authMethods: [{ id: "oauth-personal" }] } }) + "\n"); + } else if (request.method === "authenticate") { + const url = mode === "wrong-url" || mode === "late-wrong-url" ? "https://example.invalid/synthetic-private-marker" : authorizationUrl; + const emitUrl = () => stderr.write("Open the following link to authenticate the ACP server: " + url + "\n"); + const resultFirst = ["result-first", "late-wrong-url", "late-duplicate-url"].includes(mode); + if (!resultFirst) emitUrl(); + if (mode === "duplicate-url") emitUrl(); + if (mode === "unexpected-prompt") stdout.write(JSON.stringify({ jsonrpc: "2.0", id: 9, method: "session/prompt", params: {} }) + "\n"); + stdout.write(JSON.stringify(mode === "auth-error" + ? { jsonrpc: "2.0", id: request.id, error: { code: -32000, message: "synthetic-private-marker" } } + : { jsonrpc: "2.0", id: request.id, result: {} }) + "\n"); + if (resultFirst) queueMicrotask(() => { emitUrl(); if (mode === "late-duplicate-url") emitUrl(); }); + } else throw new Error("Unexpected session/model request"); + }); + done(); + } }) }); + setup.close.mockResolvedValue(undefined); + setup.retire.mockImplementation(async (_child, closed: Promise) => { + await Promise.resolve(); child.emit("close", 0); await closed; + }); + if (mode === "retirement-failure") setup.retire.mockImplementationOnce(async () => { throw new Error("synthetic-private-marker"); }); + setup.spawn.mockReturnValue(child); + setup.verify.mockResolvedValue({ openCommand: async () => ({ spawn: setup.spawn, close: setup.close }) }); + const fetch = vi.fn(async (url: string) => { + if (mode === "credential-failure") return new Response(null, { status: 401 }); + if (url === "https://oauth2.googleapis.com/token") return Response.json({ access_token: "synthetic-access-token", token_type: "Bearer" }); + if (url === "https://www.googleapis.com/oauth2/v3/userinfo") return Response.json({ email: "person@example.test", email_verified: true }); + throw new Error("Unexpected network request"); + }); + vi.stubGlobal("fetch", fetch); + return { home, requests, fetch }; +} + +describe("Antigravity login cross-stream completion", () => { + it.each(["url-first", "result-first"] as const)("accepts the same authenticated exchange with %s delivery", async mode => { + const { home, requests, fetch } = await fixture(mode); + const login = startAntigravityBrowserLogin(home); + await vi.waitFor(() => expect(login.outcome).toBeDefined()); + expect(login.outcome).toBe("success"); + expect(login.authorizationUrl).toBe(authorizationUrl); + expect(requests).toEqual(["initialize", "authenticate"]); + expect(setup.retire).toHaveBeenCalledOnce(); expect(setup.close).toHaveBeenCalledOnce(); + expect(fetch).toHaveBeenCalledTimes(2); + await login.abort(); + }); + + it.each(["wrong-url", "duplicate-url", "unexpected-prompt", "retirement-failure", "credential-failure", "auth-error", "late-wrong-url", "late-duplicate-url"] as const)("keeps %s failures closed and private", async mode => { + const { home, requests, fetch } = await fixture(mode); + const login = startAntigravityBrowserLogin(home); + await vi.waitFor(() => expect(login.outcome).toBeDefined()); + expect(login.outcome).toBe("failure"); expect(login.error).toBe("Google sign-in did not complete. Start sign-in again."); + expect(login.error).not.toContain("synthetic-private-marker"); + await login.abort(); expect(setup.close).toHaveBeenCalledOnce(); + expect(requests).toEqual(["initialize", "authenticate"]); + expect(fetch).toHaveBeenCalledTimes(mode === "credential-failure" ? 1 : 0); + if (mode === "wrong-url" || mode === "late-wrong-url") expect(login.authorizationUrl).toBeUndefined(); + }); + + it.each(["valid", "invalid", "duplicate"] as const)("handles a %s URL from a real child after authenticate completion and stdin EOF", async mode => { + const home = await realpath(await mkdtemp(join(tmpdir(), "paperclip-antigravity-real-stream-"))); roots.push(home); + const peer = join(home, "peer.cjs"); + await writeFile(peer, ` +const fs = require('node:fs'); +const path = require('node:path'); +const readline = require('node:readline'); +const home = process.argv[2], mode = process.argv[3], requests = [], events = []; +const reader = readline.createInterface({input:process.stdin}); +reader.on('line', line => { + const request = JSON.parse(line); requests.push(request.method); + fs.writeFileSync(path.join(home,'requests.json'),JSON.stringify(requests)); + if(request.method==='initialize') { + process.stdout.write(JSON.stringify({jsonrpc:'2.0',id:request.id,result:{protocolVersion:1, + agentInfo:{name:'antigravity-acp',version:'1.4.3'},agentCapabilities:{_meta:{antigravity:{paperclipAdmission:1}}}, + authMethods:[{id:'oauth-personal'}]}})+'\\n'); + } else if(request.method==='authenticate') { + fs.mkdirSync(path.join(home,'antigravity-acp'),{mode:0o700}); + fs.writeFileSync(path.join(home,'antigravity-acp/acp_token.json'),${JSON.stringify(JSON.stringify({ client_id: clientId, + client_secret: "synthetic-client-secret", refresh_token: "synthetic-refresh-token", token_uri: "https://oauth2.googleapis.com/token", scopes }))},{mode:0o600}); + events.push('authenticate-result'); + process.stdout.write(JSON.stringify({jsonrpc:'2.0',id:request.id,result:{}})+'\\n'); + } else throw new Error('Unexpected session/model request'); +}); +reader.on('close',()=>{ + events.push('stdin-eof'); + const url = mode==='invalid' ? 'https://example.invalid/synthetic-private-marker' : ${JSON.stringify(authorizationUrl)}; + const message = 'Open the following link to authenticate the ACP server: '+url+'\\n'; + process.stderr.write(mode==='duplicate' ? message+message : message,()=>{ + events.push('url-flushed');fs.writeFileSync(path.join(home,'events.json'),JSON.stringify(events));process.exit(0); + }); +}); +`); + setup.spawn.mockImplementation((_args, options) => { + const child = spawn(process.execPath, [peer, home, mode], { ...options, detached: true }); + physicalChildren.push(child); return child; + }); + setup.close.mockResolvedValue(undefined); + setup.verify.mockResolvedValue({ openCommand: async () => ({ spawn: setup.spawn, close: setup.close }) }); + const realProcess = await vi.importActual("./antigravity-process.js"); + setup.retire.mockImplementation(realProcess.retireAntigravityProcess); + const fetch = vi.fn(async (url: string) => { + if (url === "https://oauth2.googleapis.com/token") return Response.json({ access_token: "synthetic-access-token", + refresh_token: "synthetic-rotated-refresh", token_type: "Bearer" }); + if (url === "https://www.googleapis.com/oauth2/v3/userinfo") return Response.json({ email: "person@example.test", email_verified: true }); + throw new Error("Unexpected network request"); + }); + vi.stubGlobal("fetch", fetch); + const login = startAntigravityBrowserLogin(home); + await vi.waitFor(() => expect(login.outcome).toBeDefined(), { timeout: 5_000 }); + expect(login.outcome).toBe(mode === "valid" ? "success" : "failure"); + expect(JSON.parse(await readFile(join(home, "events.json"), "utf8"))).toEqual(["authenticate-result", "stdin-eof", "url-flushed"]); + expect(JSON.parse(await readFile(join(home, "requests.json"), "utf8"))).toEqual(["initialize", "authenticate"]); + expect(physicalChildren.at(-1)!.exitCode).toBe(0); + expect(setup.retire).toHaveBeenCalledOnce(); expect(setup.close).toHaveBeenCalledOnce(); + expect(fetch).toHaveBeenCalledTimes(mode === "valid" ? 2 : 0); + if (mode === "valid") { + expect(login.authorizationUrl).toBe(authorizationUrl); + expect(JSON.parse(await readFile(join(home, "antigravity-acp", "acp_token.json"), "utf8")).refresh_token).toBe("synthetic-rotated-refresh"); + } else { + expect(login.error).toBe("Google sign-in did not complete. Start sign-in again."); + expect(login.error).not.toContain("synthetic-private-marker"); + if (mode === "invalid") expect(login.authorizationUrl).toBeUndefined(); + } + await login.abort(); + expect(setup.close).toHaveBeenCalledOnce(); + }, 15_000); +}); diff --git a/packages/paperclip-runner/src/drivers/acpx/antigravity-auth.ts b/packages/paperclip-runner/src/drivers/acpx/antigravity-auth.ts index 2921fd141db..7f4b3bf54bd 100644 --- a/packages/paperclip-runner/src/drivers/acpx/antigravity-auth.ts +++ b/packages/paperclip-runner/src/drivers/acpx/antigravity-auth.ts @@ -203,7 +203,7 @@ export function startAntigravityBrowserLogin(home: string): AntigravityBrowserLo if (!line.startsWith(AUTH_PREFIX)) continue; try { const url = validateAntigravityAuthorizationUrl(line.slice(AUTH_PREFIX.length)); - if (phase !== 2 || state.authorizationUrl !== undefined) throw new Error(LOGIN_ERROR); + if ((phase !== 2 && phase !== 3) || state.authorizationUrl !== undefined) throw new Error(LOGIN_ERROR); state.authorizationUrl = url; } catch { fail(); } } From 91261d5a038d4d009c01ba102da4a00921caa175 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:36:36 -0500 Subject: [PATCH 2/8] fix(server): preserve budget history for deleted resources (#18389) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Budget views show saved policies, incidents and spend for each company. > - A policy can remain after its agent or project has been deleted. > - The overview currently stops when it cannot resolve that old resource. > - A current agent's budget then becomes unreadable during normal setup. > - This pull request preserves the historical row in display summaries. > - Admission and policy changes still require a real company-owned resource. ## Linked Issues or Issue Description **What happened?** The budget overview throws `Agent not found` when a saved policy references a deleted agent. One old resource prevents the company from reading its current budgets. The same problem affects historical project policies and incident summaries. **Expected behavior** The overview should retain historical policy and incident IDs and spend. It should display `Deleted agent` or `Deleted project` for a missing resource. Read access must not grant permission to execute work or change a budget for that resource. **Steps to reproduce** 1. Create an agent and a budget policy for it. 2. Permanently delete the agent after its other dependencies have been cleared. 3. Read the company's budget overview. The existing service throws instead of returning its remaining current and historical rows. **Paperclip version or commit** The isolated repair starts from merged commit `d9354bbfc35ef74333e90e596ce21dc8e59149b9` and integrates the exact merged baseline test fix `8595efc20bda3ce20a9fbbf34a5d156a6fcf6d57`. It also integrates the actual merged Antigravity auth fix `18b1637b6371a6a6adade857e2a73654ee626fef` (#18254). The diff against the selected merged parent remains two budget files. **Related work** Earlier reports and implementations: #11058 and #4333. This extracts the product prerequisite from #18351 so it can qualify independently of that journey's other dependencies. The change contains only the budget service repair and four database regression cases. ## What Changed - Resolve deleted agent and project scopes only when building display summaries. Preserve their original IDs, policies, incidents and accounting records. - Reject a saved policy or incident that resolves to another company's resource. Propagate other read failures. - Keep missing-company resolution, work admission and policy changes strict. Validate company, policy and incident scope bindings before either incident action. A budget raise requires a live company-owned resource. Historical dismissal may proceed only for a genuinely deleted owned agent/project, and must reject live foreign or mismatched scopes. - Add four canonical PostgreSQL regression cases: deleted agent history, deleted project history, missing-scope admission and budget raises with safe historical dismissal, and foreign/mismatched references. The history cases also verify current ledger spend and unchanged stored rows. ## Verification - Real-service RED/GREEN against official PGlite with every canonical migration: the original overview throws for deleted history; the repair returns the historical IDs, incidents and spend while retaining current budgets. - Mutation regression RED/GREEN: missing-resource budget raises must reject before writes; owned deleted-scope dismissal must reject its linked pending approval, retain policy/agent/ledger data and incident history, and remove the incident from the active list. Live foreign and mismatched scopes reject both actions without mutations. - Current integrated head `b3434f5ad05fd3067379352700c36bfe826179c4`: all four canonical PostgreSQL regressions pass in [server 2/12](https://github.com/paperclipai/paperclip/actions/runs/41142588583/job/124361194077), and all five existing budget-service tests pass in [server 7/12](https://github.com/paperclipai/paperclip/actions/runs/41142588583/job/124361194080). Both use the CI merge against selected parent `18b1637`. - Clean-build declaration typing passes in the current CI typecheck gate. The earlier local source-alias no-emit check also passed, including the canonical actor argument. The local worktree borrows compiled dependencies from a sibling checkout; a later real-service import was blocked by its stale `sandboxCreationOperations` export, so clean-build CI is authoritative for the final integration. No production workaround was made. - `git diff --check` passes. Broad repository typecheck, tests and build run in current-head CI rather than repeated locally. Native local PostgreSQL could not start because this host exhausted its semaphore pool; no global IPC settings or unrelated processes were changed. - One unchanged Runner lane retry was authorized after a probable intermittent close-settlement fixture failure. The [original failure](https://github.com/paperclipai/paperclip/actions/runs/41142588583/job/124360234111) occurred in `omitted-context` cleanup. A [fresh passing Linux lane](https://github.com/paperclipai/paperclip/actions/runs/41142552195/job/124360110386) passed the same case and all 1,998 tests with byte-identical entire Runner and adapter-utils trees, lockfile, root package, scripts and workflows, against the same merged base. The fixture has a five-second close allowance with a 2.5-second suspension reserve; a host stall is not proven. No budget path or changed close input was found. The [single targeted retry](https://github.com/paperclipai/paperclip/actions/runs/41142588583/job/124365812609) passed the same case in 1,034 ms and all 1,998 lane tests; dependent verification also passed. All 57 current PR checks are terminal and green. - The product helper requires all current CI and clean review before merge. Actual QA budget-overview proof will run on the combined final app before hiring qualification. No deployed success is claimed. The older `b4863dd` artifact was never deployed; selected parent `18b1637` adds sandbox/lifecycle changes and migration 0418, so whole-app identity is not claimed. The budget module and relevant budget contracts remain unchanged, and final hiring requires the combined final app with explicit source identities. ## Risks Budget summaries now return historical rows whose resources have been deleted. These rows use explicit display names and preserve their accounting data. They do not create a resource or relax work admission, authorization or budget grants. Historical incident dismissal changes only that incident and its linked approval; it does not resume work or raise a budget. Cross-company references still fail closed. There is no schema migration and no new background work. ## Model Used OpenAI Codex, GPT-6 family. The session does not expose a more specific backend model ID or fixed context window. Used reasoning, repository tools, TypeScript and database test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (the PR description documents the display-only history and strict-mutation contract) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- server/src/__tests__/budgets-history.test.ts | 109 +++++++++++++++++++ server/src/services/budgets.ts | 24 +++- 2 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 server/src/__tests__/budgets-history.test.ts diff --git a/server/src/__tests__/budgets-history.test.ts b/server/src/__tests__/budgets-history.test.ts new file mode 100644 index 00000000000..5b106329d72 --- /dev/null +++ b/server/src/__tests__/budgets-history.test.ts @@ -0,0 +1,109 @@ +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { agents, approvals, budgetIncidents, budgetPolicies, companies, costEvents, createDb, projects, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db"; +import { budgetService } from "../services/budgets.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; + +vi.mock("../services/activity-log.js", () => ({ logActivity: vi.fn() })); +const support = await getEmbeddedPostgresTestSupport(); + +describe.skipIf(!support.supported)("budget history for deleted scopes", () => { + let db: ReturnType; + let database: Awaited>; + beforeAll(async () => { + database = await startEmbeddedPostgresTestDatabase("paperclip-budget-history-"); + db = createDb(database.connectionString); + }, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS); + afterEach(async () => { + await db.delete(budgetIncidents); await db.delete(approvals); await db.delete(budgetPolicies); await db.delete(costEvents); + await db.delete(projects); await db.delete(agents); await db.delete(companies); + }); + afterAll(async () => { await db?.$client.end({ timeout: 1 }); await database?.cleanup(); }); + + async function fixture() { + const companyId = randomUUID(), agentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "History company", issuePrefix: `B${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}` }); + await db.insert(agents).values({ id: agentId, companyId, name: "Current agent", role: "engineer" }); + const [policy] = await db.insert(budgetPolicies).values({ companyId, scopeType: "agent", scopeId: agentId, + metric: "billed_cents", windowKind: "calendar_month_utc", amount: 300 }).returning(); + await db.insert(costEvents).values({ companyId, agentId, provider: "openai", model: "history-fixture", inputTokens: 1, outputTokens: 1, costCents: 17, occurredAt: new Date() }); + return { companyId, agentId, policy: policy! }; + } + + it.each(["agent", "project"] as const)("keeps current budgets and deleted %s policy/incident history readable without changing stored rows", async scopeType => { + const { companyId, policy } = await fixture(), deletedId = randomUUID(); + if (scopeType === "agent") await db.insert(agents).values({ id: deletedId, companyId, name: "Former agent", role: "engineer" }); + else await db.insert(projects).values({ id: deletedId, companyId, name: "Former project" }); + const [historical] = await db.insert(budgetPolicies).values({ companyId, scopeType, scopeId: deletedId, + metric: "billed_cents", windowKind: "calendar_month_utc", amount: 200 }).returning(); + await db.insert(budgetIncidents).values({ companyId, policyId: historical!.id, scopeType, scopeId: deletedId, + metric: "billed_cents", windowKind: "calendar_month_utc", windowStart: new Date("2020-01-01"), windowEnd: new Date("2020-02-01"), + thresholdType: "hard", amountLimit: 200, amountObserved: 201 }); + if (scopeType === "agent") await db.delete(agents).where(eq(agents.id, deletedId)); + else await db.delete(projects).where(eq(projects.id, deletedId)); + const policiesBefore = await db.select().from(budgetPolicies), incidentsBefore = await db.select().from(budgetIncidents), costsBefore = await db.select().from(costEvents); + + const overview = await budgetService(db).overview(companyId); + expect(overview.policies).toHaveLength(2); + expect(overview.policies.find(row => row.policyId === policy.id)).toMatchObject({ companyId, scopeName: "Current agent", amount: 300, observedAmount: 17 }); + expect(overview.policies.find(row => row.policyId === historical!.id)).toMatchObject({ companyId, scopeType, scopeId: deletedId, scopeName: `Deleted ${scopeType}`, amount: 200 }); + expect(overview.activeIncidents).toMatchObject([{ companyId, policyId: historical!.id, scopeId: deletedId, scopeName: `Deleted ${scopeType}`, status: "open", amountObserved: 201 }]); + expect(await db.select().from(budgetPolicies)).toEqual(policiesBefore); + expect(await db.select().from(budgetIncidents)).toEqual(incidentsBefore); + expect(await db.select().from(costEvents)).toEqual(costsBefore); + }); + it("rejects missing-scope budget grants but can dismiss their historical incidents", async () => { + const { companyId, agentId } = await fixture(), missingId = randomUUID(), service = budgetService(db); + await expect(service.evaluateAdmission(companyId, missingId)).rejects.toThrow("Agent not found"); + await expect(service.evaluateAdmission(companyId, agentId, { projectId: missingId })).rejects.toThrow("Project not found"); + for (const scopeType of ["agent", "project"] as const) { + await expect(service.upsertPolicy(companyId, { scopeType, scopeId: missingId, amount: 100 }, "board")).rejects.toThrow(`${scopeType === "agent" ? "Agent" : "Project"} not found`); + const [policy] = await db.insert(budgetPolicies).values({ companyId, scopeType, scopeId: missingId, + windowKind: "calendar_month_utc", amount: 100 }).returning(); + const [approval] = await db.insert(approvals).values({ companyId, type: "budget_override_required", payload: {} }).returning(); + const [incident] = await db.insert(budgetIncidents).values({ companyId, policyId: policy!.id, approvalId: approval!.id, scopeType, scopeId: missingId, + metric: "billed_cents", windowKind: "calendar_month_utc", windowStart: new Date("2020-01-01"), windowEnd: new Date("2020-02-01"), + thresholdType: "hard", amountLimit: 100, amountObserved: 101 }).returning(); + const before = { policies: await db.select().from(budgetPolicies), incidents: await db.select().from(budgetIncidents), + agents: await db.select().from(agents), costs: await db.select().from(costEvents), approvals: await db.select().from(approvals) }; + await expect(service.resolveIncident(companyId, incident!.id, { action: "raise_budget_and_resume", amount: 200 }, "board")) + .rejects.toThrow(`${scopeType === "agent" ? "Agent" : "Project"} not found`); + expect({ policies: await db.select().from(budgetPolicies), incidents: await db.select().from(budgetIncidents), + agents: await db.select().from(agents), costs: await db.select().from(costEvents), approvals: await db.select().from(approvals) }).toEqual(before); + const dismissed = await service.resolveIncident(companyId, incident!.id, { action: "keep_paused" }, "board"); + expect(dismissed).toMatchObject({ companyId, policyId: policy!.id, scopeId: missingId, scopeName: `Deleted ${scopeType}`, status: "dismissed" }); + const { status: _status, resolvedAt: _resolvedAt, updatedAt: _updatedAt, ...preserved } = incident!; + const [saved] = await db.select().from(budgetIncidents).where(eq(budgetIncidents.id, incident!.id)); + expect(saved).toMatchObject({ ...preserved, status: "dismissed", resolvedAt: expect.any(Date) }); + expect((await db.select().from(approvals).where(eq(approvals.id, approval!.id)))[0]).toMatchObject({ status: "rejected" }); + expect(await db.select().from(budgetPolicies)).toEqual(before.policies); + expect(await db.select().from(agents)).toEqual(before.agents); + expect(await db.select().from(costEvents)).toEqual(before.costs); + expect((await service.overview(companyId)).activeIncidents.some(row => row.id === incident!.id)).toBe(false); + } + }); + + it("rejects a corrupted policy pointing to another company's scope instead of exposing its name", async () => { + const { companyId } = await fixture(), foreignCompanyId = randomUUID(), foreignAgentId = randomUUID(); + await db.insert(companies).values({ id: foreignCompanyId, name: "Other company", issuePrefix: `F${foreignCompanyId.replace(/-/g, "").slice(0, 6).toUpperCase()}` }); + await db.insert(agents).values({ id: foreignAgentId, companyId: foreignCompanyId, name: "Private other-company agent", role: "engineer" }); + const [policy] = await db.insert(budgetPolicies).values({ companyId, scopeType: "agent", scopeId: foreignAgentId, windowKind: "calendar_month_utc", amount: 200 }).returning(); + const [incident] = await db.insert(budgetIncidents).values({ companyId, policyId: policy!.id, scopeType: "agent", scopeId: foreignAgentId, + metric: "billed_cents", windowKind: "calendar_month_utc", windowStart: new Date("2020-01-01"), windowEnd: new Date("2020-02-01"), + thresholdType: "hard", amountLimit: 200, amountObserved: 201 }).returning(); + const service = budgetService(db); + await expect(service.overview(companyId)).rejects.toThrow("Budget scope not found"); + for (const scopeId of [foreignAgentId, randomUUID()]) { + await db.update(budgetIncidents).set({ scopeId }).where(eq(budgetIncidents.id, incident!.id)); + const before = { policies: await db.select().from(budgetPolicies), incidents: await db.select().from(budgetIncidents), + agents: await db.select().from(agents), costs: await db.select().from(costEvents) }; + for (const action of ["raise_budget_and_resume", "keep_paused"] as const) { + await expect(service.resolveIncident(companyId, incident!.id, { action, amount: 300 }, "board")).rejects.toThrow("Budget scope not found"); + expect({ policies: await db.select().from(budgetPolicies), incidents: await db.select().from(budgetIncidents), + agents: await db.select().from(agents), costs: await db.select().from(costEvents) }).toEqual(before); + } + } + }); + +}); diff --git a/server/src/services/budgets.ts b/server/src/services/budgets.ts index 6777124ee90..3a15d489f0a 100644 --- a/server/src/services/budgets.ts +++ b/server/src/services/budgets.ts @@ -163,6 +163,19 @@ async function resolveScopeRecord(db: Db, scopeType: BudgetScopeType, scopeId: s }; } +async function resolveSummaryScopeRecord(db: Db, companyId: string, scopeType: BudgetScopeType, scopeId: string): Promise { + let scope: ScopeRecord; + try { + scope = await resolveScopeRecord(db, scopeType, scopeId); + } catch (error) { + if (!(error instanceof HttpError) || error.status !== 404 || (scopeType !== "agent" && scopeType !== "project")) throw error; + return { companyId, name: scopeType === "agent" ? "Deleted agent" : "Deleted project", + paused: false, pauseReason: null, budgetHeld: false }; + } + if (scope.companyId !== companyId) throw notFound("Budget scope not found"); + return scope; +} + // Keep a large dashboard from filling the database pool with queued reads. async function mapBudgetReads(rows: T[], read: (row: T) => Promise): Promise { const result: R[] = []; @@ -395,7 +408,7 @@ export function budgetServiceInTransaction(db: Db, publications: ActivityPublica } async function buildPolicySummary(policy: PolicyRow): Promise { - const scope = await resolveScopeRecord(db, policy.scopeType as BudgetScopeType, policy.scopeId); + const scope = await resolveSummaryScopeRecord(db, policy.companyId, policy.scopeType as BudgetScopeType, policy.scopeId); const observed = await computeObservedSpend(db, policy); const { total: observedAmount, totalExact: observedAmountExact, unpricedEventCount, pendingRunCount } = observed; const { windowStart: start, windowEnd: end } = observed; @@ -560,7 +573,7 @@ export function budgetServiceInTransaction(db: Db, publications: ActivityPublica const approvalStatusById = new Map(approvalRows.map((row) => [row.id, row.status])); return mapBudgetReads(rows, async (row) => { - const scope = await resolveScopeRecord(db, row.scopeType as BudgetScopeType, row.scopeId); + const scope = await resolveSummaryScopeRecord(db, row.companyId, row.scopeType as BudgetScopeType, row.scopeId); return { id: row.id, companyId: row.companyId, @@ -925,6 +938,13 @@ export function budgetServiceInTransaction(db: Db, publications: ActivityPublica if (incident.companyId !== companyId) throw notFound("Budget incident not found"); const policy = await getPolicyRow(incident.policyId); + if (policy.companyId !== companyId || policy.scopeType !== incident.scopeType || policy.scopeId !== incident.scopeId) { + throw notFound("Budget scope not found"); + } + const scope = input.action === "keep_paused" + ? await resolveSummaryScopeRecord(db, companyId, policy.scopeType as BudgetScopeType, policy.scopeId) + : await resolveScopeRecord(db, policy.scopeType as BudgetScopeType, policy.scopeId); + if (scope.companyId !== companyId) throw notFound("Budget scope not found"); if (input.action === "raise_budget_and_resume") { const nextAmount = Math.max(0, Math.floor(input.amount ?? 0)); const observed = await computeObservedSpend(db, policy); From 1353a232f2cd9e5fb58933689e485a9ff1f1480f Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:37:17 -0500 Subject: [PATCH 3/8] test(e2e): add a portable active follow-up journey (#18345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip lets people direct agents while their work is active. > - Product E2E checks must prove that saved user input reaches the intended work. > - The existing active chat story depends on a file in a local agent workspace. > - That setup cannot qualify an attached deployment with remote agent execution. > - This change adds one shared active follow-up journey through the normal task UI and API. > - Independent native operation, saved input, final reply, and cleanup checks make the result inspectable across targets. ## Linked Issues or Issue Description **What existing behavior does this improve?** Product E2E coverage for a user follow-up sent while a native agent is working. **Current behavior** The classic chat story holds work open with a local file. The shared deployment-targeted harness has no active follow-up journey. A running heartbeat alone cannot prove that work was active when input arrived. **Proposed behavior** Run one bounded command through the real native provider. Observe its active execution before and after one follow-up is saved. Require one final reply with the late requirement, correct task and agent identity, an empty input queue, and visible readback after reload. Use the same story on managed and attached targets. ## What Changed - Add a shared `followup-while-running` journey with normal task creation and reply submission. - Require a bound native turn and pending shell execution at the input boundary. Retain a closed receipt with source identity and a row hash. - Accept one steered work run or one normal successor (1–2 native runs). Preserve every other journey's declared run contract. - Reuse the saved native final reply observer, run binding checks, usage collection, target lifecycle and private evidence path. - Accept canonical builtin/process execution events independent of provider tool display names; calibrate against real OpenCode, Codex and ACP mappers. Reject Paperclip semantic tool names across native wrappers/prefixes and unnamed or ambiguous builtin calls. - Recognize only the observed legacy PRP v1 builtin/bash/unknown/readOnly=false command shape and fail promptly when owned work finishes before a live boundary can be observed. - Ready the task page/composer before the observed command boundary, use a bounded 90-second command, and preserve active-after-persistence proof. - Add adversarial oracle and retained-result calibration. Register the journey, include its sources in the definition hash, and document its coverage. - Keep six real browser document-readiness checks and verify shared-deadline behavior with a deterministic clock/state test that rejects timeout-reset mutations. ## Verification Exact current head `e72e20ad111bd94728d38606015d1fdac3c92929` passed [full CI run 41142552195](https://github.com/paperclipai/paperclip/actions/runs/41142552195): all 49 workflow jobs reached accepted terminal results, including typecheck, build, every test lane and aggregate verify. All 59 reported checks are terminal: 55 success, three expected skips and one neutral 5/5 Paperclip review. Snyk passed. [Fresh Greptile review](https://github.com/paperclipai/paperclip/pull/18345#issuecomment-6528913235) explicitly names this exact head and gives 5/5. No review threads remain open. The PR is mergeable. These source gates do not establish a live QA pass at this head. Fresh protected QA at this exact source and the separately recorded deployed application/runtime remains required before primary merge and daily admission. Historical live QA staging **PASS** on qualified commit `af9e091b15b5e24b6b2cc1b424523430cc2ab024`: 6/6 checks, all five phases including cleanup, two native work runs, and the configured model/environment check passed. The [sanitized qualification report](https://pages.paperclip.ing/evals-qa-smoke-41122620146-1/) records that exact measurement. Final integration head `e72e20ad111bd94728d38606015d1fdac3c92929` merges exact actual squash `18b1637b6371a6a6adade857e2a73654ee626fef` from [#18254](https://github.com/paperclipai/paperclip/pull/18254) into previous head `e54d64743964b1a19d165537bebbe62c50389a23`. The merge had no conflicts. All 18 files in this PR's change set, the complete 45-file shared journey directory, and six shared native/evidence helpers retain their exact previous-head Git blobs: 59 scoped files in total. The production clarification helper remains blob `800a747284db19b8484650b6759e8bd1f356839d`. Follow-up 1–2 runs, question 1–3 runs, older exact counts, owned native event checks, active-before/after persistence proof, cleanup/accounting and feature mappings remain unchanged. Source integration head `e55d74491f5bcd99c1455e9ace02bd7ad7319fae` integrates exact actual squash `8595efc20bda3ce20a9fbbf34a5d156a6fcf6d57` from [#18385](https://github.com/paperclipai/paperclip/pull/18385). The merge had no conflicts. All 17 files in this PR's change set and six shared native/evidence helpers retain their exact Git blobs from previous head `162e341cda5b627639b486b4019a1675bb018ea2`. This preserves the follow-up and question readiness checks, feature mappings, six closed follow-up check IDs, immutable before/after active-operation proof, follow-up 1–2 runs, question 1–3 runs and older exact counts. Previously reviewed head `e54d64743964b1a19d165537bebbe62c50389a23` adds a separate test-only reliability repair on top of that source integration. It replaces one scheduler-sensitive browser case with a deterministic Vitest clock and locator-state model that calls the actual production helper. The six other real Chromium checks remain byte-for-byte unchanged and pass **6/6**. Focused unit suites pass **51/51**, including the deterministic deadline case. This is six browser checks plus focused unit coverage, not seven browser checks. The model makes the panel ready at 100 ms, the supplied answer ready at 600 ms, and full valid content ready at 1,300 ms. It requires rejection at the shared 1,000 ms deadline and records immutable rejection state before simulating a 10-second scheduler stall. Both restarted-timeout mutations fail: resetting all waits wrongly resolves at 1,300 ms; resetting matcher timeouts while keeping the final real deadline guard rejects only at 1,300 ms with valid content present. The genuine helper rejects at 1,000 ms with the answer still incomplete. The production helper is unchanged, Git blob `800a747284db19b8484650b6759e8bd1f356839d`. That two-file test-only patch changed no journey, application, run, accounting, model or runtime behavior. Its six-browser/51-unit results are prior-head evidence; the complete patch is unchanged in the new integration. The final source also retains the actual merged question-primary squash `d9354bbfc35ef74333e90e596ce21dc8e59149b9` from [#18346](https://github.com/paperclipai/paperclip/pull/18346). Later commits `409117451da260a03fc28a3107c7808ecbe68f8f` and `ea3e7b1d1bed562351fcf4722c7175001b987b59` bound question document readiness to one campaign deadline and test that waits cannot restart it. Commit `162e341cda5b627639b486b4019a1675bb018ea2` adds question/follow-up report feature mappings and coverage. These files are unchanged by the final merge. The imported upstream source has real application changes: [#18334](https://github.com/paperclipai/paperclip/pull/18334) makes managed skill content durable and extends native skill-test contracts; [#18350](https://github.com/paperclipai/paperclip/pull/18350) requires managed Cloud shared storage and bootstrap prerequisites. #18385 updates TestOps job metadata and repairs their combined maintenance fixture. The new merge additionally imports [#18383](https://github.com/paperclipai/paperclip/pull/18383) at `81ecaf0654fe79a2a2dbda6588f033e39ee610e4`, which bounds failed lifecycle verification; [#18357](https://github.com/paperclipai/paperclip/pull/18357) at `45a3a92e6dc52d20923b9aa34a22fee2ec5443f9`, which persists sandbox-creation ownership and updates cleanup/adapter/plugin contracts; and #18254 at `18b1637b6371a6a6adade857e2a73654ee626fef`, which accepts a valid late Antigravity authorization URL while native authentication drains. This PR's diff against exact upstream `18b1637b6371a6a6adade857e2a73654ee626fef` contains only journey tests, report coverage and documentation. Harness source and deployed application/runtime identity are separate inputs. This source integration does not deploy the imported application changes or claim the QA target runs this application revision. This final SHA has not been measured by the historical run above: **fresh QA at this exact final head is required before primary merge**, followed by normal TestOps verification/admission. Fresh exact-head CI and review have passed as recorded above. The live measurement must record the actual separately deployed QA application/runtime identity. Seven whole-file blobs remain identical to the qualified SHA. All paths are relative to `tests/runner-e2e/`: | Journey/helper | Unchanged Git blob | | --- | --- | | `product-journeys/active-followup-evidence.ts` | `83a5d9fab52abb9849a40e83eca6b6a9c29bf587` | | `product-journeys/active-followup.test.ts` | `728160c610c18292d2cf06d4a87d25a8719f1734` | | `native-task-reply.ts` | `59d11f1a178dad9db3da8fa2342ff93643cb9922` | | `native-event-envelope.ts` | `305b0b456c59c511ee328e529ad574c0583075a0` | | `run-observations.ts` | `08c8ad184a821a7cd536926b3d7e3d1156a1e3da` | | `user-actions.ts` | `e9611348dc41e17899367dab2e3b36d3d1be9480` | | `stop-redirect-flow.ts` | `4580bd605a425bc72511e02d988492acbec8d60e` | The follow-up preservation comparison against historical `af9` has three explicit exceptions; no whole-file identity is claimed for them. The separately documented question readiness and report additions above also require final-source verification: 1. `product-journeys/agent-flows.ts` adds `question-answer-resume` to the `AgentJourney` type exclusion, alongside follow-up. Its remaining contents and runtime helper bodies are byte-for-byte unchanged. Qualified blob `a0afaa968c1d854476159fef21de7b6dd50a4a98`; final blob `0cc797c0ae594e7769338e63a80c4f67e29e4a54`. 2. The common run-count helper is centralized in `success-contract.ts` and re-exported by the catalog. It accepts follow-up **1–2**, clarification **1–3**, and each older paid journey's exact count. The follow-up catalog entry, maximum of two runs, six required checks, native dispatch, collection and retained-run contract remain byte-for-byte unchanged. The helper's text differs; its preserved count semantics are verified explicitly. 3. `product-journeys/active-followup-flow.ts` changes exactly one composer readiness assertion from Playwright's implicit five-second default to an explicit 30-second timeout, matching `submitTaskReply` for attached targets. Prompts, actions, native evidence, before/after persistence proof and grading otherwise compare byte-for-byte unchanged. Qualified blob `64392329198a1a4d9828e32834d4507e175216e6`; final blob `2746b5ba89e2af26b4097ba7897913186786ac5d`. This correction is why a fresh final-head QA run is required. - **310 focused tests passed at current final head `e72e20ad111bd94728d38606015d1fdac3c92929`**: 286 journey/evidence/catalog/report/shared-deadline tests in 14 files and 24 native authentication/real-child stream-order tests in two files. Both invocations used Node24 and one Vitest worker. The earlier `e55` integration also passed 365 focused tests, including 40 Cloud configuration and 40 TestOps metadata cases; those results remain prior-source evidence. - **157 explicit count assertions passed again at the current final head**, including both interactive ranges, older exact counts, invalid/fractional/non-number values and catalog/contract export identity. The maximum bounds and six closed follow-up check IDs also matched. - Maps and diff checks passed. The official QA runtime-import preflight passed on Node24 at the current final head without network access. It uses a dummy adapter and no credentials. It does not prove target connectivity, sign-in, model readiness or deployed application/runtime identity. Those require the normal protected QA run. No model/provider work or optional Runner release was dispatched. - Current local runtime dependencies use ignored isolated workspace-source views. No shared dependencies or compilers were changed. A previous local typecheck attempt hit stale reused Runner declarations. No full repository typecheck, test run or build was repeated locally; the successful exact-head full CI run above now supplies these gates. - CI on previous head `162e341cda5b627639b486b4019a1675bb018ea2` failed the Cloud/local maintenance fixture in [server shard 9](https://github.com/paperclipai/paperclip/actions/runs/41133716802/job/124331643100), plus dependent verify. The actual tested merge combined that head with master `198218bf554877fe3b7a83742eefb3b8f12392c6`. A Cloud child selected local-disk storage and hit the new shared-storage policy. This was a deterministic upstream fixture mismatch; no unchanged failed-job retry was used. - The actual #18385 repair configures a valid Cloud child, verifies local bytes independently, checks the expected provider mismatch and retains exact stored-config readback. Its own final head `b76d5dd40d7711f1b5f6b6316b1114c0aae02d57` passed 59 checks, including [server shard 9](https://github.com/paperclipai/paperclip/actions/runs/41133876636/job/124331450635). That prior result supports the repair; it does not replace this final head's fresh CI. Separate repair candidate #18392 and its source/evidence are retained as superseded history. - A local attempt to run the database fixture could not start PostgreSQL because the host exhausted macOS semaphores. A first setup attempt also failed package resolution before test execution. Skipped database cases and these setup failures are not passes. No kernel settings, shared processes or databases were changed. - The complete previous-head CI run [41138618869](https://github.com/paperclipai/paperclip/actions/runs/41138618869) finished with one underlying failure: [Runner Vitest shard 1](https://github.com/paperclipai/paperclip/actions/runs/41138618869/job/124347859445) expected native Antigravity login success but observed failure at `antigravity-auth.test.ts:101`. The lane had one failed test, 1,994 passing tests and eight skipped tests. Aggregate verify failed only because of this lane. All other 57 checks were accepted, including build, typecheck, all server/browser suites, Runner shard 2, Snyk and exact-head 5/5 reviews. No unchanged retry was requested. - The independently reviewed #18254 repair removes an invalid cross-pipe ordering assumption: a valid URL on stderr can arrive after the authenticate response on stdout during drain. Its real-child regression forces both orders. The current final source imports that actual squash; the 24 focused authentication cases pass here. Fresh full CI and exact-head review have now passed for this source, as recorded above. The earlier failed result is not relabeled. - The original live compatibility failure is retained: an older PRP v1 pack labeled builtin `bash` as operation `unknown`, causing the observer to miss the command and never send the follow-up. Its exact shape was reproduced red before repair. Generic unknown operations, semantic tools, wrappers, read-only work and foreign provenance remain rejected. The historical qualified pass used the repaired oracle; no failed result was relabeled. - Earlier CI on `6fad965d69` hit an unchanged heartbeat test's three-second adapter-call wait; its original failure and one narrow rerun remain recorded. The qualified SHA subsequently passed CI. An earlier report-catalog subprocess failure is retained. The final focused Node24 run now includes that suite and passes after the documented report feature mappings. ## Risks The fixture asks the native agent to run one bounded `sleep 90` command. The observer must see actual active native execution; a fast or noncompliant turn fails rather than passing from elapsed time. A completed result requires one or two bounded native work runs, source attribution, configured model/environment, and collection. Unknown provider prices stay unknown. This change does not alter production execution or grant new access. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and code execution. The host does not expose an exact underlying model version or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with available version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs; existing follow-up execution fixes do not provide this deployment-targeted test - [x] I have described the issue in-PR following the enhancement template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket or instance-derived details - [x] I have run the new focused tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation - [x] I have considered and documented risks - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before merge --------- Co-authored-by: Paperclip --- doc/evals.md | 6 + maps/feature-map/steering.md | 7 + tests/runner-e2e/PRODUCT-JOURNEYS.md | 32 ++++ tests/runner-e2e/catalog.test.ts | 2 +- .../runner-e2e/clarification-readback.spec.ts | 34 ++++ tests/runner-e2e/journey-features.ts | 2 + tests/runner-e2e/playwright-support.config.ts | 2 +- .../active-followup-evidence.ts | 82 +++++++++ .../product-journeys/active-followup-flow.ts | 110 +++++++++++ .../product-journeys/active-followup.test.ts | 174 ++++++++++++++++++ .../product-journeys/agent-flows.ts | 4 +- tests/runner-e2e/product-journeys/catalog.ts | 3 +- .../product-journeys/clarification-flow.ts | 18 +- .../clarification-readback.test.ts | 49 +++++ tests/runner-e2e/product-journeys/launch.ts | 5 +- .../product-journeys/success-contract.ts | 5 +- tests/runner-e2e/product-journeys/types.ts | 2 +- tests/runner-e2e/report-catalog.test.ts | 25 +++ 18 files changed, 550 insertions(+), 12 deletions(-) create mode 100644 tests/runner-e2e/clarification-readback.spec.ts create mode 100644 tests/runner-e2e/product-journeys/active-followup-evidence.ts create mode 100644 tests/runner-e2e/product-journeys/active-followup-flow.ts create mode 100644 tests/runner-e2e/product-journeys/active-followup.test.ts create mode 100644 tests/runner-e2e/product-journeys/clarification-readback.test.ts diff --git a/doc/evals.md b/doc/evals.md index 78c9c3ce467..bc0b7c9f729 100644 --- a/doc/evals.md +++ b/doc/evals.md @@ -76,6 +76,12 @@ one bound continuation, and an exact saved confirmation. Its four development cells use the recorded Codex and Claude models. Registration and calibration do not establish Slack, staging authentication, warm lifecycle, or live qualification. +The explicit `followup-while-running` [Product journey](../tests/runner-e2e/PRODUCT-JOURNEYS.md#active-follow-up) +checks a saved message during independently observed native work, one attributed +reply with the late requirement, and persistence after reload. Its attached and +managed targets share the same story and permit one steered run or one normal +successor. Registration and calibration are separate from a live staging pass. + [Reusable Product journeys](../tests/runner-e2e/PRODUCT-JOURNEYS.md) add explicit deployment targets and starting-state recipes. The shared task-properties journey uses no model; Cloud onboarding supplies reusable verified state. Cloud diff --git a/maps/feature-map/steering.md b/maps/feature-map/steering.md index 840fcd1ba74..611f0e56275 100644 --- a/maps/feature-map/steering.md +++ b/maps/feature-map/steering.md @@ -60,6 +60,13 @@ inspect the resulting task and run in Paperclip. The explicit `stop-redirect` [Product journey](../../tests/runner-e2e/PRODUCT-JOURNEYS.md) covers `steering/stop-and-pause` only for Stop and one new direction. It requires active work, native cancellation acknowledgement, resource cleanup, and completion without later original-turn activity. It does not qualify pause, interrupt, or all queued-message behavior. Source registration is not a live Cloud pass. +The explicit `followup-while-running` [Product journey](../../tests/runner-e2e/PRODUCT-JOURNEYS.md#active-follow-up) +covers `steering/queue`: one follow-up is saved while a bound native command is +active, its late requirement appears in one attributed final reply, the queue +empties, and the result survives reload. One steered turn or one normal successor +is allowed. Unit calibration and source registration are not live acceptance. + + Preconditions: follow the [baseline](./README.md#before-driving-a-journey). Use an agent that stays busy long enough to inspect the queue. Record the adapter, native/legacy mode, and advertised steering capability. Test supported and diff --git a/tests/runner-e2e/PRODUCT-JOURNEYS.md b/tests/runner-e2e/PRODUCT-JOURNEYS.md index 895005cd15a..98864325189 100644 --- a/tests/runner-e2e/PRODUCT-JOURNEYS.md +++ b/tests/runner-e2e/PRODUCT-JOURNEYS.md @@ -34,6 +34,7 @@ never start a server or depend on a tenant filesystem. | `protected-actions` | Native agent; four work runs; authenticated reachable review-service fixture | Approve one action and decline another on separate owned tasks; verify zero calls before either decision, one after approval, zero after decline and a truthful saved explanation | | `create-skill-studio` | Native agent with skill-write permission; one work run | Create a uniquely named company skill, open its activity card, edit in Studio, reload and verify task-card readback | | `stop-redirect` | Native agent; three work runs; qualified Docker oracle | Download a verified checkpoint, Stop an active revision, require native cancellation acknowledgement, submit a new direction once and verify completion without original-run activity | +| `followup-while-running` | Native agent; a maximum of two work runs | Save a follow-up while an independently observed native shell command is still active, then require one attributed final reply with the late requirement after reload; in-turn steering and one normal successor are accepted | | `cloud-onboarding` | Signed-in admitted Cloud account; fresh workspace; qualified bounded provider route | Create workspace in the portal, connect API key and native Codex agent in the wizard, complete a task, download and independently check its artifact | The question-resumption journey uses the same natural-language scenario, browser @@ -228,6 +229,37 @@ its disposable agent, makes owned tasks terminal, verifies no active runs, retai the records for decision-history integrity, and releases both QA leases. This is reported as retained data, rather than confirmed deletion. +## Active follow-up + +The `followup-while-running` story uses the same ordinary task composer and saved +reply observer on managed self-hosted and attached Cloud targets. The initial +request includes one bounded `sleep 90` command. It supplies no changed launch +day or final marker. The observer requires a valid running native turn and a +pending named builtin command or process execution before sending the follow-up. +Paperclip semantic tools, including completion/block/progress calls, cannot +satisfy this boundary even if a provider labels them builtin execute or wraps +their names with a native prefix. Unnamed or ambiguous builtin calls fail closed. +The observed older PRP v1 builtin `bash` shape is also supported when its +operation is `unknown` and `readOnly` is explicitly false; other unknown +operations cannot satisfy the boundary. If all owned work settles before an +active boundary is observed, the journey fails immediately rather than polling +until the campaign deadline. +The same command must remain active after exactly one user message is saved. +The task page and composer are ready before observing the boundary, so navigation is outside the +submission window. The longer bounded command leaves room for remote persistence. +Merely observing a running heartbeat or waiting for a fixed interval cannot satisfy the boundary. + +The late message changes the launch day and supplies a unique final marker. +The independent oracle requires the expected reply from one bound native run, +one or two successful work runs in total, an empty saved-message queue, Done, +and visible readback after reload. A provider can steer the original turn or +queue one normal successor; retries, duplicate replies and unrelated work fail. +Source/model/environment checks and full usage collection remain required. +Only the closed boundary receipt and attributed final reply are retained in the +private story directory. No local workspace path or provider-side fixture hook +is required. The registered source and unit calibration do not qualify a live +Cloud target; admission to scheduled testing requires its own fresh pass. + ## Evidence and lifecycle Results use the existing campaign/result schemas and dashboard. Each result diff --git a/tests/runner-e2e/catalog.test.ts b/tests/runner-e2e/catalog.test.ts index 1e98adc848b..cac4fcc784a 100644 --- a/tests/runner-e2e/catalog.test.ts +++ b/tests/runner-e2e/catalog.test.ts @@ -59,7 +59,7 @@ describe("runner E2E catalog", () => { }); it("exports deployment journeys separately from the model matrix and default selection", () => { - expect(productJourneys.map(journey => journey.id)).toEqual(["task-create", "project-task-context", "ask-question", "question-answer-resume", "task-properties", "task-search-filter", "plan-revise-accept", "build-revise", "protected-actions", "create-skill-studio", "stop-redirect", "cloud-onboarding"]); + expect(productJourneys.map(journey => journey.id)).toEqual(["task-create", "project-task-context", "ask-question", "question-answer-resume", "task-properties", "task-search-filter", "plan-revise-accept", "build-revise", "protected-actions", "create-skill-studio", "stop-redirect", "followup-while-running", "cloud-onboarding"]); expect(runnerSuites.some(suite => suite.id === "product-journeys")).toBe(false); expect(runnerMatrix.some(execution => execution.suite.id === "product-journeys")).toBe(false); expect(selectRunnerExecutions(parseRunnerSelectors(["--all"])).some(execution => execution.suite.id === "product-journeys")).toBe(false); diff --git a/tests/runner-e2e/clarification-readback.spec.ts b/tests/runner-e2e/clarification-readback.spec.ts new file mode 100644 index 00000000000..087457d5707 --- /dev/null +++ b/tests/runner-e2e/clarification-readback.spec.ts @@ -0,0 +1,34 @@ +import { expect, test } from "@playwright/test"; +import { waitForClarificationDocument } from "./product-journeys/clarification-flow.js"; + +const answer = "Use AMBERreadback."; +const selector = '[id="document-welcome-note"]:visible, [id="side-panel-content-document:welcome-note"]:visible'; + +for (const stage of ["mount", "text"] as const) { + test(`clarification readback waits beyond the default timeout for ${stage}`, async ({ page }) => { + const body = `Welcome to the afternoon meetup. ${answer}`; + await page.setContent(stage === "mount" ? "
" : '
Loading
'); + await page.evaluate(({ stage, body }) => { + setTimeout(() => { + if (stage === "mount") document.querySelector("main")!.innerHTML = `
${body}
`; + else document.querySelector("section")!.textContent = body; + }, 5_200); + }, { stage, body }); + await waitForClarificationDocument(page.locator(selector), answer, Date.now() + 8_000); + await expect(page.locator(selector)).toHaveText(body); + }); +} + +test("clarification readback rejects an exhausted campaign before waiting", async ({ page }) => { + await page.setContent(`
Welcome to the afternoon meetup. ${answer}
`); + await expect(waitForClarificationDocument(page.locator(selector), answer, Date.now() - 1)) + .rejects.toMatchObject({ code: "clarification_document_not_ready" }); +}); + +for (const invalid of ["duplicate", "wrong-reference", "wrong-choice"] as const) { + test(`clarification readback rejects ${invalid} evidence within the campaign deadline`, async ({ page }) => { + const body = invalid === "wrong-reference" ? "Welcome to the afternoon meetup. Use OLD." : invalid === "wrong-choice" ? answer : `Welcome to the afternoon meetup. ${answer}`; + await page.setContent(`
${body}
${invalid === "duplicate" ? `
${body}
` : ""}`); + await expect(waitForClarificationDocument(page.locator(selector), answer, Date.now() + 250)).rejects.toThrow(); + }); +} diff --git a/tests/runner-e2e/journey-features.ts b/tests/runner-e2e/journey-features.ts index 683f170c1e7..b0259b504d3 100644 --- a/tests/runner-e2e/journey-features.ts +++ b/tests/runner-e2e/journey-features.ts @@ -19,6 +19,8 @@ function associate(suites: readonly string[], cases: Record; +export interface ActiveFollowupBoundary { + runId: string; + executionId: string; + startedSeq: number; + sourceEventId: string; + sourceInstanceId: string; + sourceSeq: number; + startedRowSha256: string; +} + +/** A claimed run or elapsed delay is not proof that native work is active. */ +export function activeFollowupBoundary(run: Row, events: Row[]): ActiveFollowupBoundary | null { + if (!activeNativeTurnStarted(run, events) || !events.length + || events.some((event, index) => event.seq !== index + 1)) return null; + const bound = events.filter(event => { + const envelope = event.payload?.prpEvent; + return envelope && isValidNativePrpEnvelope(envelope, event.protocolSchemaVersion) + && event.runId === run.id && event.companyId === run.companyId && event.agentId === run.agentId + && envelope.runId === run.id && envelope.eventType === event.eventType && envelope.sourceKind === "runner" + && typeof envelope.sourceEventId === "string" && Boolean(envelope.sourceEventId) && envelope.sourceEventId === event.sourceEventId + && typeof envelope.sourceInstanceId === "string" && Boolean(envelope.sourceInstanceId) && envelope.sourceInstanceId === event.sourceInstanceId + && Number.isSafeInteger(envelope.sourceSeq) && envelope.sourceSeq > 0 && envelope.sourceSeq === event.sourceSeq; + }); + const latestTurn = bound.filter(event => /^turn\.(started|completed|failed|cancelled|interrupted)$/.test(event.eventType)).at(-1); + if (latestTurn?.eventType !== "turn.started") return null; + const starts = bound.filter(event => { + const payload = event.payload.prpEvent.payload; + return event.seq > latestTurn.seq && event.eventType === "tool.execution.started" && payload?.schema === "paperclip.tool.execution.v1" + && typeof payload.executionId === "string" && Boolean(payload.executionId) + && ["builtin", "process"].includes(payload.transport) + && (payload.operation === "execute" + // Older native packs classify builtin bash as unknown. Recognize only + // that named v1 shell shape; unknown semantic/custom tools still fail. + || event.protocolSchemaVersion === 1 && payload.operation === "unknown" + && payload.transport === "builtin" && payload.name === "bash" && payload.readOnly === false) + // Provider display titles vary, but Paperclip semantic calls never prove + // native command work, even when a provider maps them to builtin execute. + && payload.namespace == null + && (payload.transport === "process" || typeof payload.name === "string" && Boolean(payload.name.trim()) + && !/^(?:unknown|tool(?:[ _-]+call)?)$/i.test(payload.name.trim()) + && !/(?:^|[._:/-])paperclip(?:[._:/-]|$)/i.test(payload.name.trim())) + && payload.readOnly !== true && payload.status === "running"; + }); + const pending = starts.filter(start => { + const id = start.payload.prpEvent.payload.executionId; + return starts.filter(other => other.payload.prpEvent.payload.executionId === id).length === 1 + && !events.some(event => event.eventType === "tool.execution.completed" + && event.payload?.prpEvent?.payload?.executionId === id); + }); + const started = pending.at(-1); + if (!started) return null; + const envelope = started.payload.prpEvent; + return { runId: run.id, executionId: envelope.payload.executionId, startedSeq: started.seq, + sourceEventId: envelope.sourceEventId, sourceInstanceId: envelope.sourceInstanceId, sourceSeq: envelope.sourceSeq, + startedRowSha256: createHash("sha256").update(JSON.stringify(started)).digest("hex") }; +} + +export function sameActiveFollowupBoundary(before: ActiveFollowupBoundary, after: ActiveFollowupBoundary | null) { + return Boolean(after && Object.keys(before).every(key => before[key as keyof ActiveFollowupBoundary] === after[key as keyof ActiveFollowupBoundary])); +} + +/** Accept in-turn steering or one normal successor; reject duplicate delivery. */ +export function activeFollowupOutcome(input: { + issueId: string; agentId: string; before: ActiveFollowupBoundary; after: ActiveFollowupBoundary | null; + runs: Row[]; expectedReply: string; marker: string; replies: Array<{ runId: string; body: string }>; + storedFollowupCount: number; queuedCommentCount: number; finalIssueStatus: string; +}) { + const ids = input.runs.map(run => run.id); + const matching = input.replies.filter(reply => Boolean(input.marker) && reply.body.includes(input.marker)); + return sameActiveFollowupBoundary(input.before, input.after) + && input.storedFollowupCount === 1 && input.queuedCommentCount === 0 && input.finalIssueStatus === "done" + && input.runs.length >= 1 && input.runs.length <= 2 && new Set(ids).size === ids.length && ids.includes(input.before.runId) + && input.runs.every(run => run.agentId === input.agentId && run.status === "succeeded" && run.runtimeMode === "native" + && (run.nativeIssueId ?? run.issueId ?? run.contextSnapshot?.issueId ?? run.contextSnapshot?.taskId) === input.issueId) + && matching.length === 1 && matching[0]!.body === input.expectedReply && ids.includes(matching[0]!.runId) + && (input.runs.length === 1 || matching[0]!.runId !== input.before.runId); +} diff --git a/tests/runner-e2e/product-journeys/active-followup-flow.ts b/tests/runner-e2e/product-journeys/active-followup-flow.ts new file mode 100644 index 00000000000..2746b5ba89e --- /dev/null +++ b/tests/runner-e2e/product-journeys/active-followup-flow.ts @@ -0,0 +1,110 @@ +import { mkdir, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { expect } from "@playwright/test"; +import { createTaskThroughUi, submitTaskReply } from "../user-actions.js"; +import { collectRunEvents } from "../run-observations.js"; +import { nativeFinalReplyEvents, readBoundTaskFinalReply } from "../native-task-reply.js"; +import { fixtureReplyStoredOnce, validateAgentTaskState, waitFor } from "./agent-flows.js"; +import { activeFollowupBoundary, activeFollowupOutcome, sameActiveFollowupBoundary } from "./active-followup-evidence.js"; +import { JourneyError, type JourneyContext, type JourneyEvidence, type TargetSession } from "./types.js"; + +type Row = Record; +const id = "followup-while-running"; + +export function activeFollowupStory(campaignId: string) { + const nonce = campaignId.replace(/[^a-z0-9]/gi, ""); + const reference = `BRIEF${nonce}`, marker = `UPDATED${nonce}`; + const expectedReply = `Launch Friday. Reference ${reference}. Update ${marker}.`; + return { + first: `Our launch is planned for Monday. Run the bounded command sleep 90 exactly once before answering, so I can send additional context while you work. After the command finishes, briefly acknowledge the launch day and reference ${reference}, then finish this task. No files, projects or child tasks are needed.`, + followup: `Change the launch day to Friday. When ready, reply with exactly this one line and finish: ${expectedReply}`, + reference, marker, expectedReply, + }; +} + +/** Once owned work has settled, an unobserved live boundary cannot arrive. */ +export async function observeActiveFollowupCommand(taskRuns: Row[], loadEvents: (runId: string) => Promise) { + if (taskRuns.length > 0 && taskRuns.every(run => run.status === "succeeded")) + throw new JourneyError("failed", "followup_native_command_not_active"); + const run = taskRuns.find(run => run.status === "running"); + return { run, boundary: run ? activeFollowupBoundary(run, await loadEvents(run.id)) : null }; +} + +/** The same browser/API/native story runs against managed and attached targets. */ +export async function runActiveFollowup(context: JourneyContext, session: TargetSession, campaignId: string, + evidence: JourneyEvidence, options: { deadlineAt: number; directory: string }) { + const agent = context.agent; + if (!agent || agent.runtimeMode !== "native" || !context.environmentId) throw new JourneyError("unsupported", "native_agent_and_environment_required"); + const story = activeFollowupStory(campaignId), title = `Journey QA active followup ${campaignId}`; + const check = (name: string, passed: boolean) => { + evidence.checks.push({ id: name, passed }); + if (!passed) throw new JourneyError("failed", name.replaceAll("-", "_")); + }; + await mkdir(options.directory, { recursive: true, mode: 0o700 }); + const retain = (name: string, value: unknown) => writeFile(path.join(options.directory, name), JSON.stringify(value, null, 2), { mode: 0o600 }); + const created = await createTaskThroughUi({ page: context.page, origin: context.api.origin, issuePrefix: context.company.issuePrefix, + agentName: agent.name, title, prompt: story.first, workMode: "standard", requireExplicitTitle: true, + beforeDispatch: () => session.own({ kind: "issue", name: title, journeyId: id }) }); + await session.own({ kind: "issue", name: title, id: created.issueId, journeyId: id }); + const route = `/api/issues/${created.issueId}`; + const read = async () => { + const [currentIssue, summaries, comments, queue] = await Promise.all([ + context.api.get(route), context.api.get(`${route}/runs`), context.api.get(`${route}/comments?order=asc`), + context.api.get(`${route}/queued-comments`), + ]); + if (!Array.isArray(summaries) || summaries.length > 2) throw new JourneyError("failed", "followup_run_limit_exceeded"); + const taskRuns = await Promise.all(summaries.map(summary => context.api.get(`/api/heartbeat-runs/${summary.runId}`))); + validateAgentTaskState(context, created.issueId, { currentIssue, taskRuns, comments, interactions: [], attachments: [] }, 2); + if (currentIssue.description !== story.first) throw new JourneyError("failed", "followup_initial_prompt_changed"); + return { currentIssue, taskRuns, comments, queue }; + }; + const events = (runId: string) => collectRunEvents((afterSeq, limit) => + context.api.get(`/api/heartbeat-runs/${runId}/events?afterSeq=${afterSeq}&limit=${limit}`)); + const issue = (await read()).currentIssue; + await context.page.goto(`${context.api.origin}/${context.company.issuePrefix}/issues/${issue.identifier ?? issue.id}`, { waitUntil: "domcontentloaded" }); + await expect(context.page.getByTestId("task-chat-composer-input").last()).toBeVisible({ timeout: 30_000 }); + const beforeComments = (await read()).comments; + const active = await waitFor(options.deadlineAt, async () => { + const state = await read(); + return observeActiveFollowupCommand(state.taskRuns, events); + }, value => Boolean(value.boundary), "followup_native_command_not_active"); + const before = active.boundary!; + await submitTaskReply(context.page, story.followup); + const persisted = await waitFor(Math.min(options.deadlineAt, Date.now() + 30_000), read, state => state.comments.some(comment => comment.authorUserId === context.user.id + && !beforeComments.some(old => old.id === comment.id)), "followup_not_persisted"); + const afterRun = persisted.taskRuns.find(run => run.id === before.runId)!; + const after = activeFollowupBoundary(afterRun, await events(before.runId)); + await retain("followup-boundary.json", { before, after }); + check("followup-native-command-active", sameActiveFollowupBoundary(before, after)); + check("followup-stored-once", fixtureReplyStoredOnce(beforeComments, persisted.comments, context.user.id, story.followup)); + + const final = await waitFor(options.deadlineAt, async () => { + const state = await read(); + const replies = [] as Array<{ runId: string; body: string }>; + for (const run of state.taskRuns.filter(run => run.status === "succeeded")) { + const stream = await events(run.id); + const decision = run.resultJson?.presentationDecision?.commentId; + const posted = state.comments.filter(comment => comment.id === decision && comment.authorAgentId === agent.id && comment.createdByRunId === run.id); + if (posted.length) replies.push(...posted.map(comment => ({ runId: run.id, body: comment.body }))); + else replies.push(...nativeFinalReplyEvents(stream, run).map(reply => ({ runId: run.id, body: reply.body }))); + } + return { state, replies }; + }, value => activeFollowupOutcome({ issueId: created.issueId, agentId: agent.id, before, after, + runs: value.state.taskRuns, expectedReply: story.expectedReply, marker: story.marker, replies: value.replies, + storedFollowupCount: value.state.comments.filter(comment => comment.authorUserId === context.user.id + && comment.body?.replaceAll("\\_", "_") === story.followup).length, + queuedCommentCount: Array.isArray(value.state.queue.entries) ? value.state.queue.entries.length : -1, + finalIssueStatus: value.state.currentIssue.status }), "followup_final_outcome_missing"); + check("followup-consumed-once", true); + const finalRunId = final.replies.find(reply => reply.body === story.expectedReply)!.runId; + const companies = await context.api.get>("/api/companies?scope=accessible"); + if (!Array.isArray(companies) || !companies.some(company => company.id === context.company.id) || companies.some(company => !company.issuePrefix)) throw new JourneyError("failed", "reply_company_context_missing"); + const reply = await readBoundTaskFinalReply({ page: context.page, runId: finalRunId, agentId: agent.id, + expectedText: story.expectedReply, deadlineAt: options.deadlineAt, knownCompanyPrefixes: companies.map(company => company.issuePrefix), + load: async () => { const latest = await read(); return { run: latest.taskRuns.find(run => run.id === finalRunId)!, comments: latest.comments, events: await events(finalRunId) }; } }); + await retain("followup-final.json", { before, after, runIds: final.state.taskRuns.map(run => run.id), reply }); + check("followup-final-answer-persisted", reply.observedAfterReload === true); + check("followup-one-or-two-native-runs", final.state.taskRuns.length >= 1 && final.state.taskRuns.length <= 2); + await expect(context.page.getByText(story.expectedReply, { exact: true }).last()).toBeVisible(); + check("configured-model-and-environment", true); +} diff --git a/tests/runner-e2e/product-journeys/active-followup.test.ts b/tests/runner-e2e/product-journeys/active-followup.test.ts new file mode 100644 index 00000000000..728160c610c --- /dev/null +++ b/tests/runner-e2e/product-journeys/active-followup.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, it } from "vitest"; +import { canonicalProviderEventsFromCodex, canonicalProviderEventsFromAcpxRuntimeEvent, canonicalProviderEventsFromOpenCodePart } from "../../../packages/paperclip-runner/src/provider-events.js"; +import { activeFollowupBoundary, activeFollowupOutcome, sameActiveFollowupBoundary } from "./active-followup-evidence.js"; +import { activeFollowupStory, observeActiveFollowupCommand } from "./active-followup-flow.js"; +import { journeyRequiredChecks } from "./success-contract.js"; +import { parseTargetConfig, preparationPreview, agentJourneyRunCountValid } from "./catalog.js"; +import { validateRetainedRunnerResult } from "../result-validation.js"; + +const run = { id: "run", companyId: "company", agentId: "agent", issueId: "issue", runtimeMode: "native", status: "running" }; +function event(seq: number, type: string, payload = {}): Record { + return { seq, eventType: type, runId: run.id, companyId: run.companyId, agentId: run.agentId, + protocolSchemaVersion: 2, sourceEventId: `source-${seq}`, sourceInstanceId: "source", sourceSeq: seq, + payload: { prpEvent: { schema: "paperclip.prp.event.v2", schemaVersion: 2, eventType: type, runId: run.id, + sourceKind: "runner", sourceEventId: `source-${seq}`, sourceInstanceId: "source", sourceSeq: seq, payload } } }; +} +const command = { schema: "paperclip.tool.execution.v1", executionId: "wait", transport: "builtin", operation: "execute", name: "bash", status: "running" }; +const events = () => [event(1, "turn.started"), event(2, "tool.execution.started", command)]; +const boundary = () => activeFollowupBoundary(run, events())!; +// Synthetic identity and filler rows; the v1 operation/transport/name/readOnly +// shape and native/source sequence positions match observed deployment metadata. +function legacyEvents(change: Record = {}): Record[] { + const rows: Record[] = Array.from({ length: 81 }, (_, index) => ({ seq: index + 1, eventType: "adapter.message" })); + const bind = (seq: number, sourceSeq: number, type: string, payload: Record) => { + const row = event(seq, type, payload); + row.protocolSchemaVersion = 1; row.sourceSeq = sourceSeq; + Object.assign(row.payload.prpEvent, { schema: "paperclip.prp.event.v1", schemaVersion: 1, sourceSeq }); + return row; + }; + rows[70] = bind(71, 9, "turn.started", { status: "inProgress" }); + rows[80] = bind(81, 12, "tool.execution.started", { ...command, namespace: null, operation: "unknown", readOnly: false, ...change }); + return rows; +} +const outcome = () => ({ issueId: "issue", agentId: "agent", before: boundary(), after: boundary(), + runs: [{ ...run, status: "succeeded" }], expectedReply: "Friday UPDATED", marker: "UPDATED", replies: [{ runId: "run", body: "Friday UPDATED" }], + storedFollowupCount: 1, queuedCommentCount: 0, finalIssueStatus: "done" }); + +describe("deployment-independent active follow-up", () => { + it("withholds the changed requirement and result marker until the follow-up", () => { + const story = activeFollowupStory("campaign-123"); + expect(story.first).toContain("sleep 90"); expect(story.first).toContain(story.reference); + expect(story.first).not.toContain(story.marker); expect(story.first).not.toContain("Friday"); + expect(story.followup).toContain(story.expectedReply); + }); + it("requires a bound running native turn and an actual pending shell execution", () => { + expect(boundary()).toMatchObject({ runId: "run", executionId: "wait", startedSeq: 2 }); + for (const rows of [[], events().slice(0, 1), [...events(), event(3, "tool.execution.completed", { ...command, status: "completed" })], + [...events(), event(3, "turn.completed")], [...events(), event(3, "turn.started")], [events()[0]!, event(2, "tool.execution.started", { ...command, operation: "read" })], + [events()[0]!, event(2, "tool.execution.started", { ...command, readOnly: true })], + [events()[0]!, event(2, "tool.execution.started", { ...command, name: "paperclip_finish" })], + [events()[0]!, event(2, "tool.execution.started", { ...command, name: "paperclip_block" })], + [events()[0]!, event(2, "tool.execution.started", { ...command, namespace: "paperclip" })], + [events()[0]!, event(2, "tool.execution.started", { ...command, name: null })], + [events()[0]!, event(2, "tool.execution.started", { ...command, transport: "mcp" })], + [events()[0]!, event(3, "tool.execution.started", command)], + [...events(), event(3, "tool.execution.started", command)]]) expect(activeFollowupBoundary(run, rows)).toBeNull(); + for (const field of ["companyId", "agentId", "runId", "sourceEventId", "sourceInstanceId", "sourceSeq", "protocolSchemaVersion"]) { + const rows = events(); rows[1]![field] = "wrong"; expect(activeFollowupBoundary(run, rows)).toBeNull(); + } + const rows = events(); rows[0]!.payload.prpEvent.sourceKind = "control_plane"; + expect(activeFollowupBoundary(run, rows)).toBeNull(); + for (const changed of [{ status: "succeeded" }, { runtimeMode: "legacy" }]) expect(activeFollowupBoundary({ ...run, ...changed }, events())).toBeNull(); + }); + it("accepts canonical Codex command text and ACP display titles without provider-name coupling", () => { + const codex = canonicalProviderEventsFromCodex("item/started", { + item: { id: "wait", type: "commandExecution", command: "sleep 90", status: "inProgress" }, + })[0]!; + const acp = canonicalProviderEventsFromAcpxRuntimeEvent({ + type: "tool_call", tag: "tool_call", toolCallId: "wait", title: "Bounded settlement fixture", kind: "execute", status: "pending", + } as never, "fallback")[0]!; + expect(codex.payload).toMatchObject({ transport: "process", operation: "execute", name: "sleep 90", status: "running" }); + expect(acp.payload).toMatchObject({ transport: "builtin", operation: "execute", name: "Bounded settlement fixture", readOnly: false, status: "running" }); + for (const mapped of [codex, acp]) { + expect(activeFollowupBoundary(run, [event(1, "turn.started"), event(2, mapped.eventType, mapped.payload)])).toMatchObject({ runId: "run", startedSeq: 2 }); + } + }); + it("accepts the real OpenCode shell fixture and rejects semantic calls normalized as builtin execute", () => { + const mapped = (tool: string, callID = "") => canonicalProviderEventsFromOpenCodePart({ + id: "wait", type: "tool", tool, callID, state: { status: "pending", input: { command: "sleep 90" } }, + })[0]!; + const shell = mapped("bash"); + expect(shell.payload).toMatchObject({ transport: "builtin", operation: "execute", name: "bash", readOnly: false, status: "running" }); + expect(activeFollowupBoundary(run, [event(1, "turn.started"), event(2, shell.eventType, shell.payload)])).toMatchObject({ runId: "run", startedSeq: 2 }); + for (const semantic of [mapped("paperclip_finish"), mapped("paperclip_block"), mapped("paperclip_report_progress"), + mapped("paperclip_paperclip_finish"), mapped("unknown", "functions.paperclip_block:0"), + mapped("functions.paperclip_finish"), mapped("mcp__paperclip__paperclip_block"), mapped("paperclip.paperclip_report_progress")]) { + expect(semantic.payload).toMatchObject({ transport: "builtin", operation: "execute", readOnly: false, status: "running" }); + expect(activeFollowupBoundary(run, [event(1, "turn.started"), event(2, semantic.eventType, semantic.payload)])).toBeNull(); + } + for (const name of [null, "", " ", "unknown", "tool", "tool call", "tool_call", "tool-call"]) { + expect(activeFollowupBoundary(run, [event(1, "turn.started"), event(2, shell.eventType, { ...shell.payload, name })])).toBeNull(); + } + }); + it("recognizes the observed legacy v1 builtin bash shape without admitting unknown operations generally", () => { + const before = activeFollowupBoundary(run, legacyEvents()); + expect(before).toMatchObject({ runId: "run", executionId: "wait", startedSeq: 81, sourceSeq: 12 }); + for (const change of [{ name: "unknown" }, { name: "custom-command" }, { name: "paperclip_finish" }, { name: "paperclip_block" }, + { name: "paperclip_report_progress" }, { name: "functions.paperclip_finish" }, { readOnly: true }, { readOnly: undefined }, + { transport: "process" }, { namespace: "paperclip" }]) expect(activeFollowupBoundary(run, legacyEvents(change))).toBeNull(); + for (const field of ["companyId", "agentId", "runId", "sourceEventId", "sourceInstanceId", "sourceSeq", "protocolSchemaVersion"]) { + const rows = legacyEvents(); rows[80]![field] = "foreign"; + expect(activeFollowupBoundary(run, rows)).toBeNull(); + } + expect(activeFollowupBoundary(run, [events()[0]!, event(2, "tool.execution.started", { ...command, operation: "unknown", readOnly: false })])).toBeNull(); + const after = legacyEvents(); + after.push(event(82, "tool.execution.progressed", { ...after[80]!.payload.prpEvent.payload })); + expect(sameActiveFollowupBoundary(before!, activeFollowupBoundary(run, after))).toBe(true); + after.push(event(83, "tool.execution.completed", { ...after[80]!.payload.prpEvent.payload, status: "completed" })); + expect(activeFollowupBoundary(run, after)).toBeNull(); + }); + it("fails promptly after owned work settles without an observed active boundary", async () => { + let eventReads = 0; + const load = async () => { eventReads++; return legacyEvents(); }; + for (const taskRuns of [[{ ...run, status: "succeeded" }], [{ ...run, status: "succeeded" }, { ...run, id: "second", status: "succeeded" }]]) + await expect(observeActiveFollowupCommand(taskRuns, load)).rejects.toMatchObject({ outcome: "failed", code: "followup_native_command_not_active" }); + expect(eventReads).toBe(0); + for (const taskRuns of [[], [{ ...run, status: "queued" }], [{ ...run, status: "succeeded" }, { ...run, id: "second", status: "queued" }]]) + expect((await observeActiveFollowupCommand(taskRuns, load)).boundary).toBeNull(); + expect(eventReads).toBe(0); + expect((await observeActiveFollowupCommand([run], load)).boundary).toMatchObject({ runId: "run", startedSeq: 81 }); + expect(eventReads).toBe(1); + }); + it("proves the same execution remains active after the user message is stored", () => { + expect(sameActiveFollowupBoundary(boundary(), boundary())).toBe(true); + for (const changed of [null, { ...boundary(), executionId: "other" }, { ...boundary(), startedRowSha256: "changed" }]) + expect(sameActiveFollowupBoundary(boundary(), changed)).toBe(false); + }); + it("accepts one steered run or one normal successor with the unique late answer", () => { + expect(activeFollowupOutcome(outcome())).toBe(true); + const value = outcome(); value.runs.push({ ...value.runs[0]!, id: "successor" }); value.replies[0]!.runId = "successor"; + expect(activeFollowupOutcome(value)).toBe(true); + }); + it("rejects lost input, duplicate delivery/work, stale answers and unfinished queues", () => { + const changes = [ + (v: ReturnType) => { v.storedFollowupCount = 0; }, + (v: ReturnType) => { v.storedFollowupCount = 2; }, + (v: ReturnType) => { v.queuedCommentCount = 1; }, + (v: ReturnType) => { v.finalIssueStatus = "in_progress"; }, + (v: ReturnType) => { v.replies = []; }, + (v: ReturnType) => { v.replies.push(v.replies[0]!); }, + (v: ReturnType) => { v.replies[0]!.body = "Monday"; }, + (v: ReturnType) => { v.replies.push({ runId: "run", body: "UPDATED again" }); }, + (v: ReturnType) => { v.replies[0]!.runId = "other"; }, + (v: ReturnType) => { v.runs[0]!.status = "failed"; }, + (v: ReturnType) => { v.runs[0]!.agentId = "other"; }, + (v: ReturnType) => { v.runs[0]!.issueId = "other"; }, + (v: ReturnType) => { v.runs = []; }, + (v: ReturnType) => { v.runs.push(v.runs[0]!); }, + (v: ReturnType) => { v.runs.push({ ...v.runs[0]!, id: "successor" }); }, + ]; + for (const change of changes) { const value = outcome(); change(value); expect(activeFollowupOutcome(value)).toBe(false); } + }); + it("allows a maximum of two declared runs while preserving every other exact bound", () => { + for (const count of [1, 2]) expect(agentJourneyRunCountValid("followup-while-running", count)).toBe(true); + for (const count of [undefined, 0, 3, NaN, 1.5]) expect(agentJourneyRunCountValid("followup-while-running", count)).toBe(false); + expect(agentJourneyRunCountValid("build-revise", 1)).toBe(false); + const config = { version: 1, id: "qa", mode: "existing-cloud", adapterModule: "/private/provider.mjs", journeys: ["followup-while-running"], + executionEnvironment: "daytona", baseline: { features: {}, connections: [] }, limits: { durationMs: 600_000, modelBudgetCents: 100, maxRuns: 2 } }; + expect(preparationPreview(parseTargetConfig(config)).journeys[0]).toMatchObject({ id: "followup-while-running", paid: true }); + expect(() => parseTargetConfig({ ...config, limits: { ...config.limits, maxRuns: 1 } })).toThrow("agent_journeys_require_bounded_runs"); + }); + it("retains one-run and two-run passes only with the whole success contract", () => { + const result = { schema: "paperclip.runner-e2e.result/v2", executionId: "product-journeys.qa.configured-agent.daytona.followup-while-running", + suiteId: "product-journeys", suiteDefinitionHash: "test", profileId: "configured-agent", environmentId: "daytona", caseId: "followup-while-running", + provider: "opencode", model: "configured-model", runtimeMode: "native", status: "passed", cleanup: "passed", attempt: 1, + startedAt: "2026-01-01T00:00:00Z", finishedAt: "2026-01-01T00:00:01Z", durationMs: 1000, runIds: ["run"], + productJourney: { outcome: "passed", startingState: "recipe", privacy: "private", resourceDisposition: "retained", + phases: [{ phase: "journey", outcome: "passed" }], checks: journeyRequiredChecks["followup-while-running"]!.map(id => ({ id, passed: true })), + deployment: { kind: "cloud", targetId: "qa", origin: "https://example.test", commit: "a".repeat(40), cloudRelease: "b".repeat(40), runtimeImage: "pinned", configurationHash: "test" } } }; + for (const runIds of [["run"], ["run", "successor"]]) expect(() => validateRetainedRunnerResult({ ...result, runIds })).not.toThrow(); + for (const runIds of [[], ["a", "b", "c"]]) expect(() => validateRetainedRunnerResult({ ...result, runIds })).toThrow("result.agentJourneyRun"); + for (const check of result.productJourney.checks) expect(() => validateRetainedRunnerResult({ ...result, productJourney: { ...result.productJourney, + checks: result.productJourney.checks.filter(row => row.id !== check.id) } })).toThrow("result.productJourney"); + }); +}); diff --git a/tests/runner-e2e/product-journeys/agent-flows.ts b/tests/runner-e2e/product-journeys/agent-flows.ts index 37ccb8995bf..0cc797c0ae5 100644 --- a/tests/runner-e2e/product-journeys/agent-flows.ts +++ b/tests/runner-e2e/product-journeys/agent-flows.ts @@ -18,7 +18,7 @@ import { agentJourneyRuns } from "./catalog.js"; import { JourneyError, type JourneyContext, type JourneyEvidence, type TargetSession } from "./types.js"; type Row = Record; -type AgentJourney = Exclude; +type AgentJourney = Exclude; interface TaskState { currentIssue: Row & { id: string; status: string }; taskRuns: Array; comments: Row[]; interactions: Array; attachments: StoryDelivery[] } interface Options { deadlineAt: number; directory: string; capture(id: string, label: string, file: string): Promise } const active = (run: Row) => ["queued", "running"].includes(run.status); @@ -46,7 +46,7 @@ export function validateAgentTaskState(context: JourneyContext, issueId: string, } } -async function waitFor(deadlineAt: number, load: () => Promise, accept: (value: T) => boolean, code: string): Promise { +export async function waitFor(deadlineAt: number, load: () => Promise, accept: (value: T) => boolean, code: string): Promise { while (Date.now() < deadlineAt) { const value = await load(); if (accept(value)) return value; diff --git a/tests/runner-e2e/product-journeys/catalog.ts b/tests/runner-e2e/product-journeys/catalog.ts index 3d40c2cdc66..3013851a360 100644 --- a/tests/runner-e2e/product-journeys/catalog.ts +++ b/tests/runner-e2e/product-journeys/catalog.ts @@ -1,5 +1,5 @@ import { agentJourneyRuns } from "./success-contract.js"; -export { agentJourneyRuns } from "./success-contract.js"; +export { agentJourneyRuns, agentJourneyRunCountValid } from "./success-contract.js"; import type { JourneyId, TargetConfig } from "./types.js"; export const productJourneys = [ { id: "task-create", label: "Create a task for yourself and reopen it", requires: ["company", "board-user", "task-write"], capabilities: ["tasks/create"], paid: false }, @@ -13,6 +13,7 @@ export const productJourneys = [ { id: "protected-actions", label: "Approve and decline protected service actions", requires: ["company", "board-user", "task-write", "agent", "review-service"], capabilities: ["questions-and-approvals/tool-review"], paid: true }, { id: "create-skill-studio", label: "Create a company skill and persist a Skill Studio edit", requires: ["company", "board-user", "task-write", "agent", "skill-write"], capabilities: ["skills/authoring"], paid: true }, { id: "stop-redirect", label: "Stop active work and send a new direction once", requires: ["company", "board-user", "task-write", "agent"], capabilities: ["steering/stop-and-pause"], paid: true }, + { id: "followup-while-running", label: "Send a follow-up during active work and verify its saved result", requires: ["company", "board-user", "task-write", "agent"], capabilities: ["steering/queue"], paid: true }, { id: "cloud-onboarding", label: "Create a Cloud workspace and complete a first task", requires: ["cloud-account", "fresh-workspace", "provider-key"], capabilities: ["onboarding/first-task"], paid: true }, ] as const; diff --git a/tests/runner-e2e/product-journeys/clarification-flow.ts b/tests/runner-e2e/product-journeys/clarification-flow.ts index 932efbebf92..800a747284d 100644 --- a/tests/runner-e2e/product-journeys/clarification-flow.ts +++ b/tests/runner-e2e/product-journeys/clarification-flow.ts @@ -1,6 +1,6 @@ import { mkdir, writeFile } from "node:fs/promises"; import path from "node:path"; -import { expect } from "@playwright/test"; +import { expect, type Locator } from "@playwright/test"; import { continuationScenario } from "../continuation-cases.js"; import { continuationAnswerCommitted, continuationCheckpointReady } from "../continuation-readiness.js"; import type { ContinuationCheckpoint } from "../continuation-scoring.js"; @@ -13,6 +13,19 @@ import { JourneyError, type JourneyContext, type JourneyEvidence, type TargetSes type Row = Record; interface Options { deadlineAt: number; directory: string; capture(id: string, label: string, file: string): Promise } +export async function waitForClarificationDocument(visible: Locator, answer: string, deadlineAt: number) { + const remaining = () => { + const timeout = deadlineAt - Date.now(); + if (!Number.isFinite(timeout) || timeout <= 0) throw new JourneyError("failed", "clarification_document_not_ready"); + return timeout; + }; + await expect(visible).toHaveCount(1, { timeout: remaining() }); + await expect(visible).toBeVisible({ timeout: remaining() }); + await expect(visible).toContainText(answer, { timeout: remaining() }); + await expect(visible).toContainText(/afternoon/i, { timeout: remaining() }); + remaining(); +} + export async function readClarificationState(context: JourneyContext, issueId: string) { const route = `/api/issues/${issueId}`; const [currentIssue, summaries, interactions, documents, attachments, children] = await Promise.all([ @@ -96,8 +109,7 @@ export async function runClarificationJourney(context: JourneyContext, session: await context.page.goto(new URL(href, context.api.origin).href, { waitUntil: "domcontentloaded" }); await context.page.reload({ waitUntil: "domcontentloaded" }); const visible = context.page.locator(`[id=${JSON.stringify(`document-${document.key}`)}]:visible, [id=${JSON.stringify(`side-panel-content-document:${document.key}`)}]:visible`); - await expect(visible).toHaveCount(1); await expect(visible).toBeVisible(); - await expect(visible).toContainText(scenario.answer); await expect(visible).toContainText(/afternoon/i); + await waitForClarificationDocument(visible, scenario.answer, options.deadlineAt); const persisted = await context.api.get(`/api/issues/${issue.id}/documents/${encodeURIComponent(document.key)}`); readback = { issueId: persisted.issueId, key: persisted.key, revisionId: persisted.latestRevisionId, body: persisted.body, visibleBody: await visible.innerText() }; await options.capture("reloaded", "Saved answers after reload", "clarification-reloaded.png"); diff --git a/tests/runner-e2e/product-journeys/clarification-readback.test.ts b/tests/runner-e2e/product-journeys/clarification-readback.test.ts new file mode 100644 index 00000000000..5ec51654630 --- /dev/null +++ b/tests/runner-e2e/product-journeys/clarification-readback.test.ts @@ -0,0 +1,49 @@ +import { afterEach, expect, it, vi } from "vitest"; +import type { Locator } from "@playwright/test"; + +const clock = vi.hoisted(() => ({ now: 0, locator: {} })); +const answer = "Use AMBERreadback."; +const body = `Welcome to the afternoon meetup. ${answer}`; + +function stateAt(now: number) { + return { count: now >= 100 ? 1 : 0, visible: now >= 100, + text: now >= 1_300 ? body : now >= 600 ? answer : "Loading" }; +} + +vi.mock("@playwright/test", async importOriginal => { + const actual = await importOriginal(); + const wait = (accept: (state: ReturnType) => boolean, options: { timeout: number }) => { + if (!Number.isFinite(options.timeout) || options.timeout <= 0) throw Error("Invalid wait budget"); + if (accept(stateAt(clock.now))) return; + const deadline = clock.now + options.timeout; + const readyAt = [100, 600, 1_300].find(time => time > clock.now && time <= deadline && accept(stateAt(time))); + clock.now = readyAt ?? deadline; + if (readyAt === undefined) throw Error("Document not ready within wait budget"); + }; + return { ...actual, expect: (locator: Locator) => { + if (locator !== clock.locator) throw Error("Unexpected document locator"); + return { + async toHaveCount(count: number, options: { timeout: number }) { wait(state => state.count === count, options); }, + async toBeVisible(options: { timeout: number }) { wait(state => state.visible, options); }, + async toContainText(text: string | RegExp, options: { timeout: number }) { + wait(state => typeof text === "string" ? state.text.includes(text) : text.test(state.text), options); + }, + }; + } }; +}); + +import { waitForClarificationDocument } from "./clarification-flow.js"; + +afterEach(() => vi.restoreAllMocks()); + +it("shares one document deadline and retains rejection evidence through a later scheduler stall", async () => { + clock.now = 0; + vi.spyOn(Date, "now").mockImplementation(() => clock.now); + const observed = await waitForClarificationDocument(clock.locator as Locator, answer, 1_000).then( + () => ({ outcome: "resolved", at: clock.now, text: stateAt(clock.now).text }), + (error: Error) => ({ outcome: "rejected", error: error.message, at: clock.now, text: stateAt(clock.now).text }), + ); + clock.now += 10_000; + expect(stateAt(clock.now).text).toBe(body); + expect(observed).toEqual({ outcome: "rejected", error: "Document not ready within wait budget", at: 1_000, text: answer }); +}); diff --git a/tests/runner-e2e/product-journeys/launch.ts b/tests/runner-e2e/product-journeys/launch.ts index 4be973f1e81..72706d53ba9 100644 --- a/tests/runner-e2e/product-journeys/launch.ts +++ b/tests/runner-e2e/product-journeys/launch.ts @@ -14,6 +14,7 @@ import { taskSearchFilter } from "./task-search-filter.js"; import { askQuestion, taskCreate } from "./basic-flows.js"; import { projectTaskContext } from "./project-context-flow.js"; import { runAgentJourney } from "./agent-flows.js"; +import { runActiveFollowup } from "./active-followup-flow.js"; import { runClarificationJourney } from "./clarification-flow.js"; import { JourneyError, type JourneyContext, type JourneyEvidence, type JourneyOutcome, type TargetLifecycleInput, type TargetProvider, type TargetSession } from "./types.js"; import type { RunnerE2EResult } from "../types.js"; @@ -106,7 +107,7 @@ export async function launchProductJourneys(args: string[], repositoryRoot: stri try { sha = execFileSync("git", ["rev-parse", "HEAD"], { cwd: repositoryRoot, encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }).trim(); } catch { /* Packaged harnesses may not include Git metadata. */ } const source = resolveRunnerE2ESource({ sha, ref: null, workflowRunUrl: null }); const definitionHash = createHash("sha256").update(JSON.stringify(productJourneys)); - for (const name of ["catalog.ts", "task-properties.ts", "task-search-filter.ts", "task-search-evidence.ts", "basic-flows.ts", "basic-evidence.ts", "project-context-flow.ts", "project-context-readback.ts", "project-context-evidence.ts", "agent-flows.ts", "clarification-flow.ts", "clarification-evidence.ts", "../question-resume-ui.ts", "../question-resume-scoring.ts", "../runtime-question-readiness.ts", "../chat-flow.ts", "../continuation-readiness.ts", "../continuation-cases.ts", "success-contract.ts", "product-screenshot.ts", "cloud-onboarding.ts", "onboarding-connect.ts", "onboarding-first-task.ts", "workspace-deferral.ts", "onboarding-pre-acceptance.ts", "artifact-evidence.ts", "fixtures.ts", "launch.ts", "types.ts", "collection.ts", "local-target.ts"]) definitionHash.update(await readFile(path.join(import.meta.dirname, name))); + for (const name of ["catalog.ts", "task-properties.ts", "task-search-filter.ts", "task-search-evidence.ts", "basic-flows.ts", "basic-evidence.ts", "project-context-flow.ts", "project-context-readback.ts", "project-context-evidence.ts", "agent-flows.ts", "active-followup-flow.ts", "active-followup-evidence.ts", "clarification-flow.ts", "clarification-evidence.ts", "../question-resume-ui.ts", "../question-resume-scoring.ts", "../runtime-question-readiness.ts", "../chat-flow.ts", "../continuation-readiness.ts", "../continuation-cases.ts", "success-contract.ts", "product-screenshot.ts", "cloud-onboarding.ts", "onboarding-connect.ts", "onboarding-first-task.ts", "workspace-deferral.ts", "onboarding-pre-acceptance.ts", "artifact-evidence.ts", "fixtures.ts", "launch.ts", "types.ts", "collection.ts", "local-target.ts"]) definitionHash.update(await readFile(path.join(import.meta.dirname, name))); for (const file of ["catalog.ts", "api.ts", "run-observations.ts", "native-task-reply.ts", "native-event-envelope.ts", "user-actions.ts", "plan-revision-flow.ts", "skill-studio-flow.ts", "protected-action-flow.ts", "story-artifact.ts", "stop-redirect-flow.ts", "everyday-cases.ts", "everyday-delivery.ts", "everyday-decisions.ts", "connection-guidance-evidence.ts", "everyday-artifact.py"]) definitionHash.update(await readFile(path.join(import.meta.dirname, "..", file))); definitionHash.update(runnerSuites.flatMap(suite => suite.tasks).find(task => task.id === "ask-question")!.buildPrompt("definition")); const suiteDefinitionHash = definitionHash.digest("hex"); @@ -203,6 +204,8 @@ export async function launchProductJourneys(args: string[], repositoryRoot: stri result.model = ready.agent.model; result.runtimeMode = ready.agent.runtimeMode; if (id === "ask-question") await askQuestion(ready, session!, campaignId, evidence); + else if (id === "followup-while-running") await runActiveFollowup(ready, session!, campaignId, evidence, + { deadlineAt, directory: path.join(outputDir, "private-stories", id) }); else if (id === "question-answer-resume") await runClarificationJourney(ready, session!, campaignId, evidence, { deadlineAt, directory: path.join(outputDir, "private-stories", id), capture: async (_id, _label, file) => { diff --git a/tests/runner-e2e/product-journeys/success-contract.ts b/tests/runner-e2e/product-journeys/success-contract.ts index f49db32a938..67d37578e3e 100644 --- a/tests/runner-e2e/product-journeys/success-contract.ts +++ b/tests/runner-e2e/product-journeys/success-contract.ts @@ -1,11 +1,11 @@ /** Closed, target-independent success requirements used by dispatch and retained evidence. */ export const agentJourneyRuns = { "ask-question": 1, "plan-revise-accept": 3, "build-revise": 2, - "protected-actions": 4, "create-skill-studio": 1, "stop-redirect": 3, "question-answer-resume": 3 } as const; + "followup-while-running": 2, "protected-actions": 4, "create-skill-studio": 1, "stop-redirect": 3, "question-answer-resume": 3 } as const; export function agentJourneyRunCountValid(id: string, count: unknown): boolean { if (!Object.hasOwn(agentJourneyRuns, id) || !Number.isSafeInteger(count)) return false; const maximum = agentJourneyRuns[id as keyof typeof agentJourneyRuns]; - const minimum = id === "question-answer-resume" ? 1 : maximum; + const minimum = id === "question-answer-resume" || id === "followup-while-running" ? 1 : maximum; return (count as number) >= minimum && (count as number) <= maximum; } export const journeyRequiredChecks: Record = { @@ -24,5 +24,6 @@ export const journeyRequiredChecks: Record = { "create-skill-studio": ["created-skill-persisted", "skill-creation-attributed", "skill-studio-edit-persisted", "configured-model-and-environment"], "stop-redirect": ["checkpoint-artifact-verified", "active-work-stopped", "stop-native-terminal-retained", "new-direction-completed", "stopped-run-remained-terminal", "stopped-run-no-later-events", "stop-preserved-download", "redirect-consumed-once", "user-reply-stored-once", "configured-model-and-environment"], + "followup-while-running": ["followup-native-command-active", "followup-stored-once", "followup-consumed-once", "followup-final-answer-persisted", "followup-one-or-two-native-runs", "configured-model-and-environment"], "cloud-onboarding": ["fresh-workspace", "ui-connection-and-native-agent", "first-task-complete", "downloadable-artifact-verified", "native-trace-verified"], }; diff --git a/tests/runner-e2e/product-journeys/types.ts b/tests/runner-e2e/product-journeys/types.ts index 0f899efca5e..24f57c2cf7f 100644 --- a/tests/runner-e2e/product-journeys/types.ts +++ b/tests/runner-e2e/product-journeys/types.ts @@ -3,7 +3,7 @@ import type { ConnectionApi } from "../connection-target.js"; export type JourneyOutcome = "passed" | "failed" | "blocked" | "unsupported"; export type JourneyId = "task-properties" | "task-create" | "project-task-context" | "ask-question" | "cloud-onboarding" - | "question-answer-resume" | "task-search-filter" | "plan-revise-accept" | "build-revise" | "protected-actions" | "create-skill-studio" | "stop-redirect"; + | "question-answer-resume" | "task-search-filter" | "followup-while-running" | "plan-revise-accept" | "build-revise" | "protected-actions" | "create-skill-studio" | "stop-redirect"; export type TargetMode = "managed-self-hosted" | "existing-cloud" | "new-cloud"; export interface DeploymentIdentity { kind: "self-hosted" | "cloud"; diff --git a/tests/runner-e2e/report-catalog.test.ts b/tests/runner-e2e/report-catalog.test.ts index 28929783eb1..09d6bd6d6d7 100644 --- a/tests/runner-e2e/report-catalog.test.ts +++ b/tests/runner-e2e/report-catalog.test.ts @@ -11,6 +11,7 @@ import { buildRunnerCampaign } from "./history.js"; import { renderRunnerE2EDashboard } from "./dashboard.js"; import { regenerateRunnerDashboard } from "./dashboard-regenerate.js"; import { journeyCapabilities, journeyFeatureIds } from "./journey-features.js"; +import { journeyRequiredChecks } from "./product-journeys/success-contract.js"; import type { RunnerE2EResult } from "./types.js"; const temporary: string[] = []; @@ -91,6 +92,30 @@ describe("trusted report catalog discovery", () => { expect(journeyCapabilities({ ...product, task: { ...known.task, id: "task-search-filter" } }).map(capability => capability.id)).toEqual(["tasks/organize"]); }); + it.each([ + { caseId: "question-answer-resume", feature: "questions-and-approvals", capabilities: ["questions-and-approvals/questions", "questions-and-approvals/continuation"] }, + { caseId: "followup-while-running", feature: "steering", capabilities: ["steering/queue"] }, + ])("reports deployment journey $caseId in its feature coverage", ({ caseId, feature, capabilities }) => { + const known = runnerMatrix.find(execution => execution.id === "core-compatibility.legacy-codex.local.ask-question")!; + const execution = { ...known, id: `product-journeys.${known.profile.id}.${known.environment.id}.${caseId}`, + suite: { ...known.suite, id: "product-journeys" }, task: { ...known.task, id: caseId } }; + expect(journeyCapabilities(execution).map(capability => capability.id)).toEqual(capabilities); + expect(journeyFeatureIds(execution)).toEqual([feature]); + expect(journeyCapabilities({ ...execution, suite: { ...execution.suite, id: "unrecognized-suite" } })).toEqual([]); + const page = renderRunnerE2EDashboard({ title: "Deployment coverage", generatedAt: "2026-10-20T12:00:00Z", + expected: [execution.id], catalog: [execution], entries: [{ valid: true, errors: [], + result: { ...result(), executionId: execution.id, suiteId: execution.suite.id, profileId: execution.profile.id, + environmentId: execution.environment.id, caseId, provider: "opencode", runIds: ["run-1"], + productJourney: { targetId: "qa", outcome: "passed", startingState: "recipe", privacy: "private", resourceDisposition: "retained", + phases: [{ phase: "journey", outcome: "passed" }], checks: journeyRequiredChecks[caseId]!.map(id => ({ id, passed: true })), + deployment: { kind: "cloud", targetId: "qa", origin: "https://qa.example.test", commit: "a".repeat(40), + cloudRelease: "b".repeat(40), runtimeImage: "immutable-image", configurationHash: "config" } } } }] }); + expect(page).toContain(`data-execution-id="${execution.id}"`); + expect(page).toContain(`data-report-features="${feature}"`); + expect(page).toContain(`title="${capabilities.join(", ")}"`); + expect(page).toContain("1/1 passed"); + }); + it("counts the project/task deployment journey toward projects and task creation coverage", () => { const known = runnerMatrix.find(execution => execution.id === "core-compatibility.legacy-codex.local.ask-question")!; const project = { ...known, suite: { ...known.suite, id: "product-journeys" }, task: { ...known.task, id: "project-task-context" } }; From 9e904ee5194f6f126701f1798faf792695a1d327 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 30 Oct 2026 16:49:20 -0700 Subject: [PATCH 4/8] perf(server): overlap independent heartbeat preparation reads (#18372) --- doc/observability.md | 8 + .../heartbeat-execution-configuration.test.ts | 57 ++++++- .../__tests__/heartbeat-task-context.test.ts | 100 ++++++++++- server/src/services/heartbeat.ts | 47 +----- .../heartbeat/execution-configuration.ts | 47 ++++-- .../heartbeat/run-preparation.test.ts | 43 +++++ .../src/services/heartbeat/run-preparation.ts | 155 +++++++----------- server/src/services/heartbeat/task-context.ts | 92 +++++++---- .../runner-api.integration.test.ts | 2 +- 9 files changed, 354 insertions(+), 197 deletions(-) diff --git a/doc/observability.md b/doc/observability.md index e2bb4065caf..996d3a807f5 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -328,6 +328,14 @@ once known, `paperclip.native_reconciliation.candidates` (a count). A failed pass marks the pass span and the phase that was running with an error status. Faster passes record nothing. +Heartbeat configuration preparation overlaps independent reads. Skill preparation +runs alongside connector assignments; standing connection instructions wait for +assignment catalog repairs, but can overlap the skill branch. These spans can +finish out of order. Do not add their durations as serial elapsed time. The +historical `skills.prepare` span ends when the skill branch settles, rather than +when its concurrent branches finish. Each preparation attempt settles its started +branches before it returns an error or releases acquired credentials. + ### Detailed execution phases The `paperclip.execution` tracer adds these content-free spans beneath the current diff --git a/server/src/__tests__/heartbeat-execution-configuration.test.ts b/server/src/__tests__/heartbeat-execution-configuration.test.ts index 509a9cfa7e6..2cd52f84849 100644 --- a/server/src/__tests__/heartbeat-execution-configuration.test.ts +++ b/server/src/__tests__/heartbeat-execution-configuration.test.ts @@ -19,11 +19,20 @@ import { secretService } from "../services/secrets.js"; import { instanceSettingsService } from "../services/instance-settings.js"; import { aiConnectionService } from "../services/ai-connections.js"; import * as aiRuntime from "../services/ai-connection-runtime.js"; +import * as connectorRuntime from "../services/connector-runtime.js"; +import * as assignedMcp from "../services/native-runtime/assigned-mcp-tools.js"; import * as sentry from "../sentry.js"; import { waitForPendingRunFailureReports } from "../services/run-failure-report.js"; import { recoveryPostgresSupport, startRecoveryPostgres } from "./helpers/recovery-postgres.js"; +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise(complete => { resolve = complete; }); + return { promise, resolve }; +} + const skillFailures = vi.hoisted(() => new Map()); +const skillWaits = vi.hoisted(() => new Map Promise>()); describe("Runner home mode during recovery preparation", () => { it.each([ @@ -52,6 +61,7 @@ vi.mock("../services/company-skills.js", async importOriginal => { return { ...service, withRuntimeSkillEntries: async (companyId: string, options: Parameters[1], consume: (entries: Awaited>) => T | Promise) => { + await skillWaits.get(companyId)?.(); const failure = skillFailures.get(companyId); if (failure) throw new Error(failure); return service.withRuntimeSkillEntries(companyId, options, consume); @@ -91,7 +101,7 @@ describePostgres("heartbeat execution configuration through adapter dispatch", ( testEnvironment: async () => ({ adapterType, status: "pass", checks: [], testedAt: new Date().toISOString() }), }); }, 60_000); - afterEach(() => { vi.restoreAllMocks(); vi.unstubAllEnvs(); skillFailures.clear(); }); + afterEach(() => { vi.restoreAllMocks(); vi.unstubAllEnvs(); skillFailures.clear(); skillWaits.clear(); }); afterAll(async () => { await heartbeat?.drainActiveRunExecutions(); unregisterServerAdapter(adapterType); @@ -288,6 +298,45 @@ describePostgres("heartbeat execution configuration through adapter dispatch", ( const [unchangedAgent] = await db.select().from(agents).where(eq(agents.id, f.agentId)); expect(unchangedAgent.adapterConfig).toMatchObject({ paperclipSkillSync: { desiredSkills: [other.key] } }); }, 60_000); + it("overlaps skills with assignments and dependent connection instructions before composition", async () => { + const f = await seed(); + const skillGate = deferred(); + skillWaits.set(f.companyId, () => skillGate.promise); + const assignments = vi.spyOn(connectorRuntime, "resolveConnectorAssignments"); + const instructions = vi.spyOn(assignedMcp, "resolveAssignedConnectionInstructionsForRun"); + const configured = await configuration(f, { provider: "codex", model: "gpt-6.1-sol", homeMode: "isolated" }); + const opening = configured.prepare(); + try { + await vi.waitFor(() => { + expect(assignments).toHaveBeenCalledOnce(); + expect(instructions).toHaveBeenCalledOnce(); + }); + } finally { + skillGate.resolve(); + } + expect(await opening).toMatchObject({ prepared: true, runtimeConfig: { homeMode: "isolated", model: "gpt-6.1-sol" } }); + }); + + it("settles concurrent configuration reads before exposing the first preparation failure", async () => { + const f = await seed(); + const assignmentGate = deferred>>(); + const assignments = vi.spyOn(connectorRuntime, "resolveConnectorAssignments").mockReturnValue(assignmentGate.promise); + const instructions = vi.spyOn(assignedMcp, "resolveAssignedConnectionInstructionsForRun").mockRejectedValue(new Error("later instruction failure")); + skillFailures.set(f.companyId, "first skill failure"); + const configured = await configuration(f, { provider: "codex", homeMode: "isolated" }); + let settled = false; + const opening = configured.prepare(); + void opening.then(() => { settled = true; }, () => { settled = true; }); + try { + await vi.waitFor(() => expect(assignments).toHaveBeenCalledOnce()); + await Promise.resolve(); + expect(settled).toBe(false); + expect(instructions).not.toHaveBeenCalled(); + } finally { + assignmentGate.resolve([]); + } + await expect(opening).rejects.toThrow("first skill failure"); + }); it("does not install new environment declarations when recovering a saved skill selection", async () => { const f = await seed(), pluginId = randomUUID(), pluginKey = `fixture.${randomUUID()}`; @@ -445,11 +494,11 @@ describePostgres("heartbeat execution configuration through adapter dispatch", ( const runIdHash = createHash("sha256").update(finished.id).digest("hex").slice(0, 12); const steps = exporter.getFinishedSpans().filter(span => span.name.startsWith("heartbeat.prepare.configuration_") && span.attributes["paperclip.task.run.run_id"] === runIdHash); - expect(steps.map(span => span.name)).toEqual([ + expect(steps.map(span => span.name).sort()).toEqual([ "configuration_mentions", "configuration_github_identity", "configuration_secrets", "configuration_ai_runtime", "configuration_skills", "configuration_connector_assignments", "configuration_connector_skills", "configuration_connector_delivery", "configuration_connection_instructions", "configuration_agent_identity", - ].map(step => `heartbeat.prepare.${step}`)); + ].map(step => `heartbeat.prepare.${step}`).sort()); for (const span of steps) { expect(span.status).toEqual({ code: 0 }); expect(span.attributes).toEqual({ "paperclip.task.run.run_id": runIdHash }); @@ -494,7 +543,7 @@ describePostgres("heartbeat execution configuration through adapter dispatch", ( && span.attributes["paperclip.task.run.run_id"] === runIdHash); expect(steps.find(span => span.name === "heartbeat.prepare.configuration_ai_runtime")?.status.code).toBe(0); expect(steps.find(span => span.name === "heartbeat.prepare.configuration_skills")?.status).toEqual({ code: 2 }); - expect(steps.some(span => span.name === "heartbeat.prepare.configuration_connector_assignments")).toBe(false); + expect(steps.some(span => span.name === "heartbeat.prepare.configuration_connector_assignments")).toBe(true); for (const span of steps) { expect(span.attributes).toEqual({ "paperclip.task.run.run_id": runIdHash }); expect(span.events).toEqual([]); diff --git a/server/src/__tests__/heartbeat-task-context.test.ts b/server/src/__tests__/heartbeat-task-context.test.ts index 5449d7def6c..e073db37676 100644 --- a/server/src/__tests__/heartbeat-task-context.test.ts +++ b/server/src/__tests__/heartbeat-task-context.test.ts @@ -1,10 +1,11 @@ import { createHash, randomUUID } from "node:crypto"; import { eq } from "drizzle-orm"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, authUsers, companies, companyMemberships, createDb, heartbeatRuns, issueComments, issues } from "@paperclipai/db"; import { FAST_RESPONSE_AGENT_GUIDANCE } from "@paperclipai/shared"; import { registerServerAdapter, unregisterServerAdapter, type AdapterExecutionContext } from "../adapters/index.js"; import { heartbeatService } from "../services/heartbeat.js"; +import { prepareHeartbeatTaskContext } from "../services/heartbeat/task-context.js"; import { documentService } from "../services/documents.js"; import { ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY } from "../services/issue-continuation-summary.js"; import { createRunSecretRedactionRegistry } from "../services/run-secret-redaction.js"; @@ -19,6 +20,7 @@ if (!support.supported) console.warn(`Skipping heartbeat task context tests: ${s describePostgres("heartbeat task context delivered to an adapter", () => { const adapterType = "task_context_test"; const captured = new Map(); + const inspectPreparation = new Map void>(); let temporary: Awaited>; let db: ReturnType; let heartbeat: ReturnType; @@ -33,6 +35,7 @@ describePostgres("heartbeat task context delivered to an adapter", () => { type: adapterType, supportsLocalAgentJwt: false, execute: async (input) => { + inspectPreparation.get(input.agent.id)?.(); captured.set(input.runId, input); return { exitCode: 0, signal: null, timedOut: false, sessionId: "context-session", resultJson: {} }; }, @@ -118,6 +121,101 @@ describePostgres("heartbeat task context delivered to an adapter", () => { }); }); + it("reads selected comments once for both wake projections", async () => { + const f = await seed(); + const commentId = randomUUID(); + await db.insert(issueComments).values({ + id: commentId, companyId: f.companyId, issueId: f.issueId, authorType: "user", authorUserId: "context-user", + body: "selected wake canary", + }); + const select = vi.spyOn(db, "select"); + inspectPreparation.set(f.agentId, () => { + const commentReads = select.mock.calls.filter(([projection]) => projection + && projection.body === issueComments.body && projection.id === issueComments.id && projection.deletedAt === issueComments.deletedAt); + expect(commentReads).toHaveLength(1); + }); + try { + const { context } = await execute(f, { wakeCommentId: commentId, wakeCommentIds: [commentId] }); + expect(context.paperclipWakeComment).toMatchObject({ id: commentId, body: "selected wake canary" }); + expect(context.paperclipWake).toMatchObject({ comments: [{ id: commentId, body: "selected wake canary" }] }); + } finally { + inspectPreparation.delete(f.agentId); + select.mockRestore(); + } + }); + + it("cancels deleted-only queued comments before adapter dispatch", async () => { + const f = await seed(), commentId = randomUUID(); + await db.insert(issueComments).values({ id: commentId, companyId: f.companyId, issueId: f.issueId, + authorType: "user", authorUserId: "context-user", body: "discarded wake", deletedAt: new Date() }); + const run = await heartbeat.wakeup(f.agentId, { source: "on_demand", triggerDetail: "manual", reason: "issue_commented", + payload: { issueId: f.issueId }, contextSnapshot: { issueId: f.issueId, wakeCommentId: commentId, wakeCommentIds: [commentId] } }); + expect(run).not.toBeNull(); + await heartbeat.drainActiveRunExecutions(); + const finished = await heartbeat.getRun(run!.id); + expect(finished).toMatchObject({ status: "cancelled", error: "Queued messages were discarded before dispatch" }); + expect(captured.has(run!.id)).toBe(false); + }); + + it("preserves deleted comment metadata without delivering its body", async () => { + const f = await seed(); + const commentId = randomUUID(); + await db.insert(issueComments).values({ id: commentId, companyId: f.companyId, issueId: f.issueId, + authorType: "user", authorUserId: "context-user", body: "deleted wake canary", deletedAt: new Date(), + presentation: { fixture: "deleted" }, metadata: { fixture: "deleted" } }); + const [run] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, + status: "running", issueId: f.issueId }).returning(); + const [agent] = await db.select().from(agents).where(eq(agents.id, f.agentId)); + const [issueRef] = await db.select().from(issues).where(eq(issues.id, f.issueId)); + const context: Record = { issueId: f.issueId, wakeCommentId: commentId, wakeCommentIds: [commentId] }; + await prepareHeartbeatTaskContext(db, { + run, agent, task: { issueRef, issueContext: null, context, taskSession: null }, + policy: { trustPreset: { kind: "standard" }, experimentalInstanceSettings: {} }, + services: { issuesSvc: { getAncestors: async () => [] }, getPinnedSkillTestContext: async () => null }, + }); + expect(context.paperclipWakeComment).toMatchObject({ id: commentId, body: "", presentation: null, metadata: null }); + expect(context.paperclipWake).toMatchObject({ comments: [{ id: commentId, body: "", presentation: null, metadata: null }] }); + expect(context.paperclipTaskMarkdown).not.toContain("deleted wake canary"); + }); + + it.each([false, true])("uses admitted skill-test scope when recovery is %s", async (recovering) => { + const f = await seed(); + const [run] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, + status: "running", issueId: f.issueId }).returning(); + const [agent] = await db.select().from(agents).where(eq(agents.id, f.agentId)); + const [issueRef] = await db.select().from(issues).where(eq(issues.id, f.issueId)); + const context: Record = { issueId: f.issueId, paperclipAgentJwtScope: { kind: "skill_test", issueId: f.issueId } }; + const pinned = vi.fn(async () => null); + const preparing = prepareHeartbeatTaskContext(db, { + run, agent, task: { issueRef, issueContext: null, context, taskSession: null }, + policy: { recovering, trustPreset: { kind: "standard" }, experimentalInstanceSettings: {} }, + services: { issuesSvc: { getAncestors: async () => [] }, getPinnedSkillTestContext: pinned }, + }); + if (recovering) await expect(preparing).rejects.toThrow("skill test's saved version is unavailable"); + else await preparing; + expect(pinned).toHaveBeenCalledTimes(recovering ? 1 : 0); + expect(context.paperclipAgentJwtScope).toEqual({ kind: recovering ? "skill_test" : "standard", ...(recovering ? { issueId: f.issueId } : {}) }); + }); + + it("retains the context issue scope when no issue reference is available", async () => { + const f = await seed(); + const otherIssueId = randomUUID(), commentId = randomUUID(); + await db.insert(issues).values({ id: otherIssueId, companyId: f.companyId, title: "Other issue", issueNumber: 2 }); + await db.insert(issueComments).values({ id: commentId, companyId: f.companyId, issueId: otherIssueId, + authorType: "user", authorUserId: "context-user", body: "other issue canary" }); + const [run] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, + status: "running", issueId: f.issueId }).returning(); + const [agent] = await db.select().from(agents).where(eq(agents.id, f.agentId)); + const context: Record = { issueId: f.issueId, wakeCommentId: commentId, wakeCommentIds: [commentId] }; + await prepareHeartbeatTaskContext(db, { + run, agent, task: { issueRef: null, issueContext: null, context, taskSession: null }, + policy: { trustPreset: { kind: "standard" }, experimentalInstanceSettings: {} }, + services: { issuesSvc: { getAncestors: async () => [] }, getPinnedSkillTestContext: async () => null }, + }); + expect(JSON.stringify(context.paperclipWake)).not.toContain("other issue canary"); + expect(context.paperclipWakeComment).toBeUndefined(); + }); + it("quarantines low-trust summaries and comments before preparing higher-trust prompts", async () => { const f = await seed(); const sourceTrust = buildLowTrustSourceTrust({ issueId: f.issueId }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index eb655fdef25..20b8ebe11c4 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -2007,8 +2007,11 @@ export function heartbeatService( const dispatchIssueId = readNonEmptyString(parseObject(run.contextSnapshot).issueId); const resumingAdmittedConversationTurn = !!runOptions.nativeLeaseOwner && typeof run.contextSnapshot?.conversationSessionGeneration === "number"; - if (dispatchIssueId && isConversation(await getIssueExecutionContext(run.companyId, dispatchIssueId)) - && !resumingAdmittedConversationTurn && !(await instanceSettings.getExperimental()).enableAgentChat) { + const conversationIdentity = dispatchIssueId && !resumingAdmittedConversationTurn + ? await db.select({ conversationAgentId: issues.conversationAgentId, conversationUserId: issues.conversationUserId }) + .from(issues).where(and(eq(issues.companyId, run.companyId), eq(issues.id, dispatchIssueId))).then(rows => rows[0] ?? null) + : null; + if (isConversation(conversationIdentity) && !(await instanceSettings.getExperimental()).enableAgentChat) { await setRunStatus(run.id, "cancelled", { finishedAt: new Date(), error: "Agent Chat is disabled", errorCode: "agent_chat_disabled" }); await setWakeupStatus(run.wakeupRequestId, "cancelled", { finishedAt: new Date() }); await releaseIssueExecutionAndPromote((await getRun(run.id))!, { suppressImmediateRecovery: true }); @@ -2204,44 +2207,6 @@ export function heartbeatService( context[PAPERCLIP_EXTERNAL_CHAT_EXECUTION_BOUND_KEY] = true; } const wakeCommentId = deriveCommentId(context, null); - const wakeCommentContext = - issueContext && wakeCommentId - ? await db - .select({ - id: issueComments.id, - body: issueComments.body, - authorType: issueComments.authorType, - authorAgentId: issueComments.authorAgentId, - authorUserId: issueComments.authorUserId, - presentation: issueComments.presentation, - metadata: issueComments.metadata, - deletedAt: issueComments.deletedAt, - deletedByType: issueComments.deletedByType, - deletedByAgentId: issueComments.deletedByAgentId, - deletedByUserId: issueComments.deletedByUserId, - deletedByRunId: issueComments.deletedByRunId, - sourceTrust: issueComments.sourceTrust, - }) - .from(issueComments) - .where( - and( - eq(issueComments.id, wakeCommentId), - eq(issueComments.issueId, issueContext.id), - eq(issueComments.companyId, agent.companyId), - ), - ) - .then((rows) => { - const row = rows[0] ?? null; - return row?.deletedAt - ? { - ...row, - body: "", - presentation: null, - metadata: null, - } - : row; - }) - : null; let issueAssigneeOverrides = issueContext && issueContext.assigneeAgentId === agent.id ? parseIssueAssigneeAdapterOverrides( @@ -2610,7 +2575,7 @@ export function heartbeatService( continuationSummary, executionContinuation, paperclipWakePayload, safeWakeComments, safeWakeCommentContext, pinnedSkillTest, } = await timePreparation("task_context", () => prepareHeartbeatTaskContext(db, { run, agent, - task: { issueRef, issueContext, wakeCommentContext, context, taskSession }, + task: { issueRef, issueContext, context, taskSession }, policy: { trustPreset, experimentalInstanceSettings, recovering: Boolean(runOptions.nativeRestartRecovery || runOptions.nativeLeaseOwner) }, services: { issuesSvc, getPinnedSkillTestContext }, })); diff --git a/server/src/services/heartbeat/execution-configuration.ts b/server/src/services/heartbeat/execution-configuration.ts index 9dd767a3811..fb9626b932f 100644 --- a/server/src/services/heartbeat/execution-configuration.ts +++ b/server/src/services/heartbeat/execution-configuration.ts @@ -139,28 +139,27 @@ export async function prepareHeartbeatExecutionConfiguration(db: Db, input: Hear stripWorkspaceRuntimeFromExecutionRunConfig(mergedConfig); const skillActor: AuthorizationActor = { type: "agent", agentId: agent.id, companyId: agent.companyId, runId: run.id, source: "agent_jwt", onBehalfOfUserId: responsibleUserId === "local-board" ? null : responsibleUserId }; - const runScopedMentionedSkillKeys = - preconfiguredHome || pinnedSkillTest ? [] : await time("configuration_mentions", () => resolveRunScopedMentionedSkillKeys({ + const [mentionedSkills, githubIdentity] = await Promise.allSettled([ + preconfiguredHome || pinnedSkillTest ? Promise.resolve([]) : time("configuration_mentions", () => resolveRunScopedMentionedSkillKeys({ db, companyId: agent.companyId, issueId, commentIds: extractWakeCommentIds(context), actor: skillActor, - })); + })), + time("configuration_github_identity", () => resolveManagedGitHubIdentitySelection(db, agent.companyId, { + agentId: agent.id, responsibleUserId, allowStandingDelegation: false, + })), + ]); + if (mentionedSkills.status === "rejected") throw mentionedSkills.reason; + if (githubIdentity.status === "rejected") throw githubIdentity.reason; + const runScopedMentionedSkillKeys = mentionedSkills.value; const runScopedSkillKeys = !pinnedSkillTest && acceptedPlanContinuationWake && !acceptedPlanWakeRoutingDecision?.suppressAcceptedContinuation ? [...runScopedMentionedSkillKeys, ACCEPTED_PLAN_CONVERSION_SKILL_KEY] : runScopedMentionedSkillKeys; - const githubSelection = await time("configuration_github_identity", () => resolveManagedGitHubIdentitySelection( - db, - agent.companyId, - { - agentId: agent.id, - responsibleUserId, - allowStandingDelegation: false, - }, - )); + const githubSelection = githubIdentity.value; const useHostGitHub = !githubSelection.configured && trustPreset.kind === "standard" && @@ -274,6 +273,7 @@ export async function prepareHeartbeatExecutionConfiguration(db: Db, input: Hear ); const nativeRunnerPreparationSpans: NativeRunHistoricalSpan[] = []; const skillsPrepareStartedAtMs = Date.now(); + let skillsPrepareEndedAtMs = skillsPrepareStartedAtMs; let skillDeliveryConfig = effectiveResolvedConfig; const prepareSkills = async () => { @@ -346,24 +346,35 @@ export async function prepareHeartbeatExecutionConfiguration(db: Db, input: Hear throw error; } }; - const runtimeSkillEntries = preconfiguredHome ? [] : await time("configuration_skills", prepareSkills); + const skillsPreparation = (preconfiguredHome ? Promise.resolve([]) : time("configuration_skills", prepareSkills)) + .finally(() => { skillsPrepareEndedAtMs = Date.now(); }); + const assignmentsPreparation = preconfiguredHome ? Promise.resolve([]) : time("configuration_connector_assignments", () => resolveConnectorAssignments(db, { companyId: agent.companyId, agentId: agent.id, runId: run.id, issueId: typeof context.issueId === "string" ? context.issueId : undefined })); + const [preparedSkills, preparedAssignments, preparedInstructions] = await Promise.allSettled([ + skillsPreparation, + assignmentsPreparation, + assignmentsPreparation.then(() => time("configuration_connection_instructions", () => resolveAssignedConnectionInstructionsForRun(db, { companyId: agent.companyId, agentId: agent.id, runId: run.id }))), + ]); + if (preparedSkills.status === "rejected") throw preparedSkills.reason; + if (preparedAssignments.status === "rejected") throw preparedAssignments.reason; + if (preparedInstructions.status === "rejected") throw preparedInstructions.reason; + const runtimeSkillEntries = preparedSkills.value; nativeRunnerPreparationSpans.push({ name: "skills.prepare", parentName: "task.prepare", startedAtMs: skillsPrepareStartedAtMs, - endedAtMs: Date.now(), + endedAtMs: skillsPrepareEndedAtMs, }); - const connectorAssignments = preconfiguredHome ? [] : await time("configuration_connector_assignments", () => resolveConnectorAssignments(db, { companyId: agent.companyId, agentId: agent.id, runId: run.id, issueId: typeof context.issueId === "string" ? context.issueId : undefined })); + const connectorAssignments = preparedAssignments.value; const connectorSkillConfig = await time("configuration_connector_skills", () => applyConnectorSkills(skillDeliveryConfig, runtimeSkillEntries, connectorAssignments)); // Both CLI adapters and native context materialization use the same resolved set. runtimeSkillEntries.splice(0, runtimeSkillEntries.length, ...connectorSkillConfig.paperclipRuntimeSkills); const connectorDelivery = await time("configuration_connector_delivery", () => prepareConnectorSkillDelivery(connectorSkillConfig, agent.adapterType)); // Always replace this runtime-only field; caller wake data cannot supply skills. context.paperclipWake = { ...parseObject(context.paperclipWake), connectorSkillInstructions: connectorDelivery.instructions }; - let runtimeConfig = await time("configuration_connection_instructions", () => prepareConnectionInstructionDelivery({ - resolve: () => resolveAssignedConnectionInstructionsForRun(db, { companyId: agent.companyId, agentId: agent.id, runId: run.id }), + let runtimeConfig = await prepareConnectionInstructionDelivery({ + resolve: async () => preparedInstructions.value, context, config: connectorDelivery.config, native: agent.adapterType === "paperclip_runner", - })); + }); const agentIdentity = supportsManagedAgentIdentity(agent.adapterType, agent.adapterConfig, run.runtimeMode === "native" ? run.driverKind : undefined) ? await time("configuration_agent_identity", () => agentIdentityService(db).ensureAgentIdentity(agent.companyId, agent.id)) : undefined; diff --git a/server/src/services/heartbeat/run-preparation.test.ts b/server/src/services/heartbeat/run-preparation.test.ts index 10a447e1a74..7d5a070d91d 100644 --- a/server/src/services/heartbeat/run-preparation.test.ts +++ b/server/src/services/heartbeat/run-preparation.test.ts @@ -14,6 +14,7 @@ import { ConfigurationIncompleteFailure, createHeartbeatRunPreparation, resolveExecutionRunAdapterConfig, + readHeartbeatWakeComments, } from "./run-preparation.js"; // Existing integration suites exercise the real database and dispatch path. @@ -38,6 +39,12 @@ function databaseWithResults(...results: unknown[][]) { return { db: { select } as unknown as Db, select, scopes }; } +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise(complete => { resolve = complete; }); + return { promise, resolve }; +} + const emptyRoutineContext = { routineId: null, env: null, responsibleUserId: null }; describe("heartbeat run preparation module", () => { @@ -120,6 +127,42 @@ describe("heartbeat run preparation module", () => { expect(database.select).toHaveBeenCalledTimes(2); }); + it("scopes one selected-comment snapshot for both wake consumers", async () => { + const database = databaseWithResults([{ id: "comment-one", body: "selected wake" }]); + expect(await readHeartbeatWakeComments(database.db, "company-one", "issue-one", ["comment-one"])) + .toEqual([{ id: "comment-one", body: "selected wake" }]); + expect(database.scopes[0]).toEqual(expect.arrayContaining(["company-one", "issue-one", "comment-one"])); + expect(await readHeartbeatWakeComments(database.db, "company-one", "issue-one", [])).toEqual([]); + expect(database.select).toHaveBeenCalledOnce(); + }); + + it("settles independent binding checks before surfacing a failure or resolving values", async () => { + const projectCheck = deferred<[]>(); + const firstFailure = new Error("agent binding check failed"); + const secretsSvc = { + resolveAdapterConfigForRuntime: vi.fn(), resolveEnvBindings: vi.fn(), + collectMissingRuntimeBindings: vi.fn(async (_company, _env, context) => { + if (context.consumerType === "agent") throw firstFailure; + return context.consumerType === "project" ? projectCheck.promise : []; + }), + collectMissingAdapterConfigRuntimeBindings: vi.fn(async () => []), + }; + let settled = false; + const opening = resolveExecutionRunAdapterConfig({ + companyId: "company-one", agentId: "agent-one", projectId: "project-one", + environmentId: "environment-one", environmentEnv: { SHARED: "environment" }, + executionRunConfig: { env: { SHARED: "agent" } }, projectEnv: { SHARED: "project" }, secretsSvc, + }); + void opening.then(() => { settled = true; }, () => { settled = true; }); + await vi.waitFor(() => expect(secretsSvc.collectMissingRuntimeBindings).toHaveBeenCalledTimes(3)); + expect(secretsSvc.collectMissingAdapterConfigRuntimeBindings).toHaveBeenCalledOnce(); + expect(settled).toBe(false); + expect(secretsSvc.resolveAdapterConfigForRuntime).not.toHaveBeenCalled(); + expect(secretsSvc.resolveEnvBindings).not.toHaveBeenCalled(); + projectCheck.resolve([]); + await expect(opening).rejects.toBe(firstFailure); + }); + it("keeps missing scoped credentials as a pre-dispatch failure", async () => { const secretsSvc = { resolveAdapterConfigForRuntime: vi.fn(), diff --git a/server/src/services/heartbeat/run-preparation.ts b/server/src/services/heartbeat/run-preparation.ts index bf288599a51..bfbe6c862c1 100644 --- a/server/src/services/heartbeat/run-preparation.ts +++ b/server/src/services/heartbeat/run-preparation.ts @@ -394,76 +394,31 @@ export async function resolveExecutionRunAdapterConfig(input: { // no binding before resolving any secret value. Missing bindings short-circuit // to a configuration-incomplete blocker routed to a human owner instead of a // dispatched-then-failed run (which previously surfaced as opaque setup_failed). - if (typeof input.secretsSvc.collectMissingRuntimeBindings === "function") { - const missingBindings: MissingRuntimeBinding[] = []; - if (environmentEnv && input.environmentId) { - missingBindings.push( - ...(await input.secretsSvc.collectMissingRuntimeBindings( - input.companyId, - environmentEnv, - { - consumerType: "environment", - consumerId: input.environmentId, - responsibleUserId: input.responsibleUserId ?? null, - }, - )), - ); - } + const collectMissing = input.secretsSvc.collectMissingRuntimeBindings; + if (typeof collectMissing === "function") { + const checks: Array<() => Promise> = []; + const checkEnv = (env: unknown, consumerType: "environment" | "agent" | "project" | "routine", consumerId: string) => { + checks.push(() => collectMissing(input.companyId, env, { + consumerType, consumerId, responsibleUserId: input.responsibleUserId ?? null, + })); + }; + if (environmentEnv && input.environmentId) checkEnv(environmentEnv, "environment", input.environmentId); if (input.agentId) { - missingBindings.push( - ...(await input.secretsSvc.collectMissingRuntimeBindings( - input.companyId, - parseObject(executionRunConfig.env), - { - consumerType: "agent", - consumerId: input.agentId, - responsibleUserId: input.responsibleUserId ?? null, - }, - )), - ); - if ( - typeof input.secretsSvc.collectMissingAdapterConfigRuntimeBindings === - "function" - ) { - missingBindings.push( - ...(await input.secretsSvc.collectMissingAdapterConfigRuntimeBindings( - input.companyId, - executionRunConfig, - input.adapterType ?? null, - { - consumerType: "agent", - consumerId: input.agentId, - responsibleUserId: input.responsibleUserId ?? null, - }, - )), - ); + checkEnv(parseObject(executionRunConfig.env), "agent", input.agentId); + const collectAdapterMissing = input.secretsSvc.collectMissingAdapterConfigRuntimeBindings; + if (typeof collectAdapterMissing === "function") { + checks.push(() => collectAdapterMissing(input.companyId, executionRunConfig, input.adapterType ?? null, { + consumerType: "agent", consumerId: input.agentId!, responsibleUserId: input.responsibleUserId ?? null, + })); } } - if (projectEnv && input.projectId) { - missingBindings.push( - ...(await input.secretsSvc.collectMissingRuntimeBindings( - input.companyId, - projectEnv, - { - consumerType: "project", - consumerId: input.projectId, - responsibleUserId: input.responsibleUserId ?? null, - }, - )), - ); - } - if (routineEnv && input.routineId) { - missingBindings.push( - ...(await input.secretsSvc.collectMissingRuntimeBindings( - input.companyId, - routineEnv, - { - consumerType: "routine", - consumerId: input.routineId, - responsibleUserId: input.responsibleUserId ?? null, - }, - )), - ); + if (projectEnv && input.projectId) checkEnv(projectEnv, "project", input.projectId); + if (routineEnv && input.routineId) checkEnv(routineEnv, "routine", input.routineId); + const results = await Promise.allSettled(checks.map(check => Promise.resolve().then(check))); + const missingBindings: MissingRuntimeBinding[] = []; + for (const result of results) { + if (result.status === "rejected") throw result.reason; + missingBindings.push(...result.value); } if (requiredScopedEnvBinding) { const requiredEnvKeys = new Set(requiredScopedEnvBinding.keys); @@ -2244,12 +2199,45 @@ export async function attestReviewedExternalChatRun(input: { throw new Error("reviewed_chat_execution_binding_not_ready"); } +export async function readHeartbeatWakeComments(db: Db, companyId: string, issueId: string | null, commentIds: string[]) { + if (commentIds.length === 0) return []; + return db + .select({ + id: issueComments.id, + companyId: issueComments.companyId, + issueId: issueComments.issueId, + body: issueComments.body, + origin: issueComments.origin, + authorType: issueComments.authorType, + authorAgentId: issueComments.authorAgentId, + authorUserId: issueComments.authorUserId, + presentation: issueComments.presentation, + metadata: issueComments.metadata, + deletedAt: issueComments.deletedAt, + deletedByType: issueComments.deletedByType, + deletedByAgentId: issueComments.deletedByAgentId, + deletedByUserId: issueComments.deletedByUserId, + deletedByRunId: issueComments.deletedByRunId, + sourceTrust: issueComments.sourceTrust, + createdAt: issueComments.createdAt, + }) + .from(issueComments) + .where( + and( + eq(issueComments.companyId, companyId), + issueId ? eq(issueComments.issueId, issueId) : undefined, + inArray(issueComments.id, commentIds), + ), + ); +} + export async function buildPaperclipWakePayload(input: { db: Db; companyId: string; agentId?: string | null; runId?: string | null; contextSnapshot: Record; + wakeCommentRows?: Awaited>; continuationSummary?: { key: string; title: string | null; @@ -2314,36 +2302,9 @@ export async function buildPaperclipWakePayload(input: { ) return null; - const commentRows = - commentIds.length === 0 - ? [] - : await input.db - .select({ - id: issueComments.id, - issueId: issueComments.issueId, - body: issueComments.body, - origin: issueComments.origin, - authorType: issueComments.authorType, - authorAgentId: issueComments.authorAgentId, - authorUserId: issueComments.authorUserId, - presentation: issueComments.presentation, - metadata: issueComments.metadata, - deletedAt: issueComments.deletedAt, - deletedByType: issueComments.deletedByType, - deletedByAgentId: issueComments.deletedByAgentId, - deletedByUserId: issueComments.deletedByUserId, - deletedByRunId: issueComments.deletedByRunId, - sourceTrust: issueComments.sourceTrust, - createdAt: issueComments.createdAt, - }) - .from(issueComments) - .where( - and( - eq(issueComments.companyId, input.companyId), - issueId ? eq(issueComments.issueId, issueId) : undefined, - inArray(issueComments.id, commentIds), - ), - ); + const commentRows = input.wakeCommentRows + ? input.wakeCommentRows.filter(row => row.companyId === input.companyId && (!issueId || row.issueId === issueId) && commentIds.includes(row.id)) + : await readHeartbeatWakeComments(input.db, input.companyId, issueId, commentIds); const commentsById = new Map( commentRows.map((comment) => [comment.id, comment]), diff --git a/server/src/services/heartbeat/task-context.ts b/server/src/services/heartbeat/task-context.ts index 85bdd65ca61..9c1fb122dd4 100644 --- a/server/src/services/heartbeat/task-context.ts +++ b/server/src/services/heartbeat/task-context.ts @@ -1,5 +1,5 @@ import { createHash } from "node:crypto"; -import type { Db, agents, agentTaskSessions, heartbeatRuns, issueComments } from "@paperclipai/db"; +import type { Db, agents, agentTaskSessions, heartbeatRuns } from "@paperclipai/db"; import { FAST_RESPONSE_AGENT_GUIDANCE, fastResponseHistoryBody } from "@paperclipai/shared"; import type { PaperclipTurnContext } from "@paperclipai/adapter-utils/server-utils"; import { parseObject } from "../../adapters/utils.js"; @@ -15,8 +15,9 @@ import { getTaskPlanContext } from "../task-plan-context.js"; import { caseExecutionMarkdown, getCaseExecutionContext } from "../case-context.js"; import type { TrustPresetResolution } from "../trust-preset-resolver.js"; import { liveVoiceExecutionGuidance } from "../voice/voice-execution-guidance.js"; -import { buildPaperclipWakePayload, ConfigurationIncompleteFailure, PAPERCLIP_WAKE_PAYLOAD_KEY, type createHeartbeatRunPreparation } from "./run-preparation.js"; +import { buildPaperclipWakePayload, ConfigurationIncompleteFailure, readHeartbeatWakeComments, PAPERCLIP_WAKE_PAYLOAD_KEY, type createHeartbeatRunPreparation } from "./run-preparation.js"; import { buildPaperclipTaskMarkdown } from "./task-markdown.js"; +import { deriveCommentId, extractWakeCommentIds } from "../../modules/run-dispatch/index.js"; import { workspacePreferenceService } from "../workspace-preferences.js"; type RunPreparation = ReturnType; @@ -30,8 +31,6 @@ export interface HeartbeatTaskContextInput { "id" | "identifier" | "title" | "status" | "priority" | "workMode" | "conversationAgentId" | "description" | "projectId"> | null; issueContext: IssueContext | null; - wakeCommentContext: Pick | null; context: Record; taskSession: Pick | null; }; @@ -53,35 +52,9 @@ function readNonEmptyString(value: unknown): string | null { /** Rebuilds task prompt fields on the run's context; acquires no execution resources. */ export async function prepareHeartbeatTaskContext(db: Db, input: HeartbeatTaskContextInput) { const { run, agent } = input; - const { issueRef, issueContext, wakeCommentContext, context, taskSession } = input.task; + const { issueRef, issueContext, context, taskSession } = input.task; const { trustPreset, experimentalInstanceSettings } = input.policy; const { issuesSvc, getPinnedSkillTestContext } = input.services; - const continuationSummary = issueRef && !isConversation(issueContext) - ? await getIssueContinuationSummaryDocument(db, issueRef.id) - : null; - const exposeLowTrustRaw = trustPreset.kind === "low_trust_review"; - const safeContinuationSummary = - continuationSummary && !exposeLowTrustRaw - ? redactQuarantinedBodyForHigherTrust(continuationSummary) - : continuationSummary; - const safeWakeCommentContext = - wakeCommentContext && !exposeLowTrustRaw - ? sanitizeQuarantinedCommentForHigherTrust(wakeCommentContext) - : wakeCommentContext; - const issueAncestors = issueRef - ? await issuesSvc.getAncestors(issueRef.id) - : []; - if (continuationSummary) { - context.paperclipContinuationSummary = { - key: safeContinuationSummary!.key, - title: safeContinuationSummary!.title, - body: safeContinuationSummary!.body, - sourceTrust: safeContinuationSummary!.sourceTrust ?? null, - updatedAt: safeContinuationSummary!.updatedAt.toISOString(), - }; - } else { - delete context.paperclipContinuationSummary; - } const admittedScope = parseObject(context.paperclipAgentJwtScope); if (!input.policy.recovering || (admittedScope.kind !== "standard" && admittedScope.kind !== "skill_test")) { const savedIssue = parseObject(context.paperclipIssue); @@ -93,10 +66,58 @@ export async function prepareHeartbeatTaskContext(db: Db, input: HeartbeatTaskCo : { kind: "standard" }; } const isSkillTest = parseObject(context.paperclipAgentJwtScope).kind === "skill_test"; - const pinnedSkillTestContext = - isSkillTest && issueRef - ? await getPinnedSkillTestContext(agent.companyId, issueRef.id) - : null; + const wakeCommentId = deriveCommentId(context, null); + const commentIds = [...new Set([...extractWakeCommentIds(context), ...(wakeCommentId ? [wakeCommentId] : [])])]; + const [summary, ancestors, pinnedSkillTestRead, wakeComments] = await Promise.allSettled([ + issueRef && !isConversation(issueContext) ? getIssueContinuationSummaryDocument(db, issueRef.id) : Promise.resolve(null), + issueRef ? issuesSvc.getAncestors(issueRef.id) : Promise.resolve([]), + isSkillTest && issueRef ? getPinnedSkillTestContext(agent.companyId, issueRef.id) : Promise.resolve(null), + readHeartbeatWakeComments(db, agent.companyId, readNonEmptyString(context.issueId), commentIds), + ]); + if (summary.status === "rejected") throw summary.reason; + if (ancestors.status === "rejected") throw ancestors.reason; + if (pinnedSkillTestRead.status === "rejected") throw pinnedSkillTestRead.reason; + if (wakeComments.status === "rejected") throw wakeComments.reason; + const wakeCommentRows = wakeComments.value; + const selectedWakeComment = issueRef && wakeCommentId ? wakeCommentRows.find(comment => comment.id === wakeCommentId && comment.issueId === issueRef.id) ?? null : null; + const wakeCommentContext = selectedWakeComment ? { + id: selectedWakeComment.id, + body: selectedWakeComment.deletedAt ? "" : selectedWakeComment.body, + authorType: selectedWakeComment.authorType, + authorAgentId: selectedWakeComment.authorAgentId, + authorUserId: selectedWakeComment.authorUserId, + presentation: selectedWakeComment.deletedAt ? null : selectedWakeComment.presentation, + metadata: selectedWakeComment.deletedAt ? null : selectedWakeComment.metadata, + deletedAt: selectedWakeComment.deletedAt, + deletedByType: selectedWakeComment.deletedByType, + deletedByAgentId: selectedWakeComment.deletedByAgentId, + deletedByUserId: selectedWakeComment.deletedByUserId, + deletedByRunId: selectedWakeComment.deletedByRunId, + sourceTrust: selectedWakeComment.sourceTrust, + } : null; + const continuationSummary = summary.value; + const issueAncestors = ancestors.value; + const pinnedSkillTestContext = pinnedSkillTestRead.value; + const exposeLowTrustRaw = trustPreset.kind === "low_trust_review"; + const safeContinuationSummary = + continuationSummary && !exposeLowTrustRaw + ? redactQuarantinedBodyForHigherTrust(continuationSummary) + : continuationSummary; + const safeWakeCommentContext = + wakeCommentContext && !exposeLowTrustRaw + ? sanitizeQuarantinedCommentForHigherTrust(wakeCommentContext) + : wakeCommentContext; + if (continuationSummary) { + context.paperclipContinuationSummary = { + key: safeContinuationSummary!.key, + title: safeContinuationSummary!.title, + body: safeContinuationSummary!.body, + sourceTrust: safeContinuationSummary!.sourceTrust ?? null, + updatedAt: safeContinuationSummary!.updatedAt.toISOString(), + }; + } else { + delete context.paperclipContinuationSummary; + } if (isSkillTest && !pinnedSkillTestContext) { throw new ConfigurationIncompleteFailure("The skill test's saved version is unavailable.", { configurationIncomplete: { reason: "skill_unavailable", actionUrl: "/skills" }, @@ -132,6 +153,7 @@ export async function prepareHeartbeatTaskContext(db: Db, input: HeartbeatTaskCo agentId: agent.id, runId: run.id, contextSnapshot: context, + wakeCommentRows, continuationSummary, issueSummary: issueRef ? { diff --git a/server/src/services/native-runtime/runner-api.integration.test.ts b/server/src/services/native-runtime/runner-api.integration.test.ts index 4e1831b734a..fdb5b7f70a2 100644 --- a/server/src/services/native-runtime/runner-api.integration.test.ts +++ b/server/src/services/native-runtime/runner-api.integration.test.ts @@ -497,7 +497,7 @@ else if(m.id!==undefined) send({id:m.id,result:{}}); const [agent] = await server.db.select().from(agents).where(eq(agents.id, fixture.agentId)); const context = structuredClone(run.contextSnapshot ?? {}); await prepareHeartbeatTaskContext(server.db, { run, agent, - task: { issueRef: issue, issueContext: null, wakeCommentContext: null, context, taskSession: null }, + task: { issueRef: issue, issueContext: null, context, taskSession: null }, policy: { recovering: true, trustPreset: { kind: "standard" }, experimentalInstanceSettings: {} }, services: { issuesSvc: { getAncestors: async () => [] }, getPinnedSkillTestContext: createHeartbeatRunPreparation(server.db).getPinnedSkillTestContext }, From 105a67b52c66a738c0a8eb7807332b99931ac45d Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 30 Oct 2026 16:54:04 -0700 Subject: [PATCH 5/8] Reuse signed master images for npm canaries (#18404) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Release workflows publish npm packages and multi-platform images. > - A master push can finish its image before the npm canary dispatch starts. > - The dispatch currently builds that same source again on two runners. > - This PR verifies and reuses the signed master image by digest. > - Missing images retain the native build path. Channel promotion still follows npm. ## Linked Issues or Issue Description **What existing behavior does this improve?** The Docker workflow dispatched after npm canary publication. **Current behavior** Every canary dispatch builds both architectures, even when master has already published the same source. This consumes runner capacity and delays the image channel update. **Proposed behavior** Resolve the full-SHA master image. Verify its platform index, digest and signed source identity. Retag that immutable digest. Build normally when the image does not exist. **Reason and benefit** Remove duplicate image builds while keeping missing-image recovery and the current npm channel check. A recent duplicate used about 25 runner-minutes. The saving depends on whether the master image exists. ## What Changed - Add a canary-only reuse job before the native matrix. - Require signed provenance from the canonical Docker workflow on master at the exact source and signer commit. Read the proof from GHCR. - Reuse the validated digest or build both platforms when the full-SHA image is absent. Stop on registry and verification errors. - Keep the serialized npm dist-tag promotion after either path. Keep other build lanes independent. - Add failure tests, register them in CI and update release documentation. ## Verification - Passed 603 tests: `node --test '.github/scripts/tests/*.test.mjs' scripts/standard-image-contract.test.mjs scripts/reuse-canary-image.test.mjs scripts/__tests__/release-verify-workflow.test.mjs scripts/cloud-source-verification.test.mjs`. - Passed `actionlint -shellcheck='' .github/workflows/docker.yml .github/workflows/pr-trusted.yml` and `git diff --check`. - Full actionlint reports the same six existing shellcheck findings as the unchanged master workflow. No new findings. - A published master image passed the exact provenance command. This was a read-only check. No images were retagged. - Gitleaks and manual diff inspection found no secrets or private references. - Attempted `pnpm -r typecheck`, `pnpm test:run` and `pnpm build`. They cannot start because this isolated checkout has no installed dependencies (`tsc` and `tsx` are absent). Repository CI passed on `3c46f0152d4db94a7f393bc79f466aa6c2c745e2` after the documented browser-test rerun. All checks are complete. The original failure and prior baseline occurrence remain recorded in the PR comments; its root cause remains unknown. ## Risks - The image can appear just before its attestation. Verification then fails closed. Rerun after the master producer completes. - Reuse preserves the first build's resolved dependencies and tool versions. It intentionally does not rebuild the same source to refresh floating inputs. - A skipped or unfinished master image still needs a full build. No speedup is claimed for that path. - End-to-end reuse and runner savings require observation after merge. No live publication was performed. ## Model Used OpenAI `gpt-6.1-sol`, through the Codex harness in T3 Code, with xhigh reasoning effort and shell/code tools. The context window size is not exposed by this runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- .../tests/docker-canary-promotion.test.mjs | 2 +- .github/workflows/docker.yml | 39 ++++++++-- .github/workflows/pr-trusted.yml | 2 +- doc/RELEASING.md | 23 +++++- scripts/reuse-canary-image.mjs | 34 +++++++++ scripts/reuse-canary-image.test.mjs | 75 +++++++++++++++++++ scripts/standard-image-contract.mjs | 6 +- scripts/standard-image-contract.test.mjs | 14 ++++ 8 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 scripts/reuse-canary-image.mjs create mode 100644 scripts/reuse-canary-image.test.mjs diff --git a/.github/scripts/tests/docker-canary-promotion.test.mjs b/.github/scripts/tests/docker-canary-promotion.test.mjs index a9b0727465f..fe80e25168b 100644 --- a/.github/scripts/tests/docker-canary-promotion.test.mjs +++ b/.github/scripts/tests/docker-canary-promotion.test.mjs @@ -11,7 +11,7 @@ const script = job.split(" run: |\n")[1].split("\n").map(line => line.rep const sha = "a".repeat(40); test("canary promotion waits for the standard manifest and keeps its serialized channel", () => { - assert.match(job, /needs: \[merge-and-push\]/); + assert.match(job, /needs: \[reuse_canary_image, merge-and-push\]/); assert.match(job, /group: docker-canary-channel-promotion/); assert.match(job, /cancel-in-progress: false/); }); diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index ed52dfa700d..ec7c71000e5 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -33,6 +33,35 @@ concurrency: cancel-in-progress: false jobs: + reuse_canary_image: + if: github.repository == 'paperclipai/paperclip' && github.repository_id == '1170821064' && startsWith(github.ref, 'refs/tags/canary/v') + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + packages: write + attestations: read + outputs: + reused: ${{ steps.reuse.outputs.reused }} + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 + - name: Setup Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 + with: + node-version: 24 + - name: Login to GitHub Container Registry + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Reuse the verified master image when present + id: reuse + env: + GH_TOKEN: ${{ github.token }} + run: node scripts/reuse-canary-image.mjs + # Multi-arch by native runner, not QEMU. # # This was one job building linux/amd64,linux/arm64 together on an x86 @@ -56,6 +85,8 @@ jobs: # digest; `merge` assembles the manifest list. Both architectures remain # available to self-hosted installations and downstream image composers. build-and-push: + needs: [reuse_canary_image] + if: ${{ !cancelled() && (needs.reuse_canary_image.result == 'skipped' || (needs.reuse_canary_image.result == 'success' && needs.reuse_canary_image.outputs.reused == 'false')) }} strategy: # Independent legs: one architecture failing should still publish # nothing, but it must not also hide the other's logs behind a @@ -277,6 +308,7 @@ jobs: # tags. Nothing is publicly tagged until this runs, so a half-published # multi-arch image is not a state anything can pull. merge-and-push: + if: ${{ !cancelled() && needs.build-and-push.result == 'success' }} needs: build-and-push runs-on: ubuntu-latest timeout-minutes: 20 @@ -407,11 +439,8 @@ jobs: # promotion is gated on its own completed pushes). Every interleaving # converges the Docker channel onto the npm channel. promote_canary_channel: - if: startsWith(github.ref, 'refs/tags/canary/v') - # merge-and-push, not build-and-push: the per-arch legs push untagged - # digests, and the production `sha-*` tags this promotion retags only - # exist once the manifest merge has named them. - needs: [merge-and-push] + if: ${{ !cancelled() && startsWith(github.ref, 'refs/tags/canary/v') && (needs.merge-and-push.result == 'success' || (needs.reuse_canary_image.result == 'success' && needs.reuse_canary_image.outputs.reused == 'true')) }} + needs: [reuse_canary_image, merge-and-push] runs-on: ubuntu-latest timeout-minutes: 10 permissions: diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index b1b73c1f085..dae203d6216 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -202,7 +202,7 @@ jobs: run: node "$CI_NODE_TEST" ./scripts/__tests__/e2e-shard.test.mjs - name: Test release verify workflow wiring - run: node "$CI_NODE_TEST" ./scripts/__tests__/release-verify-workflow.test.mjs ./scripts/__tests__/eval-schedule-monitor.test.mjs ./scripts/__tests__/prepare-runner-release-image.test.mjs ./scripts/cloud-source-verification.test.mjs ./scripts/standard-image-contract.test.mjs + run: node "$CI_NODE_TEST" ./scripts/__tests__/release-verify-workflow.test.mjs ./scripts/__tests__/eval-schedule-monitor.test.mjs ./scripts/__tests__/prepare-runner-release-image.test.mjs ./scripts/cloud-source-verification.test.mjs ./scripts/standard-image-contract.test.mjs ./scripts/reuse-canary-image.test.mjs - name: Test standalone package build concurrency run: node "$CI_NODE_TEST" ./scripts/__tests__/build-standalone-concurrency.test.mjs diff --git a/doc/RELEASING.md b/doc/RELEASING.md index dbf40956a50..70e28fe798f 100644 --- a/doc/RELEASING.md +++ b/doc/RELEASING.md @@ -86,7 +86,7 @@ internal traceability tag. moves them to the versioned name - canaries, nightlies, and betas never create GitHub Releases - canaries, nightlies, and betas never require changelog generation -- Docker `:latest` moves only on stable releases; master builds publish +- Docker `:latest` moves only on stable releases; canary dispatches promote `:canary`, nightly builds `:nightly`, and beta builds `:beta` ## TL;DR @@ -310,12 +310,12 @@ The workflow: ## Docker Image Tags -[`docker.yml`](../.github/workflows/docker.yml) publishes both the self-hosted -image and the `-cloud` variant with the same lane mapping: +[`docker.yml`](../.github/workflows/docker.yml) publishes the standard image: | Build ref | Tags | | --- | --- | -| `master` push | `:canary`, `:sha-` | +| `master` push | `:sha-`, `:sha-` | +| `canary/v*` dispatch | `:sha-`, then `:canary` for the current npm canary | | `nightly/v*` tag | `:nightly`, `:sha-` | | `beta/v*` tag | `:beta`, `:sha-` | | `v*` tag (stable) | `:latest`, `:YYYY.MDD.P`, `:YYYY.MDD`, `:sha-` | @@ -325,6 +325,21 @@ suppresses push-triggered workflow runs for those pushes. The release jobs therefore dispatch `docker.yml` explicitly at the new tag ref; the tag mapping keys off `github.ref` either way. +Canary dispatches first look up the full-SHA master image. Reuse requires a +valid multi-platform index with both `linux/amd64` and `linux/arm64`, a matching +registry digest, and signed provenance from this repository's `docker.yml` +on `master` at the exact source commit. The image is retagged by immutable +digest. This preserves the original build inputs and skips both native builds +and the manifest merge. A missing full-SHA image runs the normal build instead; +registry errors or invalid provenance fail the run. Rerun after the master +image's attestation finishes if the image became visible before its proof. + +Both paths retain the serialized promotion that resolves the current npm +`canary` dist-tag before moving Docker `:canary`. Nightly, beta, stable, and +master builds retain their existing build paths. Downstream image builds can +continue to consume the signed full-SHA master image independently of npm +canary publication. + The Docker build matrix and the release preview image builder configure BuildKit to check `mirror.gcr.io` for Docker Hub images first. This covers both the Dockerfile frontend and base images. BuildKit keeps Docker Hub as diff --git a/scripts/reuse-canary-image.mjs b/scripts/reuse-canary-image.mjs new file mode 100644 index 00000000000..3a43524dd0b --- /dev/null +++ b/scripts/reuse-canary-image.mjs @@ -0,0 +1,34 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { appendFile } from "node:fs/promises"; +import { pathToFileURL } from "node:url"; +import { resolveStandardImageDigest } from "./standard-image-contract.mjs"; + +export async function reuseCanaryImage(sha, { + resolve = source => resolveStandardImageDigest(source, fetch, { missing: "return-null" }), + run = (command, args) => execFileSync(command, args, { stdio: "inherit" }), +} = {}) { + assert.match(sha ?? "", /^[a-f0-9]{40}$/); + const digest = await resolve(sha); + if (digest === null) return false; + assert.match(digest, /^sha256:[a-f0-9]{64}$/); + const image = `ghcr.io/paperclipai/paperclip@${digest}`; + await run("gh", ["attestation", "verify", `oci://${image}`, + "--repo", "paperclipai/paperclip", "--bundle-from-oci", + "--signer-workflow", "paperclipai/paperclip/.github/workflows/docker.yml", + "--source-ref", "refs/heads/master", "--source-digest", sha, + "--signer-digest", sha, "--deny-self-hosted-runners"]); + await run("docker", ["buildx", "imagetools", "create", "-t", + `ghcr.io/paperclipai/paperclip:sha-${sha.slice(0, 7)}`, image]); + return true; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const reused = await reuseCanaryImage(process.env.GITHUB_SHA); + if (process.env.GITHUB_OUTPUT) await appendFile(process.env.GITHUB_OUTPUT, `reused=${reused}\n`); + const summary = reused + ? "Reused the signed multi-platform master image. Skipped both image builds and the manifest merge." + : "No full-SHA master image exists. Building both platforms for this canary."; + console.log(summary); + if (process.env.GITHUB_STEP_SUMMARY) await appendFile(process.env.GITHUB_STEP_SUMMARY, `${summary}\n`); +} diff --git a/scripts/reuse-canary-image.test.mjs b/scripts/reuse-canary-image.test.mjs new file mode 100644 index 00000000000..26dfbc83c5f --- /dev/null +++ b/scripts/reuse-canary-image.test.mjs @@ -0,0 +1,75 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; +import { reuseCanaryImage } from "./reuse-canary-image.mjs"; + +const sha = "a".repeat(40); +const digest = `sha256:${"b".repeat(64)}`; + +test("reuses only an exact-source, master-signed multi-platform image by digest", async () => { + const calls = []; + const reused = await reuseCanaryImage(sha, { + resolve: async source => { assert.equal(source, sha); return digest; }, + run: async (...args) => calls.push(args), + }); + assert.equal(reused, true); + assert.deepEqual(calls, [ + ["gh", ["attestation", "verify", `oci://ghcr.io/paperclipai/paperclip@${digest}`, + "--repo", "paperclipai/paperclip", "--bundle-from-oci", + "--signer-workflow", "paperclipai/paperclip/.github/workflows/docker.yml", + "--source-ref", "refs/heads/master", "--source-digest", sha, + "--signer-digest", sha, "--deny-self-hosted-runners"]], + ["docker", ["buildx", "imagetools", "create", "-t", + "ghcr.io/paperclipai/paperclip:sha-aaaaaaa", `ghcr.io/paperclipai/paperclip@${digest}`]], + ]); +}); + +test("a missing master image enables the fallback without retagging", async () => { + assert.equal(await reuseCanaryImage(sha, { + resolve: async () => null, + run: () => assert.fail("A missing image must not be promoted"), + }), false); +}); + +test("registry and provenance failures stop promotion rather than bypass verification", async () => { + await assert.rejects(reuseCanaryImage(sha, { + resolve: async () => { throw new Error("HTTP 403"); }, + run: () => assert.fail("A registry failure must not be promoted"), + }), /HTTP 403/); + let commands = 0; + await assert.rejects(reuseCanaryImage(sha, { + resolve: async () => digest, + run: command => { + assert.equal(command, "gh"); + commands++; + throw new Error("Untrusted source or signature"); + }, + }), /Untrusted source/); + assert.equal(commands, 1); +}); + +test("invalid source refs and digests cannot reach external commands", async () => { + for (const invalid of ["master", "a".repeat(7), "a".repeat(40) + "\n"]) { + await assert.rejects(reuseCanaryImage(invalid, { + resolve: () => assert.fail("Invalid source accepted"), + })); + } + await assert.rejects(reuseCanaryImage(sha, { + resolve: async () => "mutable:tag", + run: () => assert.fail("Invalid digest accepted"), + })); +}); + +test("canary reuse gates both native builds and still runs convergent promotion", () => { + const workflow = readFileSync(new URL("../.github/workflows/docker.yml", import.meta.url), "utf8"); + const reuse = workflow.split(" reuse_canary_image:\n")[1].split(" build-and-push:\n")[0]; + assert.match(reuse, /github.repository_id == '1170821064'.*startsWith\(github.ref, 'refs\/tags\/canary\/v'\)/); + assert.match(reuse, /node scripts\/reuse-canary-image.mjs/); + const build = workflow.split(" build-and-push:\n")[1].split(" merge-and-push:\n")[0]; + assert.match(build, /needs: \[reuse_canary_image\]/); + assert.match(build, /!cancelled\(\).*result == 'skipped'.*result == 'success'.*outputs.reused == 'false'/); + assert.match(build, /platform: linux\/amd64/); + assert.match(build, /platform: linux\/arm64/); + const promotion = workflow.split(" promote_canary_channel:\n")[1]; + assert.match(promotion, /!cancelled\(\).*needs.merge-and-push.result == 'success'.*outputs.reused == 'true'/); +}); diff --git a/scripts/standard-image-contract.mjs b/scripts/standard-image-contract.mjs index 62a8249e2da..5e0ae3cc24c 100644 --- a/scripts/standard-image-contract.mjs +++ b/scripts/standard-image-contract.mjs @@ -21,6 +21,7 @@ export function standardImageDigest(bytes) { /** Read registry bytes directly; CLI stdout formatting is not artifact content. */ export async function resolveStandardImageDigest(sha, fetchImpl = fetch, { sleep = ms => new Promise(resolve => setTimeout(resolve, ms)), + missing = "error", } = {}) { assert.match(sha ?? "", /^[a-f0-9]{40}$/); const request = async (url, headers, consume, allowNotFound = false) => { @@ -35,6 +36,7 @@ export async function resolveStandardImageDigest(sha, fetchImpl = fetch, { } if (!response.ok) { await response.body?.cancel(); + if (allowNotFound && response.status === 404 && missing === "return-null") return null; const transient = [408, 425, 429].includes(response.status) || response.status >= 500 || (allowNotFound && response.status === 404); assert.ok(transient && attempt < 5, `Registry lookup failed: HTTP ${response.status}`); const retryAfter = Number(response.headers.get("retry-after")); @@ -53,7 +55,7 @@ export async function resolveStandardImageDigest(sha, fetchImpl = fetch, { }; const { token } = await request("https://ghcr.io/token?service=ghcr.io&scope=repository:paperclipai/paperclip:pull", {}, response => response.json()); assert.ok(typeof token === "string" && token, "Missing public pull token"); - const { raw, expected } = await request(`https://ghcr.io/v2/paperclipai/paperclip/manifests/sha-${sha}`, { + const manifest = await request(`https://ghcr.io/v2/paperclipai/paperclip/manifests/sha-${sha}`, { authorization: `Bearer ${token}`, accept: "application/vnd.oci.image.index.v1+json, application/vnd.docker.distribution.manifest.list.v2+json", }, async response => { assert.ok(response.body, "Standard image index has no body"); @@ -71,6 +73,8 @@ export async function resolveStandardImageDigest(sha, fetchImpl = fetch, { } finally { await reader.cancel(); } return { raw: Buffer.concat(chunks), expected: response.headers.get("docker-content-digest") }; }, true); + if (manifest === null) return null; + const { raw, expected } = manifest; const digest = standardImageDigest(raw); assert.equal(digest, expected, "Registry digest does not match the index bytes"); return digest; diff --git a/scripts/standard-image-contract.test.mjs b/scripts/standard-image-contract.test.mjs index af2d4462977..d61f3fb862d 100644 --- a/scripts/standard-image-contract.test.mjs +++ b/scripts/standard-image-contract.test.mjs @@ -77,3 +77,17 @@ test("permanent registry errors fail immediately and transient errors exhaust a assert.equal(calls, expectedCalls); } }); + +test("optional lookup falls back only for a missing manifest", async () => { + for (const status of [401, 403, 404, 503]) { + let calls = 0; + const lookup = resolveStandardImageDigest("a".repeat(40), async url => { + if (url.includes("/token?")) return Response.json({ token: "public-fixture" }); + calls++; + return new Response(null, { status }); + }, { missing: "return-null", sleep: async () => {} }); + if (status === 404) assert.equal(await lookup, null); + else await assert.rejects(lookup, /Registry lookup failed/); + assert.equal(calls, status === 503 ? 5 : 1); + } +}); From 87ae20ae650de687ea9a2f8ca72e5a8d89050431 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 30 Oct 2026 17:04:04 -0700 Subject: [PATCH 6/8] fix(server): reconcile failed sandbox cleanup with signed absence (#18407) Reconcile failed ephemeral Cloud leases through a signed cleanup-only absence observation when local historical inspection is unavailable. Preserve failure outcomes, exact resource bindings, retention and row-version fences; grant no run-continuation authority. All 54 current-head checks/statuses passed. Greptile is 5/5, Medium, with no findings or unresolved threads. The 38 focused tests pass; both new reconciliation cases fail without the fallback. Isolated baseline and branch idle suites each pass all 225 cases. The original broader local teardown timeout is retained in the PR, and the full local run remains in progress. Cloud protocol support is already serving in staging. Exact candidate composition and hosted provider sleep/wake qualification follow this merge. Production has not been changed. Co-Authored-By: Paperclip --- doc/idle-sleep-safety.md | 11 +++ maps/agent-access-map/instance-operations.md | 8 +++ maps/feature-map/instance-operations.md | 9 +++ .../historical-cleanup-verification.test.ts | 54 ++++++++++++++- server/src/services/environment-runtime.ts | 8 ++- .../services/sandbox-absence-proof.test.ts | 52 ++++++++++++-- server/src/services/sandbox-absence-proof.ts | 68 +++++++++++++++---- 7 files changed, 190 insertions(+), 20 deletions(-) diff --git a/doc/idle-sleep-safety.md b/doc/idle-sleep-safety.md index e535edcd569..7b8ef115430 100644 --- a/doc/idle-sleep-safety.md +++ b/doc/idle-sleep-safety.md @@ -101,6 +101,17 @@ The manager permits at most two outstanding inspection RPCs across workers. Caller timeouts retain their slots until the actual worker reply or exit; timeouts cannot accumulate additional provider reads in later sweeps. +If a failed ephemeral lease has no usable local inspection path, a managed +Daytona instance can ask Cloud for a cleanup-only absence receipt. Cloud checks +the stack's historical credential organization and reads the exact allocation +with its manager credential. Only confirmed absence can produce a signed receipt +bound to the stack, company, environment, lease, provider handle and nullable run. +The separate cleanup type and audience cannot authorize run recovery. Older +Cloud servers, unsupported providers, invalid signatures, changed organizations +and failed reads leave the lease unresolved. Existing bounded cleanup batches +and retry deadlines own these reads; no new idle polling is added. An explicit +local provider observation is retained rather than replaced by this fallback. + Host-owned verification metadata binds the company, environment, resource, provider handle, optional run, and exact row version. Concurrent resume, replacement, cleanup, or configuration changes invalidate verification. diff --git a/maps/agent-access-map/instance-operations.md b/maps/agent-access-map/instance-operations.md index 3e21ef72901..f2d4250a4cb 100644 --- a/maps/agent-access-map/instance-operations.md +++ b/maps/agent-access-map/instance-operations.md @@ -62,6 +62,14 @@ authority as company-agent API access. Gaps: Instance start/stop and task workspace start/stop are different capabilities. Local service control is not agent company authority. +Historical Cloud sandbox cleanup remains server-owned. Cleanup-only absence +reads authenticate the tenant host with its server capability; they expose no +new board or agent endpoint. Their signed scope cannot authorize task recovery. +[Runtime and idle-gate regressions](../../server/src/__tests__/historical-cleanup-verification.test.ts) +cover reconciliation, while [receipt checks](../../server/src/services/sandbox-absence-proof.test.ts) +cover signature, identity, freshness and separation from recovery authority. +These checks do not establish a deployed sleep/wake result. + ## `configuration` Capability ID: `instance-operations/configuration`. diff --git a/maps/feature-map/instance-operations.md b/maps/feature-map/instance-operations.md index 7a01c24ebf6..dd43901323e 100644 --- a/maps/feature-map/instance-operations.md +++ b/maps/feature-map/instance-operations.md @@ -61,6 +61,15 @@ served by the admitted application before it finishes. Repeat with a disconnect before headers arrive. Confirm that neither leaves a permanent request hold. An unfinished application write must still prevent sleep after its client leaves. +Automated: [historical cleanup](../../server/src/__tests__/historical-cleanup-verification.test.ts) +exercises the runtime inspection path for failed Cloud sandbox leases, including +verification leases without a run. A signed cleanup-only absence observation +can remove this blocker without changing failed outcomes or granting run +continuation. An unresolved provider resource still prevents idle sleep. +Manual: on an owned Cloud staging workspace, confirm the exact published build, +reconcile failed test allocations, then verify actual provider suspension and +normal hosted entry after wake. Local tests do not prove that hosted journey. + ### `settings-backup` Cloud startup reads nonsecret application choices from a private, versioned diff --git a/server/src/__tests__/historical-cleanup-verification.test.ts b/server/src/__tests__/historical-cleanup-verification.test.ts index 37bc2dd6692..1269c02f1a2 100644 --- a/server/src/__tests__/historical-cleanup-verification.test.ts +++ b/server/src/__tests__/historical-cleanup-verification.test.ts @@ -1,8 +1,9 @@ -import { randomUUID } from "node:crypto"; +import { generateKeyPairSync, randomUUID, sign } from "node:crypto"; import { eq, sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, companies, createDb, environmentCustomImageSetupSessions, environmentLeases, environments, heartbeatRuns } from "@paperclipai/db"; import { historicalCleanupVerificationService } from "../services/historical-cleanup-verification.js"; +import { environmentRuntimeService } from "../services/environment-runtime.js"; import { withoutCleanupVerification } from "../services/cleanup-verification-proof.js"; import { idleWorkSnapshot } from "../services/task-admission.js"; import { readIdleSleepSafety, type IdleSleepDrainStatus } from "../services/idle-sleep-safety.js"; @@ -16,7 +17,7 @@ if (!support.supported) console.warn(`Skipping historical cleanup database tests (support.supported ? describe : describe.skip)("historical cleanup verification", () => { let db: ReturnType, database: Awaited>; beforeAll(async () => { database = await startEmbeddedPostgresTestDatabase("historical-cleanup-"); db = createDb(database.connectionString); }, 60_000); - afterEach(async () => { await db.execute(sql`SET client_min_messages = warning; TRUNCATE companies, plugins, environments CASCADE`); }); + afterEach(async () => { vi.unstubAllEnvs(); vi.unstubAllGlobals(); await db.execute(sql`SET client_min_messages = warning; TRUNCATE companies, plugins, environments CASCADE`); }); afterAll(async () => { await db?.$client.end(); await database?.cleanup(); }); const read = () => readIdleSleepSafety(db, owned, () => now, ownerId, async () => "none", undefined, undefined, true, true); const state = async () => (await read()).backgroundWork; @@ -37,6 +38,55 @@ if (!support.supported) console.warn(`Skipping historical cleanup database tests inspectSetup: async row => ({ providerLeaseId: row.providerLeaseId, state: setupState }), }); } + it.each([false, true])("reconciles a legacy Cloud lease through signed absence and permits idle sleep (ad-hoc %s)", async adHoc => { + const f = await seed(adHoc); + const { privateKey, publicKey } = generateKeyPairSync("ed25519"); + for (const [key, value] of Object.entries({ PAPERCLIP_CLOUD_STACK_ID: "fixture-stack", + PAPERCLIP_CLOUD_API_ORIGIN: "https://cloud.example.test", PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN: "fleet-fixture", + PAPERCLIP_PRIMARY_HOST: "fixture.paperclip.test", PAPERCLIP_AUTH_SECRET: "fixture-secret", + PAPERCLIP_CLOUD_RUNTIME_IDENTITY_JWKS: JSON.stringify({ keys: [{ ...publicKey.export({ format: "jwk" }), kid: "fixture", alg: "EdDSA", use: "sig" }] }) })) vi.stubEnv(key, value); + let absent = false; + const fetchImpl = vi.fn(async (_url: string, init: RequestInit) => { + if (!absent) return Response.json({ absent: false }); + const binding = JSON.parse(init.body as string), seconds = Math.floor(Date.now() / 1000); + expect(binding).toEqual({ purpose: "cleanup", companyId: f.companyId, environmentId: f.environment.id, + leaseId: f.lease.id, heartbeatRunId: adHoc ? null : f.runId, provider: "daytona", providerLeaseId: f.lease.providerLeaseId }); + const header = Buffer.from(JSON.stringify({ alg: "EdDSA", kid: "fixture", typ: "paperclip-sandbox-cleanup-absence+jwt" })).toString("base64url"); + const claims = Buffer.from(JSON.stringify({ v: 1, iss: "paperclip-cloud", aud: "paperclip-sandbox-cleanup-absence/v1", + sub: "fixture-stack", ...binding, absent: true, iat: seconds, exp: seconds + 60 })).toString("base64url"); + return Response.json({ receipt: `${header}.${claims}.${sign(null, Buffer.from(`${header}.${claims}`), privateKey).toString("base64url")}` }); + }); + vi.stubGlobal("fetch", fetchImpl); + const runtime = environmentRuntimeService(db); + const service = historicalCleanupVerificationService(db, { + inspectLease: row => runtime.inspectLeaseResource(row.id), inspectSetup: async () => undefined, + }); + expect(await state()).toBe("present"); + expect(await service.sweep()).toMatchObject({ checked: 1, deferred: 1, verified: 0 }); + expect(await state()).toBe("present"); + absent = true; + expect(await service.sweep()).toMatchObject({ checked: 1, verified: 1, failed: 0 }); + expect(await state()).toBe("none"); + const [current] = await db.select().from(environmentLeases); + expect(current).toMatchObject({ status: "failed", cleanupStatus: "success", heartbeatRunId: adHoc ? null : f.runId }); + expect(current!.metadata).not.toHaveProperty("remoteExecutionTermination"); + expect(await service.sweep()).toMatchObject({ checked: 0 }); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); + it.each(["active", "stopped", "unknown"] as const)("does not replace an explicit %s provider observation with a Cloud read", async providerState => { + await seed(true); + for (const [key, value] of Object.entries({ PAPERCLIP_CLOUD_STACK_ID: "fixture-stack", + PAPERCLIP_CLOUD_API_ORIGIN: "https://cloud.example.test", PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN: "fleet-fixture", + PAPERCLIP_PRIMARY_HOST: "fixture.paperclip.test", PAPERCLIP_AUTH_SECRET: "fixture-secret" })) vi.stubEnv(key, value); + const fetchImpl = vi.fn(); vi.stubGlobal("fetch", fetchImpl); + const runtime = environmentRuntimeService(db, { drivers: [{ driver: "sandbox", + acquireRunLease: async () => { throw new Error("unexpected acquire"); }, releaseRunLease: async () => null, + inspectLeaseResource: async ({ lease }) => ({ providerLeaseId: lease.providerLeaseId!, state: providerState }), + }] }); + const service = historicalCleanupVerificationService(db, { inspectLease: row => runtime.inspectLeaseResource(row.id), inspectSetup: async () => undefined }); + expect(await service.sweep()).toMatchObject({ checked: 1, verified: 0, deferred: 1 }); + expect(await state()).toBe("present"); expect(fetchImpl).not.toHaveBeenCalled(); + }); it.each([false, true])("accepts exact deletion proof without changing outcomes or granting run authority (ad-hoc %s)", async adHoc => { const f = await seed(adHoc); expect(await state()).toBe("present"); const service = verifier(); expect(await service.sweep()).toMatchObject({ checked: 1, verified: 1, failed: 0 }); diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 277a5a0254c..33f161457f9 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -15,6 +15,7 @@ import { hasStopOnlyCleanup, prepareSandboxStopAndRetain, readStopOnlyCleanup, s import { JsonRpcCallError, readEnvironmentAcquisitionDiagnostic, readEnvironmentCreationCleanupError } from "@paperclipai/plugin-sdk"; import { preserveEnvironmentSyncOutErrorDiagnostic } from "./environment-sync-out-error.js"; import { remoteTerminationReceipt } from "./remote-execution-termination.js"; +import { inspectCloudSandboxCleanupAbsence } from "./sandbox-absence-proof.js"; import { hasNativeWorkspaceExportResume, releaseCompletedNativeWorkspaceExportRetention } from "./native-runtime/native-workspace-export-resume.js"; import { createHash, randomUUID } from "node:crypto"; import { and, eq, inArray, ne, or, sql } from "drizzle-orm"; @@ -4021,7 +4022,12 @@ export function environmentRuntimeService( const lease = await environmentsSvc.getLeaseById(leaseId); if (!lease || lease.status !== "failed" || lease.leasePolicy !== "ephemeral" || !lease.providerLeaseId) return undefined; const environment = lease.environmentId ? await environmentsSvc.getById(lease.environmentId) : null; - return await getDriver(getLeaseDriverKey(lease, environment))?.inspectLeaseResource?.({ environment, lease }); + // Legacy Cloud leases can lack a credential version that permits a + // provider-local observation. Cloud's manager verifies the historical + // organization and signs only exact absence; no replacement key or + // cleanup label is treated as proof. Explicit provider answers win. + return await getDriver(getLeaseDriverKey(lease, environment))?.inspectLeaseResource?.({ environment, lease }) + ?? await inspectCloudSandboxCleanupAbsence(lease); }, async retryPendingSandboxTeardown(input: { diff --git a/server/src/services/sandbox-absence-proof.test.ts b/server/src/services/sandbox-absence-proof.test.ts index d145425137e..82a7a3cfbba 100644 --- a/server/src/services/sandbox-absence-proof.test.ts +++ b/server/src/services/sandbox-absence-proof.test.ts @@ -1,6 +1,7 @@ import { createHmac, generateKeyPairSync, randomUUID, sign } from "node:crypto"; import { describe, expect, it, vi } from "vitest"; -import { requestSandboxAbsenceProof, verifySandboxAbsenceReceipt } from "./sandbox-absence-proof.js"; +import { inspectCloudSandboxCleanupAbsence, requestSandboxAbsenceProof, verifySandboxAbsenceReceipt, + verifySandboxCleanupAbsenceReceipt } from "./sandbox-absence-proof.js"; const { privateKey, publicKey } = generateKeyPairSync("ed25519"); const binding = { companyId: randomUUID(), issueId: randomUUID(), runId: randomUUID(), leaseId: randomUUID(), @@ -9,10 +10,10 @@ const env = { PAPERCLIP_CLOUD_STACK_ID: "fixture-stack", PAPERCLIP_CLOUD_API_ORI PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN: "fleet-token", PAPERCLIP_PRIMARY_HOST: "fixture.paperclip.test", PAPERCLIP_AUTH_SECRET: "fixture-secret", PAPERCLIP_CLOUD_RUNTIME_IDENTITY_JWKS: JSON.stringify({ keys: [{ ...publicKey.export({ format: "jwk" }), kid: "fixture", alg: "EdDSA", use: "sig" }] }) }; const now = new Date("2099-01-01T00:00:00Z"), seconds = now.getTime() / 1000; -function receipt(overrides: Record = {}) { - const header = Buffer.from(JSON.stringify({ alg: "EdDSA", kid: "fixture", typ: "paperclip-sandbox-absence+jwt" })).toString("base64url"); - const payload = Buffer.from(JSON.stringify({ v: 1, iss: "paperclip-cloud", aud: "paperclip-sandbox-absence/v1", sub: "fixture-stack", - ...binding, absent: true, iat: seconds, exp: seconds + 60, ...overrides })).toString("base64url"); +function receipt(overrides: Record = {}, purpose = "sandbox-absence", source: Record = binding) { + const header = Buffer.from(JSON.stringify({ alg: "EdDSA", kid: "fixture", typ: `paperclip-${purpose}+jwt` })).toString("base64url"); + const payload = Buffer.from(JSON.stringify({ v: 1, iss: "paperclip-cloud", aud: `paperclip-${purpose}/v1`, sub: "fixture-stack", + ...source, absent: true, iat: seconds, exp: seconds + 60, ...overrides })).toString("base64url"); return `${header}.${payload}.${sign(null, Buffer.from(`${header}.${payload}`), privateKey).toString("base64url")}`; } describe("sandbox absence proof", () => { @@ -45,3 +46,44 @@ describe("sandbox absence proof", () => { expect(fetchImpl).not.toHaveBeenCalled(); }); }); + +describe("cleanup-only sandbox absence", () => { + const cleanup = { purpose: "cleanup" as const, companyId: binding.companyId, environmentId: randomUUID(), + leaseId: binding.leaseId, heartbeatRunId: null as string | null, provider: binding.provider, providerLeaseId: binding.providerLeaseId }; + const lease = { id: cleanup.leaseId, ...cleanup }; + const signed = (overrides: Record = {}) => receipt(overrides, "sandbox-cleanup-absence", cleanup); + it.each([null, binding.runId])("accepts exact absence with nullable run %s without issuing a run receipt", async heartbeatRunId => { + const source = { ...cleanup, heartbeatRunId }; + const fetchImpl = vi.fn().mockResolvedValue(Response.json({ receipt: receipt({}, "sandbox-cleanup-absence", source) })); + expect(await inspectCloudSandboxCleanupAbsence({ ...lease, heartbeatRunId }, { env, now, fetchImpl })) + .toEqual({ providerLeaseId: cleanup.providerLeaseId, state: "destroyed" }); + const [url, init] = fetchImpl.mock.calls[0]; + expect(url).toBe("https://cloud.example.test/v1/tenant/sandbox-absence"); + expect(JSON.parse(init.body)).toEqual(source); + expect(init.headers.authorization).not.toContain(env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN); + expect(init.redirect).toBe("error"); + }); + it("requires every binding, current stack, signature and freshness, and rejects recovery receipts", () => { + expect(verifySandboxCleanupAbsenceReceipt(signed(), cleanup, env, now)).toMatchObject(cleanup); + for (const overrides of [ + ...Object.keys(cleanup).map(key => ({ [key]: randomUUID() })), + { sub: "foreign-stack" }, { absent: false }, { aud: "paperclip-sandbox-absence/v1" }, { iss: "other" }, + { iat: seconds + 31 }, { exp: seconds }, { exp: seconds + 61 }, + ]) expect(verifySandboxCleanupAbsenceReceipt(signed(overrides), cleanup, env, now)).toBeNull(); + expect(verifySandboxCleanupAbsenceReceipt(signed().slice(0, -8) + "tampered", cleanup, env, now)).toBeNull(); + expect(verifySandboxCleanupAbsenceReceipt(receipt(), cleanup, env, now)).toBeNull(); + expect(verifySandboxAbsenceReceipt(signed(), binding, env, now)).toBeNull(); + }); + it("keeps old Clouds, outages, invalid replies and unconfigured instances inconclusive", async () => { + for (const answer of [{ absent: false }, { absent: true }, { receipt: "forged" }, { receipt: signed({ heartbeatRunId: binding.runId }) }]) + expect(await inspectCloudSandboxCleanupAbsence(lease, { env, now, fetchImpl: vi.fn().mockResolvedValue(Response.json(answer)) })).toBeUndefined(); + for (const status of [400, 401, 403, 503]) expect(await inspectCloudSandboxCleanupAbsence(lease, + { env, now, fetchImpl: vi.fn().mockResolvedValue(new Response(null, { status })) })).toBeUndefined(); + expect(await inspectCloudSandboxCleanupAbsence(lease, { env, now, fetchImpl: vi.fn().mockRejectedValue(new Error("offline")) })).toBeUndefined(); + const fetchImpl = vi.fn(); + expect(await inspectCloudSandboxCleanupAbsence(lease, { env: {}, now, fetchImpl })).toBeUndefined(); + for (const change of [{ provider: "other" }, { environmentId: null }, { providerLeaseId: "invalid" }]) + expect(await inspectCloudSandboxCleanupAbsence({ ...lease, ...change }, { env, now, fetchImpl })).toBeUndefined(); + expect(fetchImpl).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/services/sandbox-absence-proof.ts b/server/src/services/sandbox-absence-proof.ts index 8ae852688ed..ae6044dd0af 100644 --- a/server/src/services/sandbox-absence-proof.ts +++ b/server/src/services/sandbox-absence-proof.ts @@ -7,8 +7,18 @@ const bindingSchema = z.object({ companyId: z.string().uuid(), issueId: z.string().uuid(), runId: z.string().uuid(), leaseId: z.string().uuid(), ownerToken: z.string().uuid(), provider: z.literal("daytona"), providerLeaseId: z.string().uuid(), -}); +}).strict(); +const cleanupBindingSchema = z.object({ + purpose: z.literal("cleanup"), companyId: z.string().uuid(), environmentId: z.string().uuid(), + leaseId: z.string().uuid(), heartbeatRunId: z.string().uuid().nullable(), + provider: z.literal("daytona"), providerLeaseId: z.string().uuid(), +}).strict(); export type SandboxAbsenceBinding = z.infer; +export type SandboxCleanupAbsenceBinding = z.infer; +export type SandboxCleanupAbsenceProof = SandboxCleanupAbsenceBinding & { + v: 1; iss: "paperclip-cloud"; aud: "paperclip-sandbox-cleanup-absence/v1"; sub: string; + absent: true; iat: number; exp: number; +}; export type SandboxAbsenceProof = SandboxAbsenceBinding & { v: 1; iss: "paperclip-cloud"; aud: "paperclip-sandbox-absence/v1"; sub: string; absent: true; iat: number; exp: number; @@ -16,19 +26,30 @@ export type SandboxAbsenceProof = SandboxAbsenceBinding & { export function verifySandboxAbsenceReceipt(receipt: string, binding: SandboxAbsenceBinding, env: NodeJS.ProcessEnv = process.env, now = new Date()): SandboxAbsenceProof | null { + const parsed = bindingSchema.safeParse(binding); + return parsed.success ? verifyReceipt(receipt, parsed.data, "sandbox-absence", env, now) as SandboxAbsenceProof | null : null; +} + +export function verifySandboxCleanupAbsenceReceipt(receipt: string, binding: SandboxCleanupAbsenceBinding, + env: NodeJS.ProcessEnv = process.env, now = new Date()): SandboxCleanupAbsenceProof | null { + const parsed = cleanupBindingSchema.safeParse(binding); + return parsed.success ? verifyReceipt(receipt, parsed.data, "sandbox-cleanup-absence", env, now) as SandboxCleanupAbsenceProof | null : null; +} + +function verifyReceipt(receipt: string, binding: Record, purpose: string, + env: NodeJS.ProcessEnv, now: Date): Record | null { try { - const parsed = bindingSchema.safeParse(binding); - if (receipt.length > 16_000 || !parsed.success) return null; + if (receipt.length > 16_000) return null; const parts = receipt.split("."); if (parts.length !== 3 || parts.some(part => !/^[A-Za-z0-9_-]+$/.test(part))) return null; const header = JSON.parse(Buffer.from(parts[0], "base64url").toString("utf8")); - if (header.alg !== "EdDSA" || header.typ !== "paperclip-sandbox-absence+jwt" || typeof header.kid !== "string") return null; + if (header.alg !== "EdDSA" || header.typ !== `paperclip-${purpose}+jwt` || typeof header.kid !== "string") return null; if (!verify(null, Buffer.from(`${parts[0]}.${parts[1]}`, "ascii"), cloudRuntimePublicKeyForKid(env, header.kid), Buffer.from(parts[2], "base64url"))) return null; - const proof = JSON.parse(Buffer.from(parts[1], "base64url").toString("utf8")) as SandboxAbsenceProof; + const proof = JSON.parse(Buffer.from(parts[1], "base64url").toString("utf8")); const seconds = Math.floor(now.getTime() / 1000); - if (proof.v !== 1 || proof.iss !== "paperclip-cloud" || proof.aud !== "paperclip-sandbox-absence/v1" + if (proof.v !== 1 || proof.iss !== "paperclip-cloud" || proof.aud !== `paperclip-${purpose}/v1` || !env.PAPERCLIP_CLOUD_STACK_ID || proof.sub !== env.PAPERCLIP_CLOUD_STACK_ID || proof.absent !== true - || Object.keys(parsed.data).some(key => proof[key as keyof SandboxAbsenceBinding] !== parsed.data[key as keyof SandboxAbsenceBinding]) + || Object.keys(binding).some(key => proof[key] !== binding[key]) || !Number.isInteger(proof.iat) || !Number.isInteger(proof.exp) || proof.iat > seconds + 30 || proof.exp <= seconds || proof.exp <= proof.iat || proof.exp - proof.iat > 60) return null; return proof; @@ -38,11 +59,35 @@ export function verifySandboxAbsenceReceipt(receipt: string, binding: SandboxAbs export async function requestSandboxAbsenceProof(binding: SandboxAbsenceBinding, options: { env?: NodeJS.ProcessEnv; fetchImpl?: typeof fetch; now?: Date; } = {}): Promise<{ proof: SandboxAbsenceProof; receipt: string } | null> { + const parsed = bindingSchema.safeParse(binding); + if (!parsed.success) return null; + const receipt = await requestReceipt(parsed.data, options); + if (!receipt) return null; + const proof = verifySandboxAbsenceReceipt(receipt, parsed.data, options.env, options.now); + return proof ? { proof, receipt } : null; +} + +/** Cleanup-only observation: never grants continuation authority to a run. */ +export async function inspectCloudSandboxCleanupAbsence(lease: { + id: string; companyId: string; environmentId: string | null; heartbeatRunId: string | null; + provider: string | null; providerLeaseId: string | null; +}, options: { env?: NodeJS.ProcessEnv; fetchImpl?: typeof fetch; now?: Date } = {}) { + const parsed = cleanupBindingSchema.safeParse({ purpose: "cleanup", companyId: lease.companyId, + environmentId: lease.environmentId, leaseId: lease.id, heartbeatRunId: lease.heartbeatRunId, + provider: lease.provider, providerLeaseId: lease.providerLeaseId }); + if (!parsed.success) return undefined; + const receipt = await requestReceipt(parsed.data, options); + if (!receipt || !verifySandboxCleanupAbsenceReceipt(receipt, parsed.data, options.env, options.now)) return undefined; + return { providerLeaseId: parsed.data.providerLeaseId, state: "destroyed" as const }; +} + +async function requestReceipt(binding: Record, options: { + env?: NodeJS.ProcessEnv; fetchImpl?: typeof fetch; +}): Promise { const env = options.env ?? process.env; const context = getCloudStackContext(env); const secret = env.PAPERCLIP_AUTH_SECRET ?? env.BETTER_AUTH_SECRET; - const parsed = bindingSchema.safeParse(binding); - if (!context?.stackId || !context.primaryHost || !context.cloudOrigin || !secret?.trim() || !parsed.success) return null; + if (!context?.stackId || !context.primaryHost || !context.cloudOrigin || !secret?.trim()) return null; try { const origin = new URL(context.cloudOrigin); const host = new URL(`https://${context.primaryHost}`); @@ -51,12 +96,11 @@ export async function requestSandboxAbsenceProof(binding: SandboxAbsenceBinding, const response = await (options.fetchImpl ?? fetch)(`${origin.origin}/v1/tenant/sandbox-absence`, { method: "POST", redirect: "error", signal: AbortSignal.timeout(10_000), headers: { authorization: `Bearer ${capability}`, "x-paperclip-cloud-tenant-host": host.hostname, "content-type": "application/json" }, - body: JSON.stringify(parsed.data), + body: JSON.stringify(binding), }); if (!response.ok) return null; const payload = await response.json() as { receipt?: unknown }; if (typeof payload.receipt !== "string") return null; - const proof = verifySandboxAbsenceReceipt(payload.receipt, binding, env, options.now); - return proof ? { proof, receipt: payload.receipt } : null; + return payload.receipt; } catch { return null; } } From 493e229d93ba7798e80152f4549e2e9177ce50ae Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 30 Oct 2026 17:07:32 -0700 Subject: [PATCH 7/8] perf(runner): cache verified native executable digests (#18390) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip manages AI agents and their task execution. > - Runner checks qualified native programs before it starts a provider. > - Installation checks and command opening can hash the same program again. > - A new sidecar process repeats this work on a later warm task. > - This pull request stores the computed digest in a private filesystem cache. > - Every cache hit still checks the independently selected trusted digest and current file metadata. > - An unchanged program can start without another full native file read. ## Linked Issues or Issue Description **What existing behavior does this improve?** Repeated **qualified ACPX** native executable admission in Runner. Claude, ACPX-Codex, and Grok use this verifier in both Core-owned and plugin-backed launches. The ordinary native Codex app-server launcher is a different path and is not changed. A retained provider that opens no new native command has no saved hash work here. The measured Claude task path does not require a comment backlog. **Current behavior** Installation verification and final command opening each read and hash the native executable. The qualified Linux x64 Claude file is 241,667,256 bytes. A new verification and command lease reads 483,334,512 native bytes. Another Node process repeats both reads. **Proposed behavior** Store the actual computed SHA-256 digest by device, inode, size, nanosecond modification time, and nanosecond change time. Reuse it across Runner and sidecar processes. Compare each hit with the current build-owned digest. Hash the file on a miss or changed identity. This replaces the earlier Claude-only deferral design in this same PR. **Reason and benefit** The first verification and command lease reads the native file once. Later processes read none of its native bytes while its identity stays the same. Related: #16372 established qualified executable admission. This change is independent of other preparation PRs. It does not change provider pins or profile versions. ## What Changed - Add a shared filesystem digest cache under the absolute OS temporary root at `paperclip-runner-/executable-digests-v1`. - Use private directories and files, versioned hash-only filenames, bounded JSON entries, and atomic temporary-file replacement. Cache files contain a version and the computed digest. They contain no credentials or source paths. - Fall back to full hashing when the cache is missing, unreadable, malformed, or unusable. Keep current expected-digest selection outside the cache. - Keep cache data outside task homes. Neither `HOME` nor task cwd selects the path. The OS temporary-root selection honors absolute platform `TMPDIR`, `TMP`, or `TEMP` overrides. A relative root disables caching. Hosts must keep that root outside task workspaces. Temp cleanup causes a later rehash. - Keep the no-follow executable open, regular-file and size checks, execute permission, descriptor ownership, before/after identity checks, and preparation-to-launch identity comparison. Cache publication occurs before the final identity checks. - Use this path for standalone installation checks and final command opening. Remove the earlier pending-digest type and Claude-only launch helper. Runtime host and profile selection keep their original structure. - Test changed content, all fingerprint fields, symlinks, mode changes, inode replacement, mutation during lookup/publication, concurrent cache writers, wrong trusted pins, and cache failures. - Include a deliberate trust-boundary test: a process running as the same OS user can plant a matching cached digest for changed bytes. It opens a test descriptor and executes nothing. In plain language: Runner remembers the hash for the same file. It still checks which file it opened and which hash the build requires. The cache trusts the OS user who can write it. ## Verification From `packages/paperclip-runner`: - `../../node_modules/.bin/vitest run src/drivers/acpx/executable-digest-cache.test.ts src/drivers/acpx/installation-integrity.test.ts src/drivers/acpx/runtime-host.test.ts` — 177 passed, 6 platform-specific skips. - `../../node_modules/.bin/tsc -p tsconfig.json --noEmit` — passed. - `git diff --check` and the added-line secret, PII, private-address, and code-comment scan — passed. The offline comparison uses baseline `45a3a92e6dc52d20923b9aa34a22fee2ec5443f9` and treatment `c4356df12032448de90516ae329d8f939af10984`. Each sample starts a different Node process. Each normal sample runs the actual qualified installation verifier and opens a command lease. Seven samples per case follow one warmup. The cache is removed before each cold case. The next process reuses its entry. Baseline order alternates around the treatment pair. Command digest and package paths match in every pair. | Verifier plus command opening | Baseline | Cold cache | Later process hit | | --- | ---: | ---: | ---: | | Full native reads | 2 | 1 | 0 | | Native bytes read | 483,334,512 | 241,667,256 | 0 | | Local median, ms | 1041.26 | 602.70 | 130.17 | | Local range, ms | 994.20–1133.11 | 580.27–999.83 | 121.96–179.45 | | Median whole child process, ms | 1379.96 | 988.75 | 523.01 | Raw verifier-plus-open samples in ms: - Baseline: 1133.11, 1074.40, 1061.99, 1041.26, 994.20, 1036.12, 1037.70. - Cold: 647.13, 646.33, 999.83, 584.83, 580.27, 583.37, 602.70. - Later process hit: 179.45, 138.37, 134.39, 130.17, 121.96, 122.33, 126.68. Separate direct-opener fault probes use a disposable copy of that same native file: | Change | Native bytes read | Result | Local open, ms | | --- | ---: | --- | ---: | | First copied file | 241,667,256 | Accepted | 526.14 | | Next process, different HOME and task cwd | 0 | Accepted | 24.91 | | Changed timestamps | 241,667,256 | Accepted after hashing | 531.40 | | New inode, identical bytes | 241,667,256 | Accepted after hashing | 536.70 | | Corrupt byte, same size and exactly restored mtime | 241,667,256 | Rejected | Whole process: 841.36 | | Different independent trusted pin | 0 | Rejected | Whole process: 374.76 | The corruption probe keeps `mtimeNs` exactly equal before and after, at `1000000000000000000`. Its changed `ctimeNs` invalidates the entry. Cache expiry is not required. The changed-pin probe rejects the old actual digest without a native read. The cached image is `sha256:99ce80690a0c13328f41fd84156cc73e1ac562eaed3120fd41d04d7acdfb325d`. The Node binary is `/opt/paperclip/runner/libexec/node`, version `v24.20.0`. The artifact is Claude SDK `0.3.286`, with expected digest `sha256:fe503f65c6289d59c23e5b21ae44f03583f997dd33a2cbfc75ab4f96fb8fc73f`. The container used a read-only root, a disposable writable tmpfs, and disabled network. No provider process, model request, credential, or deployment was used. **These elapsed values are diagnostics from Linux x64 CPU emulation on an ARM64 Docker host. They are not production latency or TTFT estimates.** The exact byte counts, cross-process reuse, output equality, and invalidation results are the portable evidence. The saved phase 0 exports have no directly comparable native executable hashing span. Their broad `runner.transport.attach` spans include other work. A production comparison within 20% is therefore unavailable for this seam. The earlier local 919 ms control was also an emulation diagnostic; it is not a production baseline. The initial Docker attempt was denied access to the local socket by the sandbox. The same offline command then ran with local socket permission. An initial fault probe restored mtime through a `Date`, which did not prove exact nanosecond equality. Its raw report is retained as historical; the final probe sets and verifies an exact mtime before and after corruption. The prior Claude-only implementation and its checks and measurements remain historical evidence. They do not qualify this replacement. The superseded home-directory cache head had one browser recovery test timeout in its GitHub workflow. It expected a saved title after session recovery. The failure is retained. No browser code was changed; the current head reruns CI after moving the cache outside task homes. No full local CI suite was run, as requested. GitHub CI supplies full regression coverage. Current head `c4356df12032448de90516ae329d8f939af10984` passed the complete [GitHub workflow](https://github.com/paperclipai/paperclip/actions/runs/41145223331), including all browser shards. Its latest checks have 56 successes and 2 intended skips, including both security scans started when the PR became ready. Standard Greptile reviewed this exact head at 5/5 with no open findings. Apex then reviewed the same head at 5/5 under the documented OS-user trust policy. It reported no new findings. There are no unresolved review threads. The PR is ready for review and remains unmerged. ## Risks - This deliberately weakens protection against the same OS user. That user can write the cache and can plant a chosen digest for a file fingerprint. A cache hit trusts that record; metadata identity is not a fresh content hash. The explicit fault test documents this accepted policy. - A read-only administrator-owned Runner can use this user-writable cache. The cache has no signature, secret, or daemon. It does not require a Launcher or ROM change. - Other users cannot use normal cache permissions to write these entries. Cache errors fall back to hashing. Removing the cache directory restores full hashing. Old fingerprints leave small unused entries until the cache is removed or OS temp cleanup evicts it. Temp eviction costs one later full hash. - A changed file normally rehashes. A digest that differs from the independent current pin still fails admission. Existing command, package, credential, process-lifetime, and final dispatch checks remain. - No schema, public protocol, package dependency, profile version, or provider pin changes. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, code editing, test execution, and tool use. This session does not expose a more precise serving model ID or context-window capacity. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used with the details available to this session - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR following the enhancement template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket ID - [x] The targeted local tests pass; the full local suite was not run as requested - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation - [x] I have considered and documented risks - [x] All Paperclip CI gates are green for the replacement head - [x] Standard Greptile is 5/5 with no open findings for the replacement head - [x] Apex is 5/5 on the current head with no unresolved findings - [x] I will address review findings before requesting merge --------- Co-authored-by: Paperclip --- .../paperclip-runner/docs/durable-recovery.md | 29 ++++ .../acpx/executable-digest-cache.test.ts | 160 ++++++++++++++++++ .../drivers/acpx/installation-integrity.ts | 52 +++--- .../src/executable-digest-cache.ts | 70 ++++++++ 4 files changed, 286 insertions(+), 25 deletions(-) create mode 100644 packages/paperclip-runner/src/drivers/acpx/executable-digest-cache.test.ts create mode 100644 packages/paperclip-runner/src/executable-digest-cache.ts diff --git a/packages/paperclip-runner/docs/durable-recovery.md b/packages/paperclip-runner/docs/durable-recovery.md index 0fbef29f508..f04aab08a23 100644 --- a/packages/paperclip-runner/docs/durable-recovery.md +++ b/packages/paperclip-runner/docs/durable-recovery.md @@ -98,6 +98,35 @@ digest, and refuse the rest. A profile identity does not establish hosted harness qualification. [`rules/acpx-profile-identity.mdc`](../../../rules/acpx-profile-identity.mdc) summarizes these practices for contributors. +Qualified ACPX native executables use Runner's persistent digest cache. The cache +stores a computed SHA-256 digest under a key made from device, inode, size, +nanosecond modification time, and nanosecond change time. Each check compares +that digest with the independently selected build-owned pin. A changed file +identity, missing entry, or unusable cache causes a full descriptor read and +hash. A hit reuses the earlier digest; metadata identity is not a fresh content +hash. Claude, Codex, and Grok use the same native executable admission path. + +Cache entries live in the operating-system temporary directory under +`paperclip-runner-/executable-digests-v1`, outside task workspaces such as +`/home/agent`. Neither `HOME` nor the task working directory selects this path. +Node's `os.tmpdir()` selects the root and honors the platform's absolute +`TMPDIR`, `TMP`, or `TEMP` override. Hosts must keep that root outside task +workspaces; a relative or unusable root disables caching. Temporary-directory +eviction removes the optimization until a later full hash fills it again. The +directory and files are private to that user. Writes use a temporary file and +atomic rename, so later Runner or sidecar processes can reuse complete entries. +Cache errors fall back to hashing. The cache stores no credentials or file paths. + +Every admission still opens the executable without following its final symlink, +checks its regular-file type, bounds, execute permission, and before/after file +identity, and retains the descriptor for launch. Package qualification and the +preparation-to-launch identity check remain. The cache deliberately trusts the +operating-system user: a process with that user's access can plant a digest for +a chosen fingerprint. It is not authenticated with a secret or signature. +An administrator-owned read-only Runner can therefore use a user-writable cache, +but this provides weaker protection against that same user than hashing every +executable on each admission. Removing the cache restores full hashing. + ## External controller checkpoints The optional controller persistence callback stores recovery control state, not diff --git a/packages/paperclip-runner/src/drivers/acpx/executable-digest-cache.test.ts b/packages/paperclip-runner/src/drivers/acpx/executable-digest-cache.test.ts new file mode 100644 index 00000000000..f33eb4fd39f --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/executable-digest-cache.test.ts @@ -0,0 +1,160 @@ +import { createHash } from "node:crypto"; +import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rename, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createExecutableDigestCache, executableDigestCache, type ExecutableFileIdentity } from "../../executable-digest-cache.js"; +import { openVerifiedRuntimeExecutable } from "./installation-integrity.js"; + +const temporary: string[] = []; +const identity: ExecutableFileIdentity = { device: "1", inode: "2", size: "3", modifiedNanoseconds: "4", changedNanoseconds: "5" }; +const digest = `sha256:${"a".repeat(64)}`; +afterEach(async () => { + vi.restoreAllMocks(); + await Promise.all(temporary.splice(0).map(path => rm(path, { recursive: true, force: true }))); +}); + +async function fixture() { + const root = await mkdtemp(join(tmpdir(), "runner-executable-digest-cache-")); + temporary.push(root); + const directory = join(root, "cache"); + const cache = createExecutableDigestCache(directory); + return { root, directory, cache }; +} + +describe("persistent executable digest cache", () => { + it("stores the actual digest in a private, versioned entry and survives a new cache instance", async () => { + const f = await fixture(); + expect(await f.cache.read(identity)).toBeNull(); + await f.cache.write(identity, digest); + expect(await createExecutableDigestCache(f.directory).read(identity)).toBe(digest); + const [name] = await readdir(f.directory); + expect(name).toMatch(/^v1-[a-f0-9]{64}\.json$/); + expect(JSON.parse(await readFile(join(f.directory, name), "utf8"))).toEqual({ version: 1, digest }); + expect((await lstat(f.directory)).mode & 0o777).toBe(0o700); + expect((await lstat(join(f.directory, name))).mode & 0o777).toBe(0o600); + }); + + it.each(Object.keys(identity) as (keyof ExecutableFileIdentity)[])("invalidates a changed %s", async key => { + const f = await fixture(); + await f.cache.write(identity, digest); + expect(await f.cache.read({ ...identity, [key]: "changed" })).toBeNull(); + }); + + it.each(["malformed", "version", "digest", "oversized", "permissions", "symlink"] as const)( + "treats an unusable %s entry as a miss", async kind => { + const f = await fixture(); + await f.cache.write(identity, digest); + const [name] = await readdir(f.directory), file = join(f.directory, name); + if (kind === "malformed") await writeFile(file, "{"); + else if (kind === "version") await writeFile(file, JSON.stringify({ version: 2, digest })); + else if (kind === "digest") await writeFile(file, JSON.stringify({ version: 1, digest: "expected" })); + else if (kind === "oversized") await writeFile(file, "x".repeat(1025)); + else if (kind === "permissions") await chmod(file, 0o644); + else { await rename(file, `${file}.target`); await symlink(`${file}.target`, file); } + expect(await f.cache.read(identity)).toBeNull(); + }, + ); + + it("falls back when the cache directory cannot be used and leaves existing permissions unchanged", async () => { + const f = await fixture(); + await writeFile(f.directory, "not a directory"); + await expect(f.cache.write(identity, digest)).resolves.toBeUndefined(); + expect(await f.cache.read(identity)).toBeNull(); + await rm(f.directory); + await mkdir(f.directory, { mode: 0o755 }); + await f.cache.write(identity, digest); + expect(await f.cache.read(identity)).toBeNull(); + expect((await lstat(f.directory)).mode & 0o777).toBe(0o755); + }); + + it("does not resolve a relative cache directory through the task working directory", async () => { + const cache = createExecutableDigestCache("relative-task-cache"); + await cache.write(identity, digest); + expect(await cache.read(identity)).toBeNull(); + }); + + it("publishes complete entries under concurrent writers without leftover temporary files", async () => { + const f = await fixture(); + await Promise.all(Array.from({ length: 8 }, () => f.cache.write(identity, digest))); + expect(await f.cache.read(identity)).toBe(digest); + expect(await readdir(f.directory)).toHaveLength(1); + }); +}); + +describe.skipIf(!["linux", "darwin"].includes(process.platform))("descriptor admission with cached digests", () => { + async function executable() { + const f = await fixture(), path = join(f.root, "native"), bytes = Buffer.from("qualified native fixture"); + await writeFile(path, bytes, { mode: 0o700 }); + vi.spyOn(executableDigestCache, "read").mockImplementation(f.cache.read); + vi.spyOn(executableDigestCache, "write").mockImplementation(f.cache.write); + return { ...f, path, digest: `sha256:${createHash("sha256").update(bytes).digest("hex")}` }; + } + + it("hashes a miss, reuses the actual digest, and still rejects a different independent pin", async () => { + const f = await executable(); + await (await openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).handle.close(); + expect(executableDigestCache.write).toHaveBeenCalledOnce(); + await (await openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).handle.close(); + expect(executableDigestCache.write).toHaveBeenCalledOnce(); + await expect(openVerifiedRuntimeExecutable(f.path, digest, "fixture")).rejects.toThrow("digest mismatch"); + }); + + it("rehashes changed contents and rejects corruption despite a previous valid entry", async () => { + const f = await executable(); + await (await openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).handle.close(); + await writeFile(f.path, "corrupted native fixture"); + await expect(openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).rejects.toThrow("digest mismatch"); + }); + + it.each(["permissions", "symlink", "replacement"] as const)("retains admission checks after cached %s changes", async kind => { + const f = await executable(); + await (await openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).handle.close(); + if (kind === "permissions") { + await chmod(f.path, 0o600); + await expect(openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).rejects.toThrow("bounded executable file"); + } else if (kind === "symlink") { + await rename(f.path, `${f.path}.original`); await symlink(`${f.path}.original`, f.path); + await expect(openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).rejects.toThrow("real regular file"); + } else { + await rename(f.path, `${f.path}.original`); await writeFile(f.path, "new invalid executable", { mode: 0o700 }); + await expect(openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).rejects.toThrow("digest mismatch"); + } + }); + + it("rejects mutation while a cache hit is being read", async () => { + const f = await executable(); + await (await openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).handle.close(); + vi.mocked(executableDigestCache.read).mockImplementationOnce(async () => { + await writeFile(f.path, "mutation during lookup"); + return f.digest; + }); + await expect(openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).rejects.toThrow("changed while it was verified"); + }); + + it("checks executable identity after publishing a cold cache entry", async () => { + const f = await executable(); + vi.mocked(executableDigestCache.write).mockImplementationOnce(async (identity, digest) => { + await f.cache.write(identity, digest); + await writeFile(f.path, "mutation during cache publication"); + }); + await expect(openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).rejects.toThrow("changed while it was verified"); + }); + + it("falls back to hashing when the cache is unavailable", async () => { + const f = await executable(); + await writeFile(f.directory, "unavailable cache"); + await (await openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).handle.close(); + await writeFile(f.path, "invalid native fixture"); + await expect(openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).rejects.toThrow("digest mismatch"); + }); + + it("trusts a digest planted by the same operating-system user for current metadata", async () => { + const f = await executable(); + await writeFile(f.path, "content changed by the same user"); + const metadata = await lstat(f.path, { bigint: true }); + await f.cache.write({ device: metadata.dev.toString(), inode: metadata.ino.toString(), size: metadata.size.toString(), + modifiedNanoseconds: metadata.mtimeNs.toString(), changedNanoseconds: metadata.ctimeNs.toString() }, f.digest); + await (await openVerifiedRuntimeExecutable(f.path, f.digest, "fixture")).handle.close(); + }); +}); diff --git a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts index fbf39064e18..1fab0150997 100644 --- a/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts +++ b/packages/paperclip-runner/src/drivers/acpx/installation-integrity.ts @@ -2,6 +2,7 @@ import { MAX_ACPX_RUNTIME_EXECUTABLE_BYTES, ACPX_PRIVATE_SNAPSHOT_ENV, createAcp import { createNativeAcpxDistributionSnapshot, NATIVE_ACPX_BOOTSTRAP_NAME, readNativeAcpxDistributionEntries, type NativeAcpxDistributionInput } from "./native-distribution-integrity.js"; export type { NativeAcpxDistributionInput } from "./native-distribution-integrity.js"; import { createHash } from "node:crypto"; +import { executableDigestCache, type ExecutableFileIdentity } from "../../executable-digest-cache.js"; import { spawn as spawnChildProcess, type ChildProcess, @@ -606,13 +607,7 @@ export async function awaitVerifiedAcpxProviderExit( await exitProof; } -interface VerifiedAcpxCommandIdentity { - device: string; - inode: string; - size: string; - modifiedNanoseconds: string; - changedNanoseconds: string; -} +type VerifiedAcpxCommandIdentity = ExecutableFileIdentity; interface VerifiedAcpxRuntimeExecutable { path: string; @@ -1204,7 +1199,7 @@ async function inspectCommand( } } -async function openVerifiedRuntimeExecutable( +export async function openVerifiedRuntimeExecutable( executablePath: string, expectedDigest: string, agent: string, @@ -1246,23 +1241,31 @@ async function openVerifiedRuntimeExecutable( `ACPX ${agent} runtime executable must be a bounded executable file`, ); } - const hash = createHash("sha256"); - const buffer = Buffer.alloc(1024 * 1024); + const cachedDigest = await executableDigestCache.read(fileIdentity(before)); + let digest = cachedDigest; let position = 0; - try { - while (position < Number(before.size)) { - const { bytesRead } = await handle.read( - buffer, - 0, - Math.min(buffer.length, Number(before.size) - position), - position, - ); - if (bytesRead === 0) break; - hash.update(buffer.subarray(0, bytesRead)); - position += bytesRead; + if (digest === null) { + const hash = createHash("sha256"); + const buffer = Buffer.alloc(1024 * 1024); + try { + while (position < Number(before.size)) { + const { bytesRead } = await handle.read( + buffer, + 0, + Math.min(buffer.length, Number(before.size) - position), + position, + ); + if (bytesRead === 0) break; + hash.update(buffer.subarray(0, bytesRead)); + position += bytesRead; + } + } finally { + buffer.fill(0); } - } finally { - buffer.fill(0); + digest = `sha256:${hash.digest("hex")}`; + } + if (cachedDigest === null && position === Number(before.size) && digest === expectedDigest) { + await executableDigestCache.write(fileIdentity(before), digest); } const after = await handle.stat({ bigint: true }); const lexicalAfter = await lstat(executablePath, { bigint: true }).catch( @@ -1271,7 +1274,7 @@ async function openVerifiedRuntimeExecutable( const beforeIdentity = fileIdentity(before); const afterIdentity = fileIdentity(after); if ( - position !== Number(before.size) || + (cachedDigest === null && position !== Number(before.size)) || lexicalAfter === null || lexicalAfter.isSymbolicLink() || !lexicalAfter.isFile() || @@ -1283,7 +1286,6 @@ async function openVerifiedRuntimeExecutable( `ACPX ${agent} runtime executable changed while it was verified`, ); } - const digest = `sha256:${hash.digest("hex")}`; if (digest !== expectedDigest) { throw new Error(`ACPX ${agent} runtime executable digest mismatch`); } diff --git a/packages/paperclip-runner/src/executable-digest-cache.ts b/packages/paperclip-runner/src/executable-digest-cache.ts new file mode 100644 index 00000000000..b50b54d9059 --- /dev/null +++ b/packages/paperclip-runner/src/executable-digest-cache.ts @@ -0,0 +1,70 @@ +import { createHash, randomUUID } from "node:crypto"; +import { constants } from "node:fs"; +import { lstat, mkdir, open, rename, unlink } from "node:fs/promises"; +import { tmpdir, userInfo } from "node:os"; +import { isAbsolute, join } from "node:path"; + +export interface ExecutableFileIdentity { + device: string; + inode: string; + size: string; + modifiedNanoseconds: string; + changedNanoseconds: string; +} + +export function createExecutableDigestCache(directory?: string) { + async function location(identity: ExecutableFileIdentity, create: boolean): Promise { + const user = userInfo(); + const temporaryRoot = tmpdir(); + if (!isAbsolute(directory ?? temporaryRoot)) throw new Error("Executable digest cache directory is unavailable"); + const base = join(temporaryRoot, `paperclip-runner-${user.uid}`); + const directories = directory ? [directory] : [base, join(base, "executable-digests-v1")]; + for (const [index, path] of directories.entries()) { + if (create) await mkdir(path, { mode: 0o700 }).catch(error => { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + }); + const metadata = await lstat(path); + if (!metadata.isDirectory() || metadata.isSymbolicLink() || metadata.uid !== user.uid || + (metadata.mode & (index === directories.length - 1 ? 0o077 : 0o022)) !== 0) { + throw new Error("Executable digest cache directory is unavailable"); + } + } + const key = createHash("sha256").update(JSON.stringify([1, identity.device, identity.inode, identity.size, + identity.modifiedNanoseconds, identity.changedNanoseconds])).digest("hex"); + return join(directories.at(-1)!, `v1-${key}.json`); + } + + return { + async read(identity: ExecutableFileIdentity): Promise { + try { + const path = await location(identity, false); + const handle = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW); + try { + const metadata = await handle.stat(); + if (!metadata.isFile() || metadata.uid !== userInfo().uid || (metadata.mode & 0o077) !== 0 || + metadata.size < 1 || metadata.size > 1024) return null; + const entry: unknown = JSON.parse(await handle.readFile("utf8")); + if (typeof entry !== "object" || entry === null || !("version" in entry) || entry.version !== 1 || + !("digest" in entry) || typeof entry.digest !== "string" || !/^sha256:[a-f0-9]{64}$/.test(entry.digest)) return null; + return entry.digest; + } finally { await handle.close(); } + } catch { return null; } + }, + async write(identity: ExecutableFileIdentity, digest: string): Promise { + let temporary: string | undefined; + try { + if (!/^sha256:[a-f0-9]{64}$/.test(digest)) return; + const path = await location(identity, true); + temporary = `${path}.${randomUUID()}.tmp`; + const handle = await open(temporary, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | constants.O_NOFOLLOW, 0o600); + try { await handle.writeFile(JSON.stringify({ version: 1, digest })); } + finally { await handle.close(); } + await rename(temporary, path); + } catch { } finally { + if (temporary) await unlink(temporary).catch(() => undefined); + } + }, + }; +} + +export const executableDigestCache = createExecutableDigestCache(); From 8f58be556a566235779319cb664aa085d8afee1f Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 30 Oct 2026 20:18:34 -0500 Subject: [PATCH 8/8] fix(ui): retain startup autosave through session recovery Co-Authored-By: Paperclip --- tests/e2e/disconnect-write-recovery.spec.ts | 23 +++++++- ui/src/hooks/useAccountBoundMutation.ts | 60 +-------------------- ui/src/hooks/useDurableAutosave.test.tsx | 45 ++++++++++++++++ ui/src/hooks/useDurableAutosave.ts | 57 +++++++++++++------- ui/src/lib/account-scope.ts | 59 ++++++++++++++++++++ 5 files changed, 167 insertions(+), 77 deletions(-) create mode 100644 ui/src/lib/account-scope.ts diff --git a/tests/e2e/disconnect-write-recovery.spec.ts b/tests/e2e/disconnect-write-recovery.spec.ts index 8f81ee178dc..780f6f27a04 100644 --- a/tests/e2e/disconnect-write-recovery.spec.ts +++ b/tests/e2e/disconnect-write-recovery.spec.ts @@ -178,10 +178,19 @@ test("replaying an accepted comment does not reopen completed work", async ({ pa } }); -test("startup work retries a failed session lookup while other APIs stay healthy", async ({ page, request }) => { +for (const delayWorkspacePreference of [false, true]) test(delayWorkspacePreference + ? "startup title survives session recovery while workspace preferences remount the editor" + : "startup work retries a failed session lookup while other APIs stay healthy", async ({ page, request }) => { const company = await json(await request.post("/api/companies", { data: { name: `Session recovery ${Date.now()}` } })); const session = await json(await request.get("/api/auth/get-session")); let healthy = false, saving = false, failedAfterSave = 0; + let releasePreference!: () => void; + const preferenceGate = new Promise(resolve => { releasePreference = resolve; }); + let sawPreference!: () => void; + const preferenceRequested = new Promise(resolve => { sawPreference = resolve; }); + if (delayWorkspacePreference) await page.route("**/api/workspace-preferences/me", async route => { + sawPreference(); await preferenceGate; await route.continue(); + }); try { const issue = await json(await request.post(`/api/companies/${company.id}/issues`, { data: { title: "Before session recovery", status: "backlog" }, @@ -199,12 +208,24 @@ test("startup work retries a failed session lookup while other APIs stay healthy saving = true; await header.locator("textarea").press("Enter"); await expect.poll(() => failedAfterSave).toBeGreaterThan(0); + if (delayWorkspacePreference) { + await expect(page.getByText("Couldn't update the task", { exact: true })).toBeVisible(); + await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => resolve(null)))); + } healthy = true; + if (delayWorkspacePreference) { + // Returning to the tab retries session lookup while the saved edit waits. + await page.evaluate(() => window.dispatchEvent(new Event("visibilitychange"))); + await preferenceRequested; + await expect(page.getByTestId("task-chat-history-content")).toBeHidden(); + releasePreference(); + } await expect.poll(async () => (await json(await request.get(`/api/issues/${issue.id}`))).title, { timeout: 15_000 }).toBe("Saved after session recovery"); await page.reload(); await expect(header.getByRole("heading", { name: "Saved after session recovery", exact: true })).toBeVisible(); } finally { + releasePreference(); await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } }); } }); diff --git a/ui/src/hooks/useAccountBoundMutation.ts b/ui/src/hooks/useAccountBoundMutation.ts index b011b9ec8ad..5cbf156b703 100644 --- a/ui/src/hooks/useAccountBoundMutation.ts +++ b/ui/src/hooks/useAccountBoundMutation.ts @@ -5,68 +5,12 @@ import { QueryClientContext, type DefaultError, type MutateOptions, - type QueryClient, type UseMutationOptions, type UseMutationResult, type MutationFunctionContext, } from "@tanstack/react-query"; -import { queryKeys } from "@/lib/queryKeys"; -import { authApi } from "@/api/auth"; -import { classifySendFailure, PendingSendAccountChangedError } from "@/lib/pending-send"; - -type AccountGeneration = { owner: string | undefined; generation: number }; -const accounts = new WeakMap(); - -function currentOwner(client: QueryClient): string | undefined { - const session = client.getQueryData<{ user?: { id?: string }; session?: { userId?: string } } | null>(queryKeys.auth.session); - return session === undefined ? undefined : session?.user?.id ?? session?.session?.userId ?? "signed-out"; -} - -function bindAccount(client: QueryClient) { - let account = accounts.get(client); - if (!account) { - account = { owner: currentOwner(client), generation: 0 }; - accounts.set(client, account); - const tracked = account; - client.getQueryCache().subscribe(event => { - const owner = currentOwner(client); - const removed = event.type === "removed" && JSON.stringify(event.query.queryKey) === JSON.stringify(queryKeys.auth.session); - if (owner !== tracked.owner || removed) { - if (removed || tracked.owner !== undefined || tracked.generation !== 0) tracked.generation += 1; - tracked.owner = owner; - } - }); - } - const { owner, generation } = account; - const isCurrentScope = () => account.generation === generation && (owner === undefined ? generation === 0 : account.owner === owner); - const isCurrentAccount = () => account.owner !== undefined && isCurrentScope(); - const whenAccountReady = async () => { - if (account.owner !== undefined || !isCurrentScope()) return; - let stopped = false; - let unsubscribe = () => {}; - const ready = new Promise(resolve => { - unsubscribe = client.getQueryCache().subscribe(() => { - if (account.owner !== undefined || !isCurrentScope()) resolve(); - }); - }); - const lookup = async () => { - for (let attempt = 0; attempt < 3; attempt += 1) { - if (stopped || account.owner !== undefined || !isCurrentScope()) return; - try { - await client.fetchQuery({ queryKey: queryKeys.auth.session, queryFn: () => authApi.getSession(), retry: false }); - return; - } catch (error) { - if (stopped || account.owner !== undefined || !isCurrentScope()) return; - if (attempt === 2 || classifySendFailure(error) !== "pending") throw error; - await new Promise(resolve => setTimeout(resolve, 250 * 2 ** attempt)); - } - } - }; - try { await Promise.race([ready, lookup()]); } - finally { stopped = true; unsubscribe(); } - }; - return { isCurrentAccount, isCurrentScope, whenAccountReady }; -} +import { PendingSendAccountChangedError } from "@/lib/pending-send"; +import { bindAccount } from "@/lib/account-scope"; export function useCurrentAccountGuard(): () => boolean { const client = useContext(QueryClientContext); diff --git a/ui/src/hooks/useDurableAutosave.test.tsx b/ui/src/hooks/useDurableAutosave.test.tsx index edad11f04a0..af5860e0a85 100644 --- a/ui/src/hooks/useDurableAutosave.test.tsx +++ b/ui/src/hooks/useDurableAutosave.test.tsx @@ -173,6 +173,51 @@ describe("useDurableAutosave", () => { client.clear(); }); + it.each([false, true])("keeps an unowned startup edit when identity and unmount race (identityFirst=%s)", async identityFirst => { + store.setBrowserOnline(false); + const client = new QueryClient({ defaultOptions: { queries: { staleTime: Infinity, retry: false, enabled: false } } }); + const save = vi.fn().mockResolvedValue(undefined); + act(() => root.render( + + )); + await act(async () => { await current.run("Retained startup title"); }); + expect(current.status).toBe("waiting"); + act(() => { + if (identityFirst) client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + root.unmount(); root = createRoot(container); + if (!identityFirst) client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + }); + await act(async () => { store.setBrowserOnline(true); store.reportSuccess(); }); + await advance(2_000); + expect(save).toHaveBeenCalledExactlyOnceWith("Retained startup title"); + expect(store.getSnapshot().pendingWrites).toBe(0); + client.clear(); + }); + + it.each(["different-account", "round-trip", "removed-session", "signed-out"])( + "does not adopt a later identity for a detached startup edit (%s)", async transition => { + store.setBrowserOnline(false); + const client = new QueryClient({ defaultOptions: { queries: { staleTime: Infinity, retry: false, enabled: false } } }); + const save = vi.fn().mockResolvedValue(undefined); + act(() => root.render( + + )); + await act(async () => { await current.run("Private startup title"); }); + act(() => { root.unmount(); root = createRoot(container); }); + act(() => { + if (transition === "removed-session") client.removeQueries({ queryKey: queryKeys.auth.session }); + else client.setQueryData(queryKeys.auth.session, transition === "signed-out" ? null : { user: { id: "user-1" } }); + if (transition === "round-trip") client.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); + client.setQueryData(queryKeys.auth.session, { user: { id: transition === "different-account" ? "user-2" : "user-1" } }); + }); + await act(async () => { store.setBrowserOnline(true); store.reportSuccess(); }); + await advance(2_000); + expect(save).not.toHaveBeenCalled(); + expect(store.getSnapshot().pendingWrites).toBe(0); + client.clear(); + }, + ); + it("bounds retries after an editor unmounts", async () => { const save = vi.fn().mockRejectedValue(outage()); render({ save }); diff --git a/ui/src/hooks/useDurableAutosave.ts b/ui/src/hooks/useDurableAutosave.ts index 658d68cd149..0293975cc8b 100644 --- a/ui/src/hooks/useDurableAutosave.ts +++ b/ui/src/hooks/useDurableAutosave.ts @@ -1,4 +1,6 @@ -import { useCallback, useEffect, useId, useRef, useState } from "react"; +import { useCallback, useContext, useEffect, useId, useRef, useState } from "react"; +import { QueryClientContext } from "@tanstack/react-query"; +import { bindAccount } from "@/lib/account-scope"; import { classifyError, describeError, type ErrorDescription } from "@/api/errors"; import { loadStructuredDraft, saveStructuredDraft } from "@/lib/composer-draft"; import { useConnectivityStore } from "@/lib/connectivity"; @@ -108,6 +110,7 @@ const SAVED_LINGER_MS = 1600; * - Errors become readable copy for "Couldn't save (reason)". */ export function useDurableAutosave(options: DurableAutosaveOptions): DurableAutosave { + const client = useContext(QueryClientContext); const owner = useDraftOwner(); const readOwner = useDraftOwnerReader(); const instanceId = useId(); @@ -120,7 +123,7 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab const targetKey = options.targetKey ?? options.sourceId; const targetRef = useRef(targetKey); targetRef.current = targetKey; - type Job = { token: symbol; registryKey: string; resends: number; saving?: boolean; id: number; target: string; owner: string | undefined; draft: T; + type Job = { token: symbol; registryKey: string; resends: number; saving?: boolean; id: number; target: string; owner: string | undefined; account: ReturnType | null; draft: T; save: (draft: T) => Promise; matchesServer?: DurableAutosaveOptions["matchesServer"]; onRetryFailed?: DurableAutosaveJob["onRetryFailed"]; storageKey: string | null; action: string }; const jobs = useRef(new Map()); @@ -142,6 +145,22 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab entry?.active.delete(job.token); if (entry && !entry.active.size) registry.delete(job.registryKey); }; + // Resolve first identity from the same generation guard used by mutations. + // This also runs after unmount, when React cannot migrate the job for us. + function owned(job: Job) { + if (job.account && !job.account.isCurrentScope()) return false; + const current = readOwner(); + if (job.owner === undefined && current !== undefined) { + const nextKey = JSON.stringify([current, job.target]); + if (registry.get(job.registryKey)?.latest !== job.token || registry.has(nextKey)) return false; + releaseJob(job); + job.owner = current; + job.registryKey = nextKey; + registry.set(nextKey, { latest: job.token, active: new Set([job.token]) }); + } + return current === job.owner && generations.current.get(job.target) === job.id + && registry.get(job.registryKey)?.latest === job.token; + } const mounted = useRef(true); const savingTimer = useRef | null>(null); const savedTimer = useRef | null>(null); @@ -160,17 +179,11 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab if (previous === undefined && owner !== undefined) { for (const job of jobs.current.values()) { if (job.owner !== undefined) continue; - const nextKey = JSON.stringify([owner, job.target]); - const superseded = registry.get(job.registryKey)?.latest !== job.token || registry.has(nextKey); - releaseJob(job); - if (superseded) { + if (!owned(job)) { + releaseJob(job); jobs.current.delete(job.target); if (generations.current.get(job.target) === job.id) generations.current.delete(job.target); - continue; } - job.owner = owner; - job.registryKey = nextKey; - registry.set(nextKey, { latest: job.token, active: new Set([job.token]) }); } refresh(value => value + 1); return; @@ -194,12 +207,12 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab return stored?.version === 1 && stored.draft !== undefined ? stored.draft : null; }, [storageKey]); const execute = useCallback(async (job: Job, retry = false): Promise => { - const owned = () => readOwner() === job.owner && generations.current.get(job.target) === job.id && registry.get(job.registryKey)?.latest === job.token; - const visible = () => mounted.current && owned() && targetRef.current === job.target; + const stillOwned = () => owned(job); + const visible = () => mounted.current && stillOwned() && targetRef.current === job.target; const publish = (status: DurableAutosaveStatus, error: ErrorDescription | null = null) => { if (visible()) setView({ target: job.target, status, error }); }; - if (!owned()) { + if (!stillOwned()) { if (jobs.current.get(job.target) === job) jobs.current.delete(job.target); releaseJob(job); if (mounted.current) refresh(value => value + 1); @@ -234,7 +247,12 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab remove(); return { kind: "saved" }; }; - try { await job.save(job.draft); return done(); } + try { + if (!mounted.current && job.account && !job.account.isCurrentAccount()) await job.account.whenAccountReady(); + if (!stillOwned()) { remove(); return { kind: "waiting" }; } + await job.save(job.draft); + return done(); + } catch (initialCause) { let cause = initialCause; if (classifyError(cause) === "conflict" && job.matchesServer) { @@ -255,7 +273,7 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab const current = optionsRef.current; const target = current.targetKey ?? current.sourceId; const admittedOwner = ownerRef.current ?? readOwner(); - const job: Job = { token: Symbol(), registryKey: JSON.stringify([admittedOwner, target]), resends: 0, id: ++sequence.current, target, owner: admittedOwner, draft, + const job: Job = { token: Symbol(), registryKey: JSON.stringify([admittedOwner, target]), resends: 0, id: ++sequence.current, target, owner: admittedOwner, account: client ? bindAccount(client) : null, draft, save: overrides.save ?? current.save, matchesServer: overrides.matchesServer ?? current.matchesServer, onRetryFailed: overrides.onRetryFailed, storageKey: current.storageKey ?? null, action: current.action ?? "save" }; const previous = jobs.current.get(target); @@ -266,7 +284,7 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab generations.current.set(target, job.id); jobs.current.set(target, job); refresh(value => value + 1); return execute(job); - }, [execute, readOwner]); + }, [client, execute, readOwner]); const waitingJobs = [...jobs.current.values()].filter(job => !job.saving).length; const auto = useAutoResend({ sourceId: options.sourceId, active: waitingJobs > 0, pendingCount: waitingJobs, resend: async (): Promise => { @@ -281,7 +299,6 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab detachedWork.current = () => { if (!jobs.current.size) return; if (auto.stalled) { for (const job of jobs.current.values()) releaseJob(job); jobs.current.clear(); return; } - const admittedOwner = ownerRef.current; const source = `${optionsRef.current.sourceId}:detached:${instanceId}`; let timer: ReturnType | null = null; let disposed = false, running = false; @@ -298,7 +315,11 @@ export function useDurableAutosave(options: DurableAutosaveOptions): Durab }; const wake = async () => { if (disposed || running) return; - if (mounted.current || readOwner() !== admittedOwner || admittedOwner === undefined) { release(); return; } + if (mounted.current) { release(); return; } + for (const job of jobs.current.values()) if (!owned(job)) { + releaseJob(job); + jobs.current.delete(job.target); + } if (!jobs.current.size) { release(); return; } store.setPendingWrites(source, jobs.current.size); if (store.getSnapshot().status !== "online") return; diff --git a/ui/src/lib/account-scope.ts b/ui/src/lib/account-scope.ts new file mode 100644 index 00000000000..0e7d050e502 --- /dev/null +++ b/ui/src/lib/account-scope.ts @@ -0,0 +1,59 @@ +import type { QueryClient } from "@tanstack/react-query"; +import { queryKeys } from "./queryKeys"; +import { authApi } from "@/api/auth"; +import { classifySendFailure } from "./pending-send"; + +type AccountGeneration = { owner: string | undefined; generation: number }; +const accounts = new WeakMap(); + +function currentOwner(client: QueryClient): string | undefined { + const session = client.getQueryData<{ user?: { id?: string }; session?: { userId?: string } } | null>(queryKeys.auth.session); + return session === undefined ? undefined : session?.user?.id ?? session?.session?.userId ?? "signed-out"; +} + +export function bindAccount(client: QueryClient) { + let account = accounts.get(client); + if (!account) { + account = { owner: currentOwner(client), generation: 0 }; + accounts.set(client, account); + const tracked = account; + client.getQueryCache().subscribe(event => { + const owner = currentOwner(client); + const removed = event.type === "removed" && JSON.stringify(event.query.queryKey) === JSON.stringify(queryKeys.auth.session); + if (owner !== tracked.owner || removed) { + if (removed || tracked.owner !== undefined || tracked.generation !== 0) tracked.generation += 1; + tracked.owner = owner; + } + }); + } + const { owner, generation } = account; + const isCurrentScope = () => account.generation === generation && (owner === undefined ? generation === 0 : account.owner === owner); + const isCurrentAccount = () => account.owner !== undefined && isCurrentScope(); + const whenAccountReady = async () => { + if (account.owner !== undefined || !isCurrentScope()) return; + let stopped = false; + let unsubscribe = () => {}; + const ready = new Promise(resolve => { + unsubscribe = client.getQueryCache().subscribe(() => { + if (account.owner !== undefined || !isCurrentScope()) resolve(); + }); + }); + const lookup = async () => { + for (let attempt = 0; attempt < 3; attempt += 1) { + if (stopped || account.owner !== undefined || !isCurrentScope()) return; + try { + await client.fetchQuery({ queryKey: queryKeys.auth.session, queryFn: () => authApi.getSession(), retry: false }); + return; + } catch (error) { + if (stopped || account.owner !== undefined || !isCurrentScope()) return; + if (attempt === 2 || classifySendFailure(error) !== "pending") throw error; + await new Promise(resolve => setTimeout(resolve, 250 * 2 ** attempt)); + } + } + }; + try { await Promise.race([ready, lookup()]); } + finally { stopped = true; unsubscribe(); } + }; + return { isCurrentAccount, isCurrentScope, whenAccountReady }; +} +