From 91fdf02e956f80c29595c34c196735cf5ccc6dc8 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 31 Oct 2026 12:56:06 -0500 Subject: [PATCH 1/3] perf(tasks): defer completed transcripts on compact opens (#18461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task conversations must show the saved instruction and recent replies quickly. > - Compact task reads already limit the saved message window. > - The conversation still waits for completed run transcripts even when a saved reply contains the result. > - Removing that wait alone can insert activity after the reader starts reading. > - This pull request defers that output until the reader opens history and keeps required output in the initial reveal. > - The benefit is a useful conversation sooner, with a stable first layout and a working composer. ## Linked Issues or Issue Description Refs #17593, #18092, #18312, #18358, #18422. **What happened?** Opening a task with a long saved investigation can wait for native events or legacy logs before showing its existing replies. A slow transcript request can hold the conversation behind the loading screen until the 15-second fallback. **Expected behavior** Show the bounded saved conversation without reading completed output that already has a usable saved reply. Keep that output available through the existing history controls. **Steps to reproduce** 1. Create a task with many completed runs and long saved agent replies. 2. Delay its run events and logs. 3. Open the compact task directly. Before this change, the transcript read blocks the saved replies and composer. ## What Changed - Defer completed successful run transcripts on compact task opens when a usable saved reply is present. Load that output through the existing history controls. - Keep required output on the initial path: live and failed runs, yielded runs, pending interactions, placeholder/progress-only replies, explicit anchors, chats, and full history. Retain output already selected during this visit when its run completes. - Keep saved plans visible when their owning run output is deferred or still loading. - Keep the same composer and draft through history expansion. Offer **More run output** when a single saved run has deferred output. - Position same-task comment links after their target arrives. Wait only for a missing target, with a 15-second limit. Unrelated stalled metadata cannot block an available target after the reveal deadline. - Preserve the reading anchor when a queued scroll event arrives during a properties-pane width transition. - Add a 200-reply/200-run PostgreSQL browser fixture with a saved plan, covering both task views on desktop and mobile. Correct stale browser setup for current routes and the shared mobile scroller. - Remove a random company-prefix collision in the isolated privacy test fixture that failed during CI. ## Verification - Final revision `fba3f2fcbb`: 386 focused thread/scroller unit cases and all 32 production-browser journeys passed. The browser cases cover both task views on desktop/mobile, saved plans, delayed transcripts, history, comment links, Back, drafts, retry, avatar continuity, media and pane resizing. The 25 privacy fixture cases also passed against PostgreSQL. - Workspace typecheck and full build passed with Node 24. A broader local UI run passed 10,220 cases across 849 files before the final navigation/plan review fixes; the changed components were then rechecked in the 386-case focused run. - Five alternating pairs compared master `12b0cc6304` with this revision, using 200 long saved replies and 200 completed native runs. Both production UI builds used the same loopback API/database, Chromium 151.0.7922.34, 1440 × 900, fresh browser contexts, service workers blocked, no CPU throttle, and a controlled 3-second events/log delay. Builds and other suites were stopped during measurement. No page errors occurred. | Useful conversation + editable composer | Baseline median (range) | Fixed median (range) | | --- | --- | --- | | Cold direct open | 3.570s (3.538–3.616s) | 1.056s (1.022–1.141s) | | Task-list navigation | 3.373s (3.342–3.394s) | 0.899s (0.885–0.915s) | | Back | 22ms (22–25ms) | 25ms (25–28ms) | - Median paired savings: 2.519s cold, 2.470s from the list. Completed-output requests: two per baseline sample, zero per fixed sample. Recent-comments API control medians were 13.6ms for both, with ranges 12.1–18.7ms and 11.6–50.8ms. No Back improvement is claimed; an earlier revision's run also included one 1-second Back outlier, retained in the local evidence. - The full local default test command did **not** pass: it stopped on the existing chat test line-selection error. After restoring embedded PostgreSQL library symlinks omitted by an install with scripts disabled, Node 24 passed 672 cases in the first two chat shards, then failed to select the third. This is separate from the passing focused suites and remote CI. - Earlier CI failures were addressed: the privacy prefix collision is fixed; the delegated task-completion reply case passed locally after merging master's reply-recovery fix (#18465). Browser fixture corrections retain the original behavior assertions. Red tests reproduced the loading, navigation, resize and saved-plan regressions before their fixes. - Map, module-boundary, token-gate and added-line token checks passed. The full-tree token scan still finds pre-existing active-username fixtures; this PR adds zero forbidden token lines. - Greptile: **5/5**, final revision, all four review threads resolved. The final-revision PR CI workflow passed, including build, typecheck, all server/chat/workspace test shards, all eight E2E shards, and every Runner lane. GitHub reports 55 successful checks and three skipped checks. The separate **Paperclip Review** check remains queued with **Waiting for the assigned agent**; it has not produced an assessment and has not been bypassed. The branch has no merge conflicts. ## Risks - The default compact task omits completed transcript details when the saved result is sufficient. The reader must open history to inspect them. - Required structural data remains inside the initial reveal boundary. Slow server responses for that data remain visible latency. - Link positioning falls back after 15 seconds when its target never arrives. The conversation remains readable. - The measurements use a controlled local delay. They do not establish a deployed staging speedup. - No API, schema, authorization, or migration changes. ## Model Used OpenAI Codex, GPT-6, with tool use and code 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 - [ ] I have run tests locally and they pass (focused suites pass; the full default wrapper has the existing selection error documented above) - [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 - [ ] All Paperclip CI gates are green (code CI passed; the separate assigned-agent Paperclip Review remains queued) - [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 --- maps/feature-map/tasks.md | 4 +- .../privacy-production-review.test.ts | 3 +- tests/perf/issue-detail/README.md | 30 +++- .../issue-detail/layout-stability.spec.ts | 76 ++++++++--- tests/perf/issue-detail/plan-startup.spec.ts | 32 +++-- .../issue-detail/saved-reply-startup.spec.ts | 126 +++++++++++++++++ ui/src/components/TaskChatThread.test.tsx | 129 ++++++++++++++++++ ui/src/components/TaskChatThread.tsx | 69 ++++++++-- .../task-chat/TaskMessageScroller.test.tsx | 57 ++++++++ .../task-chat/TaskMessageScroller.tsx | 30 +++- .../task-chat/scroll-navigation.tsx | 4 +- 11 files changed, 504 insertions(+), 56 deletions(-) create mode 100644 tests/perf/issue-detail/saved-reply-startup.spec.ts diff --git a/maps/feature-map/tasks.md b/maps/feature-map/tasks.md index b9c16a8ed13..28c794fd07c 100644 --- a/maps/feature-map/tasks.md +++ b/maps/feature-map/tasks.md @@ -59,7 +59,7 @@ Filters → **Routine tasks** selects tasks created by routine runs. The ### `task-detail` -Open `/issues/:issueId` from a row, mention, or direct link. Cold opens show the instruction, recent exchange and saved cards together. No URL fragment expands history; older-comment anchors fetch only their target, and document anchors open the pane. **More messages** appears only when earlier comments or runs are omitted and loads that history. Fresh tasks with no omitted history have no disclosure. A single legacy run with a truncated log offers **More run output** instead. **More task actions → Show task activity** loads full activity even for a single native run; incoming messages remain visible while the task stays open. +Open `/issues/:issueId` from a row, mention, or direct link. Cold opens show the instruction, recent exchange and saved cards together. Completed runs with saved replies defer their transcripts until the reader opens history. Live runs, failures, yielded runs and runs without a saved reply still hydrate; an already shown transcript stays available after completion. No URL fragment expands history; older-comment anchors fetch only their target, and document anchors open the pane. **More messages** appears only when earlier comments or runs are omitted and loads that history. Fresh tasks with no omitted history have no disclosure. A single run with deferred output or a truncated legacy log offers **More run output** instead. **More task actions → Show task activity** loads full activity even for a single native run; incoming messages remain visible while the task stays open. ### `task-sharing` @@ -113,6 +113,8 @@ Manual: Open a long task with slow historical requests. Read the instruction and Automated: [cold-open reads](../../server/src/__tests__/task-cold-open.test.ts) cover bounded reads, event filtering and company denial. [Task detail](../../ui/src/pages/IssueDetail.test.tsx) covers component behavior, while [issue service](../../server/src/__tests__/issues-service.test.ts) covers server contracts. +The opt-in [saved-reply browser case](../../tests/perf/issue-detail/saved-reply-startup.spec.ts) uses 200 completed native runs and long saved replies in a disposable PostgreSQL instance. It holds transcript reads, checks desktop/mobile first-reveal geometry, then opens history while retaining the editor and draft. See its [setup](../../tests/perf/issue-detail/README.md). + Failure markers have no dashed divider rules. An adjacent run error and no-live-execution-path notice show only the last error, with its details and retry action, including a permitted legacy run retry carried onto a later diff --git a/server/src/__tests__/privacy-production-review.test.ts b/server/src/__tests__/privacy-production-review.test.ts index 1e0b37396ca..dd01aebef67 100644 --- a/server/src/__tests__/privacy-production-review.test.ts +++ b/server/src/__tests__/privacy-production-review.test.ts @@ -16,6 +16,7 @@ import { canActorReadWorkspaceOperation } from "../services/heartbeat-run-privac describe("private task production review", () => { let tempDb: Awaited>; let db: ReturnType; + let fixtureCompanyNumber = 0; beforeAll(async () => { process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce"; tempDb = await startEmbeddedPostgresTestDatabase("paperclip-private-review-"); @@ -26,7 +27,7 @@ describe("private task production review", () => { afterAll(async () => { await tempDb?.cleanup(); }); async function fixture() { - const [company] = await db.insert(companies).values({ name: randomUUID(), issuePrefix: `T${randomUUID().slice(0, 5)}` }).returning(); + const [company] = await db.insert(companies).values({ name: randomUUID(), issuePrefix: `T${++fixtureCompanyNumber}` }).returning(); const owner = randomUUID(), outsider = randomUUID(); await db.insert(authUsers).values([owner, outsider].map(id => ({ id, name: id, email: `${id}@example.test`, createdAt: new Date(), updatedAt: new Date() }))); await db.insert(companyMemberships).values([owner, outsider].map(principalId => ({ companyId: company.id, principalType: "user", principalId, status: "active", membershipRole: "operator" }))); diff --git a/tests/perf/issue-detail/README.md b/tests/perf/issue-detail/README.md index f344845ad8a..ce6a5a25199 100644 --- a/tests/perf/issue-detail/README.md +++ b/tests/perf/issue-detail/README.md @@ -75,17 +75,23 @@ Back navigation, and covers mobile reduced motion and explicit comment links. The eight scenarios also exercise same-task hash navigation without remounting on desktop and mobile, and stalled native/log reads that expose Retry after the 15-second request deadline. -`plan-startup.spec.ts` also opens tasks with and without a saved plan while -blocking the separate plan endpoint. Both must reveal the saved conversation -from the task response without issuing that redundant read. -It also checks task-list navigation, where a list-only snapshot needs one normal -plan fetch and must still show a saved plan when present. +`plan-startup.spec.ts` opens tasks directly and from the task list, with and +without a saved plan, while holding separate plan reads. The core projection +must reveal the saved conversation and plan before those reads finish. Separate +plan reads are recorded as diagnostics. The hook tests cover fallback reads for +list-only snapshots and responses without a plan projection. Videos and per-frame `layout.json` measurements are written under each test's output directory. The two-pixel assertion measures a logical row's viewport offset; total scroll offset legitimately changes when history is prepended. Ordinary task opens defer the concealed history and composer until their initial data is ready. The skeleton and scroll/composer containers retain their geometry. +Compact tasks reuse saved replies from completed runs. Their transcript reads +start when the reader opens older history through **More messages** or **More run +output**. Saved Plans from completed runs stay visible while their output is deferred or pending. +Live runs, failures, yielded runs, explicit anchors, and runs without a +saved reply still load their output. A transcript already shown during this visit +stays mounted when the run completes. Comment-link navigation prepares content eagerly so the target can be positioned before reveal. Revealed content stays mounted during refresh. The draft scenario holds a background task read, checks the same editor remains mounted, reloads the @@ -105,6 +111,20 @@ isolated instance as before. Live provider walkthroughs additionally require a configured native Paperclip runner and Codex authentication; deterministic browser fixtures do not substitute for watching an actual provider run. +`saved-reply-startup.spec.ts` adds a PostgreSQL fixture with 200 long saved agent +replies, 200 completed native runs, and a Plan linked to the latest run. It holds +transcript requests, checks both task presentations on desktop and mobile, and measures +reply positions on each frame. Opening older history must start the transcript +reads and preserve the editor and draft. This case is skipped unless +`PAPERCLIP_ISSUE_PERF_DATABASE_URL` points to a disposable loopback PostgreSQL +database whose name ends in `_test`. Use the same database as the local server: + +```sh +PAPERCLIP_ISSUE_PERF_BASE_URL=http://127.0.0.1:3102 \ +PAPERCLIP_ISSUE_PERF_DATABASE_URL=postgres://paperclip:paperclip@127.0.0.1:5432/task_loading_test \ + pnpm exec playwright test --config tests/perf/issue-detail/playwright.config.ts saved-reply-startup.spec.ts +``` + For the live walkthrough, first assign a disposable task to a native Paperclip runner configured with the Codex provider. Its scratch project should contain a small `sum.mjs` fixture. The script sends a paced job, scrolls up, disconnects diff --git a/tests/perf/issue-detail/layout-stability.spec.ts b/tests/perf/issue-detail/layout-stability.spec.ts index d377a0415c2..1f3521d74b5 100644 --- a/tests/perf/issue-detail/layout-stability.spec.ts +++ b/tests/perf/issue-detail/layout-stability.spec.ts @@ -99,6 +99,7 @@ test("reading anchor survives late media, composer resizing, older history and b await page.goto(`/${prefix}/issues/${issue.identifier}`); await ready(page); const scroller = page.getByTestId("task-chat-scroller"); + await page.getByRole("button", { name: "More messages", exact: true }).click(); await expect(page.getByText("Historical message 10", { exact: true })).toBeAttached(); await scroller.evaluate((el) => { el.scrollTop = el.scrollHeight / 2; }); await page.waitForTimeout(100); @@ -153,6 +154,45 @@ test("explicit comment links resolve older pages before revealing at the target" }).toBeLessThanOrEqual(2); }); +test("a loaded comment target positions after the reveal deadline when live-run metadata stalls", async ({ page }) => { + let release!: () => void; + let metadataReads = 0; + const held = new Promise(resolve => { release = resolve; }); + await page.route("**/api/issues/*/live-runs", async route => { metadataReads++; await held; await route.continue().catch(() => {}); }); + try { + await page.goto(`/${prefix}/issues/${issue.identifier}#comment-${oldestCommentId}`); + await expect(page.getByTestId("task-chat-thread").locator(':scope > [aria-busy="false"]')).toBeVisible({ timeout: 20_000 }); + expect(metadataReads).toBeGreaterThan(0); + await expect(page.getByText("Some task history is still loading.", { exact: true })).toBeVisible(); + const target = page.locator(`[id="comment-${oldestCommentId}"]`); + await expect(target).toBeVisible(); + await expect.poll(() => target.evaluate(el => Math.abs(el.getBoundingClientRect().top - document.querySelector('[data-testid="task-chat-scroller"]')!.getBoundingClientRect().top))) + .toBeLessThanOrEqual(2); + await expect(page.getByTestId("task-chat-composer-stack").locator('[contenteditable="true"]')).toBeVisible(); + } finally { release(); } +}); + +test("a late linked comment positions after the reveal deadline without losing the draft", async ({ page }) => { + let release!: () => void; + const held = new Promise(resolve => { release = resolve; }); + await page.route(`**/api/issues/*/comments/${oldestCommentId}`, async route => { await held; await route.continue().catch(() => {}); }); + try { + await page.goto(`/${prefix}/issues/${issue.identifier}#comment-${oldestCommentId}`); + await expect(page.getByTestId("task-chat-thread").locator(':scope > [aria-busy="false"]')).toBeVisible({ timeout: 20_000 }); + const target = page.locator(`[id="comment-${oldestCommentId}"]`); + await expect(target).toHaveCount(0); + const editor = page.getByTestId("task-chat-composer-stack").locator('[contenteditable="true"]'); + await editor.fill("Keep this draft while the linked comment loads."); + const originalEditor = await editor.elementHandle(); + release(); + await expect(target).toBeVisible(); + await expect.poll(() => target.evaluate(el => Math.abs(el.getBoundingClientRect().top - document.querySelector('[data-testid="task-chat-scroller"]')!.getBoundingClientRect().top))) + .toBeLessThanOrEqual(2); + await expect(editor).toHaveText("Keep this draft while the linked comment loads."); + expect(await originalEditor!.evaluate(element => element.isConnected)).toBe(true); + } finally { release(); } +}); + test("a draft survives background task refresh, reload, and one persisted reply", async ({ page, request }) => { await page.goto(`/${prefix}/issues/${draftIssue.identifier}`); await ready(page); @@ -200,10 +240,10 @@ test("a draft survives background task refresh, reload, and one persisted reply" }); test("failed initial comments expose retry, then recover without an indefinite skeleton", async ({ page }) => { - await page.route("**/api/issues/*/comments?*", (route) => route.fulfill({ status: 503, contentType: "application/json", body: '{"error":"Temporarily unavailable"}' })); + await page.route("**/api/issues/*/recent-comments", (route) => route.fulfill({ status: 500, contentType: "application/json", body: '{"error":"Comment read failed"}' })); await page.goto(`/${prefix}/issues/${issue.identifier}`); await expect(page.getByRole("button", { name: "Retry", exact: true })).toBeVisible({ timeout: 30_000 }); - await page.unroute("**/api/issues/*/comments?*"); + await page.unroute("**/api/issues/*/recent-comments"); await page.getByRole("button", { name: "Retry", exact: true }).click(); await ready(page); await expect(page.getByText("Historical message 159", { exact: true })).toBeAttached(); @@ -215,13 +255,15 @@ test("mobile reduced-motion cold open and rapid task switching", async ({ page } await page.emulateMedia({ reducedMotion: "reduce" }); await page.goto(`/${prefix}/issues/${issue.identifier}`); await ready(page); - await expect.poll(() => page.evaluate(() => document.documentElement.scrollHeight - window.scrollY - window.innerHeight)).toBeLessThan(2); - await page.evaluate(() => window.scrollTo(0, 1200)); + const scroller = page.getByTestId("task-chat-scroller"); + const distanceToLatest = () => scroller.evaluate(el => el.scrollHeight - el.scrollTop - el.clientHeight); + await expect.poll(distanceToLatest).toBeLessThan(2); + await scroller.evaluate(el => { el.scrollTop = 1200; }); await page.waitForTimeout(100); - const saved = await page.evaluate(() => window.scrollY); + const saved = await scroller.evaluate(el => el.scrollTop); await page.reload(); await ready(page); - await expect.poll(() => page.evaluate(() => window.scrollY)).toBeGreaterThan(1200); + await expect.poll(distanceToLatest).toBeLessThan(2); // A fresh navigation must show the new task, never retain the previous feed. await page.goto(`/${prefix}/issues/${other.identifier}`); await ready(page); @@ -234,34 +276,34 @@ for (const mobile of [false, true]) { if (mobile) await page.setViewportSize({ width: 390, height: 844 }); await page.goto(`/${prefix}/issues/${issue.identifier}`); await ready(page); - await page.getByText("Historical message 10", { exact: true }).waitFor({ state: "attached" }); + await page.getByText("Historical message 159", { exact: true }).waitFor({ state: "attached" }); await expect(page.locator(`[id="comment-${oldestCommentId}"]`)).toHaveCount(0); const originalThread = await page.getByTestId("task-chat-thread").elementHandle(); - const before = await page.evaluate((mobile) => { - const root = document.querySelector(mobile ? '[data-testid="task-chat-thread"]' : '[data-testid="task-chat-scroller"]')!; - const top = mobile ? 0 : root.getBoundingClientRect().top; + const before = await page.evaluate(() => { + const root = document.querySelector('[data-testid="task-chat-scroller"]')!; + const top = root.getBoundingClientRect().top; const row = [...root.querySelectorAll("[data-thread-anchor]")].find((el) => el.getBoundingClientRect().bottom > top)!; return { id: row.dataset.threadAnchor!, offset: row.getBoundingClientRect().top - top }; - }, mobile); + }); await page.getByRole("link", { name: "Read the oldest message", exact: true }).click(); const target = page.locator(`[id="comment-${oldestCommentId}"]`); await expect(target).toBeAttached(); - const targetOffset = () => target.evaluate((el, mobile) => el.getBoundingClientRect().top - (mobile ? 0 : document.querySelector('[data-testid="task-chat-scroller"]')!.getBoundingClientRect().top), mobile); + const targetOffset = () => target.evaluate(el => el.getBoundingClientRect().top - document.querySelector('[data-testid="task-chat-scroller"]')!.getBoundingClientRect().top); await expect.poll(async () => Math.abs(await targetOffset())).toBeLessThanOrEqual(2); expect(await originalThread!.evaluate((el) => el === document.querySelector('[data-testid="task-chat-thread"]'))).toBe(true); await page.goBack(); - await expect.poll(async () => Math.abs(await page.evaluate(({ mobile, before }) => { - const root = document.querySelector(mobile ? '[data-testid="task-chat-thread"]' : '[data-testid="task-chat-scroller"]')!; + await expect.poll(async () => Math.abs(await page.evaluate(before => { + const root = document.querySelector('[data-testid="task-chat-scroller"]')!; const row = [...root.querySelectorAll("[data-thread-anchor]")].find((el) => el.dataset.threadAnchor === before.id)!; - return row.getBoundingClientRect().top - (mobile ? 0 : root.getBoundingClientRect().top) - before.offset; - }, { mobile, before }))).toBeLessThanOrEqual(2); + return row.getBoundingClientRect().top - root.getBoundingClientRect().top - before.offset; + }, before))).toBeLessThanOrEqual(2); }); } test("stalled native and log history reveal loaded content with Retry after the request deadline", async ({ page }) => { const runId = "20000000-0000-4000-8000-000000000001"; const at = new Date().toISOString(); - await page.route("**/api/issues/*/runs", (route) => route.fulfill({ json: [{ runId, runtimeMode: "native", status: "succeeded", agentId: "20000000-0000-4000-8000-000000000002", adapterType: "paperclip_runner", createdAt: at, startedAt: at, finishedAt: at }] })); + await page.route(/\/api\/issues\/[^/]+\/runs(?:\?|$)/, (route) => route.fulfill({ json: [{ runId, runtimeMode: "native", status: "succeeded", agentId: "20000000-0000-4000-8000-000000000002", adapterType: "paperclip_runner", createdAt: at, startedAt: at, finishedAt: at }] })); let stalled = true; let release!: () => void; const held = new Promise((resolve) => { release = resolve; }); diff --git a/tests/perf/issue-detail/plan-startup.spec.ts b/tests/perf/issue-detail/plan-startup.spec.ts index faa514308b1..e2b779f0a5b 100644 --- a/tests/perf/issue-detail/plan-startup.spec.ts +++ b/tests/perf/issue-detail/plan-startup.spec.ts @@ -5,7 +5,7 @@ test.setTimeout(60_000); for (const hasPlan of [false, true]) { for (const entry of ["direct", "list"]) { - test(`task reuses a full plan projection and fetches one for list-only snapshots (plan=${hasPlan}, entry=${entry})`, async ({ page, request }) => { + test(`task reveals its core plan projection while separate plan reads are stalled (plan=${hasPlan}, entry=${entry})`, async ({ page, request }, info) => { const companyResponse = await request.post("/api/companies", { data: { name: `Plan startup ${Date.now()}` } }); expect(companyResponse.ok()).toBe(true); const company = await companyResponse.json(); @@ -19,22 +19,26 @@ for (const hasPlan of [false, true]) { expect(planResponse.ok()).toBe(true); } let planRequests = 0; + let release!: () => void; + const held = new Promise(resolve => { release = resolve; }); await page.route("**/api/issues/*/documents/plan", async (route) => { planRequests += 1; - if (entry === "direct") await route.abort(); - else await route.continue(); + await held; + await route.continue().catch(() => {}); }); - if (entry === "list") { - await page.goto(`/${company.issuePrefix}/issues`); - await page.locator("[data-inbox-issue-link]", { hasText: "Read a saved conversation" }).first().click(); - } else { - await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`); - } - const thread = page.getByTestId("task-chat-thread"); - await expect(thread.locator(':scope > [aria-busy="false"]')).toBeVisible({ timeout: 10_000 }); - await expect(thread.getByText("This saved reply must be readable without a redundant plan request.", { exact: true })).toBeVisible(); - if (hasPlan) await expect(thread.getByTestId("task-chat-plan-preview")).toBeVisible(); - expect(planRequests).toBe(entry === "direct" ? 0 : 1); + try { + if (entry === "list") { + await page.goto(`/${company.issuePrefix}/issues`); + await page.locator("[data-inbox-issue-link]", { hasText: "Read a saved conversation" }).first().click(); + } else { + await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`); + } + const thread = page.getByTestId("task-chat-thread"); + await expect(thread.locator(':scope > [aria-busy="false"]')).toBeVisible({ timeout: 10_000 }); + await expect(thread.getByText("This saved reply must be readable without a redundant plan request.", { exact: true })).toBeVisible(); + if (hasPlan) await expect(thread.getByTestId("task-chat-plan-preview")).toBeVisible(); + await info.attach("plan-reads.json", { body: JSON.stringify({ planRequests }), contentType: "application/json" }); + } finally { release(); } }); } } diff --git a/tests/perf/issue-detail/saved-reply-startup.spec.ts b/tests/perf/issue-detail/saved-reply-startup.spec.ts new file mode 100644 index 00000000000..48cdee108b0 --- /dev/null +++ b/tests/perf/issue-detail/saved-reply-startup.spec.ts @@ -0,0 +1,126 @@ +import fs from "node:fs/promises"; +import { randomUUID } from "node:crypto"; +import { expect, test } from "@playwright/test"; + +test.use({ serviceWorkers: "block" }); +test.setTimeout(60_000); +test.skip(!process.env.PAPERCLIP_ISSUE_PERF_DATABASE_URL, "The native saved-reply fixture requires a disposable PostgreSQL database"); + +let fixture: { prefix: string; identifier: string; runId: string; lastCommentId: string }; + +test.beforeAll(async ({ request }) => { + const databaseUrl = process.env.PAPERCLIP_ISSUE_PERF_DATABASE_URL; + if (!databaseUrl) throw new Error("Set PAPERCLIP_ISSUE_PERF_DATABASE_URL for a disposable local database"); + const target = new URL(databaseUrl); + if (!["localhost", "127.0.0.1", "[::1]"].includes(target.hostname) || !target.pathname.endsWith("_test")) { + throw new Error("The saved-reply fixture requires a disposable loopback database ending in _test"); + } + const { createDb, closeRegisteredClients, heartbeatRuns, heartbeatRunEvents, issueComments, activityLog } = await import("../../../packages/db/src/index"); + const companyResponse = await request.post("/api/companies", { data: { name: `Saved reply startup ${Date.now()}` } }); + expect(companyResponse.ok(), await companyResponse.text()).toBeTruthy(); + const company = await companyResponse.json(); + const agentResponse = await request.post(`/api/companies/${company.id}/agents`, { data: { + name: "Saved investigation author", role: "engineer", status: "paused", adapterType: "paperclip_runner", adapterConfig: { provider: "codex" }, + } }); + expect(agentResponse.ok(), await agentResponse.text()).toBeTruthy(); + const agent = await agentResponse.json(); + const issueResponse = await request.post(`/api/companies/${company.id}/issues`, { data: { + title: "Read the saved investigation", status: "backlog", description: "Review the latest findings and load older investigation history when needed.", + } }); + expect(issueResponse.ok(), await issueResponse.text()).toBeTruthy(); + const issue = await issueResponse.json(); + const planResponse = await request.put(`/api/issues/${issue.id}/documents/plan`, { data: { + title: "Saved investigation plan", format: "markdown", body: "# Saved investigation plan\n\nKeep the saved evidence available before loading its completed run output.", + } }); + expect(planResponse.ok(), await planResponse.text()).toBeTruthy(); + const plan = await planResponse.json(); + const db = createDb(databaseUrl); + const comments: (typeof issueComments.$inferInsert)[] = []; + const runs: (typeof heartbeatRuns.$inferInsert)[] = []; + const activity: (typeof activityLog.$inferInsert)[] = []; + const base = Date.now() - 30 * 24 * 3600_000; + for (let index = 0; index < 200; index++) { + const runId = randomUUID(), id = randomUUID(), createdAt = index === 199 + ? new Date(new Date(plan.updatedAt).getTime() + 50) + : new Date(base + index * 60_000); + const startedAt = index === 199 ? new Date(new Date(plan.updatedAt).getTime() - 1000) : new Date(+createdAt - 900); + runs.push({ id: runId, companyId: company.id, agentId: agent.id, issueId: issue.id, runtimeMode: "native", status: "succeeded", + createdAt: new Date(+startedAt - 100), startedAt, finishedAt: createdAt, contextSnapshot: { issueId: issue.id } }); + comments.push({ id, companyId: company.id, issueId: issue.id, authorType: "agent", authorAgentId: agent.id, + createdByRunId: runId, body: `## Saved investigation ${index}\n\n${"Verified the saved evidence and checked the result. ".repeat(60)}`, createdAt, updatedAt: createdAt }); + activity.push({ companyId: company.id, actorType: "agent", actorId: agent.id, agentId: agent.id, runId, + entityType: "issue", entityId: issue.id, action: "issue.comment_added", details: { commentId: id }, createdAt }); + } + const last = comments.at(-1)!, runId = runs.at(-1)!.id!; + try { + for (let offset = 0; offset < runs.length; offset += 100) await db.insert(heartbeatRuns).values(runs.slice(offset, offset + 100)); + for (let offset = 0; offset < comments.length; offset += 100) await db.insert(issueComments).values(comments.slice(offset, offset + 100)); + for (let offset = 0; offset < activity.length; offset += 100) await db.insert(activityLog).values(activity.slice(offset, offset + 100)); + await db.insert(heartbeatRunEvents).values({ companyId: company.id, agentId: agent.id, runId, seq: 1, eventType: "item.completed", createdAt: last.createdAt, + payload: { prpEvent: { schema: "paperclip.prp.event.v1", schemaVersion: 1, eventType: "item.completed", runId, + emittedAt: last.createdAt!.toISOString(), itemId: `answer-${runId}`, payload: { kind: "agentMessage", text: last.body } } } }); + } finally { await closeRegisteredClients(databaseUrl); } + fixture = { prefix: company.issuePrefix, identifier: issue.identifier, runId, lastCommentId: last.id! }; +}); + +for (const coordinator of [false, true]) for (const mobile of [false, true]) test(`saved task replies reveal before historical output and keep their geometry (Coordinator=${coordinator}, mobile=${mobile})`, async ({ page }, info) => { + await page.setViewportSize(mobile ? { width: 390, height: 844 } : { width: 1440, height: 900 }); + await page.route("**/api/workspace-preferences/me", async route => { + const response = await route.fetch(); + await route.fulfill({ json: { ...await response.json(), coordinatorWorkspaceEnabled: coordinator } }); + }); + await page.addInitScript(() => { + const frames: { height: number; latest: number }[] = []; + Object.assign(window, { savedReplyFrames: frames }); + const sample = () => { + const thread = document.querySelector('[data-testid="task-chat-thread"] > [aria-busy="false"]'); + const scroller = thread?.querySelector('[data-testid="task-chat-scroller"]'); + if (scroller) frames.push({ height: scroller.scrollHeight, latest: [...scroller.querySelectorAll('[id^="comment-"]')].at(-1)?.getBoundingClientRect().top ?? 0 }); + requestAnimationFrame(sample); + }; + requestAnimationFrame(sample); + }); + const reads: string[] = []; + let release!: () => void; + const held = new Promise(resolve => { release = resolve; }); + await page.route(`**/api/heartbeat-runs/*/{events,log}?*`, async route => { + reads.push(route.request().url()); + await held; + await route.continue(); + }); + try { + const started = performance.now(); + await page.goto(`/${fixture.prefix}/issues/${fixture.identifier}`); + await expect(page.getByTestId("task-chat-thread").locator(':scope > [aria-busy="false"]')).toBeVisible({ timeout: 8_000 }); + const readyMs = performance.now() - started; + const reply = page.locator(`[id="comment-${fixture.lastCommentId}"]`); + await expect(reply).toContainText("Saved investigation 199"); + await expect(page.getByTestId("task-chat-plan-preview")).toHaveCount(1); + await expect(page.getByTestId("task-chat-plan-preview")).toContainText("Saved investigation plan"); + expect(reads).toEqual([]); + await page.waitForTimeout(600); + const frames = await page.evaluate(() => (window as unknown as { savedReplyFrames: { height: number; latest: number }[] }).savedReplyFrames); + expect(frames.length).toBeGreaterThan(5); + for (const dimension of ["height", "latest"] as const) { + const values = frames.map(frame => frame[dimension]); + expect(Math.max(...values) - Math.min(...values)).toBeLessThanOrEqual(2); + } + const editor = page.getByTestId("task-chat-composer-stack").locator('[contenteditable="true"]'); + await editor.fill("Keep this draft while historical output is unavailable."); + const originalEditor = await editor.elementHandle(); + const before = await reply.boundingBox(); + const height = () => page.getByTestId("task-chat-scroller").evaluate(element => element.scrollHeight); + const beforeHeight = await height(); + await page.waitForTimeout(600); + expect(await height()).toBe(beforeHeight); + expect(Math.abs((await reply.boundingBox())!.y - before!.y)).toBeLessThanOrEqual(2); + await page.getByRole("button", { name: "More messages", exact: true }).evaluate(element => (element as HTMLButtonElement).click()); + await expect.poll(() => reads.some(url => url.includes(fixture.runId))).toBe(true); + await expect(editor).toHaveText("Keep this draft while historical output is unavailable."); + expect(await originalEditor!.evaluate(element => element.isConnected)).toBe(true); + release(); + await expect(page.getByRole("heading", { name: "Saved investigation 195", exact: true })).toBeAttached(); + await expect(editor).toHaveText("Keep this draft while historical output is unavailable."); + await fs.writeFile(info.outputPath("readiness.json"), JSON.stringify({ readyMs, reads, frames }, null, 2)); + } finally { release(); } +}); diff --git a/ui/src/components/TaskChatThread.test.tsx b/ui/src/components/TaskChatThread.test.tsx index c834c8df0a1..e298551b3cf 100644 --- a/ui/src/components/TaskChatThread.test.tsx +++ b/ui/src/components/TaskChatThread.test.tsx @@ -21,6 +21,7 @@ import { heartbeatsApi } from "@/api/heartbeats"; import { nativeRunEventsToTranscript } from "./transcript/native-run-events"; import { pendingConnectionIntentInteraction } from "@/fixtures/issueThreadInteractionFixtures"; import { TaskChatScrollNavigation, useTaskChatScrollNavigation } from "./task-chat/scroll-navigation"; +import { CoordinatorPresentation } from "./coordinator/CoordinatorPresentation"; import type { HeartbeatRunEvent } from "@paperclipai/shared"; const transcriptState = vi.hoisted(() => ({ @@ -400,6 +401,56 @@ it("keeps revealed history scrollable while metadata for another page is pending expect(container.querySelector('[data-testid="history-scroll-ready"]')?.textContent).toBe("true"); }); +it("waits for a same-task comment target before applying its scroll position without hiding the thread", async () => { + function Readiness() { return {String(useTaskChatScrollNavigation().ready)}; } + const props = { issueId: "linked-task", compactHistory: true, comments: createLongThreadComments(), onAdd: async () => {}, threadHeader: }; + const show = (hash: string, pending: boolean) => render( + + + , + ); + show("", false); + await settleThreadLayout(); + const editor = container.querySelector('[data-testid="mock-editor"]'); + show("#comment-older", true); + expect(container.querySelector('[data-testid="target-scroll-ready"]')?.textContent).toBe("false"); + expect(container.querySelector('[data-testid="task-chat-history-loading"]')).toBeNull(); + expect(container.querySelector('[data-testid="mock-editor"]')).toBe(editor); + show("#comment-older", false); + expect(container.querySelector('[data-testid="target-scroll-ready"]')?.textContent).toBe("true"); +}); + +it("positions an available comment after the reveal deadline even if unrelated metadata is stalled", async () => { + vi.useFakeTimers(); + try { + function Readiness() { return {String(useTaskChatScrollNavigation().ready)}; } + render( + {}} initialHistoryPending threadHeader={} /> + ); + await act(async () => { await vi.advanceTimersByTimeAsync(15_000); }); + expect(container.querySelector('[data-testid="target-scroll-ready"]')?.textContent).toBe("true"); + expect(container.querySelector('[data-testid="mock-editor"]')).not.toBeNull(); + } finally { vi.useRealTimers(); } +}); + +it("bounds the scroll wait for a same-task target that never arrives", async () => { + const props = { issueId: "stalled-target", comments: createLongThreadComments(), onAdd: async () => {} }; + function Readiness() { return {String(useTaskChatScrollNavigation().ready)}; } + const show = (hash: string, pending: boolean) => render( + } /> + ); + show("", false); + await settleThreadLayout(); + vi.useFakeTimers(); + try { + show("#comment-unavailable", true); + expect(container.querySelector('[data-testid="target-scroll-ready"]')?.textContent).toBe("false"); + await act(async () => { await vi.advanceTimersByTimeAsync(15_000); }); + expect(container.querySelector('[data-testid="target-scroll-ready"]')?.textContent).toBe("true"); + expect(container.querySelector('[data-testid="task-chat-history-loading"]')).toBeNull(); + } finally { vi.useRealTimers(); } +}); + it.each([0, 32_000, 100_000])("offers earlier output only when a single legacy run exceeds the compact window (%s bytes)", (logBytes) => { const onExpandHistory = vi.fn(); if (logBytes > 32_000) transcriptState.truncatedRunIds.add("legacy-run"); @@ -703,6 +754,57 @@ describe.each(["legacy", "native"] as const)("%s task history readiness", (runti nativeTranscriptState.hydratedRunIds = new Set(); }); + it("opens saved compact replies without fetching their completed run and loads output on expansion", async () => { + const props = { + issueId: "saved-compact-task", compactHistory: true, onAdd: async () => {}, onExpandHistory: vi.fn(), + linkedRuns: [{ ...retryRun, runId: "saved-run", status: "succeeded", startedAt: retryRun.createdAt }], + comments: [{ ...createLongThreadComments()[3], id: "saved-answer", authorType: "agent" as const, + authorAgentId: "agent-1", body: "Saved investigation result", runId: "saved-run" }], + }; + render(); + expect(transcriptHookRuns.native.at(-1)).toEqual([]); + expect(transcriptHookRuns.legacy.at(-1)).toEqual([]); + render(); + await settleThreadLayout(); + expect(container.textContent).toContain("Saved investigation result"); + expect(transcriptHookRuns.native.at(-1)).toEqual([]); + expect(transcriptHookRuns.legacy.at(-1)).toEqual([]); + const editor = container.querySelector('[data-testid="mock-editor"]'); + act(() => editorCallbacks.change("Keep this draft")); + const output = [...container.querySelectorAll("button")].find(button => button.textContent === "More run output"); + expect(output).toBeDefined(); + act(() => output!.click()); + expect(props.onExpandHistory).toHaveBeenCalledOnce(); + render(); + expect((runtimeMode === "native" ? transcriptHookRuns.native : transcriptHookRuns.legacy).at(-1)).toMatchObject([{ id: "saved-run" }]); + expect(container.querySelector('[data-testid="mock-editor"]')).toBe(editor); + expect(editor?.textContent).toBe("Keep this draft"); + }); + + it.each(["running", "failed", "no-reply", "placeholder", "chat", "yielded", "no-presentation", "progress"])("hydrates compact output when it is required (%s)", (scenario) => { + const props = { + issueId: "required-output", compactHistory: true, onAdd: async () => {}, onExpandHistory: vi.fn(), + conversationMode: scenario === "chat", + linkedRuns: [{ ...retryRun, runId: "saved-run", status: ["running", "failed"].includes(scenario) ? scenario : "succeeded", startedAt: retryRun.createdAt, + resultJson: scenario === "yielded" ? { nativeResult: { schema: "paperclip.run_result.v1", reportedWorkDisposition: "yielded" } } + : scenario === "no-presentation" ? { presentationDecision: { schema: "paperclip.run_presentation_decision.v1", commentId: null } } + : scenario === "progress" ? { semanticToolReceipts: { progress: { operationId: "report_progress", result: { commentId: "saved-answer" } } } } : {} }], + comments: scenario === "no-reply" ? [] : [{ ...createLongThreadComments()[3], id: "saved-answer", authorType: "agent" as const, + authorAgentId: "agent-1", body: scenario === "placeholder" ? "Run completed. Agent did not post a summary comment this run (transcript withheld — see run log)." : "Saved investigation result", runId: "saved-run" }], + }; + render(); + expect((runtimeMode === "native" ? transcriptHookRuns.native : transcriptHookRuns.legacy).at(-1)).toMatchObject([{ id: "saved-run" }]); + }); + + it("retains a live transcript after the saved reply arrives", () => { + const props = { issueId: "live-to-saved", compactHistory: true, onAdd: async () => {}, onExpandHistory: vi.fn(), + linkedRuns: [{ ...retryRun, runId: "live-run", status: "running", startedAt: retryRun.createdAt }], comments: createLongThreadComments() }; + render(); + render(); + expect((runtimeMode === "native" ? transcriptHookRuns.native : transcriptHookRuns.legacy).at(-1)).toMatchObject([{ id: "live-run" }]); + }); + it("reveals comments while a scheduled retry has no transcript to hydrate", async () => { renderThread({ issueId: "issue-1", @@ -1555,6 +1657,33 @@ describe("TaskChatThread runtime transcript selection", () => { expect(container.querySelector('[data-testid="task-chat-run-failed-try-again"]')).toBeNull(); }); + it.each(["timestamp", "resolved-review"].flatMap(owner => [false, true].map(coordinator => ({ owner, coordinator }))))("keeps a saved Plan visible while its completed output is deferred ($owner, Coordinator=$coordinator)", async ({ owner, coordinator }) => { + const runId = "saved-plan-run"; + planState.data = planDocument({ updatedAt: new Date("2026-08-25T18:00:02.000Z") }); + nativeTranscriptState.hydratedRunIds = new Set(); + nativeTranscriptState.terminalHydratedRunIds = new Set(); + const props = { + issueId: "issue-1", compactHistory: true, onAdd: async () => {}, onExpandHistory: vi.fn(), + comments: [{ ...createLongThreadComments()[3], id: "saved-plan-reply", authorType: "agent" as const, + authorAgentId: "agent-1", body: "The saved Plan is ready.", runId }], + interactions: owner === "resolved-review" ? [planReviewInteraction("accepted", "revision-3", runId)] : [], + linkedRuns: [{ runId, runtimeMode: "native" as const, status: "succeeded", agentId: "agent-1", adapterType: "paperclip_runner", + createdAt: "2026-08-25T18:00:00.000Z", startedAt: "2026-08-25T18:00:00.000Z", finishedAt: "2026-08-25T18:00:03.000Z" }], + }; + const show = (compactHistory: boolean) => render( + + ); + show(true); + await settleThreadLayout(); + expect(transcriptHookRuns.native.at(-1)).toEqual([]); + expect(container.querySelectorAll('[data-testid="task-chat-plan-preview"]')).toHaveLength(1); + expect(container.textContent).toContain("Preview the Plan"); + const editor = container.querySelector('[data-testid="mock-editor"]'); + show(false); + expect(container.querySelectorAll('[data-testid="task-chat-plan-preview"]')).toHaveLength(1); + expect(container.querySelector('[data-testid="mock-editor"]')).toBe(editor); + }); + it("projects the saved Plan inline at its native write_document boundary", () => { planState.data = planDocument({ updatedAt: new Date("2026-08-25T18:00:02.000Z"), diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index e89adcb5a54..42738c1cd56 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -20,7 +20,7 @@ import { CoordinatorChatReceiptStatus, useCoordinatorChatDecisionReceipts } from import { CoordinatorChatQuestionStatus, useCoordinatorChatPendingQuestions } from "@/components/coordinator/CoordinatorChatPendingQuestions"; import { requiresExecutionReconciliation } from "@paperclipai/shared"; import { TaskChatExpansionState } from "@/components/task-chat/expansion-state"; -import { TaskChatScrollReady, useTaskChatScrollNavigation } from "@/components/task-chat/scroll-navigation"; +import { TaskChatScrollReady, taskChatHashAnchor, useTaskChatScrollNavigation } from "@/components/task-chat/scroll-navigation"; import { useCallback, useEffect, @@ -971,12 +971,45 @@ export function TaskChatThread(props: TaskChatThreadProps) { }, [linkedRuns, liveRuns, activeRun]); const openedAt = useMemo(() => Date.now(), [issueId]); + const scrollNavigation = useTaskChatScrollNavigation(); + const requestedTranscripts = useRef({ issueId, ids: new Set() }); + if (requestedTranscripts.current.issueId !== issueId) { + requestedTranscripts.current = { issueId, ids: new Set() }; + } + const deferredTranscriptRunIds = useMemo(() => { + const deferred = new Set(); + if (!props.compactHistory || conversationMode || !props.onExpandHistory || scrollNavigation.hash) return deferred; + for (const run of runs) { + if (requestedTranscripts.current.ids.has(run.id) || run.status !== "succeeded" || + !run.createdAt || toMs(run.createdAt) > openedAt) continue; + const meta = linkedRunMetaById.get(run.id); + if (acceptedSemanticResultDisposition(meta?.resultJson) === "yielded" || + interactions?.some(interaction => interaction.sourceRunId === run.id && interaction.status === "pending")) continue; + const presentationCommentId = presentationDecisionCommentId(meta?.resultJson); + const progressCommentIds = semanticProgressCommentIds(meta?.resultJson); + if (projectedComments.some(comment => + comment.authorType === "agent" && !comment.deletedAt && comment.body.trim() && + comment.body !== LEGACY_WITHHELD_RUN_COMMENT && !progressCommentIds.has(comment.id) && + (comment.runId ?? comment.createdByRunId ?? comment.derivedCreatedByRunId) === run.id && + (presentationCommentId === undefined || presentationCommentId === comment.id), + )) deferred.add(run.id); + } + return deferred; + }, [issueId, runs, projectedComments, linkedRunMetaById, interactions, props.compactHistory, conversationMode, props.onExpandHistory, scrollNavigation.hash, openedAt]); + const transcriptRuns = useMemo(() => runs.filter(run => + !deferredTranscriptRunIds.has(run.id) && + !(props.compactHistory && !conversationMode && props.onExpandHistory && !scrollNavigation.hash && + initialHistoryPending && isTerminalRunStatus(run.status) && !requestedTranscripts.current.ids.has(run.id)), + ), [issueId, runs, deferredTranscriptRunIds, props.compactHistory, conversationMode, props.onExpandHistory, scrollNavigation.hash, initialHistoryPending]); + useLayoutEffect(() => { + for (const run of transcriptRuns) requestedTranscripts.current.ids.add(run.id); + }, [transcriptRuns]); const nativeRuns = useMemo( - () => runs.filter((run) => run.runtimeMode === "native").map((run) => ({ + () => transcriptRuns.filter((run) => run.runtimeMode === "native").map((run) => ({ ...run, conversationOnly: Boolean(props.compactHistory && (!run.createdAt || new Date(run.createdAt).getTime() <= openedAt)), })), - [runs, props.compactHistory, openedAt], + [transcriptRuns, props.compactHistory, openedAt], ); const { transcriptByRun: nativeTranscriptByRun, @@ -986,7 +1019,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { terminalHydratedRunIds, retry: retryNativeEvents, } = useNativeRunTranscripts(nativeRuns, { companyId }); - const logRuns = useMemo(() => runs.filter((run) => + const logRuns = useMemo(() => transcriptRuns.filter((run) => (!nativeRuns.some((nativeRun) => nativeRun.id === run.id && nativeRun.conversationOnly) || nativeTranscriptErrorsByRun.has(run.id) || (hydratedNativeRunIds.has(run.id) && (nativeTranscriptByRun.get(run.id)?.length ?? 0) === 0)) && ( @@ -997,7 +1030,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { nativeTranscriptErrorsByRun.has(run.id) || (hydratedNativeRunIds?.has(run.id) && (nativeTranscriptByRun.get(run.id)?.length ?? 0) === 0) - )), [runs, nativeRuns, nativeTranscriptErrorsByRun, hydratedNativeRunIds, nativeTranscriptByRun]); + )), [transcriptRuns, nativeRuns, nativeTranscriptErrorsByRun, hydratedNativeRunIds, nativeTranscriptByRun]); const { transcriptByRun: logTranscriptByRun, isInitialHydrating: logsAreInitiallyHydrating, @@ -1168,7 +1201,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { // for stable embedding, so their Plans remain chronological thread entries. // Prefer a native review request's explicit run binding; while that request // is still being created, infer the native owner from the document timestamp. - const planDocumentSourceRunId = useMemo(() => { + const planDocumentOwnerRunId = useMemo(() => { // Coordinator keeps saved plans in document chronology. A run may write // several documents (or use shell tools with no semantic write boundary); // attaching the plan to its last write can put it after the resulting work. @@ -1231,6 +1264,12 @@ export function TaskChatThread(props: TaskChatThreadProps) { return matchingRuns[0]?.id ?? null; }, [activeRun, coordinatorEnabled, interactions, linkedRuns, liveRuns, planDocument]); + const planDocumentSourceRunId = planDocumentOwnerRunId && + !deferredTranscriptRunIds.has(planDocumentOwnerRunId) && + ((transcriptByRun.get(planDocumentOwnerRunId)?.length ?? 0) > 0 || + !runs.some(run => run.id === planDocumentOwnerRunId && isTerminalRunStatus(run.status))) + ? planDocumentOwnerRunId : null; + const planTurnItem = useMemo(() => { if (!planDocument || !planDocumentSourceRunId) return null; return { @@ -2913,7 +2952,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { const historyControl = props.historyControl ?? ( props.compactHistory && props.onExpandHistory - && logRuns.some((run) => truncatedRunIds?.has(run.id)) + && (deferredTranscriptRunIds.size > 0 || logRuns.some((run) => truncatedRunIds?.has(run.id))) ? : null ); @@ -3039,7 +3078,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { comment.derivedCreatedByRunId, ]), ); - const initialRuns = runs.filter((run) => { + const initialRuns = transcriptRuns.filter((run) => { if ( !initialCommentWindow.current || !isTerminalRunStatus(run.status) || @@ -3082,7 +3121,6 @@ export function TaskChatThread(props: TaskChatThreadProps) { (taskHandoffs.enabled && taskHandoffs.isPending) )); const historyPending = initialHistoryPending || planLoading || transcriptHistoryPending || chatMetadataPending; - const scrollNavigation = useTaskChatScrollNavigation(); useEffect(() => { recordIssueDetailReadiness({ plan: !planLoading, transcripts: !transcriptHistoryPending, history: !historyPending }); }, [issueId, planLoading, transcriptHistoryPending, historyPending]); @@ -3101,6 +3139,17 @@ export function TaskChatThread(props: TaskChatThreadProps) { const historyShellRef = useRef(null); const [expiredHistoryWait, setExpiredHistoryWait] = useState<{ historyKey: typeof historyKey } | null>(null); const historyWaitExpired = expiredHistoryWait !== null && expiredHistoryWait.historyKey === historyKey; + const scrollTarget = taskChatHashAnchor(scrollNavigation.hash ?? ""); + const scrollCommentId = scrollTarget?.startsWith("comment-") ? scrollTarget.slice("comment-".length) : null; + const scrollWaitKey = scrollCommentId ? JSON.stringify([issueId, scrollNavigation.key, scrollNavigation.hash]) : ""; + const [expiredScrollWaitKey, setExpiredScrollWaitKey] = useState(null); + const scrollTargetPending = Boolean(scrollCommentId && initialHistoryPending && + !comments.some(comment => comment.id === scrollCommentId) && expiredScrollWaitKey !== scrollWaitKey); + useEffect(() => { + if (!scrollTargetPending) return; + const timer = window.setTimeout(() => setExpiredScrollWaitKey(scrollWaitKey), INITIAL_HISTORY_REVEAL_TIMEOUT_MS); + return () => window.clearTimeout(timer); + }, [scrollTargetPending, scrollWaitKey]); const prepareInitialContent = Boolean(scrollNavigation.hash) || !historyPending || historyRevealed || historyWaitExpired; // Supporting requests can stall without rejecting. Bound the first reveal // independently of their pending states so saved conversation and the @@ -3146,7 +3195,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { return ( - + diff --git a/ui/src/components/task-chat/TaskMessageScroller.test.tsx b/ui/src/components/task-chat/TaskMessageScroller.test.tsx index fbc0ce1d9f2..24bd3e99097 100644 --- a/ui/src/components/task-chat/TaskMessageScroller.test.tsx +++ b/ui/src/components/task-chat/TaskMessageScroller.test.tsx @@ -186,6 +186,38 @@ describe("TaskMessageScroller", () => { expect(scroller().scrollTop).toBe(1000); }); + it("retains the reading anchor when a resize scroll event precedes the next width reflow", async () => { + const resize: (() => void)[] = []; + vi.stubGlobal("ResizeObserver", class { + constructor(callback: ResizeObserverCallback) { resize.push(() => callback([], this as unknown as ResizeObserver)); } + observe() {} + disconnect() {} + }); + let width = 400, rowTop = 100; + flushSync(() => root.render( +
{ + if (!node) return; + const viewport = node.parentElement!; + fakeGeometry(viewport, { clamp: true }); + Object.defineProperty(viewport, "clientWidth", { get: () => width, configurable: true }); + viewport.getBoundingClientRect = () => ({ top: 0, bottom: 400 } as DOMRect); + }}> +
{ + if (node) node.getBoundingClientRect = () => ({ top: rowTop - scroller().scrollTop, bottom: rowTop + 100 - scroller().scrollTop, height: 100 } as DOMRect); + }}>Saved reply
+
+ )); + await scrollTo(scroller(), 150); + width = 500; rowTop = 80; + for (const callback of resize) callback(); + expect(scroller().scrollTop).toBe(130); + width = 600; rowTop = 60; + await dispatch(scroller(), new Event("scroll", { bubbles: true })); + for (const callback of resize) callback(); + expect(rowTop - scroller().scrollTop).toBe(-50); + expect(pill()).not.toBeNull(); + }); + it("retains upward intent when content reconciles before the scroll event", async () => { const load = vi.fn(); const update = (tick: number) => flushSync(() => root.render( @@ -300,6 +332,31 @@ describe("TaskMessageScroller", () => { expect(viewport.scrollTop).toBe(500); }); + it.each([[false, "none"], [false, "wheel"], [false, "pointer"], [true, "none"], [true, "wheel"], [true, "pointer"]] as const)("positions a late hash target unless the reader moved on (restore=%s, readerMoved=%s)", async (restore, readerMoved) => { + let initialized = false; + const show = (targetReady: boolean) => flushSync(() => root.render( + + +
{ + if (node && !initialized) { fakeGeometry(node.parentElement!, { clamp: true }); initialized = true; } + }}> + Saved conversation + {targetReady ?
{ + if (node) node.getBoundingClientRect = () => ({ top: 100 - scroller().scrollTop, bottom: 200 - scroller().scrollTop, height: 100 } as DOMRect); + }}>Linked comment
: null} +
+ + , + )); + show(false); + if (readerMoved !== "none") { + await scrollTo(scroller(), 300); + await dispatch(scroller(), readerMoved === "wheel" ? new WheelEvent("wheel", { bubbles: true, deltaY: -100 }) : new Event("pointerdown", { bubbles: true })); + } + show(true); + expect(scroller().scrollTop).toBe(readerMoved === "none" ? 100 : 300); + }); + it("does not replace a saved reading position with a hidden route's geometry", async () => { function mount(restore: boolean) { flushSync(() => root.render( diff --git a/ui/src/components/task-chat/TaskMessageScroller.tsx b/ui/src/components/task-chat/TaskMessageScroller.tsx index a0f7c9e6db2..dba3d3863fa 100644 --- a/ui/src/components/task-chat/TaskMessageScroller.tsx +++ b/ui/src/components/task-chat/TaskMessageScroller.tsx @@ -4,7 +4,7 @@ import { cn } from "@/lib/utils"; import { useStreamlinedTaskChatPresentation } from "./presentation-mode"; import { ArrowDown } from "lucide-react"; import { parseCssTimeMs } from "./motion-tokens"; -import { useTaskChatScrollNavigation } from "./scroll-navigation"; +import { taskChatHashAnchor, useTaskChatScrollNavigation } from "./scroll-navigation"; const PIN_THRESHOLD_PX = 48; @@ -57,11 +57,13 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling const navigation = useTaskChatScrollNavigation(); const initialPositionApplied = useRef(false); const appliedNavigation = useRef({ key: navigation.key, hash: navigation.hash }); + const pendingHashTarget = useRef(taskChatHashAnchor(navigation.hash ?? "")); const ref = useRef(null); const anchorRef = useRef(null); const pinnedRef = useRef(true); const easingRef = useRef(false); const clientHeightRef = useRef(null); + const clientWidthRef = useRef(null); const scrollbarIdleTimerRef = useRef(null); const scrollbarIdleDelayRef = useRef(null); const [pillPhase, setPillPhase] = useState("hidden"); @@ -130,8 +132,15 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling const el = ref.current; if (!el) return false; const previousClientHeight = clientHeightRef.current; + const previousClientWidth = clientWidthRef.current; const nextClientHeight = el.clientHeight; clientHeightRef.current = nextClientHeight; + clientWidthRef.current = el.clientWidth; + if (previousClientWidth != null && previousClientWidth !== el.clientWidth && !pinnedRef.current) { + const delta = threadScrollAnchorDelta(el, anchorRef.current, el.getBoundingClientRect().top); + if (delta) el.scrollTop += delta; + return true; + } if ( previousClientHeight == null || previousClientHeight <= 0 || @@ -192,7 +201,7 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling // scroll away from the bottom. if (followViewportResize()) { rememberAnchor(false); - hidePill(); + if (pinnedRef.current) hidePill(); return; } const pinned = requestingHistory.current ? false : isPinned(); @@ -222,6 +231,7 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling ]); const handleJumpToLatest = useCallback(() => { + pendingHashTarget.current = null; const el = ref.current; if (!el) return; if (isPinned()) { @@ -271,9 +281,10 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling const el = ref.current; if (!el) return; clientHeightRef.current = el.clientHeight; + clientWidthRef.current = el.clientWidth; if (typeof ResizeObserver === "undefined") return; const observer = new ResizeObserver(() => { - if (followViewportResize()) hidePill(); + if (followViewportResize() && pinnedRef.current) hidePill(); reconcileContent(); }); observer.observe(el); @@ -289,13 +300,17 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling if (appliedNavigation.current.key !== navigation.key || appliedNavigation.current.hash !== navigation.hash) { appliedNavigation.current = { key: navigation.key, hash: navigation.hash }; initialPositionApplied.current = false; + pendingHashTarget.current = taskChatHashAnchor(navigation.hash ?? ""); } - if (el && el.clientHeight > 0 && navigation.ready && !initialPositionApplied.current) { - const top = navigation.initialPosition(el, el.getBoundingClientRect().top, el.scrollTop); + const target = pendingHashTarget.current ? document.getElementById(pendingHashTarget.current) : null; + if (el && el.clientHeight > 0 && navigation.ready && + (!initialPositionApplied.current || (target && el.contains(target)))) { + const top = navigation.initialPosition(el, el.getBoundingClientRect().top, el.scrollTop, { restore: !initialPositionApplied.current }); if (top !== null) { el.scrollTop = top; pinnedRef.current = isPinned(); rememberAnchor(); + pendingHashTarget.current = null; } initialPositionApplied.current = true; } @@ -307,13 +322,15 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling
{ pendingHashTarget.current = null; }} tabIndex={scrollingHistory ? 0 : undefined} role={scrollingHistory ? "region" : undefined} aria-label={scrollingHistory ? "Conversation messages" : undefined} onKeyDown={event => { + if (["ArrowUp", "ArrowDown", "PageUp", "PageDown", "Home", "End"].includes(event.key)) pendingHashTarget.current = null; if (["ArrowUp", "PageUp", "Home"].includes(event.key) && ref.current && ref.current.scrollTop <= 192) requestEarlierHistory(); }} - onTouchStart={event => { touchStartY.current = event.touches.length === 1 ? event.touches[0]!.clientY : null; }} + onTouchStart={event => { pendingHashTarget.current = null; touchStartY.current = event.touches.length === 1 ? event.touches[0]!.clientY : null; }} onTouchMove={event => { const start = touchStartY.current; if (start !== null && event.touches.length === 1 && event.touches[0]!.clientY > start + 16 @@ -324,6 +341,7 @@ export function TaskMessageScroller({ children, contentKey, className, scrolling }} onTouchEnd={() => { touchStartY.current = null; }} onWheel={event => { + pendingHashTarget.current = null; upwardIntent.current = event.deltaY < 0; if (event.deltaY < 0 && ref.current && ref.current.scrollTop <= 192) requestEarlierHistory(); }} diff --git a/ui/src/components/task-chat/scroll-navigation.tsx b/ui/src/components/task-chat/scroll-navigation.tsx index b87c5f51289..f2fd7df25c3 100644 --- a/ui/src/components/task-chat/scroll-navigation.tsx +++ b/ui/src/components/task-chat/scroll-navigation.tsx @@ -40,9 +40,9 @@ export function useTaskChatScrollNavigation() { key: navigation?.key, hash: navigation?.hash, ready, - initialPosition(root: Element, viewportTop: number, scrollTop: number): number | null { + initialPosition(root: Element, viewportTop: number, scrollTop: number, options?: { restore?: boolean }): number | null { if (!navigation) return null; - if (navigation.restore && positionKey && positions.has(positionKey)) { + if (options?.restore !== false && navigation.restore && positionKey && positions.has(positionKey)) { const saved = positions.get(positionKey)!; // A saved pixel offset goes stale when rotation changes text wrapping. // Bottom-follow needs no row measurements, even across navigation. From 3937215a058d18555e80a2bc87397ffeffe5d898 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:16:40 -0500 Subject: [PATCH 2/3] Qualify explicit native written references in the basic question journey (#18459) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Basic task journeys must prove that people can answer an agent and receive saved work. > - The question journey asks for a choice, then a typed reference, then a saved note. > - A native choice card can offer a real written-answer control as well as predefined options. > - The old observer rejected that form before a person could type the reference. > - This pull request accepts that explicit native control and keeps exact answer and work evidence. > - The separate text-only and named-tool qualifications keep their stricter contracts. ## Linked Issues or Issue Description Refs #18346 and #18456. The integration inherits the reviewed fixture prerequisite #18387 and Agent Chat recovery #18465 through their actual merged parent. The producer fix in #18456 is separate from this basic journey contract correction. Public duplicate PR and issue searches found no other correction for this form equivalence. **What happened?** The basic question journey requires the reference card to have `answerMode: text`. It rejects a native single-select card even when its canonical form explicitly enables a written answer. This makes the observer test a widget shape in addition to the requested behavior. **Expected behavior** The person must make a real choice and type the complete reference. A native card with an explicit written-answer control can satisfy the second step. Selecting a predefined option cannot satisfy it. The agent must resume the bound work and save both actual answers in its own task document. The document must survive reload. **Steps to reproduce** Use the focused recording and real QuestionForm browser test in this PR. Keep the reference as a single-select native question with one predefined option and `customAnswer.enabled: true`. The previous grader rejects `question-resume.forms`. The previous browser helper rejects `single_select` before it uses the written-answer control. **Paperclip version / commit** Original source base `f2db909393de124caf6a5380a52d4e0e1d621661`. Current integration `a417fd827347ed6f7998774a9de6aee70b76d867` merges exactly `12b0cc630471ee151ec5bc8e41a34f09236d6723` into reviewed question source `faec4ce6d775a34d303d200987e4e84d6dc26caf`. The scheduled failure on `82d15946c5844d3ae41c6ade62fa224053f20a9a` remains a failure. Its saved card had no canonical written-answer capability and would still fail this change. The retained normalized events do not prove whether the original provider flag was omitted or false. **Deployment mode** Attached native task journeys, including QA Cloud. This PR changes the observer and grader. It does not change the app, provider prompt, model, budget, timeouts, cleanup, or runtime deployment. ## What Changed - Accept one provider-bound single-select reference card with explicit canonical custom answers and valid, distinct options. Keep pure-text reference support. - Make the browser select the actual written-answer control and fill the real rich input. Opt in only for the basic question journey. - Keep the exact no-option typed response, immutable card, board actor, native request and continuation, saved agent document, reload, model, environment, checkpoints, and existing one-to-three-run checks. - Reject missing or disabled custom capability, inferred legacy capability, malformed or extra questions, foreign native identities, changed cards, substituted options, and wrong or missing reference text. - Add a real production QuestionForm and TaskChatRichInput Chromium probe. It first selects a predefined option, then proves the helper clears it and submits only the exact custom text. - Document the contract correction and preserve the historical failed attempt and its separate cleanup outcome. Keep necessary-question and named-tool documentation qualification unchanged. ## Verification - Before the observer change, the focused grader regression failed only the new valid-written-reference case; the other 108 cases passed. - Before the browser helper change, real Chromium passed the text control and four rejection controls but failed the valid native written-answer control at the old pure-text assertion. - Initial focused Runner E2E unit selection at `af07f4de`: **256 passed across 9 files**. This includes question/resumption, clarification evidence/flow/collection/deadline, necessary question, documentation, contracts, and report catalog tests. - Initial real Chromium selection at `af07f4de`: **12 passed**. Six new production QuestionForm/control checks and all six existing document-readiness checks pass. No form or rich-input component is mocked. - Historical test-only head `faec4ce6d775a34d303d200987e4e84d6dc26caf` adds a direct canonical-absent legacy `freeText` grader negative and renders the actual legacy presentation in the browser. All four production helper blobs remain identical to the reviewed head. A temporary legacy inference mutant makes the new negative fail. After restoring production source, **179 focused unit tests and all 6 real QuestionForm checks pass**. The six document-readiness specs and their helper are unchanged. - Original [CI run 41205748589](https://github.com/paperclipai/paperclip/actions/runs/41205748589) failed the staged-listen Runner handoff test; that failure is preserved and was not retried. Its Runner source closure was identical to an earlier passing canonical lane. The symptom also recurred on other heads and is tracked independently; no infrastructure attribution or live pass is claimed. - Current integration `a417fd827347ed6f7998774a9de6aee70b76d867` preserves **all 12 question files byte-for-byte** from faec. Every other tracked path is identical to exact merged parent `12b0cc630471ee151ec5bc8e41a34f09236d6723`, which contains actual prerequisite `baf651c8d0122efc93f3ef03e891919eeb666215`. There is no manual conflict resolution or additional production edit. - On that integrated head: **257 focused Vitest tests across 9 files, 206 required Node source/reporter tests, and 12 real Chromium tests pass**. Map/module-boundary checks and catalog discovery pass. The actual `--preview` command passes with a synthetic question target and an intentionally unavailable adapter; it performs no credential read, adapter loading, provisioning or model call. It retains maxRuns3, a 600000ms campaign bound and zero retries. - Exact-head [CI run 41210806950](https://github.com/paperclipai/paperclip/actions/runs/41210806950) passes, including official typecheck, build, all eight browser shards, both Runner Vitest lanes and aggregate verify. All 59 current PR contexts are terminal with no failures. Fresh [Greptile summary](https://github.com/paperclipai/paperclip/pull/18459#issuecomment-6543878408) explicitly reviews `a417fd827347ed6f7998774a9de6aee70b76d867` at 5/5 with no findings; its exact-head check succeeds and all threads are resolved. No fresh deployed journey pass is claimed yet. - Catalog discovery lists the existing question journey. Map checks and module-boundary checks pass. Diff checks pass. - `tsc -p tests/runner-e2e/tsconfig.json` was attempted. The fresh integrated checkout still lacks compiled Runner, plugin SDK, and adapter declarations. The attempted local check reports 344 diagnostic lines from dependency/type closure and zero diagnostics in all 12 question-owned paths. Official exact-head CI establishes the full typecheck/build gates. Full repository build, typecheck, and test suites were not repeated locally for this observer change. - The first broad local report selection lacked the CLI loader and then hit sandbox IPC restrictions. Locked dependency installation and an authorized local IPC run resolved those setup errors; the full selected 256 tests pass. The initial localhost probe also needed the normal local-listen permission. - No fresh QA pass is claimed. The coordinator must qualify the final reviewed runtime and harness together. Prior failed results remain preserved. ## Risks The basic journey accepts one additional form with explicit native provenance. It still rejects the original closed card without a written-answer capability. The separate producer default fix must reach the selected remote provider pack before it can affect a deployed run. Local browser and grader tests do not prove that artifact served or that a model completed the live journey. Existing cleanup and physical-stop obligations remain separate. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, source inspection, browser testing, and code execution. The host does not expose the 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 and linked related work above - [x] I have described the issue in-PR following the bug 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 selected 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 requesting merge --- doc/evals.md | 4 +- tests/runner-e2e/PRODUCT-JOURNEYS.md | 7 +- tests/runner-e2e/README.md | 14 +++- tests/runner-e2e/continuation-flow.ts | 9 +- tests/runner-e2e/playwright-support.config.ts | 2 +- .../clarification-evidence.test.ts | 15 +++- .../clarification-evidence.ts | 5 +- .../product-journeys/clarification-flow.ts | 2 +- tests/runner-e2e/question-resume-scoring.ts | 13 ++- tests/runner-e2e/question-resume-ui.spec.ts | 83 +++++++++++++++++++ tests/runner-e2e/question-resume-ui.ts | 12 ++- tests/runner-e2e/question-resume.test.ts | 47 +++++++++++ 12 files changed, 194 insertions(+), 19 deletions(-) create mode 100644 tests/runner-e2e/question-resume-ui.spec.ts diff --git a/doc/evals.md b/doc/evals.md index e396d676907..517eb0b4ecd 100644 --- a/doc/evals.md +++ b/doc/evals.md @@ -71,7 +71,9 @@ tests do not establish a completed live pass or unattended CI qualification. The deployment-targeted `question-answer-resume` journey reuses the classic two-question scenario and native continuation oracle on an attached target. It checks the actual board answers, one to three bound work runs, an agent-authored -task document and browser readback after reload. It accepts provider questions and +task document and browser readback after reload. A typed reference may use a text +field or the native card's explicit written-answer control; a predefined option +cannot replace the exact typed answer. It accepts provider questions and durable questions proven by an applied semantic receipt or an immutable completed native API mutation receipt. It does not qualify named-tool selection. Its provider owns target setup, usage and cleanup. Registration and local calibration do not establish a fresh diff --git a/tests/runner-e2e/PRODUCT-JOURNEYS.md b/tests/runner-e2e/PRODUCT-JOURNEYS.md index 34cdd31a158..f0ace922eeb 100644 --- a/tests/runner-e2e/PRODUCT-JOURNEYS.md +++ b/tests/runner-e2e/PRODUCT-JOURNEYS.md @@ -40,7 +40,12 @@ never start a server or depend on a tenant filesystem. The question-resumption journey uses the same natural-language scenario, browser answer helper and behavioral grader as the classic `question-resume` suite. Native -provider questions can stay in one run; durable native questions create one exactly bound +reference cards may expose a text field or an explicit canonical written-answer +control on one single-select question. The browser must type the complete reference +and submit no predefined option. An absent or disabled custom-answer capability is +not an equivalent form. The user still answers exactly two distinct native questions; +the document must retain the actual choice and typed reference after reload. +Provider questions can stay in one run; durable native questions create one exactly bound continuation each. Creation must have an applied semantic receipt or a completed native API mutation receipt with the original card, exact scope, form and idempotency key. An API receipt must retain its creation identity unchanged after the answer. A form or diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index fbfbe27169c..33231603802 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -1771,7 +1771,13 @@ remain unchanged after the answer. Both durable paths require one new response wake bound to that interaction and source run. Missing, pending, foreign or mismatched receipts fail; form shape and run counts cannot replace them. Only a verified provider choice may carry the adapter's optional Other field. A second substantive question -is still rejected; the subsequent reference question must be text-only. Both real +is still rejected. The subsequent reference must use a text field or one native +provider single-select question with an explicit canonical `customAnswer.enabled=true`. +The browser selects its written-answer control and submits the exact typed reference +with no predefined option. Missing or disabled custom answers, inferred legacy +capabilities, extra questions and substituted choices fail. This behavioral form +equivalence does not change the text-only necessary-question or named-tool +documentation qualifications. Both real board answers must remain bound to unchanged forms, and one saved document must include both answers. Existing lifecycle, ownership, premature-output, terminal state and no-extra-task checks apply. Three checkpoints are required even when @@ -1785,6 +1791,12 @@ Other field stopped the driver before any answer remains a FAIL. A new behavior pass does not establish semantic-tool selection, documentation placement, default hiring behavior or general reliability. +The original scheduled attempt that stopped at a closed reference card remains a +FAIL, including its separate cleanup failure. Supporting a server-bound written +answer does not regrade that card, infer a missing provider flag, or establish a +fresh Cloud pass. The general OpenCode omitted-default normalization fix requires +its own reviewed runtime artifact; browser/grader calibration cannot prove it served. + Continuation screenshots wait for the correct task heading and fully revealed conversation before capture. A loading screen or wrong task fails capture. Browser-only regressions exercise delayed rendering without provider calls: diff --git a/tests/runner-e2e/continuation-flow.ts b/tests/runner-e2e/continuation-flow.ts index 89072dc005e..3da0668ebae 100644 --- a/tests/runner-e2e/continuation-flow.ts +++ b/tests/runner-e2e/continuation-flow.ts @@ -1,6 +1,6 @@ import { gradeNecessaryQuestion } from "./necessary-question.js"; import { necessaryQuestionContract } from "./continuation-cases.js"; -import { validResumeQuestionForm } from "./question-resume-scoring.js"; +import { validResumeQuestionForm, validResumeReferenceForm } from "./question-resume-scoring.js"; import { answerResumeQuestionThroughUi } from "./question-resume-ui.js"; import { gradeLifecycleBaseline, type LifecycleCheckpoint } from "./lifecycle-baseline.js"; import { lifecycleLiveCase, lifecycleLiveContinuation, gradeLifecycleNarrative } from "./lifecycle-live-cases.js"; @@ -220,12 +220,15 @@ export async function runContinuationFlow(input: { if (scenario.id === "necessary-free-text") { expect(validResumeQuestionForm(checkpoints[0]!, questions[0], false), "a bound text-only question").toBe(true); } else if (scenario.id === "question-answer-resume") { - expect(validResumeQuestionForm(checkpoints.at(-1)!, questions[0], !!choice), "a verified native question path and the requested form").toBe(true); + expect(choice ? validResumeQuestionForm(checkpoints.at(-1)!, questions[0], true) + : validResumeReferenceForm(checkpoints.at(-1)!, questions[0]), "a verified native question path and the requested form").toBe(true); } else if (scenario.id === "provider-question-bridge") { expect(isSingleClaudeQuestion(set.questions), "one choice question with only the optional provider Other field").toBe(true); } else expect(set.questions, "ask only the requested next question").toHaveLength(1); const before = new Set(runs.map((r) => r.id)); - await answerResumeQuestionThroughUi(page, questions[0], choice ? { choice } : { text: scenario.answer }); + await answerResumeQuestionThroughUi(page, questions[0], choice ? { choice } : { + text: scenario.answer, allowProviderCustomAnswer: scenario.id === "question-answer-resume", + }); await settle(before, false, questions[0].id); } async function reply(body: string) { diff --git a/tests/runner-e2e/playwright-support.config.ts b/tests/runner-e2e/playwright-support.config.ts index 000c7d502c6..72cc2bfac65 100644 --- a/tests/runner-e2e/playwright-support.config.ts +++ b/tests/runner-e2e/playwright-support.config.ts @@ -3,7 +3,7 @@ import { defineConfig } from "@playwright/test"; /** Browser-only harness regressions: no Paperclip instance or provider credentials. */ export default defineConfig({ testDir: ".", - testMatch: ["clarification-readback.spec.ts", "project-context-readback.spec.ts", "skill-studio-flow.spec.ts", "native-task-reply.spec.ts", "screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts", "kimi-permission-card.spec.ts", "github-review-check.spec.ts", "antigravity-journey-browser.spec.ts", "task-search-filter.spec.ts"], + testMatch: ["question-resume-ui.spec.ts", "clarification-readback.spec.ts", "project-context-readback.spec.ts", "skill-studio-flow.spec.ts", "native-task-reply.spec.ts", "screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts", "kimi-permission-card.spec.ts", "github-review-check.spec.ts", "antigravity-journey-browser.spec.ts", "task-search-filter.spec.ts"], workers: 1, retries: 0, timeout: 10_000, diff --git a/tests/runner-e2e/product-journeys/clarification-evidence.test.ts b/tests/runner-e2e/product-journeys/clarification-evidence.test.ts index 88c0ccb47cf..2a0287cc8d3 100644 --- a/tests/runner-e2e/product-journeys/clarification-evidence.test.ts +++ b/tests/runner-e2e/product-journeys/clarification-evidence.test.ts @@ -28,7 +28,7 @@ import type { JourneyContext } from "./types.js"; type Row = Record; const context = { company: { id: "company" }, user: { id: "board" }, environmentId: "daytona", agent: { id: "agent", provider: "opencode", model: "model", runtimeMode: "native" } } as JourneyContext; -function recording(paths: Array<"provider" | "semantic" | "api">) { +function recording(paths: Array<"provider" | "semantic" | "api">, writtenReference = false) { const runs: Row[] = [], cards: Row[] = [], checkpoints: any[] = []; const createRun = (previous?: Row) => ({ id: `run-${runs.length}`, status: "running", runtimeMode: "native", companyId: "company", issueId: "task", agentId: "agent", contextSnapshot: { paperclipEnvironment: { id: "daytona" }, ...(previous ? { issueId: "task", sourceRunId: previous.sourceRunId, @@ -41,7 +41,9 @@ function recording(paths: Array<"provider" | "semantic" | "api">) { runs.push(createRun()); for (const [index, path] of paths.entries()) { const run = runs.at(-1)!; - const field = index === 0 ? { id: "choice", answerMode: "single_select", options: [{ id: "am", label: "Morning" }, { id: "pm", label: "Afternoon" }] } : { id: "reference", answerMode: "text" }; + const field = index === 0 ? { id: "choice", answerMode: "single_select", options: [{ id: "am", label: "Morning" }, { id: "pm", label: "Afternoon" }] } + : writtenReference ? { id: "reference", answerMode: "single_select", options: [{ id: "none", label: "No reference" }], customAnswer: { enabled: true } } + : { id: "reference", answerMode: "text" }; const request = `request-${index}`, semantic = path !== "provider"; const card: Row = { id: `question-${index}`, kind: "ask_user_questions", status: "pending", companyId: "company", issueId: "task", createdByAgentId: "agent", sourceRunId: run.id, idempotencyKey: semantic ? `semantic-${index}` : `paperclip-runner-question:${run.id}:${request}`, continuationPolicy: semantic ? "wake_assignee" : "none", createdByUserId: null, @@ -65,6 +67,11 @@ function recording(paths: Array<"provider" | "semantic" | "api">) { const failures = (f: ReturnType) => gradeClarificationEvidence(context, "task", "AMBERnonce", f.checkpoints, f.readback).filter(c => !c.passed); describe("attached clarification evidence", () => { + it("accepts an explicit provider written reference with both exact board answers and saved native work", () => { + const f = recording(["provider", "provider"], true), point = f.checkpoints[1]; + expect(clarificationQuestionFailure(point, point.interactions[1], false)).toBeUndefined(); + expect(failures(f)).toEqual([]); + }); it.each([true, false])("rejects an incomplete API receipt instead of inferring native question evidence (choice=%s)", choice => { const f = recording(["semantic", "semantic"]), point = f.checkpoints[choice ? 0 : 1]; const question = point.interactions.find((i: Row) => i.status === "pending"), source = point.runs.at(-1); @@ -87,8 +94,8 @@ describe("attached clarification evidence", () => { expect(failures(recording([...paths]))).toEqual([]); }); it.each(["foreign-user", "foreign-agent-document", "foreign-task-document", "lost-revision", "wrong-visible-answer", "changed-readback-body", "premature-document", "extra-child", "foreign-model", "foreign-environment", "missing-native-input", "duplicate-question", "wrong-answer"])("rejects a plausible false pass: %s", kind => { - for (const path of ["semantic", "api"] as const) { - const f = recording([path, path]), first = f.checkpoints[0], final = f.checkpoints[2]; + for (const path of ["semantic", "api", "provider"] as const) { + const f = recording([path, path], path === "provider"), first = f.checkpoints[0], final = f.checkpoints[2]; if (kind === "foreign-user") final.interactions[1].resolvedByUserId = "other-board"; if (kind === "foreign-agent-document") final.documents[0].updatedByAgentId = "other-agent"; if (kind === "foreign-task-document") final.documents[0].issueId = "other-task"; diff --git a/tests/runner-e2e/product-journeys/clarification-evidence.ts b/tests/runner-e2e/product-journeys/clarification-evidence.ts index 69e39b7fa62..4d3ab02e70a 100644 --- a/tests/runner-e2e/product-journeys/clarification-evidence.ts +++ b/tests/runner-e2e/product-journeys/clarification-evidence.ts @@ -1,4 +1,4 @@ -import { gradeQuestionResume, questionPath, validResumeQuestionForm } from "../question-resume-scoring.js"; +import { gradeQuestionResume, questionPath, validResumeQuestionForm, validResumeReferenceForm } from "../question-resume-scoring.js"; import type { ContinuationCheckpoint } from "../continuation-scoring.js"; import { validateAgentTaskState } from "./agent-flows.js"; import type { JourneyContext } from "./types.js"; @@ -19,7 +19,8 @@ export function clarificationQuestionFailure(point: ContinuationCheckpoint, ques }); return apiCreated ? `clarification_${stage}_api_fallback` : `clarification_${stage}_native_path_invalid`; } - return validResumeQuestionForm(point, question, choice) ? undefined : `clarification_${stage}_form_invalid`; + return (choice ? validResumeQuestionForm(point, question, true) : validResumeReferenceForm(point, question)) + ? undefined : `clarification_${stage}_form_invalid`; } export function gradeClarificationEvidence(context: JourneyContext, issueId: string, marker: string, diff --git a/tests/runner-e2e/product-journeys/clarification-flow.ts b/tests/runner-e2e/product-journeys/clarification-flow.ts index 800a747284d..8edb160c5e0 100644 --- a/tests/runner-e2e/product-journeys/clarification-flow.ts +++ b/tests/runner-e2e/product-journeys/clarification-flow.ts @@ -91,7 +91,7 @@ export async function runClarificationJourney(context: JourneyContext, session: await session.own({ kind: "issue", name: title, id: created.issueId, journeyId: "question-answer-resume" }); issue = await context.api.get(`/api/issues/${created.issueId}`); let state = await settle(); - for (const [index, input] of [{ choice: "Afternoon" }, { text: scenario.answer }].entries()) { + for (const [index, input] of [{ choice: "Afternoon" }, { text: scenario.answer, allowProviderCustomAnswer: true }].entries()) { const point = await snapshot(index === 0 ? "initial" : "answered", state); const question = state.interactions.find(i => i.kind === "ask_user_questions" && i.status === "pending")!; const questionFailure = clarificationQuestionFailure(point, question, index === 0); diff --git a/tests/runner-e2e/question-resume-scoring.ts b/tests/runner-e2e/question-resume-scoring.ts index 85f8902d95e..30f1e46c350 100644 --- a/tests/runner-e2e/question-resume-scoring.ts +++ b/tests/runner-e2e/question-resume-scoring.ts @@ -67,6 +67,15 @@ export function validResumeQuestionForm(checkpoint: ContinuationCheckpoint, i: R (fields.length === 1 || (path === "provider" && isSingleClaudeQuestion(fields))); } +export function validResumeReferenceForm(checkpoint: ContinuationCheckpoint, i: Row): boolean { + if (validResumeQuestionForm(checkpoint, i, false)) return true; + const fields = resumeQuestionFields(i), field = fields[0], options: Row[] = Array.isArray(field?.options) ? field.options : []; + return questionPath(checkpoint, i) === "provider" && fields.length === 1 && nonempty(field.id) + && field.answerMode === "single_select" && field.customAnswer?.enabled === true + && options.length > 0 && options.every(option => nonempty(option.id) && nonempty(option.label)) + && new Set(options.map(option => option.id)).size === options.length; +} + export function sameQuestion(a: Row, b?: Row) { return !!b && ["id", "kind", "sourceRunId", "issueId", "companyId", "createdByAgentId", "idempotencyKey", "continuationPolicy"] .every(k => a[k] === b[k]) && isDeepStrictEqual(a.payload, b.payload); @@ -110,8 +119,8 @@ export function gradeQuestionResume(checkpoints: ContinuationCheckpoint[], marke options.every(o => nonempty(o.id)) && new Set(options.map(o => o.id)).size === options.length && options.some(o => /^morning\b/i.test(String(o.label).trim())) && !!afternoon && questions(middle).length === 2 && questions(middle).filter(i => i.status === "pending").length === 1 && - nonempty(second?.id) && second?.id !== first?.id && validResumeQuestionForm(middle, second, false), - detail: "One choice plus only a verified provider Other companion, then a distinct text-only question.", + nonempty(second?.id) && second?.id !== first?.id && validResumeReferenceForm(middle, second), + detail: "One choice plus only a verified provider Other companion, then a distinct typed reference with an explicit provider written-answer control or a text field.", }]; for (const [before, after, i, field, answer] of [ [initial, middle, first, choice, "choice"], [middle, final, second, text, "text"], diff --git a/tests/runner-e2e/question-resume-ui.spec.ts b/tests/runner-e2e/question-resume-ui.spec.ts new file mode 100644 index 00000000000..e1510b3aee7 --- /dev/null +++ b/tests/runner-e2e/question-resume-ui.spec.ts @@ -0,0 +1,83 @@ +import path from "node:path"; +import type { IncomingMessage, ServerResponse } from "node:http"; +import { expect, test } from "@playwright/test"; +import { createServer, type ViteDevServer } from "../../ui/node_modules/vite/dist/node/index.js"; +import { answerResumeQuestionThroughUi } from "./question-resume-ui.js"; +import { chatQuestionPresentation } from "./chat-flow.js"; + +let server: ViteDevServer; +let origin: string; +const uiRoot = path.resolve(import.meta.dirname, "../../ui"); +const probe = path.join(uiRoot, "src/__question_resume_probe.tsx"); + +test.beforeAll(async () => { + server = await createServer({ + root: uiRoot, configFile: false, + resolve: { alias: { "@": path.join(uiRoot, "src") } }, + oxc: { jsx: { runtime: "automatic" } }, + server: { host: "127.0.0.1", port: 0 }, + plugins: [{ + name: "question-resume-controls", + resolveId(id: string) { if (id === "/src/__question_resume_probe.tsx") return probe; }, + load(id: string) { + if (id !== probe) return; + return `import React from "react"; + import { createRoot } from "react-dom/client"; + import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + import { QuestionForm } from "/src/components/task-chat/QuestionForm.tsx"; + const root = createRoot(document.getElementById("root")); + const client = new QueryClient(); + window.showQuestion = questionSet => root.render( + { window.submitted = response; }} /> + );`; + }, + configureServer(vite: ViteDevServer) { + vite.middlewares.use((req: IncomingMessage, res: ServerResponse, next: () => void) => { + if (req.url !== "/") return next(); + res.setHeader("Content-Type", "text/html"); + res.end('
'); + }); + }, + }], + }); + await server.listen(); + const address = server.httpServer!.address(); + if (!address || typeof address === "string") throw new Error("Missing test server address"); + origin = `http://127.0.0.1:${address.port}`; +}); +test.afterAll(async () => { await server?.close(); }); + +for (const mode of ["text", "single_select"] as const) { + test(`submits an exact typed reference through the real ${mode} QuestionForm control`, async ({ page }) => { + const field = { id: "reference", prompt: "What reference should I include?", required: true, answerMode: mode, + ...(mode === "single_select" ? { options: [{ id: "none", label: "No reference" }], customAnswer: { enabled: true, label: "Write reference" } } : {}) }; + const card = { payload: { questionSet: { schema: "paperclip.question_set.v1", questions: [field] } } }; + await page.goto(origin); + await page.waitForFunction(() => typeof (window as any).showQuestion === "function"); + await page.evaluate(set => (window as any).showQuestion(set), card.payload.questionSet); + if (mode === "single_select") await page.getByRole("radio", { name: "No reference", exact: true }).click(); + await answerResumeQuestionThroughUi(page, card, { text: "Use AMBERwritten.", allowProviderCustomAnswer: true }); + expect(await page.evaluate(() => (window as any).submitted)).toEqual({ + schema: "paperclip.question_response.v1", answers: { reference: mode === "text" + ? { text: "Use AMBERwritten." } : { selectedOptionIds: [], customText: "Use AMBERwritten." } }, + }); + if (mode === "single_select") await expect(page.getByRole("radio", { name: "No reference", exact: true })).toHaveAttribute("aria-checked", "false"); + }); +} + +for (const invalid of ["missing", "disabled", "legacy", "strict"] as const) { + test(`does not infer a written-answer capability from ${invalid} metadata`, async ({ page }) => { + const field = { id: "reference", prompt: "Reference?", answerMode: "single_select", options: [{ id: "none", label: "No reference" }], + ...(invalid === "missing" ? {} : { customAnswer: { enabled: invalid !== "disabled" } }) }; + const card = { payload: invalid === "legacy" ? { questions: [{ ...field, options: [{ id: "none", label: "No reference", freeText: true }] }] } + : { questionSet: { schema: "paperclip.question_set.v1", questions: [field] } } }; + await page.goto(origin); + await page.waitForFunction(() => typeof (window as any).showQuestion === "function"); + const set = invalid === "legacy" ? chatQuestionPresentation(card.payload as any) : card.payload.questionSet; + await page.evaluate(set => (window as any).showQuestion(set), set); + await expect(page.getByRole("radio", { name: "No reference", exact: true })).toBeVisible(); + await expect(page.getByRole("radio", { name: "Other", exact: true })).toHaveCount(invalid === "strict" ? 1 : 0); + await expect(answerResumeQuestionThroughUi(page, card, { text: "Use AMBERwritten.", ...(invalid === "strict" ? {} : { allowProviderCustomAnswer: true }) })).rejects.toThrow(); + expect(await page.evaluate(() => (window as any).submitted)).toBeUndefined(); + }); +} diff --git a/tests/runner-e2e/question-resume-ui.ts b/tests/runner-e2e/question-resume-ui.ts index ac6fa3f9de8..9e325337e93 100644 --- a/tests/runner-e2e/question-resume-ui.ts +++ b/tests/runner-e2e/question-resume-ui.ts @@ -3,7 +3,7 @@ import { chatQuestionPresentation } from "./chat-flow.js"; type Row = Record; -export async function answerResumeQuestionThroughUi(page: Page, interaction: Row, input: { choice: string } | { text: string }) { +export async function answerResumeQuestionThroughUi(page: Page, interaction: Row, input: { choice: string } | { text: string; allowProviderCustomAnswer?: boolean }) { const set = chatQuestionPresentation(interaction.payload); if ("choice" in input) { expect(set.questions[0]?.answerMode, "choices must use radio controls").toBe("single_select"); @@ -12,8 +12,14 @@ export async function answerResumeQuestionThroughUi(page: Page, interaction: Row const choice = input.choice.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); await page.getByRole("radio", { name: new RegExp(`^${choice}\\b`, "i") }).last().click(); } else { - expect(set.questions[0]?.answerMode, "open answers must render a text field, not a choice question").toBe("text"); - await page.getByTestId("question-text-answer-composer").last() + const field = set.questions[0]; + const custom = field?.answerMode !== "text"; + if (custom) { + expect(input.allowProviderCustomAnswer && interaction.payload.questionSet && field?.answerMode === "single_select" + && field.customAnswer?.enabled === true, "the canonical provider form must explicitly permit a written reference").toBeTruthy(); + await page.getByRole("radio", { name: field!.customAnswer?.label ?? "Other", exact: true }).last().click(); + } + await page.getByTestId(custom ? "question-other-answer-composer" : "question-text-answer-composer").last() .locator('[contenteditable="true"],textarea').first().fill(input.text); } for (let index = 1; index < set.questions.length; index += 1) { diff --git a/tests/runner-e2e/question-resume.test.ts b/tests/runner-e2e/question-resume.test.ts index bded4d1c8d2..c312d4af580 100644 --- a/tests/runner-e2e/question-resume.test.ts +++ b/tests/runner-e2e/question-resume.test.ts @@ -60,8 +60,55 @@ function recording(paths: QuestionPath[] = ["provider", "semantic"]) { } const failed = (r: any[]) => gradeQuestionResume(r, "AMBERnonce").filter(c => !c.passed).map(c => c.id); const question = (r: any[], n = 0) => r[n].interactions[n]; +function writtenReference(r: any[]) { + for (const point of r.slice(1)) Object.assign(point.interactions[1].payload.questionSet.questions[0], { + answerMode: "single_select", options: [{ id: "none", label: "No reference" }], customAnswer: { enabled: true }, + }); + return r; +} describe("native question/resume path qualification", () => { + it("accepts an explicit provider written answer without selecting a predefined option", () => { + const r = writtenReference(recording(["provider", "provider"])); + expect(failed(r)).toEqual([]); + expect(validResumeQuestionForm(r[1], question(r, 1), false)).toBe(false); + }); + it("rejects an inferred legacy freeText reference without a canonical question set", () => { + const r = writtenReference(recording(["provider", "provider"])); + for (const point of r.slice(1)) { + const payload = point.interactions[1].payload; + payload.questions = [{ id: "reference", prompt: "What reference should I include?", selectionMode: "single", + options: [{ id: "write", label: "Write reference", freeText: true }] }]; + delete payload.questionSet; + } + expect(failed(r)).toContain("question-resume.forms"); + }); + it.each(["missing-custom", "disabled-custom", "missing-options", "duplicate-options", "missing-option-id", "multi-select", "extra-question", + "wrong-request", "foreign-source", "foreign-agent", "foreign-company", "semantic", "api"])("rejects an unverified written-reference form: %s", kind => { + const r = writtenReference(recording(["provider", kind === "semantic" || kind === "api" ? kind : "provider"])); + const card = question(r, 1), field = card.payload.questionSet.questions[0]; + if (kind === "missing-custom") delete field.customAnswer; + if (kind === "disabled-custom") field.customAnswer.enabled = false; + if (kind === "missing-options") field.options = []; + if (kind === "duplicate-options") field.options.push(structuredClone(field.options[0])); + if (kind === "missing-option-id") delete field.options[0].id; + if (kind === "multi-select") field.answerMode = "multi_select"; + if (kind === "extra-question") card.payload.questionSet.questions.push({ id: "extra", answerMode: "text" }); + if (kind === "wrong-request") card.payload.runtimeRequestId = "other"; + if (kind === "foreign-source") card.sourceRunId = "other"; + if (kind === "foreign-agent") card.createdByAgentId = "other"; + if (kind === "foreign-company") card.companyId = "other"; + expect(failed(r)).toContain("question-resume.forms"); + }); + it.each(["predefined-option", "wrong-text", "missing-text", "foreign-user", "changed-form"])("rejects a corrupt written-reference answer: %s", kind => { + const r = writtenReference(recording(["provider", "provider"])), card = r[2].interactions[1], answer = card.result.answers[0]; + if (kind === "predefined-option") answer.optionIds = ["none"]; + if (kind === "wrong-text") answer.otherText = "Use OLD."; + if (kind === "missing-text") delete answer.otherText; + if (kind === "foreign-user") { card.resolvedByUserId = null; card.resolvedByAgentId = "agent"; } + if (kind === "changed-form") card.payload.questionSet.questions[0].customAnswer.label = "Changed"; + expect(failed(r)).toContain("question-resume.text.answer"); + }); it("accepts a native API creation receipt with the retained server response shape", () => { const point = JSON.parse(readFileSync(new URL("./fixtures/native-api-question.json", import.meta.url), "utf8")); expect(questionPath(point, point.interactions[0])).toBe("api"); From 9a3f395737e7a92faf99971459979061d423531a Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:19:31 -0500 Subject: [PATCH 3/3] fix(server): settle routine history without idle scans (#18399) Reviewed and verified at 0b65f490d98d53c7031d9d306d4f65da9e821ed3. Co-Authored-By: Paperclip --- doc/plans/2026-10-30-routine-status-sleep.md | 61 ++++ ...tbeat-workspace-branch-containment.test.ts | 20 +- .../routine-execution-status-work.test.ts | 314 ++++++++++++++++++ .../routine-execution-status-worker.test.ts | 107 ++++++ .../routine-status-reconciliation.test.ts | 23 +- .../server-startup-feedback-export.test.ts | 27 ++ .../status-card-evaluation-work.test.ts | 12 +- server/src/index.ts | 17 +- .../services/delivery-work-notifications.ts | 1 + .../execution-control-notifications.ts | 3 + .../services/execution-recovery-resolution.ts | 2 + server/src/services/heartbeat/queue.ts | 2 + .../src/services/heartbeat/run-lifecycle.ts | 3 +- .../issue-review-state-notifications.ts | 2 + server/src/services/issue-tree-control.ts | 2 + server/src/services/issues.ts | 11 + .../native-routine-status.test.ts | 6 +- .../native-workspace-reconstruction.test.ts | 8 +- .../native-workspace-reconstruction.ts | 2 + .../routine-execution-status-notifications.ts | 8 + .../services/routine-execution-status-work.ts | 27 ++ server/src/services/routine-run-status.ts | 71 +++- server/src/services/routines.ts | 44 +-- 23 files changed, 703 insertions(+), 70 deletions(-) create mode 100644 doc/plans/2026-10-30-routine-status-sleep.md create mode 100644 server/src/__tests__/routine-execution-status-work.test.ts create mode 100644 server/src/__tests__/routine-execution-status-worker.test.ts create mode 100644 server/src/services/routine-execution-status-notifications.ts create mode 100644 server/src/services/routine-execution-status-work.ts diff --git a/doc/plans/2026-10-30-routine-status-sleep.md b/doc/plans/2026-10-30-routine-status-sleep.md new file mode 100644 index 00000000000..0afe809bb8a --- /dev/null +++ b/doc/plans/2026-10-30-routine-status-sleep.md @@ -0,0 +1,61 @@ +# Routine execution status settlement + +The Cloud routine status worker repairs a routine invocation when its linked +execution task and saved routine status disagree. The previous heartbeat callback +scanned every five minutes, even with no routines or only completed history. + +## Ownership and trigger + +`routine-run-status.ts` owns the projection, custody checks and repair transaction. +The worker loads actual mismatches in keyset pages of 50 at startup. It accepts issue hints from issue +creation, status/origin changes, checkout/release, routine issue association, +execution control writes, review state writes and subtree status restoration. +Bulk restoration registers each issue hint in the same transaction before its +grouped write. Restored history does not depend on an optional agent wake. It uses the existing +transaction-aware work notification protocol. An outer transaction owns the hint; +a savepoint release does not wake the repair worker before commit. + +An execution must retain reciprocal task/invocation links in the same company +and belong to a routine in that company. This is the existing routine status +owner contract. Origin labels can change when a plugin declaration changes or +is removed; retained reciprocal links preserve historical execution custody. Repair +locks the routine and task and rechecks the linked invocation before writing. +Direct route hooks also lock the execution task and linked invocation in their +own transaction, nested under caller transactions when present. An older hook +cannot write after a newer task status has committed. It preserves skipped/coalesced history and unrelated dispatch failures. A task +that resumes clears an execution-task transient failure through the same owner +used by native completion and issue routes. + +`activeIssue` remains derived from open tasks and live heartbeat runs. A terminal +heartbeat with an open task does not create a completion obligation. The worker +does not retry merely because a run is active or a routine exists. + +## Lifecycle and writer contract + +`registerScopedDeadlineWork` owns coalescing, fair batches, failure backoff, drain +and shutdown. A failed owner retries with bounded backoff. Successful settlement +removes that owner. Settled configuration, healthy active tasks and completed +history leave no recurring worker SQL. The existing Cloud and heartbeat scheduler +enablement gates remain in place. Ordinary routine scheduling is unchanged. + +Supported app writers register `notifyRoutineExecutionStatus(tx, issueId)` before +their mutation. Common execution writers without an issue hint request debt reconstruction only +for their recorded company. Projection debt settles even when that company is +paused; company activity does not create or cancel this obligation. Backups and independent SQL import/repair tools must restore +before startup or call that same transaction contract; process-local hints do +not observe independent SQL or another process. No permanent listener or periodic +configured-routine scan is added. + +The in-process retry deadline does not establish external Cloud wake coverage. +This change preserves the existing host sleep safety checks and does not claim +measured host or database suspension. + +## Acceptance + +Tests exercise ordinary issue writes and a real heartbeat lifecycle transition +against PostgreSQL with the production coordinator. Provider wakeup is simulated. +They inspect durable routine rows and the normal routine detail read separately. +Coverage includes empty/configured/history quiet periods, first producer, +terminal outcomes and reopen, legacy mismatch reconstruction, skipped/coalesced +custody, outer commit/rollback, retry/restart, drain/resume and standby/disable. +Focused worker tests exercise in-flight invalidations, retry isolation and stop. diff --git a/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts b/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts index 52d434a3078..484cf55e8ff 100644 --- a/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts +++ b/server/src/__tests__/heartbeat-workspace-branch-containment.test.ts @@ -1,3 +1,5 @@ +import { subscribeDatabaseWork } from "@paperclipai/db"; +import { DELIVERY_QUEUES } from "../services/delivery-work-notifications.js"; import { EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db"; import { execFile } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; @@ -888,7 +890,7 @@ describeEmbeddedPostgres("heartbeat workspace branch containment", () => { const tempRoots: string[] = []; beforeAll(async () => { - if (externalTestDatabaseUrl) db = createDb(externalTestDatabaseUrl); + if (externalTestDatabaseUrl) db = createDb(externalTestDatabaseUrl, { maxConnections: 6 }); else { tempDb = await startEmbeddedPostgresTestDatabase("paperclip-branch-containment-"); db = createDb(tempDb.connectionString); @@ -1173,15 +1175,15 @@ describeEmbeddedPostgres("heartbeat workspace branch containment", () => { }, }); + const routineEvents: string[] = []; + const unsubscribeRoutine = subscribeDatabaseWork(db, DELIVERY_QUEUES.routineExecutionStatus, event => routineEvents.push(event)); const heartbeat = heartbeatService(db); - const run = await heartbeat.wakeup(agentId, { - source: "assignment", - triggerDetail: "system", - reason: "issue_assigned", - payload: { issueId }, - contextSnapshot: { issueId, wakeReason: "issue_assigned" }, - }); - + let run; + try { + run = await heartbeat.wakeup(agentId, { source: "assignment", triggerDetail: "system", reason: "issue_assigned", + payload: { issueId }, contextSnapshot: { issueId, wakeReason: "issue_assigned" } }); + } finally { unsubscribeRoutine(); } + expect(routineEvents).toContain("intent"); expect(routineEvents).toContain("settled"); expect(run).toBeNull(); expect(adapterExecute).not.toHaveBeenCalled(); diff --git a/server/src/__tests__/routine-execution-status-work.test.ts b/server/src/__tests__/routine-execution-status-work.test.ts new file mode 100644 index 00000000000..0a6ff9339cb --- /dev/null +++ b/server/src/__tests__/routine-execution-status-work.test.ts @@ -0,0 +1,314 @@ +import { issueTreeControlService } from "../services/issue-tree-control.js"; +import { writeExecutionControl } from "../services/execution-control-notifications.js"; +import { DELIVERY_QUEUES } from "../services/delivery-work-notifications.js"; +import { randomUUID } from "node:crypto"; +import { eq, sql } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { agents, companies, createDb, heartbeatRuns, issues, pluginManagedResources, plugins, routineRuns, type Db } from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; +import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js"; +import { notifyRoutineExecutionStatus } from "../services/routine-execution-status-notifications.js"; +import { registerRoutineExecutionStatusWork } from "../services/routine-execution-status-work.js"; +import { routineExecutionStatusService } from "../services/routine-run-status.js"; +import { routineService } from "../services/routines.js"; +import { issueService } from "../services/issues.js"; +import { idleWorkSnapshot } from "../services/task-admission.js"; +import { createHeartbeatLifecycle, type HeartbeatLifecycleDependencies } from "../services/heartbeat/run-lifecycle.js"; +import { createHeartbeatRunState } from "../services/heartbeat/run-state.js"; +import { createHeartbeatRunPreparation } from "../services/heartbeat/run-preparation.js"; + +const external = resolveExternalTestDatabaseUrl("PAPERCLIP_ROUTINE_STATUS_WORK_TEST_DATABASE_URL"); +const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport(); +(support.supported ? describe : describe.skip)("routine execution status work with PostgreSQL", () => { + let db: Db, temporary: Awaited> | undefined; + let coordinator: ReturnType | undefined; + let enabled = true, draining = false, standby = false; + const errors: unknown[] = []; + beforeAll(async () => { + if (!external) temporary = await startEmbeddedPostgresTestDatabase("routine-status-work-"); + db = createDb(external ?? temporary!.connectionString, { maxConnections: 6 }); + }, 60_000); + beforeEach(() => { + enabled = true; draining = false; standby = false; errors.length = 0; + vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout"], shouldAdvanceTime: true }); + }); + afterEach(async () => { + await coordinator?.stop(); coordinator = undefined; + vi.useRealTimers(); vi.restoreAllMocks(); + await db.execute(sql`SET client_min_messages = warning; TRUNCATE companies CASCADE`); + }); + afterAll(async () => { await db?.$client.end(); await temporary?.cleanup(); }); + const canRun = () => enabled && !draining && !standby; + async function start(service = routineExecutionStatusService(db)) { + coordinator = createDeliveryWorkCoordinator({ owner: db, canRun, canReconcile: () => !standby, + onError: error => errors.push(error) }); + const worker = registerRoutineExecutionStatusWork(coordinator, service, canRun); + await worker.ready; + return worker; + } + async function settled() { await vi.waitFor(() => expect(idleWorkSnapshot().active).toBe(0)); } + async function quiet() { + await settled(); + const select = vi.spyOn(db, "select"), execute = vi.spyOn(db, "execute"), transaction = vi.spyOn(db, "transaction"); + try { + await vi.advanceTimersByTimeAsync(20 * 60_000); + expect(select).not.toHaveBeenCalled(); expect(execute).not.toHaveBeenCalled(); expect(transaction).not.toHaveBeenCalled(); + } finally { select.mockRestore(); execute.mockRestore(); transaction.mockRestore(); } + } + async function fixture() { + const companyId = randomUUID(), agentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Routine status work", issuePrefix: `R${companyId.slice(0, 6)}`, + defaultResponsibleUserId: randomUUID(), requireBoardApprovalForNewAgents: false }); + await db.insert(agents).values({ id: agentId, companyId, name: "Routine worker", status: "active", lifecycleState: "ready", adapterType: "codex_local" }); + const service = routineService(db, { heartbeat: { wakeup: async () => null }, runtimeEnv: {} }); + const routine = await service.create(companyId, { title: "Status projection", assigneeAgentId: agentId, + priority: "medium", status: "active", concurrencyPolicy: "always_enqueue", catchUpPolicy: "skip_missed" }, {}); + return { companyId, agentId, routine, service }; + } + async function dispatch(f: Awaited>) { + return f.service.runRoutine(f.routine.id, { source: "manual", idempotencyKey: randomUUID() }); + } + async function saved(id: string) { return (await db.select().from(routineRuns).where(eq(routineRuns.id, id)))[0]; } + async function projected(id: string, status: string) { + await vi.waitFor(async () => expect((await saved(id)).status).toBe(status)); + await settled(); + } + + it("settles empty startup, configured idle, healthy active and retained terminal history without recurring SQL", async () => { + await start(); await quiet(); + const f = await fixture(); await quiet(); + const run = await dispatch(f); await quiet(); + await issueService(db).update(run.linkedIssueId!, { status: "done" }); + await projected(run.id, "completed"); + const before = await saved(run.id); await quiet(); + expect(await saved(run.id)).toEqual(before); expect(errors).toEqual([]); + }); + it.each(["done", "blocked", "cancelled"])("reactivates from the first ordinary %s transition and preserves input", async status => { + await start(); await quiet(); + const f = await fixture(), run = await dispatch(f); + await issueService(db).update(run.linkedIssueId!, { status }); + await projected(run.id, status === "done" ? "completed" : "failed"); + expect((await saved(run.id)).completedAt).toBeInstanceOf(Date); + await quiet(); expect(errors).toEqual([]); + if (status !== "done") { + await issueService(db).update(run.linkedIssueId!, { status: "in_progress" }); + await projected(run.id, "issue_created"); + expect(await saved(run.id)).toMatchObject({ completedAt: null, failureReason: null }); + await quiet(); + } + }); + it("reconstructs legacy terminal debt and reopened transient failures but leaves skipped and coalesced history unchanged", async () => { + const f = await fixture(), first = await dispatch(f), second = await dispatch(f); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, first.linkedIssueId!)); + await db.update(routineRuns).set({ status: "failed", failureReason: "Execution issue moved to blocked", completedAt: new Date() }).where(eq(routineRuns.id, second.id)); + const terminal = await db.insert(routineRuns).values(["skipped", "coalesced"].map(status => ({ companyId: f.companyId, + routineId: f.routine.id, source: "manual", linkedIssueId: first.linkedIssueId, status, completedAt: new Date() }))).returning(); + await start(); await projected(first.id, "completed"); await projected(second.id, "issue_created"); + for (const row of terminal) expect(await saved(row.id)).toEqual(row); + await quiet(); expect(errors).toEqual([]); + }); + it("settles the declared custom origin of a real plugin-managed dispatch and preserves other companies", async () => { + const f = await fixture(), other = await fixture(); + const pluginKey = `routine-status-${randomUUID()}`, originId = `operation:${randomUUID()}`; + const [plugin] = await db.insert(plugins).values({ pluginKey, packageName: pluginKey, version: "0.1.0", + manifestJson: { id: pluginKey, apiVersion: 1, version: "0.1.0", displayName: "Routine test", description: "Routine test", + author: "Paperclip", categories: ["automation"], capabilities: ["routines.managed"], entrypoints: { worker: "./worker.js" } } }).returning(); + await db.insert(pluginManagedResources).values({ companyId: f.companyId, pluginId: plugin.id, pluginKey, + resourceKind: "routine", resourceKey: "status", resourceId: f.routine.id, defaultsJson: { issueTemplate: { originId } } }); + const run = await dispatch(f), otherRun = await dispatch(other); + expect((await db.select().from(issues).where(eq(issues.id, run.linkedIssueId!)))[0].originId).toBe(originId); + await start(); await quiet(); + await issueService(db).update(run.linkedIssueId!, { status: "done" }); await projected(run.id, "completed"); + const before = await saved(run.id); + await db.transaction(async tx => { await notifyRoutineExecutionStatus(tx, run.linkedIssueId!); await notifyRoutineExecutionStatus(tx, run.linkedIssueId!); }); + await settled(); await quiet(); + expect(await saved(run.id)).toEqual(before); expect((await saved(otherRun.id)).status).toBe("issue_created"); expect(errors).toEqual([]); + }); + it.each(["changed", "removed"])("keeps historical custom-origin custody after its plugin declaration is %s", async mode => { + const f = await fixture(), pluginKey = `routine-history-${randomUUID()}`, originId = `operation:${randomUUID()}`; + const [plugin] = await db.insert(plugins).values({ pluginKey, packageName: pluginKey, version: "0.1.0", + manifestJson: { id: pluginKey, apiVersion: 1, version: "0.1.0", displayName: "Routine test", description: "Routine test", + author: "Paperclip", categories: ["automation"], capabilities: ["routines.managed"], entrypoints: { worker: "./worker.js" } } }).returning(); + const [resource] = await db.insert(pluginManagedResources).values({ companyId: f.companyId, pluginId: plugin.id, pluginKey, + resourceKind: "routine", resourceKey: "history", resourceId: f.routine.id, defaultsJson: { issueTemplate: { originId } } }).returning(); + const run = await dispatch(f); + expect((await db.select().from(issues).where(eq(issues.id, run.linkedIssueId!)))[0].originId).toBe(originId); + if (mode === "changed") await db.update(pluginManagedResources).set({ defaultsJson: { issueTemplate: { originId: "new-origin" } } }).where(eq(pluginManagedResources.id, resource.id)); + else await db.delete(pluginManagedResources).where(eq(pluginManagedResources.id, resource.id)); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, run.linkedIssueId!)); + await start(); await projected(run.id, "completed"); await quiet(); expect(errors).toEqual([]); + }); + it("reconstructs more than one page and does not lose a committed issue hint behind the startup cursor", async () => { + const f = await fixture(), ids = Array.from({ length: 53 }, () => randomUUID()).sort(); + const issueIds = ids.map(() => randomUUID()); + await db.insert(issues).values(ids.map((id, index) => ({ id: issueIds[index], companyId: f.companyId, title: "Legacy execution", + status: index === 0 ? "todo" : "done", originKind: "routine_execution", originRunId: id, originId: f.routine.id }))); + await db.insert(routineRuns).values(ids.map((id, index) => ({ id, companyId: f.companyId, routineId: f.routine.id, + source: "schedule", linkedIssueId: issueIds[index], status: "issue_created" }))); + const service = routineExecutionStatusService(db), load = service.load; + let entered!: () => void, release!: () => void; + const enteredGate = new Promise(resolve => { entered = resolve; }); + const releaseGate = new Promise(resolve => { release = resolve; }); + const loaded = vi.spyOn(service, "load").mockImplementationOnce(async (scope, afterId) => { + const page = await load(scope, afterId); expect(page).toHaveLength(50); entered(); await releaseGate; return page; + }); + const starting = start(service); await enteredGate; + await issueService(db).update(issueIds[0]!, { status: "done" }); release(); await starting; + await vi.advanceTimersByTimeAsync(3000); + await vi.waitFor(async () => expect((await db.select().from(routineRuns)).every(row => row.status === "completed")).toBe(true)); + expect(loaded.mock.calls.some(([scope, afterId]) => scope === undefined && afterId !== undefined)).toBe(true); + expect(loaded.mock.calls.some(([scope]) => scope?.issueId === issueIds[0])).toBe(true); + await quiet(); expect(errors).toEqual([]); + }); + it("scopes shared execution writes to their company and settles paused-company projection debt", async () => { + const f = await fixture(), other = await fixture(), run = await dispatch(f), otherRun = await dispatch(other); + const service = routineExecutionStatusService(db), load = vi.spyOn(service, "load"); + await start(service); await quiet(); load.mockClear(); + await db.update(companies).set({ status: "paused" }).where(eq(companies.id, f.companyId)); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, otherRun.linkedIssueId!)); + await writeExecutionControl(db, DELIVERY_QUEUES.executionStatus, + tx => tx.update(issues).set({ status: "done" }).where(eq(issues.id, run.linkedIssueId!)), { companyId: f.companyId }); + await projected(run.id, "completed"); + expect(load.mock.calls.every(([scope]) => scope?.companyId === f.companyId)).toBe(true); + expect((await saved(otherRun.id)).status).toBe("issue_created"); await quiet(); + await db.transaction(async tx => { await notifyRoutineExecutionStatus(tx, otherRun.linkedIssueId!); }); + await projected(otherRun.id, "completed"); await quiet(); expect(errors).toEqual([]); + }); + it("projects subtree cancel and restore without waking agents, then becomes quiet", async () => { + const f = await fixture(), root = await dispatch(f), child = await dispatch(f); + await issueService(db).update(child.linkedIssueId!, { parentId: root.linkedIssueId }); + const tree = issueTreeControlService(db), actor = { actorType: "system" as const, actorId: "routine-status-test" }; + await start(); await quiet(); + const cancel = await tree.createHold(f.companyId, root.linkedIssueId!, { mode: "cancel", actor }); + expect((await tree.cancelIssueStatusesForHold(f.companyId, root.linkedIssueId!, cancel.hold.id)).updatedIssueIds.sort()) + .toEqual([root.linkedIssueId, child.linkedIssueId].sort()); + await projected(root.id, "failed"); await projected(child.id, "failed"); await quiet(); + const restore = await tree.createHold(f.companyId, root.linkedIssueId!, { mode: "restore", actor }); + const restored = await tree.restoreIssueStatusesForHold(f.companyId, root.linkedIssueId!, restore.hold.id, { actor }); + expect(restored.updatedIssues).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: root.linkedIssueId, status: "todo" }), + expect.objectContaining({ id: child.linkedIssueId, status: "todo" }), + ])); + await projected(root.id, "issue_created"); await projected(child.id, "issue_created"); + expect(await saved(root.id)).toMatchObject({ completedAt: null, failureReason: null }); + expect(await saved(child.id)).toMatchObject({ completedAt: null, failureReason: null }); + expect(await db.select().from(heartbeatRuns)).toEqual([]); + await quiet(); expect(errors).toEqual([]); + }); + it("serializes a direct status hook with later task changes before projecting the routine", async () => { + const f = await fixture(), run = await dispatch(f); await start(); await quiet(); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, run.linkedIssueId!)); + let entered!: () => void, release!: () => void; + const enteredGate = new Promise(resolve => { entered = resolve; }); + const releaseGate = new Promise(resolve => { release = resolve; }); + const holding = db.transaction(async tx => { + await tx.select().from(routineRuns).where(eq(routineRuns.id, run.id)).for("update"); + entered(); await releaseGate; + }); + let direct: Promise | undefined, changing: Promise | undefined; + let directPid: number | undefined; + try { + await enteredGate; direct = f.service.syncRunStatusForIssue(run.linkedIssueId!); + await vi.waitFor(async () => { + const rows = await db.execute(sql`select pid from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock' + and (query like 'update "routine_runs"%' or query like 'select %"routine_runs"%')`); + expect(rows).toHaveLength(1); directPid = Number(rows[0]!.pid); + }, { timeout: 5000 }); + await expect(db.transaction(tx => tx.select({ id: issues.id }).from(issues) + .where(eq(issues.id, run.linkedIssueId!)).for("update", { noWait: true }))) + .rejects.toMatchObject({ cause: expect.objectContaining({ code: "55P03" }) }); + changing = issueService(db).update(run.linkedIssueId!, { status: "blocked" }); + await vi.waitFor(async () => { + const rows = await db.execute(sql`select count(*)::int as count from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock' + and ${directPid}::int = any(pg_blocking_pids(pid))`); + expect(Number(rows[0]?.count)).toBeGreaterThan(0); + }, { timeout: 5000 }); + release(); await holding; await direct; await changing; + await projected(run.id, "failed"); await quiet(); expect(errors).toEqual([]); + } finally { release(); await holding; await direct; await changing; } + }); + it("waits for the real outer transaction and forgets a rolled-back nested status write", async () => { + const f = await fixture(), run = await dispatch(f); await start(); await quiet(); + let release!: () => void, entered!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const written = new Promise(resolve => { entered = resolve; }); + const writing = db.transaction(async tx => { + await issueService(db).update(run.linkedIssueId!, { status: "done" }, tx); + entered(); await gate; + }); + try { await written; expect((await saved(run.id)).status).toBe("issue_created"); } + finally { release(); await writing; } + await projected(run.id, "completed"); await quiet(); + const other = await dispatch(f); + await expect(db.transaction(async tx => { + await issueService(db).update(other.linkedIssueId!, { status: "cancelled" }, tx); + throw new Error("outer rollback"); + })).rejects.toThrow("outer rollback"); + await settled(); expect((await saved(other.id)).status).toBe("issue_created"); await quiet(); expect(errors).toEqual([]); + }); + it("keeps the first write during drain and reconstructs on resume without passive SQL", async () => { + const f = await fixture(), run = await dispatch(f); await start(); await quiet(); + draining = true; + await issueService(db).update(run.linkedIssueId!, { status: "cancelled" }); + await settled(); await quiet(); expect((await saved(run.id)).status).toBe("issue_created"); + draining = false; await vi.advanceTimersByTimeAsync(30_000); await projected(run.id, "failed"); + await quiet(); expect(errors).toEqual([]); + }); + it.each(["standby", "disabled"])("does no owner SQL while %s and reconstructs when enabled", async mode => { + const f = await fixture(), run = await dispatch(f); + await issueService(db).update(run.linkedIssueId!, { status: "done" }); + if (mode === "standby") standby = true; else enabled = false; + await start(); await quiet(); expect((await saved(run.id)).status).toBe("issue_created"); + standby = false; enabled = true; await vi.advanceTimersByTimeAsync(30_000); + await projected(run.id, "completed"); await quiet(); expect(errors).toEqual([]); + }); + it("retries failed exact owners, retains the retry across drain and reconstructs after restart", async () => { + const f = await fixture(), run = await dispatch(f), service = routineExecutionStatusService(db); + const settle = vi.spyOn(service, "settle").mockRejectedValueOnce(new Error("projection unavailable")); + await start(service); + await issueService(db).update(run.linkedIssueId!, { status: "done" }); + await vi.waitFor(() => expect(errors).toHaveLength(1)); + expect((await saved(run.id)).status).toBe("issue_created"); + await vi.advanceTimersByTimeAsync(29_000); expect(settle).toHaveBeenCalledTimes(1); + draining = true; await vi.advanceTimersByTimeAsync(60_000); await quiet(); + draining = false; await vi.advanceTimersByTimeAsync(30_000); await projected(run.id, "completed"); await quiet(); + await coordinator!.stop(); coordinator = undefined; + const other = await dispatch(f); await issueService(db).update(other.linkedIssueId!, { status: "blocked" }); + await start(); await projected(other.id, "failed"); await quiet(); + }); + it("preserves notifications during a sweep, coalesces duplicate hints and never overlaps", async () => { + const f = await fixture(), first = await dispatch(f), second = await dispatch(f), service = routineExecutionStatusService(db); + let entered!: () => void, release!: () => void; + const enteredGate = new Promise(resolve => { entered = resolve; }); + const releaseGate = new Promise(resolve => { release = resolve; }); + const load = service.load; + vi.spyOn(service, "load").mockImplementationOnce(async issueId => { const rows = await load(issueId); entered(); await releaseGate; return rows; }); + const starting = start(service); await enteredGate; + await issueService(db).update(second.linkedIssueId!, { status: "done" }); + await db.transaction(async tx => { await notifyRoutineExecutionStatus(tx, second.linkedIssueId!); await notifyRoutineExecutionStatus(tx, second.linkedIssueId!); }); + release(); await starting; + await projected(second.id, "completed"); expect((await saved(first.id)).status).toBe("issue_created"); + await quiet(); expect(errors).toEqual([]); + }); + it.each(["succeeded", "failed", "cancelled", "interrupted", "timed_out"])("uses real %s run lifecycle changes for activeIssue without an open-issue retry", async status => { + const f = await fixture(), run = await dispatch(f); await start(); + const [execution] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, + status: "running", contextSnapshot: { issueId: run.linkedIssueId } }).returning(); + await db.update(issues).set({ executionRunId: execution.id }).where(eq(issues.id, run.linkedIssueId!)); + expect((await f.service.getDetail(f.routine.id))?.activeIssue?.id).toBe(run.linkedIssueId); + const deps = { + ...createHeartbeatRunState(db), ...createHeartbeatRunPreparation(db), + issuesSvc: { addComment: vi.fn(), listReviewAttention: vi.fn() }, + recovery: { escalateStrandedAssignedIssue: vi.fn() }, companySkills: { completeTestRunForIssue: vi.fn() }, + budgets: { getInvocationBlock: vi.fn(async () => null) }, treeControlSvc: { getActivePauseHoldGate: vi.fn(async () => null) }, + enqueueWakeup: vi.fn(), getCurrentUserRedactionOptions: vi.fn(async () => ({})), + getAgentInvokability: vi.fn(async () => ({ invokable: true })), emitTerminalAgentTaskRun: vi.fn(), budgetHooks: {}, + } satisfies HeartbeatLifecycleDependencies; + await createHeartbeatLifecycle(db, deps).setRunStatusIfRunning(execution.id, status, { finishedAt: new Date() }); + expect((await f.service.getDetail(f.routine.id))?.activeIssue).toBeNull(); + expect((await saved(run.id)).status).toBe("issue_created"); await quiet(); expect(errors).toEqual([]); + }); +}); diff --git a/server/src/__tests__/routine-execution-status-worker.test.ts b/server/src/__tests__/routine-execution-status-worker.test.ts new file mode 100644 index 00000000000..b62245ce3e2 --- /dev/null +++ b/server/src/__tests__/routine-execution-status-worker.test.ts @@ -0,0 +1,107 @@ +import type { RoutineExecutionStatusScope } from "../services/routine-execution-status-notifications.js"; +import { installDatabaseWorkSignals } from "../../../packages/db/src/work-signals.js"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js"; +import { registerRoutineExecutionStatusWork } from "../services/routine-execution-status-work.js"; + +let coordinator: ReturnType; +let canRun = true; +const errors: unknown[] = []; +const entry = { id: "run", companyId: "company", routineId: "routine", issueId: "issue" }; +beforeEach(() => { + vi.useFakeTimers(); canRun = true; errors.length = 0; + const owner = installDatabaseWorkSignals({ execute: vi.fn(async () => []) }); + coordinator = createDeliveryWorkCoordinator({ owner, canRun: () => canRun, canReconcile: () => true, + onError: error => errors.push(error) }); +}); +afterEach(async () => { await coordinator.stop(); vi.useRealTimers(); }); + +it("loads actual debt once and leaves no recurring owner work", async () => { + const service = { load: vi.fn(async () => []), settle: vi.fn(async () => true) }; + await registerRoutineExecutionStatusWork(coordinator, service, () => canRun).ready; + expect(service.load).toHaveBeenCalledExactlyOnceWith(undefined, undefined); + expect(coordinator.nextWakeAt()).toBeNull(); + await vi.advanceTimersByTimeAsync(20 * 60_000); + expect(service.load).toHaveBeenCalledOnce(); expect(service.settle).not.toHaveBeenCalled(); expect(errors).toEqual([]); +}); + +it("reconstructs only an invalidated issue and coalesces duplicate requests", async () => { + const service = { load: vi.fn(async (_scope?: RoutineExecutionStatusScope, _afterId?: string) => [] as typeof entry[]), settle: vi.fn(async () => true) }; + const worker = registerRoutineExecutionStatusWork(coordinator, service, () => canRun); await worker.ready; + service.load.mockResolvedValueOnce([entry]); + worker.invalidate(JSON.stringify({ issueId: "issue" })); worker.invalidate(JSON.stringify({ issueId: "issue" })); + await vi.advanceTimersByTimeAsync(1000); + expect(service.load).toHaveBeenLastCalledWith({ issueId: "issue" }, undefined); expect(service.load).toHaveBeenCalledTimes(2); + expect(service.settle).toHaveBeenCalledExactlyOnceWith(expect.objectContaining(entry)); + expect(coordinator.nextWakeAt()).toBeNull(); expect(errors).toEqual([]); +}); + +it("retries only the failed owner at backoff and disarms after success", async () => { + const service = { load: vi.fn(async () => [entry]), settle: vi.fn(async () => true).mockRejectedValueOnce(new Error("locked")) }; + await registerRoutineExecutionStatusWork(coordinator, service, () => canRun).ready; + expect(errors).toHaveLength(1); + const retryAt = coordinator.nextWakeAt(); expect(retryAt).toBe(Date.now() + 30_000); + await vi.advanceTimersByTimeAsync(29_999); expect(service.settle).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); expect(service.settle).toHaveBeenCalledTimes(2); + expect(service.load).toHaveBeenCalledOnce(); expect(coordinator.nextWakeAt()).toBeNull(); + await vi.advanceTimersByTimeAsync(20 * 60_000); expect(service.settle).toHaveBeenCalledTimes(2); +}); + +it("retains startup and first work during drain without passive owner calls", async () => { + canRun = false; + const service = { load: vi.fn(async () => [entry]), settle: vi.fn(async () => true) }; + const worker = registerRoutineExecutionStatusWork(coordinator, service, () => canRun); await worker.ready; + worker.invalidate(JSON.stringify({ issueId: "issue" })); + await vi.advanceTimersByTimeAsync(20 * 60_000); + expect(service.load).not.toHaveBeenCalled(); expect(service.settle).not.toHaveBeenCalled(); + canRun = true; await vi.advanceTimersByTimeAsync(30_000); + expect(service.settle).toHaveBeenCalledOnce(); expect(coordinator.nextWakeAt()).toBeNull(); expect(errors).toEqual([]); +}); + +it("keeps a failed owner across drain and retries after resume", async () => { + const service = { load: vi.fn(async () => [entry]), settle: vi.fn(async () => true).mockRejectedValueOnce(new Error("write failed")) }; + await registerRoutineExecutionStatusWork(coordinator, service, () => canRun).ready; + canRun = false; await vi.advanceTimersByTimeAsync(20 * 60_000); + expect(service.settle).toHaveBeenCalledOnce(); expect(service.load).toHaveBeenCalledOnce(); + canRun = true; await vi.advanceTimersByTimeAsync(30_000); + expect(service.settle).toHaveBeenCalledTimes(2); expect(coordinator.nextWakeAt()).toBeNull(); +}); + +it("retains a notification that arrives while the startup reconstruction is in flight", async () => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const service = { load: vi.fn(async (_scope?: RoutineExecutionStatusScope, _afterId?: string) => [] as typeof entry[]), settle: vi.fn(async () => true) }; + service.load.mockImplementationOnce(async () => { await gate; return []; }); + const worker = registerRoutineExecutionStatusWork(coordinator, service, () => canRun); + await vi.waitFor(() => expect(service.load).toHaveBeenCalledOnce()); + worker.invalidate(JSON.stringify({ issueId: "issue" })); service.load.mockResolvedValueOnce([entry]); release(); await worker.ready; + await vi.advanceTimersByTimeAsync(1000); + expect(service.settle).toHaveBeenCalledOnce(); expect(coordinator.nextWakeAt()).toBeNull(); expect(errors).toEqual([]); +}); + +it("cancels retries and subscriptions at shutdown", async () => { + const service = { load: vi.fn(async () => [entry]), settle: vi.fn(async () => true).mockRejectedValue(new Error("offline")) }; + const worker = registerRoutineExecutionStatusWork(coordinator, service, () => canRun); await worker.ready; + await worker.stop(); worker.invalidate(JSON.stringify({ issueId: "issue" })); await vi.advanceTimersByTimeAsync(20 * 60_000); + expect(service.settle).toHaveBeenCalledOnce(); expect(coordinator.nextWakeAt()).toBeNull(); +}); + + +it("reconstructs a backlog in bounded keyset pages while retaining a scoped hint behind the cursor", async () => { + const firstPage = Array.from({ length: 50 }, (_, i) => ({ ...entry, id: `run-${String(i).padStart(3, "0")}` })); + const late = { ...entry, id: "run-late", issueId: "late-issue" }; + const service = { + load: vi.fn(async (scope?: RoutineExecutionStatusScope, afterId?: string) => scope?.issueId ? [late] + : afterId ? [{ ...entry, id: "run-050" }] : firstPage), + settle: vi.fn(async () => true), + }; + const worker = registerRoutineExecutionStatusWork(coordinator, service, () => canRun); await worker.ready; + expect(service.load).toHaveBeenCalledExactlyOnceWith(undefined, undefined); + worker.invalidate(JSON.stringify({ issueId: "late-issue" })); + await vi.advanceTimersByTimeAsync(3000); + expect(service.load).toHaveBeenCalledWith(undefined, "run-049"); + expect(service.load).toHaveBeenCalledWith({ issueId: "late-issue" }, undefined); + expect(service.settle).toHaveBeenCalledTimes(52); + expect(service.settle).toHaveBeenCalledWith(expect.objectContaining(late)); + expect(coordinator.nextWakeAt()).toBeNull(); expect(errors).toEqual([]); +}); diff --git a/server/src/__tests__/routine-status-reconciliation.test.ts b/server/src/__tests__/routine-status-reconciliation.test.ts index eba0718ce25..343c6fb38b6 100644 --- a/server/src/__tests__/routine-status-reconciliation.test.ts +++ b/server/src/__tests__/routine-status-reconciliation.test.ts @@ -1,3 +1,4 @@ +import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; import { randomUUID } from "node:crypto"; import { eq, sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; @@ -5,11 +6,12 @@ import { companies, createDb, issues, routineRuns, routines } from "@paperclipai import { routineService } from "../services/routines.js"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; -const support = await getEmbeddedPostgresTestSupport(); +const external = resolveExternalTestDatabaseUrl("PAPERCLIP_ROUTINE_STATUS_TEST_DATABASE_URL"); +const support = external ? { supported: true, reason: undefined } : await getEmbeddedPostgresTestSupport(); if (!support.supported) console.warn(`Skipping routine status database tests: ${support.reason}`); (support.supported ? describe : describe.skip)("routine status reconciliation", () => { let db: ReturnType, database: Awaited>; - beforeAll(async () => { database = await startEmbeddedPostgresTestDatabase("routine-status-"); db = createDb(database.connectionString); }, 60_000); + beforeAll(async () => { if (!external) database = await startEmbeddedPostgresTestDatabase("routine-status-"); db = createDb(external ?? database.connectionString, { maxConnections: 6 }); }, 60_000); afterEach(async () => { vi.restoreAllMocks(); await db.execute(sql`SET client_min_messages = warning; TRUNCATE companies CASCADE`); }); afterAll(async () => { await db?.$client.end(); await database?.cleanup(); }); async function seed(status = "blocked") { @@ -36,11 +38,10 @@ if (!support.supported) console.warn(`Skipping routine status database tests: ${ expect((await db.select().from(routineRuns))[0]).toMatchObject({ status: "issue_created", completedAt: null }); } }); - it.each(["active_issue", "wrong_origin", "wrong_routine", "wrong_run", "wrong_company", "wrong_routine_company", "different_status"])("does not settle an unsupported handoff: %s", async mode => { + it.each(["active_issue", "wrong_origin", "wrong_run", "wrong_company", "wrong_routine_company", "different_status"])("does not settle an unsupported handoff: %s", async mode => { const f = await seed(); if (mode === "active_issue") await db.update(issues).set({ status: "in_progress" }).where(eq(issues.id, f.issue.id)); if (mode === "wrong_origin") await db.update(issues).set({ originKind: "manual" }).where(eq(issues.id, f.issue.id)); - if (mode === "wrong_routine") await db.update(issues).set({ originId: randomUUID() }).where(eq(issues.id, f.issue.id)); if (mode === "wrong_run") await db.update(issues).set({ originRunId: randomUUID() }).where(eq(issues.id, f.issue.id)); if (mode === "wrong_company") { const other = await seed("done"); await db.update(routineRuns).set({ companyId: other.companyId }).where(eq(routineRuns.id, f.run.id)); } if (mode === "wrong_routine_company") { const other = await seed("done"); @@ -53,6 +54,20 @@ if (!support.supported) console.warn(`Skipping routine status database tests: ${ expect((await db.select().from(routineRuns)).find(row => row.id === f.run.id)).toEqual(before.find(row => row.id === f.run.id)); if (mode === "wrong_routine_company") await expect(routineService(db).syncRunStatusForIssue(f.issue.id)).resolves.toBeNull(); }); + it("uses reciprocal durable custody when a retained origin no longer names the current routine declaration", async () => { + const f = await seed("done"); + await db.update(issues).set({ originId: "previous-declaration-origin" }).where(eq(issues.id, f.issue.id)); + expect(await routineService(db).reconcileExecutionRunStatuses()).toEqual({ scanned: 1, settled: 1, deferred: 0 }); + expect((await db.select().from(routineRuns).where(eq(routineRuns.id, f.run.id)))[0].status).toBe("completed"); + }); + it.each(["unknown-status", "other-code"])("preserves a failed invocation with an unrelated transient marker: %s", async mode => { + const f = await seed("in_progress"); + await db.update(routineRuns).set({ status: "failed", failureReason: null, + triggerPayload: { transientFailure: { code: mode === "other-code" ? "unrelated_failure" : "execution_issue_status", + status: mode === "unknown-status" ? "unrecognized" : "blocked" } } }).where(eq(routineRuns.id, f.run.id)); + expect(await routineService(db).reconcileExecutionRunStatuses()).toEqual({ scanned: 0, settled: 0, deferred: 0 }); + expect((await db.select().from(routineRuns).where(eq(routineRuns.id, f.run.id)))[0].status).toBe("failed"); + }); it.each(["resume", "retarget", "routine_transfer"])("rechecks %s after candidate selection and before repair", async mode => { const f = await seed(); const transaction = db.transaction.bind(db); vi.spyOn(db, "transaction").mockImplementationOnce(async callback => { diff --git a/server/src/__tests__/server-startup-feedback-export.test.ts b/server/src/__tests__/server-startup-feedback-export.test.ts index 65efb679fc1..cf49503dfbf 100644 --- a/server/src/__tests__/server-startup-feedback-export.test.ts +++ b/server/src/__tests__/server-startup-feedback-export.test.ts @@ -1,3 +1,4 @@ +import { routineExecutionStatusService } from "../services/routine-run-status.js"; import { attentionRetentionService } from "../services/attention-retention.js"; import { statusCardEvaluationService, registerStatusCardEvaluationWork } from "../services/status-card-evaluation.js"; import { nativeHarnessBackupExpirationService } from "../services/native-runtime/native-harness-backup-expiration.js"; @@ -402,6 +403,11 @@ vi.mock("../services/approval-retention.js", async importOriginal => ({ approvalRetentionService: vi.fn(() => ({ sweep: vi.fn(async () => ({ archived: 0 })), next: vi.fn(async () => null) })), })); +vi.mock("../services/routine-run-status.js", async importOriginal => ({ + ...await importOriginal(), + routineExecutionStatusService: vi.fn(() => ({ load: vi.fn(async () => []), settle: vi.fn(async () => false) })), +})); + vi.mock("../services/index.js", () => ({ backfillLegacyToolOAuthTokens: vi.fn(async () => ({ scannedConnections: 0, @@ -1427,6 +1433,27 @@ describe("startServer feedback export wiring", () => { } }); + it.each([{ cloud: false, enabled: true }, { cloud: true, enabled: false }, { cloud: true, enabled: true }])( + "registers routine status only for enabled Cloud scheduling ($cloud/$enabled)", async ({ cloud, enabled }) => { + if (cloud) vi.stubEnv("PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN", "startup-test-token"); + else vi.stubEnv("PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN", undefined); + loadConfigMock.mockReturnValue(buildTestConfig({ heartbeatSchedulerEnabled: enabled })); + const intervalCallbacks: Array<() => void> = []; + const intervals = vi.spyOn(globalThis, "setInterval").mockImplementation(((callback: () => void) => { + intervalCallbacks.push(callback); return 1 as unknown as ReturnType; + }) as typeof setInterval); + try { + await startServer(); await settleStartupHousekeeping(); + expect(deliveryWorkRegisterMock.mock.calls.filter(([queue]) => queue === DELIVERY_QUEUES.routineExecutionStatus)).toHaveLength(cloud && enabled ? 1 : 0); + const service = vi.mocked(routineExecutionStatusService).mock.results.at(-1)?.value; + if (cloud && enabled) expect(service?.load).toHaveBeenCalledExactlyOnceWith(undefined, undefined); + for (let index = 0; index < 4; index++) for (const callback of intervalCallbacks) callback(); + await settleStartupHousekeeping(); + if (cloud && enabled) expect(service?.load).toHaveBeenCalledOnce(); + } finally { intervals.mockRestore(); vi.unstubAllEnvs(); } + }, + ); + it("keeps routine ticks and setup cleanup active when heartbeat scheduling is suppressed", async () => { loadConfigMock.mockReturnValue(buildTestConfig({ heartbeatSchedulerEnabled: true, diff --git a/server/src/__tests__/status-card-evaluation-work.test.ts b/server/src/__tests__/status-card-evaluation-work.test.ts index d980c03249d..757d5b28b23 100644 --- a/server/src/__tests__/status-card-evaluation-work.test.ts +++ b/server/src/__tests__/status-card-evaluation-work.test.ts @@ -352,15 +352,19 @@ const support = external ? { supported: true } : await getEmbeddedPostgresTestSu await flush(); } await flush(); - // One eager reconciliation after idle and one bounded batched continuation; - // the producers themselves are not counted as background SQL. - const dueReads = () => select.mock.calls.filter(([fields]) => fields && "card" in fields && fields.card === statusCards).length; + // One eager reconciliation after idle and one bounded batched continuation. + // Each pass reads candidates once; evaluating its one changed card also + // reads that card's revision for generation CAS. Bound both query classes + // independently; the producers are not counted as background SQL. + const dueReads = () => select.mock.calls.filter(([fields]) => fields && "card" in fields && fields.card === statusCards && !("revision" in fields)).length; + const revisionReads = () => select.mock.calls.filter(([fields]) => fields && "card" in fields && fields.card === statusCards && "revision" in fields).length; const nextReads = () => select.mock.calls.filter(([fields]) => fields && "at" in fields && fields.at === statusCards.nextEvalAt).length; const elapsedMs = Date.now() - burstStarted; const maximum = Math.ceil(elapsedMs / 1000) + 2; - console.info("status-card burst", { elapsedMs, maximum, runs: run.mock.calls.length, due: dueReads(), next: nextReads(), searches: searchedCompanies.length }); + console.info("status-card burst", { elapsedMs, maximum, runs: run.mock.calls.length, due: dueReads(), revisions: revisionReads(), next: nextReads(), searches: searchedCompanies.length }); expect(run.mock.calls.length).toBeLessThanOrEqual(maximum); expect(dueReads()).toBeLessThanOrEqual(maximum); expect(nextReads()).toBeLessThanOrEqual(maximum); + expect(revisionReads()).toBeLessThanOrEqual(maximum); expect(searchedCompanies.length).toBeLessThanOrEqual(maximum); expect(searchedCompanies.length).toBeGreaterThan(0); expect(new Set(searchedCompanies)).toEqual(new Set([originalCompany])); diff --git a/server/src/index.ts b/server/src/index.ts index d7122c1fcaa..c6e420b2748 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -1,3 +1,5 @@ +import { registerRoutineExecutionStatusWork } from "./services/routine-execution-status-work.js"; +import { routineExecutionStatusService } from "./services/routine-run-status.js"; import { attentionRetentionService, registerAttentionRetentionWork } from "./services/attention-retention.js"; import { registerProviderTraceRetentionWork } from "./services/provider-trace-retention-work.js"; import { registerRunEvidenceMigrationWork } from "./services/run-evidence-migration-work.js"; @@ -1560,15 +1562,6 @@ async function startServerWithDatabaseTeardown( heartbeat.drainActiveRunExecutions(); prepareHotRestartShutdown = heartbeat.prepareHotRestartShutdown; const routines = routineService(db as any, { pluginWorkerManager }); - let nextRoutineStatusReconcile = 0; - const scheduleRoutineStatusReconcile = () => { - if (!isCloudManagedInstance() || isWarmStandby() || isIdleTaskDrainActive() || heartbeatSchedulerStopped || - Date.now() < nextRoutineStatusReconcile) return; - nextRoutineStatusReconcile = Date.now() + 5 * 60_000; - trackHeartbeatSchedulerWork(routines.reconcileExecutionRunStatuses().then(result => { - if (result.settled > 0) logger.info(result, "reconciled routine execution issue status"); - }).catch(() => logger.warn("routine execution status reconciliation deferred"))); - }; const statusCardEvaluation = statusCardEvaluationService(db as any, heartbeat); const issues = issueService(db as any); const mergedPullRequestConfirmations = issueThreadInteractionService(db as any, { @@ -1941,7 +1934,6 @@ async function startServerWithDatabaseTeardown( // captures the authoritative set of running heartbeat rows. trackHeartbeatSchedulerWork((async () => { if (isIdleTaskDrainActive() || heartbeatSchedulerStopped) return; - scheduleRoutineStatusReconcile(); const sweptRuntimeStatuses = heartbeat.sweepExpiredRuntimeStatuses(); if (sweptRuntimeStatuses > 0) { logger.info( @@ -1982,6 +1974,11 @@ async function startServerWithDatabaseTeardown( })); }; const startHeartbeatScheduler = () => { + if (isCloudManagedInstance()) { + const status = registerRoutineExecutionStatusWork(deliveryWork, routineExecutionStatusService(db as any), + () => !isIdleTaskDrainActive() && !isWarmStandby() && !heartbeatSchedulerStopped); + trackHeartbeatSchedulerWork(status.ready); + } const scheduled = registerScheduledWork(deliveryWork, { retryMs: config.heartbeatSchedulerIntervalMs, tickAgentTimers: async signal => { diff --git a/server/src/services/delivery-work-notifications.ts b/server/src/services/delivery-work-notifications.ts index db41058a201..654244eb1bf 100644 --- a/server/src/services/delivery-work-notifications.ts +++ b/server/src/services/delivery-work-notifications.ts @@ -127,6 +127,7 @@ export const DELIVERY_QUEUES = { pluginLifecycle: "plugin-resource-lifecycle", toolAction: "tool-action-receipts", terminalWorkspaces: "terminal-workspaces", + routineExecutionStatus: "routine-execution-status", routineSchedules: "routine-schedules", issueMonitors: "issue-monitors", agentTimers: "agent-timers", diff --git a/server/src/services/execution-control-notifications.ts b/server/src/services/execution-control-notifications.ts index fa00196bf41..431ff1210e2 100644 --- a/server/src/services/execution-control-notifications.ts +++ b/server/src/services/execution-control-notifications.ts @@ -1,3 +1,4 @@ +import { notifyRoutineExecutionStatus } from "./routine-execution-status-notifications.js"; import { notifyAttentionRetentionWork } from "./retention-work-notifications.js"; import { notifyStatusCardIssueChange } from "./status-card-notifications.js"; import { notifyDeferredExecutionEligibility } from "./deferred-execution-notifications.js"; @@ -8,6 +9,7 @@ import { DELIVERY_QUEUES, notifyDeliveryWork, type DeliveryQueue } from "./deliv export type ExecutionWorkScope = { companyId: string; agentId?: string; runId?: string; issueId?: string }; export async function notifyExecutionRunWork(transaction: object, scope?: ExecutionWorkScope) { + if (scope) await notifyRoutineExecutionStatus(transaction, scope.issueId, scope.companyId); if (scope) { await notifyAttentionRetentionWork(transaction, scope); await notifyStatusCardIssueChange(transaction, scope.companyId); @@ -27,6 +29,7 @@ export function writeExecutionControl( scope: ExecutionWorkScope | undefined, ): Promise { return db.transaction(async transaction => { + if (scope) await notifyRoutineExecutionStatus(transaction, scope.issueId, scope.companyId); if (scope) await notifyAttentionRetentionWork(transaction, scope); if (scope) await notifyStatusCardIssueChange(transaction, scope.companyId); await notifyRunDispatch(transaction, scope); diff --git a/server/src/services/execution-recovery-resolution.ts b/server/src/services/execution-recovery-resolution.ts index d56e7d926f1..a6f80f26ba0 100644 --- a/server/src/services/execution-recovery-resolution.ts +++ b/server/src/services/execution-recovery-resolution.ts @@ -1,3 +1,4 @@ +import { notifyRoutineExecutionStatus } from "./routine-execution-status-notifications.js"; import { notifyNativeStatusReassessment } from "./native-status-reassessment-notifications.js"; import { notifyUnsafeWorkspaceExportRecovery } from "./unsafe-workspace-export-notifications.js"; import { notifyChatResponseAvailability } from "./chat-response-availability.js"; @@ -596,6 +597,7 @@ export async function settleUnrecoverableExecutions( if (task.status !== "blocked") await notifyChatResponseAvailability(tx, { companyId: task.companyId, issueId: task.id }); await notifyDeliveryWork(tx, DELIVERY_QUEUES.issueMonitors); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, task.id, task.companyId); const [projected] = await tx .update(issues) .set({ diff --git a/server/src/services/heartbeat/queue.ts b/server/src/services/heartbeat/queue.ts index a2975bdb454..d5df95eaf81 100644 --- a/server/src/services/heartbeat/queue.ts +++ b/server/src/services/heartbeat/queue.ts @@ -1,3 +1,4 @@ +import { notifyRoutineExecutionStatus } from "../routine-execution-status-notifications.js"; import { notifyStatusCardIssueChange } from "../status-card-notifications.js"; import { PersistedExecutionWaitRefusal, writeDeferredExecutionWait } from "../deferred-execution-wait.js"; import { notifyDeferredExecutionWork } from "../deferred-execution-notifications.js"; @@ -3934,6 +3935,7 @@ export function createHeartbeatQueue(db: Db, dependencies: HeartbeatQueueDepende await notifyDeliveryWork(tx, DELIVERY_QUEUES.issueMonitors); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); if (issue.status !== "blocked") await notifyChatResponseAvailability(tx, { companyId: issue.companyId, issueId: issue.id }); + await notifyRoutineExecutionStatus(tx, issue.id, issue.companyId); await tx .update(issues) .set({ diff --git a/server/src/services/heartbeat/run-lifecycle.ts b/server/src/services/heartbeat/run-lifecycle.ts index 9d3b30367d2..8224f75208e 100644 --- a/server/src/services/heartbeat/run-lifecycle.ts +++ b/server/src/services/heartbeat/run-lifecycle.ts @@ -1045,7 +1045,8 @@ export function createHeartbeatLifecycle(db: Db, dependencies: HeartbeatLifecycl if (previous && isHeartbeatRunTerminalStatus(status)) await notifyRetainedNativeSessionCleanup(tx, { companyId: previous.companyId, runId: previous.id }); if (isHeartbeatRunTerminalStatus(status)) await notifyDeliveryWork(tx, DELIVERY_QUEUES.accounting, companyId); - await notifyExecutionRunWork(tx, companyId ? { companyId } : undefined); + await notifyExecutionRunWork(tx, previous ? { companyId: previous.companyId, runId: previous.id, + issueId: previous.nativeIssueId ?? previous.issueId ?? readNonEmptyString(parseObject(previous.contextSnapshot).issueId) ?? undefined } : undefined); if (previous) await notifyChatRunMilestones(tx, { companyId: previous.companyId, runId: previous.id, issueId: previous.nativeIssueId ?? previous.issueId ?? readNonEmptyString(parseObject(previous.contextSnapshot).issueId) ?? undefined }); return write(tx); diff --git a/server/src/services/issue-review-state-notifications.ts b/server/src/services/issue-review-state-notifications.ts index bf669ff9694..d1dff39e1ee 100644 --- a/server/src/services/issue-review-state-notifications.ts +++ b/server/src/services/issue-review-state-notifications.ts @@ -1,3 +1,4 @@ +import { notifyRoutineExecutionStatus } from "./routine-execution-status-notifications.js"; import { notifyStatusCardIssueChange } from "./status-card-notifications.js"; import { notifyRunDispatch } from "./run-dispatch-notifications.js"; import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notifications.js"; @@ -6,6 +7,7 @@ import { notifyNativeReviewResponse } from "./native-review-response-notificatio import { notifyNativeStatusReassessment } from "./native-status-reassessment-notifications.js"; export async function notifyIssueReviewState(tx: object, scope: { companyId: string; issueId?: string; runId?: string; agentId?: string }) { + await notifyRoutineExecutionStatus(tx, scope.issueId, scope.companyId); await notifyStatusCardIssueChange(tx, scope.companyId); await notifyRunDispatch(tx, scope); await notifyDeliveryWork(tx, DELIVERY_QUEUES.issueMonitors); diff --git a/server/src/services/issue-tree-control.ts b/server/src/services/issue-tree-control.ts index 730384118dc..3f2cf140237 100644 --- a/server/src/services/issue-tree-control.ts +++ b/server/src/services/issue-tree-control.ts @@ -1,3 +1,4 @@ +import { notifyRoutineExecutionStatus } from "./routine-execution-status-notifications.js"; import { notifyStatusCardIssueChange } from "./status-card-notifications.js"; import { notifyDeferredExecutionEligibility } from "./deferred-execution-notifications.js"; import { notifyIssueRecovery } from "./issue-recovery-notifications.js"; @@ -1014,6 +1015,7 @@ export function issueTreeControlService(db: Db) { for (const [status, issueIdsForStatus] of issueIdsByStatus) { if (issueIdsForStatus.length === 0) continue; for (const issueId of issueIdsForStatus) { + await notifyRoutineExecutionStatus(tx, issueId, companyId); await notifyNativeStatusReassessment(tx, { companyId, issueId }); await notifyTerminalWorkspaceWork(tx, `issue:${issueId}`); await notifyChatResponseAvailability(tx, { companyId, issueId }); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index d44926cfad5..1bbdacbbbc2 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -1,3 +1,4 @@ +import { notifyRoutineExecutionStatus } from "./routine-execution-status-notifications.js"; import { notifyIssueAttentionRetentionWork } from "./retention-work-notifications.js"; import { notifyStatusCardIssueChange } from "./status-card-notifications.js"; import { notifyDeferredExecutionEligibility } from "./deferred-execution-notifications.js"; @@ -10653,6 +10654,7 @@ export function issueService(db: Db) { values.id ??= randomUUID(); await notifyIssueAttentionRetentionWork(tx, { companyId, issueId: values.id, parentId: values.parentId, originKind: values.originKind, originId: values.originId }); await notifyStatusCardIssueChange(tx, companyId); + if (values.originKind === "routine_execution") await notifyRoutineExecutionStatus(tx, values.id, companyId); const [issue] = await tx.insert(issues).values(values).returning(); await attachOwnedDraftImages(tx, issue, issue.description, { userId: issueData.createdByUserId, agentId: issueData.createdByAgentId }); if (issue.visibility === "private" && !inheritedPrivateSubtree && issueData.createdByAgentId) { @@ -11549,6 +11551,7 @@ export function issueService(db: Db) { await notifyRunDispatch(tx, { companyId: receiptExisting.companyId }); await notifyDeferredExecutionEligibility(tx, { companyId: receiptExisting.companyId }); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, id); } if (patch.monitorNextCheckAt !== undefined || (receiptExisting.monitorNextCheckAt && (patch.status !== undefined || patch.assigneeAgentId !== undefined || patch.assigneeUserId !== undefined))) { @@ -11597,6 +11600,9 @@ export function issueService(db: Db) { await notifyNativeWorkspaceFinalization(tx, { companyId: receiptExisting.companyId, issueId: id }); await notifyNativeStatusReassessment(tx, { companyId: receiptExisting.companyId, issueId: id }); } + if (patch.originKind !== undefined || patch.originRunId !== undefined || patch.originId !== undefined) { + await notifyRoutineExecutionStatus(tx, id); + } const updated = await tx .update(issues) .set(patch) @@ -12020,6 +12026,7 @@ export function issueService(db: Db) { await notifyRunDispatch(tx, owner ? { companyId: owner.companyId } : undefined); if (owner) await notifyDeferredExecutionEligibility(tx, owner); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, id); removedIssue = await tx .delete(issues) .where(eq(issues.id, id)) @@ -12152,6 +12159,7 @@ export function issueService(db: Db) { await notifyRunDispatch(tx, { companyId: issueCompany.companyId }); await notifyDeferredExecutionEligibility(tx, { companyId: issueCompany.companyId }); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, id); return tx .update(issues) .set({ @@ -12288,6 +12296,7 @@ export function issueService(db: Db) { await notifyRunDispatch(tx, { companyId: issueCompany.companyId }); await notifyDeferredExecutionEligibility(tx, { companyId: issueCompany.companyId }); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, id); return tx .update(issues) .set(adoptionSet) @@ -12546,6 +12555,7 @@ export function issueService(db: Db) { await notifyRunDispatch(tx, { companyId: existing.companyId }); await notifyDeferredExecutionEligibility(tx, { companyId: existing.companyId }); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, id); const updated = await tx .update(issues) .set({ @@ -12610,6 +12620,7 @@ export function issueService(db: Db) { await notifyRunDispatch(tx, { companyId: existing.companyId }); await notifyDeferredExecutionEligibility(tx, { companyId: existing.companyId }); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, id); const updated = await tx .update(issues) .set(patch) diff --git a/server/src/services/native-runtime/native-routine-status.test.ts b/server/src/services/native-runtime/native-routine-status.test.ts index 668de83a4d5..415622ea2c3 100644 --- a/server/src/services/native-runtime/native-routine-status.test.ts +++ b/server/src/services/native-runtime/native-routine-status.test.ts @@ -1,3 +1,4 @@ +import { resolveExternalTestDatabaseUrl } from "../../__tests__/helpers/external-test-database.js"; import { randomUUID } from "node:crypto"; import { eq } from "drizzle-orm"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; @@ -8,13 +9,14 @@ import { routineService } from "../routines.js"; import { commitNativeStatusDecision } from "./status-decision-committer.js"; import { NATIVE_STATUS_ARBITER_POLICY_VERSION, type NativeStatusDecision } from "./status-arbiter.js"; -const support = await getEmbeddedPostgresTestSupport(); +const external = resolveExternalTestDatabaseUrl("PAPERCLIP_NATIVE_ROUTINE_STATUS_TEST_DATABASE_URL"); +const support = external ? { supported: true, reason: undefined } : await getEmbeddedPostgresTestSupport(); if (!support.supported) console.warn(`Skipping native routine status tests: ${support.reason}`); (support.supported ? describe : describe.skip)("native routine history status projection", () => { let db: ReturnType, database: Awaited>; const companyId = randomUUID(), agentId = randomUUID(); beforeAll(async () => { - database = await startEmbeddedPostgresTestDatabase("native-routine-status-"); db = createDb(database.connectionString); + if (!external) database = await startEmbeddedPostgresTestDatabase("native-routine-status-"); db = createDb(external ?? database.connectionString, { maxConnections: 6 }); await db.insert(companies).values({ id: companyId, name: "Routine status fixture", issuePrefix: "NRS" }); await db.insert(agents).values({ id: agentId, companyId, name: "Routine worker", adapterType: "paperclip_runner", status: "running" }); }, 60_000); diff --git a/server/src/services/native-runtime/native-workspace-reconstruction.test.ts b/server/src/services/native-runtime/native-workspace-reconstruction.test.ts index fcff7eb58ec..8afb9820bee 100644 --- a/server/src/services/native-runtime/native-workspace-reconstruction.test.ts +++ b/server/src/services/native-runtime/native-workspace-reconstruction.test.ts @@ -38,7 +38,7 @@ describe("reconstruction after confirmed native workspace loss", () => { const external = resolveExternalTestDatabaseUrl("PAPERCLIP_NATIVE_RECONSTRUCTION_TEST_DATABASE_URL"); if (external) { await applyPendingMigrations(external); - db = createDb(external); + db = createDb(external, { maxConnections: 6 }); return; } temporary = await startEmbeddedPostgresTestDatabase("workspace-reconstruction-"); @@ -144,17 +144,21 @@ describe("reconstruction after confirmed native workspace loss", () => { expect(await queue.nextTimerHeartbeatAt(agent)).toBeNull(); const events: string[] = []; const unsubscribe = subscribeDatabaseWork(db, DELIVERY_QUEUES.agentTimers, event => events.push(event)); + const routineEvents: string[] = []; + const unsubscribeRoutine = subscribeDatabaseWork(db, DELIVERY_QUEUES.routineExecutionStatus, event => routineEvents.push(event)); try { await db.transaction(async tx => { await reconstructNativeWorkspace({ ...f, db: tx as unknown as Db }); expect(events).toContain("intent"); expect(events).not.toContain("settled"); + expect(routineEvents).toContain("intent"); expect(routineEvents).not.toContain("settled"); expect((await db.select().from(issues).where(eq(issues.id, f.issueId)))[0].status).toBe("blocked"); }); expect(events).toContain("settled"); + expect(routineEvents).toContain("settled"); expect((await db.select().from(issues).where(eq(issues.id, f.issueId)))[0].status).toBe("todo"); expect(await queue.nextTimerHeartbeatAt(agent)).toBe(agent.createdAt.getTime() + 600_000); - } finally { unsubscribe(); } + } finally { unsubscribe(); unsubscribeRoutine(); } }); it("pins a historical cancelled workspace through public heartbeat startup without replaying its turn", async () => { diff --git a/server/src/services/native-runtime/native-workspace-reconstruction.ts b/server/src/services/native-runtime/native-workspace-reconstruction.ts index d616ed266ed..bc34cc3bb3c 100644 --- a/server/src/services/native-runtime/native-workspace-reconstruction.ts +++ b/server/src/services/native-runtime/native-workspace-reconstruction.ts @@ -1,3 +1,4 @@ +import { notifyRoutineExecutionStatus } from "../routine-execution-status-notifications.js"; import { notifyAttentionRetentionWork } from "../retention-work-notifications.js"; import { notifyStatusCardIssueChange } from "../status-card-notifications.js"; import { DELIVERY_QUEUES, notifyDeliveryWork } from "../delivery-work-notifications.js"; @@ -190,6 +191,7 @@ export async function reconstructNativeWorkspace(input: { }).where(eq(issueRecoveryActions.id, source.action.id)); await notifyDeliveryWork(tx, DELIVERY_QUEUES.issueMonitors); await notifyDeliveryWork(tx, DELIVERY_QUEUES.agentTimers); + await notifyRoutineExecutionStatus(tx, input.issueId, input.companyId); await notifyStatusCardIssueChange(tx, input.companyId); await tx.update(issues).set({ status: "todo", unblockDescriptor: null, blockedTransitionAt: null, blockedOwnerNotifiedAt: null, updatedAt: now }).where(eq(issues.id, issue.id)); diff --git a/server/src/services/routine-execution-status-notifications.ts b/server/src/services/routine-execution-status-notifications.ts new file mode 100644 index 00000000000..fb9df28dda9 --- /dev/null +++ b/server/src/services/routine-execution-status-notifications.ts @@ -0,0 +1,8 @@ +import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notifications.js"; + +export type RoutineExecutionStatusScope = { issueId?: string; companyId?: string }; + +export function notifyRoutineExecutionStatus(tx: object, issueId?: string, companyId?: string) { + return notifyDeliveryWork(tx, DELIVERY_QUEUES.routineExecutionStatus, + issueId || companyId ? JSON.stringify({ issueId, companyId } satisfies RoutineExecutionStatusScope) : "*"); +} diff --git a/server/src/services/routine-execution-status-work.ts b/server/src/services/routine-execution-status-work.ts new file mode 100644 index 00000000000..0a4437f89f2 --- /dev/null +++ b/server/src/services/routine-execution-status-work.ts @@ -0,0 +1,27 @@ +import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js"; +import { DELIVERY_QUEUES } from "./delivery-work-notifications.js"; +import { registerScopedDeadlineWork } from "./scoped-deadline-work.js"; +import type { RoutineExecutionStatusScope } from "./routine-execution-status-notifications.js"; +import { ROUTINE_EXECUTION_STATUS_PAGE_SIZE, type routineExecutionStatusService } from "./routine-run-status.js"; + +export function registerRoutineExecutionStatusWork( + coordinator: ReturnType, + service: ReturnType, + canRun: () => boolean, +) { + const cursors = new Map(); + return registerScopedDeadlineWork(coordinator, { + queue: DELIVERY_QUEUES.routineExecutionStatus, + decodeScope: hint => hint === "*" ? undefined : JSON.parse(hint) as RoutineExecutionStatusScope, + matches: (entry, scope?: RoutineExecutionStatusScope) => (!scope?.issueId || entry.issueId === scope.issueId) + && (!scope?.companyId || entry.companyId === scope.companyId), + load: async (scope?: RoutineExecutionStatusScope) => { + const key = JSON.stringify(scope) ?? "*"; + const rows = await service.load(scope, cursors.get(key)); + const complete = rows.length < ROUTINE_EXECUTION_STATUS_PAGE_SIZE; + if (complete) cursors.delete(key); else cursors.set(key, rows.at(-1)!.id); + return { rows: rows.map(row => ({ ...row, dueAt: Date.now() })), complete }; + }, + check: async entry => { await service.settle(entry); return null; }, + }, canRun); +} diff --git a/server/src/services/routine-run-status.ts b/server/src/services/routine-run-status.ts index 77341f9cba4..46a2bae4cf5 100644 --- a/server/src/services/routine-run-status.ts +++ b/server/src/services/routine-run-status.ts @@ -1,4 +1,5 @@ -import { and, eq } from "drizzle-orm"; +import { notifyRoutineExecutionStatus, type RoutineExecutionStatusScope } from "./routine-execution-status-notifications.js"; +import { and, eq, inArray, or, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { issues, routineRuns, routines } from "@paperclipai/db"; @@ -37,6 +38,7 @@ function legacyExecutionIssueTransientFailureStatus( } export async function finalizeRoutineRun(executor: Db, runId: string, patch: Partial) { + if (patch.linkedIssueId) await notifyRoutineExecutionStatus(executor, patch.linkedIssueId); return executor .update(routineRuns) .set({ @@ -49,6 +51,10 @@ export async function finalizeRoutineRun(executor: Db, runId: string, patch: Par } export async function syncRoutineRunStatusForIssue(executor: Db, issueId: string) { + return executor.transaction(tx => syncLockedRoutineRunStatusForIssue(tx as unknown as Db, issueId)); +} + +async function syncLockedRoutineRunStatusForIssue(executor: Db, issueId: string) { const issue = await executor .select({ id: issues.id, @@ -58,7 +64,8 @@ export async function syncRoutineRunStatusForIssue(executor: Db, issueId: string originRunId: issues.originRunId, }) .from(issues) - .where(eq(issues.id, issueId)) + .where(and(eq(issues.id, issueId), eq(issues.originKind, "routine_execution"))) + .for("update") .then((rows) => rows[0] ?? null); if (!issue || issue.originKind !== "routine_execution" || !issue.originRunId) return null; const run = await executor @@ -67,14 +74,17 @@ export async function syncRoutineRunStatusForIssue(executor: Db, issueId: string status: routineRuns.status, failureReason: routineRuns.failureReason, triggerPayload: routineRuns.triggerPayload, + completedAt: routineRuns.completedAt, }) .from(routineRuns) .innerJoin(routines, and(eq(routines.id, routineRuns.routineId), eq(routines.companyId, issue.companyId))) .where(and(eq(routineRuns.id, issue.originRunId), eq(routineRuns.companyId, issue.companyId), eq(routineRuns.linkedIssueId, issue.id))) + .for("update", { of: routineRuns }) .then((rows) => rows[0] ?? null); - if (!run) return null; + if (!run || ["received", "skipped", "coalesced"].includes(run.status)) return null; if (issue.status === "done") { + if (run.status === "completed" && run.failureReason === null && run.completedAt) return null; const transientFailureStatus = executionIssueTransientFailureStatusFromPayload(run.triggerPayload) ?? legacyExecutionIssueTransientFailureStatus(run.failureReason); const transientFailureClearedAt = executionIssueTransientFailureClearedAtFromPayload(run.triggerPayload); @@ -99,6 +109,7 @@ export async function syncRoutineRunStatusForIssue(executor: Db, issueId: string } if (issue.status === "blocked" || issue.status === "cancelled") { const failureReason = executionIssueTransientFailureReason(issue.status); + if (run.status === "failed" && run.failureReason === failureReason && run.completedAt) return null; return finalizeRoutineRun(executor, issue.originRunId, { status: "failed", failureReason, @@ -135,3 +146,57 @@ export async function syncRoutineRunStatusForIssue(executor: Db, issueId: string return null; } + +export const ROUTINE_EXECUTION_STATUS_PAGE_SIZE = 50; + +export function routineExecutionStatusDebtCondition() { + const transientFailure = sql`((${routineRuns.triggerPayload} -> 'transientFailure' ->> 'code' = ${EXECUTION_ISSUE_TRANSIENT_FAILURE_CODE} + and ${routineRuns.triggerPayload} -> 'transientFailure' ->> 'status' in ('blocked', 'cancelled')) + or ${routineRuns.failureReason} in ('Execution issue moved to blocked', 'Execution issue moved to cancelled'))`; + return or( + and(eq(routineRuns.status, "issue_created"), inArray(issues.status, ["done", "blocked", "cancelled"])), + and(eq(routineRuns.status, "completed"), or(inArray(issues.status, ["blocked", "cancelled"]), + and(eq(issues.status, "done"), sql`(${routineRuns.completedAt} is null or ${routineRuns.failureReason} is not null)`))), + and(eq(routineRuns.status, "failed"), transientFailure, + sql`(${issues.status} not in ('blocked', 'cancelled') + or ${routineRuns.completedAt} is null + or ${routineRuns.failureReason} is distinct from 'Execution issue moved to ' || ${issues.status})`), + ); +} + +export function routineExecutionStatusService(db: Db) { + async function load(scope?: RoutineExecutionStatusScope, afterId?: string) { + return db.select({ id: routineRuns.id, companyId: routineRuns.companyId, + issueId: issues.id, routineId: routineRuns.routineId }) + .from(routineRuns) + .innerJoin(routines, and(eq(routines.id, routineRuns.routineId), eq(routines.companyId, routineRuns.companyId))) + .innerJoin(issues, and(eq(issues.id, routineRuns.linkedIssueId), eq(issues.companyId, routineRuns.companyId), + eq(issues.originKind, "routine_execution"), sql`${issues.originRunId} = ${routineRuns.id}::text`)) + .where(and(routineExecutionStatusDebtCondition(), + ...(scope?.issueId ? [eq(issues.id, scope.issueId)] : []), + ...(scope?.companyId ? [eq(routineRuns.companyId, scope.companyId)] : []), + ...(afterId ? [sql`${routineRuns.id} > ${afterId}::uuid`] : []))) + .orderBy(routineRuns.id).limit(ROUTINE_EXECUTION_STATUS_PAGE_SIZE); + } + async function settle(candidate: Awaited>[number]) { + return db.transaction(async tx => { + await tx.execute(sql`SET LOCAL lock_timeout = '500ms'`); + await tx.execute(sql`SET LOCAL statement_timeout = '5s'`); + const [routine] = await tx.select({ id: routines.id }).from(routines).where(and( + eq(routines.id, candidate.routineId), eq(routines.companyId, candidate.companyId))).for("share"); + if (!routine) return false; + const [issue] = await tx.select().from(issues).where(and(eq(issues.id, candidate.issueId), + eq(issues.companyId, candidate.companyId), eq(issues.originKind, "routine_execution"), + eq(issues.originRunId, candidate.id))).for("update"); + if (!issue) return false; + const [run] = await tx.select({ id: routineRuns.id }).from(routineRuns) + .innerJoin(issues, eq(issues.id, routineRuns.linkedIssueId)) + .where(and(eq(routineRuns.id, candidate.id), eq(routineRuns.companyId, candidate.companyId), + eq(routineRuns.routineId, candidate.routineId), eq(routineRuns.linkedIssueId, issue.id), + routineExecutionStatusDebtCondition())).for("update", { of: routineRuns }); + if (!run) return false; + return (await syncRoutineRunStatusForIssue(tx as unknown as Db, issue.id)) !== null; + }); + } + return { load, settle }; +} diff --git a/server/src/services/routines.ts b/server/src/services/routines.ts index a831a3e489a..30fefbbabbf 100644 --- a/server/src/services/routines.ts +++ b/server/src/services/routines.ts @@ -74,7 +74,7 @@ import { secretService } from "./secrets.js"; import { getSecretProvider } from "../secrets/provider-registry.js"; import { parseCron, validateCron } from "./cron.js"; import { heartbeatService } from "./heartbeat.js"; -import { finalizeRoutineRun, syncRoutineRunStatusForIssue } from "./routine-run-status.js"; +import { finalizeRoutineRun, routineExecutionStatusService, syncRoutineRunStatusForIssue } from "./routine-run-status.js"; import { instanceSettingsService, isTruthyRuntimeEnvValue, @@ -2003,7 +2003,8 @@ export function routineService( return run; } - let statusReconcileCursor: string | null = null; + + let statusReconcileCursor: string | undefined; async function syncRunStatusForIssue(issueId: string, executor: Db = db) { return syncRoutineRunStatusForIssue(executor, issueId); @@ -3247,41 +3248,14 @@ export function routineService( return { triggered }; }, - // Repair a missed issue-status hook through the same status transition as - // normal issue writes. Never discard queued input or infer run completion - // from age. Lock both owners and re-read the issue before applying repair. reconcileExecutionRunStatuses: async (): Promise<{ scanned: number; settled: number; deferred: number }> => { - const candidates = await db.select({ id: routineRuns.id, companyId: routineRuns.companyId, - issueId: issues.id, routineId: routineRuns.routineId }) - .from(routineRuns).innerJoin(routines, and(eq(routines.id, routineRuns.routineId), eq(routines.companyId, routineRuns.companyId))) - .innerJoin(issues, and(eq(issues.id, routineRuns.linkedIssueId), - eq(issues.companyId, routineRuns.companyId), eq(issues.originKind, "routine_execution"), - sql`${issues.originRunId} = ${routineRuns.id}::text`, sql`${issues.originId} = ${routineRuns.routineId}::text`)) - .where(and(eq(routineRuns.status, "issue_created"), inArray(issues.status, ["done", "cancelled", "blocked"]), - ...(statusReconcileCursor ? [sql`${routineRuns.id}::text > ${statusReconcileCursor}`] : []))) - .orderBy(asc(routineRuns.id)).limit(50); - statusReconcileCursor = candidates.length === 50 ? candidates.at(-1)!.id : null; - let settled = 0; - let deferred = 0; + const service = routineExecutionStatusService(db); + const candidates = await service.load(undefined, statusReconcileCursor); + statusReconcileCursor = candidates.length === 50 ? candidates.at(-1)!.id : undefined; + let settled = 0, deferred = 0; for (const candidate of candidates) { try { - const repaired = await db.transaction(async tx => { - await tx.execute(sql`SET LOCAL lock_timeout = '500ms'`); - await tx.execute(sql`SET LOCAL statement_timeout = '5s'`); - const [routine] = await tx.select({ id: routines.id }).from(routines).where(and( - eq(routines.id, candidate.routineId), eq(routines.companyId, candidate.companyId))).for("share"); - if (!routine) return false; - const [issue] = await tx.select().from(issues).where(and(eq(issues.id, candidate.issueId), - eq(issues.companyId, candidate.companyId), eq(issues.originKind, "routine_execution"), - eq(issues.originRunId, candidate.id), eq(issues.originId, candidate.routineId))).for("update"); - if (!issue || !["done", "cancelled", "blocked"].includes(issue.status)) return false; - const [run] = await tx.select().from(routineRuns).where(and(eq(routineRuns.id, candidate.id), - eq(routineRuns.companyId, candidate.companyId), eq(routineRuns.routineId, candidate.routineId), - eq(routineRuns.linkedIssueId, issue.id), eq(routineRuns.status, "issue_created"))).for("update"); - if (!run) return false; - return (await syncRunStatusForIssue(issue.id, tx as unknown as Db)) !== null; - }); - if (repaired) settled += 1; + if (await service.settle(candidate)) settled++; } catch (error) { let current: unknown = error; let retryable = false; @@ -3291,7 +3265,7 @@ export function routineService( current = cause.cause; } if (!retryable) throw error; - deferred += 1; + deferred++; } } return { scanned: candidates.length, settled, deferred };