From 9b3fe260bac576d622ffdfefb923db73cc5273f7 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 6 Oct 2026 08:05:25 -0500 Subject: [PATCH 1/6] fix(tasks): surface Codex ChatGPT model rejection (#15299) 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. > - Native Runner runs report provider errors and can also save a structured failure result. > - Codex rejects a model that the user's ChatGPT account cannot use. > - Master now diagnoses this rejection, but the task's compact run data omits its message. The thread can still call it a generic run failure. > - Users need the account restriction and a clear step to repair the model selection. > - This pull request shows the existing diagnosis as “Model unavailable” on the task. ## Linked Issues or Issue Description **What happened?** Codex returns HTTP 400 with `invalid_request_error` and the message `The 'gpt-6.1-sol' model is not supported when using Codex with a ChatGPT account.` Master now stores an actionable diagnosis for this rejection. The task thread still labels it “Run failed” and does not receive its error text in compact run data. **Expected behavior** Show the account restriction on the task and in the run error. Tell the user to choose a supported model or clear the task's model override before retrying. **Steps to reproduce** 1. Use a Native Runner Codex agent signed in with a ChatGPT account. 2. Select a model that produces the rejection above and start a task. 3. Let the runner save its generic failed result. Inspect the task's failure marker and recovery notice. **Additional context** Refs: #15304. That merged PR diagnoses the provider failure and preserves worker and review recovery rules. This PR adds its task-facing message and guidance without changing that diagnosis or those rules. Refs: #13134. That PR improves model discovery for ChatGPT accounts. This PR exposes the rejection when a configured model still fails at execution time. ## What Changed - Return a bounded model rejection message in compact issue-run data. - Show “Model unavailable” and model-change guidance in the task thread and recovery notice. - Add database and UI regression tests, including both the original rejection text and master's fixed diagnosis. Document the new failure message. ## Verification - Focused merged-branch validation: 279 tests passed across activity service, native provider failure observation and PostgreSQL integration, TaskChatThread, and ExecutionBlockerNotice. - `pnpm -r typecheck`, `pnpm build`, and `pnpm check:token-gates` passed on the merged source tree. - Greptile reviewed conflict-resolution commit `1db39c4e636a92e2e76ba224b71c6f1e2f556c81` at 5/5 with no findings or inline comments. - All 55 check runs completed without failure on that commit. The two optional Storybook jobs were skipped. The legacy Snyk status passed. The branch has no merge conflicts. - UI regression assertion: the task's failure marker says “Model unavailable” and retains the account restriction. It no longer says that this failure happened after a final response. ## Risks - Provider recognition and recovery are owned by the existing master implementation. This PR exposes only bounded error text for failed runs with `native_provider_model_rejected`. - Historical runs with the generic `adapter_failed` code are not reclassified. No stored run is rewritten. - Existing retry and reconciliation gates remain in place. No schema migration or model configuration change is required. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, code execution, and browser inspection. The exact deployment model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-Authored-By: Paperclip --- doc/DEVELOPING.md | 5 ++++ server/src/__tests__/activity-service.test.ts | 7 +++-- server/src/services/activity.ts | 3 +++ ui/src/api/activity.ts | 2 ++ .../ExecutionBlockerNotice.test.tsx | 22 ++++++++++++++++ ui/src/components/ExecutionBlockerNotice.tsx | 4 ++- ui/src/components/TaskChatThread.test.tsx | 26 +++++++++++++++++++ ui/src/components/TaskChatThread.tsx | 6 +++-- ui/src/lib/issue-chat-messages.ts | 1 + 9 files changed, 71 insertions(+), 5 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 47e399e98c7..e1927b42962 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -192,6 +192,11 @@ choices. Codex uses the curated adapter catalog unless the instance declares `PAPERCLIP_ADAPTER_MODELS`; general OpenAI API models are not Codex choices. The Paperclip Runner Codex profile shows the same known model effort levels. Its selected effort is saved with the run and sent to Codex for each turn. +When Codex reports that a selected model is not supported with a ChatGPT account, +the task shows **Model unavailable**, the provider's account restriction, and +guidance to choose a supported model or clear the task's model override before +retrying. The run retains this reason even when the runner saves a generic +failure result. Claude Code uses model-specific effort levels; Haiku has no effort slider. Grok uses its adapter's reasoning levels, including for its default model. Kimi shows effort only when its agent uses the CLI engine, including with its diff --git a/server/src/__tests__/activity-service.test.ts b/server/src/__tests__/activity-service.test.ts index 328b61acff5..6f1a9601634 100644 --- a/server/src/__tests__/activity-service.test.ts +++ b/server/src/__tests__/activity-service.test.ts @@ -150,7 +150,7 @@ describeEmbeddedPostgres("activity service", () => { expect(result.map((event) => event.action)).toEqual(["test.newest", "test.middle"]); }); - it("returns compact usage and result summaries for issue runs", async () => { + it.each([null, "native_provider_model_rejected", "adapter_failed"])("returns compact issue runs with bounded model rejection details: %s", async (errorCode) => { const companyId = randomUUID(); const agentId = randomUUID(); const issueId = randomUUID(); @@ -180,7 +180,9 @@ describeEmbeddedPostgres("activity service", () => { companyId, agentId, invocationSource: "assignment", - status: "succeeded", + status: errorCode ? "failed" : "succeeded", + errorCode, + error: "provider-error".repeat(200), contextSnapshot: { issueId }, usageJson: { inputTokens: 11, @@ -215,6 +217,7 @@ describeEmbeddedPostgres("activity service", () => { agentId, invocationSource: "assignment", contextIssueId: issueId, + error: errorCode === "native_provider_model_rejected" ? "provider-error".repeat(200).slice(0, 2000) : null, }); expect(runs[0]?.usageJson).toEqual({ inputTokens: 11, diff --git a/server/src/services/activity.ts b/server/src/services/activity.ts index 8a2310cbad5..96685ad2870 100644 --- a/server/src/services/activity.ts +++ b/server/src/services/activity.ts @@ -403,6 +403,9 @@ export function activityService(db: Db) { invocationSource: heartbeatRuns.invocationSource, responsibleUserId: heartbeatRuns.responsibleUserId, errorCode: heartbeatRuns.errorCode, + error: sql`case when ${heartbeatRuns.status} = 'failed' + and ${heartbeatRuns.errorCode} = 'native_provider_model_rejected' + then left(${heartbeatRuns.error}, 2000) else null end`, usageJson: summarizedUsageJson, resultJson: summarizedResultJson, logBytes: heartbeatRuns.logBytes, diff --git a/ui/src/api/activity.ts b/ui/src/api/activity.ts index 50ecab9a5a4..5daa3701ccb 100644 --- a/ui/src/api/activity.ts +++ b/ui/src/api/activity.ts @@ -16,6 +16,8 @@ export interface RunForIssue { invocationSource: string; responsibleUserId?: string | null; errorCode?: string | null; + /** Bounded provider message for a failed model rejection. */ + error?: string | null; usageJson: Record | null; resultJson: Record | null; logBytes?: number | null; diff --git a/ui/src/components/ExecutionBlockerNotice.test.tsx b/ui/src/components/ExecutionBlockerNotice.test.tsx index 3683c76883a..d5d2b0a95dc 100644 --- a/ui/src/components/ExecutionBlockerNotice.test.tsx +++ b/ui/src/components/ExecutionBlockerNotice.test.tsx @@ -107,6 +107,28 @@ describe("stopped task recovery notice", () => { expect(onRetried).toHaveBeenCalledOnce(); }); + it.each([ + "The 'gpt-6.1-sol' model is not supported when using Codex with a ChatGPT account.", + "The selected model is not supported by the current ChatGPT connection. Choose a supported model or a compatible AI connection.", + ])("shows the model rejection and repair action without expanding run logs: %s", async (runError) => { + vi.mocked(activityApi.runsForIssue).mockResolvedValue([{ runId: "model-run", agentId: "agent", + status: "failed", errorCode: "native_provider_model_rejected" }] as never); + await act(async () => root.render( + + )); + await act(async () => { await new Promise(resolve => setTimeout(resolve, 10)); }); + expect(container.textContent).toContain("Model unavailable."); + expect(container.textContent).toContain(runError); + expect(container.textContent).toContain("clear the task's model override, then retry"); + expect(container.textContent).toContain("Inspect the original failure before continuing."); + expect(container.querySelector("button")?.textContent).toBe("Retry"); + }); + it.each(["native_continuation_requires_reconciliation", "uncertain_external_action"])("offers Retry for a server-admitted native failure: %s", async cause => { await act(async () => root.render( run.runId === blocker.runId && ["failed", "timed_out"].includes(run.status)); + const modelRejected = failedRun?.errorCode === "native_provider_model_rejected"; const requiresInspection = blocker.cause === "native_continuation_requires_reconciliation" || blocker.cause === "native_session_cleanup_quarantined"; const retry = useMutation({ @@ -34,7 +35,8 @@ export function ExecutionBlockerNotice({ companyId, issueId, blocker, onRetried return (
-

Recovery needed.{blocker.runError ? ` ${blocker.runError}` : ""}

+

{modelRejected ? "Model unavailable." : "Recovery needed."}{blocker.runError ? ` ${blocker.runError}` : ""}

+ {modelRejected &&

Choose a supported model or clear the task's model override, then retry.

}

{blocker.nextAction}

{Boolean(blocker.savedMessageCount) && (

{blocker.savedMessageCount} saved {blocker.savedMessageCount === 1 ? "message is" : "messages are"} waiting for recovery.

diff --git a/ui/src/components/TaskChatThread.test.tsx b/ui/src/components/TaskChatThread.test.tsx index 10854e06cc8..1a111c1167c 100644 --- a/ui/src/components/TaskChatThread.test.tsx +++ b/ui/src/components/TaskChatThread.test.tsx @@ -1803,6 +1803,32 @@ describe("TaskChatThread runtime transcript selection", () => { expect(container.textContent).not.toContain("The runner stopped"); }); + it.each([ + "The 'gpt-6.1-sol' model is not supported when using Codex with a ChatGPT account.", + "The selected model is not supported by the current ChatGPT connection. Choose a supported model or a compatible AI connection.", + ])("promotes a rejected model above the runner's generated failure response: %s", (error) => { + nativeTranscriptState.transcriptByRun.set("model-rejected", [{ + kind: "run_result", ts: "2026-08-25T18:00:01.000Z", + summary: "The Codex run failed before it completed.", + disposition: "needs_review", objectiveSatisfied: false, verification: [], + remainingWork: [], blocker: null, artifacts: [], + }]); + render( {}} linkedRuns={[{ + runId: "model-rejected", runtimeMode: "native", status: "failed", + errorCode: "native_provider_model_rejected", + error, + agentId: "agent-1", agentName: "Runner", adapterType: "paperclip_runner", + createdAt: "2026-08-25T18:00:00.000Z", startedAt: "2026-08-25T18:00:00.000Z", + finishedAt: "2026-08-25T18:00:02.000Z", + }]} />); + const marker = container.querySelector('[data-testid="task-chat-collapsible-marker"]'); + expect(marker?.textContent).toContain("Model unavailable"); + flushSync(() => marker!.querySelector('button[aria-expanded="false"]')!.click()); + expect(container.textContent).toContain(error); + expect(container.textContent).toContain("clear the task's model override, then retry"); + expect(container.textContent).not.toContain("after returning a final response"); + }); + it("keeps workspace contention out of the conversation's cancellation markers", () => { render( {}} linkedRuns={[{ runId: "workspace-wait", runtimeMode: "native", status: "cancelled", errorCode: "workspace_busy", diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index 3c70f2aafc0..45690233df5 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -1637,7 +1637,9 @@ export function TaskChatThread(props: TaskChatThreadProps) { ? "native_runner_timed_out" : "native_runner_process_exited"); const label = - code === "native_provider_approval_required" && source.status === "failed" + code === "native_provider_model_rejected" && source.status === "failed" + ? "Model unavailable" + : code === "native_provider_approval_required" && source.status === "failed" ? "Approval required" : code === "native_provider_usage_limit" && source.status === "failed" ? "Usage limit reached" @@ -1659,7 +1661,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { : code === "native_provider_approval_required" ? "This operation requires approval, but this runner has no interactive approval handler. Review the operation and update the agent's permission setting before retrying." : code === "native_provider_model_rejected" - ? "The provider rejected the selected model. Check the model ID and your account's access, save the agent configuration, then retry. View the run for the provider's full error." + ? `${meta?.error || "The provider rejected the selected model."} Choose a supported model or clear the task's model override, then retry.` : code === "native_provider_usage_limit" && source.status === "failed" ? "The model provider has reached its current usage limit. Try again after the limit resets." diff --git a/ui/src/lib/issue-chat-messages.ts b/ui/src/lib/issue-chat-messages.ts index bc6e7dfef1d..cd184c00425 100644 --- a/ui/src/lib/issue-chat-messages.ts +++ b/ui/src/lib/issue-chat-messages.ts @@ -59,6 +59,7 @@ export interface IssueChatLinkedRun { hasStoredOutput?: boolean; logBytes?: number | null; errorCode?: string | null; + error?: string | null; scheduledRetryAt?: string | null; nextAction?: string | null; resultJson?: Record | null; From 3290d974177a1d4c5b7d635f6a59c6f3c342920f Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 6 Oct 2026 06:42:21 -0700 Subject: [PATCH 2/6] Scope the release smoke opening question to its card (#15339) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The release lane checks onboarding before it promotes a nightly candidate. > - The first task shows an unanswered-question summary and an open question card. > - Both display the same prompt, so the smoke test's page-wide text locator fails strict matching. > - This pull request scopes that assertion to the open interaction card. > - The smoke still requires the actual prompt and verifies that onboarding does not start an agent run. ## Linked Issues or Issue Description **What happened?** [Scheduled release run 37441718130](https://github.com/paperclipai/paperclip/actions/runs/37441718130) failed `smoke_nightly / smoke` on both attempts. The page-wide `getByText("What would you like to do?")` matched two elements: the timeline summary and the open question card. This skipped `publish_nightly`. **Expected behavior** The test confirms that the seeded task's open question card displays the exact prompt. A summary row alone must not satisfy that check. **Steps to reproduce** Run the Docker onboarding smoke against published candidate `2026.1006.0-canary.11`, then run the release-smoke Playwright spec. The old assertion fails after the greeting appears. **Paperclip version or commit** Release workflow commit `f858207161ba29c01c82f4674aef83d91b74480f`; published candidate `2026.1006.0-canary.11`. The assertion is unchanged on the current master base `9b3fe260bac576d622ffdfefb923db73cc5273f7`. **Deployment mode** Docker smoke harness, authenticated/private, with its existing mock provider. Related: #13166 added the first-task chat assertion. I also reviewed open #12316, which fixes a separate bootstrap race and does not change this selector. Searches found no duplicate selector fix or public issue. ## What Changed - Scope the exact opening-prompt text to `task-chat-interaction`. - Explain why the timeline summary cannot satisfy the assertion. - Preserve the rest of the smoke flow, including the 15-second check for no agent runs. ## Verification - Local Docker harness ran the exact failed published candidate, `2026.1006.0-canary.11`, with the existing mock provider. - The old spec reproduced the same two-element strict-mode error in Chrome. - The fixed spec passed the full authenticated onboarding flow and the 15-second no-run check: 1/1, 32.6 seconds. The executed spec copy was byte-identical to the changed repository file; only artifact output paths and the local port were overridden. - Independent Chrome checks: the scoped locator passes with both summary and card present. Summary-only, wrong prompt, hidden prompt, and duplicate active prompts each fail as intended. - `pnpm typecheck` and `pnpm build` passed on Node 24.21.0. Canonical Linux CI passed all 55 checks (53 success, 2 intentional Storybook skips), including the full aggregate tests, build, typecheck, all eight E2E shards, and post-ready security scan. - Greptile scored the exact head `5a30e667396600a052a4041c819652c19b8c68ff` at 5/5 with no actionable issues. Independent review found no issues; there are no review threads. - `git diff --check` passed. The diff and PR text were scanned for secrets and private identifiers. ## Risks Low risk: one assertion in a release smoke test changes. A missing or hidden prompt still fails, and multiple matching prompts in interaction cards still fail strict mode. This does not change the product, release selection, or publishing. The scheduled release lane still needs its next normal successful run to prove nightly recovery. ## Model Used OpenAI GPT-6 through Codex, with reasoning, shell tools, and an independent Codex review. The exact serving model identifier and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- tests/release-smoke/docker-auth-onboarding.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/release-smoke/docker-auth-onboarding.spec.ts b/tests/release-smoke/docker-auth-onboarding.spec.ts index db436a760d7..b1d9324aa23 100644 --- a/tests/release-smoke/docker-auth-onboarding.spec.ts +++ b/tests/release-smoke/docker-auth-onboarding.spec.ts @@ -252,7 +252,13 @@ test.describe("Docker authenticated onboarding smoke", () => { await expect( page.getByText("Welcome to Paperclip!").first() ).toBeVisible({ timeout: 20_000 }); - await expect(page.getByText("What would you like to do?")).toBeVisible(); + // The timeline also summarizes the unanswered question. Check the open + // question card so a summary alone cannot satisfy this assertion. + await expect( + page + .getByTestId("task-chat-interaction") + .getByText("What would you like to do?", { exact: true }) + ).toBeVisible(); // … and that the no-run contract holds. This spec used to poll for an // assignment-triggered heartbeat run here; a run appearing before the From 90182b4f8b40d6ee217937ba61199b4abc31dee7 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 6 Oct 2026 06:58:36 -0700 Subject: [PATCH 3/6] Report bounded Cloud portfolio failure diagnostics (#15340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip manages work for AI agents and their companies. > - Cloud instances proxy a trusted user's portfolio through the control plane. > - A failed request returns a generic error to the client. > - That replacement error loses the failure phase and network code in Sentry. > - Operators need bounded evidence without upstream messages or credentials. > - This PR adds safe diagnostics while preserving the existing request behavior. ## Linked Issues or Issue Description Refs #10850, which added the portfolio proxy. No open PR for this diagnostic gap was found. **What happened?** A rejected portfolio fetch becomes a generic 502 in Sentry. The event cannot distinguish a connection reset, deadline, HTTP response failure, or body failure. The route's previous warning also included the original error and a stack identifier. **Steps to reproduce** Make the portfolio proxy's fetch reject with a TypeError whose cause has `code: ECONNRESET`. The client correctly receives the generic 502, but the captured replacement error loses that code. **Expected behavior** Keep the existing client response. Attach only bounded server-side diagnostic fields to the failure event. Do not retry the request or expose the original error. ## What Changed - Add a typed portfolio error with a private frozen diagnostic record: phase, upstream HTTP status, elapsed milliseconds, and an allowlisted network code. - Read at most four error/cause objects through own data properties. Unknown codes, messages, getters, and out-of-range values do not enter the record. - Replace the route's raw-error warnings with safe fields. Send a plain error plus event-local context through the existing optional Sentry gate. Keep the route callsite and default fingerprint policy. - Preserve authentication, trusted headers, cookies, exact HTTP error bodies, cache behavior, the ten-second deadline, and one fetch per request. Public responses receive no diagnostic fields. - Document the fields and test HTTP behavior, privacy, and event isolation with the real Sentry SDK. ## Verification - `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1 pnpm exec vitest run server/src/__tests__/cloud-portfolio-error.test.ts server/src/__tests__/cloud-routes.test.ts server/src/__tests__/sentry.test.ts server/src/__tests__/run-failure-sentry-real-sdk.test.ts`: 65 passed, with the audited optional SDK installed. - Full local `pnpm -r typecheck` and `pnpm build` passed on Node 24.21.0 and pnpm 9.15.4. - Independent review found no blockers and independently passed all 65 tests, including the real SDK checks, on this exact commit. - Full Linux CI passed on this exact commit and provides aggregate suite coverage (54 successful checks, 2 intentional skips). A duplicate full local aggregate was not run. - The first SDK contract job failed before tests when npm could not resolve an OpenTelemetry transitive package. A subsequent empty-cache install first encountered a missing tarball, then succeeded after the registry artifact became available. All 6 real-SDK tests passed against that fresh install; the single unchanged-head CI retry passed. The SDK pin, workflow, and dependency files are unchanged. - The first browser shard 8 run timed out waiting for the inbox retry reply after 45 seconds. The unchanged isolated case passed (1/1), and the test, UI handler, fixture, and recovery files match the base commit. The failed log contains no wakeup POST before the test's immediate navigation; a navigation/request timing race is suspected but unproven without a trace. The single unchanged-head shard retry passed (20 passed, 1 skipped); no timeout or source change was made. - Greptile reviewed this exact commit at 5/5 with no unresolved review threads. - No live portfolio request was replayed. Route tests use controlled local upstream responses. - The added diff passed the secret and PII scan and `git diff --check`. ## Risks This is a diagnostic change. It does not identify or repair the origin of a connection reset. Unknown transport failures remain `unknown`. Elapsed values outside 0–60,000 ms become null. Default Sentry fingerprinting remains enabled; exact historical group membership is not guaranteed. No retry, migration, deployment, or configuration change is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository editing, code execution, and independent agent review. The exact deployment model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/observability.md | 13 +++ .../__tests__/cloud-portfolio-error.test.ts | 45 +++++++++ server/src/__tests__/cloud-routes.test.ts | 99 ++++++++++++++++++- .../run-failure-sentry-real-sdk.test.ts | 42 ++++++++ server/src/__tests__/sentry.test.ts | 23 +++++ server/src/routes/cloud.ts | 44 ++++++--- server/src/sentry.ts | 9 ++ server/src/services/cloud-portfolio-error.ts | 70 +++++++++++++ 8 files changed, 331 insertions(+), 14 deletions(-) create mode 100644 server/src/__tests__/cloud-portfolio-error.test.ts create mode 100644 server/src/services/cloud-portfolio-error.ts diff --git a/doc/observability.md b/doc/observability.md index e877610b3d0..5c1e57c1784 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -467,6 +467,19 @@ native failure only while its saved review is still current and pending; caller payloads cannot select a different review. Dispatch checks that assignment again. Superseded or completed reviews retain their authority. +Cloud portfolio proxy failures have an event-local `cloud_portfolio` context. +It contains only the phase (`fetch`, `http_response`, `response_body`, or +`response_write`), upstream HTTP status when available, elapsed milliseconds, +and an allowlisted network error code. Unknown values become `unknown` or +`null`; elapsed time outside 0–60,000 ms becomes `null`. Codes come from at +most four error/cause objects' own data properties, never error messages. +`DEADLINE_EXCEEDED` means the request's own ten-second signal was aborted. +The route's warning uses the same safe fields. Neither record includes URLs, +headers, cookies, bodies, user/stack IDs, or the original exception. Sentry +receives a plain error with the existing generic message and default grouping. +The context does not change the HTTP response, authentication, cookies, cache, +deadline, or single-fetch behavior, and does not carry into unrelated events. + An unconfirmed adapter Stop timeout has an event-local `adapter_stop` context: the run UUID, built-in adapter type, native/legacy runtime mode, configured wait duration, and whether abort was requested. Invalid identities become diff --git a/server/src/__tests__/cloud-portfolio-error.test.ts b/server/src/__tests__/cloud-portfolio-error.test.ts new file mode 100644 index 00000000000..32e35eff953 --- /dev/null +++ b/server/src/__tests__/cloud-portfolio-error.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; +import { CloudPortfolioError } from "../services/cloud-portfolio-error.js"; + +const input = { phase: "fetch" as const, elapsedMs: 270.2, upstreamStatus: null }; + +describe("CloudPortfolioError", () => { + it("retains only an allowlisted nested network code, not the original exception", () => { + const cause = Object.assign(new Error("private upstream payload"), { code: "ECONNRESET", url: "https://private.example.test" }); + const error = new CloudPortfolioError("upstream", input, new TypeError("fetch failed", { cause })); + expect(error.diagnostics).toEqual({ phase: "fetch", elapsedMs: 270, upstreamStatus: null, networkCode: "ECONNRESET" }); + expect(error).not.toHaveProperty("cause"); + expect(JSON.stringify(error)).not.toContain("private"); + expect(Object.isFrozen(error.diagnostics)).toBe(true); + }); + + it.each([ + new Error("ECONNRESET in a private message is not evidence"), + { code: "private-provider-code" }, + { get code() { throw new Error("private getter"); }, get cause() { throw new Error("private getter"); } }, + new Proxy({}, { getOwnPropertyDescriptor() { throw new Error("private proxy"); } }), + ])("fails closed for unknown errors without invoking getters", (cause) => { + expect(new CloudPortfolioError("upstream", input, cause).diagnostics.networkCode).toBe("unknown"); + }); + + it("bounds cause traversal and handles cycles", () => { + const cycle: { cause?: unknown } = {}; + cycle.cause = cycle; + expect(new CloudPortfolioError("upstream", input, cycle).diagnostics.networkCode).toBe("unknown"); + const deep = { cause: { cause: { cause: { cause: { code: "ECONNRESET" } } } } }; + expect(new CloudPortfolioError("upstream", input, deep).diagnostics.networkCode).toBe("unknown"); + }); + + it("normalizes unsafe diagnostics and keeps non-network phases distinct", () => { + const error = new CloudPortfolioError("upstream", { + phase: "private-phase" as never, elapsedMs: Infinity, upstreamStatus: 900, + }, { code: "ECONNRESET" }); + expect(error.diagnostics).toEqual({ phase: "unknown", elapsedMs: null, upstreamStatus: null, networkCode: "unknown" }); + for (const phase of ["http_response", "response_write"] as const) { + expect(new CloudPortfolioError("upstream", { ...input, phase, upstreamStatus: 503, deadlineExceeded: true }, { code: "ECONNRESET" }).diagnostics) + .toMatchObject({ phase, upstreamStatus: 503, networkCode: "unknown" }); + } + expect(new CloudPortfolioError("upstream", { ...input, elapsedMs: 60_001 }).diagnostics.elapsedMs).toBeNull(); + expect(new CloudPortfolioError("upstream", { ...input, elapsedMs: -1 }).diagnostics.elapsedMs).toBeNull(); + }); +}); diff --git a/server/src/__tests__/cloud-routes.test.ts b/server/src/__tests__/cloud-routes.test.ts index a6453e05a25..a85c4c4087b 100644 --- a/server/src/__tests__/cloud-routes.test.ts +++ b/server/src/__tests__/cloud-routes.test.ts @@ -1,8 +1,13 @@ import express from "express"; import request from "supertest"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { errorHandler } from "../middleware/index.js"; import { cloudRoutes } from "../routes/cloud.js"; +import { CloudPortfolioError } from "../services/cloud-portfolio-error.js"; +import * as sentry from "../sentry.js"; +import { logger } from "../middleware/logger.js"; + +afterEach(() => vi.restoreAllMocks()); const cloudEnv = { PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN: "tenant-secret", @@ -51,6 +56,98 @@ function jsonResponse(payload: unknown, status = 200) { } describe("GET /api/cloud/stacks", () => { + it.each([401, 403, 429, 503])("preserves the response contract for upstream HTTP %s without reading the body", async (status) => { + const upstream = jsonResponse({ secret: "private upstream payload" }, status); + const json = vi.spyOn(upstream, "json"); + const fetchImpl = vi.fn().mockResolvedValue(upstream); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const warn = vi.spyOn(logger, "warn").mockImplementation(() => {}); + const response = await request(createApp({ fetchImpl })).get("/api/cloud/stacks").set("Cookie", "client-session=private-cookie"); + expect(response.status).toBe(502); + expect(response.body).toEqual({ error: "Paperclip Cloud portfolio request failed", code: "cloud_portfolio_upstream_error", details: { code: "cloud_portfolio_upstream_error" } }); + expect(response.headers["set-cookie"]).toBeUndefined(); + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(new Headers(fetchImpl.mock.calls[0]![1]!.headers).has("cookie")).toBe(false); + expect(json).not.toHaveBeenCalled(); + expect(capture).toHaveBeenCalledWith(expect.any(CloudPortfolioError)); + const error = capture.mock.calls[0]![0] as CloudPortfolioError; + expect(error.diagnostics).toMatchObject({ phase: "http_response", upstreamStatus: status, networkCode: "unknown", elapsedMs: expect.any(Number) }); + expect(error.stack).toContain("routes/cloud.ts:"); + expect(error.stack).not.toContain("at portfolioError"); + expect(warn).toHaveBeenCalledWith({ cloudPortfolio: error.diagnostics }, "Paperclip Cloud portfolio request failed"); + expect(JSON.stringify(warn.mock.calls)).not.toContain("private"); + }); + + it("classifies a fetch reset without retrying or caching the failure", async () => { + const fetchImpl = vi.fn() + .mockRejectedValueOnce(new TypeError("private fetch message", { cause: Object.assign(new Error("private cause"), { code: "ECONNRESET" }) })) + .mockResolvedValueOnce(jsonResponse({ stacks: [] })); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const warn = vi.spyOn(logger, "warn").mockImplementation(() => {}); + const app = createApp({ fetchImpl }); + const failure = await request(app).get("/api/cloud/stacks"); + expect(failure.status).toBe(502); + expect(failure.body).toEqual({ error: "Paperclip Cloud portfolio request failed", code: "cloud_portfolio_upstream_error", details: { code: "cloud_portfolio_upstream_error" } }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + const error = capture.mock.calls[0]![0] as CloudPortfolioError; + expect(error.diagnostics).toMatchObject({ phase: "fetch", upstreamStatus: null, networkCode: "ECONNRESET" }); + expect(JSON.stringify(warn.mock.calls)).not.toContain("private"); + expect((await request(app).get("/api/cloud/stacks")).body).toEqual({ stacks: [] }); + expect((await request(app).get("/api/cloud/stacks")).body).toEqual({ stacks: [] }); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); + + it.each(["invalid_json", "body_reset", "deadline"] as const)("keeps body-phase %s failures separate from fetch failures", async (kind) => { + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const controller = new AbortController(); + const timeout = vi.spyOn(AbortSignal, "timeout").mockReturnValue(controller.signal); + const upstream = jsonResponse({}); + vi.spyOn(upstream, "json").mockImplementation(async () => { + if (kind === "deadline") controller.abort(new DOMException("private timeout", "TimeoutError")); + throw kind === "body_reset" ? Object.assign(new Error("private body"), { code: "ECONNRESET" }) : new SyntaxError("private body"); + }); + const fetchImpl = vi.fn().mockResolvedValue(upstream); + const app = createApp({ fetchImpl }); + for (let count = 1; count <= 2; count++) { + const response = await request(app).get("/api/cloud/stacks"); + expect(response.status).toBe(502); + expect(response.body).toEqual({ error: "Paperclip Cloud portfolio returned invalid JSON", code: "cloud_portfolio_invalid_response", details: { code: "cloud_portfolio_invalid_response" } }); + expect(fetchImpl).toHaveBeenCalledTimes(count); + } + expect(timeout).toHaveBeenCalledWith(10_000); + expect((capture.mock.calls[0]![0] as CloudPortfolioError).diagnostics).toMatchObject({ + phase: "response_body", upstreamStatus: 200, + networkCode: kind === "deadline" ? "DEADLINE_EXCEEDED" : kind === "body_reset" ? "ECONNRESET" : "unknown", + }); + }); + + it("records the owned fetch deadline without changing the ten-second signal", async () => { + const signal = AbortSignal.abort(new DOMException("private timeout", "TimeoutError")); + const timeout = vi.spyOn(AbortSignal, "timeout").mockReturnValue(signal); + const fetchImpl = vi.fn().mockRejectedValue(signal.reason); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const response = await request(createApp({ fetchImpl })).get("/api/cloud/stacks"); + expect(response.body).toEqual({ error: "Paperclip Cloud portfolio request failed", code: "cloud_portfolio_upstream_error", details: { code: "cloud_portfolio_upstream_error" } }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(fetchImpl.mock.calls[0]![1]?.signal).toBe(signal); + expect(timeout).toHaveBeenCalledWith(10_000); + expect((capture.mock.calls[0]![0] as CloudPortfolioError).diagnostics).toMatchObject({ phase: "fetch", networkCode: "DEADLINE_EXCEEDED" }); + }); + + it("does not mislabel a response serialization failure as an upstream fetch error", async () => { + const payload: { cycle?: unknown } = {}; + payload.cycle = payload; + const upstream = jsonResponse({}); + vi.spyOn(upstream, "json").mockResolvedValue(payload); + const fetchImpl = vi.fn().mockResolvedValue(upstream); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const response = await request(createApp({ fetchImpl })).get("/api/cloud/stacks"); + expect(response.status).toBe(502); + expect(response.body).toEqual({ error: "Paperclip Cloud portfolio request failed", code: "cloud_portfolio_upstream_error", details: { code: "cloud_portfolio_upstream_error" } }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect((capture.mock.calls[0]![0] as CloudPortfolioError).diagnostics).toMatchObject({ phase: "response_write", upstreamStatus: 200, networkCode: "unknown" }); + }); + it("returns the actor's portfolio without forwarding client-supplied identity", async () => { const portfolio = { stacks: [{ slug: "current", displayName: "Current" }] }; const fetchImpl = vi.fn().mockResolvedValue(jsonResponse(portfolio)); diff --git a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts index fd518304020..5cac0081e31 100644 --- a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts +++ b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts @@ -42,6 +42,48 @@ afterEach(async () => { }); describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", () => { + it("keeps portfolio diagnostics bounded and isolated from unrelated captures", async () => { + const Sentry = sentryPackage!; + const events: Array> = []; + vi.stubEnv("SENTRY_DSN_BACKEND", "https://public@example.invalid/1"); + vi.doMock("../peer-version-check.js", () => ({ checkExactPeerVersions: () => ({ ok: true }) })); + vi.doMock("@sentry/node", () => ({ + ...Sentry, + init: (options: Record) => Sentry.init({ + ...options, + transport: () => ({ send: async () => ({}), flush: async () => true }), + beforeSend: (event: Record) => { events.push(event); return event; }, + }), + })); + vi.resetModules(); + const { sentryReady, captureException } = await import("../sentry.js"); + const { CloudPortfolioError } = await import("../services/cloud-portfolio-error.js"); + await sentryReady; + const failure = new CloudPortfolioError("upstream", { + phase: "fetch", elapsedMs: 270, upstreamStatus: null, + }, new TypeError("private-portfolio-message", { cause: { code: "ECONNRESET", token: "private-portfolio-token" } })); + Object.assign(failure, { + cause: new Error("private-portfolio-cause"), + request: { url: "https://private-portfolio.invalid", headers: { cookie: "private-portfolio-cookie" } }, + }); + Object.defineProperty(failure, "diagnostics", { value: { extra: "private-portfolio-replacement" } }); + captureException(failure); + captureException(new Error("unrelated portfolio fixture")); + await Sentry.flush(2000); + expect(events).toHaveLength(2); + const captured = events.find((event) => (event.exception as { values: Array<{ value: string }> }).values.some((entry) => entry.value === failure.message)); + expect(captured).toMatchObject({ + tags: { error_code: "cloud_portfolio_failure" }, fingerprint: ["{{ default }}"], + contexts: { cloud_portfolio: { phase: "fetch", elapsedMs: 270, upstreamStatus: null, networkCode: "ECONNRESET" } }, + }); + expect((captured!.exception as { values: unknown[] }).values).toHaveLength(1); + expect(JSON.stringify(events)).not.toContain("private-portfolio-"); + const unrelated = events.find((event) => event !== captured)!; + expect(unrelated).not.toHaveProperty("contexts.cloud_portfolio"); + expect(unrelated).not.toHaveProperty("tags.error_code"); + expect(unrelated).not.toHaveProperty("fingerprint"); + }); + it.each([ { phase: "workspace_restore", phaseElapsedMs: 60_123, expectedPhase: "workspace_restore", expectedElapsedMs: 60_123 }, { phase: "private-provider-phase", phaseElapsedMs: 100, expectedPhase: "unknown", expectedElapsedMs: null }, diff --git a/server/src/__tests__/sentry.test.ts b/server/src/__tests__/sentry.test.ts index ba044e8b1b2..865ae21fc4c 100644 --- a/server/src/__tests__/sentry.test.ts +++ b/server/src/__tests__/sentry.test.ts @@ -127,6 +127,29 @@ describe("sentryReady", () => { }); describe("captureException", () => { + it("sends only private normalized portfolio diagnostics on the matching event", async () => { + process.env[BACKEND_DSN_ENV] = "https://fixture@example.com/1"; + const sdk = mockSentryPackage(); + const { captureException, sentryReady } = await importFreshSentry(); + const { CloudPortfolioError } = await import("../services/cloud-portfolio-error.js"); + await sentryReady; + const error = new CloudPortfolioError("upstream", { phase: "fetch", elapsedMs: 270, upstreamStatus: null }, { code: "ECONNRESET" }); + Object.assign(error, { cause: new Error("private cause"), headers: { authorization: "private token" } }); + Object.defineProperty(error, "diagnostics", { value: { token: "private replacement" } }); + captureException(error); + const unrelated = new Error("unrelated"); + captureException(unrelated); + expect(sdk.captureException.mock.calls[0]).toEqual([ + expect.objectContaining({ message: error.message, stack: error.stack }), + { tags: { error_code: "cloud_portfolio_failure" }, fingerprint: ["{{ default }}"], contexts: { + cloud_portfolio: { phase: "fetch", elapsedMs: 270, upstreamStatus: null, networkCode: "ECONNRESET" }, + } }, + ]); + expect(JSON.stringify(sdk.captureException.mock.calls[0])).not.toContain("private"); + expect(sdk.captureException.mock.calls[0]![0]).not.toHaveProperty("cause"); + expect(sdk.captureException.mock.calls[1]).toEqual([unrelated]); + }); + it("adds bounded Stop timeout context only to that event", async () => { process.env[BACKEND_DSN_ENV] = "https://fixture@example.com/1"; const sdk = mockSentryPackage(); diff --git a/server/src/routes/cloud.ts b/server/src/routes/cloud.ts index 0b4aefc8fc6..e8456d7e40f 100644 --- a/server/src/routes/cloud.ts +++ b/server/src/routes/cloud.ts @@ -1,6 +1,7 @@ import { Router } from "express"; import { forbidden, HttpError, notFound } from "../errors.js"; import { logger } from "../middleware/logger.js"; +import { CloudPortfolioError, type CloudPortfolioPhase } from "../services/cloud-portfolio-error.js"; import { getCloudStackContext, type CloudInstanceEnv, @@ -67,7 +68,24 @@ export function cloudRoutes(opts: { }); } + const startedAt = performance.now(); + let phase: CloudPortfolioPhase = "fetch"; + let upstreamStatus: number | null = null; + let upstreamSignal: AbortSignal | undefined; + const portfolioError = (kind: "upstream" | "invalid_response", error?: unknown) => { + const reportableError = new CloudPortfolioError(kind, { + phase, + elapsedMs: performance.now() - startedAt, + upstreamStatus, + deadlineExceeded: upstreamSignal?.aborted === true, + }, error); + // Preserve the route callsite instead of grouping by this helper frame. + Error.captureStackTrace?.(reportableError, portfolioError); + return reportableError; + }; + try { + upstreamSignal = AbortSignal.timeout(10_000); const upstream = await fetchImpl(portfolioUrl, { method: "GET", headers: { @@ -76,26 +94,27 @@ export function cloudRoutes(opts: { "x-paperclip-cloud-user-id": actorUserId, "x-paperclip-cloud-stack-id": context.stackId, }, - signal: AbortSignal.timeout(10_000), + signal: upstreamSignal, }); + phase = "http_response"; + upstreamStatus = upstream.status; if (!upstream.ok) { + const error = portfolioError("upstream"); logger.warn( - { status: upstream.status, stackId: context.stackId }, + { cloudPortfolio: error.diagnostics }, "Paperclip Cloud portfolio request failed", ); - throw new HttpError(502, "Paperclip Cloud portfolio request failed", { - code: "cloud_portfolio_upstream_error", - }); + throw error; } + phase = "response_body"; let payload: unknown; try { payload = await upstream.json(); - } catch { - throw new HttpError(502, "Paperclip Cloud portfolio returned invalid JSON", { - code: "cloud_portfolio_invalid_response", - }); + } catch (error) { + throw portfolioError("invalid_response", error); } + phase = "response_write"; portfolioCache.set(cacheKey, { expiresAt: currentTime + cacheTtlMs, payload, @@ -104,13 +123,12 @@ export function cloudRoutes(opts: { res.json(payload); } catch (error) { if (error instanceof HttpError) throw error; + const reportableError = portfolioError("upstream", error); logger.warn( - { err: error, stackId: context.stackId }, + { cloudPortfolio: reportableError.diagnostics }, "Paperclip Cloud portfolio request failed", ); - throw new HttpError(502, "Paperclip Cloud portfolio request failed", { - code: "cloud_portfolio_upstream_error", - }); + throw reportableError; } }); diff --git a/server/src/sentry.ts b/server/src/sentry.ts index 665cab811f5..1ecd5c3935e 100644 --- a/server/src/sentry.ts +++ b/server/src/sentry.ts @@ -57,6 +57,7 @@ import os from "node:os"; import { AdapterStopTimeoutError } from "./services/adapter-stop-timeout.js"; +import { CloudPortfolioError } from "./services/cloud-portfolio-error.js"; import type { RunFailureDiagnostics } from "./services/run-failure-diagnostics.js"; import { readBuildCommit } from "./build-commit.js"; import { checkExactPeerVersions } from "./peer-version-check.js"; @@ -116,6 +117,14 @@ export function captureException(error: unknown): void { contexts: { adapter_stop: { ...error.diagnostics } }, fingerprint: ["{{ default }}"], }); + } else if (error instanceof CloudPortfolioError) { + const exception = new Error(error.message); + exception.stack = error.stack; + sentryHandle.captureException(exception, { + tags: { error_code: "cloud_portfolio_failure" }, + contexts: { cloud_portfolio: { ...CloudPortfolioError.diagnosticsFor(error) } }, + fingerprint: ["{{ default }}"], + }); } else { sentryHandle.captureException(error); } diff --git a/server/src/services/cloud-portfolio-error.ts b/server/src/services/cloud-portfolio-error.ts new file mode 100644 index 00000000000..6868830eb28 --- /dev/null +++ b/server/src/services/cloud-portfolio-error.ts @@ -0,0 +1,70 @@ +import { HttpError } from "../errors.js"; + +const phases = ["fetch", "http_response", "response_body", "response_write"] as const; +export type CloudPortfolioPhase = typeof phases[number]; +const networkCodes = new Set([ + "ECONNRESET", "ECONNREFUSED", "ETIMEDOUT", "ENOTFOUND", "EAI_AGAIN", + "ENETUNREACH", "EHOSTUNREACH", "UND_ERR_CONNECT_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", "UND_ERR_BODY_TIMEOUT", "UND_ERR_SOCKET", + "ERR_TLS_CERT_ALTNAME_INVALID", "CERT_HAS_EXPIRED", + "DEPTH_ZERO_SELF_SIGNED_CERT", "UNABLE_TO_VERIFY_LEAF_SIGNATURE", "ABORT_ERR", +]); + +// Inspect data properties only. A provider error can contain getters, cycles, +// arbitrary messages or nested request objects; none are diagnostic payloads. +function ownValue(value: unknown, key: string): unknown { + if (!value || typeof value !== "object") return undefined; + try { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + return descriptor && "value" in descriptor ? descriptor.value : undefined; + } catch { + return undefined; + } +} + +function networkCode(error: unknown): string { + let current = error; + for (let depth = 0; depth < 4; depth++) { + const code = ownValue(current, "code"); + if (typeof code === "string" && networkCodes.has(code)) return code; + current = ownValue(current, "cause"); + } + return "unknown"; +} + +/** Server-only diagnostics. The existing HTTP response details stay unchanged. */ +export class CloudPortfolioError extends HttpError { + readonly #diagnostics: Readonly>; + + constructor( + kind: "upstream" | "invalid_response", + input: { phase: CloudPortfolioPhase; elapsedMs: number; upstreamStatus: number | null; deadlineExceeded?: boolean }, + error?: unknown, + ) { + super(502, kind === "invalid_response" + ? "Paperclip Cloud portfolio returned invalid JSON" + : "Paperclip Cloud portfolio request failed", { + code: kind === "invalid_response" ? "cloud_portfolio_invalid_response" : "cloud_portfolio_upstream_error", + }); + Error.captureStackTrace?.(this, CloudPortfolioError); + const phase = phases.includes(input.phase) ? input.phase : "unknown"; + this.#diagnostics = Object.freeze({ + phase, + upstreamStatus: Number.isInteger(input.upstreamStatus) && input.upstreamStatus! >= 100 && input.upstreamStatus! <= 599 + ? input.upstreamStatus : null, + elapsedMs: Number.isFinite(input.elapsedMs) && input.elapsedMs >= 0 && input.elapsedMs <= 60_000 + ? Math.round(input.elapsedMs) : null, + networkCode: phase === "fetch" || phase === "response_body" + ? input.deadlineExceeded === true ? "DEADLINE_EXCEEDED" : networkCode(error) + : "unknown", + }); + } + + get diagnostics() { + return this.#diagnostics; + } + + static diagnosticsFor(error: CloudPortfolioError) { + return error.#diagnostics; + } +} From 1477d1ecea0d4fce0a906ac47ad1fc4eea9d3428 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Tue, 6 Oct 2026 07:19:15 -0700 Subject: [PATCH 4/6] test: remove the no-op sequential describe modifier (#15286) 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. > - Paperclip runs the server and runner test suites with Vitest. > - Vitest 5 removes the deprecated `describe.sequential` property, so the pending Vitest 5 upgrade fails the type-check and test jobs. > - `describe.sequential` only changes behaviour inside a `describe.concurrent` suite, or when `sequence.concurrent` is on. > - This repository has neither, so the modifier changed nothing at run time. > - The benefit is that the Vitest 5 upgrade can land, and the test files lose a modifier that did no work. ## Linked Issues or Issue Description Refs: #12969 ## What Changed - Replace every `describe.sequential` use with a plain `describe` call. - Drop the `{ concurrent: false }` suite option from the two runner test files. - Add a comment to `server/vitest.config.ts` that records why these suites must run one test at a time. - Leave the package manifests and the lockfile unchanged. ## Why the modifier did nothing The Vitest documentation states that `describe.sequential` is useful to run tests in sequence inside a `describe.concurrent` suite, or with the `--sequence.concurrent` option. `sequence.concurrent` defaults to `false`. This repository satisfies neither condition: - No test file uses `describe.concurrent`, `it.concurrent`, or `test.concurrent`. - `server/vitest.config.ts` sets `sequence.concurrent: false`, with `maxWorkers: 1`, `maxConcurrency: 1`, and `isolate: true`. - `packages/paperclip-runner/vitest.config.ts` sets no `sequence` block, so the `false` default applies. `packages/db` and `cli` already run the same embedded-Postgres suites with a plain `describe`, and those jobs are green. The server package was the only outlier. The modifier did carry one real piece of knowledge: these suites need their tests to run one at a time. The new comment in `server/vitest.config.ts` records that reason next to the setting that enforces it. ## Verification - `git grep` for `describe.sequential` returns nothing outside `node_modules`. - The author ran the changed server test files under the installed Vitest 4, and the results match the results without this change. - Two very large embedded-Postgres test files exceeded the author's local memory limit, so the CI test jobs cover those two. - The two changed runner test files have pre-existing local failures caused by a missing Rust toolchain and a missing global `pnpm` binary. The failures are identical with and without this change. - The author type-checked the changed files and found no new error. - CI must pass the typecheck, build, server test, and runner verify jobs. ## Risks - Low risk. Suite execution stays serial, because the Vitest config enforces it. - The change adds no dependency and changes no package manifest or lockfile. - A future change that turns `sequence.concurrent` on would break these suites. The new config comment warns against it. ## Model Used - Claude Sonnet 5 — code edits and local verification. - OpenAI Codex, GPT-5 — the earlier revision of this branch. ## Test plan - [x] Every CI check reaches a terminal green state. A pending or queued check is not a pass. - [x] The `Typecheck + Release Registry` job passes. This change must not introduce a type error. - [x] The `Build` job passes. - [x] The server test jobs and the runner verify jobs pass. - [x] Greptile re-reviews this commit set and posts a passing verdict. The dependabot waiver does not apply to this pull request. - [x] `mergeable` reads `MERGEABLE` as a terminal value. ## 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 linked the related public issue with `Refs: #12969` - [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 and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-Authored-By: Priya Raman --------- Co-authored-by: Priya Raman Co-authored-by: Paperclip Co-authored-by: Claude Sonnet 5 Co-authored-by: nickyleach <331803+nickyleach@users.noreply.github.com> --- .../control-plane/durable-prp-control-plane.test.ts | 2 +- .../src/mock-core/local-runner.test.ts | 2 +- server/src/__tests__/activity-routes.test.ts | 2 +- server/src/__tests__/adapter-routes-authz.test.ts | 2 +- .../__tests__/agent-cross-tenant-authz-routes.test.ts | 2 +- server/src/__tests__/agent-permissions-routes.test.ts | 2 +- server/src/__tests__/agent-skills-routes.test.ts | 2 +- server/src/__tests__/auth-routes.test.ts | 2 +- server/src/__tests__/cases-routes.test.ts | 2 +- server/src/__tests__/chat-channels.integration.test.ts | 2 +- server/src/__tests__/cli-auth-routes.test.ts | 2 +- .../companies-route-cross-company-authz.test.ts | 2 +- .../__tests__/company-import-transfer-routes.test.ts | 2 +- .../src/__tests__/company-portability-routes.test.ts | 2 +- .../company-skills-import-authz-routes.test.ts | 2 +- .../environment-selection-route-guards.test.ts | 2 +- ...xecution-workspace-runtime-control-conflict.test.ts | 2 +- .../src/__tests__/execution-workspaces-routes.test.ts | 2 +- server/src/__tests__/github-connection-events.test.ts | 2 +- .../src/__tests__/invite-test-resolution-route.test.ts | 2 +- .../__tests__/issue-closed-workspace-routes.test.ts | 2 +- .../src/__tests__/issue-comment-cancel-routes.test.ts | 2 +- server/src/__tests__/issue-comment-redaction.test.ts | 2 +- .../src/__tests__/issue-comment-reopen-routes.test.ts | 2 +- .../src/__tests__/issue-queued-comments-routes.test.ts | 2 +- .../__tests__/issue-thread-interaction-routes.test.ts | 2 +- .../src/__tests__/issues-goal-context-routes.test.ts | 2 +- .../src/__tests__/merged-pr-confirmation-sweep.test.ts | 2 +- .../src/__tests__/multilingual-issues-routes.test.ts | 2 +- .../src/__tests__/openclaw-invite-prompt-route.test.ts | 2 +- .../src/__tests__/photon/channel.integration.test.ts | 2 +- server/src/__tests__/pipelines-routes.test.ts | 2 +- server/src/__tests__/plugin-install-autobuild.test.ts | 2 +- .../__tests__/plugin-install-route-security.test.ts | 2 +- server/src/__tests__/plugin-routes-authz.test.ts | 10 +++++----- server/src/__tests__/plugin-scoped-api-routes.test.ts | 2 +- server/src/__tests__/plugin-secrets-handler.test.ts | 2 +- server/src/__tests__/routines-e2e.test.ts | 2 +- .../teams-catalog-install-no-overrides.test.ts | 2 +- .../__tests__/workspace-runtime-routes-authz.test.ts | 2 +- server/src/__tests__/write-path-membership.test.ts | 2 +- .../chat-discord-command-registration-store.test.ts | 2 +- .../src/services/chat-teams-file-publication.test.ts | 2 +- server/src/services/chat-teams-file-transfers.test.ts | 2 +- .../native-warm-transition-restart.integration.test.ts | 2 +- server/vitest.config.ts | 3 +++ 46 files changed, 52 insertions(+), 49 deletions(-) diff --git a/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts b/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts index 8980c1c4e37..03ba01a956c 100644 --- a/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts +++ b/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts @@ -1445,7 +1445,7 @@ it("refuses a processing-drain absence claim while local or remote ingress remai } }); -describe.sequential("DurablePrpControlPlane", () => { +describe("DurablePrpControlPlane", () => { it.each(["pending_first", "all_pending", "completed_first"] as const)( "retains unanswered semantic input across the bounded event window (%s)", async (mode) => { diff --git a/packages/paperclip-runner/src/mock-core/local-runner.test.ts b/packages/paperclip-runner/src/mock-core/local-runner.test.ts index 8566e516e3d..3fe96dcdfe0 100644 --- a/packages/paperclip-runner/src/mock-core/local-runner.test.ts +++ b/packages/paperclip-runner/src/mock-core/local-runner.test.ts @@ -13,7 +13,7 @@ import { startLocalRunnerScenario, } from "./local-runner.js"; -describe.sequential("Local runner and fake harness", () => { +describe("Local runner and fake harness", () => { it("runs lifecycle, tool, file, structured-result, and process-exit events", async () => { const trace = await runLocalRunnerScenario({ scenario: "happy-path", delayMs: 1 }); const eventTypes = trace.events.map((event) => event.eventType); diff --git a/server/src/__tests__/activity-routes.test.ts b/server/src/__tests__/activity-routes.test.ts index 45871e32363..0bb574486c6 100644 --- a/server/src/__tests__/activity-routes.test.ts +++ b/server/src/__tests__/activity-routes.test.ts @@ -101,7 +101,7 @@ async function requestApp( } } -describe.sequential("activity routes", () => { +describe("activity routes", () => { beforeEach(() => { for (const mock of Object.values(mockActivityService)) mock.mockReset(); for (const mock of Object.values(mockHeartbeatService)) mock.mockReset(); diff --git a/server/src/__tests__/adapter-routes-authz.test.ts b/server/src/__tests__/adapter-routes-authz.test.ts index 685a30bbcb2..22aa7b2c268 100644 --- a/server/src/__tests__/adapter-routes-authz.test.ts +++ b/server/src/__tests__/adapter-routes-authz.test.ts @@ -258,7 +258,7 @@ function resetInstalledExternalAdapterState() { setOverridePaused("claude_local", false); } -describe.sequential("adapter management route authorization", () => { +describe("adapter management route authorization", () => { beforeEach(async () => { vi.resetModules(); vi.doUnmock("../services/adapter-plugin-store.js"); diff --git a/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts index d5fabca417e..9571e69b133 100644 --- a/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts +++ b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts @@ -343,7 +343,7 @@ function resetMockDefaults() { mockLogActivity.mockImplementation(async () => undefined); } -describe.sequential("agent cross-tenant route authorization", () => { +describe("agent cross-tenant route authorization", () => { beforeEach(() => { resetMockDefaults(); }); diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 4322844170b..8428ab4e6f5 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -267,7 +267,7 @@ async function requestApp( } } -describe.sequential("agent permission routes", () => { +describe("agent permission routes", () => { const routeModules = hoistModuleGraph(registerModuleMocks, async () => { const [{ errorHandler }, { agentRoutes }] = await Promise.all([ vi.importActual("../middleware/index.js"), diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts index f64229e3d4c..23722b3abf5 100644 --- a/server/src/__tests__/agent-skills-routes.test.ts +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -249,7 +249,7 @@ function makeAgent(adapterType: string) { }; } -describe.sequential("agent skill routes", () => { +describe("agent skill routes", () => { beforeEach(async () => { vi.resetModules(); vi.doUnmock("../routes/agents.js"); diff --git a/server/src/__tests__/auth-routes.test.ts b/server/src/__tests__/auth-routes.test.ts index 7b3588ae07c..08d2a047f09 100644 --- a/server/src/__tests__/auth-routes.test.ts +++ b/server/src/__tests__/auth-routes.test.ts @@ -51,7 +51,7 @@ function createApp(actor: Express.Request["actor"], row: Record return app; } -describe.sequential("auth routes", () => { +describe("auth routes", () => { const baseUser = { id: "user-1", name: "Jane Example", diff --git a/server/src/__tests__/cases-routes.test.ts b/server/src/__tests__/cases-routes.test.ts index d758670c7f7..0fa40f81832 100644 --- a/server/src/__tests__/cases-routes.test.ts +++ b/server/src/__tests__/cases-routes.test.ts @@ -37,7 +37,7 @@ import { instanceSettingsService } from "../services/instance-settings.js"; import type { StorageService } from "../storage/types.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 6798347819f..f8ac3bcc560 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -198,7 +198,7 @@ const embeddedPostgresSupport = externalTestDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported - ? describe.sequential + ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { diff --git a/server/src/__tests__/cli-auth-routes.test.ts b/server/src/__tests__/cli-auth-routes.test.ts index f735c6f13ca..0b30c358567 100644 --- a/server/src/__tests__/cli-auth-routes.test.ts +++ b/server/src/__tests__/cli-auth-routes.test.ts @@ -90,7 +90,7 @@ async function createApp(actor: any, db: any = {} as any) { return app; } -describe.sequential("cli auth routes", () => { +describe("cli auth routes", () => { beforeEach(() => { vi.resetModules(); vi.doUnmock("../services/index.js"); diff --git a/server/src/__tests__/companies-route-cross-company-authz.test.ts b/server/src/__tests__/companies-route-cross-company-authz.test.ts index 58adaa4aa54..4e49604c9dc 100644 --- a/server/src/__tests__/companies-route-cross-company-authz.test.ts +++ b/server/src/__tests__/companies-route-cross-company-authz.test.ts @@ -215,7 +215,7 @@ function boardActor(input: { }; } -describe.sequential("company route cross-company authorization", () => { +describe("company route cross-company authorization", () => { beforeEach(() => { vi.resetModules(); vi.doUnmock("../routes/authz.js"); diff --git a/server/src/__tests__/company-import-transfer-routes.test.ts b/server/src/__tests__/company-import-transfer-routes.test.ts index b1dd3f9d840..dae45dbd46c 100644 --- a/server/src/__tests__/company-import-transfer-routes.test.ts +++ b/server/src/__tests__/company-import-transfer-routes.test.ts @@ -89,7 +89,7 @@ vi.mock("../services/company-import-transfers.js", async (importOriginal) => { const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported - ? describe.sequential + ? describe : describe.skip; const companyId = "11111111-1111-4111-8111-111111111111"; diff --git a/server/src/__tests__/company-portability-routes.test.ts b/server/src/__tests__/company-portability-routes.test.ts index 9872cd2f6b1..bea84600887 100644 --- a/server/src/__tests__/company-portability-routes.test.ts +++ b/server/src/__tests__/company-portability-routes.test.ts @@ -334,7 +334,7 @@ const importMeta = { collisionStrategy: importRequest.collisionStrategy, }; -describe.sequential("company portability routes", () => { +describe("company portability routes", () => { beforeEach(() => { vi.clearAllMocks(); mockAgentService.getById.mockImplementation(async (id: string) => ({ diff --git a/server/src/__tests__/company-skills-import-authz-routes.test.ts b/server/src/__tests__/company-skills-import-authz-routes.test.ts index b7a945d1e41..a889767582a 100644 --- a/server/src/__tests__/company-skills-import-authz-routes.test.ts +++ b/server/src/__tests__/company-skills-import-authz-routes.test.ts @@ -26,7 +26,7 @@ import { errorHandler } from "../middleware/error-handler.js"; import { companySkillRoutes } from "../routes/company-skills.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/environment-selection-route-guards.test.ts b/server/src/__tests__/environment-selection-route-guards.test.ts index 195753a1264..ff0b60ee1f9 100644 --- a/server/src/__tests__/environment-selection-route-guards.test.ts +++ b/server/src/__tests__/environment-selection-route-guards.test.ts @@ -164,7 +164,7 @@ async function closeServer(server: Server | null) { }); } -describe.sequential("execution environment route guards", () => { +describe("execution environment route guards", () => { afterAll(async () => { await closeServer(projectServer); await closeServer(issueServer); diff --git a/server/src/__tests__/execution-workspace-runtime-control-conflict.test.ts b/server/src/__tests__/execution-workspace-runtime-control-conflict.test.ts index c733af6a365..07976472721 100644 --- a/server/src/__tests__/execution-workspace-runtime-control-conflict.test.ts +++ b/server/src/__tests__/execution-workspace-runtime-control-conflict.test.ts @@ -236,7 +236,7 @@ function mockVerifiedReseed( * stable 409 while a control is genuinely live, and a start that fails must leave the workspace * stopped and retryable instead of "desired running" with residue. */ -describe.sequential("execution workspace runtime control conflict and failure reconciliation", () => { +describe("execution workspace runtime control conflict and failure reconciliation", () => { beforeEach(() => { vi.clearAllMocks(); mockSpawn.mockReset(); diff --git a/server/src/__tests__/execution-workspaces-routes.test.ts b/server/src/__tests__/execution-workspaces-routes.test.ts index 89111b9b0aa..8f5e1c193b3 100644 --- a/server/src/__tests__/execution-workspaces-routes.test.ts +++ b/server/src/__tests__/execution-workspaces-routes.test.ts @@ -88,7 +88,7 @@ function createApp(actor: Record = { return app; } -describe.sequential("execution workspace routes", () => { +describe("execution workspace routes", () => { beforeEach(() => { vi.clearAllMocks(); mockAccessService.decide.mockResolvedValue({ diff --git a/server/src/__tests__/github-connection-events.test.ts b/server/src/__tests__/github-connection-events.test.ts index aef64320619..905e6e7ea33 100644 --- a/server/src/__tests__/github-connection-events.test.ts +++ b/server/src/__tests__/github-connection-events.test.ts @@ -22,7 +22,7 @@ import { const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; -describeEmbeddedPostgres.sequential("GitHub connection event delivery", () => { +describeEmbeddedPostgres("GitHub connection event delivery", () => { let db!: ReturnType; let tempDb: Awaited> | null = null; diff --git a/server/src/__tests__/invite-test-resolution-route.test.ts b/server/src/__tests__/invite-test-resolution-route.test.ts index 7ca22a99c4f..631b49cb558 100644 --- a/server/src/__tests__/invite-test-resolution-route.test.ts +++ b/server/src/__tests__/invite-test-resolution-route.test.ts @@ -74,7 +74,7 @@ async function createApp( return app; } -describe.sequential("GET /invites/:token/test-resolution", () => { +describe("GET /invites/:token/test-resolution", () => { beforeEach(() => { vi.clearAllMocks(); }); diff --git a/server/src/__tests__/issue-closed-workspace-routes.test.ts b/server/src/__tests__/issue-closed-workspace-routes.test.ts index b15070a9065..4d405a71b13 100644 --- a/server/src/__tests__/issue-closed-workspace-routes.test.ts +++ b/server/src/__tests__/issue-closed-workspace-routes.test.ts @@ -204,7 +204,7 @@ async function assertNoBackgroundClearWithinRetryWindow() { expect(mockExecutionWorkspaceService.clearReopenPendingConsumptionForUnconsumedReopen).not.toHaveBeenCalled(); } -describe.sequential("closed isolated workspace issue routes", () => { +describe("closed isolated workspace issue routes", () => { const routeModules = hoistModuleGraph(registerServiceMocks, async () => { const [{ issueRoutes }, { errorHandler }] = await Promise.all([ vi.importActual("../routes/issues.js"), diff --git a/server/src/__tests__/issue-comment-cancel-routes.test.ts b/server/src/__tests__/issue-comment-cancel-routes.test.ts index 289a4dbdfa0..e4ef7fb9746 100644 --- a/server/src/__tests__/issue-comment-cancel-routes.test.ts +++ b/server/src/__tests__/issue-comment-cancel-routes.test.ts @@ -191,7 +191,7 @@ function makeComment(overrides: Record = {}) { }; } -describe.sequential("issue comment cancel routes", () => { +describe("issue comment cancel routes", () => { const routeModules = hoistModuleGraph(registerModuleMocks, async () => { const [{ issueRoutes }, { errorHandler }] = await Promise.all([ vi.importActual("../routes/issues.js"), diff --git a/server/src/__tests__/issue-comment-redaction.test.ts b/server/src/__tests__/issue-comment-redaction.test.ts index d44a9113360..a34b27ae2d5 100644 --- a/server/src/__tests__/issue-comment-redaction.test.ts +++ b/server/src/__tests__/issue-comment-redaction.test.ts @@ -43,7 +43,7 @@ const embeddedPostgresSupport = externalTestDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported - ? describe.sequential + ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index 23443339205..d0cc06b0f2d 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -317,7 +317,7 @@ async function waitForWakeup(assertion: () => void) { await vi.waitFor(assertion); } -describe.sequential("issue comment reopen routes", () => { +describe("issue comment reopen routes", () => { beforeEach(() => { vi.clearAllMocks(); mockIssueService.getById.mockReset(); diff --git a/server/src/__tests__/issue-queued-comments-routes.test.ts b/server/src/__tests__/issue-queued-comments-routes.test.ts index 552cab5c486..bb42982787b 100644 --- a/server/src/__tests__/issue-queued-comments-routes.test.ts +++ b/server/src/__tests__/issue-queued-comments-routes.test.ts @@ -45,7 +45,7 @@ vi.mock("../services/native-runtime/native-session-executor.js", async (importOr const { NativeSessionSteeringError } = await import("../services/native-runtime/native-session-executor.js"); const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/issue-thread-interaction-routes.test.ts b/server/src/__tests__/issue-thread-interaction-routes.test.ts index f44c061209d..e2db55feecd 100644 --- a/server/src/__tests__/issue-thread-interaction-routes.test.ts +++ b/server/src/__tests__/issue-thread-interaction-routes.test.ts @@ -308,7 +308,7 @@ async function resolveMockInteraction( return interaction; } -describe.sequential("issue thread interaction routes", () => { +describe("issue thread interaction routes", () => { beforeEach(async () => { vi.resetModules(); vi.doUnmock("../routes/issues.js"); diff --git a/server/src/__tests__/issues-goal-context-routes.test.ts b/server/src/__tests__/issues-goal-context-routes.test.ts index 843fd17a379..952dbacaa4f 100644 --- a/server/src/__tests__/issues-goal-context-routes.test.ts +++ b/server/src/__tests__/issues-goal-context-routes.test.ts @@ -190,7 +190,7 @@ const projectGoal = { updatedAt: new Date("2026-03-20T00:00:00Z"), }; -describe.sequential("issue goal context routes", () => { +describe("issue goal context routes", () => { beforeEach(() => { vi.clearAllMocks(); mockAccessService.decide.mockResolvedValue({ diff --git a/server/src/__tests__/merged-pr-confirmation-sweep.test.ts b/server/src/__tests__/merged-pr-confirmation-sweep.test.ts index ddc6fe9f096..1e4d56436e7 100644 --- a/server/src/__tests__/merged-pr-confirmation-sweep.test.ts +++ b/server/src/__tests__/merged-pr-confirmation-sweep.test.ts @@ -134,7 +134,7 @@ describe("merged pull-request confirmation extraction", () => { const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; -describeEmbeddedPostgres.sequential("merged pull-request confirmation sweep", () => { +describeEmbeddedPostgres("merged pull-request confirmation sweep", () => { let db!: ReturnType; let tempDb: Awaited> | null = null; diff --git a/server/src/__tests__/multilingual-issues-routes.test.ts b/server/src/__tests__/multilingual-issues-routes.test.ts index 4b0eac47765..3ed24b5b8e5 100644 --- a/server/src/__tests__/multilingual-issues-routes.test.ts +++ b/server/src/__tests__/multilingual-issues-routes.test.ts @@ -12,7 +12,7 @@ import { issueRoutes } from "../routes/issues.js"; import type { StorageService } from "../storage/types.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/openclaw-invite-prompt-route.test.ts b/server/src/__tests__/openclaw-invite-prompt-route.test.ts index a50de066c0d..3599e2bc924 100644 --- a/server/src/__tests__/openclaw-invite-prompt-route.test.ts +++ b/server/src/__tests__/openclaw-invite-prompt-route.test.ts @@ -126,7 +126,7 @@ function createApp(actor: Record, db: Record) return app; } -describe.sequential("POST /companies/:companyId/openclaw/invite-prompt", () => { +describe("POST /companies/:companyId/openclaw/invite-prompt", () => { const companyBranding = { name: "Acme AI", logoAssetId: "logo-1", diff --git a/server/src/__tests__/photon/channel.integration.test.ts b/server/src/__tests__/photon/channel.integration.test.ts index 257992f8ee3..1c33b0a565b 100644 --- a/server/src/__tests__/photon/channel.integration.test.ts +++ b/server/src/__tests__/photon/channel.integration.test.ts @@ -73,7 +73,7 @@ function idleStream(): TypedEventStream { async () => close(), ); } -describe.sequential("iMessage Photon channel control plane", () => { +describe("iMessage Photon channel control plane", () => { let database: Awaited>; let db: ReturnType; let secrets: string; diff --git a/server/src/__tests__/pipelines-routes.test.ts b/server/src/__tests__/pipelines-routes.test.ts index cfdc6c1fb4e..3cb7bb7243d 100644 --- a/server/src/__tests__/pipelines-routes.test.ts +++ b/server/src/__tests__/pipelines-routes.test.ts @@ -46,7 +46,7 @@ import { import { instanceSettingsService } from "../services/instance-settings.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/plugin-install-autobuild.test.ts b/server/src/__tests__/plugin-install-autobuild.test.ts index d9d3a599610..79fec328c51 100644 --- a/server/src/__tests__/plugin-install-autobuild.test.ts +++ b/server/src/__tests__/plugin-install-autobuild.test.ts @@ -38,7 +38,7 @@ vi.mock("../services/live-events.js", () => ({ })); const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/plugin-install-route-security.test.ts b/server/src/__tests__/plugin-install-route-security.test.ts index ff8ad17be49..eedde8598be 100644 --- a/server/src/__tests__/plugin-install-route-security.test.ts +++ b/server/src/__tests__/plugin-install-route-security.test.ts @@ -43,7 +43,7 @@ vi.mock("../services/live-events.js", () => ({ })); const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index 3bded76b2e2..35264ee14cf 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -144,7 +144,7 @@ function readyPlugin() { }); } -describe.sequential("plugin install and upgrade authz", () => { +describe("plugin install and upgrade authz", () => { beforeEach(() => { vi.clearAllMocks(); }); @@ -410,7 +410,7 @@ describe.sequential("plugin install and upgrade authz", () => { }, 20_000); }); -describe.sequential("scoped plugin API routes", () => { +describe("scoped plugin API routes", () => { beforeEach(() => { vi.clearAllMocks(); }); @@ -476,7 +476,7 @@ describe.sequential("scoped plugin API routes", () => { }, 20_000); }); -describe.sequential("plugin local folder routes", () => { +describe("plugin local folder routes", () => { beforeEach(() => { vi.clearAllMocks(); mockRegistry.getCompanySettings.mockResolvedValue(null); @@ -531,7 +531,7 @@ describe.sequential("plugin local folder routes", () => { }); }); -describe.sequential("plugin tool and bridge authz", () => { +describe("plugin tool and bridge authz", () => { beforeEach(() => { vi.clearAllMocks(); }); @@ -1104,7 +1104,7 @@ describe.sequential("plugin tool and bridge authz", () => { }); }); -describe.sequential("operator-hidden plugin management floor", () => { +describe("operator-hidden plugin management floor", () => { beforeEach(() => { vi.clearAllMocks(); process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.plugins"; diff --git a/server/src/__tests__/plugin-scoped-api-routes.test.ts b/server/src/__tests__/plugin-scoped-api-routes.test.ts index 4e2ce297bc0..36e8444197f 100644 --- a/server/src/__tests__/plugin-scoped-api-routes.test.ts +++ b/server/src/__tests__/plugin-scoped-api-routes.test.ts @@ -94,7 +94,7 @@ async function createApp(input: { return { app, workerManager }; } -describe.sequential("plugin scoped API routes", () => { +describe("plugin scoped API routes", () => { const pluginId = "11111111-1111-4111-8111-111111111111"; const companyId = "22222222-2222-4222-8222-222222222222"; const agentId = "33333333-3333-4333-8333-333333333333"; diff --git a/server/src/__tests__/plugin-secrets-handler.test.ts b/server/src/__tests__/plugin-secrets-handler.test.ts index f073b65a93e..002384c1cf5 100644 --- a/server/src/__tests__/plugin-secrets-handler.test.ts +++ b/server/src/__tests__/plugin-secrets-handler.test.ts @@ -23,7 +23,7 @@ import { secretService } from "../services/secrets.js"; const pluginId = "11111111-1111-4111-8111-111111111111"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/routines-e2e.test.ts b/server/src/__tests__/routines-e2e.test.ts index 42e7ef0b465..d08c2b51ada 100644 --- a/server/src/__tests__/routines-e2e.test.ts +++ b/server/src/__tests__/routines-e2e.test.ts @@ -87,7 +87,7 @@ function registerRoutineServiceMock() { } const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/teams-catalog-install-no-overrides.test.ts b/server/src/__tests__/teams-catalog-install-no-overrides.test.ts index 7e085dd8e69..562a2f39040 100644 --- a/server/src/__tests__/teams-catalog-install-no-overrides.test.ts +++ b/server/src/__tests__/teams-catalog-install-no-overrides.test.ts @@ -12,7 +12,7 @@ import { import { teamsCatalogService } from "../services/teams-catalog.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); -const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip; +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { console.warn( diff --git a/server/src/__tests__/workspace-runtime-routes-authz.test.ts b/server/src/__tests__/workspace-runtime-routes-authz.test.ts index cd7efe6ea47..98522fd333d 100644 --- a/server/src/__tests__/workspace-runtime-routes-authz.test.ts +++ b/server/src/__tests__/workspace-runtime-routes-authz.test.ts @@ -202,7 +202,7 @@ function buildExecutionWorkspace(overrides: Record = {}) { }; } -describe.sequential("workspace runtime service route authorization", () => { +describe("workspace runtime service route authorization", () => { const projectId = "11111111-1111-4111-8111-111111111111"; const workspaceId = "22222222-2222-4222-8222-222222222222"; const executionWorkspaceId = "33333333-3333-4333-8333-333333333333"; diff --git a/server/src/__tests__/write-path-membership.test.ts b/server/src/__tests__/write-path-membership.test.ts index 2a4a7658439..39d25762185 100644 --- a/server/src/__tests__/write-path-membership.test.ts +++ b/server/src/__tests__/write-path-membership.test.ts @@ -116,7 +116,7 @@ function resetMocks() { mockGetTelemetryClient.mockReturnValue({ track: vi.fn() }); } -describe.sequential("write-path membership checks (viewer / inactive)", () => { +describe("write-path membership checks (viewer / inactive)", () => { beforeEach(() => { resetMocks(); }); diff --git a/server/src/services/chat-discord-command-registration-store.test.ts b/server/src/services/chat-discord-command-registration-store.test.ts index 41dedaae65e..02f8c877475 100644 --- a/server/src/services/chat-discord-command-registration-store.test.ts +++ b/server/src/services/chat-discord-command-registration-store.test.ts @@ -32,7 +32,7 @@ const external = process.env.PAPERCLIP_TEST_DATABASE_URL; const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport(); -const suite = support.supported ? describe.sequential : describe.skip; +const suite = support.supported ? describe : describe.skip; const token = "PRIVATE-DISCORD-REGISTRATION-TOKEN"; let serial = 0n; const appId = () => diff --git a/server/src/services/chat-teams-file-publication.test.ts b/server/src/services/chat-teams-file-publication.test.ts index fbea0cf7968..72f48c6f93e 100644 --- a/server/src/services/chat-teams-file-publication.test.ts +++ b/server/src/services/chat-teams-file-publication.test.ts @@ -29,7 +29,7 @@ const external = process.env.PAPERCLIP_TEST_DATABASE_URL; const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport(); -const suite = support.supported ? describe.sequential : describe.skip; +const suite = support.supported ? describe : describe.skip; suite("Teams same-transaction publication projection (real PostgreSQL)", () => { let db: ReturnType; let temporary: diff --git a/server/src/services/chat-teams-file-transfers.test.ts b/server/src/services/chat-teams-file-transfers.test.ts index 7b5814dd7e1..ad6b5084c17 100644 --- a/server/src/services/chat-teams-file-transfers.test.ts +++ b/server/src/services/chat-teams-file-transfers.test.ts @@ -48,7 +48,7 @@ const external = process.env.PAPERCLIP_TEST_DATABASE_URL; const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport(); -const suite = support.supported ? describe.sequential : describe.skip; +const suite = support.supported ? describe : describe.skip; const bytes = Buffer.from("Exact Teams file bytes\n"); const uploadUrl = "https://tenant-my.sharepoint.com/personal/user/_api/upload?token=PRIVATE-TEAMS-UPLOAD-CANARY"; diff --git a/server/src/services/native-runtime/native-warm-transition-restart.integration.test.ts b/server/src/services/native-runtime/native-warm-transition-restart.integration.test.ts index 7fd2fe0ac19..c60b376fed5 100644 --- a/server/src/services/native-runtime/native-warm-transition-restart.integration.test.ts +++ b/server/src/services/native-runtime/native-warm-transition-restart.integration.test.ts @@ -44,7 +44,7 @@ const managedFixtureDirectory = process.env.PAPERCLIP_WARM_TRANSITION_MANAGED_FIXTURE_DIRECTORY; const databaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL; const generated = - fixtureDirectory && databaseUrl ? describe.sequential : describe.skip; + fixtureDirectory && databaseUrl ? describe : describe.skip; type Identity = { runnerInstanceId: string; diff --git a/server/vitest.config.ts b/server/vitest.config.ts index 239bce1bc3b..71e8df328a2 100644 --- a/server/vitest.config.ts +++ b/server/vitest.config.ts @@ -39,6 +39,9 @@ export default defineConfig({ maxWorkers: 1, minWorkers: 1, pool: "forks", + // Server suites share process state and one embedded Postgres instance, + // so tests inside a file must run one at a time. Do not set + // sequence.concurrent to true. sequence: { concurrent: false, hooks: "list", From 44e4979d23bfb0728baad89f844033e1b77faefe Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Tue, 6 Oct 2026 07:33:18 -0700 Subject: [PATCH 5/6] Capture project and repository update lifecycle events (#15306) ## Thinking Path > - Existing lifecycle capture records agent transitions and project creation. > - Project and repository edits need matching project update records. > - The record must commit with the mutation so failed writes cannot lose a hook. > - Creation with repositories is one creation, and repository replacement is one update. > - Archive-only changes preserve project state without creating a hook. > - Plugin consumption and provider behavior are separate work. ## Linked Issues or Issue Description **Problem or motivation** Project edits and repository/workspace changes lack durable lifecycle records. A future resource plugin needs those changes captured alongside the existing project creation hook. Archive-only changes must produce no hook. **Proposed solution** Allow project `update` records in the existing lifecycle journal. Record project and workspace mutations in their database transaction while holding the project row lock. Suppress intermediate workspace hooks during project creation and aggregate repository replacement. **Alternatives considered** Route-only hooks miss shared service callers. Recording after commit can lose an event. Emitting a hook for each child mutation exposes intermediate repository state. **Roadmap alignment** This completes project lifecycle capture begun in #15280. Plugin delivery, VM/volume provisioning, and backfill remain separate. Searches found no duplicate project lifecycle work; related #13306 concerns decision events on the in-process plugin bus. ## What Changed - Record project edits and workspace additions, updates, and removals as project `update` events. - Commit each event atomically with its mutation under the project row lock. - Keep project creation with repositories to one creation event and repository replacement to one aggregate update. - Ignore archive-only changes and retain workspace records. - Extend the journal action constraint and document project update capture. ## Verification - `pnpm -r typecheck` passed on the narrowed scope. - 57 tests passed across six lifecycle, project, repository, and chat-project suites. - Seven managed-sandbox workspace route tests and the CLI lagging-worktree migration regression passed (65 targeted tests total). - Full GitHub CI passed on `8ba6f97f22`; all required gates are green. - Greptile scored the final project-only commit 5/5 with zero unresolved review threads. - The branch is current with `master` and has no merge conflicts. - `git diff --check` and a local secret/PII scan passed. ## Risks - Apply migration `0300_chunky_chamber.sql` before running the new server. It permits project update actions and tolerates older JavaScript worktree backups that omitted the prior CHECK constraint. - Event-write failure intentionally rolls back the project or repository mutation. - Records contain identity and action; future consumers must load current authorized project/workspace data. - Plugin consumption, provider calls, volume cleanup, and backfill are outside this PR. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository inspection, code execution, and tool use. The exact deployment model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/DATABASE.md | 4 + .../db/src/migrations/0300_chunky_chamber.sql | 3 + ...{0295_snapshot.json => 0300_snapshot.json} | 666 +++++++++++++++++- packages/db/src/migrations/meta/_journal.json | 7 + .../src/schema/resource_lifecycle_events.ts | 4 +- .../resource-lifecycle-events.test.ts | 47 +- server/src/services/projects.ts | 61 +- .../src/services/resource-lifecycle-events.ts | 5 + 8 files changed, 767 insertions(+), 30 deletions(-) create mode 100644 packages/db/src/migrations/0300_chunky_chamber.sql rename packages/db/src/migrations/meta/{0295_snapshot.json => 0300_snapshot.json} (98%) diff --git a/doc/DATABASE.md b/doc/DATABASE.md index 2b4480f9d61..f194265751b 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -471,6 +471,10 @@ A partial unique `(company_id, resource_type, resource_id)` index deduplicates creation. Each actual agent pause, resume, or termination appends another event, including budget actions and generic status updates. The agent row stays locked until status and event commit, so concurrent repeat requests emit one hook. +Project edits and workspace additions, updates, and removals append `update`. +Repository replacement emits one aggregate update; project creation with repositories +emits only creation. Project mutations hold the project row lock until their record +commits. An archive-only change emits nothing and preserves workspace records. Termination commits API-key revocation in that same transaction. Hire approval and rejection commit with agent activation or termination, so a failed event write leaves the decision pending and retryable. diff --git a/packages/db/src/migrations/0300_chunky_chamber.sql b/packages/db/src/migrations/0300_chunky_chamber.sql new file mode 100644 index 00000000000..4b300a0b5bb --- /dev/null +++ b/packages/db/src/migrations/0300_chunky_chamber.sql @@ -0,0 +1,3 @@ +-- Older JavaScript worktree backups may omit CHECK constraints. +ALTER TABLE "resource_lifecycle_events" DROP CONSTRAINT IF EXISTS "resource_lifecycle_events_action_check";--> statement-breakpoint +ALTER TABLE "resource_lifecycle_events" ADD CONSTRAINT "resource_lifecycle_events_action_check" CHECK ("resource_lifecycle_events"."action" = 'create' OR ("resource_lifecycle_events"."resource_type" = 'project' AND "resource_lifecycle_events"."action" = 'update') OR ("resource_lifecycle_events"."resource_type" = 'agent' AND "resource_lifecycle_events"."action" IN ('pause', 'resume', 'terminate'))); \ No newline at end of file diff --git a/packages/db/src/migrations/meta/0295_snapshot.json b/packages/db/src/migrations/meta/0300_snapshot.json similarity index 98% rename from packages/db/src/migrations/meta/0295_snapshot.json rename to packages/db/src/migrations/meta/0300_snapshot.json index 13a13294ad3..ebf532f8cdf 100644 --- a/packages/db/src/migrations/meta/0295_snapshot.json +++ b/packages/db/src/migrations/meta/0300_snapshot.json @@ -1,6 +1,6 @@ { - "id": "02d31b6c-7df3-4f6c-b906-e2d369b5e1cd", - "prevId": "66a0b3c9-91bb-46b8-a868-a6facedbc14b", + "id": "dfa5064e-7b35-4d20-8a3d-aa808c4d92e0", + "prevId": "0eac674e-f69a-4f25-b800-3be01b8fd28d", "version": "7", "dialect": "postgresql", "tables": { @@ -2903,6 +2903,362 @@ }, "isRLSEnabled": false }, + "public.ai_connection_pools": { + "name": "ai_connection_pools", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "plugin_key": { + "name": "plugin_key", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "config": { + "name": "config", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "revision": { + "name": "revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 1 + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "ai_connection_pools_id_tool_connections_id_fk": { + "name": "ai_connection_pools_id_tool_connections_id_fk", + "tableFrom": "ai_connection_pools", + "tableTo": "tool_connections", + "columnsFrom": [ + "id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_pools_company_id_companies_id_fk": { + "name": "ai_connection_pools_company_id_companies_id_fk", + "tableFrom": "ai_connection_pools", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_pools_company_connection_fk": { + "name": "ai_connection_pools_company_connection_fk", + "tableFrom": "ai_connection_pools", + "tableTo": "tool_connections", + "columnsFrom": [ + "company_id", + "id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "ai_connection_pools_company_id_uq": { + "name": "ai_connection_pools_company_id_uq", + "nullsNotDistinct": false, + "columns": [ + "company_id", + "id" + ] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.ai_connection_router_cursors": { + "name": "ai_connection_router_cursors", + "schema": "", + "columns": { + "pool_id": { + "name": "pool_id", + "type": "uuid", + "primaryKey": true, + "notNull": true + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "last_member_id": { + "name": "last_member_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "version": { + "name": "version", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + } + }, + "indexes": {}, + "foreignKeys": { + "ai_connection_router_cursors_pool_id_ai_connection_pools_id_fk": { + "name": "ai_connection_router_cursors_pool_id_ai_connection_pools_id_fk", + "tableFrom": "ai_connection_router_cursors", + "tableTo": "ai_connection_pools", + "columnsFrom": [ + "pool_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_router_cursors_company_id_companies_id_fk": { + "name": "ai_connection_router_cursors_company_id_companies_id_fk", + "tableFrom": "ai_connection_router_cursors", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_router_cursors_company_pool_fk": { + "name": "ai_connection_router_cursors_company_pool_fk", + "tableFrom": "ai_connection_router_cursors", + "tableTo": "ai_connection_pools", + "columnsFrom": [ + "company_id", + "pool_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.ai_connection_task_pins": { + "name": "ai_connection_task_pins", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "pool_id": { + "name": "pool_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "agent_id": { + "name": "agent_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "task_key": { + "name": "task_key", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "selection": { + "name": "selection", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "member": { + "name": "member", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "ai_connection_task_pins_affinity_uq": { + "name": "ai_connection_task_pins_affinity_uq", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "pool_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "agent_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "task_key", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "ai_connection_task_pins_company_id_companies_id_fk": { + "name": "ai_connection_task_pins_company_id_companies_id_fk", + "tableFrom": "ai_connection_task_pins", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_task_pins_pool_id_ai_connection_pools_id_fk": { + "name": "ai_connection_task_pins_pool_id_ai_connection_pools_id_fk", + "tableFrom": "ai_connection_task_pins", + "tableTo": "ai_connection_pools", + "columnsFrom": [ + "pool_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_task_pins_agent_id_agents_id_fk": { + "name": "ai_connection_task_pins_agent_id_agents_id_fk", + "tableFrom": "ai_connection_task_pins", + "tableTo": "agents", + "columnsFrom": [ + "agent_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_task_pins_company_pool_fk": { + "name": "ai_connection_task_pins_company_pool_fk", + "tableFrom": "ai_connection_task_pins", + "tableTo": "ai_connection_pools", + "columnsFrom": [ + "company_id", + "pool_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "ai_connection_task_pins_company_agent_fk": { + "name": "ai_connection_task_pins_company_agent_fk", + "tableFrom": "ai_connection_task_pins", + "tableTo": "agents", + "columnsFrom": [ + "company_id", + "agent_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.ai_provider_defaults": { "name": "ai_provider_defaults", "schema": "", @@ -12347,6 +12703,13 @@ "primaryKey": false, "notNull": false }, + "ai_session_epoch": { + "name": "ai_session_epoch", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + }, "latest_version": { "name": "latest_version", "type": "integer", @@ -38918,6 +39281,140 @@ }, "isRLSEnabled": false }, + "public.resource_lifecycle_events": { + "name": "resource_lifecycle_events", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "bigint", + "primaryKey": true, + "notNull": true, + "identity": { + "type": "always", + "name": "resource_lifecycle_events_id_seq", + "schema": "public", + "increment": "1", + "startWith": "1", + "minValue": "1", + "maxValue": "9223372036854775807", + "cache": "1", + "cycle": false + } + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "resource_type": { + "name": "resource_type", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "resource_id": { + "name": "resource_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "action": { + "name": "action", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "resource_lifecycle_events_creation_idx": { + "name": "resource_lifecycle_events_creation_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "resource_type", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "resource_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "where": "\"resource_lifecycle_events\".\"action\" = 'create'", + "concurrently": false, + "method": "btree", + "with": {} + }, + "resource_lifecycle_events_company_idx": { + "name": "resource_lifecycle_events_company_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "resource_lifecycle_events_company_id_companies_id_fk": { + "name": "resource_lifecycle_events_company_id_companies_id_fk", + "tableFrom": "resource_lifecycle_events", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": { + "resource_lifecycle_events_resource_type_check": { + "name": "resource_lifecycle_events_resource_type_check", + "value": "\"resource_lifecycle_events\".\"resource_type\" IN ('agent', 'project')" + }, + "resource_lifecycle_events_action_check": { + "name": "resource_lifecycle_events_action_check", + "value": "\"resource_lifecycle_events\".\"action\" = 'create' OR (\"resource_lifecycle_events\".\"resource_type\" = 'project' AND \"resource_lifecycle_events\".\"action\" = 'update') OR (\"resource_lifecycle_events\".\"resource_type\" = 'agent' AND \"resource_lifecycle_events\".\"action\" IN ('pause', 'resume', 'terminate'))" + } + }, + "isRLSEnabled": false + }, "public.routine_documents": { "name": "routine_documents", "schema": "", @@ -44557,6 +45054,12 @@ "primaryKey": false, "notNull": true, "default": "now()" + }, + "error_at": { + "name": "error_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false } }, "indexes": { @@ -44630,6 +45133,159 @@ "checkConstraints": {}, "isRLSEnabled": false }, + "public.tool_connection_app_syncs": { + "name": "tool_connection_app_syncs", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "connection_id": { + "name": "connection_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "credential_key": { + "name": "credential_key", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "lease_id": { + "name": "lease_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "checked": { + "name": "checked", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + }, + "total": { + "name": "total", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + }, + "failed": { + "name": "failed", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + }, + "last_completed_at": { + "name": "last_completed_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "tool_connection_app_syncs_owner_key_uq": { + "name": "tool_connection_app_syncs_owner_key_uq", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "connection_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "credential_key", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "tool_connection_app_syncs_company_id_companies_id_fk": { + "name": "tool_connection_app_syncs_company_id_companies_id_fk", + "tableFrom": "tool_connection_app_syncs", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "tool_connection_app_syncs_company_connection_fk": { + "name": "tool_connection_app_syncs_company_connection_fk", + "tableFrom": "tool_connection_app_syncs", + "tableTo": "tool_connections", + "columnsFrom": [ + "company_id", + "connection_id" + ], + "columnsTo": [ + "company_id", + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.tool_connection_installs": { "name": "tool_connection_installs", "schema": "", @@ -44923,6 +45579,12 @@ "notNull": true, "default": false }, + "agent_instructions": { + "name": "agent_instructions", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, "config": { "name": "config", "type": "jsonb", diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index e411db27423..67089625939 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -2087,6 +2087,13 @@ "when": 1791248126600, "tag": "0299_absent_ser_duncan", "breakpoints": true + }, + { + "idx": 300, + "version": "7", + "when": 1791260726828, + "tag": "0300_chunky_chamber", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/db/src/schema/resource_lifecycle_events.ts b/packages/db/src/schema/resource_lifecycle_events.ts index 087af412f9a..c9bae44d3d2 100644 --- a/packages/db/src/schema/resource_lifecycle_events.ts +++ b/packages/db/src/schema/resource_lifecycle_events.ts @@ -8,11 +8,11 @@ export const resourceLifecycleEvents = pgTable("resource_lifecycle_events", { companyId: uuid("company_id").notNull().references(() => companies.id, { onDelete: "cascade" }), resourceType: text("resource_type").$type<"agent" | "project">().notNull(), resourceId: uuid("resource_id").notNull(), - action: text("action").$type<"create" | "pause" | "resume" | "terminate">().notNull(), + action: text("action").$type<"create" | "update" | "pause" | "resume" | "terminate">().notNull(), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), }, (table) => ({ resourceIdx: uniqueIndex("resource_lifecycle_events_creation_idx").on(table.companyId, table.resourceType, table.resourceId).where(sql`${table.action} = 'create'`), companyIdx: index("resource_lifecycle_events_company_idx").on(table.companyId, table.id), resourceTypeCheck: check("resource_lifecycle_events_resource_type_check", sql`${table.resourceType} IN ('agent', 'project')`), - actionCheck: check("resource_lifecycle_events_action_check", sql`${table.action} = 'create' OR (${table.resourceType} = 'agent' AND ${table.action} IN ('pause', 'resume', 'terminate'))`), + actionCheck: check("resource_lifecycle_events_action_check", sql`${table.action} = 'create' OR (${table.resourceType} = 'project' AND ${table.action} = 'update') OR (${table.resourceType} = 'agent' AND ${table.action} IN ('pause', 'resume', 'terminate'))`), })); diff --git a/server/src/__tests__/resource-lifecycle-events.test.ts b/server/src/__tests__/resource-lifecycle-events.test.ts index a61e31f51ab..229f514d783 100644 --- a/server/src/__tests__/resource-lifecycle-events.test.ts +++ b/server/src/__tests__/resource-lifecycle-events.test.ts @@ -198,10 +198,10 @@ describePostgres("Resource lifecycle events", () => { await db.insert(projects).values({ id: oldProjectId, companyId, name: "Existing project" }); await agentService(db).update(oldAgentId, { name: "Renamed agent" }); await projectService(db).update(oldProjectId, { name: "Renamed project" }); - expect(await events()).toEqual([]); + expect(await events()).toEqual([expect.objectContaining({ action: "update", resourceId: oldProjectId })]); const agent = await createAgent(); const project = await projectService(db).create(companyId, { name: "New project" }); - expect((await events()).map(row => row.resourceId).sort()).toEqual([agent.id, project.id].sort()); + expect((await events()).filter(row => row.action === "create").map(row => row.resourceId).sort()).toEqual([agent.id, project.id].sort()); }); it("scopes resource identities by company and type", async () => { @@ -218,4 +218,47 @@ describePostgres("Resource lifecycle events", () => { await db.delete(companies).where(eq(companies.id, otherCompanyId)); expect(await db.select().from(resourceLifecycleEvents).where(eq(resourceLifecycleEvents.companyId, otherCompanyId))).toEqual([]); }); + + it("captures project and workspace updates, batches repository replacement, and ignores archive-only changes", async () => { + const service = projectService(db); + const project = await service.createWithRepositories(companyId, { name: "Mutable project" }, [ + { id: "1", fullName: "fixture/one", url: "https://github.com/fixture/one", connections: [] }, + ]); + expect((await events()).map(row => row.action)).toEqual(["create"]); + await service.update(project.id, { name: "Updated project" }); + const workspace = await service.createWorkspace(project.id, { repoUrl: "https://github.com/fixture/two" }); + await service.updateWorkspace(project.id, workspace!.id, { repoRef: "main" }); + await service.removeWorkspace(project.id, workspace!.id); + const beforeReplace = (await events()).length; + await service.replaceRepositories(project.id, [ + { id: "3", fullName: "fixture/three", url: "https://github.com/fixture/three", connections: [] }, + { id: "4", fullName: "fixture/four", url: "https://github.com/fixture/four", connections: [] }, + ]); + expect(await events()).toHaveLength(beforeReplace + 1); + const beforeArchive = await events(); + await service.update(project.id, { archivedAt: new Date(), name: undefined }); + expect(await events()).toEqual(beforeArchive); + expect((await service.getById(project.id))?.workspaces).toHaveLength(2); + expect(beforeArchive.sort((a, b) => a.id - b.id).map(row => row.action)).toEqual(["create", "update", "update", "update", "update", "update"]); + }); + + it("rolls back project and repository mutations when their update record fails", async () => { + const service = projectService(db); + const project = await service.create(companyId, { name: "Retained project" }); + const workspace = await service.createWorkspace(project.id, { repoUrl: "https://github.com/fixture/retained" }); + const before = await events(); + await db.execute(sql`ALTER TABLE resource_lifecycle_events ADD CONSTRAINT fixture_reject_update CHECK (action <> 'update') NOT VALID`); + try { + await expect(service.update(project.id, { name: "Rejected update" })).rejects.toThrow(); + await expect(service.createWorkspace(project.id, { repoUrl: "https://github.com/fixture/rejected" })).rejects.toThrow(); + await expect(service.updateWorkspace(project.id, workspace!.id, { repoRef: "rejected" })).rejects.toThrow(); + await expect(service.removeWorkspace(project.id, workspace!.id)).rejects.toThrow(); + await expect(service.replaceRepositories(project.id, [])).rejects.toThrow(); + expect(await service.getById(project.id)).toMatchObject({ name: "Retained project", workspaces: [expect.objectContaining({ id: workspace!.id, repoRef: null })] }); + expect(await events()).toEqual(before); + } finally { + await db.execute(sql`ALTER TABLE resource_lifecycle_events DROP CONSTRAINT fixture_reject_update`); + } + }); + }); diff --git a/server/src/services/projects.ts b/server/src/services/projects.ts index 40a28a876df..f00057f06a5 100644 --- a/server/src/services/projects.ts +++ b/server/src/services/projects.ts @@ -34,7 +34,7 @@ import { listCurrentRuntimeServicesForProjectWorkspaces } from "./workspace-runt import { parseProjectExecutionWorkspacePolicy } from "./execution-workspace-policy.js"; import { mergeProjectWorkspaceRuntimeConfig, readProjectWorkspaceRuntimeConfig } from "./project-workspace-runtime-config.js"; import { resolveManagedProjectWorkspaceDir } from "../home-paths.js"; -import { recordResourceCreationEvent } from "./resource-lifecycle-events.js"; +import { recordProjectUpdateEvent, recordResourceCreationEvent } from "./resource-lifecycle-events.js"; type ProjectRow = typeof projects.$inferSelect; type ProjectWorkspaceRow = typeof projectWorkspaces.$inferSelect; @@ -568,7 +568,7 @@ async function ensureSinglePrimaryWorkspace( ); } -export function projectService(db: Db) { +export function projectService(db: Db, options: { captureWorkspaceUpdates?: boolean } = {}) { const createProject = async ( companyId: string, data: Omit & { goalIds?: string[] }, @@ -838,7 +838,7 @@ export function projectService(db: Db) { createWithRepositories: async (companyId: string, data: Parameters[1], repositories: import("@paperclipai/shared").ProjectRepository[]): Promise => { return db.transaction(async (tx) => { - const service = projectService(tx as unknown as Db); + const service = projectService(tx as unknown as Db, { captureWorkspaceUpdates: false }); const project = await service.create(companyId, data); for (const repo of repositories) { await service.createWorkspace(project.id, { name: repo.fullName, repoUrl: repo.url, metadata: { githubRepositoryId: repo.id } }); @@ -851,7 +851,7 @@ export function projectService(db: Db) { return db.transaction(async (tx) => { const [project] = await tx.select().from(projects).where(eq(projects.id, projectId)).for("update"); if (!project) return null; - const service = projectService(tx as unknown as Db); + const service = projectService(tx as unknown as Db, { captureWorkspaceUpdates: false }); const existing = await service.listWorkspaces(projectId); const ids = new Set(repositories.map((repo) => repo.id)); for (const workspace of existing) { @@ -878,6 +878,7 @@ export function projectService(db: Db) { } else await service.createWorkspace(projectId, { name: repo.fullName, repoUrl: repo.url, metadata: { githubRepositoryId: repo.id } }); } await tx.update(projects).set({ updatedAt: new Date() }).where(eq(projects.id, projectId)); + await recordProjectUpdateEvent(tx as unknown as Db, project.companyId, projectId); return service.getById(projectId); }); }, @@ -887,13 +888,14 @@ export function projectService(db: Db) { update: async ( id: string, data: Partial & { goalIds?: string[] }, - ): Promise => { + ): Promise => db.transaction(async tx => { + const db = tx as unknown as Db; const { goalIds: inputGoalIds, ...projectData } = data; const ids = resolveGoalIds({ goalIds: inputGoalIds, goalId: projectData.goalId }); const existingProject = await db .select({ id: projects.id, companyId: projects.companyId, name: projects.name }) .from(projects) - .where(eq(projects.id, id)) + .where(eq(projects.id, id)).for("update") .then((rows) => rows[0] ?? null); if (!existingProject) return null; if (ids && ids.length > 0) { @@ -935,10 +937,14 @@ export function projectService(db: Db) { await syncGoalLinks(db, id, row.companyId, ids); } + if (Object.entries(projectData).some(([key, value]) => value !== undefined && key !== "archivedAt" && key !== "updatedAt") || ids !== undefined) { + await recordProjectUpdateEvent(db, row.companyId, id); + } + const [withGoals] = await attachGoals(db, [row]); const [enriched] = withGoals ? await attachWorkspaces(db, [withGoals]) : []; return enriched ?? null; - }, + }), clearExecutionWorkspaceEnvironmentSelection: async (companyId: string, environmentId: string) => { const rows = await db @@ -954,16 +960,9 @@ export function projectService(db: Db) { const policy = parseProjectExecutionWorkspacePolicy(row.executionWorkspacePolicy); if (policy?.environmentId !== environmentId) continue; - await db - .update(projects) - .set({ - executionWorkspacePolicy: { - ...policy, - environmentId: null, - }, - updatedAt: new Date(), - }) - .where(eq(projects.id, row.id)); + await projectService(db).update(row.id, { + executionWorkspacePolicy: { ...policy, environmentId: null }, + }); cleared += 1; } @@ -1004,11 +1003,12 @@ export function projectService(db: Db) { createWorkspace: async ( projectId: string, data: CreateWorkspaceInput, - ): Promise => { + ): Promise => db.transaction(async tx => { + const db = tx as unknown as Db; const project = await db .select() .from(projects) - .where(eq(projects.id, projectId)) + .where(eq(projects.id, projectId)).for("update") .then((rows) => rows[0] ?? null); if (!project) return null; @@ -1079,14 +1079,19 @@ export function projectService(db: Db) { return row; }); + if (created && options.captureWorkspaceUpdates !== false) { + await recordProjectUpdateEvent(db, created.companyId, projectId); + } return created ? toWorkspace(created) : null; - }, + }), updateWorkspace: async ( projectId: string, workspaceId: string, data: UpdateWorkspaceInput, - ): Promise => { + ): Promise => db.transaction(async tx => { + const db = tx as unknown as Db; + await db.select({ id: projects.id }).from(projects).where(eq(projects.id, projectId)).for("update"); const existing = await db .select() .from(projectWorkspaces) @@ -1231,10 +1236,15 @@ export function projectService(db: Db) { return row; }); + if (updated && options.captureWorkspaceUpdates !== false) { + await recordProjectUpdateEvent(db, updated.companyId, projectId); + } return updated ? toWorkspace(updated) : null; - }, + }), - removeWorkspace: async (projectId: string, workspaceId: string): Promise => { + removeWorkspace: async (projectId: string, workspaceId: string): Promise => db.transaction(async tx => { + const db = tx as unknown as Db; + await db.select({ id: projects.id }).from(projects).where(eq(projects.id, projectId)).for("update"); const existing = await db .select() .from(projectWorkspaces) @@ -1281,8 +1291,11 @@ export function projectService(db: Db) { return row; }); + if (removed && options.captureWorkspaceUpdates !== false) { + await recordProjectUpdateEvent(db, removed.companyId, projectId); + } return removed ? toWorkspace(removed) : null; - }, + }), resolveByReference: async (companyId: string, reference: string) => { const raw = reference.trim(); diff --git a/server/src/services/resource-lifecycle-events.ts b/server/src/services/resource-lifecycle-events.ts index ed1d7536f0a..9e34dcb5d38 100644 --- a/server/src/services/resource-lifecycle-events.ts +++ b/server/src/services/resource-lifecycle-events.ts @@ -23,3 +23,8 @@ export async function recordAgentStatusEvent(db: Db, companyId: string, agentId: if (!action) return; await db.insert(resourceLifecycleEvents).values({ companyId, resourceType: "agent", resourceId: agentId, action }); } + +/** Call while holding the project row lock, in the mutation transaction. */ +export async function recordProjectUpdateEvent(db: Db, companyId: string, projectId: string): Promise { + await db.insert(resourceLifecycleEvents).values({ companyId, resourceType: "project", resourceId: projectId, action: "update" }); +} From 16b7db35ffa0f9a95913c8cbdeea3d595435691f Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:36:01 -0500 Subject: [PATCH 6/6] Shorten planning skills and measure task decomposition (#15296) ## Thinking Path > - Paperclip manages work for AI agents. > - Planning guidance helps agents choose owners and dependencies. > - The runtime skill favors few tasks, but the catalog skill requires a child-task breakdown. > - Both add repeated process instructions that can distract from the requested outcome. > - This change keeps the ownership and dependency rules and removes the required matrix and repeated checklist. > - A bounded Product E2E comparison measures saved outcomes and task handoffs before qualification. ## Linked Issues or Issue Description Refs #11057. Related measurement work: #15218. **What existing behavior does this improve?** Planning and delegation through the runtime plan-to-tasks and bundled task-planning skills. **Current behavior** The two skills contain about 1,900 words and conflicting guidance on whether plans require child tasks. **Proposed behavior** Keep cohesive work with one owner. Split only for a real owner, parallel output, dependency, independent review, or follow-up lifecycle. Preserve existing authorization and planning mechanics. ## What Changed - Shorten both skills to about 400 words combined. Preserve their keys and installed-version behavior. - Remove the duplicate operational-skill pointer and regenerate affected source metadata. - Add twelve explicit Product E2E cells: four scenarios with current, short and disabled planning skills. - Use the current task composer and actual create-response ID; calibrate public skill APIs and browser creation without providers. - Eliminate an observed collision in chat-test company prefixes with a per-suite sequence. - Grade saved documents, exact author/run attribution, child count, prerequisite execution order, review boundaries and completion handoffs. - Retain current skill bytes and report source, selections, run accounting and failures. ## Verification - `pnpm test:e2e:runner:typecheck`: pass. - `pnpm test:e2e:runner:unit`: 1,287 Vitest tests and 128 Node checks pass. - `pnpm test:e2e:runner -- --list --suite plan-task-guidance`: twelve local Codex cells. - Archived current skills match master `72ff3a9f27e581a27acb49771e8658bbb0bbaa47` exactly. - Corrected fixture: three real public-API/database calibrations pass with zero provider runs; all 35 evaluator checks and Product E2E typecheck pass. - Setup campaign [37399550253](https://github.com/paperclipai/paperclip/actions/runs/37399550253) was canceled after source review found unsupported bundled edits and automatic core reinstallation. Its paid-cell step was skipped: zero provider runs, no behavioral grade. - The next setup [37401094799](https://github.com/paperclipai/paperclip/actions/runs/37401094799) failed before task creation on the old title-field selector: zero actual runs, original FAIL retained, cleanup passed. A real browser/API calibration of the new helper passes with paused non-provider agents and zero runs. - Full local typecheck/build pass. Full local tests retain one unchanged five-minute Git streaming timeout (also fails isolated), 9,591 passes and 5,796 skips. CI's chat failure was a proven random fixture-prefix collision; five affected cases pass after the test-only repair. - Paid behavior comparison and new-head CI/review remain pending. This PR remains a draft. ## Risks - The shorter text may change delegation decisions. Live outcomes are not yet qualified. - The initial comparison uses one profile and one attempt per cell. It cannot establish cross-model reliability or cost trends. - Disabled means unassigned company-owned copies; the company library remains discoverable. This does not qualify global removal, automatic accepted-plan wiring changes, or installed-copy migration. - Skill availability does not prove a model read or cognitively used it. - No provider/tool protocol, permission, timeout or runtime lifecycle behavior changes in production. ## Model Used OpenAI Codex (GPT-6), with repository inspection, code editing and tool use. The exact backend model ID and context-window size are not exposed in this session. The declared eval model is native Codex `gpt-5.6-sol`. ## 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 - [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 - [ ] 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 --- doc/evals.md | 2 + doc/plans/2026-10-05-plan-task-guidance.md | 98 ++++++++++++ .../docs/capability-contract.md | 8 +- .../generated/capability/capabilities.yaml | 16 +- .../capability/capability-contract.md | 2 +- .../spec/capability/capabilities.yaml | 24 +-- .../task-planning/SKILL.md | 84 +++------- .../skills-catalog/generated/catalog.json | 10 +- .../chat-channels.integration.test.ts | 2 +- .../src/__tests__/plan-task-fixture.test.ts | 104 +++++++++++++ .../SKILL.md | 67 ++------ skills/paperclip/SKILL.md | 2 - tests/e2e/plan-task-fixture.spec.ts | 32 ++++ tests/runner-e2e/PLAN-TASK-GUIDANCE.md | 87 +++++++++++ tests/runner-e2e/README.md | 6 + tests/runner-e2e/catalog.test.ts | 6 +- tests/runner-e2e/catalog.ts | 8 + .../plan-task-guidance/current-conversion.md | 60 ++++++++ .../plan-task-guidance/current-planning.md | 84 ++++++++++ tests/runner-e2e/plan-task-cases.ts | 58 +++++++ tests/runner-e2e/plan-task-exposure.test.ts | 41 +++++ tests/runner-e2e/plan-task-exposure.ts | 46 ++++++ tests/runner-e2e/plan-task-flow.ts | 139 +++++++++++++++++ tests/runner-e2e/plan-task-scoring.test.ts | 145 ++++++++++++++++++ tests/runner-e2e/plan-task-scoring.ts | 106 +++++++++++++ tests/runner-e2e/plan-task-skills.ts | 54 +++++++ tests/runner-e2e/plan-task-ui.ts | 29 ++++ tests/runner-e2e/runner.spec.ts | 14 +- tests/runner-e2e/types.ts | 1 + 29 files changed, 1181 insertions(+), 154 deletions(-) create mode 100644 doc/plans/2026-10-05-plan-task-guidance.md create mode 100644 server/src/__tests__/plan-task-fixture.test.ts create mode 100644 tests/e2e/plan-task-fixture.spec.ts create mode 100644 tests/runner-e2e/PLAN-TASK-GUIDANCE.md create mode 100644 tests/runner-e2e/fixtures/plan-task-guidance/current-conversion.md create mode 100644 tests/runner-e2e/fixtures/plan-task-guidance/current-planning.md create mode 100644 tests/runner-e2e/plan-task-cases.ts create mode 100644 tests/runner-e2e/plan-task-exposure.test.ts create mode 100644 tests/runner-e2e/plan-task-exposure.ts create mode 100644 tests/runner-e2e/plan-task-flow.ts create mode 100644 tests/runner-e2e/plan-task-scoring.test.ts create mode 100644 tests/runner-e2e/plan-task-scoring.ts create mode 100644 tests/runner-e2e/plan-task-skills.ts create mode 100644 tests/runner-e2e/plan-task-ui.ts diff --git a/doc/evals.md b/doc/evals.md index 7b9077cc4c2..ebd5d85320b 100644 --- a/doc/evals.md +++ b/doc/evals.md @@ -406,3 +406,5 @@ ambiguous proposals, and the existing card-click path with native Claude/Codex. See the [suite contract](../tests/runner-e2e/README.md#conversational-confirmation-replies-explicit-only). Hiring notification accounting now also requires exact completed action attribution. Missing native/provider ID mapping is uncomparable evidence; it must not be reported as a model task regression or waived through name/order matching. The fixture waits for both known completion callbacks and settled bracketed observations, including the gap before pending outbox work becomes a wake. Strict action replay and original machine verdicts are retained separately. + +The explicit-only [planning guidance utility comparison](../tests/runner-e2e/PLAN-TASK-GUIDANCE.md) measures task decomposition and handoffs with current, short, and disabled skills. diff --git a/doc/plans/2026-10-05-plan-task-guidance.md b/doc/plans/2026-10-05-plan-task-guidance.md new file mode 100644 index 00000000000..a0aee0b486a --- /dev/null +++ b/doc/plans/2026-10-05-plan-task-guidance.md @@ -0,0 +1,98 @@ +# Planning guidance reduction — 5 October 2026 + +Status: implementation and eval preparation; no live result yet. + +## Decision and scope + +Shorten the runtime plan-to-tasks skill and the bundled task-planning skill together. +Preserve skill identities, installed-version behavior, automatic accepted-plan +selection, authorization, and native/legacy mechanics. Remove the duplicated +operational-skill pointer. Do not merge without the human's authorization. + +Base: `72ff3a9f27e581a27acb49771e8658bbb0bbaa47`. Prior work on PR #15218 and +its separate checklist remains untouched. + +## Bounded comparison + +Four ordinary execution scenarios: cohesive work; independent specialist outputs; +a real prerequisite between specialists; an independent adverse review. +Compare current, short, and disabled planning skills using public company-copy creation +and selection APIs in isolated companies. Archived current skill bytes come from +the base above. The disabled variant is an unassigned-selection ablation (the library remains discoverable), not a production +removal or an accepted-plan continuation test. Other instructions, tools, models, +permissions, fixtures, graders, budgets, and deadlines stay matched. + +Initial scope: native Codex default profile, local, 12 cells, one attempt each, +500-cent company and per-agent hard stops, at most eight total run records per +cell, including coordination wakes. One exact pilot cell precedes the remaining +selection. No automatic retries or outcome rerolls. User authorization for paid +runs persists. Extra profiles/repetitions are not part of this initial comparison. + +Judge independently saved results, authorship, child boundaries, dependency order, +review delivery, and parent completion. Retain all run records, failures, missing +evidence, source/fixture hashes, skill selections and served bytes, screenshots, +usage and billing coverage. Byte/word reductions are not token or cost savings. +Skill availability is not proof that a model read or cognitively used a skill. + +Run credential-free source, catalog, type and grader calibration checks before +providers. Preserve positive, plausible wrong, and missing-evidence calibration. +Report exact scenario pairs rather than equal aggregate totals. Short guidance is +eligible only if the retained comparison supports it; inconclusive evidence stays +explicit. Automatic removal and existing installed copies need separate migration +and continuation qualification before a deletion recommendation can be shipped. + +## Credential-free admission + +- Product E2E TypeScript compilation: pass. +- Product E2E support: 1,286 Vitest tests and 128 Node checks pass. +- Catalog discovery: twelve explicit local Codex cells; excluded from `--all`. +- Archived source hashes: conversion `08cb036df0e05b1d704dc0cd547c4e37b73078597c072a85ff982a2bb9b3a370`; planning `9c52a44a30ec8d306119da51bf298e9e3e6c382a9a9559ffd3054bf5e0c75f36`. Both match the named base Git blobs exactly. +- Generated capability inventory and catalog are synchronized. +- No provider call has started. Live outcomes remain unqualified. + +Source review found two fixture defects before provider execution: bundled and +catalog skills cannot be edited, and deleted core skills are automatically +restored. Campaign `37399550253` at `843238f43cb266f6ca0bc9d255881558ffd2eb71` +was cancelled; its paid-cell step was skipped, with zero provider runs and no +behavioral grade. Corrected setup creates editable, byte-identical company copies +and varies their explicit selection. No bundled deletion or in-place edit occurs. + +Corrected fixture admission: all three current/short/unassigned variants pass real +company-skill creation, content readback and native-agent selection APIs against +a disposable PostgreSQL database. No heartbeat run rows were created. The +evaluator's 35 positive/wrong/missing-evidence checks pass, including rejection +of a bare issue-ID URL without the issues route. Product E2E typecheck passes. +Initial local DB checks were skipped until the dependency's missing library +symlinks were restored with its supplied postinstall script; skipped checks were +not counted as passes. + +The next setup campaign, [37401094799](https://github.com/paperclipai/paperclip/actions/runs/37401094799), +measured source `370e51d110836b942e5f90567d2bbe260bcc0f3a` using trusted workflow +revision `0e0b63e5a551388ac4601ed982b3e8f1c772f123` (workflow blob +`0600886144d3e22ea2e4a38329a79177882f3948`). It remains an original FAIL, +classified by the harness as `candidate_failure`. Inspection shows a browser +fixture error before task creation: the shared helper waited for the old Task +title field. `runIds`, the company run ledger, and planning observation are all +empty; no model executed. Cleanup passed. The billing summary's runCount=1 is +its minimum-one placeholder (`billing.ts`), not evidence of a provider run; +runtime/actual charges remain unmetered. No original result is regraded. + +The corrected planning-only browser helper uses the current description composer, +explicitly selects the owner, and captures the public task-create response ID. A +real browser/server/database calibration creates the exact prompt/assignment with +paused non-provider agents, confirms zero run rows, and deletes its company. It +passes. Before any model execution, prompts also explicitly name the already +required result document key and exact JSON fields, avoiding an unstated oracle +format assumption. The legacy helper and production UI remain unchanged. + +Repository checks at the previous source: full typecheck and build pass. Local +full tests: 9,591 pass, 5,796 skip and one unchanged macOS real-Git streaming test +hits its five-minute deadline; an isolated check also fails. Preserve this +limitation. Normal CI found a different, exact cause: truncated random fixture +company prefixes collided in chat integration setup. An initial per-suite counter fixed within-run collisions but review found that +its values repeat against an external database on subsequent runs. The corrected +prefix contains the complete company UUID, with no truncation; distinct company +IDs therefore produce distinct prefixes within and across runs. All five affected +native-modal variants pass locally (1,058 unrelated cases filtered). This is a +test-only repair; the measured planning fixture and production guidance remain +frozen at `8538cfce4c2defdedc2efba7519bcfce880c4e00`. diff --git a/packages/paperclip-runner/docs/capability-contract.md b/packages/paperclip-runner/docs/capability-contract.md index cf14bc59966..13065a0d840 100644 --- a/packages/paperclip-runner/docs/capability-contract.md +++ b/packages/paperclip-runner/docs/capability-contract.md @@ -68,10 +68,10 @@ The skill/reference inventory and eval cases are the only normative behavior sou | skill:skills/paperclip/SKILL.md:comment-style-required:607 | always_agent_tool | skills/paperclip/SKILL.md:607 | | skill:skills/paperclip/SKILL.md:update:639 | optional_agent_tool | skills/paperclip/SKILL.md:639 | | skill:skills/paperclip/SKILL.md:planning-required-when-planning-requested:649 | optional_agent_tool | skills/paperclip/SKILL.md:649 | -| skill:skills/paperclip/SKILL.md:key-endpoints-hot-routes:682 | optional_agent_tool | skills/paperclip/SKILL.md:682 | -| skill:skills/paperclip/SKILL.md:searching-issues:711 | optional_agent_tool | skills/paperclip/SKILL.md:711 | -| skill:skills/paperclip/SKILL.md:full-reference:721 | optional_agent_tool | skills/paperclip/SKILL.md:721 | -| skill:skills/paperclip/SKILL.md:conversational-confirmation-answers:725 | always_agent_tool | skills/paperclip/SKILL.md:725 | +| skill:skills/paperclip/SKILL.md:key-endpoints-hot-routes:680 | optional_agent_tool | skills/paperclip/SKILL.md:680 | +| skill:skills/paperclip/SKILL.md:searching-issues:709 | optional_agent_tool | skills/paperclip/SKILL.md:709 | +| skill:skills/paperclip/SKILL.md:full-reference:719 | optional_agent_tool | skills/paperclip/SKILL.md:719 | +| skill:skills/paperclip/SKILL.md:conversational-confirmation-answers:723 | always_agent_tool | skills/paperclip/SKILL.md:723 | | skill:skills/paperclip/references/api-reference.md:paperclip-api-reference:1 | optional_agent_tool | skills/paperclip/references/api-reference.md:1 | | skill:skills/paperclip/references/api-reference.md:response-schemas:9 | optional_agent_tool | skills/paperclip/references/api-reference.md:9 | | skill:skills/paperclip/references/api-reference.md:agent-record-get-api-agents-me-or-get-api-agents-agentid:11 | optional_agent_tool | skills/paperclip/references/api-reference.md:11 | diff --git a/packages/paperclip-runner/generated/capability/capabilities.yaml b/packages/paperclip-runner/generated/capability/capabilities.yaml index a2557fcd05c..c065b0d660f 100644 --- a/packages/paperclip-runner/generated/capability/capabilities.yaml +++ b/packages/paperclip-runner/generated/capability/capabilities.yaml @@ -245,36 +245,36 @@ "expectedMockState": "operation_result" }, { - "id": "skill:skills/paperclip/SKILL.md:682", + "id": "skill:skills/paperclip/SKILL.md:680", "kind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md#L682:key-endpoints-hot-routes", + "sourceAnchor": "skills/paperclip/SKILL.md#L680:key-endpoints-hot-routes", "heading": "Key Endpoints (Hot Routes)", "primaryDisposition": "control_plane_owned", "semanticOperation": "runtime_reconciliation", "expectedMockState": "runtime_decision_record" }, { - "id": "skill:skills/paperclip/SKILL.md:711", + "id": "skill:skills/paperclip/SKILL.md:709", "kind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md#L711:searching-issues", + "sourceAnchor": "skills/paperclip/SKILL.md#L709:searching-issues", "heading": "Searching Issues", "primaryDisposition": "control_plane_owned", "semanticOperation": "runtime_reconciliation", "expectedMockState": "runtime_decision_record" }, { - "id": "skill:skills/paperclip/SKILL.md:721", + "id": "skill:skills/paperclip/SKILL.md:719", "kind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md#L721:full-reference", + "sourceAnchor": "skills/paperclip/SKILL.md#L719:full-reference", "heading": "Full Reference", "primaryDisposition": "control_plane_owned", "semanticOperation": "runtime_reconciliation", "expectedMockState": "runtime_decision_record" }, { - "id": "skill:skills/paperclip/SKILL.md:725", + "id": "skill:skills/paperclip/SKILL.md:723", "kind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md#L725:conversational-confirmation-answers", + "sourceAnchor": "skills/paperclip/SKILL.md#L723:conversational-confirmation-answers", "heading": "Conversational confirmation answers", "primaryDisposition": "always_agent_tool", "semanticOperation": "call_api", diff --git a/packages/paperclip-runner/generated/capability/capability-contract.md b/packages/paperclip-runner/generated/capability/capability-contract.md index 4d124613015..9315a3c0a80 100644 --- a/packages/paperclip-runner/generated/capability/capability-contract.md +++ b/packages/paperclip-runner/generated/capability/capability-contract.md @@ -5,6 +5,6 @@ Generated by `scripts/generate-capability-contract.mjs`; do not edit generated f - Skill/reference headings: 161 - Legacy MCP tools: 42 - Eval cases: 106 across 16 groups -- Deterministic content SHA-256: `e577ca3ad6e6b045a7413ac11008013c8c12c2dc954f5a7876b86d96fbe017e8` +- Deterministic content SHA-256: `04cf20b7261d3063030cf04c5204a9c2f37cba8bea1a85da02548d87fa152bfc` Every row has exactly one primary disposition, a source anchor, a semantic operation, and a mock-state expectation. diff --git a/packages/paperclip-runner/spec/capability/capabilities.yaml b/packages/paperclip-runner/spec/capability/capabilities.yaml index 4170503c4bc..d6cfcea004b 100644 --- a/packages/paperclip-runner/spec/capability/capabilities.yaml +++ b/packages/paperclip-runner/spec/capability/capabilities.yaml @@ -420,9 +420,9 @@ ] }, { - "id": "skill:skills/paperclip/SKILL.md:key-endpoints-hot-routes:682", + "id": "skill:skills/paperclip/SKILL.md:key-endpoints-hot-routes:680", "sourceKind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md:682", + "sourceAnchor": "skills/paperclip/SKILL.md:680", "title": "Key Endpoints (Hot Routes)", "expectedSemantics": "Skill guidance headed “Key Endpoints (Hot Routes)”.", "primaryDisposition": "optional_agent_tool", @@ -431,13 +431,13 @@ "control_plane_invariant" ], "evidenceIds": [ - "skill:skills/paperclip/SKILL.md:682" + "skill:skills/paperclip/SKILL.md:680" ] }, { - "id": "skill:skills/paperclip/SKILL.md:searching-issues:711", + "id": "skill:skills/paperclip/SKILL.md:searching-issues:709", "sourceKind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md:711", + "sourceAnchor": "skills/paperclip/SKILL.md:709", "title": "Searching Issues", "expectedSemantics": "Skill guidance headed “Searching Issues”.", "primaryDisposition": "optional_agent_tool", @@ -446,13 +446,13 @@ "control_plane_invariant" ], "evidenceIds": [ - "skill:skills/paperclip/SKILL.md:711" + "skill:skills/paperclip/SKILL.md:709" ] }, { - "id": "skill:skills/paperclip/SKILL.md:full-reference:721", + "id": "skill:skills/paperclip/SKILL.md:full-reference:719", "sourceKind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md:721", + "sourceAnchor": "skills/paperclip/SKILL.md:719", "title": "Full Reference", "expectedSemantics": "Skill guidance headed “Full Reference”.", "primaryDisposition": "optional_agent_tool", @@ -461,13 +461,13 @@ "control_plane_invariant" ], "evidenceIds": [ - "skill:skills/paperclip/SKILL.md:721" + "skill:skills/paperclip/SKILL.md:719" ] }, { - "id": "skill:skills/paperclip/SKILL.md:conversational-confirmation-answers:725", + "id": "skill:skills/paperclip/SKILL.md:conversational-confirmation-answers:723", "sourceKind": "skill_heading", - "sourceAnchor": "skills/paperclip/SKILL.md:725", + "sourceAnchor": "skills/paperclip/SKILL.md:723", "title": "Conversational confirmation answers", "expectedSemantics": "Skill guidance headed “Conversational confirmation answers”.", "primaryDisposition": "always_agent_tool", @@ -476,7 +476,7 @@ "control_plane_invariant" ], "evidenceIds": [ - "skill:skills/paperclip/SKILL.md:725" + "skill:skills/paperclip/SKILL.md:723" ] }, { diff --git a/packages/skills-catalog/catalog/bundled/paperclip-operations/task-planning/SKILL.md b/packages/skills-catalog/catalog/bundled/paperclip-operations/task-planning/SKILL.md index ce682bd16d2..edf69c3c533 100644 --- a/packages/skills-catalog/catalog/bundled/paperclip-operations/task-planning/SKILL.md +++ b/packages/skills-catalog/catalog/bundled/paperclip-operations/task-planning/SKILL.md @@ -1,6 +1,6 @@ --- name: task-planning -description: Turn a Paperclip issue or request into a structured implementation plan with child task graph, blockers, owners, and acceptance criteria, then save it as the issue `plan` document. +description: Write or revise a useful Paperclip plan with a clear outcome and verification. Use when a plan is requested or uncertainty needs resolving before execution. key: paperclipai/bundled/paperclip-operations/task-planning recommendedForRoles: - manager @@ -13,72 +13,24 @@ tags: - delegation --- -# Task Planning +# Task planning -Produce implementation plans that the Paperclip executor can actually run: explicit child issues, real blockers, named owners, and a defined acceptance bar. Avoid plans that read well but cannot be split into work. +Plan only as much as the work needs. If execution is authorized and the next step +is small and clear, do it. A request for a plan alone does not require child tasks +or a new approval gate. -## When to use +Describe the intended outcome, relevant constraints, chosen approach, and how to +verify success. Include uncertainties or decisions that affect execution. Use the +user's preferred format; keep implementation steps within one owner's task unless +another owner, useful parallel output, dependency, or independent review warrants +separate work. -- An issue asks you to "plan", "scope", "break down", "design the rollout", "propose the work", or similar. -- A user wants a written plan before approving implementation. -- A manager needs to delegate non-trivial work and the shape of the work is not obvious yet. -- You inherited an issue too large to deliver in one heartbeat and need to split it. +Save a requested plan as the issue document with key `plan`. Update its current +revision instead of duplicating it, and link the saved document in your response. +Use the native document tools when available; legacy agents follow the `paperclip` +skill's document and planning API mechanics. -## When not to use - -- The issue is a single small change you can ship in the same heartbeat. Just ship it. -- The issue is forensic ("why did this break"). Use a diagnosis skill first; plan only after the root cause is named. -- A current `plan` document already exists and the change is minor. Update that document; do not start fresh. - -## Outputs - -1. An updated issue document with key `plan` (markdown). -2. A short comment on the issue that links to the plan document and names the next action. -3. Where the plan requires approval, an issue-thread interaction of kind `request_confirmation` bound to the latest plan revision. - -Do not create implementation subtasks until the plan is accepted. - -## Plan structure - -Required sections, in order: - -1. **Goal** — one paragraph. What changes for the user, the operator, or the system once this work lands. -2. **Context reviewed** — bullet list of documents, files, and prior issues you read. Lets reviewers spot missing inputs. -3. **Constraints and non-goals** — what must hold (compatibility, security, performance) and what this plan deliberately will not do. -4. **Approach** — the chosen path, with a short rationale. If you considered alternatives, name them and why you rejected them. -5. **Work breakdown** — ordered list of child issues. Each child has: - - Title in imperative form. - - Owner specialty (Engineer, QA, Designer, Security, DevRel, Manager, etc.). - - Scope and deliverables. - - Acceptance criteria. - - Blocks/blocked-by relationships expressed by phase letter or child title. -6. **Acceptance** — the bar for the parent issue. How the user knows the whole thing is done. -7. **Risks and mitigations** — short list. Skip if there are none. -8. **Deferrals** — what is intentionally pushed to follow-up issues, with why. - -## Rules of thumb for splitting - -- One child issue, one specialty. If two specialties have to coordinate inside the same issue, split it. -- One child issue, one acceptance verdict. If a reviewer would say "this is half done", split it. -- A child must be checkout-able by the owner from its title and description alone. Reviewers should not have to re-read the parent plan to understand a child. -- Order children by real blocker chains, not by author preference. Parallel children should explicitly say `blockers: none`. -- Avoid `polish` or `cleanup` child issues without acceptance criteria — they never close. - -## Filing the plan - -Use the Paperclip API to write the plan document, then comment: - -- `PUT /api/issues/{issueId}/documents/plan` with the markdown body. If `plan` already exists, include the latest `baseRevisionId`. -- `POST /api/issues/{issueId}/comments` with a short summary that links the plan: `//issues/#document-plan`. -- If approval is required: `POST /api/issues/{issueId}/interactions` with `kind: request_confirmation`, `targetRevisionId` set to the new plan revision, `continuationPolicy: wake_assignee`, and `idempotencyKey: "confirmation:{issueId}:plan:{revisionId}"`. -- Set the issue to `in_review` after creating the confirmation. Stay assigned so the acceptance wakes the planner. - -When the plan is accepted, see the companion skill for converting accepted plans into Paperclip executable tasks. Key requirements covered there: produce a compact task matrix (task, owner, initial status, blockers); encode every hard dependency as `blockedByIssueIds` — parent/child nesting alone does not block execution; and verify the created issue graph before closing the source planning issue. - -## Anti-patterns - -- Plan disguised as a description edit. Use the `plan` document. -- "Phases A–Z" with no work breakdown inside the phases. -- Children with descriptions that say "see parent" — they fail at delegation time. -- Acceptance written as "code review approval". Reviewers need a behavior bar, not a process bar. -- Plans that bury blocker chains in prose. Use explicit blocked-by lines. +Respect planning mode and explicit approval requests. When approval is required, +request confirmation of the latest plan revision and wait for that decision. +After acceptance, execute cohesive work on the current ordinary task. Use +`paperclip-converting-plans-to-tasks` when delegation is justified. diff --git a/packages/skills-catalog/generated/catalog.json b/packages/skills-catalog/generated/catalog.json index 3913615c549..7b465d431d1 100644 --- a/packages/skills-catalog/generated/catalog.json +++ b/packages/skills-catalog/generated/catalog.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "packageName": "@paperclipai/skills-catalog", "packageVersion": "0.3.1", - "generatedAt": "2026-08-11T15:05:34.526Z", + "generatedAt": "2026-10-06T01:21:47.421Z", "skills": [ { "id": "paperclipai:bundled:docs:doc-maintenance", @@ -185,7 +185,7 @@ "category": "paperclip-operations", "slug": "task-planning", "name": "task-planning", - "description": "Turn a Paperclip issue or request into a structured implementation plan with child task graph, blockers, owners, and acceptance criteria, then save it as the issue `plan` document.", + "description": "Write or revise a useful Paperclip plan with a clear outcome and verification. Use when a plan is requested or uncertainty needs resolving before execution.", "path": "catalog/bundled/paperclip-operations/task-planning", "entrypoint": "SKILL.md", "trustLevel": "markdown_only", @@ -207,11 +207,11 @@ { "path": "SKILL.md", "kind": "skill", - "sizeBytes": 4938, - "sha256": "9c52a44a30ec8d306119da51bf298e9e3e6c382a9a9559ffd3054bf5e0c75f36" + "sizeBytes": 1517, + "sha256": "48d6501d6ef481d5b19d0e0cd1507436d1cee1acf155e5038e1f0b2f0230d9e1" } ], - "contentHash": "sha256:487c3116eead89fef4f71c984329c41836bd69951b7ef9ed8736c69435e02438" + "contentHash": "sha256:a1724bde58be85c445590aeaa2f5f2f1de2bee24fe62f07fa3b579c6e4b28e6d" }, { "id": "paperclipai:bundled:product:paperclip-capsules", diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index f8ac3bcc560..6ffc53ff084 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -1088,7 +1088,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { await db.insert(companies).values({ id: companyId, name: `Chat Test ${companyId.slice(0, 8)}`, - issuePrefix: `C${companyId.replaceAll("-", "").slice(0, 7).toUpperCase()}`, + issuePrefix: `C${companyId.replace(/-/g, "").toUpperCase()}`, requireBoardApprovalForNewAgents: false, }); const now = new Date(); diff --git a/server/src/__tests__/plan-task-fixture.test.ts b/server/src/__tests__/plan-task-fixture.test.ts new file mode 100644 index 00000000000..d99f35cfdcc --- /dev/null +++ b/server/src/__tests__/plan-task-fixture.test.ts @@ -0,0 +1,104 @@ +import { randomUUID } from "node:crypto"; +import { mkdtemp, readFile, rm, stat } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import express from "express"; +import request from "supertest"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { agents, companies, createDb, heartbeatRuns } from "@paperclipai/db"; +import { companySkillRoutes } from "../routes/company-skills.js"; +import { agentRoutes } from "../routes/agents.js"; +import { errorHandler } from "../middleware/error-handler.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { preparePlanSkills, selectPlanSkills, verifyPlanSkills, verifyPlanSelection } from "../../../tests/runner-e2e/plan-task-skills.js"; +import { planInvocations, planInvocationPrompt } from "../../../tests/runner-e2e/plan-task-exposure.js"; +import { nativeTaskSkillInputs } from "../../../packages/paperclip-runner/src/backends/runtime-context.js"; +import { resolveRunnerdCodexSkillInputs } from "../../../packages/paperclip-runner/src/live/runnerd-codex-transport.js"; +import type { NativeRuntimeContextSnapshot } from "../../../packages/paperclip-runner/src/contracts/runtime-context.js"; +import { PLAN_SKILLS, PLAN_VARIANTS, planHash } from "../../../tests/runner-e2e/plan-task-cases.js"; +import type { RunnerApi } from "../../../tests/runner-e2e/api.js"; + +const support = await getEmbeddedPostgresTestSupport(); +if (!support.supported) console.warn(`Planning fixture API calibration skipped: ${support.reason}`); +(support.supported ? describe : describe.skip)("planning fixture public API calibration (no providers)", () => { + let tempDb: Awaited>; + let db: ReturnType; + let root: string; + const savedHome = process.env.PAPERCLIP_HOME, savedInstance = process.env.PAPERCLIP_INSTANCE_ID; + let api: Pick; + let app: express.Express; + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-plan-fixture-"); + root = await mkdtemp(path.join(os.tmpdir(), "paperclip-plan-fixture-home-")); + process.env.PAPERCLIP_HOME = root; + process.env.PAPERCLIP_INSTANCE_ID = "default"; + db = createDb(tempDb.connectionString); + app = express(); + app.use(express.json()); + app.use((req, _res, next) => { (req as any).actor = { type: "board", source: "local_implicit", isInstanceAdmin: true, userId: "local-board" }; next(); }); + app.use("/api", companySkillRoutes(db)); + app.use("/api", agentRoutes(db)); + app.use(errorHandler); + api = { + async get(url: string): Promise { + const response = await request(app).get(url); + if (response.status !== 200) throw new Error(`GET ${url}: ${response.status} ${JSON.stringify(response.body)}`); + return response.body; + }, + async post(url: string, body?: unknown): Promise { + const response = await request(app).post(url).send(body as object); + if (response.status < 200 || response.status >= 300) throw new Error(`POST ${url}: ${response.status} ${JSON.stringify(response.body)}`); + return response.body; + }, + }; + }); + afterAll(async () => { + if (savedHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = savedHome; + if (savedInstance === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; else process.env.PAPERCLIP_INSTANCE_ID = savedInstance; + await tempDb?.cleanup(); + if (root) await rm(root, { recursive: true, force: true }); + }); + it.each(PLAN_VARIANTS)("admits %s through real creation, readback and selection APIs", async variant => { + const companyId = randomUUID(), agentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Planning calibration", issuePrefix: `P${companyId.slice(0, 6)}` }); + await db.insert(agents).values({ id: agentId, companyId, name: "Fixture Lead", role: "general", adapterType: "paperclip_runner", adapterConfig: {} }); + const before = await api.get(`/api/companies/${companyId}/skills`); + const bundled = before.find(s => s.key === PLAN_SKILLS[0].key); + expect(bundled).toBeDefined(); + const sources = await preparePlanSkills(api, companyId, variant); + expect(sources).toHaveLength(2); + expect(sources.every(s => s.key.startsWith(`company/${companyId}/eval-`))).toBe(true); + const selected = await selectPlanSkills(api, companyId, agentId, sources); + const invocations = planInvocations(sources, selected); + const contextSkills = []; + for (const source of sources.filter(s => s.selected)) { + const entry = (selected as any).entries.find((e: any) => e.key === source.key && e.desired); + const bytes = await readFile(path.join(entry.sourcePath, "SKILL.md")); + expect(planHash(bytes)).toBe(source.sha256); + const mode = (await stat(path.join(entry.sourcePath, "SKILL.md"))).mode & 0o555; + expect(mode).toBe(0o444); + const digest = planHash(JSON.stringify([{ path: "SKILL.md", sha256: planHash(bytes), mode, size: bytes.length }])); + expect(invocations.find(i => i.name === entry.runtimeName)?.assetDigest).toBe(digest); + contextSkills.push({ key: source.key, runtimeName: entry.runtimeName, versionId: entry.currentVersionId, + bundle: { rootPath: path.join(root, "runtime-context-assets/bundles", digest) } }); + } + const context = { skills: contextSkills } as NativeRuntimeContextSnapshot; + const inputs = nativeTaskSkillInputs(planInvocationPrompt("Business task", invocations), context); + expect(inputs).toHaveLength(variant === "disabled" ? 0 : 2); + expect(inputs.map(i => i.name)).toEqual(invocations.map(i => i.name)); + const providerHome = path.join(root, "isolated-codex-home"); + expect(resolveRunnerdCodexSkillInputs(inputs, context, providerHome)).toEqual(inputs.map(i => ({ + ...i, path: path.join(providerHome, "skills", i.name, "SKILL.md"), + }))); + await verifyPlanSelection(api, companyId, agentId, sources); + await verifyPlanSkills(api, companyId, sources); + const after = await api.get(`/api/companies/${companyId}/skills`); + expect(after.find(s => s.key === bundled.key)?.id).toBe(bundled.id); + expect(after.find(s => s.key === bundled.key)?.markdown).toBe(bundled.markdown); + expect(await db.select().from(heartbeatRuns)).toHaveLength(0); + // Fail closed if a later actor changes either selection or source bytes. + const swapped = sources.map(s => ({ ...s, selected: !s.selected })); + await expect(verifyPlanSelection(api, companyId, agentId, swapped)).rejects.toThrow("selection differs"); + await expect(verifyPlanSkills(api, companyId, [{ ...sources[0]!, sha256: "wrong" }])).rejects.toThrow("guidance changed"); + }, 30_000); +}); diff --git a/skills/paperclip-converting-plans-to-tasks/SKILL.md b/skills/paperclip-converting-plans-to-tasks/SKILL.md index 70eeba0f7ef..a6ae07864c4 100644 --- a/skills/paperclip-converting-plans-to-tasks/SKILL.md +++ b/skills/paperclip-converting-plans-to-tasks/SKILL.md @@ -1,60 +1,27 @@ --- name: paperclip-converting-plans-to-tasks description: > - Convert Paperclip plans into executable issue graphs. Use when asked to plan, - scope, or break down Paperclip company work into assigned tasks with specialty - fit, dependencies, blockers, and parallelization. + Decide whether Paperclip work needs delegation, and connect justified tasks + with owners and real dependencies. Use when turning an authorized plan into work. --- -# Paperclip — Converting Plans to Tasks +# Plans to tasks -A companion skill for turning a plan into executable Paperclip work. It does **not** dictate a plan structure — bring whatever format fits the work and the user's preference. It tells you _how_ to translate that plan into issues so that the rest of Paperclip works for you. +Keep small, cohesive work on its current ordinary task with one end-to-end owner. +A plan's steps, files, and phases do not each need a task. -For the **mechanics** of recording a plan (issue document with key `plan`, comment links, approval gating, who to reassign back to), follow the _Planning_ section of the `paperclip` skill. This skill covers planning method, not the API surface. +Create a separate task only for a useful boundary: another owner or permission, +independent parallel output, a real prerequisite, independent review, or substantial +follow-up that needs its own lifecycle. Check the available agents before assigning. +Give each delegate enough context and a clear deliverable to work independently. -## When you're asked to plan +Set real dependencies through `blockedByIssueIds` (or the native dependency tool). +Parent/child nesting and prose do not delay execution. Independent work can start +now; dependent work must wait for its prerequisite. Verify the saved relationships. -- **Plan deeply.** Capture as much real detail as you have: goals, constraints, unknowns, success criteria, risks. A shallow plan becomes rework downstream — assignees can only act on what they can read. -- **Minimize the issue graph.** Use as few tasks as possible while still completing and verifying the job. Prefer one end-to-end task with one owner over separate tasks for each step, file, component, or phase. Keep those structural details as checklists or acceptance criteria inside the owning task unless a real execution boundary requires another issue. -- **Split only for a qualifying boundary.** Create a separate subtask only when at least one of these applies: - - A different specialist, owner, permission boundary, or external actor must own the work. - - A self-contained deliverable can usefully run in parallel with other work. - - A hard dependency or handoff needs its own `blockedByIssueIds` lifecycle. - - A review, QA pass, or governed approval gate has an independent owner. - - Substantial follow-up work needs independent tracking or retry because it cannot safely be completed and verified in the parent. -- **Know your team.** Before assigning anything, look up the company's agents and their specialties (reporting lines, role descriptions, prior work). Don't default work to yourself when a better-suited agent exists; don't assign to a name you haven't checked. -- **Assign for specialty.** Hand each piece of work to the agent most relevant to it. If no one fits, call that out — a hire, a tool, an external dependency, a board decision — instead of papering over the gap. -- **Take responsibility.** Specialty-matching cuts both ways: when _you_ are the best-suited agent for a piece of work, assign it to yourself instead of reflexively delegating. Don't hand off to avoid load. -- **Use the dependency tree.** Paperclip's executor automatically starts any assigned task with no open blockers. Parent/child issue nesting is structure, not execution blocking. Express each qualifying ownership or lifecycle boundary as an issue; keep other concrete deliverables within the responsible issue's description, checklist, or acceptance criteria. Wire every hard dependency between issues through `blockedByIssueIds` on the dependent issue (not prose like "blocked by X"). When a blocker reaches `done`, dependents auto-wake. -- **Order, then parallelize.** Sequence work by real dependencies, not by personal preference. Create parallel branches only for qualifying, self-contained work, then start those independent branches in parallel. Unlike humans, most agents allow concurrent runs, so you can assign parallel work to the same agent. -- **Write review tasks for the reviewer's boundary.** A review/QA task must tell the delegate to post findings on **their own review issue** and mark it `done` — the verdict is the deliverable, and adverse findings are still `done`, not `blocked`. Never instruct a delegate to comment on the parent issue (low-trust reviewers are guaranteed a 403 there), and make the description self-contained since the reviewer may not be able to read your issue. Wire the dependent issue's `blockedByIssueIds` to the review issue so the verdict wakes the right owner. -- **Enough is enough.** Plans exist to unblock execution, not replace it. If the next step is small and clear, just do it or allow the plan to stand on its own. Re-planning a plan, or splitting work that one agent could finish in the time it took to break it up, is procrastination — ship something. +Reviewers publish findings on their own review task and complete it, including an +adverse verdict. Make its description self-contained. Connect the verdict to the +owner who must act on it; do not require writes to the parent. -## When converting an accepted plan into tasks - -Start from one end-to-end task and add issues only for the qualifying boundaries above. Before creating tasks, write a compact task matrix with each proposed task, owner, initial status, blockers, and the specific qualifying reason it must be separate. Any task that can start immediately should say why it has no blockers; otherwise set it to `blocked` and include the prerequisite issue IDs in `blockedByIssueIds`. Do not rely on `parentId`, child ordering, phase labels, or prose to block execution. - -Run a merge-back pass before publishing or creating the graph. Require every proposed subtask to name at least one qualifying reason from this skill. If it cannot, merge it into its parent or an adjacent task and preserve the work as an internal step, checklist item, or acceptance criterion. Repeat until every remaining issue has a real ownership, scheduling, lifecycle, or governance reason to exist. - -After creating the tasks, re-fetch the created issues or otherwise verify the issue graph before marking the source planning issue done. Confirm that every separate issue still has its qualifying reason, each dependent task has the expected `blockedByIssueIds`, each independent task has an explicit "can start now" reason, review tasks respect the reviewer's write boundary, and the parent/child hierarchy is only being used for traceability. If the graph contains an unjustified split or expected blockers are missing, correct it or report the mismatch and leave the planning issue in `in_review` or `blocked` until the graph is fixed. - -## Quick checklist before you publish a plan - -- [ ] Enough detail that assignees can act without re-asking. -- [ ] The plan uses the fewest tasks that can complete and verify the job, preferring one end-to-end owner over step/file/component/phase splits. -- [ ] Every concrete deliverable is accounted for inside an issue or, only when a qualifying boundary applies, as its own issue. -- [ ] Every proposed subtask names a qualifying reason; otherwise it was merged into its parent or an adjacent task. -- [ ] Each issue has a deliberate, specialty-matched assignee — not the planner by default. -- [ ] Each issue's real blockers are declared via `blockedByIssueIds`. -- [ ] Independently owned review, QA, and governed approval tasks respect the reviewer's boundary. -- [ ] A compact task matrix names planned task, owner, initial status, blockers, and qualifying reason. -- [ ] Tasks without blockers have an explicit reason they can start immediately. -- [ ] Created issues were re-fetched or otherwise verified before closing the source planning issue. -- [ ] Qualifying independent branches can start in parallel. -- [ ] Gaps (missing skills, hires, decisions, external inputs) are surfaced, not hidden. - -## What this skill is not - -- Not a plan template. Use any format — prose, outline, table, RACI, Gantt, whatever fits. -- Not software-development–specific. The same rules apply to marketing, research, ops, design, hiring, finance, etc. -- Not a replacement for the `paperclip` skill's planning mechanics. Use both. +Honor existing authorization and approval boundaries. Use Paperclip's planning +mechanics when approval is required; decomposition does not create new approval gates. diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index 3d78273f521..ec9038bff8e 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -663,8 +663,6 @@ If the plan needs explicit approval before implementation, update the `plan` doc When asked to convert a plan into executable Paperclip tasks — depth, assignment, dependencies, parallelization — use the companion skill `paperclip-converting-plans-to-tasks`. -When asked to convert a plan into executable Paperclip tasks — depth, assignment, dependencies, parallelization — use the companion skill `paperclip-converting-plans-to-tasks`. - Recommended API flow: ```bash diff --git a/tests/e2e/plan-task-fixture.spec.ts b/tests/e2e/plan-task-fixture.spec.ts new file mode 100644 index 00000000000..1dec59fcc46 --- /dev/null +++ b/tests/e2e/plan-task-fixture.spec.ts @@ -0,0 +1,32 @@ +import { randomUUID } from "node:crypto"; +import { expect, test } from "@playwright/test"; +import { createPlanTaskThroughUi } from "../runner-e2e/plan-task-ui.js"; + +test("planning fixture creates the exact assignment through the current composer without providers", async ({ page, request }) => { + const companyResponse = await request.post("/api/companies", { data: { name: `Planning setup ${randomUUID()}` } }); + expect(companyResponse.ok()).toBe(true); + const company = await companyResponse.json(); + try { + let lead: { id: string; name: string } | undefined; + for (const name of ["A Different Owner", "Planning Fixture Lead"]) { + const response = await request.post(`/api/companies/${company.id}/agents`, { data: { + name, role: "general", adapterType: "process", adapterConfig: { command: "false" }, + runtimeConfig: { heartbeat: { enabled: false } }, + } }); + expect(response.ok()).toBe(true); + const agent = await response.json(); + expect((await request.post(`/api/agents/${agent.id}/pause`)).ok()).toBe(true); + lead = agent; + } + const prompt = "Plan the work briefly, then carry it out. Verify a sample order and save the requested result."; + const created = await createPlanTaskThroughUi({ page, companyId: company.id, issuePrefix: company.issuePrefix, + agentName: lead!.name, prompt }); + const issue = await (await request.get(`/api/issues/${created.id}`)).json(); + expect(issue).toMatchObject({ companyId: company.id, assigneeAgentId: lead!.id, description: prompt, workMode: "standard" }); + const runs = await request.get(`/api/companies/${company.id}/heartbeat-runs?limit=100`); + expect(runs.ok()).toBe(true); + expect(await runs.json()).toEqual([]); + } finally { + expect((await request.delete(`/api/companies/${company.id}`)).ok()).toBe(true); + } +}); diff --git a/tests/runner-e2e/PLAN-TASK-GUIDANCE.md b/tests/runner-e2e/PLAN-TASK-GUIDANCE.md new file mode 100644 index 00000000000..cee18ce07d8 --- /dev/null +++ b/tests/runner-e2e/PLAN-TASK-GUIDANCE.md @@ -0,0 +1,87 @@ +# Planning guidance utility + +`plan-task-guidance` is an explicit-only Product E2E suite. It runs the real UI, +public API, native Codex and saved task graph on four scenarios: one cohesive +order summary; two independent specialist approvals; a release depending on a +saved upstream result; and an independent adverse audit. + +Each scenario compares current (archived from master +`72ff3a9f27e581a27acb49771e8658bbb0bbaa47`), short (checkout production files), +and disabled planning skills. The fixture creates editable company-owned copies through the public API, +preserving each source file byte-for-byte, then explicitly selects those copies +before any task starts. Bundled/catalog skills remain read-only. Both runtime plan conversion +and catalog task planning change together. All agents get the same treatment. +Disabled leaves those copies unassigned, with an empty explicit skill selection. +The company library and automatically restored core inventory remain available; +this is a selection ablation, not proof that a determined agent cannot discover +unassigned guidance. +The eval does not exercise automatic accepted-plan selection or installed-skill +migration, and cannot by itself qualify deleting that wiring. + +The business prompts do not prescribe task counts or particular tools. Enabled +variants append explicit references to the two assigned runtime skill names; the +unassigned control appends none. This measures explicitly invoked guidance, not +automatic discovery. A separate exposure gate requires the initial lead run to +submit exactly those native skill inputs, bound to the immutable SKILL.md file +digests, and records provider turn acceptance. The controller emits the submission +receipt before a provider turn ID exists: this is a run-bound submission contract, +not a claimed provider call-ID join or proof of cognitive consumption. Native +protocol calibration separately verifies mapping into the isolated provider skill +directory. Missing or wrong exposure fails qualification even when the output +checks pass. + +The scenario prompts do not prescribe task counts or particular tools. They do +specify which specialist is accountable and what business output is required. +The grader checks independent arithmetic, actual latest document revisions and +author/run attribution through exact document-ID/revision-number activity joins, +owned work items, prerequisite execution order, reviewer write boundaries, and +completion handoffs. It reports parallel scheduling opportunity separately from +correctness. No run can pass through an agent-authored self-assessment. + +Twelve local cells, one attempt each, 12-minute cell deadline, 360-second provider +deadline, eight total run records, and 500-cent company/per-agent budget hard stops +bound the initial comparison. Coordination wakes count. Failed or missing evidence +stays failed. No automatic retries. The initial paid selection is one exact pilot, +then the eleven remaining cells only if fixture admission is sound. All cells use +the same native Codex model and production tool contracts. This is a bounded pilot, +not a cross-model reliability, speed, or cost claim. + +```sh +pnpm test:e2e:runner:typecheck +pnpm test:e2e:runner:unit +pnpm test:e2e:runner -- --list --suite plan-task-guidance +pnpm test:e2e:runner -- --id plan-task-guidance.runner-codex.local.short-cohesive --max-parallel 1 +``` + +Each attempt keeps `plan-task-source.json`, `plan-task-guidance.json`, +`plan-task-runs.json`, API state, screenshots and the existing run/accounting +reports. Selections and served hashes establish availability, not successful +skill invocation or cognitive consumption. Word/UTF-8 byte counts are exact file +measurements; provider context, token usage and billing coverage come from actual +retained run records. Cleanup, credential scanning and public projection use the +existing Product E2E pipeline. Do not publish raw sessions or hidden reasoning. + +Setup attempt [37399550253](https://github.com/paperclipai/paperclip/actions/runs/37399550253) +on source `843238f43cb266f6ca0bc9d255881558ffd2eb71` was cancelled after source +review identified unsupported bundled edits and automatic core reinstallation. +The paid-cell step was skipped: zero provider runs and no behavioral grade. +The corrected fixture uses company copies and selection only. + +The next setup campaign, [37401094799](https://github.com/paperclipai/paperclip/actions/runs/37401094799), +measured source `370e51d110836b942e5f90567d2bbe260bcc0f3a` using trusted workflow +revision `0e0b63e5a551388ac4601ed982b3e8f1c772f123` (workflow blob +`0600886144d3e22ea2e4a38329a79177882f3948`). It remains an original FAIL, +classified by the harness as `candidate_failure`. Inspection shows a browser +fixture error before task creation: the shared helper waited for the old Task +title field. `runIds`, the company run ledger, and planning observation are all +empty; no model executed. Cleanup passed. The billing summary's runCount=1 is +its minimum-one placeholder (`billing.ts`), not evidence of a provider run; +runtime/actual charges remain unmetered. No original result is regraded. + +The corrected planning-only browser helper uses the current description composer, +explicitly selects the owner, and captures the public task-create response ID. A +real browser/server/database calibration creates the exact prompt/assignment with +paused non-provider agents, confirms zero run rows, and deletes its company. It +passes. Before any model execution, prompts also explicitly name the already +required result document key and exact JSON fields, avoiding an unstated oracle +format assumption. The legacy helper and production UI remain unchanged. diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index 5feba8909d3..e723bbc9d8c 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -797,6 +797,12 @@ and unredacted logs are removed after each attempt. Daytona teardown destroys the environment and any reusable leases through the public API; provider-side auto-stop/archive/delete values remain as cancellation backstops. +## Planning guidance utility + +The explicit-only [planning comparison](PLAN-TASK-GUIDANCE.md) tests current, short, +and disabled planning skills across four saved business outcomes on native Codex. +It adds twelve single-attempt cells and does not expand `--all`. + ## GitHub Actions `Runner Full-Stack E2E` has only `schedule` and `workflow_dispatch` triggers; it diff --git a/tests/runner-e2e/catalog.test.ts b/tests/runner-e2e/catalog.test.ts index 8e764f9f015..fac5080e1b1 100644 --- a/tests/runner-e2e/catalog.test.ts +++ b/tests/runner-e2e/catalog.test.ts @@ -136,10 +136,10 @@ describe("runner E2E catalog", () => { expect(localIntegrityTasks).toHaveLength(2); expect(openRouterBreadthTasks).toHaveLength(3); expect(runnerSuites.map((suite) => suite.expectedMatrixSize)).toEqual([ - 6, 30, 3, 16, 16, 2, 6, 8, 46, 23, 52, 6, 6, 20, 26, 52, 28, 18, 2, 6, 6, 12, 10, 48, 16, 10, 2, 1, 1, + 12, 6, 30, 3, 16, 16, 2, 6, 8, 46, 23, 52, 6, 6, 20, 26, 52, 28, 18, 2, 6, 6, 12, 10, 48, 16, 10, 2, 1, 1, ]); - expect(validateRunnerCatalog()).toHaveLength(478); - expect(new Set(runnerMatrix.map((entry) => entry.id)).size).toBe(478); + expect(validateRunnerCatalog()).toHaveLength(490); + expect(new Set(runnerMatrix.map((entry) => entry.id)).size).toBe(490); expect( runnerMatrix.filter((entry) => entry.suite.id === "core-compatibility"), ).toHaveLength(48); diff --git a/tests/runner-e2e/catalog.ts b/tests/runner-e2e/catalog.ts index 2943c23efe6..4bec77f0472 100644 --- a/tests/runner-e2e/catalog.ts +++ b/tests/runner-e2e/catalog.ts @@ -1,3 +1,4 @@ +import { planTaskCases, planTaskProfile, planDefinitionDigest } from "./plan-task-cases.js"; import { nativeCompletionTasks, nativeCompletionDefinitionDigest } from "./native-completion-cases.js"; import { NATIVE_INSTRUCTION_SUITE, NATIVE_INSTRUCTION_BASE_SHA, nativeInstructionDefinitionDigest } from "./native-instruction-consolidation.js"; import { nativeCompletionProfile, NATIVE_COMPLETION_BUDGET_CENTS } from "./native-completion-defaults.js"; @@ -1082,6 +1083,13 @@ export const extendedHarnessFileTask: RunnerTaskFixture = { }; export const runnerSuites: readonly RunnerSuiteFixture[] = [ + { + id: "plan-task-guidance", label: "Planning guidance utility", manualOnly: true, + description: "Current, short and disabled planning skills on cohesive, parallel, dependent and independently reviewed work.", + groups: ["native"], profiles: runnerProfiles.filter(p => p.id === "runner-codex").map(planTaskProfile), + environments: [localEnvironment], tasks: planTaskCases, expectedMatrixSize: 12, + definitionMetadata: { version: 1, definitionDigest: planDefinitionDigest, scheduling: "explicit-only", comparison: "public-skill-availability-ablation", scope: "one-profile-single-attempt" }, + }, { id: "blocker-guidance", label: "Direct blocker handling", manualOnly: true, description: "Human authority, hiring permissions, and requester scope under the production coordination skill.", diff --git a/tests/runner-e2e/fixtures/plan-task-guidance/current-conversion.md b/tests/runner-e2e/fixtures/plan-task-guidance/current-conversion.md new file mode 100644 index 00000000000..70eeba0f7ef --- /dev/null +++ b/tests/runner-e2e/fixtures/plan-task-guidance/current-conversion.md @@ -0,0 +1,60 @@ +--- +name: paperclip-converting-plans-to-tasks +description: > + Convert Paperclip plans into executable issue graphs. Use when asked to plan, + scope, or break down Paperclip company work into assigned tasks with specialty + fit, dependencies, blockers, and parallelization. +--- + +# Paperclip — Converting Plans to Tasks + +A companion skill for turning a plan into executable Paperclip work. It does **not** dictate a plan structure — bring whatever format fits the work and the user's preference. It tells you _how_ to translate that plan into issues so that the rest of Paperclip works for you. + +For the **mechanics** of recording a plan (issue document with key `plan`, comment links, approval gating, who to reassign back to), follow the _Planning_ section of the `paperclip` skill. This skill covers planning method, not the API surface. + +## When you're asked to plan + +- **Plan deeply.** Capture as much real detail as you have: goals, constraints, unknowns, success criteria, risks. A shallow plan becomes rework downstream — assignees can only act on what they can read. +- **Minimize the issue graph.** Use as few tasks as possible while still completing and verifying the job. Prefer one end-to-end task with one owner over separate tasks for each step, file, component, or phase. Keep those structural details as checklists or acceptance criteria inside the owning task unless a real execution boundary requires another issue. +- **Split only for a qualifying boundary.** Create a separate subtask only when at least one of these applies: + - A different specialist, owner, permission boundary, or external actor must own the work. + - A self-contained deliverable can usefully run in parallel with other work. + - A hard dependency or handoff needs its own `blockedByIssueIds` lifecycle. + - A review, QA pass, or governed approval gate has an independent owner. + - Substantial follow-up work needs independent tracking or retry because it cannot safely be completed and verified in the parent. +- **Know your team.** Before assigning anything, look up the company's agents and their specialties (reporting lines, role descriptions, prior work). Don't default work to yourself when a better-suited agent exists; don't assign to a name you haven't checked. +- **Assign for specialty.** Hand each piece of work to the agent most relevant to it. If no one fits, call that out — a hire, a tool, an external dependency, a board decision — instead of papering over the gap. +- **Take responsibility.** Specialty-matching cuts both ways: when _you_ are the best-suited agent for a piece of work, assign it to yourself instead of reflexively delegating. Don't hand off to avoid load. +- **Use the dependency tree.** Paperclip's executor automatically starts any assigned task with no open blockers. Parent/child issue nesting is structure, not execution blocking. Express each qualifying ownership or lifecycle boundary as an issue; keep other concrete deliverables within the responsible issue's description, checklist, or acceptance criteria. Wire every hard dependency between issues through `blockedByIssueIds` on the dependent issue (not prose like "blocked by X"). When a blocker reaches `done`, dependents auto-wake. +- **Order, then parallelize.** Sequence work by real dependencies, not by personal preference. Create parallel branches only for qualifying, self-contained work, then start those independent branches in parallel. Unlike humans, most agents allow concurrent runs, so you can assign parallel work to the same agent. +- **Write review tasks for the reviewer's boundary.** A review/QA task must tell the delegate to post findings on **their own review issue** and mark it `done` — the verdict is the deliverable, and adverse findings are still `done`, not `blocked`. Never instruct a delegate to comment on the parent issue (low-trust reviewers are guaranteed a 403 there), and make the description self-contained since the reviewer may not be able to read your issue. Wire the dependent issue's `blockedByIssueIds` to the review issue so the verdict wakes the right owner. +- **Enough is enough.** Plans exist to unblock execution, not replace it. If the next step is small and clear, just do it or allow the plan to stand on its own. Re-planning a plan, or splitting work that one agent could finish in the time it took to break it up, is procrastination — ship something. + +## When converting an accepted plan into tasks + +Start from one end-to-end task and add issues only for the qualifying boundaries above. Before creating tasks, write a compact task matrix with each proposed task, owner, initial status, blockers, and the specific qualifying reason it must be separate. Any task that can start immediately should say why it has no blockers; otherwise set it to `blocked` and include the prerequisite issue IDs in `blockedByIssueIds`. Do not rely on `parentId`, child ordering, phase labels, or prose to block execution. + +Run a merge-back pass before publishing or creating the graph. Require every proposed subtask to name at least one qualifying reason from this skill. If it cannot, merge it into its parent or an adjacent task and preserve the work as an internal step, checklist item, or acceptance criterion. Repeat until every remaining issue has a real ownership, scheduling, lifecycle, or governance reason to exist. + +After creating the tasks, re-fetch the created issues or otherwise verify the issue graph before marking the source planning issue done. Confirm that every separate issue still has its qualifying reason, each dependent task has the expected `blockedByIssueIds`, each independent task has an explicit "can start now" reason, review tasks respect the reviewer's write boundary, and the parent/child hierarchy is only being used for traceability. If the graph contains an unjustified split or expected blockers are missing, correct it or report the mismatch and leave the planning issue in `in_review` or `blocked` until the graph is fixed. + +## Quick checklist before you publish a plan + +- [ ] Enough detail that assignees can act without re-asking. +- [ ] The plan uses the fewest tasks that can complete and verify the job, preferring one end-to-end owner over step/file/component/phase splits. +- [ ] Every concrete deliverable is accounted for inside an issue or, only when a qualifying boundary applies, as its own issue. +- [ ] Every proposed subtask names a qualifying reason; otherwise it was merged into its parent or an adjacent task. +- [ ] Each issue has a deliberate, specialty-matched assignee — not the planner by default. +- [ ] Each issue's real blockers are declared via `blockedByIssueIds`. +- [ ] Independently owned review, QA, and governed approval tasks respect the reviewer's boundary. +- [ ] A compact task matrix names planned task, owner, initial status, blockers, and qualifying reason. +- [ ] Tasks without blockers have an explicit reason they can start immediately. +- [ ] Created issues were re-fetched or otherwise verified before closing the source planning issue. +- [ ] Qualifying independent branches can start in parallel. +- [ ] Gaps (missing skills, hires, decisions, external inputs) are surfaced, not hidden. + +## What this skill is not + +- Not a plan template. Use any format — prose, outline, table, RACI, Gantt, whatever fits. +- Not software-development–specific. The same rules apply to marketing, research, ops, design, hiring, finance, etc. +- Not a replacement for the `paperclip` skill's planning mechanics. Use both. diff --git a/tests/runner-e2e/fixtures/plan-task-guidance/current-planning.md b/tests/runner-e2e/fixtures/plan-task-guidance/current-planning.md new file mode 100644 index 00000000000..ce682bd16d2 --- /dev/null +++ b/tests/runner-e2e/fixtures/plan-task-guidance/current-planning.md @@ -0,0 +1,84 @@ +--- +name: task-planning +description: Turn a Paperclip issue or request into a structured implementation plan with child task graph, blockers, owners, and acceptance criteria, then save it as the issue `plan` document. +key: paperclipai/bundled/paperclip-operations/task-planning +recommendedForRoles: + - manager + - engineer + - product +tags: + - paperclip + - planning + - issues + - delegation +--- + +# Task Planning + +Produce implementation plans that the Paperclip executor can actually run: explicit child issues, real blockers, named owners, and a defined acceptance bar. Avoid plans that read well but cannot be split into work. + +## When to use + +- An issue asks you to "plan", "scope", "break down", "design the rollout", "propose the work", or similar. +- A user wants a written plan before approving implementation. +- A manager needs to delegate non-trivial work and the shape of the work is not obvious yet. +- You inherited an issue too large to deliver in one heartbeat and need to split it. + +## When not to use + +- The issue is a single small change you can ship in the same heartbeat. Just ship it. +- The issue is forensic ("why did this break"). Use a diagnosis skill first; plan only after the root cause is named. +- A current `plan` document already exists and the change is minor. Update that document; do not start fresh. + +## Outputs + +1. An updated issue document with key `plan` (markdown). +2. A short comment on the issue that links to the plan document and names the next action. +3. Where the plan requires approval, an issue-thread interaction of kind `request_confirmation` bound to the latest plan revision. + +Do not create implementation subtasks until the plan is accepted. + +## Plan structure + +Required sections, in order: + +1. **Goal** — one paragraph. What changes for the user, the operator, or the system once this work lands. +2. **Context reviewed** — bullet list of documents, files, and prior issues you read. Lets reviewers spot missing inputs. +3. **Constraints and non-goals** — what must hold (compatibility, security, performance) and what this plan deliberately will not do. +4. **Approach** — the chosen path, with a short rationale. If you considered alternatives, name them and why you rejected them. +5. **Work breakdown** — ordered list of child issues. Each child has: + - Title in imperative form. + - Owner specialty (Engineer, QA, Designer, Security, DevRel, Manager, etc.). + - Scope and deliverables. + - Acceptance criteria. + - Blocks/blocked-by relationships expressed by phase letter or child title. +6. **Acceptance** — the bar for the parent issue. How the user knows the whole thing is done. +7. **Risks and mitigations** — short list. Skip if there are none. +8. **Deferrals** — what is intentionally pushed to follow-up issues, with why. + +## Rules of thumb for splitting + +- One child issue, one specialty. If two specialties have to coordinate inside the same issue, split it. +- One child issue, one acceptance verdict. If a reviewer would say "this is half done", split it. +- A child must be checkout-able by the owner from its title and description alone. Reviewers should not have to re-read the parent plan to understand a child. +- Order children by real blocker chains, not by author preference. Parallel children should explicitly say `blockers: none`. +- Avoid `polish` or `cleanup` child issues without acceptance criteria — they never close. + +## Filing the plan + +Use the Paperclip API to write the plan document, then comment: + +- `PUT /api/issues/{issueId}/documents/plan` with the markdown body. If `plan` already exists, include the latest `baseRevisionId`. +- `POST /api/issues/{issueId}/comments` with a short summary that links the plan: `//issues/#document-plan`. +- If approval is required: `POST /api/issues/{issueId}/interactions` with `kind: request_confirmation`, `targetRevisionId` set to the new plan revision, `continuationPolicy: wake_assignee`, and `idempotencyKey: "confirmation:{issueId}:plan:{revisionId}"`. +- Set the issue to `in_review` after creating the confirmation. Stay assigned so the acceptance wakes the planner. + +When the plan is accepted, see the companion skill for converting accepted plans into Paperclip executable tasks. Key requirements covered there: produce a compact task matrix (task, owner, initial status, blockers); encode every hard dependency as `blockedByIssueIds` — parent/child nesting alone does not block execution; and verify the created issue graph before closing the source planning issue. + +## Anti-patterns + +- Plan disguised as a description edit. Use the `plan` document. +- "Phases A–Z" with no work breakdown inside the phases. +- Children with descriptions that say "see parent" — they fail at delegation time. +- Acceptance written as "code review approval". Reviewers need a behavior bar, not a process bar. +- Plans that bury blocker chains in prose. Use explicit blocked-by lines. diff --git a/tests/runner-e2e/plan-task-cases.ts b/tests/runner-e2e/plan-task-cases.ts new file mode 100644 index 00000000000..1090ebded30 --- /dev/null +++ b/tests/runner-e2e/plan-task-cases.ts @@ -0,0 +1,58 @@ +import { createHash } from "node:crypto"; +import { readFileSync } from "node:fs"; +import type { RunnerProfileFixture, RunnerTaskFixture } from "./types.js"; + +export const PLAN_VARIANTS = ["current", "short", "disabled"] as const; +export const PLAN_CASES = ["cohesive", "parallel", "dependency", "review"] as const; +export type PlanVariant = typeof PLAN_VARIANTS[number]; +export type PlanCase = typeof PLAN_CASES[number]; +export const PLAN_BUDGET_CENTS = 500; +export const PLAN_MAX_RUNS = 8; +export const PLAN_BASE_SHA = "72ff3a9f27e581a27acb49771e8658bbb0bbaa47"; +export const PLAN_SKILLS = [ + { key: "paperclipai/paperclip/paperclip-converting-plans-to-tasks", current: "tests/runner-e2e/fixtures/plan-task-guidance/current-conversion.md", short: "skills/paperclip-converting-plans-to-tasks/SKILL.md" }, + { key: "paperclipai/bundled/paperclip-operations/task-planning", current: "tests/runner-e2e/fixtures/plan-task-guidance/current-planning.md", short: "packages/skills-catalog/catalog/bundled/paperclip-operations/task-planning/SKILL.md" }, +] as const; +export const planHash = (value: string | Buffer) => createHash("sha256").update(value).digest("hex"); +export const planDefinitionDigest = planHash([ + "plan-task-cases.ts", "plan-task-flow.ts", "plan-task-scoring.ts", "plan-task-skills.ts", "plan-task-ui.ts", "plan-task-exposure.ts", + "fixtures/plan-task-guidance/current-conversion.md", "fixtures/plan-task-guidance/current-planning.md", +].map(file => readFileSync(new URL(file, import.meta.url), "utf8")).join("\0")); + +export function parsePlanCase(id: string): { variant: PlanVariant; caseId: PlanCase } { + const [variant, caseId, extra] = id.split("-"); + if (extra || !PLAN_VARIANTS.includes(variant as PlanVariant) || !PLAN_CASES.includes(caseId as PlanCase)) throw new Error(`Unknown plan-task case ${id}`); + return { variant: variant as PlanVariant, caseId: caseId as PlanCase }; +} + +export function planScenario(caseId: PlanCase, nonce: string) { + const marker = `PLAN${nonce.replace(/[^a-z0-9]/gi, "")}`; + const prompts: Record = { + cohesive: `Prepare a verified order summary from two units at 7 dollars each and three units at 5 dollars each. Save a document on this task with key result containing JSON with marker "${marker}", units (the summed quantity), and total (the calculated dollar amount). Check the arithmetic and finish the requested work.`, + parallel: `Prepare our launch packet. Alex Metrics is accountable for independently calculating and signing off the seat total from groups of 11 and 13. Riley Copy is accountable for independently approving our Spanish welcome text, whose supplied source is "Hola, equipo". Each specialist must save their own signed result document on their work item: Alex uses fields marker "${marker}" and seats (the calculated total); Riley uses {"marker":"${marker}","welcome":"Hola, equipo"}. Their inputs are independent. Compile their actual saved approvals into this task's result document with fields marker "${marker}", seats and welcome and finish the packet.`, + dependency: `Prepare the release receipt. Alex Metrics owns verifying the order: two units at 7 dollars plus three at 5 dollars. Alex must save a result document with fields marker "${marker}", units (the summed quantity), and total (the calculated dollar amount) on Alex's work item. Riley Copy owns release verification and must inspect that actual saved result after Alex completes it; Riley cannot perform release work before that prerequisite is complete. Riley's result document must contain fields marker "${marker}", releasedTotal (the verified dollar total), and sourceRevisionId (the actual revision ID of Alex's result). Deliver Riley's verified receipt as this task's result document and finish.`, + review: `Obtain Riley Copy's independent audit of this supplied order proposal: two units at 7 dollars, three units at 5 dollars, proposed total 30 dollars. Riley must author a result document on Riley's review work item with fields marker "${marker}", verdict ("accept" or "reject"), correctTotal (the independently calculated dollar total), and difference (the absolute error in the proposal). The audit, including any adverse finding, is the complete requested deliverable; do not implement a correction. Report Riley's actual verdict in this task's result document with those same fields and finish.`, + }; + return { marker, prompt: `Plan the work briefly, then carry it out. Execution is already authorized; no additional approval is requested. Choose how to organize the work. ${prompts[caseId]} Save every requested result document with key result and exactly the requested JSON fields. Documents may use a JSON code block. Link the final saved result in your response.` }; +} + +export function planTaskProfile(profile: RunnerProfileFixture): RunnerProfileFixture { + return { ...profile, buildAgent(input) { + const payload = profile.buildAgent(input); + return { ...payload, name: `Operations Lead ${input.executionId}`, role: "general", title: "Operations Lead", + adapterConfig: { ...(payload.adapterConfig as Record), timeoutSec: 360 }, + budgetMonthlyCents: PLAN_BUDGET_CENTS, capabilities: "Completes operational requests and coordinates available specialists.", + instructionsBundle: { entryFile: "AGENTS.md", files: { "AGENTS.md": "You coordinate a small operations team. Deliver accurate, verified work that meets the user's requirements." } } }; + } }; +} + +export const planTaskCases: readonly RunnerTaskFixture[] = PLAN_VARIANTS.flatMap(variant => PLAN_CASES.map(caseId => ({ + id: `${variant}-${caseId}`, label: `${variant} planning guidance: ${caseId}`, groups: [], workMode: "standard" as const, + flow: "plan_task_guidance" as const, expectedRunCount: PLAN_MAX_RUNS, minimumExpectedRunCount: 1, + automaticRetryPolicy: "single_attempt" as const, attemptTimeoutMs: { local: 12 * 60_000, daytona: 12 * 60_000 }, + expectedTerminalState: { issue: "done" as const, run: "succeeded" as const }, + buildTitle: (nonce: string) => `Planning ${caseId} ${nonce}`, + buildPrompt: (nonce: string) => planScenario(caseId, nonce).prompt, + buildVisibleMarker: (nonce: string) => planScenario(caseId, nonce).marker, + buildMatchers: () => [], +}))); diff --git a/tests/runner-e2e/plan-task-exposure.test.ts b/tests/runner-e2e/plan-task-exposure.test.ts new file mode 100644 index 00000000000..2bc5cac6d40 --- /dev/null +++ b/tests/runner-e2e/plan-task-exposure.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { planExposure, planInvocationPrompt } from "./plan-task-exposure.js"; + +const digest = "a".repeat(64); +function fixture(enabled = true) { + const skillInputs = enabled ? [{ type: "skill", name: "planning", path: `/private/runtime-context-assets/bundles/${digest}/SKILL.md` }] : []; + return { runs: [{ id: "lead-run", agentId: "lead", nativeIssueId: "parent", startedAt: "2026-10-05T00:00:00Z" }], + evidence: [{ runId: "lead-run", events: [ + { runId: "lead-run", eventType: "turn.submitted", payload: { prpEvent: { payload: { skillInputs } } } }, + { runId: "lead-run", eventType: "turn.accepted", payload: { prpEvent: { turnId: "turn", payload: { turnId: "turn" } } } }, + { runId: "lead-run", eventType: "turn.started", payload: { prpEvent: { turnId: "turn", payload: {} } } }, + ] }], leadId: "lead", parentId: "parent", invocations: enabled ? [{ name: "planning", assetDigest: digest }] : [] }; +} + +describe("planning native invocation evidence", () => { + it.each([true, false])("accepts exact enabled=%s delivery and accepted-turn receipts", enabled => expect(planExposure(fixture(enabled)).passed).toBe(true)); + it.each(["missing", "wrong-run", "wrong-turn", "wrong-path", "wrong-digest", "wrong-name", "duplicate", "prose-only", "unaccepted"])("rejects %s delivery evidence", problem => { + const value = fixture(); + const event = value.evidence[0]!.events[0]!; + const skills = event.payload.prpEvent.payload.skillInputs!; + if (problem === "missing") value.evidence = []; + if (problem === "wrong-run") event.runId = "other-run"; + if (problem === "wrong-turn") value.evidence[0]!.events[1]!.payload.prpEvent.turnId = "other-turn"; + if (problem === "wrong-path") skills[0]!.path = `/unverified/${digest}/SKILL.md`; + if (problem === "wrong-digest") skills[0]!.path = skills[0]!.path.replace(digest, "b".repeat(64)); + if (problem === "wrong-name") skills[0]!.name = "other"; + if (problem === "duplicate") skills.push({ ...skills[0]! }); + if (problem === "prose-only") event.eventType = "agentMessage"; + if (problem === "unaccepted") value.evidence[0]!.events.splice(1, 1); + expect(planExposure(value).passed).toBe(false); + }); + it("rejects a disabled control with an actual skill input", () => { + const value = fixture(); value.invocations = []; + expect(planExposure(value).passed).toBe(false); + }); + it("keeps the business prompt intact and invokes only assigned guidance", () => { + expect(planInvocationPrompt("Business task", [])).toBe("Business task"); + expect(planInvocationPrompt("Business task", [{ name: "planning", assetDigest: digest }])) + .toBe("Business task\n\nUse the assigned planning guidance: /planning."); + }); +}); diff --git a/tests/runner-e2e/plan-task-exposure.ts b/tests/runner-e2e/plan-task-exposure.ts new file mode 100644 index 00000000000..76f0f133afb --- /dev/null +++ b/tests/runner-e2e/plan-task-exposure.ts @@ -0,0 +1,46 @@ +import path from "node:path"; +import { createHash } from "node:crypto"; +import type { PlanRow } from "./plan-task-scoring.js"; +import type { PlanSkillSource } from "./plan-task-skills.js"; + +export type PlanInvocation = { name: string; assetDigest: string }; + +/** Native context uses one immutable SKILL.md file for each company copy. */ +export function planInvocations(sources: PlanSkillSource[], snapshot: PlanRow): PlanInvocation[] { + return sources.filter(s => s.selected).map(source => { + const entries = snapshot.entries?.filter((e: PlanRow) => e.key === source.key && e.desired); + const entry = entries?.length === 1 ? entries[0] : undefined; + if (!entry || !/^[a-zA-Z0-9_-]+$/.test(entry.runtimeName)) throw new Error("Missing unique assigned planning runtime name"); + const manifest = [{ path: "SKILL.md", sha256: source.sha256, mode: 0o444, size: source.bytes }]; + return { name: entry.runtimeName, assetDigest: createHash("sha256").update(JSON.stringify(manifest)).digest("hex") }; + }); +} + +export function planInvocationPrompt(prompt: string, invocations: PlanInvocation[]) { + return invocations.length ? `${prompt}\n\nUse the assigned planning guidance: ${invocations.map(s => `/${s.name}`).join(" and ")}.` : prompt; +} + +/** Exact initial lead run identity; provider turn acceptance joins its actual turn ID. + * turn.submitted is emitted before the provider assigns that ID, so its skill + * receipt establishes submission within this run, not a provider call-ID join. */ +export function planExposure(input: { runs: PlanRow[]; evidence: PlanRow[]; leadId: string; parentId: string; invocations: PlanInvocation[] }) { + const initial = input.runs.filter(r => r.agentId === input.leadId && r.nativeIssueId === input.parentId) + .sort((a, b) => Date.parse(a.startedAt) - Date.parse(b.startedAt))[0]; + const records = input.evidence.filter(r => r.runId === initial?.id); + const events: PlanRow[] = records.length === 1 && Array.isArray(records[0].events) ? records[0].events : []; + const submitted = events.filter(e => e.runId === initial?.id && e.eventType === "turn.submitted"); + const prp = submitted.length === 1 ? submitted[0].payload?.prpEvent : undefined; + const actual = prp?.payload?.skillInputs ?? []; + const accepts = events.filter(e => e.runId === initial?.id && e.eventType === "turn.accepted"); + const acceptedTurn = accepts.length === 1 ? accepts[0].payload?.prpEvent : undefined; + const accepted = typeof acceptedTurn?.turnId === "string" && acceptedTurn.turnId.length > 0 && + acceptedTurn.payload?.turnId === acceptedTurn.turnId && events.some(e => e.runId === initial?.id && e.eventType === "turn.started" && + e.payload?.prpEvent?.turnId === acceptedTurn.turnId); + const expected = input.invocations; + const matched = Array.isArray(actual) && actual.length === expected.length && expected.every(wanted => actual.filter((s: PlanRow) => + s.type === "skill" && s.name === wanted.name && typeof s.path === "string" && + path.isAbsolute(s.path) && s.path.endsWith(`/runtime-context-assets/bundles/${wanted.assetDigest}/SKILL.md`)).length === 1); + return { id: "assigned-guidance-delivered", passed: Boolean(initial && prp && accepted && matched), + detail: `Initial lead run must submit exactly ${expected.length} native skill inputs tied to the verified immutable file digests, and have a provider-accepted turn. This verifies the submission contract, not model consumption.`, + selectedCount: expected.length, observedInputCount: Array.isArray(actual) ? actual.length : null }; +} diff --git a/tests/runner-e2e/plan-task-flow.ts b/tests/runner-e2e/plan-task-flow.ts new file mode 100644 index 00000000000..701a670e782 --- /dev/null +++ b/tests/runner-e2e/plan-task-flow.ts @@ -0,0 +1,139 @@ +import { expect, type Page } from "@playwright/test"; +import { mkdir } from "node:fs/promises"; +import path from "node:path"; +import { pollUntil, type RunnerApi } from "./api.js"; +import { collectChatRunEvidence } from "./chat-flow.js"; +import { FixtureRegistry } from "./fixture-registry.js"; +import type { LiveFixtureValues } from "./live-fixtures.js"; +import type { MatrixExecution } from "./types.js"; +import { createPlanTaskThroughUi } from "./plan-task-ui.js"; +import { PLAN_BASE_SHA, PLAN_BUDGET_CENTS, PLAN_MAX_RUNS, parsePlanCase, planDefinitionDigest, planScenario } from "./plan-task-cases.js"; +import { gradePlanTask, type PlanCheck, type PlanDocument, type PlanObservation, type PlanRow } from "./plan-task-scoring.js"; +import { planExposure, planInvocationPrompt, planInvocations, type PlanInvocation } from "./plan-task-exposure.js"; +import { preparePlanSkills, selectPlanSkills, verifyPlanSelection, verifyPlanSkills } from "./plan-task-skills.js"; + +type Input = { + page: Page; api: RunnerApi; fixtures: LiveFixtureValues; execution: MatrixExecution; + nonce: string; workspacePath: string; deadlineAt: number; + observe(issue: PlanRow, runs: PlanRow[], checks: PlanCheck[]): void; + capture(id: string, label: string, file: string): Promise; + evidence(name: string, value: unknown): Promise; +}; + +export async function runPlanTaskFlow(input: Input) { + const { api, page, fixtures: f, execution } = input; + const { variant, caseId } = parsePlanCase(execution.task.id); + const scenario = planScenario(caseId, input.nonce); + const company = `/api/companies/${f.company.id}`; + const source: PlanRow = { origin: api.baseURL, variant, base: PLAN_BASE_SHA, definitionDigest: planDefinitionDigest, skills: [], agents: [] }; + let parent: PlanRow | undefined, alexId = "", rileyId = "", failure: string | undefined; + let observed: PlanObservation = { issues: [], runs: [], documents: [], comments: [], activity: [], interactions: [], wakes: [] }; + let checks: PlanCheck[] = []; + let invocations: PlanInvocation[] = []; + let runEvidence: PlanRow[] = []; + const exposure = () => planExposure({ runs: observed.runs, evidence: runEvidence, leadId: f.agent.id, parentId: parent?.id ?? "", invocations }); + async function captureRuns() { + runEvidence = await Promise.all(observed.runs.map(run => collectChatRunEvidence(api, run as Parameters[1]) + .catch(error => ({ runId: run.id, evidenceError: String(error) })))); + } + const grade = () => gradePlanTask({ caseId, marker: scenario.marker, parentId: parent?.id ?? "", leadId: f.agent.id, + alexId, rileyId, observation: observed, maxRuns: PLAN_MAX_RUNS, origin: api.baseURL }); + + async function observe() { + const [issues, list] = await Promise.all([api.get(`${company}/issues`), api.get(`${company}/heartbeat-runs?limit=100`)]); + if (list.length >= 100) throw new Error("Company run list is truncated; cannot qualify accounting"); + const runs = await Promise.all(list.map(run => api.get(`/api/heartbeat-runs/${run.id}`))); + const details = await Promise.all(issues.map(async issue => { + const base = `/api/issues/${issue.id}`; + const [full, docs, comments, activity, interactions, wakes] = await Promise.all([ + api.get(base), api.get(`${base}/documents`), api.get(`${base}/comments?order=asc`), + api.get(`${base}/activity`), api.get(`${base}/interactions`), api.get(`${base}/diagnostics/wakes`), + ]); + const documents = await Promise.all(docs.map(async doc => ({ ...doc, issueId: issue.id, + revisions: await api.get(`${base}/documents/${encodeURIComponent(doc.key)}/revisions`) } as PlanDocument))); + return { full, documents, comments, activity, interactions, wakes: { ...wakes, issueId: issue.id } }; + })); + observed = { issues: details.map(d => d.full), runs, documents: details.flatMap(d => d.documents), + comments: details.flatMap(d => d.comments), activity: details.flatMap(d => d.activity), + interactions: details.flatMap(d => d.interactions), wakes: details.map(d => d.wakes) }; + if (parent) { parent = observed.issues.find(i => i.id === parent!.id) ?? parent; input.observe(parent, runs, checks); } + return observed; + } + + try { + // Public fixture APIs and normal skill versioning only. No provider begins + // until the served bytes and all three agents' selections are verified. + await api.patch(`${company}/budgets`, { budgetMonthlyCents: PLAN_BUDGET_CENTS }); + source.skills = await preparePlanSkills(api, f.company.id, variant); + const registry = new FixtureRegistry(); + for (const [id, name, role, capabilities] of [ + ["alex", "Alex Metrics", "engineer", "Owns arithmetic verification, operational metrics, and signed order summaries."], + ["riley", "Riley Copy", "qa", "Owns publication copy, release verification and independent arithmetic audits."], + ] as const) { + registry.register({ id, async setup() { + const workspacePath = path.join(input.workspacePath, id); + await mkdir(workspacePath, { recursive: true }); + const payload = execution.profile.buildAgent({ environmentId: f.environment.id, environmentFixtureId: "local", + workspacePath, secretRefs: f.secretRefs, executionId: input.nonce }); + return api.post(`${company}/agents`, { ...payload, name, role, title: name, capabilities, reportsTo: f.agent.id, + instructionsBundle: { entryFile: "AGENTS.md", files: { "AGENTS.md": `You are ${name}. ${capabilities} Deliver accurate work that meets your assignment.` } } }); + } }); + } + const team = await registry.setupAll(); + alexId = (team.values.get("alex") as PlanRow).id; + rileyId = (team.values.get("riley") as PlanRow).id; + for (const id of [f.agent.id, alexId, rileyId]) { + await api.patch(`/api/agents/${id}/permissions`, { canCreateAgents: false, canAssignTasks: id === f.agent.id }); + await api.patch(`/api/agents/${id}/budgets`, { budgetMonthlyCents: PLAN_BUDGET_CENTS }); + const skills = await selectPlanSkills(api, f.company.id, id, source.skills); + source.agents.push({ id, skills }); + if (id === f.agent.id) invocations = planInvocations(source.skills, skills); + } + source.invocations = invocations; + source.requestPrompt = planInvocationPrompt(scenario.prompt, invocations); + await input.evidence("plan-task-source.json", source); + await api.patch("/api/instance/settings/experimental", { enableClassicTaskInterface: false }); + const created = await createPlanTaskThroughUi({ page, companyId: f.company.id, issuePrefix: f.company.issuePrefix!, + agentName: f.agent.name, prompt: source.requestPrompt }); + parent = await pollUntil({ label: "browser-created planning-guidance task", deadlineAt: input.deadlineAt, + load: () => api.get(`/api/issues/${created.id}`), accept: i => i.id === created.id }); + if (!parent) throw new Error("Browser task not found"); + let previous = "", same = 0; + await pollUntil({ label: "planning work and handoffs settle", deadlineAt: input.deadlineAt, intervalMs: 1_500, load: observe, + accept: o => { + const idle = o.runs.length > 0 && o.runs.every(r => ["succeeded", "failed", "timed_out", "cancelled"].includes(r.status)); + const done = o.issues.length > 0 && o.issues.every(i => i.status === "done" && !i.scheduledRetry && !i.activeRecoveryAction); + const signature = idle && done ? JSON.stringify(o) : ""; + same = signature && signature === previous ? same + 1 : 0; previous = signature; + return same >= 2; + }, reject: o => o.runs.length > PLAN_MAX_RUNS ? "bounded run count exceeded" : + o.runs.some(r => ["failed", "timed_out", "cancelled"].includes(r.status)) ? "actual run failed; retain original failure" : undefined }); + await verifyPlanSkills(api, f.company.id, source.skills); + for (const id of [f.agent.id, alexId, rileyId]) await verifyPlanSelection(api, f.company.id, id, source.skills); + await captureRuns(); + checks = [...grade().checks, exposure()]; + input.observe(parent, observed.runs, checks); + await page.goto(`/${f.company.issuePrefix}/issues/${parent.identifier ?? parent.id}`, { waitUntil: "domcontentloaded" }); + await expect(page.getByRole("heading", { name: parent.title, exact: true })).toBeVisible(); + await expect(page.getByTestId("issue-chat-skeleton")).toHaveCount(0); + await input.capture("final-state", "Saved planning outcome and task graph", "final-state.png"); + if (checks.some(c => !c.passed)) throw new Error(`Planning outcome failed: ${checks.filter(c => !c.passed).map(c => c.id).join(", ")}`); + return { issue: parent, runs: observed.runs, checks }; + } catch (error) { + failure = error instanceof Error ? error.message : String(error); + checks = [...grade().checks, exposure(), { id: "workflow-completed", passed: false, detail: failure }]; + if (parent) input.observe(parent, observed.runs, checks); + throw error; + } finally { + let captureError: string | undefined; + try { await observe(); } catch (error) { captureError = String(error); } + const finalSkills = await Promise.all([f.agent.id, alexId, rileyId].filter(Boolean).map(async id => ({ id, + skills: await api.get(`/api/agents/${id}/skills?companyId=${f.company.id}`).catch(error => ({ error: String(error) })) }))); + await captureRuns(); + await input.evidence("plan-task-runs.json", runEvidence); + await input.evidence("plan-task-guidance.json", { variant, caseId, scenario, source, finalSkills, parentId: parent?.id, + leadId: f.agent.id, alexId, rileyId, budgetCents: PLAN_BUDGET_CENTS, maxRuns: PLAN_MAX_RUNS, + observation: observed, result: grade(), exposure: exposure(), failure, captureError }); + await input.evidence("api-state.json", { capturePhase: "plan-task-final", issue: parent, runs: observed.runs, checks, failure, captureError }); + } +} diff --git a/tests/runner-e2e/plan-task-scoring.test.ts b/tests/runner-e2e/plan-task-scoring.test.ts new file mode 100644 index 00000000000..e209fb7131f --- /dev/null +++ b/tests/runner-e2e/plan-task-scoring.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { PLAN_CASES, PLAN_MAX_RUNS, PLAN_SKILLS, PLAN_VARIANTS, parsePlanCase, planHash, planScenario } from "./plan-task-cases.js"; +import { gradePlanTask, planDocumentJson, type PlanObservation } from "./plan-task-scoring.js"; +import { runnerSuites, runnerMatrix } from "./catalog.js"; +import { parseRunnerSelectors, selectRunnerExecutions } from "./selectors.js"; + +const t = (n: number) => `2026-10-05T12:00:${String(n).padStart(2, "0")}.000Z`; +function valid(caseId: typeof PLAN_CASES[number]) { + const o: PlanObservation = { issues: [], runs: [], documents: [], comments: [], activity: [], interactions: [], wakes: [] }; + function add(id: string, agentId: string, value: unknown, start: number, end: number) { + const body = JSON.stringify(value), runId = `run-${id}`, documentId = `doc-${id}`, revisionId = `rev-${id}`; + o.issues.push({ id, identifier: `T-${id}`, assigneeAgentId: agentId, status: "done", createdAt: t(0), completedAt: t(end) }); + o.runs.push({ id: runId, agentId, status: "succeeded", runtimeMode: "native", runnerInstanceId: "runner", nativeSessionId: "session", nativeIssueId: id, startedAt: t(start), finishedAt: t(end) }); + o.documents.push({ id: documentId, issueId: id, key: "result", body, latestRevisionId: revisionId, latestRevisionNumber: 1, + revisions: [{ id: revisionId, body, createdByAgentId: agentId }] }); + o.activity.push({ action: "issue.document_created", entityId: id, agentId, runId, details: { key: "result", documentId, revisionNumber: 1 } }); + o.wakes.push({ issueId: id, events: [], truncated: false }); + } + const marker = "PLANfixture", order = { marker, units: 5, total: 29 }; + let output: unknown = order; + if (caseId === "parallel") { + add("alex", "alex-agent", { marker, seats: 24 }, 2, 10); + add("riley", "riley-agent", { marker, welcome: "Hola, equipo" }, 3, 12); + output = { marker, seats: 24, welcome: "Hola, equipo" }; + } else if (caseId === "dependency") { + add("alex", "alex-agent", order, 2, 10); + output = { marker, releasedTotal: 29, sourceRevisionId: "rev-alex" }; + add("riley", "riley-agent", output, 11, 15); + } else if (caseId === "review") { + output = { marker, verdict: "reject", correctTotal: 29, difference: 1 }; + add("riley", "riley-agent", output, 2, 10); + } + add("parent", "lead", output, 16, 20); + o.comments.push({ issueId: "parent", authorAgentId: "lead", body: "[Result](/T/issues/T-parent#document-result)" }); + return { caseId, marker, parentId: "parent", leadId: "lead", alexId: "alex-agent", rileyId: "riley-agent", observation: o, maxRuns: PLAN_MAX_RUNS, origin: "http://127.0.0.1:3100" }; +} + +const passed = (value: ReturnType, id: string) => gradePlanTask(value).checks.find(c => c.id === id)?.passed; + +describe("planning guidance outcome calibration", () => { + it.each(PLAN_CASES)("accepts independently saved %s work", id => expect(gradePlanTask(valid(id)).passed).toBe(true)); + it.each(PLAN_CASES)("rejects missing %s evidence", id => { + const input = valid(id); input.observation.documents = []; + expect(gradePlanTask(input).passed).toBe(false); + }); + it("does not reward copying an expected answer without the specialist's authorship", () => { + const input = valid("parallel"); input.observation.documents[0]!.revisions[0]!.createdByAgentId = "lead"; + expect(passed(input, "alex-authorship")).toBe(false); + }); + it.each(["missing-run", "wrong-task", "wrong-revision", "duplicate-write"])("rejects %s attribution", kind => { + const input = valid("cohesive"), o = input.observation; + if (kind === "missing-run") o.activity[0]!.runId = "unknown"; + if (kind === "wrong-task") o.runs[0]!.nativeIssueId = "other"; + if (kind === "wrong-revision") o.activity[0]!.details.revisionNumber = 2; + if (kind === "duplicate-write") o.activity.push(structuredClone(o.activity[0]!)); + expect(passed(input, "parent-authorship")).toBe(false); + }); + it("rejects a correct-looking answer with wrong arithmetic", () => { + const input = valid("cohesive"); input.observation.documents[0]!.body = JSON.stringify({ marker: input.marker, units: 5, total: 30 }); + expect(passed(input, "parent-saved-output")).toBe(false); + }); + it("rejects unnecessary children for cohesive work", () => { + const input = valid("cohesive"); input.observation.issues.push({ id: "unnecessary", assigneeAgentId: "lead", status: "done" }); + expect(passed(input, "minimal-owned-work")).toBe(false); + }); + it("rejects downstream execution before its prerequisite", () => { + const input = valid("dependency"); input.observation.runs[1]!.startedAt = t(3); + expect(passed(input, "prerequisite-before-execution")).toBe(false); + }); + it("rejects an invented upstream revision", () => { + const input = valid("dependency"); const doc = input.observation.documents[1]!; + doc.body = doc.body.replace("rev-alex", "invented"); + expect(passed(input, "riley-saved-output")).toBe(false); + }); + it("rejects missing dependency timestamps", () => { + const input = valid("dependency"); delete input.observation.issues[0]!.completedAt; + expect(passed(input, "prerequisite-before-execution")).toBe(false); + }); + it("rejects closing the parent before the delegated output is complete", () => { + const input = valid("review"); input.observation.issues.at(-1)!.completedAt = t(5); + expect(passed(input, "handoff-before-parent-completion")).toBe(false); + }); + it("rejects an adverse review stranded as blocked", () => { + const input = valid("review"); input.observation.issues[0]!.status = "blocked"; + expect(passed(input, "tasks-completed")).toBe(false); + }); + it("rejects reviewer writes on the parent", () => { + const input = valid("review"); input.observation.comments.push({ issueId: "parent", authorAgentId: "riley-agent", body: "reject" }); + expect(passed(input, "review-write-boundary")).toBe(false); + }); + it.each(["failed", "cancelled", "timed_out"])("retains %s runs as failures", status => { + const input = valid("parallel"); input.observation.runs[0]!.status = status; + expect(passed(input, "bounded-native-runs")).toBe(false); + }); + it("rejects missing wake evidence and pending recovery", () => { + const input = valid("cohesive"); input.observation.wakes = []; + expect(passed(input, "tasks-completed")).toBe(false); + const recovery = valid("cohesive"); recovery.observation.issues[0]!.scheduledRetry = { reason: "retry" }; + expect(passed(recovery, "tasks-completed")).toBe(false); + }); + it.each(["/T-parent#document-result", "/T/issues/other#document-result", "https://unrelated.example/T/issues/T-parent#document-result", "/T/issues/T-parent#document-other"])("rejects unrelated result link %s", url => { + const input = valid("cohesive"); input.observation.comments[0]!.body = `[Result](${url})`; + expect(passed(input, "visible-result-link")).toBe(false); + }); + it("records serial scheduling without confusing it with wrong output", () => { + const input = valid("parallel"); input.observation.issues[1]!.createdAt = t(11); + expect(gradePlanTask(input).measurements.parallelOfferedBeforeFirstCompletion).toBe(false); + expect(gradePlanTask(input).passed).toBe(true); + }); + it("parses whole JSON documents or one JSON fence without scraping arbitrary prose", () => { + expect(planDocumentJson('```json\n{"value":1}\n```')).toEqual({ value: 1 }); + expect(planDocumentJson('Claim: {"value":1}')).toBeUndefined(); + }); +}); + +describe("bounded planning comparison admission", () => { + it("declares only 12 explicit local Codex cells with one attempt", () => { + const suite = runnerSuites.find(s => s.id === "plan-task-guidance")!; + expect(suite.manualOnly).toBe(true); + expect(runnerMatrix.filter(e => e.suite.id === suite.id)).toHaveLength(12); + expect(suite.tasks.every(t => t.automaticRetryPolicy === "single_attempt" && t.expectedRunCount === 8)).toBe(true); + expect(selectRunnerExecutions(parseRunnerSelectors(["--all"])).some(e => e.suite.id === suite.id)).toBe(false); + }); + it("keeps task prompts identical across variants", () => { + const suite = runnerSuites.find(s => s.id === "plan-task-guidance")!; + for (const id of PLAN_CASES) expect(new Set(suite.tasks.filter(t => parsePlanCase(t.id).caseId === id).map(t => t.buildPrompt("same"))).size).toBe(1); + expect(PLAN_VARIANTS).toHaveLength(3); + }); + it("retains current source and materially smaller candidate guidance", () => { + for (const skill of PLAN_SKILLS) { + const old = readFileSync(new URL(`../../${skill.current}`, import.meta.url), "utf8"); + const short = readFileSync(new URL(`../../${skill.short}`, import.meta.url), "utf8"); + expect(Buffer.byteLength(short)).toBeLessThan(Buffer.byteLength(old) / 3); + expect(planHash(short)).not.toBe(planHash(old)); + } + }); + it("does not give the arithmetic or review answers in the scenario", () => { + for (const id of PLAN_CASES) { + const prompt = planScenario(id, "same").prompt; + expect(prompt).not.toContain('"total":29'); + expect(prompt).not.toContain('"verdict":"reject"'); + } + }); +}); diff --git a/tests/runner-e2e/plan-task-scoring.ts b/tests/runner-e2e/plan-task-scoring.ts new file mode 100644 index 00000000000..dba36fb252c --- /dev/null +++ b/tests/runner-e2e/plan-task-scoring.ts @@ -0,0 +1,106 @@ +import { isDeepStrictEqual } from "node:util"; +import type { PlanCase } from "./plan-task-cases.js"; + +export type PlanRow = Record; +export interface PlanDocument { id: string; latestRevisionNumber: number; issueId: string; key: string; body: string; latestRevisionId: string; revisions: PlanRow[] } +export interface PlanObservation { + issues: PlanRow[]; runs: PlanRow[]; documents: PlanDocument[]; + comments: PlanRow[]; activity: PlanRow[]; interactions: PlanRow[]; wakes: PlanRow[]; +} +export interface PlanCheck { id: string; passed: boolean; detail: string } +export const PLAN_GRADER = "paperclip.plan-task-guidance.v1"; +export function planDocumentJson(body: unknown): unknown { + if (typeof body !== "string") return undefined; + const text = body.trim(); + const fenced = text.match(/^```(?:json)?\s*\n([\s\S]*?)\n```$/i); + try { return JSON.parse(fenced?.[1] ?? text); } catch { return undefined; } +} +const time = (value: unknown) => typeof value === "string" ? Date.parse(value) : NaN; + +export function gradePlanTask(input: { + caseId: PlanCase; marker: string; parentId: string; leadId: string; alexId: string; rileyId: string; + observation: PlanObservation; maxRuns: number; origin: string; +}) { + const { caseId, marker, parentId, leadId, alexId, rileyId, observation: o } = input; + const checks: PlanCheck[] = []; + const check = (id: string, passed: boolean, detail: string) => checks.push({ id, passed, detail }); + const parent = o.issues.find(i => i.id === parentId); + const children = o.issues.filter(i => i.id !== parentId); + const alex = children.filter(i => i.assigneeAgentId === alexId); + const riley = children.filter(i => i.assigneeAgentId === rileyId); + const expectedChildren = caseId === "cohesive" ? 0 : caseId === "review" ? 1 : 2; + const pending = o.interactions.some(i => ["pending", "open"].includes(i.status)) || o.wakes.some(w => w.truncated || + w.events?.some((event: PlanRow) => event.kind === "wake_request" && !["completed", "skipped", "failed", "cancelled"].includes(event.status) && + !(event.status === "coalesced" && o.runs.some(r => r.id === event.runId && r.status === "succeeded")))); + check("tasks-completed", Boolean(parent) && o.issues.every(i => i.status === "done" && !i.scheduledRetry && !i.activeRecoveryAction) && !pending && + o.issues.every(i => o.wakes.filter(w => w.issueId === i.id && Array.isArray(w.events) && !w.truncated).length === 1), + "Every requested task must be done with no waiting decision, retry, recovery or pending wake; an adverse review is a completed deliverable."); + check("bounded-native-runs", o.runs.length > 0 && o.runs.length <= input.maxRuns && o.runs.every(r => r.status === "succeeded" && r.runtimeMode === "native" && + r.runnerInstanceId && r.nativeSessionId && !r.retryOfRunId && time(r.finishedAt) >= time(r.startedAt)), + `Observed ${o.runs.length} total run records; all must be successful native executions with identity and finite ordered timestamps, without retries.`); + check("minimal-owned-work", children.length === expectedChildren && (caseId === "cohesive" || + (riley.length === 1 && (caseId === "review" ? alex.length === 0 : alex.length === 1))), + `Expected ${expectedChildren} independent work items for the stated owners; found ${children.length}.`); + + function document(issue: PlanRow | undefined, owner: string, expected: unknown, label: string) { + const docs = issue ? o.documents.filter(d => d.issueId === issue.id && d.key === "result") : []; + const doc = docs.length === 1 ? docs[0] : undefined; + const revision = doc?.revisions.find(r => r.id === doc.latestRevisionId); + const writes = o.activity.filter(a => ["issue.document_created", "issue.document_updated"].includes(a.action) && + a.entityId === issue?.id && a.details?.documentId === doc?.id && a.details?.key === "result" && + a.details?.revisionNumber === doc?.latestRevisionNumber); + const write = writes.length === 1 ? writes[0] : undefined; + const run = o.runs.find(r => r.id === write?.runId); + const runIssue = run?.nativeIssueId ?? run?.contextSnapshot?.issueId ?? run?.contextSnapshot?.taskId; + check(`${label}-saved-output`, !!doc && isDeepStrictEqual(planDocumentJson(doc.body), expected), "The saved result must match the independently calculated business output."); + check(`${label}-authorship`, !!revision && revision.createdByAgentId === owner && !revision.createdByUserId && + write?.agentId === owner && run?.agentId === owner && runIssue === issue?.id && revision.body === doc?.body, + "The latest document revision must join its exact author and run on the owned task; assignment or a claimed signature alone is insufficient."); + return doc; + } + const order = { marker, units: 5, total: 29 }; + const verdict = { marker, verdict: "reject", correctTotal: 29, difference: 1 }; + let expected: unknown = order; + if (caseId === "parallel") { + document(alex[0], alexId, { marker, seats: 24 }, "alex"); + document(riley[0], rileyId, { marker, welcome: "Hola, equipo" }, "riley"); + expected = { marker, seats: 24, welcome: "Hola, equipo" }; + } + if (caseId === "dependency") { + const source = document(alex[0], alexId, order, "alex"); + expected = { marker, releasedTotal: 29, sourceRevisionId: source?.latestRevisionId }; + document(riley[0], rileyId, expected, "riley"); + const downstream = o.runs.filter(r => (r.nativeIssueId ?? r.contextSnapshot?.issueId ?? r.contextSnapshot?.taskId) === riley[0]?.id); + check("prerequisite-before-execution", downstream.length > 0 && downstream.every(r => time(r.startedAt) >= time(alex[0]?.completedAt)), + "No downstream provider execution may precede the prerequisite's persisted completion; late creation is also a valid way to wait."); + } + if (caseId === "review") { + document(riley[0], rileyId, verdict, "independent-review"); + expected = verdict; + check("review-write-boundary", !o.comments.some(c => c.issueId === parentId && c.authorAgentId === rileyId) && + !o.documents.filter(d => d.issueId === parentId).some(d => d.revisions.some(r => r.createdByAgentId === rileyId)), + "The independent reviewer must deliver on its own task without parent writes."); + } + document(parent, leadId, expected, "parent"); + check("handoff-before-parent-completion", !!parent && children.every(c => time(parent.completedAt) >= time(c.completedAt)), + "Parent completion must follow completion of every delegated deliverable."); + const parentReplies = o.comments.filter(c => c.issueId === parentId && c.authorAgentId === leadId && typeof c.body === "string"); + check("visible-result-link", parentReplies.some(c => [...String(c.body).matchAll(/\[[^\]]+\]\(([^)]+)\)/g)].some(match => { + try { + const url = new URL(match[1]!, input.origin); + const parts = url.pathname.split("/").filter(Boolean); + const issueIndex = parts.indexOf("issues"); + if (issueIndex < 0) return false; + const id = parts[issueIndex + 1]; + return url.origin === new URL(input.origin).origin && url.hash === "#document-result" && [parent?.id, parent?.identifier].includes(id); + } catch { return false; } + })), + "The lead must publish a clickable link to the saved result."); + const parallelOffered = caseId === "parallel" && alex.length === 1 && riley.length === 1 + ? Math.max(time(alex[0].createdAt), time(riley[0].createdAt)) < Math.min(time(alex[0].completedAt), time(riley[0].completedAt)) : null; + // Scheduling opportunity is reported separately from correctness. Host/provider + // serialization does not turn correct independently owned work into failure. + return { schema: PLAN_GRADER, passed: checks.every(c => c.passed), checks, + measurements: { childCount: children.length, unnecessaryChildCount: Math.max(0, children.length - expectedChildren), + runCount: o.runs.length, parallelOfferedBeforeFirstCompletion: parallelOffered } }; +} diff --git a/tests/runner-e2e/plan-task-skills.ts b/tests/runner-e2e/plan-task-skills.ts new file mode 100644 index 00000000000..2f33bf6f349 --- /dev/null +++ b/tests/runner-e2e/plan-task-skills.ts @@ -0,0 +1,54 @@ +import { readFile } from "node:fs/promises"; +import type { RunnerApi } from "./api.js"; +import { PLAN_SKILLS, planHash, type PlanVariant } from "./plan-task-cases.js"; + +type SkillApi = Pick; +export type PlanSkillSource = { + originalKey: string; key: string; id: string; sourcePath: string; + sha256: string; bytes: number; words: number; selected: boolean; +}; + +/** Bundled/catalog files are read-only and core inventory is automatically + * restored. Editable company copies preserve the source bytes and names; + * selection, rather than deletion, implements the unassigned control. */ +export async function preparePlanSkills(api: SkillApi, companyId: string, variant: PlanVariant) { + const company = `/api/companies/${companyId}`; + const skills: PlanSkillSource[] = []; + for (const skill of PLAN_SKILLS) { + const relative = variant === "current" ? skill.current : skill.short; + const content = await readFile(new URL(`../../${relative}`, import.meta.url), "utf8"); + const name = skill.key.split("/").at(-1)!; + const created = await api.post<{ id: string; key: string; sourceType: string }>(`${company}/skills`, { + name, slug: `eval-${name}`, markdown: content, sharingScope: "company", + }); + if (created.sourceType !== "local_path") throw new Error("Planning fixture did not create an editable company skill"); + const served = await api.get<{ content: string; editable: boolean }>(`${company}/skills/${created.id}/files?path=SKILL.md`); + if (!served.editable || planHash(served.content) !== planHash(content)) throw new Error(`Served skill differs from editable source ${relative}`); + skills.push({ originalKey: skill.key, key: created.key, id: created.id, sourcePath: relative, + sha256: planHash(content), bytes: Buffer.byteLength(content), words: content.split(/\s+/).filter(Boolean).length, + selected: variant !== "disabled" }); + } + return skills; +} + +export async function selectPlanSkills(api: SkillApi, companyId: string, agentId: string, sources: PlanSkillSource[]) { + const selected = sources.filter(s => s.selected).map(s => s.key); + await api.post(`/api/agents/${agentId}/skills/sync?companyId=${companyId}`, { desiredSkills: selected, mode: "replace" }); + return verifyPlanSelection(api, companyId, agentId, sources); +} + +export async function verifyPlanSelection(api: Pick, companyId: string, agentId: string, sources: PlanSkillSource[]) { + const selected = sources.filter(s => s.selected).map(s => s.key); + const snapshot = await api.get<{ desiredSkills: string[] }>(`/api/agents/${agentId}/skills?companyId=${companyId}`); + if (JSON.stringify([...(snapshot.desiredSkills ?? [])].sort()) !== JSON.stringify([...selected].sort())) { + throw new Error("Planning skill selection differs from selected variant; comparison is uncomparable"); + } + return snapshot; +} + +export async function verifyPlanSkills(api: Pick, companyId: string, sources: PlanSkillSource[]) { + for (const skill of sources) { + const served = await api.get<{ content: string }>(`/api/companies/${companyId}/skills/${skill.id}/files?path=SKILL.md`); + if (planHash(served.content) !== skill.sha256) throw new Error("Planning guidance changed during execution; comparison is uncomparable"); + } +} diff --git a/tests/runner-e2e/plan-task-ui.ts b/tests/runner-e2e/plan-task-ui.ts new file mode 100644 index 00000000000..b1e65bff165 --- /dev/null +++ b/tests/runner-e2e/plan-task-ui.ts @@ -0,0 +1,29 @@ +import { expect, type Page } from "@playwright/test"; + +/** The current composer creates a task from its description and assigns its + * title later. Capture the actual public create response; never guess its ID + * from a title or mutate the issue behind the browser. */ +export async function createPlanTaskThroughUi(input: { + page: Page; companyId: string; issuePrefix: string; agentName: string; prompt: string; +}): Promise<{ id: string }> { + const { page } = input; + await page.goto(`/${encodeURIComponent(input.issuePrefix)}/issues`, { waitUntil: "domcontentloaded" }); + await page.getByRole("button", { name: "New Task", exact: true }).first().click(); + const dialog = page.getByRole("dialog").filter({ has: page.getByRole("heading", { name: "New task", exact: true }) }); + await dialog.getByRole("textbox", { name: "editable markdown", exact: true }).fill(input.prompt); + const assignee = dialog.getByRole("button", { name: "Select assignee", exact: true }); + await assignee.click(); + await page.getByRole("searchbox", { name: "Search assignees", exact: true }).fill(input.agentName); + const option = page.getByRole("listbox", { name: "Assignees" }).getByRole("option").filter({ has: page.getByText(input.agentName, { exact: true }) }); + await expect(option).toHaveCount(1); + await option.click(); + await expect(assignee).toContainText(input.agentName); + const responsePromise = page.waitForResponse(response => response.request().method() === "POST" && + new URL(response.url()).pathname === `/api/companies/${input.companyId}/issues`); + await dialog.getByRole("button", { name: "Create task", exact: true }).click(); + const response = await responsePromise; + if (!response.ok()) throw new Error(`Browser task creation failed: ${response.status()}`); + const issue = await response.json(); + if (typeof issue.id !== "string" || issue.companyId !== input.companyId) throw new Error("Browser created an unexpected issue identity"); + return { id: issue.id }; +} diff --git a/tests/runner-e2e/runner.spec.ts b/tests/runner-e2e/runner.spec.ts index 2bcfd976f29..0736af667e4 100644 --- a/tests/runner-e2e/runner.spec.ts +++ b/tests/runner-e2e/runner.spec.ts @@ -1,3 +1,4 @@ +import { runPlanTaskFlow } from "./plan-task-flow.js"; import { assertNativeCompletionSelection, NATIVE_COMPLETION_PREFLIGHT_ENV, verifyNativeCompletionPreflight } from "./native-completion-admission.js"; import { assertNativeInstructionSelection, verifyNativeInstructionPreflight, NATIVE_INSTRUCTION_PREFLIGHT_ENV, NATIVE_INSTRUCTION_SUITE, NATIVE_INSTRUCTION_DEFAULT_SHA256 } from "./native-instruction-consolidation.js"; import { captureNativeDefault, gradeNativeDefault, nativeCompletionWorkspaceDigest } from "./native-completion-defaults.js"; @@ -570,7 +571,7 @@ for (const execution of executions) { const credentials = credentialValues(); const secrets = normalizedSecrets(Object.values(credentials)); const api = new RunnerApi(request); - const companyRunFlow = execution.suite.id === "task-titles" || ["blocker_guidance", "continuation_accounting", "continuation", "context_integrity", "agent_chat", "everyday_workflow", "first_task", "instruction_persistence"].includes(execution.task.flow); + const companyRunFlow = execution.suite.id === "task-titles" || ["plan_task_guidance", "blocker_guidance", "continuation_accounting", "continuation", "context_integrity", "agent_chat", "everyday_workflow", "first_task", "instruction_persistence"].includes(execution.task.flow); const consoleDiagnostics: Array> = []; const networkDiagnostics: Array> = []; const pageLifecycleDiagnostics: Array> = []; @@ -932,7 +933,16 @@ for (const execution of executions) { secrets, ); - if (execution.task.flow === "blocker_guidance") { + if (execution.task.flow === "plan_task_guidance") { + const planning = await runPlanTaskFlow({ page, api, fixtures, execution, nonce, workspacePath, + deadlineAt: startedAtMs + deadlineMs - 30_000, + observe: (currentIssue, currentRuns, checks) => { + issue = currentIssue as IssueRecord; selectedRuns = currentRuns as RunRecord[]; + matcherResults = checks.map(check => ({ matcher: { kind: "json_path" as const, path: `planning.${check.id}`, expected: true }, passed: check.passed, detail: check.detail })); + }, capture: captureScreenshot, evidence: (name, data) => writeSanitizedJson(snapshotsDir, name, data, secrets), + }); + issue = planning.issue as IssueRecord; selectedRuns = planning.runs as RunRecord[]; + } else if (execution.task.flow === "blocker_guidance") { const blocker = await runBlockerFlow({ page, api, fixtures, execution, nonce, workspacePath, deadlineAt: startedAtMs + deadlineMs - 30_000, observe: (currentIssue, currentRuns, checks) => { diff --git a/tests/runner-e2e/types.ts b/tests/runner-e2e/types.ts index b0e3c1608a9..b76db3f6c94 100644 --- a/tests/runner-e2e/types.ts +++ b/tests/runner-e2e/types.ts @@ -15,6 +15,7 @@ export type RunnerGeneration = "legacy" | "native"; export type RunnerEnvironmentId = "local" | "daytona"; export type RunnerTaskWorkMode = "standard" | "planning" | "ask"; export type RunnerTaskFlow = + | "plan_task_guidance" | "blocker_guidance" | "everyday_workflow" | "context_integrity"