diff --git a/DESIGN.md b/DESIGN.md index d80f6a7075..75a2972dbc 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -56,6 +56,11 @@ paused.” and “Resume this task to send a message.” with a “Resume task Subtrees use “Subtree is paused.” and “Resume subtree.” The takeover cannot be dismissed, retains drafts, and hides message inputs until the pause is released. +Pending questions, confirmations, and other task-thread inputs appear in a separate +card directly above the ordinary composer. The composer stays available for new +messages while the card is open. Dismissing a card leaves a pending indicator that +can reopen it; resolving or skipping the input removes that indicator. + ## Enforcement (what "compliant" means for the extraction run) - **Zero visual change is proven, not promised:** Storybook visual snapshots are baselined before any refactor, and all snapshots match baseline after it. A change that alters rendered output must be intentional and human-approved. diff --git a/doc/DATABASE.md b/doc/DATABASE.md index 3c82a93a2b..151581873d 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -165,6 +165,15 @@ idempotent actor synchronization operations, not arbitrary transactions. A persistent outage still fails the request after the bounded retries; each connection attempt remains subject to the configured database connect timeout. +## Execution identity row locks + +Identity initialization, credential acquisition, and steering reconciliation lock +the task before its run. These operations use `FOR NO KEY UPDATE`: they change +identity state, not parent keys. The lock still serializes identity writers and +blocks concurrent task or run updates. It allows audit inserts to retain their +foreign-key `KEY SHARE` locks without waiting on identity acquisition. The audit +foreign keys and their deletion behavior remain enforced. + ## Switching between modes The database mode is controlled by `DATABASE_URL`: diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 417715fc92..c830de4bcb 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -106,6 +106,16 @@ next question. Reduced-motion mode advances without animation. Multi-select and custom answers wait for Next, and the final page waits for Submit answers. The adjacent **Verified** story exercises the full flow. +Use **Composer → Interaction above composer** to review the production +pending-input layout. The stories cover questions, confirmations, checkbox +choices, item verdicts, suggested tasks, tool reviews, runtime questions, and phone layouts with +the bottom navigation. The normal message composer remains usable below the +pending card. + +Use **Composer → Model and effort picker** to review harness-specific model +choices. Codex uses the curated adapter catalog unless the instance declares +`PAPERCLIP_ADAPTER_MODELS`; general OpenAI API models are not Codex choices. + The Storybook visual regression suite uses external PNG baselines instead of committed screenshots: @@ -982,6 +992,8 @@ eval "$(npx paperclipai worktree env)" For project execution worktrees, Paperclip can also run a project-defined provision command after it creates or reuses an isolated git worktree. Configure this on the project's execution workspace policy (`workspaceStrategy.provisionCommand`). The command runs inside the derived worktree and receives `PAPERCLIP_WORKSPACE_*`, `PAPERCLIP_PROJECT_ID`, `PAPERCLIP_AGENT_ID`, and `PAPERCLIP_ISSUE_*` environment variables so each repo can bootstrap itself however it wants. +An issue's partial `workspaceStrategy` inherits omitted fields from the enabled project's strategy when both use the same type. For example, an issue can override `baseRef` without losing the project's provision, runtime provision, or teardown commands. An explicit value, including `null` or an empty string, replaces the project value. Clearing a command restores the runtime's usual default behavior; use `provisionCommand: "true"` for an explicit no-op. A different strategy type or a disabled project policy does not supply these defaults. An issue's `existingBranch` pin also excludes the project's `branchTemplate`. + An issue can pin its isolated worktree to an exact pre-existing branch instead of a template-derived one — the contract PR-preparation tasks use. Set the issue's `executionWorkspaceSettings` to `{ "mode": "isolated_workspace", "workspaceStrategy": { "type": "git_worktree", "existingBranch": "" } }`. The validator requires isolated mode plus a `git_worktree` strategy and rejects `branchTemplate` alongside `existingBranch`. At dispatch the runtime attaches (never creates, renames, fast-forwards, or resets) that branch: it reuses a registered worktree that already has the branch checked out (including legacy `.worktrees/` paths), otherwise it attaches the branch under the managed worktree parent. A missing branch, an occupied worktree path on another branch, or a non-worktree strategy fails closed with a `workspace_validation_failed` error instead of falling back to the shared checkout or a derived branch, and an inherited `reuse_existing` workspace binding on a different branch is ignored in favor of realizing the pinned branch. Heavier setup that is only needed by a managed runtime service can use `workspaceStrategy.runtimeProvisionCommand`. Paperclip runs this command lazily before spawning the first service in a start batch, serializes concurrent provisioning for the same workspace, and records the attempt as `workspace_runtime_provision`. The command receives the same workspace environment as `provisionCommand` and should be idempotent because later service-start batches invoke it again. @@ -1097,6 +1109,35 @@ agent workspace. The host `HOME` itself, a directory that contains it, a filesystem root, a `CODEX_HOME` overlap, or a canonical path outside the assigned workspace is rejected before provider startup. +### Sandbox ACP input delivery + +The legacy sandbox process bridge retries recognized Daytona and Cloudflare +HTTP 502, 503, and 504 failures while writing an input message, with at most +three attempts and a short backoff. +Retries keep the message sequence and use separate temporary upload files. +The remote wrapper discards already-consumed sequences, so a lost provider +response cannot send the same input bytes twice. Messages remain ordered. +This does not restart an agent turn or replay a tool call. Authentication and +shell errors fail immediately; exhausted input delivery closes the bridge and +records a fixed diagnostic without logging the input payload. Persisting that +failure diagnostic does not block bridge teardown. +Run-log finalization closes its write handle and waits for accepted file +appends before computing the size, hash, and durable copy. Writes submitted +after finalization starts are ignored; later progress persistence is not part +of that file-write barrier. If accepted writes remain stalled after three +seconds, finalization returns unknown size/hash metadata and skips the final +durable copy so the run can reach a terminal state. A late write cannot restart +mirroring or produce a claimed verified snapshot. +Readers still attempt bounded reads when size is unknown. Legacy comment +attribution retains its existing 2 MB scan limit and allows three seconds per +log. Storage errors or timeouts preserve any evidence already read and leave +the comments available without additional derived attribution. +The read deadline requests cancellation of local file streams, S3 HEAD and GET +requests, and S3 response streams. The listing stops waiting at the deadline +even if filesystem I/O delays cancellation. Late results cannot add evidence +or start another page. Each listing retains its existing batches of eight reads; +concurrent listings do not skip healthy logs because another listing is busy. + ### Preinstalled remote runner runtime For fast sandbox startup, bake `paperclip-runnerd` and the latest stable agent @@ -1631,3 +1672,16 @@ disconnected, visible active queries refresh every 15 seconds. This fallback stops when the socket opens, the tab is hidden, or the provider unmounts. A reconnected socket also refreshes visible queries to recover missed events. Run log views retain their existing HTTP polling fallback. + +### Company context during hot reload + +The company React context retains only its object identity in Vite's per-module +`hot.data`. Provider values remain in the mounted React tree and keep their normal +account scope. This lets a refreshed consumer read a provider from the preceding +module version. Production builds do not use the development cache. + +Run `pnpm test:e2e:browser-context` to test this with real Vite modules and +Chromium. The test starts its own loopback Vite server and mocks API responses; +it needs no running Paperclip instance or provider credentials. The same spec lives +in the default `test:e2e` discovery tree, so the existing Chrome CI shards run it +on pull requests. diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 04aaa912d5..927e7f1a7e 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -698,6 +698,11 @@ Issue-thread interactions are coordination records, not grants of authority. Eve interaction kind defaults to resolver policy `anyone` when the create request omits `resolverPolicy`. Restrictions are opt-in. +Question, confirmation, checkbox confirmation, and item verdict cards stay pending +when a user sends an ordinary task comment. Their `supersedeOnUserComment` flag +defaults to `false`. A creator may set it to `true` when a comment should replace +the pending request, as the opening onboarding question does. + Canonical resolver policies are: - `anyone`: any authenticated actor in the interaction's company who can read the @@ -1348,7 +1353,8 @@ Board can at any time: Ask-first connection calls use a server-owned tool-action confirmation linked to the authoritative action request. The task feed retains a stable record; dismissal -only hides the composer takeover. Task and Connections decisions share one +only hides the pending card above the composer. The ordinary composer remains +available while the card is open. Task and Connections decisions share one transaction. Approval runs stored, signed arguments once; decline runs nothing. The human decision remains distinct from provider execution success or failure. diff --git a/doc/composer-stop.md b/doc/composer-stop.md index 7c77b2d113..5579d55f10 100644 --- a/doc/composer-stop.md +++ b/doc/composer-stop.md @@ -110,8 +110,8 @@ Run from the worktree: pnpm --filter @paperclipai/ui exec storybook dev -p 6016 -c storybook/.storybook --no-open ``` -Open `http://localhost:6016/?path=/story/tasks-execution-controls--running-empty`. -The `Tasks / Execution Controls` stories compose the production composer and +Open `http://localhost:6016/?path=/story/composer-execution-controls--running-empty`. +The `Composer / Execution controls` stories compose the production composer and menu/dialog controls together. They cover text switching, attachment-only, idle, stopping, paused, errors, cancellation preview/loading, and mobile/light presentations. The Storybook state transitions simulate requests; runner diff --git a/doc/observability.md b/doc/observability.md index 7b5bef23c8..491e8e35c8 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -418,13 +418,21 @@ Application and route error-boundary reports also include: URLs, arguments, and unrecognized lines are omitted. Parsing examines at most 16 KiB of input. Production builds preserve function names so this trace remains useful after minification; this adds some bundle size. + +All browser error reports, including global promise rejections, include: + +- `browser_build_mode`: `development` or `production`, from the loaded bundle. +- `browser_rejection_kind`: the primitive type of an unhandled rejected value + (or `null`). The diagnostic does not read object properties or copy the value. - `browser_state`: document readiness, visibility, and a boolean indicating the `translated-ltr` or `translated-rtl` root class used by browser translation. The marker is evidence of DOM translation, not proof of the error's cause; its absence does not exclude other translators or DOM-changing extensions. -These fields are captured at the failure, before asynchronous reporting, and -attached only to that event. They include no component props, DOM text, HTML, +Boundary state is captured at the failure, before asynchronous reporting. +Global reports without that snapshot read document state before sending. +All fields are attached only to that event. They include no component props, +DOM text, HTML, element identifiers, arbitrary CSS classes, route, or query string. Failed diagnostic reads do not prevent the original exception from being reported. The monitoring gate and sign-out behavior still apply. This context does not @@ -478,6 +486,17 @@ These fields contain build identifiers; they add no tenant or user identity. `errorCode`, and `agentAdapter`. The server redacts the error message and the error code before it sends the event. +Native runner identity and harness failures retain their existing error prefixes. +Their terminal messages now include a bounded guard reason, such as +`session_scope_mismatch`, `durable_identity_unreadable`, or +`backup_without_reusable_lease`. Provider-pack read failures distinguish a missing +file, invalid JSON, permission denial, invalid path type, and other I/O errors. +These reasons contain no session identifiers, provider output, or filesystem +paths. They help diagnose recurrence; they do not authorize a retry, quarantine, +replacement, or a weaker identity check. Existing chat recovery recognizes the +same failure category with or without a reason suffix; it still requires the +exact cleanup receipt, checkpoint, and absence of provider work. + **Server events the default integrations add** - `OnUncaughtException` — each uncaught exception on the main thread, at diff --git a/package.json b/package.json index f5566737bf..fc3c8b0bae 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "test:e2e:runner:models:update": "node cli/node_modules/tsx/dist/cli.mjs tests/runner-e2e/openrouter-models-update.ts", "test:e2e:runner:history:publish": "node cli/node_modules/tsx/dist/cli.mjs tests/runner-e2e/history-publish.ts", "test:runner-recovery": "vitest run server/src/services/native-runtime/native-replacement-evidence.test.ts server/src/services/native-runtime/stopped-codex-turn.test.ts server/src/services/native-runtime/native-safe-replacement.test.ts", + "test:e2e:browser-context": "playwright test --config tests/e2e/playwright-company-context.config.ts", "test:e2e:runner:browser-support": "playwright test --config tests/runner-e2e/playwright-support.config.ts", "test:e2e:runner:unit": "vitest run --config tests/runner-e2e/vitest.config.ts", "test:e2e:runner:typecheck": "tsc -p tests/runner-e2e/tsconfig.json", diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 3293014bc7..a26f980fc9 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -1601,6 +1601,55 @@ describe("shared ACPX engine runtime behavior", () => { expect(path.resolve(path.dirname(managedAuth), await fs.readlink(managedAuth))).toBe(sourceAuth); }); + it.each(["OPENAI_API_KEY", "CODEX_API_KEY"] as const)( + "uses isolated API-key auth instead of the host ChatGPT login for %s", + async (keyName) => { + const root = await makeTempRoot(); + const sourceCodexHome = path.join(root, "source-codex-home"); + const paperclipHome = path.join(root, "paperclip-home"); + await fs.mkdir(sourceCodexHome, { recursive: true }); + const sourceAuth = path.join(sourceCodexHome, "auth.json"); + await fs.writeFile(sourceAuth, JSON.stringify({ tokens: "host-login" }), "utf8"); + const managedHome = path.join( + paperclipHome, "instances", "test-instance", "companies", "company-1", + "acp-engine", "agents", "agent-1", "codex-home", + ); + await fs.mkdir(managedHome, { recursive: true }); + const managedAuth = path.join(managedHome, "auth.json"); + if (process.platform === "win32") { + await fs.writeFile(managedAuth, JSON.stringify({ tokens: "stale-login" }), "utf8"); + } else { + await fs.symlink(sourceAuth, managedAuth); + } + + vi.stubEnv("CODEX_HOME", sourceCodexHome); + vi.stubEnv("PAPERCLIP_HOME", paperclipHome); + vi.stubEnv("PAPERCLIP_INSTANCE_ID", "test-instance"); + vi.stubEnv("OPENAI_API_KEY", ""); + vi.stubEnv("CODEX_API_KEY", ""); + try { + const { sessionInputs } = await runExecutor({ + agent: "codex", + stateDir: path.join(root, "state"), + env: { [keyName]: "sk-acp-test-key" }, + paperclipRuntimeSkills: [], + paperclipSkillSync: { desiredSkills: [] }, + }); + const sessionEnv = (sessionInputs[0]!.sessionOptions as { env: Record }).env; + expect(sessionEnv.CODEX_HOME).toBe(managedHome); + expect(sessionEnv.DEFAULT_AUTH_REQUEST).toBe(JSON.stringify({ methodId: "api-key" })); + expect((await fs.lstat(managedAuth)).isSymbolicLink()).toBe(false); + expect(JSON.parse(await fs.readFile(managedAuth, "utf8"))).toEqual({ OPENAI_API_KEY: "sk-acp-test-key" }); + expect(await fs.readFile(sourceAuth, "utf8")).toBe(JSON.stringify({ tokens: "host-login" })); + if (process.platform !== "win32") { + expect((await fs.stat(managedAuth)).mode & 0o777).toBe(0o600); + } + } finally { + vi.unstubAllEnvs(); + } + }, + ); + it("sets GROK_HOME for a Grok run from the company Grok home, and leaves CODEX_HOME unchanged for a Codex run", async () => { const root = await makeTempRoot(); const paperclipHome = path.join(root, "paperclip-home"); diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 9640ee414f..2c4b029e12 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -808,6 +808,10 @@ function resolveManagedCodexHomeDir(companyId: string): string { return path.join(defaultPaperclipInstanceDir(), "companies", companyId, "codex-home"); } +function resolveManagedCodexApiKeyHomeDir(companyId: string, agentId: string): string { + return path.join(defaultStateDir(companyId, agentId), "codex-home"); +} + // Mirrors `resolveManagedGrokHomeDir` in // `packages/adapters/grok-local/src/server/grok-home.ts` — this package // cannot import that adapter package (it would invert the dependency @@ -1002,15 +1006,30 @@ async function prepareManagedCodexHome(input: { companyId: string; sourceHome: string; targetHome: string; + apiKey?: string; onLog: AdapterExecutionContext["onLog"]; }): Promise { - const { sourceHome, targetHome, onLog } = input; + const { sourceHome, targetHome, apiKey, onLog } = input; if (path.resolve(sourceHome) === path.resolve(targetHome)) return targetHome; await fs.mkdir(targetHome, { recursive: true }); - const authJson = path.join(sourceHome, "auth.json"); - if (await pathExists(authJson)) await ensureSymlink(path.join(targetHome, "auth.json"), authJson); + const targetAuth = path.join(targetHome, "auth.json"); + if (apiKey) { + // Codex reads auth.json ahead of the process environment. Never leave a + // shared ChatGPT-login symlink in an API-key agent's managed home, and never + // write through that symlink into the operator's own Codex credentials. + // Atomic replacement also keeps concurrent turns from seeing a missing or + // partially written credential file. + await writeFileAtomically({ + target: targetAuth, + contents: JSON.stringify({ OPENAI_API_KEY: apiKey }), + mode: 0o600, + }); + } else { + const sourceAuth = path.join(sourceHome, "auth.json"); + if (await pathExists(sourceAuth)) await ensureSymlink(targetAuth, sourceAuth); + } for (const name of ["config.json", "config.toml", "instructions.md"]) { const source = path.join(sourceHome, name); @@ -1255,6 +1274,7 @@ async function reconcileManagedCodexSkills(input: { async function prepareCodexSkillRuntime(input: { companyId: string; + agentId: string; config: Record; env: Record; moduleDir: string; @@ -1282,12 +1302,19 @@ async function prepareCodexSkillRuntime(input: { typeof process.env.CODEX_HOME === "string" && process.env.CODEX_HOME.trim().length > 0 ? path.resolve(process.env.CODEX_HOME.trim()) : path.join(os.homedir(), ".codex"); - const managedCodexHome = resolveManagedCodexHomeDir(input.companyId); + const apiKey = input.env.OPENAI_API_KEY?.trim() || input.env.CODEX_API_KEY?.trim() + || process.env.OPENAI_API_KEY?.trim() || process.env.CODEX_API_KEY?.trim(); + // Keep API-key auth separate from the company home used by subscription + // agents, so one agent cannot switch another agent's login on its next turn. + const managedCodexHome = apiKey + ? resolveManagedCodexApiKeyHomeDir(input.companyId, input.agentId) + : resolveManagedCodexHomeDir(input.companyId); const effectiveCodexHome = configuredCodexHome ?? await prepareManagedCodexHome({ companyId: input.companyId, sourceHome: sourceCodexHome, targetHome: managedCodexHome, + apiKey, onLog: input.onLog, }); const { allSkills, selectedSkills, desiredSkillNames } = await resolveSelectedRuntimeSkills(input.config, input.moduleDir); @@ -2046,6 +2073,7 @@ async function buildRuntime(input: { const preparedSkills = await measureStartupStep(input.ctx, nowMs, "codex-home.seed", () => prepareCodexSkillRuntime({ companyId: agent.companyId, + agentId: agent.id, config, env, moduleDir: input.engine.moduleDir, diff --git a/packages/adapter-utils/src/execution-target-sandbox.test.ts b/packages/adapter-utils/src/execution-target-sandbox.test.ts index 36695e85c1..bf600fdd74 100644 --- a/packages/adapter-utils/src/execution-target-sandbox.test.ts +++ b/packages/adapter-utils/src/execution-target-sandbox.test.ts @@ -609,7 +609,7 @@ describe("sandbox adapter execution targets", () => { target: { kind: "remote", transport: "sandbox", providerKey: "local-test", remoteCwd: rootDir, runner: { execute: async (input) => { - if (input.args?.[1]?.includes("command.b64.paperclip-upload.b64") && input.args[1].includes(">>")) { + if (/command\.b64\.[^/]+\.paperclip-upload\.b64/.test(input.args?.[1] ?? "") && input.args![1].includes(">>")) { throw new Error("Upload interrupted"); } return delegate.execute(input); diff --git a/packages/adapter-utils/src/execution-target-stdin-race.test.ts b/packages/adapter-utils/src/execution-target-stdin-race.test.ts index 9341358c38..a87deb0e89 100644 --- a/packages/adapter-utils/src/execution-target-stdin-race.test.ts +++ b/packages/adapter-utils/src/execution-target-stdin-race.test.ts @@ -122,10 +122,10 @@ describe("stdin file race (parent PAP-4037)", () => { return new Promise((resolve) => setTimeout(resolve, ms)); } - async function waitFor(check: () => boolean, timeoutMs = 4_000): Promise { + async function waitFor(check: () => boolean | Promise, timeoutMs = 4_000): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { - if (check()) return; + if (await check()) return; await delay(20); } throw new Error("Timed out waiting for condition."); @@ -492,8 +492,247 @@ describe("stdin file race (parent PAP-4037)", () => { } }); + it.each([ + ...["prepare", "append", "finalize", "late-finalize"].map((stage) => + [stage, "Request failed with status code 502"] as const), + ...[502, 503, 504].map((status) => + ["finalize", `Cloudflare sandbox bridge request failed with HTTP ${status}.`] as const), + ])( + "recovers a transient %s failure (%s) without repeating or reordering stdin", + async (stage, failure) => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-stdin-retry-")); + cleanupDirs.push(rootDir); + const childPath = path.join(rootDir, "echo-child.mjs"); + await writeFile(childPath, "process.stdin.on('data', (c) => process.stdout.write(c));\n", "utf8"); + const first = "first-" + "x".repeat(70_000); + let delivered = ""; + let injected = false; + let lateFinalize: (() => Promise) | undefined; + const local = createLocalSandboxRunner(); + const runner = { + execute: async (input: Parameters[0]) => { + const script = input.args?.[1] ?? ""; + const matches = script.includes("/stdin/000000000001.json") && ( + stage === "prepare" ? script.includes("mkdir -p") : + stage === "append" ? script.startsWith("printf") : script.startsWith("base64 -d") + ); + if (matches && !injected) { + injected = true; + if (stage === "late-finalize") lateFinalize = () => local.execute(input); + else if (stage !== "prepare") await local.execute(input); + // The provider can lose the response after the receiver consumed + // the file. A retry must not repeat those bytes on the ACP stream. + if (stage === "finalize") await waitFor(() => delivered === first, 8_000); + throw new Error(failure); + } + return local.execute(input); + }, + }; + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "run-stdin-retry", + target: { kind: "remote", transport: "sandbox", remoteCwd: rootDir, runner }, + runtimeRootDir: path.join(rootDir, "runtime"), + adapterKey: "acpx", command: process.execPath, args: [childPath], cwd: rootDir, env: {}, + }); + let peer: net.Socket | undefined; + try { + const source = await readFile(bridge!.agentCommand, "utf8"); + const port = Number(/port: (\d+)/.exec(source)![1]); + const token = JSON.parse(/const token = (".*?");/.exec(source)![1]) as string; + peer = net.createConnection({ host: "127.0.0.1", port }); + peer.on("error", () => {}); + peer.setEncoding("utf8"); + let buffer = ""; + peer.on("data", (chunk) => { + buffer += chunk; + const lines = buffer.split("\n"); + buffer = lines.pop()!; + for (const line of lines) { + const frame = JSON.parse(line) as DeliveredFrame; + delivered += collectDelivered([frame]); + } + }); + await new Promise((resolve) => peer!.once("connect", resolve)); + for (const text of [first, "-second"]) + peer.write(JSON.stringify({ token, type: "stdin", data: Buffer.from(text).toString("base64") }) + "\n"); + await waitFor(() => delivered.endsWith("-second"), 10_000); + expect(injected).toBe(true); + expect(delivered).toBe(first + "-second"); + if (lateFinalize) { + // A provider can return 502 while its original finalize still runs. + // Cleanup may invalidate its private upload, but it cannot touch + // the retry's data or repeat input after newer messages arrived. + await lateFinalize(); + peer.write(JSON.stringify({ token, type: "stdin", data: Buffer.from("-third").toString("base64") }) + "\n"); + await waitFor(() => delivered.endsWith("-third"), 8_000); + expect(delivered).toBe(first + "-second-third"); + } + await waitFor(async () => { + const files = await readdir(path.join(rootDir, "runtime", "process-sessions"), { recursive: true }); + return files.every((file) => !file.endsWith(".paperclip-upload.b64") && !file.endsWith(".paperclip-upload.decoded")); + }); + } finally { + peer?.destroy(); + await bridge?.stop(); + } + }, + 20_000, + ); + + it.each([ + ["Request failed with status code 502", 3], + ["Request failed with status code 503", 3], + ["Request failed with status code 504", 3], + ["Cloudflare sandbox bridge request failed with HTTP 502.", 3], + ["Cloudflare sandbox bridge request failed with HTTP 503.", 3], + ["Cloudflare sandbox bridge request failed with HTTP 504.", 3], + ["Request failed with status code 403", 1], + ["Cloudflare sandbox bridge request failed with HTTP 403.", 1], + ["Remote command failed: Request failed with status code 502", 1], + ["Cloudflare sandbox bridge request failed with HTTP 502. sensitive-input", 1], + ["Remote command failed: sensitive-input", 1], + ] as const)("bounds input failure %s to %i attempts and stops later writes", async (failure, expectedAttempts) => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-stdin-failed-")); + cleanupDirs.push(rootDir); + let attempts = 0; + let laterWrite = false; + let stderr = ""; + const runner = createLocalSandboxRunner(async (script) => { + if (!script.startsWith("mkdir -p")) return; + if (script.includes("/stdin/000000000002.json")) laterWrite = true; + if (script.includes("/stdin/000000000001.json")) { + attempts += 1; + throw new Error(failure); + } + }); + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "run-stdin-failed", + target: { kind: "remote", transport: "sandbox", remoteCwd: rootDir, runner }, + runtimeRootDir: path.join(rootDir, "runtime"), + adapterKey: "acpx", command: "cat", args: [], cwd: rootDir, env: {}, + onLog: async (stream, chunk) => { if (stream === "stderr") stderr += chunk; }, + }); + let peer: net.Socket | undefined; + try { + const source = await readFile(bridge!.agentCommand, "utf8"); + const port = Number(/port: (\d+)/.exec(source)![1]); + const token = JSON.parse(/const token = (".*?");/.exec(source)![1]) as string; + peer = net.createConnection({ host: "127.0.0.1", port }); + peer.setEncoding("utf8"); + peer.on("error", () => {}); + let output = ""; + peer.on("data", (chunk) => { output += chunk; }); + const closed = new Promise((resolve) => peer!.on("close", () => resolve())); + await new Promise((resolve) => peer!.once("connect", resolve)); + for (const text of ["first", "second"]) + peer.write(JSON.stringify({ token, type: "stdin", data: Buffer.from(text).toString("base64") }) + "\n"); + await closed; + expect(attempts).toBe(expectedAttempts); + expect(laterWrite).toBe(false); + expect(JSON.parse(output)).toEqual({ type: "error", message: "ACP process session input delivery failed." }); + expect(stderr).toContain("ACP process session input delivery failed."); + expect(stderr).not.toContain(failure); + } finally { + peer?.destroy(); + await bridge?.stop(); + } + }, 15_000); + + it("stops after exhausted input retries even when failure logging stalls", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-stdin-log-stall-")); + cleanupDirs.push(rootDir); + let attempts = 0; + let loggingStarted = false; + let releaseLog!: () => void; + const stalledLog = new Promise((resolve) => { releaseLog = resolve; }); + const runner = createLocalSandboxRunner(async (script) => { + if (script.startsWith("mkdir -p") && script.includes("/stdin/000000000001.json")) { + attempts += 1; + throw new Error("Request failed with status code 502"); + } + }); + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "run-stdin-log-stall", + target: { kind: "remote", transport: "sandbox", remoteCwd: rootDir, runner }, + runtimeRootDir: path.join(rootDir, "runtime"), + adapterKey: "acpx", command: "cat", args: [], cwd: rootDir, env: {}, + onLog: async (stream) => { + if (stream === "stderr") { + loggingStarted = true; + await stalledLog; + } + }, + }); + let peer: net.Socket | undefined; + let stop: Promise | undefined; + try { + const source = await readFile(bridge!.agentCommand, "utf8"); + const port = Number(/port: (\d+)/.exec(source)![1]); + const token = JSON.parse(/const token = (".*?");/.exec(source)![1]) as string; + peer = net.createConnection({ host: "127.0.0.1", port }); + peer.setEncoding("utf8"); + peer.on("error", () => {}); + let output = ""; + peer.on("data", (chunk) => { output += chunk; }); + const closed = new Promise((resolve) => peer!.once("close", resolve)); + await new Promise((resolve) => peer!.once("connect", resolve)); + peer.write(JSON.stringify({ token, type: "stdin", data: Buffer.from("input").toString("base64") }) + "\n"); + await closed; + expect(attempts).toBe(3); + expect(loggingStarted).toBe(true); + expect(JSON.parse(output)).toEqual({ type: "error", message: "ACP process session input delivery failed." }); + let stopped = false; + stop = bridge!.stop().then(() => { stopped = true; }); + // Teardown has a three-second acknowledgement budget. It must finish + // while the run-log promise remains unresolved, including local cleanup. + await waitFor(() => stopped, 6_000); + await expect(lstat(bridge!.agentCommand)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + releaseLog(); + peer?.destroy(); + await (stop ?? bridge?.stop()); + } + }, 15_000); + // ---- Host atomic-write tests ------------------------------------------ + it.each(["fails", "stalls"])("preserves the upload failure when best-effort cleanup %s", async (cleanupMode) => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-upload-cleanup-")); + cleanupDirs.push(rootDir); + const local = createLocalSandboxRunner(); + const uploadFailure = new Error("Request failed with status code 502"); + let cleanupAttempted = false; + let rejectCleanup!: (error: Error) => void; + const stalledCleanup = new Promise((_resolve, reject) => { rejectCleanup = reject; }); + // Observe the test-owned promise even in the immediate-failure case. + void stalledCleanup.catch(() => {}); + const client = createCommandManagedSandboxCallbackBridgeQueueClient({ + remoteCwd: rootDir, + runner: { + execute: async (input) => { + const script = input.args?.[1] ?? ""; + if (script.startsWith("rm -f")) { + cleanupAttempted = true; + if (cleanupMode === "stalls") return stalledCleanup; + throw new Error("Request failed with status code 403"); + } + const result = await local.execute(input); + if (script.startsWith("printf")) throw uploadFailure; + return result; + }, + }, + }); + try { + await expect(Promise.race([ + client.writeTextFile(path.join(rootDir, "message.json"), "test input"), + delay(1_000).then(() => { throw new Error("Upload waited for stalled cleanup"); }), + ])).rejects.toBe(uploadFailure); + expect(cleanupAttempted).toBe(true); + } finally { + rejectCleanup(new Error("Cleanup unavailable")); + } + }); + // A runner that executes each bridge shell script on the local filesystem, // so the test exercises the real command-managed `writeTextFile` script. function createLocalShellRunner(scripts: string[]) { @@ -572,7 +811,7 @@ describe("stdin file race (parent PAP-4037)", () => { expect(finalizeScript).toBeDefined(); expect(finalizeScript).toContain(`mv `); expect(finalizeScript).not.toContain(`> '${jsonPath}'`); - expect(finalizeScript).toContain(`> '${jsonPath}.paperclip-upload.decoded'`); + expect(finalizeScript).toMatch(/> '[^']+\.paperclip-upload\.decoded'/); }); it("never exposes a partial .json file under a concurrent reader (command-managed host write)", async () => { diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 54b055bac6..ca816ca25c 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -1985,6 +1985,11 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { const target = input.target; const onLog = input.onLog ?? (async () => {}); + // Failure diagnostics are best effort: stalled or failed run-log persistence + // must not prevent sending shutdown or removing the bridge's session files. + const logFailureWithoutWaiting = (message: string) => { + void Promise.resolve().then(() => onLog("stderr", message)).catch(() => undefined); + }; const runner = requireSandboxRunner(target); // Run one unit of run-time work under its named wrapper span when a span // runner is injected. Without a runner, run the work under the current run @@ -2135,6 +2140,28 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // a big earlier chunk, so the wrapper reads the stdin bytes out of order and // corrupts a large prompt on the stdin path. let stdinWriteChain: Promise = Promise.resolve(); + let stdinDeliveryFailed = false; + const writeStdinFile = async (filePath: string, body: string) => { + // Retry the same sequence, never the ACP request or the tool itself. Each + // upload uses private temporary paths, and the wrapper drops sequences it + // already consumed when a provider loses the final rename's response. + for (let attempt = 1; ; attempt += 1) { + try { + await client.writeTextFile(filePath, body); + return; + } catch (error) { + // Plugin RPC preserves provider messages but not HTTP error classes. + // Match the Daytona SDK and Cloudflare bridge's gateway diagnostics + // exactly; shell failures and auth errors must still fail immediately. + const gatewayFailure = error instanceof Error && ( + /^Request failed with status code (502|503|504)$/.test(error.message) || + /^Cloudflare sandbox bridge request failed with HTTP (502|503|504)\.$/.test(error.message) + ); + if (!gatewayFailure || attempt >= 3) throw error; + await new Promise((resolve) => setTimeout(resolve, attempt * 250)); + } + } + }; let pollTimer: NodeJS.Timeout | null = null; const pendingRemoteEvents: Array<{ type?: string; @@ -2266,19 +2293,24 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // Chain this write after the previous one, so the atomic rename for // file N finishes before the write for file N+1 starts. Keep the // per-message `sandbox.agentSession.sendInput` span inside the chain. - const write = stdinWriteChain.then(() => - runRuntimeWork(AGENT_SESSION_SEND_INPUT_SPAN, () => - client.writeTextFile(filePath, jsonLine(stdinPayload)), - ), - ); - // The next message chains after this write on success or failure, so a - // failed write never blocks the chain. This mirrors the wrapper - // `writeChain` pattern for its event files. - stdinWriteChain = write.then(() => undefined, () => undefined); - // Keep the failure behavior: send one error line, then destroy the socket. - write.catch((error) => { - nextSocket.write(jsonLine({ type: "error", message: error instanceof Error ? error.message : String(error) })); - nextSocket.destroy(); + stdinWriteChain = stdinWriteChain.then(async () => { + if (stdinDeliveryFailed) return; + try { + await runRuntimeWork(AGENT_SESSION_SEND_INPUT_SPAN, () => + writeStdinFile(filePath, jsonLine(stdinPayload)), + ); + } catch { + stdinDeliveryFailed = true; + stopping = true; + const message = "ACP process session input delivery failed."; + // Flush the diagnostic before closing; destroy() can discard it + // and leave only ACP's generic connection_close error. Do not + // expose provider error text, which may contain a command payload. + nextSocket.end(jsonLine({ type: "error", message })); + // stop() awaits this input chain before sending shutdown. Run-log + // persistence must not hold teardown open when it stalls or fails. + logFailureWithoutWaiting(`[paperclip] ${message}\n`); + } }); } } @@ -2549,10 +2581,9 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { ]); stopReadingForShutdownAck = true; if (!acknowledgedInTime) { - await onLog( - "stderr", + logFailureWithoutWaiting( `[paperclip] ACP process session wrapper did not acknowledge shutdown within ${DEFAULT_PROCESS_SESSION_SHUTDOWN_WAIT_MS}ms; removing the session directory anyway.\n`, - ).catch(() => undefined); + ); } // Unconditional: this removal runs whether or not the wrapper // acknowledged, and whether or not any event (real or forged) arrived @@ -2983,6 +3014,14 @@ async function pollStdin() { for (const name of entries) { if (shuttingDown) break; const entrySeq = Number.parseInt(name, 10); + const file = path.posix.join(stdinDir, name); + // A successful publication can be retried after its provider response + // was lost, even after we consumed it. Never send those bytes twice or + // move the expected sequence backwards. This also handles late uploads. + if (Number.isFinite(entrySeq) && entrySeq < stdinExpectedSeq) { + await fs.rm(file, { force: true }).catch(() => undefined); + continue; + } // Hold the send order when an earlier file has not appeared. Do not consume // this later file: wait for the missing file on a later cycle, bounded by // the retry budget. After the budget, fail loud and advance past the gap, @@ -3001,7 +3040,6 @@ async function pollStdin() { stdinGapRetries = 0; stdinExpectedSeq = entrySeq; } - const file = path.posix.join(stdinDir, name); let message; try { // Hardening (I3): open with O_NOFOLLOW where the platform defines it, diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.ts b/packages/adapter-utils/src/sandbox-callback-bridge.ts index d9145c9c25..40776a3da5 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.ts @@ -701,23 +701,40 @@ export function createCommandManagedSandboxCallbackBridgeQueueClient(input: { // then moves the complete decoded content onto the final `.json` path. // A direct `> remotePath` redirect truncates the final path before the // decode writes it, so a reader can see an empty or partial file. - const tempPath = `${remotePath}.paperclip-upload.b64`; - const decodedPath = `${remotePath}.paperclip-upload.decoded`; - await runChecked( - `prepare upload ${remotePath}`, - `mkdir -p ${shellQuote(remoteDir)} && rm -f ${shellQuote(tempPath)} ${shellQuote(decodedPath)} && : > ${shellQuote(tempPath)}`, - ); - const base64Body = toBuffer(Buffer.from(body, "utf8")).toString("base64"); - for (const chunk of base64Chunks(base64Body)) { + // A failed provider response does not prove the remote command stopped. + // Keep concurrent or retried uploads from truncating each other's bytes. + const uploadPath = `${remotePath}.${randomUUID()}.paperclip-upload`; + const tempPath = `${uploadPath}.b64`; + const decodedPath = `${uploadPath}.decoded`; + try { await runChecked( - `append upload chunk ${remotePath}`, - `printf '%s' ${shellQuote(chunk)} >> ${shellQuote(tempPath)}`, + `prepare upload ${remotePath}`, + `mkdir -p ${shellQuote(remoteDir)} && rm -f ${shellQuote(tempPath)} ${shellQuote(decodedPath)} && : > ${shellQuote(tempPath)}`, ); + const base64Body = toBuffer(Buffer.from(body, "utf8")).toString("base64"); + for (const chunk of base64Chunks(base64Body)) { + await runChecked( + `append upload chunk ${remotePath}`, + `printf '%s' ${shellQuote(chunk)} >> ${shellQuote(tempPath)}`, + ); + } + await runChecked( + `finalize upload ${remotePath}`, + `base64 -d < ${shellQuote(tempPath)} > ${shellQuote(decodedPath)} && mv ${shellQuote(decodedPath)} ${shellQuote(remotePath)} && rm -f ${shellQuote(tempPath)}`, + ); + } catch (error) { + // Abandon only this attempt's intermediates, never the published file + // or another attempt. A late finalize may fail or finish publishing; + // either is safe for a sequence-aware caller. Preserve the original + // failure even when the provider is still unavailable for cleanup. + // Cleanup must not put another provider timeout on the retry/shutdown + // path. Its unique paths stay safe to remove after this call returns. + void runChecked( + `clean failed upload ${remotePath}`, + `rm -f ${shellQuote(tempPath)} ${shellQuote(decodedPath)}`, + ).catch(() => undefined); + throw error; } - await runChecked( - `finalize upload ${remotePath}`, - `base64 -d < ${shellQuote(tempPath)} > ${shellQuote(decodedPath)} && mv ${shellQuote(decodedPath)} ${shellQuote(remotePath)} && rm -f ${shellQuote(tempPath)}`, - ); }, writeResponseFile: async (responsePath, body, options = {}) => { const responseDir = path.posix.dirname(responsePath); diff --git a/packages/adapters/codex-local/src/index.ts b/packages/adapters/codex-local/src/index.ts index 3cb69dc4ce..5d3a95c888 100644 --- a/packages/adapters/codex-local/src/index.ts +++ b/packages/adapters/codex-local/src/index.ts @@ -114,6 +114,7 @@ export const models = [ { id: "gpt-6-luna", label: "gpt-6-luna" }, { id: "gpt-5.6-terra", label: "gpt-5.6-terra" }, { id: "gpt-5.6-luna", label: "gpt-5.6-luna" }, + { id: "gpt-5.5", label: "gpt-5.5" }, { id: "gpt-5.4", label: "gpt-5.4" }, { id: "gpt-5.4-mini", label: "gpt-5.4-mini" }, { id: "gpt-5", label: "gpt-5" }, diff --git a/packages/adapters/codex-local/src/server/acp.test.ts b/packages/adapters/codex-local/src/server/acp.test.ts index 5935b238b3..b31dafcb9f 100644 --- a/packages/adapters/codex-local/src/server/acp.test.ts +++ b/packages/adapters/codex-local/src/server/acp.test.ts @@ -1042,6 +1042,54 @@ describe("codex_local ACP lane", () => { expect(mode).toBe(0o600); }); + it("does not copy an API-key run's sandbox auth into the shared subscription home", async () => { + const root = await makeTempRoot("paperclip-codex-acp-key-copyback-"); + const localCwd = path.join(root, "worktree"); + const remoteCwd = path.join(root, "remote-workspace"); + const keyHome = path.join(root, "api-key-home"); + const sharedHostHome = path.join(root, "shared-codex-home"); + await Promise.all([localCwd, remoteCwd, keyHome, sharedHostHome].map((dir) => fs.mkdir(dir, { recursive: true }))); + const sharedAuth = subscriptionAuthJson("acct-same", OLDER_REFRESH, "host-older"); + await fs.writeFile(path.join(sharedHostHome, "auth.json"), sharedAuth, { mode: 0o600 }); + // A subscription-shaped sandbox credential must never be considered for + // the shared home when this run explicitly authenticates with an API key. + await fs.writeFile( + path.join(keyHome, "auth.json"), + subscriptionAuthJson("acct-same", NEWER_REFRESH, "sandbox-newer"), + { mode: 0o600 }, + ); + process.env.CODEX_HOME = sharedHostHome; + + const execute = createCodexAcpExecutor({ + createRuntime: (options: FakeRuntimeOptions) => new FakeRuntime(options) as never, + }); + const result = await execute(buildContext(localCwd, { + config: { + engine: "acp", + cwd: localCwd, + agentCommand: "node ./fake-acp.js", + stateDir: path.join(root, "state"), + env: { CODEX_HOME: keyHome, OPENAI_API_KEY: "sk-test-key" }, + promptTemplate: "Do the assigned work.", + }, + context: { + issueId: "issue-1", + paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" }, + }, + executionTarget: { + kind: "remote", + transport: "sandbox", + providerKey: "fake-plugin", + remoteCwd, + runner: createLocalSandboxRunner(), + } as never, + authToken: "real-run-jwt", + })); + + expect(result.exitCode).toBe(0); + expect(await fs.readFile(path.join(sharedHostHome, "auth.json"), "utf8")).toBe(sharedAuth); + }); + it("keeps the shared host Codex auth when the sandbox copy is not strictly newer", async () => { const root = await makeTempRoot("paperclip-codex-acp-copyback-older-"); const localCwd = path.join(root, "worktree"); diff --git a/packages/adapters/codex-local/src/server/acp.ts b/packages/adapters/codex-local/src/server/acp.ts index e9b19def48..c7715dc75f 100644 --- a/packages/adapters/codex-local/src/server/acp.ts +++ b/packages/adapters/codex-local/src/server/acp.ts @@ -166,17 +166,16 @@ export function buildCodexAcpConfig(config: Record): Record + restore: apiKeyAuth ? undefined : async ({ assetDir, readFile }) => void (await copyBackCodexAuth({ readSandboxAuth: () => readFile(path.posix.join(assetDir, "auth.json")), hostAuthPath: path.join(input.config.managedAiConnection ? effectiveCodexHome : resolveSharedCodexHomeDir(process.env), "auth.json"), @@ -224,7 +228,7 @@ async function prepareCodexRemoteManagedHome( return { stagedRuntime, - // Per-run copy-back: fires on EVERY run's teardown (including a compatible + // Subscription copy-back fires on EVERY run's teardown (including a compatible // resume that reuses this staged runtime). It reads the sandbox auth.json / // workspace live and copies back to the host; it does NOT remove the staged // in-sandbox home, so re-running it across resumes can't leave a later run @@ -238,7 +242,9 @@ async function prepareCodexRemoteManagedHome( teardown: createWorkspaceRestoreTeardown({ stagedRuntime, onLog, - startMessage: "[paperclip] Restoring workspace changes and Codex auth from the sandbox.\n", + startMessage: apiKeyAuth + ? "[paperclip] Restoring workspace changes from the sandbox.\n" + : "[paperclip] Restoring workspace changes and Codex auth from the sandbox.\n", failurePrefix: "[paperclip] Codex ACP teardown restore/copy-back failed", }), // One-time cleanup of the HOST staged home temp dir. Fired ONLY when the diff --git a/scripts/storybook-agent-avatar-assets.mjs b/scripts/storybook-agent-avatar-assets.mjs index 736bcb2864..3487904493 100644 --- a/scripts/storybook-agent-avatar-assets.mjs +++ b/scripts/storybook-agent-avatar-assets.mjs @@ -1,11 +1,54 @@ import { createRequire } from "node:module"; import { createHash } from "node:crypto"; -/** Build-only: reuse the API worker and finite preset contract, never a browser renderer. */ +/** Render avatar presets with the API worker in dev and package them in builds. */ export function storybookAgentAvatarAssets() { return { name: "storybook-agent-avatar-assets", - apply: "build", + configureServer(server) { + let rendererPromise; + const images = new Map(); + const renderer = () => rendererPromise ??= (async () => { + const serverRequire = createRequire(new URL("../server/package.json", import.meta.url)); + const { tsImport } = await import(serverRequire.resolve("tsx/esm/api")); + const { createAgentAvatarPool } = await tsImport("../server/src/services/agent-avatar-pool.ts", import.meta.url); + const { AGENT_PALETTE_IDS, AGENT_AVATAR_SIZES, CHARACTER_STATES, appearanceForPalette } = + await tsImport("../packages/shared/src/agent-appearance.ts", import.meta.url); + return { pool: createAgentAvatarPool(2), AGENT_PALETTE_IDS, AGENT_AVATAR_SIZES, CHARACTER_STATES, appearanceForPalette }; + })(); + server.middlewares.use((req, res, next) => { + const pathname = new URL(req.url ?? "/", "http://storybook.local").pathname; + const match = /^\/agent-avatar-images\/cap-v1\/([^/]+)\/([^/]+)-(\d+)-([12])\.png$/.exec(pathname); + if (!match) return next(); + void (async () => { + const [, palette, pose, sizeText, scaleText] = match; + const { pool, AGENT_PALETTE_IDS, AGENT_AVATAR_SIZES, CHARACTER_STATES, appearanceForPalette } = await renderer(); + const size = Number(sizeText); + const scale = Number(scaleText); + const muted = palette === "muted-dream"; + if (!(muted || AGENT_PALETTE_IDS.includes(palette)) || !CHARACTER_STATES.includes(pose) || !AGENT_AVATAR_SIZES.includes(size)) { + res.statusCode = 404; + res.end(); + return; + } + if (!images.has(pathname)) { + const appearance = appearanceForPalette(muted ? AGENT_PALETTE_IDS[0] : palette); + images.set(pathname, pool.render({ appearance, size, scale, pose, muted }).catch((error) => { + images.delete(pathname); + throw error; + })); + } + const png = await images.get(pathname); + res.setHeader("Content-Type", "image/png"); + res.setHeader("Cache-Control", "public, max-age=3600"); + res.end(png); + })().catch(() => { + res.statusCode = 500; + res.end("Avatar rendering failed"); + }); + }); + server.httpServer?.once("close", () => { void rendererPromise?.then(({ pool }) => pool.close()); }); + }, async generateBundle() { const serverRequire = createRequire(new URL("../server/package.json", import.meta.url)); const { tsImport } = await import(serverRequire.resolve("tsx/esm/api")); diff --git a/server/src/__tests__/adapter-models.test.ts b/server/src/__tests__/adapter-models.test.ts index e35ba1b447..c04f07fb00 100644 --- a/server/src/__tests__/adapter-models.test.ts +++ b/server/src/__tests__/adapter-models.test.ts @@ -5,7 +5,7 @@ import { models as codexFallbackModels } from "@paperclipai/adapter-codex-local" import { models as cursorFallbackModels } from "@paperclipai/adapter-cursor-local"; import { models as opencodeFallbackModels } from "@paperclipai/adapter-opencode-local"; import { resetOpenCodeModelsCacheForTests } from "@paperclipai/adapter-opencode-local/server"; -import { listAdapterModels, listServerAdapters, refreshAdapterModels } from "../adapters/index.js"; +import { listAdapterModels, listServerAdapters, refreshAdapterModels, registerServerAdapter, unregisterServerAdapter } from "../adapters/index.js"; import { resetCodexModelsCacheForTests } from "../adapters/codex-models.js"; import { resetCursorModelsCacheForTests, setCursorModelsRunnerForTests } from "../adapters/cursor-models.js"; @@ -195,14 +195,14 @@ describe("adapter model listing", () => { ])); }); - it("loads codex models dynamically and merges fallback options", async () => { + it("keeps general OpenAI API models out of the Codex catalog", async () => { process.env.OPENAI_API_KEY = "sk-test"; const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue({ ok: true, json: async () => ({ data: [ - { id: "gpt-5-pro" }, - { id: "gpt-5" }, + { id: "gpt-image-1" }, + { id: "text-embedding-3-large" }, ], }), } as Response); @@ -210,40 +210,26 @@ describe("adapter model listing", () => { const first = await listAdapterModels("codex_local"); const second = await listAdapterModels("codex_local"); - expect(fetchSpy).toHaveBeenCalledTimes(1); - expect(first).toEqual(second); - expect(first.some((model) => model.id === "gpt-5-pro")).toBe(true); - expect(first.some((model) => model.id === "codex-mini-latest")).toBe(true); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(first).toEqual(codexFallbackModels); + expect(second).toEqual(codexFallbackModels); }); - it("refreshes cached codex models on demand", async () => { + it("keeps the curated Codex list when refreshing with an OpenAI key", async () => { process.env.OPENAI_API_KEY = "sk-test"; - const fetchSpy = vi.spyOn(globalThis, "fetch") - .mockResolvedValueOnce({ - ok: true, - json: async () => ({ - data: [{ id: "gpt-5" }], - }), - } as Response) - .mockResolvedValueOnce({ - ok: true, - json: async () => ({ - data: [{ id: "gpt-5.6-terra" }], - }), - } as Response); + const fetchSpy = vi.spyOn(globalThis, "fetch"); const initial = await listAdapterModels("codex_local"); const refreshed = await refreshAdapterModels("codex_local"); - expect(fetchSpy).toHaveBeenCalledTimes(2); - expect(initial.some((model) => model.id === "gpt-5")).toBe(true); - expect(refreshed.some((model) => model.id === "gpt-5.6-terra")).toBe(true); - expect(refreshed.some((model) => model.id === "gpt-5.6-luna")).toBe(true); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(initial).toEqual(codexFallbackModels); + expect(refreshed).toEqual(codexFallbackModels); }); - it("falls back to static codex models when OpenAI model discovery fails", async () => { + it("uses static Codex models without calling OpenAI model discovery", async () => { process.env.OPENAI_API_KEY = "sk-test"; - vi.spyOn(globalThis, "fetch").mockResolvedValue({ + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue({ ok: false, status: 401, json: async () => ({}), @@ -251,6 +237,33 @@ describe("adapter model listing", () => { const models = await listAdapterModels("codex_local"); expect(models).toEqual(codexFallbackModels); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("uses a custom Codex adapter's model and refresh hooks", async () => { + const builtin = listServerAdapters().find((adapter) => adapter.type === "codex_local")!; + const customModels = [{ id: "plugin-codex", label: "Plugin Codex" }]; + const listModels = vi.fn(async () => customModels); + const refreshModels = vi.fn(async () => customModels); + registerServerAdapter({ ...builtin, models: [], listModels, refreshModels }); + try { + await expect(listAdapterModels("codex_local")).resolves.toEqual(customModels); + await expect(refreshAdapterModels("codex_local")).resolves.toEqual(customModels); + expect(listModels).toHaveBeenCalledOnce(); + expect(refreshModels).toHaveBeenCalledOnce(); + + process.env.PAPERCLIP_ADAPTER_MODELS = JSON.stringify({ + codex_local: [{ id: "declared-codex", label: "Declared Codex" }], + }); + const declared = [{ id: "declared-codex", label: "Declared Codex" }]; + await expect(listAdapterModels("codex_local")).resolves.toEqual(declared); + await expect(refreshAdapterModels("codex_local")).resolves.toEqual(declared); + expect(listModels).toHaveBeenCalledOnce(); + expect(refreshModels).toHaveBeenCalledOnce(); + } finally { + delete process.env.PAPERCLIP_ADAPTER_MODELS; + unregisterServerAdapter("codex_local"); + } }); @@ -315,6 +328,16 @@ describe("adapter model listing", () => { ]); }); + it("uses declared Codex models for both listing and refresh", async () => { + process.env.PAPERCLIP_ADAPTER_MODELS = JSON.stringify({ + codex_local: [{ id: "private-codex", label: "Private Codex" }], + }); + const declared = [{ id: "private-codex", label: "Private Codex" }]; + + await expect(listAdapterModels("codex_local")).resolves.toEqual(declared); + await expect(refreshAdapterModels("codex_local")).resolves.toEqual(declared); + }); + it("observes env changes between calls (memo keyed by raw env value)", async () => { process.env.PAPERCLIP_ADAPTER_MODELS = JSON.stringify({ opencode_local: [{ id: "model-a" }], diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 85d968cbc1..39d9e150fe 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -56157,9 +56157,13 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { "leased", "wrong_thread", "source_edited", - ])( - "retries only the original pre-provider Telegram request after exact cleanup: %s", - async (mode) => { + "different_error", + ].flatMap((mode) => [ + "runner_state_identity_mismatch", + "runner_state_identity_mismatch: prior_owner_active", + ].map((errorMessage) => ({ mode, errorMessage }))))( + "retries only the original pre-provider Telegram request after exact cleanup: $mode ($errorMessage)", + async ({ mode, errorMessage }) => { const context = await committedChatResponseRecoveryFixture("telegram"); const providerAccount = mode === "null_account" @@ -56286,7 +56290,9 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { agentId: context.fixture.assignedAgentId, status: "failed", errorCode: "adapter_failed", - error: "runner_state_identity_mismatch", + error: mode === "different_error" + ? errorMessage.replace("runner_state_identity_mismatch", "runner_state_identity_mismatch_other") + : errorMessage, finishedAt: new Date(), wakeupRequestId: action.id, runtimeMode: "native", @@ -69057,7 +69063,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }, ); - it("retries an unknown subscription mutation after restart using freshly observed options, not a recovered confirmation flag", async () => { + it("retries an unknown subscription mutation after restart using freshly observed options, not a recovered confirmation flag", async ({ onTestFailed }) => { + let phase = "create fixture"; + onTestFailed(() => { + console.error(`Telegram subscription recovery failed during: ${phase}`); + }); const lane = await draftFixture(); try { await db @@ -69074,6 +69084,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { throw new Error("Synthetic unknown subscription response"); return undefined; }); + phase = "first subscription attempt"; await lane.processSubscriptionAttempt(); const [action] = await db .select() @@ -69089,17 +69100,28 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { status: "failed", result: { retryable: true, providerConfirmed: false }, }); + // Keep the retry pending until the explicit post-restart transition below. + // A busy runner can otherwise exhaust the real one-second backoff here. + await db + .update(chatActions) + .set({ + result: { ...action!.result, retryAt: "2099-01-01T00:00:00.000Z" }, + }) + .where(eq(chatActions.id, action!.id)); + phase = "ordinary publication before restart"; expect((await lane.send("unknown-subscription"))?.state).toBe( "published", ); expect(lane.requests).toHaveLength(1); expect(lane.requests[0]!.method.endsWith("Draft")).toBe(false); + phase = "pending recovery before restart"; await lane.context.service.processPendingDeliveries(); expect( lane.maintenanceRequests.filter( ({ method }) => method === "setWebhook", ), ).toHaveLength(1); + phase = "restart"; await lane.restart(); lane.setSubscriptionInfo({ allowed_updates: ["message", "chat_member"], @@ -69115,10 +69137,12 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }, }) .where(eq(chatActions.id, action!.id)); + phase = "concurrent recovery after restart"; await Promise.all([ lane.context.service.processPendingDeliveries(), lane.context.service.processPendingDeliveries(), ]); + phase = "verify recovered subscription"; const mutations = lane.maintenanceRequests.filter( ({ method }) => method === "setWebhook", ); @@ -69148,9 +69172,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { 200, ); }); + phase = "publication after subscription recovery"; expect((await lane.send("repaired-after-unknown"))?.state).toBe( "cancelled", ); + phase = "close fixture"; } finally { await lane.close(); } diff --git a/server/src/__tests__/chat-interaction-publications.test.ts b/server/src/__tests__/chat-interaction-publications.test.ts index c9cba1b72f..6c4c5d5f8e 100644 --- a/server/src/__tests__/chat-interaction-publications.test.ts +++ b/server/src/__tests__/chat-interaction-publications.test.ts @@ -1009,7 +1009,11 @@ describeEmbeddedPostgres( { id: fixture.issueId, companyId: fixture.companyId }, { kind: "request_confirmation", - payload: { version: 1, prompt: "Proceed with the release?" }, + payload: { + version: 1, + prompt: "Proceed with the release?", + supersedeOnUserComment: true, + }, }, { agentId: fixture.agentId }, ); @@ -1053,6 +1057,7 @@ describeEmbeddedPostgres( kind: "ask_user_questions", payload: { version: 1, + supersedeOnUserComment: true, questions: [ { id: "priority", diff --git a/server/src/__tests__/execution-workspace-policy.test.ts b/server/src/__tests__/execution-workspace-policy.test.ts index 07f6179731..e5cf70cf01 100644 --- a/server/src/__tests__/execution-workspace-policy.test.ts +++ b/server/src/__tests__/execution-workspace-policy.test.ts @@ -300,6 +300,107 @@ describe("execution workspace policy helpers", () => { }); }); + describe("partial issue workspace strategies", () => { + const projectStrategy = { + type: "git_worktree" as const, + baseRef: "origin/main", + branchTemplate: "{{issue.identifier}}-{{slug}}", + worktreeParentDir: ".paperclip/worktrees", + provisionCommand: "true", + runtimeProvisionCommand: "npm run setup:runtime", + teardownCommand: "npm run teardown", + }; + + function resolveStrategy( + strategy: Record, + enabled = true, + ) { + return buildExecutionWorkspaceAdapterConfig({ + agentConfig: { workspaceStrategy: { type: "git_worktree", provisionCommand: "agent-setup" } }, + projectPolicy: parseProjectExecutionWorkspacePolicy({ + enabled, + defaultMode: "isolated_workspace", + workspaceStrategy: projectStrategy, + }), + issueSettings: parseIssueExecutionWorkspaceSettings({ + mode: "isolated_workspace", + workspaceStrategy: strategy, + }), + mode: "isolated_workspace", + legacyUseProjectWorkspace: null, + }).workspaceStrategy; + } + + it("retains project hooks when an issue changes only its base branch", () => { + expect(resolveStrategy({ type: "git_worktree", baseRef: "origin/release" })).toEqual({ + ...projectStrategy, + baseRef: "origin/release", + }); + }); + + it.each(["npm run issue-setup", "", null])("honors an explicit provisioning override of %j", (provisionCommand) => { + expect(resolveStrategy({ type: "git_worktree", provisionCommand })).toEqual({ + ...projectStrategy, + provisionCommand, + }); + }); + + it("preserves explicit null clears through persisted JSON parsing", () => { + const strategy = { + type: "git_worktree", + baseRef: null, + branchTemplate: null, + worktreeParentDir: null, + provisionCommand: null, + runtimeProvisionCommand: null, + teardownCommand: null, + }; + expect(resolveStrategy(strategy)).toEqual(strategy); + }); + + it.each(["cloud_sandbox", "adapter_managed", "project_primary"])("does not carry project hooks into %s", (type) => { + expect(resolveStrategy({ type })).toEqual({ type }); + }); + + it("does not inherit a disabled project strategy", () => { + expect(resolveStrategy({ type: "git_worktree", baseRef: "origin/release" }, false)).toEqual({ + type: "git_worktree", + baseRef: "origin/release", + }); + expect(resolveStrategy({}, false)).toEqual({ + type: "git_worktree", + provisionCommand: "agent-setup", + }); + }); + + it("keeps project hooks for an exact branch pin without inheriting a branch template", () => { + const resolved = resolveStrategy({ type: "git_worktree", existingBranch: "fix/existing" }); + expect(resolved).toEqual({ + ...projectStrategy, + branchTemplate: undefined, + existingBranch: "fix/existing", + }); + expect(issueExecutionWorkspaceSettingsSchema.safeParse({ + mode: "isolated_workspace", + workspaceStrategy: resolved, + }).success).toBe(true); + }); + + it("does not mutate the project or issue strategy", () => { + const issueStrategy = { type: "git_worktree" as const, baseRef: "origin/release" }; + const result = buildExecutionWorkspaceAdapterConfig({ + agentConfig: {}, + projectPolicy: { enabled: true, workspaceStrategy: Object.freeze({ ...projectStrategy }) }, + issueSettings: { workspaceStrategy: Object.freeze(issueStrategy) }, + mode: "isolated_workspace", + legacyUseProjectWorkspace: null, + }); + expect(result.workspaceStrategy).not.toBe(issueStrategy); + expect(issueStrategy).toEqual({ type: "git_worktree", baseRef: "origin/release" }); + expect(projectStrategy.baseRef).toBe("origin/main"); + }); + }); + it("preserves project authorization policy for trust-preset resolution", () => { expect(parseProjectExecutionWorkspacePolicy({ enabled: true, diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index d73a3fbd2d..d8282401e9 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -394,6 +394,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { continuationPolicy: "wake_assignee", payload: { version: 1, + supersedeOnUserComment: true, questions: [{ id: "scope", prompt: "Which scope?", @@ -1435,7 +1436,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); - it("expires ask_user_questions interactions by default when a user comments after creation", async () => { + it("expires ask_user_questions when a creator opts into comment supersede", async () => { const { companyId, issueId } = await seedConfirmationIssue("Question supersede"); const commentId = randomUUID(); @@ -1446,6 +1447,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { kind: "ask_user_questions", payload: { version: 1, + supersedeOnUserComment: true, questions: [{ id: "scope", prompt: "Choose the scope", @@ -1491,7 +1493,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); - it("keeps ask_user_questions pending when user-comment supersede is explicitly disabled", async () => { + it("keeps ask_user_questions pending by default when the user sends a message", async () => { const { companyId, issueId } = await seedConfirmationIssue("Question supersede opt-out"); await interactionsSvc.create({ @@ -1501,7 +1503,6 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { kind: "ask_user_questions", payload: { version: 1, - supersedeOnUserComment: false, questions: [{ id: "scope", prompt: "Choose the scope", @@ -1513,6 +1514,9 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { userId: "local-board", }); + const [created] = await db.select().from(issueThreadInteractions); + expect(created?.payload).toMatchObject({ supersedeOnUserComment: false }); + const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({ id: issueId, companyId, @@ -1600,6 +1604,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { kind: "ask_user_questions", payload: { version: 1, + supersedeOnUserComment: true, questions: [{ id: "scope", prompt: "Choose the scope", @@ -2519,7 +2524,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { status: "pending", continuationPolicy: "wake_assignee", payload: { - supersedeOnUserComment: true, + supersedeOnUserComment: false, allowDeclineReason: true, }, }); @@ -2618,6 +2623,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Which files should be deleted?", + supersedeOnUserComment: true, options: [{ id: "file-a", label: "a.txt" }], }, }, { @@ -2648,6 +2654,27 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); + it("keeps checkbox confirmations pending by default after a user comment", async () => { + const { companyId, issueId } = await seedConfirmationIssue("Checkbox card remains"); + const created = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_checkbox_confirmation", + payload: { + version: 1, + prompt: "Choose a file", + options: [{ id: "file-a", label: "a.txt" }], + }, + }, { userId: "local-board" }); + expect(created.payload.supersedeOnUserComment).toBe(false); + + const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment( + { id: issueId, companyId }, + { id: randomUUID(), createdAt: new Date(Date.now() + 1_000), authorUserId: "local-board" }, + { userId: "local-board" }, + ); + expect(expired).toHaveLength(0); + expect((await db.select().from(issueThreadInteractions))[0]?.status).toBe("pending"); + }); + it("submits request_item_verdicts partially and completes when all items are resolved", async () => { const { companyId, issueId } = await seedConfirmationIssue("Item verdict partial submit"); @@ -2677,7 +2704,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { verdicts: ["approve", "reject"], requireReasonOn: ["reject"], allowBulkApprove: true, - supersedeOnUserComment: true, + supersedeOnUserComment: false, }, }); @@ -2835,6 +2862,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Review generated artifacts.", + supersedeOnUserComment: true, items: [ { id: "api", label: "API route" }, { id: "docs", label: "Docs" }, @@ -2885,6 +2913,27 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); + it("keeps item verdict requests pending by default after a user comment", async () => { + const { companyId, issueId } = await seedConfirmationIssue("Verdict card remains"); + const created = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_item_verdicts", + payload: { + version: 1, + prompt: "Review the file", + items: [{ id: "file-a", label: "a.txt" }], + }, + }, { userId: "local-board" }); + expect(created.payload.supersedeOnUserComment).toBe(false); + + const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment( + { id: issueId, companyId }, + { id: randomUUID(), createdAt: new Date(Date.now() + 1_000), authorUserId: "local-board" }, + { userId: "local-board" }, + ); + expect(expired).toHaveLength(0); + expect((await db.select().from(issueThreadInteractions))[0]?.status).toBe("pending"); + }); + it("returns accepted agent confirmations from review without resetting active work", async () => { const companyId = randomUUID(); const goalId = randomUUID(); @@ -3187,7 +3236,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { .resolves.toBe("planning"); }); - it("expires request confirmations by default when a user comments after creation", async () => { + it("expires request confirmations when a creator opts into comment supersede", async () => { const { companyId, issueId } = await seedConfirmationIssue(); const commentId = randomUUID(); @@ -3199,6 +3248,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Proceed with the current draft?", + supersedeOnUserComment: true, }, }, { userId: "local-board", @@ -3234,7 +3284,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); - it("keeps request confirmations pending when user-comment supersede is explicitly disabled", async () => { + it("keeps request confirmations pending by default when the user sends a message", async () => { const { companyId, issueId } = await seedConfirmationIssue("Comment supersede opt-out"); await interactionsSvc.create({ @@ -3245,12 +3295,14 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Proceed with the current draft?", - supersedeOnUserComment: false, }, }, { userId: "local-board", }); + const [created] = await db.select().from(issueThreadInteractions); + expect(created?.payload).toMatchObject({ supersedeOnUserComment: false }); + const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({ id: issueId, companyId, @@ -3475,6 +3527,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Proceed with the current draft?", + supersedeOnUserComment: true, }, }, { userId: "local-board", diff --git a/server/src/__tests__/issue-thread-interactions-telemetry.test.ts b/server/src/__tests__/issue-thread-interactions-telemetry.test.ts index d86813db1d..d3db5df11d 100644 --- a/server/src/__tests__/issue-thread-interactions-telemetry.test.ts +++ b/server/src/__tests__/issue-thread-interactions-telemetry.test.ts @@ -438,6 +438,7 @@ describeEmbeddedPostgres("issueThreadInteractionService telemetry", () => { continuationPolicy: "wake_assignee", payload: { version: 1, + supersedeOnUserComment: true, questions: [ { id: "scope", @@ -563,6 +564,7 @@ describeEmbeddedPostgres("issueThreadInteractionService telemetry", () => { payload: { version: 1, prompt: "Approve this plan?", + supersedeOnUserComment: true, }, }, { userId: "local-board", diff --git a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts index bf7712de39..841896e033 100644 --- a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts +++ b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts @@ -201,7 +201,8 @@ function registerModuleMocks() { })); } -async function createApp() { +async function createApp(transaction: (callback: (tx: Record) => Promise) => Promise = + async (callback) => callback({})) { const [{ errorHandler }, { issueRoutes }] = await Promise.all([ vi.importActual("../middleware/index.js"), vi.importActual("../routes/issues.js"), @@ -220,7 +221,7 @@ async function createApp() { next(); }); app.use("/api", issueRoutes({ - transaction: async (callback: (tx: Record) => Promise) => callback({}), + transaction, } as any, {} as any)); app.use(errorHandler); return app; @@ -330,6 +331,7 @@ describe("issue update comment wakeups", () => { externalConversationState, assigneeAgentId: ASSIGNEE_AGENT_ID, assigneeUserId: null, + assigneeAdapterOverrides: { adapterConfig: { model: "gpt-6-astra", modelReasoningEffort: "ultra", fastMode: true } }, }); mockIssueService.getById.mockResolvedValue(existing); mockIssueService.update.mockResolvedValue(updated); @@ -347,11 +349,16 @@ describe("issue update comment wakeups", () => { assigneeUserId: null, comment: "write the whole thing", commentClientRequestId: "55555555-5555-4555-8555-555555555555", + assigneeAdapterOverrides: updated.assigneeAdapterOverrides, }); expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalledWith(existing.id, expect.objectContaining({ + assigneeAdapterOverrides: updated.assigneeAdapterOverrides, + }), expect.anything()); expect(mockIssueService.addComment).toHaveBeenCalledWith(existing.id, "write the whole thing", expect.anything(), - expect.objectContaining({ clientRequestId: "55555555-5555-4555-8555-555555555555" })); + expect.objectContaining({ clientRequestId: "55555555-5555-4555-8555-555555555555" }), expect.anything()); + expect(mockIssueService.update.mock.calls[0]?.[2]).toBe(mockIssueService.addComment.mock.calls[0]?.[4]); // The route dispatches the wake after it sends the response, so wait for // the fire-and-forget dispatch to settle. This keeps the wake inside this // test and stops it from leaking into the next test as an extra call. @@ -378,6 +385,35 @@ describe("issue update comment wakeups", () => { ); }); + it("rolls back adapter settings if the accompanying comment fails", async () => { + const existing = makeIssue({ assigneeAgentId: ASSIGNEE_AGENT_ID, assigneeUserId: null }); + let persistedModel = "gpt-6-sol"; + mockIssueService.getById.mockResolvedValue(existing); + mockIssueService.update.mockImplementation(async (_id, fields) => { + persistedModel = fields.assigneeAdapterOverrides.adapterConfig.model; + return { ...existing, ...fields }; + }); + mockIssueService.addComment.mockRejectedValue(new Error("comment write failed")); + const transaction = vi.fn(async (callback: (tx: Record) => Promise) => { + const previousModel = persistedModel; + try { + return await callback({}); + } catch (error) { + persistedModel = previousModel; + throw error; + } + }); + + const res = await request(await createApp(transaction)) + .patch(`/api/issues/${existing.id}`) + .send({ comment: "use Astra", assigneeAdapterOverrides: { adapterConfig: { model: "gpt-6-astra" } } }); + + expect(res.status).toBe(500); + expect(transaction).toHaveBeenCalledOnce(); + expect(persistedModel).toBe("gpt-6-sol"); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + }); + it("interrupts the active run and wakes the newly assigned agent with handoff context", async () => { const existing = makeIssue({ assigneeAgentId: PREVIOUS_AGENT_ID, diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 9309731a6d..a3fa946571 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -1,6 +1,6 @@ import { randomUUID } from "node:crypto"; import { asc, eq } from "drizzle-orm"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { sql } from "drizzle-orm"; import { activityLog, @@ -41,7 +41,9 @@ import { deriveIssueCommentRunLogAttribution, ISSUE_LIST_MAX_LIMIT, issueService, + readIssueCommentRunLogText, } from "../services/issues.ts"; +import { getRunLogStore } from "../services/run-log-store.js"; import { WORKSPACE_WORKTREE_REQUIRES_PROJECT_CODE, WORKSPACE_WORKTREE_REQUIRES_PROJECT_MESSAGE, @@ -149,6 +151,155 @@ describeEmbeddedPostgres("issueService run attachment artifacts", () => { }, 20_000); }); +describe("readIssueCommentRunLogText", () => { + it("cancels timed-out storage reads so later listings can recover", async () => { + let active = 0; + const cleanups: Array<() => void> = []; + const read = vi.spyOn(getRunLogStore(), "read").mockImplementation((_handle, options) => + new Promise((_resolve, reject) => { + active += 1; + let settled = false; + const abort = () => { + if (settled) return; + settled = true; + active -= 1; + reject(new DOMException("Read aborted", "AbortError")); + }; + cleanups.push(abort); + options?.signal?.addEventListener("abort", abort, { once: true }); + }), + ); + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const run = { runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes: null }; + const firstBatch = Array.from({ length: 8 }, () => readIssueCommentRunLogText(run)); + try { + await vi.advanceTimersByTimeAsync(3_000); + expect(await Promise.all(firstBatch)).toEqual(Array(8).fill("")); + expect(active).toBe(0); + read.mockResolvedValueOnce({ content: "storage recovered" }); + await expect(readIssueCommentRunLogText(run)).resolves.toBe("storage recovered"); + expect(read).toHaveBeenCalledTimes(9); + } finally { + for (const cleanup of cleanups) cleanup(); + await Promise.allSettled(firstBatch); + await vi.advanceTimersByTimeAsync(0); + vi.useRealTimers(); + read.mockRestore(); + } + }); + + it("keeps readable attribution evidence for concurrent listings", async () => { + let release!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + const read = vi.spyOn(getRunLogStore(), "read").mockImplementation(async () => { + await gate; + return { content: "comment id: legacy-comment" }; + }); + const run = { runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes: null }; + const listings = Array.from({ length: 2 }, () => + Promise.all(Array.from({ length: 8 }, () => readIssueCommentRunLogText(run))), + ); + try { + release(); + for (const listing of listings) { + expect(await listing).toEqual(Array(8).fill("comment id: legacy-comment")); + } + expect(read).toHaveBeenCalledTimes(16); + } finally { + release(); + await Promise.allSettled(listings); + read.mockRestore(); + } + }); + + it.each([null, 128])("keeps partial attribution evidence when storage fails with logBytes=%s", async (logBytes) => { + const read = vi.spyOn(getRunLogStore(), "read").mockRejectedValue(new Error("Storage gateway unavailable")); + const run = { runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes }; + try { + await expect(readIssueCommentRunLogText(run)).resolves.toBe(""); + read.mockResolvedValueOnce({ content: "earlier evidence", nextOffset: 16 }); + await expect(readIssueCommentRunLogText(run)).resolves.toBe("earlier evidence"); + } finally { + read.mockRestore(); + } + }); + + it("bounds reads that ignore cancellation and stops late pagination after the deadline", async () => { + let release!: (value: { content: string; nextOffset: number }) => void; + const stalled = new Promise<{ content: string; nextOffset: number }>((resolve) => { + release = resolve; + }); + const read = vi.spyOn(getRunLogStore(), "read") + .mockResolvedValueOnce({ content: "earlier evidence", nextOffset: 16 }) + .mockReturnValueOnce(stalled); + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + let result: string | undefined; + const pending = readIssueCommentRunLogText({ + runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes: null, + }).then((value) => { result = value; }); + try { + await vi.advanceTimersByTimeAsync(3_000); + expect(result).toBe("earlier evidence"); + expect(read.mock.calls[1]?.[1]?.signal?.aborted).toBe(true); + release({ content: "too late", nextOffset: 24 }); + await pending; + await vi.advanceTimersByTimeAsync(0); + expect(read).toHaveBeenCalledTimes(2); + expect(result).toBe("earlier evidence"); + } finally { + release({ content: "", nextOffset: 0 }); + await pending.catch(() => {}); + vi.useRealTimers(); + read.mockRestore(); + } + }); + + it.each([null, 128, 0])("reads existing attribution markers with logBytes=%s", async (logBytes) => { + const commentId = randomUUID(); + const runId = randomUUID(); + const agentId = randomUUID(); + const read = vi.spyOn(getRunLogStore(), "read") + .mockResolvedValueOnce({ content: "comment id: ", nextOffset: 12 }) + .mockResolvedValueOnce({ content: commentId + "\n" }); + try { + const logContent = await readIssueCommentRunLogText({ + runId, logStore: "local_file", logRef: "test/run.ndjson", logBytes, + }); + const derived = deriveIssueCommentRunLogAttribution( + [{ + id: commentId, + authorAgentId: null, + authorUserId: "local-board", + createdByRunId: null, + createdAt: new Date("2020-01-01T00:00:01Z"), + }], + [{ + runId, + agentId, + createdAt: new Date("2020-01-01T00:00:00Z"), + startedAt: new Date("2020-01-01T00:00:00Z"), + finishedAt: new Date("2020-01-01T00:00:02Z"), + logContent, + }], + ); + if (logBytes === 0) { + expect(read).not.toHaveBeenCalled(); + expect(derived.size).toBe(0); + } else { + expect(read).toHaveBeenCalledTimes(2); + expect(read.mock.calls[1]?.[1]?.offset).toBe(12); + expect(derived.get(commentId)).toEqual({ + derivedAuthorAgentId: agentId, + derivedCreatedByRunId: runId, + derivedAuthorSource: "run_log_comment_post", + }); + } + } finally { + read.mockRestore(); + } + }); +}); + describe("deriveIssueCommentRunLogAttribution", () => { it("recovers agent attribution from run logs that printed the posted comment id", () => { const commentId = randomUUID(); diff --git a/server/src/__tests__/run-identity.test.ts b/server/src/__tests__/run-identity.test.ts index 9155a9b9fa..d6dbbbfc3f 100644 --- a/server/src/__tests__/run-identity.test.ts +++ b/server/src/__tests__/run-identity.test.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import { eq, sql } from "drizzle-orm"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { agentWakeupRequests, agents, companies, createDb, heartbeatRuns, heartbeatRunEvents, issueComments, issueThreadInteractions, issues } from "@paperclipai/db"; +import { agentWakeupRequests, agents, companies, createDb, heartbeatRuns, heartbeatRunEvents, issueComments, issueThreadInteractions, issues, secretAccessEvents } from "@paperclipai/db"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; import { acceptSteeredIdentity, captureRunIdentity, initializeRunIdentity, listRunIdentityContexts, rejectSteeredIdentity, reserveSteeredIdentity } from "../services/run-identity.js"; @@ -217,11 +217,11 @@ const support = await getEmbeddedPostgresTestSupport(); } }); - it("does not deadlock identity initialization against a task mutation that also updates the run", async () => { + it.each(["update", "no key update"] as const)("serializes identity initialization behind a task's %s lock", async (lockMode) => { const input = await seed(); let initialization!: ReturnType; await db.transaction(async (tx) => { - await tx.select().from(issues).where(eq(issues.id, input.issueId)).for("update"); + await tx.select().from(issues).where(eq(issues.id, input.issueId)).for(lockMode); const [backend] = await tx.execute(sql`select pg_backend_pid() as pid`) as unknown as Array<{ pid: number }>; initialization = initializeRunIdentity(db, { ...input, messageIds: [], responsibleUserId: "A", cause: "instruction" }); // Wait until initialization is blocked by this task mutation, rather than @@ -241,6 +241,47 @@ const support = await getEmbeddedPostgresTestSupport(); await expect(initialization).resolves.toMatchObject({ responsibleUserId: "A" }); }); + it.each(["initialize", "capture"] as const)( + "can %s identity while an audit append holds foreign-key locks", + async (operation) => { + const input = await seed(); + if (operation === "capture") { + await initializeRunIdentity(db, { ...input, messageIds: [], responsibleUserId: "A", cause: "instruction" }); + } + // A real append holds KEY SHARE on both parent rows until it commits. + // Bound the other connection's wait so a conflicting lock fails this + // regression instead of leaving both transactions waiting for each other. + const identityDb = createDb(`${database.connectionString}?options=-c%20lock_timeout%3D1000`, { + maxConnections: 1, + }); + const [settings] = await identityDb.execute(sql`show lock_timeout`); + expect(settings?.lock_timeout).toBe("1s"); + await db.transaction(async (audit) => { + await audit.insert(secretAccessEvents).values({ + companyId: input.companyId, + heartbeatRunId: input.runId, + issueId: input.issueId, + provider: "local_encrypted", + actorType: "agent", + actorId: input.agentId, + consumerType: "agent", + consumerId: input.agentId, + outcome: "granted", + }); + if (operation === "initialize") { + await expect(initializeRunIdentity(identityDb, { + ...input, messageIds: [], responsibleUserId: "A", cause: "instruction", + })).resolves.toMatchObject({ responsibleUserId: "A" }); + } else { + await expect(captureRunIdentity(identityDb, input)).resolves.toMatchObject({ + run: { responsibleUserId: "A" }, + context: { responsibleUserId: "A" }, + }); + } + }); + }, + ); + it("does not turn a company-default fallback into personal consent on continuation", async () => { const input = await seed(); await initializeRunIdentity(db, { ...input, messageIds: [], responsibleUserId: "A", cause: "company_default" }); diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index ececaebf9e..fd0caefa8e 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -24,6 +24,11 @@ import { workspaceRuntimeServices, } from "@paperclipai/db"; import { eq } from "drizzle-orm"; +import { + buildExecutionWorkspaceAdapterConfig, + parseIssueExecutionWorkspaceSettings, + parseProjectExecutionWorkspacePolicy, +} from "../services/execution-workspace-policy.ts"; import { buildWorkspaceRuntimeDesiredStatePatch, cleanupExecutionWorkspaceArtifacts, @@ -869,6 +874,52 @@ describe("realizeExecutionWorkspace", () => { expect(second.branchName).toBe(first.branchName); }); + it("retains the project provision command when an issue overrides its base branch", async () => { + const repoRoot = await createTempRepo(); + await fs.mkdir(path.join(repoRoot, "scripts")); + await fs.writeFile( + path.join(repoRoot, "scripts", "provision-worktree.sh"), + "#!/usr/bin/env bash\necho 'Unexpected repository provision fallback' >&2\nexit 1\n", + ); + await runGit(repoRoot, ["add", "scripts/provision-worktree.sh"]); + await runGit(repoRoot, ["commit", "-m", "Add fallback provisioner"]); + await runGit(repoRoot, ["branch", "release"]); + const config = buildExecutionWorkspaceAdapterConfig({ + agentConfig: {}, + projectPolicy: parseProjectExecutionWorkspacePolicy({ + enabled: true, + defaultMode: "isolated_workspace", + workspaceStrategy: { type: "git_worktree", baseRef: "main", provisionCommand: "true" }, + }), + issueSettings: parseIssueExecutionWorkspaceSettings({ + mode: "isolated_workspace", + workspaceStrategy: { type: "git_worktree", baseRef: "release" }, + }), + mode: "isolated_workspace", + legacyUseProjectWorkspace: null, + }); + try { + const workspace = await realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config, + issue: { id: "issue-1", identifier: "TEST-1", title: "Keep project setup" }, + agent: { id: "agent-1", name: "Test agent", companyId: "company-1" }, + }); + expect(workspace.created).toBe(true); + expect(workspace.baseRefSha).toBe(await readGit(repoRoot, ["rev-parse", "release"])); + await expect(fs.stat(path.join(workspace.cwd, ".git"))).resolves.toBeTruthy(); + } finally { + await fs.rm(repoRoot, { recursive: true, force: true }); + } + }); + it("defaults the repo-provided worktree provisioner for git worktree strategies", async () => { const repoRoot = await createTempRepo(); await fs.mkdir(path.join(repoRoot, "scripts"), { recursive: true }); diff --git a/server/src/adapters/registry.ts b/server/src/adapters/registry.ts index d4abb5249e..62cef753d7 100644 --- a/server/src/adapters/registry.ts +++ b/server/src/adapters/registry.ts @@ -1042,13 +1042,22 @@ function getDeclaredAdapterModels(): ReturnType { return value; } +function declaredModelsForAdapter(type: string): { id: string; label: string }[] | null { + const declared = getDeclaredAdapterModels()?.[type]; + return declared?.length + ? declared.map((model) => ({ id: model.id, label: model.label ?? model.id })) + : null; +} + export async function listAdapterModels(type: string): Promise<{ id: string; label: string }[]> { - const declaredModels = getDeclaredAdapterModels(); - if (declaredModels && declaredModels[type]?.length) { - return declaredModels[type].map((m) => ({ id: m.id, label: m.label ?? m.id })); - } + const declaredModels = declaredModelsForAdapter(type); + if (declaredModels) return declaredModels; const adapter = findActiveServerAdapter(type); if (!adapter) return []; + // The built-in Codex adapter's OpenAI discovery includes image, audio, and + // embedding models that Codex cannot run. Use its curated list; declared + // models above and custom adapter discovery remain authoritative. + if (adapter === codexLocalAdapter) return adapter.models ?? []; if (adapter.listModels) { const discovered = await adapter.listModels(); if (discovered.length > 0) return discovered; @@ -1057,6 +1066,9 @@ export async function listAdapterModels(type: string): Promise<{ id: string; lab } export async function refreshAdapterModels(type: string): Promise<{ id: string; label: string }[]> { + const declaredModels = declaredModelsForAdapter(type); + if (declaredModels) return declaredModels; + if (findActiveServerAdapter(type) === codexLocalAdapter) return listAdapterModels(type); const adapter = findActiveServerAdapter(type); if (!adapter) return []; if (adapter.refreshModels) { diff --git a/server/src/onboarding-assets/ceo/HEARTBEAT.md b/server/src/onboarding-assets/ceo/HEARTBEAT.md index dcdc6f85df..f80d325505 100644 --- a/server/src/onboarding-assets/ceo/HEARTBEAT.md +++ b/server/src/onboarding-assets/ceo/HEARTBEAT.md @@ -50,7 +50,7 @@ Status quick guide: - Create subtasks with `POST /api/companies/{companyId}/issues`. Always set `parentId` and `goalId`. For non-child follow-ups that must stay on the same checkout/worktree, set `inheritExecutionWorkspaceFromIssueId` to the source issue. - When you know the needed work and owner, create those subtasks directly. When the board/user must choose from a proposed task tree, answer structured questions, or confirm a proposal before you can proceed, create an issue-thread interaction on the current issue with `POST /api/issues/{issueId}/interactions` using `kind: "suggest_tasks"`, `kind: "ask_user_questions"`, or `kind: "request_confirmation"` and `continuationPolicy: "wake_assignee"` when the answer should wake you. - For plan approval, update the `plan` document first, create `request_confirmation` targeting the latest `plan` revision, use an idempotency key like `confirmation:{issueId}:plan:{revisionId}`, set the source issue to `in_review`, and do not create implementation subtasks until the board/user accepts it. -- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `true`, so a later board/user comment invalidates the pending request. Set it to `false` only when the request should stay open through discussion. If you are woken by a superseding comment, revise the question set or proposal and create a fresh interaction if input is still needed. +- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `false`, so a later board/user comment keeps the pending card open while discussion continues. Set it to `true` when a new comment should replace the pending request. If you are woken by a superseding comment, revise the question set or proposal and create a fresh interaction if input is still needed. - Use `paperclip-create-agent` skill when hiring new agents. - Assign work to the right agent for the job. diff --git a/server/src/onboarding-assets/default/AGENTS.md b/server/src/onboarding-assets/default/AGENTS.md index cf9d10a55f..1472483914 100644 --- a/server/src/onboarding-assets/default/AGENTS.md +++ b/server/src/onboarding-assets/default/AGENTS.md @@ -17,7 +17,7 @@ You are an agent at Paperclip company. 3. Only then create `request_confirmation` with `target={ type: 'issue_document', key: 'plan', revisionId: latestRevisionId }` and `idempotencyKey=confirmation:{issueId}:plan:{revisionId}`. 4. Wait for acceptance before creating implementation subtasks. Never present a plan only in a thread comment or through `ask_user_questions`; comments are supporting context and questions are for gathering input, not plan review. -- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `true`, so a later board/user comment invalidates the pending request. Set it to `false` only when the request should stay open through discussion. If you wake up from a superseding comment, revise the artifact, question set, or proposal and create a fresh interaction if input is still needed. +- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `false`, so a later board/user comment keeps the pending card open while discussion continues. Set it to `true` when a new comment should replace the pending request. If you wake up from a superseding comment, revise the artifact, question set, or proposal and create a fresh interaction if input is still needed. - For human input, save a pending question/confirmation interaction and set `in_review`; prose alone does not create a waiting path. Use `blockedByIssueIds` for issue dependencies. An agent may set an `unblockDescriptor` only for itself (`owner: { "agentId": "" }` plus `action`), not for the board/user or another agent. - Respect budget, pause/cancel, approval gates, and company boundaries. diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 653c5e7965..10ac1f566c 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -13729,13 +13729,17 @@ export function issueRoutes( }); const decision = transition.decision && decisionId ? transition.decision : null; - let attachmentComment: Awaited> | null = + let transactionalComment: Awaited> | null = null; - const attachmentCommentSourceTrust = commentAttachmentIds?.length + const commentWithAdapterOverrides = Boolean( + commentBody && updateFields.assigneeAdapterOverrides !== undefined, + ); + const transactionalCommentSourceTrust = commentAttachmentIds?.length || commentWithAdapterOverrides ? await sourceTrustForActorWrite(existing, actor) : undefined; const shouldUseTransactionalIssueUpdate = Boolean(commentAttachmentIds?.length) || + commentWithAdapterOverrides || Boolean(decision) || shouldRelayStop || persistReviewActivityTransactionally || @@ -13750,10 +13754,10 @@ export function issueRoutes( return null; const updated = await updateIssue(tx); if (!updated) return null; - if (commentAttachmentIds?.length) { - // Reassignment, comment creation and upload binding commit together. - // An invalid or already-bound receipt rolls back the issue update. - attachmentComment = await svc.addComment( + if (commentAttachmentIds?.length || commentWithAdapterOverrides) { + // Adapter settings, reassignment, comment and upload binding commit together. + // A failed comment or invalid receipt rolls back the issue update. + transactionalComment = await svc.addComment( id, commentBody, { @@ -13768,7 +13772,7 @@ export function issueRoutes( clientRequestId: actor.actorType === "user" ? commentClientRequestId : undefined, mirrorToSlack: actor.actorType === "user", authorizationReason: issueMutationAuthorizationReason, - sourceTrust: attachmentCommentSourceTrust, + sourceTrust: transactionalCommentSourceTrust, }, tx, ); @@ -14353,7 +14357,7 @@ export function issueRoutes( } let comment: Awaited> | null = - attachmentComment; + transactionalComment; let goalCommentSteered = false; let lostReviewPathRef: string | null = null; if (commentBody) { diff --git a/server/src/services/chat-channels.ts b/server/src/services/chat-channels.ts index dd3694dc6a..a4c67d20b3 100644 --- a/server/src/services/chat-channels.ts +++ b/server/src/services/chat-channels.ts @@ -12006,7 +12006,11 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { run.nativeIssueId === issueId && run.status === "failed" && run.errorCode === "adapter_failed" && - run.error === "runner_state_identity_mismatch" && + // A diagnostic reason does not change this failure category. The exact + // checkpoint, cleanup receipt, and no-provider-work proofs below still + // decide whether the original request can be retried. + (run.error === "runner_state_identity_mismatch" || + run.error?.startsWith("runner_state_identity_mismatch: ")) && run.nativePhase === "observed" && coordinator.phase === "observed" && coordinator.attempt === 0 && diff --git a/server/src/services/execution-workspace-policy.ts b/server/src/services/execution-workspace-policy.ts index b430de18cc..dd36825a2a 100644 --- a/server/src/services/execution-workspace-policy.ts +++ b/server/src/services/execution-workspace-policy.ts @@ -38,17 +38,17 @@ function parseExecutionWorkspaceStrategy(raw: unknown): ExecutionWorkspaceStrate } return { type, - ...(typeof parsed.baseRef === "string" ? { baseRef: parsed.baseRef } : {}), - ...(typeof parsed.branchTemplate === "string" ? { branchTemplate: parsed.branchTemplate } : {}), + ...(typeof parsed.baseRef === "string" || parsed.baseRef === null ? { baseRef: parsed.baseRef } : {}), + ...(typeof parsed.branchTemplate === "string" || parsed.branchTemplate === null ? { branchTemplate: parsed.branchTemplate } : {}), ...(typeof parsed.existingBranch === "string" && parsed.existingBranch.trim().length > 0 ? { existingBranch: parsed.existingBranch.trim() } : {}), - ...(typeof parsed.worktreeParentDir === "string" ? { worktreeParentDir: parsed.worktreeParentDir } : {}), - ...(typeof parsed.provisionCommand === "string" ? { provisionCommand: parsed.provisionCommand } : {}), - ...(typeof parsed.runtimeProvisionCommand === "string" + ...(typeof parsed.worktreeParentDir === "string" || parsed.worktreeParentDir === null ? { worktreeParentDir: parsed.worktreeParentDir } : {}), + ...(typeof parsed.provisionCommand === "string" || parsed.provisionCommand === null ? { provisionCommand: parsed.provisionCommand } : {}), + ...(typeof parsed.runtimeProvisionCommand === "string" || parsed.runtimeProvisionCommand === null ? { runtimeProvisionCommand: parsed.runtimeProvisionCommand } : {}), - ...(typeof parsed.teardownCommand === "string" ? { teardownCommand: parsed.teardownCommand } : {}), + ...(typeof parsed.teardownCommand === "string" || parsed.teardownCommand === null ? { teardownCommand: parsed.teardownCommand } : {}), }; } @@ -418,11 +418,18 @@ export function buildExecutionWorkspaceAdapterConfig(input: { if (hasWorkspaceControl) { if (input.mode === "isolated_workspace") { - const strategy = - input.issueSettings?.workspaceStrategy ?? - input.projectPolicy?.workspaceStrategy ?? + const projectStrategy = projectHasPolicy ? input.projectPolicy?.workspaceStrategy : undefined; + const issueStrategy = input.issueSettings?.workspaceStrategy; + // An issue that changes its branch still needs the project's setup hooks. + // Do not carry those defaults into a different execution strategy. + const strategy = issueStrategy && projectStrategy?.type === issueStrategy.type + ? { ...projectStrategy, ...issueStrategy } + : issueStrategy ?? projectStrategy ?? parseExecutionWorkspaceStrategy(nextConfig.workspaceStrategy) ?? ({ type: "git_worktree" } satisfies ExecutionWorkspaceStrategy); + if (issueStrategy?.existingBranch && issueStrategy.branchTemplate === undefined && strategy !== issueStrategy) { + delete strategy.branchTemplate; + } nextConfig.workspaceStrategy = strategy as unknown as Record; } else { delete nextConfig.workspaceStrategy; diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index a62c269115..e1212ee2b0 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -24933,8 +24933,8 @@ export function heartbeatService( : null; let logSummary: { - bytes: number; - sha256?: string; + bytes: number | null; + sha256?: string | null; compressed: boolean; } | null = null; if (handle) { @@ -25652,8 +25652,8 @@ export function heartbeatService( logger.error({ err, runId }, "heartbeat execution failed"); let logSummary: { - bytes: number; - sha256?: string; + bytes: number | null; + sha256?: string | null; compressed: boolean; } | null = null; if (handle) { diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 86cd7077e2..deef5871d5 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -851,7 +851,7 @@ function normalizeCreateInteractionInput( ...input, payload: { ...input.payload, - supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true, + supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false, }, }; case "request_confirmation": @@ -859,7 +859,7 @@ function normalizeCreateInteractionInput( ...input, payload: { ...input.payload, - supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true, + supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false, }, }; case "request_checkbox_confirmation": @@ -867,7 +867,7 @@ function normalizeCreateInteractionInput( ...input, payload: { ...input.payload, - supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true, + supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false, }, }; case "request_item_verdicts": @@ -875,7 +875,7 @@ function normalizeCreateInteractionInput( ...input, payload: { ...input.payload, - supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true, + supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false, }, }; default: diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 9feb50394a..ceb13379f7 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -231,6 +231,7 @@ const ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES = 2_000_000; const ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES = 256_000; const ISSUE_COMMENT_RUN_LOG_DERIVATION_END_SLACK_MS = 60_000; const ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_PARALLEL_READS = 8; +const ISSUE_COMMENT_RUN_LOG_DERIVATION_TIMEOUT_MS = 3_000; export const ISSUE_CREATE_IDEMPOTENCY_KEY_RETENTION_DAYS = 7; const ISSUE_CREATE_IDEMPOTENCY_KEY_RETENTION_MS = ISSUE_CREATE_IDEMPOTENCY_KEY_RETENTION_DAYS * 24 * 60 * 60 * 1000; @@ -6542,6 +6543,76 @@ async function countBlockedInboxIssues( }, 0); } +export async function readIssueCommentRunLogText(run: { + runId?: string | null; + logStore: string | null; + logRef: string | null; + logBytes: number | null; +}) { + if (run.logStore !== "local_file" || !run.logRef) return ""; + // A timed-out finalization leaves size unknown even when earlier entries + // exist. Read those logs within the same byte budget as a known-size log. + if (run.logBytes !== null && (!Number.isFinite(run.logBytes) || run.logBytes <= 0)) return ""; + + const logRef = run.logRef; + const store = getRunLogStore(); + let offset = 0; + let content = ""; + let nextOffset: number | undefined = 0; + const controller = new AbortController(); + let readTimer: NodeJS.Timeout | undefined; + + const readChunks = async () => { + while (nextOffset !== undefined) { + controller.signal.throwIfAborted(); + const remainingBytes = + ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES - + Buffer.byteLength(content, "utf8"); + if (remainingBytes <= 0) break; + const chunk = await store.read( + { store: "local_file", logRef }, + { + offset, + limitBytes: Math.min(ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES, remainingBytes), + signal: controller.signal, + }, + ); + controller.signal.throwIfAborted(); + content += chunk.content; + nextOffset = chunk.nextOffset; + offset = chunk.nextOffset ?? 0; + } + }; + + try { + await Promise.race([ + readChunks(), + new Promise((_resolve, reject) => { + readTimer = setTimeout(() => { + const reason = new DOMException("Attribution log read timed out", "TimeoutError"); + // Cancellation closes storage work where supported, but filesystem + // I/O can delay stream destruction. Keep the response deadline too. + reject(reason); + controller.abort(reason); + }, ISSUE_COMMENT_RUN_LOG_DERIVATION_TIMEOUT_MS); + readTimer.unref?.(); + }), + ]); + } catch (err) { + // Attribution enriches already-authorized comments. Missing, failed, or + // stalled storage must not prevent listing them; keep any evidence read. + // Do not log raw provider errors, which can contain credentialed URLs. + logger.warn( + { runId: run.runId ?? undefined, logRef, status: err instanceof HttpError ? err.status : undefined }, + "could not read heartbeat run log while deriving optional issue comment metadata", + ); + } finally { + clearTimeout(readTimer); + } + + return content; +} + export function issueService(db: Db) { const instanceSettings = instanceSettingsService(db); const treeControlSvc = issueTreeControlService(db); @@ -6760,54 +6831,6 @@ export function issueService(db: Db) { }; } - async function readRunLogText(run: { - runId?: string | null; - logStore: string | null; - logRef: string | null; - logBytes: number | null; - }) { - if (run.logStore !== "local_file" || !run.logRef) return ""; - const logBytes = Number(run.logBytes ?? 0); - if (!Number.isFinite(logBytes) || logBytes <= 0) return ""; - - const store = getRunLogStore(); - let offset = 0; - let content = ""; - let nextOffset: number | undefined = 0; - - try { - while (nextOffset !== undefined) { - const remainingBytes = - ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES - - Buffer.byteLength(content, "utf8"); - if (remainingBytes <= 0) break; - const chunk = await store.read( - { store: "local_file", logRef: run.logRef }, - { - offset, - limitBytes: Math.min( - ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES, - remainingBytes, - ), - }, - ); - content += chunk.content; - nextOffset = chunk.nextOffset; - offset = chunk.nextOffset ?? 0; - } - } catch (err) { - if (err instanceof HttpError && err.status === 404) { - logger.warn( - { err, runId: run.runId ?? undefined, logRef: run.logRef }, - "missing heartbeat run log while deriving issue comment metadata", - ); - return content; - } - throw err; - } - - return content; - } // Persist a resolved attribution so subsequent reads stop re-scanning run // logs (and old "Board" threads stay fixed durably). Best-effort: a write @@ -7027,7 +7050,7 @@ export function issueService(db: Db) { ); await Promise.all( batch.map(async (run) => { - logByRunId.set(run.runId, await readRunLogText(run)); + logByRunId.set(run.runId, await readIssueCommentRunLogText(run)); }), ); } diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 39243c0148..6a49c25a7d 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { readFileSync } from "node:fs"; import { access, cp, @@ -57,6 +58,11 @@ import { buildNativeHeartbeatPreparationSpans } from "./native-run-trace.js"; import { NativeRunnerOwnershipUnverifiedError } from "./native-runner-ownership.js"; import type { AdapterRuntimeEvent } from "../../adapters/index.js"; +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, readFileSync: vi.fn(actual.readFileSync) }; +}); + const githubAccess = vi.hoisted(() => ({ activate: vi.fn((_binding: { runId: string }) => vi.fn()), stop: vi.fn(async () => undefined), @@ -1201,6 +1207,54 @@ describe("remote provider pack manifest", () => { }); }); +describe("provider pack read diagnostics", () => { + it.each([ + ["EACCES", "permission_denied"], + ["EPERM", "permission_denied"], + ["EIO", "io_error"], + ])("reports %s without exposing the underlying filesystem message", (code, reason) => { + const root = join(tmpdir(), "private-provider-pack"); + const manifestPath = join(root, "provider-pack.json"); + const cause = Object.assign(new Error(`${code}: cannot read ${manifestPath}`), { + code, + path: manifestPath, + }); + vi.mocked(readFileSync).mockImplementationOnce(() => { throw cause; }); + let failure: Error | undefined; + try { readRemoteProviderPackManifest(root); } catch (error) { failure = error as Error; } + expect(readFileSync).toHaveBeenLastCalledWith(manifestPath, "utf8"); + expect(failure?.message).toBe(`runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable (${reason})`); + expect(failure?.message).not.toContain(root); + expect(failure?.message).not.toContain(code); + expect(failure?.cause).toBe(cause); + }); + + it("classifies a JSON null manifest as incompatible instead of a TypeError", async () => { + const root = await mkdtemp(join(tmpdir(), "paperclip-null-pack-")); + try { + await writeFile(join(root, "provider-pack.json"), "null"); + expect(() => readRemoteProviderPackManifest(root)).toThrow( + "runner_remote_provider_artifact_incompatible: provider pack pins or source revision do not match", + ); + } finally { await rm(root, { recursive: true, force: true }); } + }); + + it.each(["missing", "invalid_json", "invalid_path_type"])("reports %s without putting the path in the terminal message", async (reason) => { + const root = await mkdtemp(join(tmpdir(), "paperclip-private-pack-")); + try { + const manifestPath = join(root, "provider-pack.json"); + if (reason === "invalid_json") await writeFile(manifestPath, "{ private-invalid-json"); + if (reason === "invalid_path_type") await mkdir(manifestPath); + let failure: Error | undefined; + try { readRemoteProviderPackManifest(root); } catch (error) { failure = error as Error; } + expect(failure?.message).toBe(`runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable (${reason})`); + expect(failure?.message).not.toContain(root); + expect(failure?.message).not.toContain("private-invalid-json"); + expect(failure?.cause).toBeDefined(); + } finally { await rm(root, { recursive: true, force: true }); } + }); +}); + describe("native harness persistence profiles", () => { const profile = (provider: Record, driverKind: string) => resolveNativeHarnessPersistenceProfile({ @@ -1507,7 +1561,7 @@ describe("verified native harness backups", () => { }, sourceProviderLeaseId: "sandbox-1", }), - ).toThrow("runner_harness_state_mismatch"); + ).toThrow("runner_harness_state_mismatch: backup_provider_identity_missing"); } finally { await rm(root, { recursive: true, force: true }); } @@ -9166,7 +9220,7 @@ describe("runnerd provider runtime wiring", () => { execution: currentExecution, runnerInstanceId: "runner-after-running-prior-scope", }), - ).rejects.toThrow("runner_state_identity_mismatch"); + ).rejects.toThrow("runner_state_identity_mismatch: prior_owner_active"); await expect(access(scopedRoot)).resolves.toBeUndefined(); await expect(access(join(stateBase, "quarantine"))).rejects.toThrow(); expect(state.createBackend).not.toHaveBeenCalled(); @@ -10738,7 +10792,7 @@ describe("runnerd provider runtime wiring", () => { } // Ambiguous ordinary recovery must still fail closed. Only the runtime's // explicitly admitted replacement may retire these prior-session backups. - await expect(prepareReplacement()).rejects.toThrow("runner_harness_state_mismatch"); + await expect(prepareReplacement()).rejects.toThrow("runner_harness_state_mismatch: backup_without_reusable_lease"); await expect(backend.openReplacementSession!({ identity: { runId: execution.binding.runId }, workingDirectory: execution.workspace.cwd, } as never, {} as never)).resolves.toBe(replacement); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 9e714a7cea..b537094a12 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -1705,7 +1705,7 @@ function migrateLegacyRunnerdStateRoot(input: { // A legacy path does not encode the full session scope. A mismatch may be // valid live state owned by another agent/workspace, so refusing the claim // is safe but moving that ambiguous directory is not. - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: legacy_owner_unverified"); } if ( exactRun && @@ -1718,7 +1718,7 @@ function migrateLegacyRunnerdStateRoot(input: { ) ) { quarantineRunnerdStateRoot(input.legacy, "identity_indeterminate"); - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: legacy_authority_indeterminate"); } try { renameSync(input.legacy, input.scoped); @@ -1742,7 +1742,7 @@ function migrateLegacyRunnerdStateRoot(input: { durableIdentityMatchesSession(scopedIdentity, input.execution), ); if (!exactScopedRun && !sameVerifiedPriorRun) { - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: migration_destination_owner_changed"); } } return input.scoped; @@ -4587,7 +4587,7 @@ async function migrateRunnerdStateRootForExecution(input: { await recoverQuiescentRunnerdState({ ...input, scoped }); } if (input.restartRecovery?.kind === "reattach_remote_runner" && !existsSync(scoped)) { - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: remote_reattach_root_missing"); } if (existsSync(scoped)) { if (!isSafeNativeStateDirectory(scoped)) { @@ -4607,14 +4607,14 @@ async function migrateRunnerdStateRootForExecution(input: { input.restartRecovery?.kind !== "reattach_remote_runner") { quarantineRunnerdStateRoot(scoped, "identity_indeterminate"); } - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: durable_identity_unreadable"); } if (!durableIdentityMatchesSession(identity, input.execution)) { if (input.restartRecovery?.kind !== "reattach_existing_runner" && input.restartRecovery?.kind !== "reattach_remote_runner") { quarantineRunnerdStateRoot(scoped, "identity_mismatch"); } - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: session_scope_mismatch"); } if (input.restartRecovery?.kind === "bootstrap_incomplete") { if (runnerdStateProvesIncompleteBootstrap(scoped)) { @@ -4624,7 +4624,7 @@ async function migrateRunnerdStateRootForExecution(input: { // Database evidence alone cannot distinguish a never-connected runner // from a partially-persisted provider bootstrap. Only the durable PRP // root can authorize a fresh bootstrap; anything else stays fail-closed. - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: bootstrap_not_proven_incomplete"); } if (input.restartRecovery?.kind === "reattach_remote_runner") { await verifyRemoteRunnerReattachment({ @@ -4645,7 +4645,7 @@ async function migrateRunnerdStateRootForExecution(input: { if (input.restartRecovery?.kind !== "reattach_existing_runner") { quarantineRunnerdStateRoot(scoped, "identity_indeterminate"); } - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: authority_indeterminate"); } } else { const verification = await verifyPriorRunnerdStateForSessionScope({ @@ -4668,7 +4668,7 @@ async function migrateRunnerdStateRootForExecution(input: { : "identity_indeterminate", ); } - throw new Error("runner_state_identity_mismatch"); + throw new Error(`runner_state_identity_mismatch: prior_owner_${verification}`); } } return; @@ -4686,7 +4686,7 @@ async function migrateRunnerdStateRootForExecution(input: { // Unlike the full-scope target above, this legacy name can legitimately // belong to another scope. Leave it in place for its owner and fail the // attempted migration visibly. - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: legacy_session_scope_mismatch"); } let verifiedPriorRunId: string | undefined; if (!durableIdentityMatchesExecution(identity, input.execution)) { @@ -4709,7 +4709,7 @@ async function migrateRunnerdStateRootForExecution(input: { // untouched because the legacy name may still belong to them. quarantineRunnerdStateRoot(legacy, "identity_indeterminate"); } - throw new Error("runner_state_identity_mismatch"); + throw new Error(`runner_state_identity_mismatch: legacy_prior_owner_${verification}`); } verifiedPriorRunId = identity.runId; } @@ -4759,7 +4759,7 @@ async function recoverQuiescentRunnerdState(input: { ) { // Do not roll back to an older valid checkpoint when a newer quarantined // root contains unconfirmed work, even if the newer root is unreadable. - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: quarantine_candidates_ambiguous"); } const verified: Array<{ root: string; @@ -4946,7 +4946,7 @@ async function recoverQuiescentRunnerdState(input: { ) { // Known provider history is not permission to start a replacement when // recovery cannot prove a unique, settled owner. - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: quarantine_owner_unverified"); } return; } @@ -4965,14 +4965,14 @@ async function recoverQuiescentRunnerdState(input: { "recovery_state_too_large", ).toString("utf8") !== expected ) { - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: recovery_evidence_changed"); } } if ( !localProcessDefinitelyGone(candidate.processPid) || !localProcessDefinitelyGone(candidate.processGroupId, true) ) { - throw new Error("runner_state_identity_mismatch"); + throw new Error("runner_state_identity_mismatch: recovery_process_not_gone"); } if (candidate.root !== input.scoped) { if (existsSync(input.scoped)) { @@ -5687,12 +5687,12 @@ export function buildNativeHarnessBackupManifest(input: { completedAt?: string; }): NativeHarnessBackupManifest { if (!providerSessionIdentityIsPresent(input.providerSessionIdentity)) { - throw new Error("runner_harness_state_mismatch"); + throw new Error("runner_harness_state_mismatch: backup_provider_identity_missing"); } const profile = resolveNativeHarnessPersistenceProfile(input.execution); const directories = profile.directories.map((directory) => { const path = resolve(input.backupRoot, directory.name); - if (!existsSync(path)) throw new Error("runner_harness_state_mismatch"); + if (!existsSync(path)) throw new Error("runner_harness_state_mismatch: backup_directory_missing"); return { name: directory.name, ...digestBackupDirectory(path) }; }); return { @@ -9399,14 +9399,22 @@ export function readRemoteProviderPackManifest( readFileSync(resolve(packRoot, "provider-pack.json"), "utf8"), ) as RemoteProviderPackManifest; } catch (error) { + // The terminal run report keeps the outer message, not the cause chain. + // Keep a bounded reason there; raw filesystem errors include private paths. + const code = (error as NodeJS.ErrnoException | null)?.code; + const reason = error instanceof SyntaxError ? "invalid_json" + : code === "ENOENT" ? "missing" + : code === "EACCES" || code === "EPERM" ? "permission_denied" + : code === "EISDIR" || code === "ENOTDIR" ? "invalid_path_type" + : "io_error"; throw new Error( - "runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable", + `runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable (${reason})`, { cause: error }, ); } const payload = manifest?.payload; if ( - manifest.schema !== REMOTE_PROVIDER_PACK_SCHEMA || + manifest?.schema !== REMOTE_PROVIDER_PACK_SCHEMA || !payload || canonicalJson(payload.pins) !== canonicalJson(REMOTE_PROVIDER_PACK_PINS) || canonicalJson(payload.acpxProfileDigests) !== @@ -11601,7 +11609,7 @@ async function createRunnerdBackendWithinSessionClaim( async () => { for (const directory of persistenceProfile.directories) { const targetPath = remotePersistencePath(directory); - if (!targetPath) throw new Error("runner_harness_state_mismatch"); + if (!targetPath) throw new Error("runner_harness_state_mismatch: restore_target_unavailable"); await stageRemoteRunnerDirectory({ target: remoteTarget, runner: remoteCommandRunner, @@ -11629,7 +11637,7 @@ async function createRunnerdBackendWithinSessionClaim( canonicalJson(restored.providerSessionIdentity) !== canonicalJson(backup.manifest.providerSessionIdentity) ) { - throw new Error("runner_harness_state_mismatch"); + throw new Error("runner_harness_state_mismatch: restored_provider_identity_changed"); } // A deliberately non-reusable environment receives a fresh provider lease // for every turn. Stamp that new lease as soon as the verified host backup @@ -11644,7 +11652,7 @@ async function createRunnerdBackendWithinSessionClaim( for (const directory of persistenceProfile.directories) { if (directory.location !== "filesystem") continue; const targetPath = remotePersistencePath(directory); - if (!targetPath) throw new Error("runner_harness_state_mismatch"); + if (!targetPath) throw new Error("runner_harness_state_mismatch: bootstrap_target_unavailable"); const escapedTarget = targetPath.replaceAll("'", "'\\''"); const created = await remoteCommandRunner.execute({ command: "sh", @@ -11824,7 +11832,7 @@ async function createRunnerdBackendWithinSessionClaim( // A continuation that has a durable backup but no recorded reusable // lease was not provider-confirmed lost. Never silently create a new // provider session from that ambiguous state. - throw new Error("runner_harness_state_mismatch"); + throw new Error("runner_harness_state_mismatch: backup_without_reusable_lease"); } } } else if (remoteTarget && remoteCommandRunner) { @@ -12008,7 +12016,7 @@ async function createRunnerdBackendWithinSessionClaim( !verified.runnerState || !verified.providerSessionIdentity ) { - throw new Error("runner_harness_state_mismatch"); + throw new Error("runner_harness_state_mismatch: checkpoint_identity_incomplete"); } const providerSessionIdentity = verified.providerSessionIdentity; @@ -12023,7 +12031,7 @@ async function createRunnerdBackendWithinSessionClaim( for (const directory of persistenceProfile.directories) { const sourcePath = remotePersistencePath(directory); if (!sourcePath) - throw new Error("runner_harness_state_mismatch"); + throw new Error("runner_harness_state_mismatch: checkpoint_source_unavailable"); const targetPath = resolve(pendingRoot, directory.name); await syncRemoteRunnerDirectoryOut({ runner: remoteCommandRunner, @@ -12033,7 +12041,7 @@ async function createRunnerdBackendWithinSessionClaim( excludeEntries: directory.excludeEntries, }); if (!existsSync(targetPath)) { - throw new Error("runner_harness_state_mismatch"); + throw new Error("runner_harness_state_mismatch: checkpoint_directory_missing"); } } const manifest = buildNativeHarnessBackupManifest({ diff --git a/server/src/services/native-runtime/paperclip-runner-tool-authority.ts b/server/src/services/native-runtime/paperclip-runner-tool-authority.ts index dab0e16d0f..1b46dfe55d 100644 --- a/server/src/services/native-runtime/paperclip-runner-tool-authority.ts +++ b/server/src/services/native-runtime/paperclip-runner-tool-authority.ts @@ -1700,7 +1700,7 @@ export class PaperclipRunnerToolAuthority { acceptLabel: normalizedPayload.acceptLabel ?? "Confirm", rejectLabel: normalizedPayload.rejectLabel ?? "Request changes", rejectRequiresReason: normalizedPayload.rejectRequiresReason ?? false, - supersedeOnUserComment: normalizedPayload.supersedeOnUserComment ?? true, + supersedeOnUserComment: normalizedPayload.supersedeOnUserComment ?? false, } : {}), }, } as never, { agentId: this.binding.agentId, userId: null, identityContextId }); diff --git a/server/src/services/run-identity.ts b/server/src/services/run-identity.ts index dfc5f5bb89..496b58719d 100644 --- a/server/src/services/run-identity.ts +++ b/server/src/services/run-identity.ts @@ -57,7 +57,13 @@ export async function explicitOperatorRunIdentity( export type RunIdentityContext = typeof runIdentityContexts.$inferSelect; type Executor = Pick; -/** Match task mutation ordering: lock the task before the run, never the reverse. */ +/** + * Lock the task before the run, matching task mutation ordering. Identity + * operations do not change parent keys: NO KEY UPDATE still serializes writers + * and steering, while allowing audit inserts to check their foreign keys. + * FOR UPDATE can deadlock with an append that holds KEY SHARE on the run and + * then checks the task while identity capture holds the task and waits on the run. + */ async function lockIdentityTask( executor: Pick, companyId: string, @@ -85,7 +91,7 @@ async function lockIdentityTask( .select({ id: issues.id }) .from(issues) .where(and(eq(issues.id, issueId), eq(issues.companyId, companyId))) - .for("update"); + .for("no key update"); } async function append( @@ -181,7 +187,7 @@ export async function initializeRunIdentity( eq(heartbeatRuns.companyId, input.companyId), ), ) - .for("update"); + .for("no key update"); if (!run) throw forbidden("Run identity does not belong to this company"); if (run.activeIdentityContextId) { const [current] = await tx @@ -345,7 +351,7 @@ export async function reserveSteeredIdentity( eq(heartbeatRuns.companyId, input.companyId), ), ) - .for("update"); + .for("no key update"); // Processes started before the broker rollout keep their original environment. if (!run?.activeIdentityContextId) return null; const [pending] = await tx @@ -439,7 +445,7 @@ export async function captureRunIdentity( eq(heartbeatRuns.agentId, input.agentId), ), ) - .for("update"); + .for("no key update"); if (!run || run.status !== "running") throw forbidden( "Credential acquisition requires this agent's active run", @@ -512,7 +518,7 @@ export async function reconcileSteeredIdentity( eq(heartbeatRuns.companyId, context.companyId), ), ) - .for("update"); + .for("no key update"); if (!run) return; await acceptSteeredIdentity(tx, context); }); diff --git a/server/src/services/run-log-store-cancellation.test.ts b/server/src/services/run-log-store-cancellation.test.ts new file mode 100644 index 0000000000..36d405d132 --- /dev/null +++ b/server/src/services/run-log-store-cancellation.test.ts @@ -0,0 +1,72 @@ +import { createServer } from "node:http"; +import { promises as fs } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createDurableRunLogStore } from "./run-log-store.js"; +import { createS3StorageProvider } from "../storage/s3-provider.js"; + +afterEach(() => vi.unstubAllEnvs()); + +describe("run-log read cancellation", () => { + it.each(["head", "get", "body"])("closes a stalled S3 %s connection and permits a subsequent read", async (stage) => { + // Exercise the real SDK against an on-host server. No provider credentials + // or external network are used by this cancellation regression. + vi.stubEnv("AWS_ACCESS_KEY_ID", "test-access-key"); + vi.stubEnv("AWS_SECRET_ACCESS_KEY", "test-secret-key"); + vi.stubEnv("AWS_SESSION_TOKEN", ""); + const basePath = await fs.mkdtemp(path.join(os.tmpdir(), "run-log-abort-")); + let recover = false; + let closed = false; + let started!: () => void; + const stalled = new Promise((resolve) => { started = resolve; }); + const server = createServer((request, response) => { + const shouldStall = !recover && (stage === "head" ? request.method === "HEAD" : request.method === "GET"); + if (shouldStall) { + response.on("close", () => { closed = true; }); + if (stage === "body") { + response.writeHead(200, { "Content-Length": "4" }); + response.write("d"); + } + started(); + return; + } + response.writeHead(200, { "Content-Length": "4" }); + response.end(request.method === "HEAD" ? undefined : "data"); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Test server did not bind"); + const provider = createS3StorageProvider({ + bucket: "test-bucket", region: "us-east-1", forcePathStyle: true, + endpoint: `http://127.0.0.1:${address.port}`, + }); + const store = createDurableRunLogStore({ basePath, s3: { provider } }); + const handle = { store: "local_file" as const, logRef: "missing.ndjson" }; + const controller = new AbortController(); + const read = store.read(handle, { signal: controller.signal }); + void read.catch(() => {}); + try { + await stalled; + controller.abort(); + await expect(read).rejects.toMatchObject({ name: "AbortError" }); + await vi.waitFor(() => expect(closed).toBe(true)); + recover = true; + expect(await store.read(handle)).toEqual({ content: "data", nextOffset: undefined }); + } finally { + controller.abort(); + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + await fs.rm(basePath, { recursive: true, force: true }); + } + }, 10_000); + + it("rejects an already-cancelled local read before opening a file", async () => { + const store = createDurableRunLogStore({ basePath: os.tmpdir() }); + const controller = new AbortController(); + controller.abort(); + await expect(store.read({ store: "local_file", logRef: "unused.ndjson" }, { + signal: controller.signal, + })).rejects.toMatchObject({ name: "AbortError" }); + }); +}); diff --git a/server/src/services/run-log-store.test.ts b/server/src/services/run-log-store.test.ts index ab12ae17b3..ccadb78c30 100644 --- a/server/src/services/run-log-store.test.ts +++ b/server/src/services/run-log-store.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { promises as fs } from "node:fs"; import path from "node:path"; import os from "node:os"; +import { createHash } from "node:crypto"; import { Readable } from "node:stream"; import { createDurableRunLogStore } from "./run-log-store.js"; import type { StorageProvider } from "../storage/types.js"; @@ -100,6 +101,102 @@ describe("createDurableRunLogStore", () => { expect(objects.get(key)!.toString("utf8")).toContain("line-B"); }); + it.each(["finishes", "fails"])("waits for an accepted file append that %s before finalizing", async (outcome) => { + const { provider, objects } = createMemoryProvider(); + const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider } }); + const handle = await store.begin(begin); + let release!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + const appendFile = fs.appendFile.bind(fs); + const spy = vi.spyOn(fs, "appendFile").mockImplementationOnce(async (...args) => { + await gate; + if (outcome === "fails") throw new Error("disk unavailable"); + await appendFile(...args); + }); + const append = store.append(handle, { stream: "stderr", chunk: "late diagnostic", ts: "t1" }); + // Observe the deliberate rejection independently of finalization. + void append.catch(() => {}); + let finalized = false; + const finalize = store.finalize(handle).then((summary) => { + finalized = true; + return summary; + }); + try { + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(finalized).toBe(false); + release(); + if (outcome === "fails") await expect(append).rejects.toThrow("disk unavailable"); + else await append; + const summary = await finalize; + const local = await fs.readFile(path.join(baseDir, handle.logRef)); + expect(summary.bytes).toBe(local.length); + expect(summary.sha256).toBe(createHash("sha256").update(local).digest("hex")); + expect(objects.get(handle.logRef)).toEqual(local); + expect(local.toString()).toBe(outcome === "fails" ? "" : JSON.stringify({ + ts: "t1", stream: "stderr", chunk: "late diagnostic", + }) + "\n"); + } finally { + release(); + await append.catch(() => {}); + await finalize; + spy.mockRestore(); + } + }); + + it("ignores appends once finalization starts so the durable snapshot stays immutable", async () => { + const { provider, objects } = createMemoryProvider(); + const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider } }); + const handle = await store.begin(begin); + await store.append(handle, { stream: "stdout", chunk: "accepted", ts: "t1" }); + const finalize = store.finalize(handle); + expect(await store.append(handle, { stream: "stderr", chunk: "too late", ts: "t2" })).toBe(0); + const summary = await finalize; + expect(await store.append(handle, { stream: "stderr", chunk: "also too late", ts: "t3" })).toBe(0); + const local = await fs.readFile(path.join(baseDir, handle.logRef)); + expect(local.toString()).not.toContain("too late"); + expect(summary.bytes).toBe(local.length); + expect(summary.sha256).toBe(createHash("sha256").update(local).digest("hex")); + expect(objects.get(handle.logRef)).toEqual(local); + }); + + it("leaves final metadata unknown when an append stalls and never mirrors its late completion", async () => { + const { provider, calls } = createMemoryProvider(); + const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider, inflightMirrorMs: 10_000 } }); + const handle = await store.begin(begin); + let release!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + const appendFile = fs.appendFile.bind(fs); + const spy = vi.spyOn(fs, "appendFile").mockImplementationOnce(async (...args) => { + await gate; + await appendFile(...args); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const append = store.append(handle, { stream: "stderr", chunk: "stalled diagnostic", ts: "t1" }); + let summary: Awaited> | undefined; + const finalize = store.finalize(handle).then((result) => { summary = result; }); + try { + await vi.advanceTimersByTimeAsync(3_000); + expect(summary).toEqual({ bytes: null, sha256: null, compressed: false }); + expect(warn).toHaveBeenCalled(); + expect(calls.put).toBe(0); + release(); + await append; + await vi.advanceTimersByTimeAsync(20_000); + await store.flushInflightMirrors!(); + expect(calls.put).toBe(0); + expect(await store.finalize(handle)).toEqual(summary); + expect(await store.append(handle, { stream: "stderr", chunk: "too late", ts: "t2" })).toBe(0); + } finally { + release(); + await append; + await finalize; + vi.useRealTimers(); + spy.mockRestore(); + warn.mockRestore(); + } + }); + it("falls back to S3 when the local file is gone (the pod-roll case that caused 'Run log not found')", async () => { const { provider } = createMemoryProvider(); const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider, keyPrefix: "run-logs" } }); @@ -163,25 +260,15 @@ describe("createDurableRunLogStore", () => { expect(caughtUp.nextOffset).toBeUndefined(); }); - it("falls back to S3 when the local file vanishes between stat() and open (TOCTOU race)", async () => { - const { provider } = createMemoryProvider(); - const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider, keyPrefix: "run-logs" } }); + it("reads local pages without waiting for a separate metadata request", async () => { + const store = createDurableRunLogStore({ basePath: baseDir }); const handle = await store.begin(begin); - await store.append(handle, { stream: "stdout", chunk: "raced-line", ts: "t1" }); - await store.finalize(handle); - // Delete the local file DURING stat(), i.e. after it reports the file - // present but before createReadStream opens it -> the open hits ENOENT. - const realStat = fs.stat.bind(fs); - const statSpy = vi.spyOn(fs, "stat").mockImplementation(async (target, ...rest) => { - const result = await realStat(target as Parameters[0], ...(rest as [])); - if (String(target).endsWith(".ndjson")) { - await fs.rm(target as string, { force: true }); - } - return result; - }); + await fs.writeFile(path.join(baseDir, handle.logRef), "0123456789"); + const statSpy = vi.spyOn(fs, "stat").mockRejectedValue(new Error("Metadata unavailable")); try { - const res = await store.read(handle); - expect(res.content).toContain("raced-line"); + expect(await store.read(handle, { offset: 2, limitBytes: 4 })).toEqual({ content: "2345", nextOffset: 6 }); + expect(await store.read(handle, { offset: 6, limitBytes: 4 })).toEqual({ content: "6789", nextOffset: undefined }); + expect(await store.read(handle, { offset: 20, limitBytes: 4 })).toEqual({ content: "", nextOffset: undefined }); } finally { statSpy.mockRestore(); } diff --git a/server/src/services/run-log-store.ts b/server/src/services/run-log-store.ts index f07ede9abd..7e7b4597ea 100644 --- a/server/src/services/run-log-store.ts +++ b/server/src/services/run-log-store.ts @@ -1,6 +1,7 @@ import { createReadStream, promises as fs } from "node:fs"; import path from "node:path"; import { createHash } from "node:crypto"; +import { addAbortSignal } from "node:stream"; import { notFound } from "../errors.js"; import { resolvePaperclipInstanceRoot } from "../home-paths.js"; import { createS3StorageProvider } from "../storage/s3-provider.js"; @@ -16,6 +17,7 @@ export interface RunLogHandle { export interface RunLogReadOptions { offset?: number; limitBytes?: number; + signal?: AbortSignal; } export interface RunLogReadResult { @@ -24,8 +26,10 @@ export interface RunLogReadResult { } export interface RunLogFinalizeSummary { - bytes: number; - sha256?: string; + // Null means a stalled write prevented a verified final snapshot. Callers + // can still settle the run without recording a false byte count or hash. + bytes: number | null; + sha256?: string | null; compressed: boolean; } @@ -100,6 +104,13 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru const s3 = options.s3; const s3Prefix = normalizeKeyPrefix(s3?.keyPrefix); const inflightMirrorMs = s3?.inflightMirrorMs && s3.inflightMirrorMs > 0 ? s3.inflightMirrorMs : 0; + // A run owns the write handle returned by begin(). Diagnostics can be + // dispatched without awaiting the rest of onLog (DB progress/live events), + // but finalize must include every file append already accepted on that handle. + // Weak collections let completed handles disappear with their owning runs. + const pendingAppends = new WeakMap>>(); + const closingHandles = new WeakSet(); + const abandonedHandles = new WeakSet(); function s3Key(logRef: string): string { return s3Prefix ? `${s3Prefix}/${logRef}` : logRef; @@ -159,6 +170,7 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru } function scheduleInflightMirror(logRef: string, entry: InflightMirrorEntry): void { + if (inflightMirrors.get(logRef) !== entry) return; if (entry.timer || entry.upload) return; const delay = Math.max(0, inflightMirrorMs - (Date.now() - entry.lastMirrorAt)); entry.timer = setTimeout(() => { @@ -205,33 +217,28 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru filePath: string, offset: number, limitBytes: number, + signal?: AbortSignal, ): Promise { - const stat = await fs.stat(filePath).catch(() => null); - if (!stat) return null; - const start = Math.max(0, Math.min(offset, stat.size)); - // No lower clamp to `start`: when the reader is fully caught up - // (offset === size) that clamp made end === start and produced a - // 1-byte-past-EOF range instead of an empty read. - const end = Math.min(start + limitBytes - 1, stat.size - 1); - if (start > end) return { content: "", nextOffset: start < stat.size ? start : undefined }; - + signal?.throwIfAborted(); + const start = Math.max(0, offset); + // Read one extra byte to discover whether another page exists. A single + // abortable stream avoids an uncancellable stat before opening the file. const chunks: Buffer[] = []; try { - await new Promise((resolve, reject) => { - const stream = createReadStream(filePath, { start, end }); - stream.on("data", (chunk) => chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))); - stream.on("error", reject); - stream.on("end", () => resolve()); - }); + const stream = createReadStream(filePath, { start, end: start + limitBytes, signal }); + for await (const chunk of stream) { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + } } catch (err) { - // File deleted between stat() and open (pod-roll cleanup racing a read): - // treat as missing so the caller falls through to the S3 mirror instead - // of surfacing the very "Run log not found" this store exists to prevent. + signal?.throwIfAborted(); + // A missing file, including deletion before open, falls back to S3. if ((err as NodeJS.ErrnoException | null)?.code === "ENOENT") return null; throw err; } - const content = Buffer.concat(chunks).toString("utf8"); - const nextOffset = end + 1 < stat.size ? end + 1 : undefined; + signal?.throwIfAborted(); + const bytes = Buffer.concat(chunks); + const content = bytes.subarray(0, limitBytes).toString("utf8"); + const nextOffset = bytes.length > limitBytes ? start + limitBytes : undefined; return { content, nextOffset }; } @@ -239,10 +246,13 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru logRef: string, offset: number, limitBytes: number, + signal?: AbortSignal, ): Promise { + signal?.throwIfAborted(); if (!s3) throw notFound("Run log not found"); const key = s3Key(logRef); - const head = await s3.provider.headObject({ objectKey: key }); + const head = await s3.provider.headObject({ objectKey: key, signal }); + signal?.throwIfAborted(); if (!head.exists) throw notFound("Run log not found"); const total = head.contentLength ?? 0; const start = Math.max(0, Math.min(offset, total)); @@ -253,13 +263,15 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru const end = Math.min(start + limitBytes - 1, total - 1); if (total === 0 || start > end) return { content: "", nextOffset: start < total ? start : undefined }; - const result = await s3.provider.getObject({ objectKey: key, range: { start, end } }); + const result = await s3.provider.getObject({ objectKey: key, range: { start, end }, signal }); + // Destroy a body that stalls after headers arrive, including a body + // returned just after the caller cancelled the request. + if (signal) addAbortSignal(signal, result.stream); const chunks: Buffer[] = []; - await new Promise((resolve, reject) => { - result.stream.on("data", (chunk) => chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))); - result.stream.on("error", reject); - result.stream.on("end", () => resolve()); - }); + for await (const chunk of result.stream) { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + } + signal?.throwIfAborted(); const content = Buffer.concat(chunks).toString("utf8"); const nextOffset = end + 1 < total ? end + 1 : undefined; return { content, nextOffset }; @@ -289,7 +301,7 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru }, async append(handle, event) { - if (handle.store !== "local_file") return 0; + if (handle.store !== "local_file" || closingHandles.has(handle)) return 0; const absPath = resolveWithin(basePath, handle.logRef); const line = JSON.stringify({ ts: event.ts, @@ -301,13 +313,47 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru ...(typeof event.seq === "number" && Number.isFinite(event.seq) ? { seq: event.seq } : {}), }); const persisted = `${line}\n`; - await fs.appendFile(absPath, persisted, "utf8"); - noteInflightAppend(handle.logRef); + let pending = pendingAppends.get(handle); + if (!pending) { + pending = new Set(); + pendingAppends.set(handle, pending); + } + const write = fs.appendFile(absPath, persisted, "utf8").then(() => { + if (!closingHandles.has(handle)) noteInflightAppend(handle.logRef); + }); + pending.add(write); + try { + await write; + } finally { + pending.delete(write); + } return Buffer.byteLength(persisted, "utf8"); }, async finalize(handle) { if (handle.store !== "local_file") return { bytes: 0, compressed: false }; + if (abandonedHandles.has(handle)) return { bytes: null, sha256: null, compressed: false }; + // Close admission before the first await. Drain file writes only, not + // heartbeat's later DB/live-event persistence, then freeze one consistent + // byte count, hash, and durable copy. Late diagnostics cannot mutate it. + closingHandles.add(handle); + let drainTimer: NodeJS.Timeout | undefined; + const drained = await Promise.race([ + Promise.allSettled(pendingAppends.get(handle) ?? []).then(() => true), + new Promise((resolve) => { + drainTimer = setTimeout(() => resolve(false), 3_000); + drainTimer.unref?.(); + }), + ]).finally(() => clearTimeout(drainTimer)); + if (!drained) { + // An in-flight fs append cannot be cancelled safely. Do not hash or + // mirror a file it may still change, and never re-arm mirroring when + // that write eventually finishes. Terminal run status can still settle. + abandonedHandles.add(handle); + void retireInflightMirror(handle.logRef).catch(() => undefined); + console.warn("[run-log-store] Pending log writes did not settle within 3000ms; final log size and hash are unknown."); + return { bytes: null, sha256: null, compressed: false }; + } await retireInflightMirror(handle.logRef); const absPath = resolveWithin(basePath, handle.logRef); const stat = await fs.stat(absPath).catch(() => null); @@ -344,14 +390,15 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru }, async read(handle, opts) { + opts?.signal?.throwIfAborted(); if (handle.store !== "local_file") throw notFound("Run log not found"); const absPath = resolveWithin(basePath, handle.logRef); const offset = opts?.offset ?? 0; const limitBytes = opts?.limitBytes ?? 256_000; - const local = await readLocalRange(absPath, offset, limitBytes); + const local = await readLocalRange(absPath, offset, limitBytes, opts?.signal); if (local) return local; // Local file gone (pod rolled) -> serve from the S3 mirror if configured. - return readS3Range(handle.logRef, offset, limitBytes); + return readS3Range(handle.logRef, offset, limitBytes, opts?.signal); }, async flushInflightMirrors() { diff --git a/server/src/storage/s3-provider.ts b/server/src/storage/s3-provider.ts index 517ccf23d9..4939bb9b25 100644 --- a/server/src/storage/s3-provider.ts +++ b/server/src/storage/s3-provider.ts @@ -6,7 +6,7 @@ import { PutObjectCommand, } from "@aws-sdk/client-s3"; import { putS3Multipart } from "./s3-multipart.js"; -import { Readable } from "node:stream"; +import { addAbortSignal, Readable } from "node:stream"; import type { StorageProvider, GetObjectResult, HeadObjectResult } from "./types.js"; import { notFound, unprocessable } from "../errors.js"; @@ -106,10 +106,13 @@ export function createS3StorageProvider(config: S3ProviderConfig): StorageProvid Key: key, Range: input.range ? `bytes=${input.range.start}-${input.range.end}` : undefined, }), + { abortSignal: input.signal }, ); + const stream = await toReadableStream(output.Body); + if (input.signal) addAbortSignal(input.signal, stream); return { - stream: await toReadableStream(output.Body), + stream, contentType: output.ContentType, contentLength: output.ContentLength, etag: output.ETag, @@ -130,6 +133,7 @@ export function createS3StorageProvider(config: S3ProviderConfig): StorageProvid Bucket: bucket, Key: key, }), + { abortSignal: input.signal }, ); return { diff --git a/server/src/storage/types.ts b/server/src/storage/types.ts index 15289f77a5..30f0845b5b 100644 --- a/server/src/storage/types.ts +++ b/server/src/storage/types.ts @@ -12,6 +12,8 @@ export interface PutObjectInput { export interface GetObjectInput { objectKey: string; + // S3 reads cancel pending requests and their response streams. + signal?: AbortSignal; range?: { start: number; end: number; diff --git a/tests/e2e/agent-chat-projects.spec.ts b/tests/e2e/agent-chat-projects.spec.ts index e9c89067a3..88751f70d3 100644 --- a/tests/e2e/agent-chat-projects.spec.ts +++ b/tests/e2e/agent-chat-projects.spec.ts @@ -246,11 +246,8 @@ for (const mode of ["Ask", "Plan"]) const f = await setup(request); try { await page.goto(f.route); - await page.getByTestId("task-chat-composer-mode").click(); - await page - .getByTestId("task-chat-composer-mode-menu") - .getByText(`${mode} mode`, { exact: true }) - .click(); + await page.getByTestId("task-chat-composer-add").click(); + await page.getByTestId(mode === "Plan" ? "composer-add-plan" : "composer-add-ask").click(); await send(page, { action: "project", name: "Forbidden mutation" }); await idle(request, f.chatPath); expect( @@ -387,11 +384,8 @@ test("plan approval hands the preserved revision to an assigned project task", a const f = await setup(request); try { await page.goto(f.route); - await page.getByTestId("task-chat-composer-mode").click(); - await page - .getByTestId("task-chat-composer-mode-menu") - .getByText("Plan mode", { exact: true }) - .click(); + await page.getByTestId("task-chat-composer-add").click(); + await page.getByTestId("composer-add-plan").click(); await send(page, { action: "plan", text: "# Approved welcome\nWrite two friendly sentences.", diff --git a/tests/e2e/artifact-tab-arrival.spec.ts b/tests/e2e/artifact-tab-arrival.spec.ts index a04a54f9b3..e4f29c6adb 100644 --- a/tests/e2e/artifact-tab-arrival.spec.ts +++ b/tests/e2e/artifact-tab-arrival.spec.ts @@ -50,7 +50,7 @@ for (const mobile of [false, true]) { await expect(artifacts).toBeVisible(); await expect(artifacts).toHaveAttribute("aria-selected", "false"); await artifacts.click(); - await expect(panel.getByText("Arriving report", { exact: true })).toBeVisible(); + await expect(panel.getByRole("heading", { name: "Arriving report", level: 2, exact: true })).toBeVisible(); await page.screenshot({ path: testInfo.outputPath("artifact-opened-by-user.png"), fullPage: true }); }); } diff --git a/tests/e2e/board-attachment-receipts.spec.ts b/tests/e2e/board-attachment-receipts.spec.ts index 3d0deb442d..de7fb5cbde 100644 --- a/tests/e2e/board-attachment-receipts.spec.ts +++ b/tests/e2e/board-attachment-receipts.spec.ts @@ -3,6 +3,7 @@ import { expect, test, type APIRequestContext, + type Locator, type Page, } from "@playwright/test"; @@ -95,15 +96,19 @@ const files = [ }, ]; +async function openAttachmentChooser(page: Page, composer: Locator) { + await composer.getByRole("button", { name: "Add to composer" }).click(); + const chooser = page.waitForEvent("filechooser"); + await page.getByRole("menuitem", { name: "Files and images", exact: true }).click(); + return chooser; +} + async function upload( page: Page, fixture: Awaited>, file: (typeof files)[number], ) { - const chooser = page.waitForEvent("filechooser"); - await fixture.composer - .getByRole("button", { name: "Attach file", exact: true }) - .click(); + const chooser = await openAttachmentChooser(page, fixture.composer); const response = page.waitForResponse( (res) => res.request().method() === "POST" && @@ -111,7 +116,7 @@ async function upload( `/issues/${fixture.issue.id}/attachments`, ), ); - await (await chooser).setFiles(file); + await chooser.setFiles(file); const receipt = await body(await response); await expect .poll(async () => @@ -376,11 +381,8 @@ for (const classic of [false, true]) await route.fulfill({ response }); }, ); - const chooser = page.waitForEvent("filechooser"); - await fixture.composer - .getByRole("button", { name: "Attach file", exact: true }) - .click(); - await (await chooser).setFiles(files[1]!); + const chooser = await openAttachmentChooser(page, fixture.composer); + await chooser.setFiles(files[1]!); await expect.poll(() => arrived).toBe(true); await expect(fixture.send).toBeDisabled(); try { @@ -430,11 +432,8 @@ test("legacy failed upload can be removed before sending the retained text", asy body: JSON.stringify({ error: "Fixture upload rejected" }), }), ); - const chooser = page.waitForEvent("filechooser"); - await fixture.composer - .getByRole("button", { name: "Attach file", exact: true }) - .click(); - await (await chooser).setFiles(files[0]!); + const chooser = await openAttachmentChooser(page, fixture.composer); + await chooser.setFiles(files[0]!); await expect( fixture.composer.getByText("Fixture upload rejected", { exact: true }), ).toBeVisible(); diff --git a/tests/e2e/chat-adapters-ui-messaging.spec.ts b/tests/e2e/chat-adapters-ui-messaging.spec.ts index 97ce3997b9..de11726d12 100644 --- a/tests/e2e/chat-adapters-ui-messaging.spec.ts +++ b/tests/e2e/chat-adapters-ui-messaging.spec.ts @@ -275,13 +275,14 @@ test.describe("Board send delivery refresh", () => { "base64", ), }; + if (!classic) { + await page.getByRole("button", { name: "Add to composer" }).click(); + } const chooserPromise = page.waitForEvent("filechooser"); - await page - .getByRole("button", { - name: classic ? "Upload attachment" : "Attach file", - exact: true, - }) - .click(); + await (classic + ? page.getByRole("button", { name: "Upload attachment", exact: true }) + : page.getByRole("menuitem", { name: "Files and images", exact: true }) + ).click(); const responsePromise = page.waitForResponse( (response) => response.request().method() === "POST" && diff --git a/tests/e2e/company-context-refresh.spec.ts b/tests/e2e/company-context-refresh.spec.ts new file mode 100644 index 0000000000..309bd841b8 --- /dev/null +++ b/tests/e2e/company-context-refresh.spec.ts @@ -0,0 +1,77 @@ +import path from "node:path"; +import { expect, test } from "@playwright/test"; +import { createServer, type ViteDevServer } from "../../ui/node_modules/vite/dist/node/index.js"; + +let server: ViteDevServer; +let origin: string; +const uiRoot = path.resolve(import.meta.dirname, "../../ui"); +const probePath = path.join(uiRoot, "src/__company_context_probe.tsx"); + +// Use real Vite module instances and React contexts. Re-importing a refreshed +// consumer must still see a provider retained from the previous module version. +test.beforeAll(async () => { + server = await createServer({ + root: uiRoot, + configFile: false, + resolve: { alias: { "@": path.join(uiRoot, "src") } }, + esbuild: { jsx: "automatic" }, + server: { host: "127.0.0.1", port: 0 }, + plugins: [{ + name: "company-context-regression", + resolveId(id) { if (id === "/src/__company_context_probe.tsx") return probePath; }, + load(id) { + if (id !== probePath) return; + return ` + import React from "react"; + import { createRoot } from "react-dom/client"; + import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + import { CompanyProvider, useCompany } from "/src/context/CompanyContext.tsx"; + const root = createRoot(document.getElementById("root")); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + function render(hook, generation) { + function Consumer() { return
{generation + ":" + (hook().selectedCompanyId ?? "loading")}
; } + root.render(); + } + render(useCompany, "initial"); + window.refreshConsumer = async () => { + const refreshed = await import(/* @vite-ignore */ "/src/context/CompanyContext.tsx?t=" + Date.now()); + render(refreshed.useCompany, "refreshed"); + }; + `; + }, + configureServer(vite) { + vite.middlewares.use((req, res, next) => { + if (req.url !== "/") return next(); + res.setHeader("Content-Type", "text/html"); + res.end('
'); + }); + }, + }], + }); + await server.listen(); + const address = server.httpServer!.address(); + if (!address || typeof address === "string") throw new Error("Missing test server address"); + origin = `http://127.0.0.1:${address.port}`; +}); + +test.afterAll(async () => { await server?.close(); }); + +test("a refreshed company consumer reads the retained provider", async ({ page }) => { + await page.route("**/api/**", async (route) => { + const pathname = new URL(route.request().url()).pathname; + if (!pathname.startsWith("/api/")) return route.continue(); + const body = pathname === "/api/auth/get-session" + ? { user: { id: "test-user" }, session: { userId: "test-user" } } + : pathname === "/api/companies" + ? [{ id: "test-company", name: "Test company", status: "active" }] + : []; + await route.fulfill({ json: body }); + }); + const errors: string[] = []; + page.on("pageerror", (error) => { errors.push(error.message); }); + await page.goto(origin); + await expect(page.getByRole("main")).toHaveText("initial:test-company"); + await page.evaluate(() => (window as unknown as { refreshConsumer: () => Promise }).refreshConsumer()); + await expect(page.getByRole("main")).toHaveText("refreshed:test-company"); + expect(errors).toEqual([]); +}); diff --git a/tests/e2e/planning-mode-visual-verification.spec.ts b/tests/e2e/planning-mode-visual-verification.spec.ts index ea53f5fc74..564ae156d9 100644 --- a/tests/e2e/planning-mode-visual-verification.spec.ts +++ b/tests/e2e/planning-mode-visual-verification.spec.ts @@ -12,12 +12,12 @@ const TASK_TITLE = "Paperclip onboarding"; /** * The first task opens with the chief of staff's opening card sitting where * the composer is. Cancel hands the plain composer back (the card stays - * pending), and the composer is where the mode toggle lives. + * pending), and the composer is where the mode chip lives. * * The card arrives with the interactions fetch, after the composer's first * paint, so a bare `count()` right after navigation sees no card and skips * the click; the card then lands on top of the composer and hides the mode - * toggle. Wait for the card (or, if it is already dismissed, the pending + * chip. Wait for the card (or, if it is already dismissed, the pending * strip it leaves behind) before deciding, and only return once the plain * composer is back. */ @@ -176,8 +176,8 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => { await page.goto(issuePath); await dismissOpeningCard(page); await page.getByTestId("task-chat-composer-mode").click(); - await page.getByRole("menuitem", { name: /Auto mode/ }).click(); - await expect(page.getByTestId("task-chat-composer-mode")).toHaveAttribute("data-pending-work-mode", "standard"); + await expect(page.getByTestId("task-chat-composer-mode")).toHaveCount(0); + await expect(page.getByTestId("task-chat-composer-add")).toBeVisible(); await page.screenshot({ path: `${screenshotDir}/desktop-standard-toggle-${timestamp}.png`, fullPage: true, diff --git a/tests/e2e/playwright-company-context.config.ts b/tests/e2e/playwright-company-context.config.ts new file mode 100644 index 0000000000..15c5f3a830 --- /dev/null +++ b/tests/e2e/playwright-company-context.config.ts @@ -0,0 +1,16 @@ +import { defineConfig } from "@playwright/test"; + +export default defineConfig({ + testDir: ".", + testMatch: "company-context-refresh.spec.ts", + workers: 1, + timeout: 30_000, + use: { + headless: true, + ...(process.env.PAPERCLIP_PLAYWRIGHT_CHANNEL + ? { channel: process.env.PAPERCLIP_PLAYWRIGHT_CHANNEL } + : {}), + }, + outputDir: "./test-results", + reporter: "list", +}); diff --git a/tests/runner-e2e/chat-flow.ts b/tests/runner-e2e/chat-flow.ts index 08a5f9c304..f8ae68be14 100644 --- a/tests/runner-e2e/chat-flow.ts +++ b/tests/runner-e2e/chat-flow.ts @@ -693,11 +693,8 @@ export async function runChatFlow(input: ChatFlowInput) { await idle(3); } else await turn(clarification, 3); } else if (caseId === "plan-handoff") { - await page.getByTestId("task-chat-composer-mode").click(); - await page - .getByTestId("task-chat-composer-mode-menu") - .getByText("Plan mode", { exact: true }) - .click(); + await page.getByTestId("task-chat-composer-add").click(); + await page.getByTestId("composer-add-plan").click(); await turn( `Let's plan a two-sentence garden club welcome note. The finished welcome note itself must contain the exact phrase ${draftMarker}. Write a plan in the plan panel that includes this requirement, and present it for approval. When I approve the final revision, create a suitable repository-free project and an assigned task for yourself, copy the plan into that task, and have it save the note as a Paperclip document attached to that execution task and finish. Do not create the project or task before approval.`, 1, diff --git a/ui/src/components/IssueChatThread.test.tsx b/ui/src/components/IssueChatThread.test.tsx index 62e1e686d1..384fa952a3 100644 --- a/ui/src/components/IssueChatThread.test.tsx +++ b/ui/src/components/IssueChatThread.test.tsx @@ -732,20 +732,19 @@ describe("IssueChatThread", () => { expect(composer?.getAttribute("data-pending-work-mode")).toBe("planning"); expect(composer?.className).toContain("amber"); - const toggle = container.querySelector( - '[data-testid="issue-chat-composer-work-mode-toggle"]', + const chip = container.querySelector( + '[data-testid="issue-chat-composer-work-mode-chip"]', ); - expect(toggle).not.toBeNull(); - expect(toggle?.getAttribute("data-pending-work-mode")).toBe("planning"); - expect(toggle?.getAttribute("aria-pressed")).toBe("true"); - expect(toggle?.textContent).toContain("Plan mode"); + expect(chip).not.toBeNull(); + expect(chip?.getAttribute("data-pending-work-mode")).toBe("planning"); + expect(chip?.textContent).toContain("Plan mode"); act(() => { root.unmount(); }); }); - it("shows a persistent neutral mode chip on a standard issue and selects planning through its menu", () => { + it("selects planning from the add menu and removes its chip", () => { const root = createRoot(container); const onWorkModeChange = vi.fn(); @@ -766,13 +765,9 @@ describe("IssueChatThread", () => { ); }); - // The mode chip is always present (mockup rev 5) — neutral "Auto mode" here. - const chip = container.querySelector( - '[data-testid="issue-chat-composer-work-mode-toggle"]', - ) as HTMLButtonElement | null; - expect(chip).not.toBeNull(); - expect(chip?.getAttribute("data-pending-work-mode")).toBe("standard"); - expect(chip?.textContent).toContain("Auto mode"); + expect(container.querySelector('[data-testid="issue-chat-composer-work-mode-chip"]')).toBeNull(); + const add = container.querySelector('[data-testid="issue-chat-composer-add"]') as HTMLButtonElement; + expect(add).not.toBeNull(); const composer = container.querySelector( '[data-testid="issue-chat-composer"]', @@ -781,11 +776,11 @@ describe("IssueChatThread", () => { expect(composer?.className).not.toContain("amber"); act(() => { - chip?.click(); + add.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, button: 0 })); }); const menuItem = document.querySelector( - '[data-testid="issue-chat-composer-work-mode-menu-planning"]', + '[data-testid="composer-add-plan"]', ) as HTMLButtonElement | null; expect(menuItem).not.toBeNull(); expect(menuItem?.textContent).toContain("Plan mode"); @@ -798,7 +793,11 @@ describe("IssueChatThread", () => { expect(onWorkModeChange).not.toHaveBeenCalled(); expect(composer?.getAttribute("data-pending-work-mode")).toBe("planning"); expect(composer?.className).toContain("amber"); + const chip = container.querySelector('[data-testid="issue-chat-composer-work-mode-chip"]') as HTMLButtonElement; expect(chip?.textContent).toContain("Plan mode"); + act(() => chip.click()); + expect(composer?.getAttribute("data-pending-work-mode")).toBe("standard"); + expect(container.querySelector('[data-testid="issue-chat-composer-work-mode-chip"]')).toBeNull(); act(() => { root.unmount(); @@ -826,21 +825,19 @@ describe("IssueChatThread", () => { ); }); - const chip = container.querySelector( - '[data-testid="issue-chat-composer-work-mode-toggle"]', - ) as HTMLButtonElement | null; + const add = container.querySelector('[data-testid="issue-chat-composer-add"]') as HTMLButtonElement; const composer = container.querySelector( '[data-testid="issue-chat-composer"]', ) as HTMLDivElement | null; - expect(chip).not.toBeNull(); + expect(add).not.toBeNull(); expect(composer).not.toBeNull(); act(() => { - chip?.click(); + add.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, button: 0 })); }); const askMenuItem = document.querySelector( - '[data-testid="issue-chat-composer-work-mode-menu-ask"]', + '[data-testid="composer-add-ask"]', ) as HTMLButtonElement | null; expect(askMenuItem).not.toBeNull(); expect(askMenuItem?.textContent).toContain("Ask mode"); @@ -852,7 +849,7 @@ describe("IssueChatThread", () => { expect(onWorkModeChange).not.toHaveBeenCalled(); expect(composer?.getAttribute("data-pending-work-mode")).toBe("ask"); expect(composer?.className).toContain("sky"); - expect(chip?.textContent).toContain("Ask mode"); + expect(container.querySelector('[data-testid="issue-chat-composer-work-mode-chip"]')?.textContent).toContain("Ask mode"); act(() => { composer?.dispatchEvent( @@ -866,7 +863,7 @@ describe("IssueChatThread", () => { }); expect(composer?.getAttribute("data-pending-work-mode")).toBe("standard"); - expect(chip?.textContent).toContain("Auto mode"); + expect(container.querySelector('[data-testid="issue-chat-composer-work-mode-chip"]')).toBeNull(); act(() => { root.unmount(); @@ -3681,26 +3678,33 @@ describe("IssueChatThread", () => { }); }); - it("shows non-image attachment upload state in the composer after a drop", async () => { + it("keeps mode controls available while a dropped file uploads", async () => { const root = createRoot(container); - const onAttachImage = vi.fn(async (file: File) => ({ - id: "attachment-1", - companyId: "company-1", - issueId: "issue-1", - issueCommentId: null, - assetId: "asset-1", - provider: "local_disk", - objectKey: "issues/issue-1/report.pdf", - contentPath: "/api/attachments/attachment-1/content", - originalFilename: file.name, - contentType: file.type, - byteSize: file.size, - sha256: "abc123", - createdByAgentId: null, - createdByUserId: "user-1", - createdAt: new Date("2026-04-24T12:00:00.000Z"), - updatedAt: new Date("2026-04-24T12:00:00.000Z"), - })); + let finishUpload: () => void = () => {}; + const uploadGate = new Promise((resolve) => { + finishUpload = resolve; + }); + const onAttachImage = vi.fn(async (file: File) => { + await uploadGate; + return { + id: "attachment-1", + companyId: "company-1", + issueId: "issue-1", + issueCommentId: null, + assetId: "asset-1", + provider: "local_disk", + objectKey: "issues/issue-1/report.pdf", + contentPath: "/api/attachments/attachment-1/content", + originalFilename: file.name, + contentType: file.type, + byteSize: file.size, + sha256: "abc123", + createdByAgentId: null, + createdByUserId: "user-1", + createdAt: new Date("2026-04-24T12:00:00.000Z"), + updatedAt: new Date("2026-04-24T12:00:00.000Z"), + }; + }); await act(async () => { root.render( @@ -3712,6 +3716,8 @@ describe("IssueChatThread", () => { liveRuns={[]} onAdd={async () => {}} onAttachImage={onAttachImage} + issueWorkMode="standard" + onWorkModeChange={() => {}} enableLiveTranscriptPolling={false} /> , @@ -3725,11 +3731,28 @@ describe("IssueChatThread", () => { type: "application/pdf", }); - await act(async () => { + act(() => { composer?.dispatchEvent(createFileDragEvent("drop", [file])); }); expect(onAttachImage).toHaveBeenCalledWith(file); + const add = container.querySelector('[data-testid="issue-chat-composer-add"]') as HTMLButtonElement; + expect(add.disabled).toBe(false); + act(() => { + add.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, button: 0 })); + }); + expect(document.querySelector('[data-testid="composer-add-file"]')?.getAttribute("data-disabled")).not.toBeNull(); + const plan = document.querySelector('[data-testid="composer-add-plan"]') as HTMLButtonElement; + act(() => plan.click()); + const chip = container.querySelector('[data-testid="issue-chat-composer-work-mode-chip"]') as HTMLButtonElement; + expect(chip?.textContent).toContain("Plan mode"); + expect(chip.disabled).toBe(false); + act(() => chip.click()); + expect(container.querySelector('[data-testid="issue-chat-composer-work-mode-chip"]')).toBeNull(); + + await act(async () => { + finishUpload(); + }); const attachmentList = container.querySelector( '[data-testid="issue-chat-composer-attachments"]', ); diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index 1b7f8418e7..328a476df8 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -1,5 +1,8 @@ import { DispositionRecoveryNotice, useDispositionRecoverySnapshot } from "./DispositionRecoveryNotice"; import { AgentAvatar } from "@/components/AgentAvatar"; +import type { ComposerRunSettings } from "./task-chat/composer-run-settings"; +import { ComposerRunSettingsPicker } from "./task-chat/ComposerRunSettingsPicker"; +import { ComposerAddMenu, ComposerModeChip } from "./task-chat/ComposerAddMenu"; import { TaskChatPausedTakeover, type TaskComposerPause } from "./task-chat/TaskChatPausedTakeover"; import { useEmailComment } from "./EmailMessageCard"; import { AssistantRuntimeProvider } from "@assistant-ui/react"; @@ -46,6 +49,7 @@ import type { SuccessfulRunHandoffState, IssueWorkMode, IssueWorkProduct, + IssueAssigneeAdapterOverrides, } from "@paperclipai/shared"; import type { ActiveRunForIssue, LiveRunForIssue } from "../api/heartbeats"; import { findUIAdapter } from "../adapters/registry"; @@ -213,9 +217,7 @@ import { cn, formatDateTime, formatShortDate } from "../lib/utils"; import { liveBlueBadge } from "../lib/status-colors"; import { nextWorkMode, - titleForPendingWorkMode, workModeMetaFor, - workModeMetaList, } from "../lib/work-mode-meta"; import { Tooltip, @@ -518,6 +520,8 @@ interface IssueChatComposerProps { enableReassign?: boolean; reassignOptions?: InlineEntityOption[]; currentAssigneeValue?: string; + companyId?: string | null; + assigneeAdapterOverrides?: IssueAssigneeAdapterOverrides | null; suggestedAssigneeValue?: string; mentions?: MentionOption[]; agentMap?: Map; @@ -609,6 +613,7 @@ interface IssueChatThreadProps { reassignment?: CommentReassignment, attachmentIds?: string[], clientRequestId?: string, + runSettings?: ComposerRunSettings, ) => Promise; onReviewConversation?: () => Promise; onCancelRun?: () => Promise; @@ -625,6 +630,7 @@ interface IssueChatThreadProps { enableReassign?: boolean; reassignOptions?: InlineEntityOption[]; currentAssigneeValue?: string; + assigneeAdapterOverrides?: IssueAssigneeAdapterOverrides | null; suggestedAssigneeValue?: string; mentions?: MentionOption[]; composerPause?: TaskComposerPause | null; @@ -4659,6 +4665,8 @@ const IssueChatComposer = forwardRef< enableReassign = false, reassignOptions = [], currentAssigneeValue = "", + companyId, + assigneeAdapterOverrides, suggestedAssigneeValue, mentions = [], agentMap, @@ -4771,6 +4779,8 @@ const IssueChatComposer = forwardRef< const [reassignTarget, setReassignTarget] = useState( effectiveSuggestedAssigneeValue, ); + const [runSettings, setRunSettings] = useState(null); + useEffect(() => setRunSettings(null), [draftKey, currentAssigneeValue]); const [noAssigneeDialogOpen, setNoAssigneeDialogOpen] = useState(false); const [dismissedCoachToken, setDismissedCoachToken] = useState( null, @@ -4779,7 +4789,6 @@ const IssueChatComposer = forwardRef< const [pendingWorkMode, setPendingWorkMode] = useState( resolvedIssueWorkMode, ); - const [workModeMenuOpen, setWorkModeMenuOpen] = useState(false); const canToggleWorkMode = typeof onWorkModeChange === "function"; const attachInputRef = useRef(null); const reassignTriggerRef = useRef(null); @@ -5054,10 +5063,11 @@ const IssueChatComposer = forwardRef< } // assistant-ui thread.append is fire-and-forget. Await the actual Board // mutation; it already owns optimistic echo and durable error handling. - const sendPromise = onSend( - submittedBody, reopen, reassignment, - attachmentIds.length ? attachmentIds : undefined, attemptId, - ); + const sendPromise = runSettings + ? onSend(submittedBody, reopen, reassignment, + attachmentIds.length ? attachmentIds : undefined, attemptId, runSettings) + : onSend(submittedBody, reopen, reassignment, + attachmentIds.length ? attachmentIds : undefined, attemptId); queueViewportRestore(viewportSnapshot); await sendPromise; // Settle the captured task even if the user navigated away. The exact @@ -5069,6 +5079,7 @@ const IssueChatComposer = forwardRef< current.filter((item) => !submittedAttachmentKeys.has(item.id)), ); setReassignTarget(effectiveSuggestedAssigneeValue); + setRunSettings(null); } catch (error) { if (mountedTaskKey.current !== draftKey) return; const nextDraft = bodyRef.current; @@ -5344,9 +5355,7 @@ const IssueChatComposer = forwardRef< ); } - const workModeOptions = workModeMetaList(); const pendingWorkModeMeta = workModeMetaFor(pendingWorkMode); - const PendingWorkModeIcon = pendingWorkModeMeta.icon; function handleComposerKeyDown(evt: ReactKeyboardEvent) { // Match the period via both `code` and `key`: iOS Safari with a hardware @@ -5586,89 +5595,38 @@ const IssueChatComposer = forwardRef<
{canAcceptFiles ? ( - <> - - - - ) : null} - {canToggleWorkMode ? ( - - - {/* Single persistent mode chip (PAP-95b mockup rev 5): yellow in - planning, neutral in standard, caret opens the switch menu. */} - - - - {workModeOptions.map((option) => { - const Icon = option.icon; - const active = option.value === pendingWorkMode; - return ( - - ); - })} -
- Cmd/Ctrl+. cycles modes -
- - + ) : null} + attachInputRef.current?.click() : undefined} + attachDisabled={attaching} + disabled={!!uncertainSubmission} + triggerTestId="issue-chat-composer-add" menuTestId="issue-chat-composer-add-menu" /> + setPendingWorkMode("standard") : undefined} + disabled={!!uncertainSubmission} + testId="issue-chat-composer-work-mode-chip" />
- {enableReassign && reassignOptions.length > 0 ? ( + {enableReassign && reassignOptions.length > 0 && companyId && agentMap ? ( + { + const selected = value.startsWith("agent:") ? agentMap.get(value.slice(6)) : null; + return selected ? : null; + }} + /> + ) : enableReassign && reassignOptions.length > 0 ? ( ( - (body, reopen, reassignment, attachmentIds, clientRequestId) => { + (body, reopen, reassignment, attachmentIds, clientRequestId, runSettings) => { pendingSubmitScrollRef.current = true; - return onAdd(body, reopen, reassignment, attachmentIds, clientRequestId); + return runSettings + ? onAdd(body, reopen, reassignment, attachmentIds, clientRequestId, runSettings) + : onAdd(body, reopen, reassignment, attachmentIds, clientRequestId); }, [onAdd], ); @@ -6805,6 +6766,8 @@ export function IssueChatThread({ enableReassign={enableReassign} reassignOptions={reassignOptions} currentAssigneeValue={currentAssigneeValue} + companyId={companyId} + assigneeAdapterOverrides={assigneeAdapterOverrides} suggestedAssigneeValue={suggestedAssigneeValue} mentions={mentions} agentMap={agentMap} diff --git a/ui/src/components/Layout.production.tsx b/ui/src/components/Layout.production.tsx index 6f88bbeeb1..c809a8cb28 100644 --- a/ui/src/components/Layout.production.tsx +++ b/ui/src/components/Layout.production.tsx @@ -63,6 +63,7 @@ import { queryKeys } from "../lib/queryKeys"; import { scheduleMainContentFocus } from "../lib/main-content-focus"; import { pinDocumentScrollToZero } from "../lib/pin-document-scroll"; import { cn } from "../lib/utils"; +import { classifyShellRoute } from "../lib/shell-navigation"; import { NotFoundPage } from "../pages/NotFound"; import { PluginSlotMount, @@ -145,6 +146,7 @@ export function Layout() { const navigate = useNavigate(); const location = useLocation(); const navigationType = useNavigationType(); + const isTaskDetailRoute = classifyShellRoute(location.pathname, companyPrefix).isTaskDetail; const isCompanySettingsRoute = [ "/company/settings", "/company/export", @@ -738,8 +740,8 @@ export function Layout() { style={ isMobile ? ({ - "--tc-composer-bottom": mobileNavVisible - ? "var(--sz-calc-14)" + "--tc-composer-bottom": mobileNavVisible + ? "var(--tc-composer-visible-nav-offset)" : "var(--sz-calc-8)", } as CSSProperties) : undefined @@ -750,7 +752,9 @@ export function Layout() { // changes (e.g. switching skill-detail tabs) don't widen/shift // when the vertical scrollbar appears or disappears (PAP-10907). isMobile - ? "overflow-visible pb-(--sz-calc-14)" + ? isTaskDetailRoute && mobileNavVisible + ? "overflow-visible pb-(--tc-composer-visible-nav-offset)" + : "overflow-visible pb-(--sz-calc-14)" : "overflow-auto [scrollbar-gutter:stable]", )} > diff --git a/ui/src/components/Layout.tsx b/ui/src/components/Layout.tsx index 79437ea31f..ccf674d853 100644 --- a/ui/src/components/Layout.tsx +++ b/ui/src/components/Layout.tsx @@ -740,7 +740,7 @@ export function Layout({ sidebarSections }: { sidebarSections?: ReactNode }) { isMobile ? ({ "--tc-composer-bottom": mobileNavVisible - ? "var(--sz-calc-14)" + ? "var(--tc-composer-visible-nav-offset)" : "var(--tc-composer-hidden-nav-offset)", } as CSSProperties) : undefined @@ -755,8 +755,10 @@ export function Layout({ sidebarSections }: { sidebarSections?: ReactNode }) { // changes (e.g. switching skill-detail tabs) don't widen/shift // when the vertical scrollbar appears or disappears (PAP-10907). isMobile - ? isTaskDetailRoute && !mobileNavVisible - ? "overflow-visible pb-(--tc-composer-hidden-nav-offset)" + ? isTaskDetailRoute + ? mobileNavVisible + ? "overflow-visible pb-(--tc-composer-visible-nav-offset)" + : "overflow-visible pb-(--tc-composer-hidden-nav-offset)" : "overflow-visible pb-(--sz-calc-14)" : "overflow-auto [scrollbar-gutter:stable]", )} diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index 6ec28f29e8..34e75aee25 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -87,6 +87,7 @@ import { taskChatContentKey, } from "@/components/task-chat/TaskChatThreadView"; import { TaskChatComposer } from "@/components/task-chat/TaskChatComposer"; +import { TaskChatComposerDock } from "@/components/task-chat/TaskChatComposerDock"; import { RunnerGoalWidget, useRunnerGoalControl, @@ -510,6 +511,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { conversationMode, reassignOptions, currentAssigneeValue, + assigneeAdapterOverrides, issueStatus, issueAssigneeAgentId = null, onAcceptInteraction, @@ -2985,28 +2987,7 @@ export function TaskChatThread(props: TaskChatThreadProps) {
) : null} {showComposer ? ( -
+ {composerAccessory} {tailTurnStatus ? ( @@ -3079,8 +3060,11 @@ export function TaskChatThread(props: TaskChatThreadProps) { conversationMode={conversationMode} reassignOptions={reassignOptions} agentMap={agentMap} + modelAgents={agentMap} userProfileMap={userProfileMap} currentAssigneeValue={currentAssigneeValue} + companyId={companyId} + assigneeAdapterOverrides={assigneeAdapterOverrides} onPendingAssigneeChange={setPendingComposerAssignee} issueStatus={issueStatus} mobile={isMobile} @@ -3106,7 +3090,7 @@ export function TaskChatThread(props: TaskChatThreadProps) {
{footer} - + ) : null} diff --git a/ui/src/components/artifacts/IssueArtifactCard.test.tsx b/ui/src/components/artifacts/IssueArtifactCard.test.tsx new file mode 100644 index 0000000000..75402929e3 --- /dev/null +++ b/ui/src/components/artifacts/IssueArtifactCard.test.tsx @@ -0,0 +1,275 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; +import type { IssueWorkProduct } from "@paperclipai/shared"; +import { + IssueArtifactFile, + IssueWorkProductArtifactCard, +} from "./IssueArtifactCard"; + +import { DocumentCard } from "./RichArtifactCards"; + +function product(overrides: Partial = {}): IssueWorkProduct { + return { + id: "wp-1", + companyId: "company-1", + projectId: null, + issueId: "issue-1", + executionWorkspaceId: null, + runtimeServiceId: null, + type: "pull_request", + provider: "github", + externalId: null, + title: "Actual artifact title", + url: "https://github.com/org/repo/pull/42", + status: "active", + reviewState: "none", + isPrimary: false, + healthStatus: "unknown", + summary: "Actual artifact summary", + metadata: null, + createdByRunId: null, + createdAt: new Date("2026-09-28"), + updatedAt: new Date("2026-09-28"), + ...overrides, + }; +} +function render(wp: IssueWorkProduct) { + return renderToStaticMarkup( + + + , + ); +} + +describe("production artifact cards", () => { + it("renders supplied PR metadata without synthesizing missing counts, checks or branch names", () => { + const sparse = render(product()); + expect(sparse).toContain("Actual artifact title"); + expect(sparse).toContain("Actual artifact summary"); + expect(sparse).toContain("Actual agent"); + expect(sparse).toContain("Checks not available"); + expect(sparse).not.toContain("Checks passed"); + expect(sparse).not.toContain("+0"); + expect(sparse).not.toContain("#0"); + expect(sparse).not.toContain("master"); + const full = render( + product({ + metadata: { + number: 42, + repo: "org/repo", + state: "draft", + additions: 0, + deletions: 8, + changedFiles: 2, + headRef: "feature", + baseRef: "main", + }, + }), + ); + for (const value of [ + "#42", + "org/repo", + "Draft", + "+0", + "−8", + "feature", + "main", + ]) + expect(full).toContain(value); + }); + it("preserves review and unhealthy states", () => { + expect( + render(product({ reviewState: "changes_requested", status: "merged" })), + ).toContain("Changes requested"); + expect( + render(product({ type: "preview_url", healthStatus: "unhealthy" })), + ).toContain("Unhealthy"); + expect( + render(product({ type: "preview_url", healthStatus: "unknown" })), + ).not.toContain("Healthy"); + }); + it.each([ + ["ready_for_review", "Review"], + ["changes_requested", "Changes requested"], + ])( + "preserves PR status %s without a separate reviewState", + (status, label) => { + expect(render(product({ status, reviewState: "none" }))).toContain(label); + }, + ); + + it("preserves browser-open actions for attachments and signed external file URLs", () => { + const signedUrl = + "https://files.example/report.pdf?signature=abc&expires=123"; + const external = render( + product({ + type: "artifact", + url: signedUrl, + metadata: { contentType: "application/pdf" }, + }), + ); + expect(external).toContain( + 'href="https://files.example/report.pdf?signature=abc&expires=123"', + ); + expect(external).not.toContain("?download=1"); + expect(external).toContain("Open file"); + expect(external).toContain("Download file"); + + const local = renderToStaticMarkup( + + + , + ); + expect(local).toContain( + 'href="/api/attachments/pdf-1/content" target="_blank"', + ); + expect(local).toContain( + 'href="/api/attachments/pdf-1/content?download=1" download="report.pdf"', + ); + }); + + it("selects commit, link, image, video and file renderers from real records", () => { + expect( + render( + product({ + type: "commit", + metadata: { sha: "123456789abcdef", repo: "org/repo" }, + }), + ), + ).toContain("12345678"); + expect(render(product({ type: "preview_url" }))).toContain("Open link"); + const attachment = { + contentPath: "/api/attachments/file-1/content", + byteSize: 25, + }; + expect( + render( + product({ + type: "artifact", + metadata: { + ...attachment, + contentType: "image/png", + originalFilename: "actual.png", + }, + }), + ), + ).toContain("View image"); + const video = render( + product({ + type: "artifact", + metadata: { + ...attachment, + contentType: "video/mp4", + originalFilename: "actual.mp4", + }, + }), + ); + expect(video).toContain(" { + const client = new QueryClient(); + client.setQueryData( + ["artifact-csv", "csv-1", "/api/attachments/csv-1/content", "Today"], + { columns: ["Region"], rows: [["Actual region"]], truncated: false }, + ); + const html = renderToStaticMarkup( + + + , + ); + expect(html).toContain("Actual region"); + expect(html).toContain("View data"); + }); + it("does not fetch remote Markdown images just to render a document preview", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).not.toContain('src="https://tracker.example'); + expect(html).toContain('href="https://tracker.example/image.png"'); + expect(html).toContain('src="/api/attachments/image-1/content"'); + }); + it("does not automatically load remote thumbnail or poster metadata", () => { + const link = render( + product({ + type: "preview_url", + metadata: { imageUrl: "https://tracker.example/pixel.png" }, + }), + ); + expect(link).not.toContain(" { + expect(render(product({ type: "branch" }))).toContain("Open on GitHub"); + expect( + render(product({ type: "runtime_service", healthStatus: "unhealthy" })), + ).toContain("Unhealthy"); + expect( + render(product({ type: "preview_url", url: "javascript:alert(1)" })), + ).not.toContain("javascript:"); + }); +}); diff --git a/ui/src/components/artifacts/IssueArtifactCard.tsx b/ui/src/components/artifacts/IssueArtifactCard.tsx new file mode 100644 index 0000000000..90118a87d7 --- /dev/null +++ b/ui/src/components/artifacts/IssueArtifactCard.tsx @@ -0,0 +1,277 @@ +import { useContext, useState } from "react"; +import { useQuery } from "@tanstack/react-query"; +import { + getAttachmentArtifactWorkProductMetadata, + type IssueWorkProduct, +} from "@paperclipai/shared"; +import { IssueGalleryContext } from "@/context/IssueGalleryContext"; +import { ImageGalleryModal } from "@/components/ImageGalleryModal"; +import { + RichWorkProductCard, + stateChipFor, +} from "@/components/task-chat/RichWorkProductCard"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { isImageLikeOutput, isVideoLikeOutput } from "@/lib/issue-output"; +import { attachmentDownloadPath } from "@/lib/issue-attachments"; +import { workProductHref } from "@/lib/issue-artifacts"; +import { formatDateTime } from "@/lib/utils"; +import { + artifactText as text, + artifactNumber as number, + artifactUrl, + artifactPreviewUrl, + artifactFileSize, + CSV_PREVIEW_MAX_BYTES, + loadArtifactCsv, +} from "@/lib/artifact-card-data"; +import { + CommitCard, + DataCard, + FileCard, + ImageCard, + LinkPreviewCard, + PullRequestCard, + VideoCard, + type ArtifactIdentity, +} from "./RichArtifactCards"; + +export interface IssueArtifactFileProps extends ArtifactIdentity { + id: string; + filename: string; + contentType: string; + contentPath: string; + downloadPath: string; + openPath?: string; + byteSize: number | null; + metadata?: Record | null; +} + +/** Shared by uploads and promoted uploads; both use the issue's existing gallery. */ +export function IssueArtifactFile(props: IssueArtifactFileProps) { + const { metadata = null } = props; + const openGallery = useContext(IssueGalleryContext); + const [galleryOpen, setGalleryOpen] = useState(false); + const [csvRequested, setCsvRequested] = useState(false); + const image = isImageLikeOutput(props.contentType, props.filename); + const video = isVideoLikeOutput(props.contentType, props.filename); + const csv = + props.contentType.split(";")[0] === "text/csv" || + /\.csv$/i.test(props.filename); + const tooLarge = + props.byteSize !== null && props.byteSize > CSV_PREVIEW_MAX_BYTES; + const localCsv = + csv && + /^\/api\/attachments\/[a-zA-Z0-9-]+\/content$/.test(props.contentPath); + const data = useQuery({ + queryKey: ["artifact-csv", props.id, props.contentPath, props.updatedAt], + queryFn: ({ signal }) => loadArtifactCsv(props.contentPath, signal), + enabled: localCsv && !tooLarge && csvRequested, + retry: false, + staleTime: Infinity, + }); + const contentPath = artifactUrl(props.contentPath); + const downloadPath = artifactUrl(props.downloadPath); + const onOpen = () => { + if (!openGallery?.(contentPath)) setGalleryOpen(true); + }; + if ((image || video) && contentPath) { + return ( + <> + {video ? ( + + ) : ( + + )} + {galleryOpen && ( + + )} + + ); + } + if (localCsv && !tooLarge && data.data) + return ; + return ( +
+ setCsvRequested(true)} + > + {data.isFetching ? "Loading preview…" : "Preview data"} + + ) : undefined + } + /> + {csv && (tooLarge || data.isError || !localCsv) && ( +
+ {tooLarge + ? "CSV is too large to preview. Download the file to view it." + : data.isError + ? data.error.message + : "CSV preview is unavailable. Download the file to view it."} + {data.isError && ( + + )} +
+ )} +
+ ); +} + +export function IssueWorkProductArtifactCard({ + workProduct: wp, + author, +}: { + workProduct: IssueWorkProduct; + author: string; +}) { + const m = wp.metadata; + const href = artifactUrl(workProductHref(wp)); + const chip = stateChipFor(wp.type, wp.status, wp.reviewState); + const identity: ArtifactIdentity = { + title: wp.title, + summary: wp.summary ?? "", + author, + updatedAt: formatDateTime(wp.updatedAt), + statusBadge: + wp.healthStatus === "unhealthy" ? ( + Unhealthy + ) : chip ? ( + {chip.label} + ) : undefined, + }; + const diff = { + additions: number(m, "additions"), + deletions: number(m, "deletions"), + filesChanged: number(m, "changedFiles", "files"), + }; + if (wp.type === "pull_request") { + const state = + text(m, "state") || (wp.status === "active" ? "open" : wp.status); + const checks = text(m, "checks"); + return ( + + ); + } + if (wp.type === "commit") + return ( + + ); + if (wp.type === "preview_url") + return ( + + ); + if (wp.type === "artifact") { + const attachment = getAttachmentArtifactWorkProductMetadata(wp); + const contentPath = artifactUrl( + attachment?.contentPath || text(m, "contentPath") || href, + ); + return ( + + ); + } + // Branches, external documents, and runtime services keep their existing actions and health semantics. + return ; +} diff --git a/ui/src/components/artifacts/IssueArtifactFile.test.tsx b/ui/src/components/artifacts/IssueArtifactFile.test.tsx new file mode 100644 index 0000000000..a4f7b845f8 --- /dev/null +++ b/ui/src/components/artifacts/IssueArtifactFile.test.tsx @@ -0,0 +1,73 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { describe, expect, it, vi } from "vitest"; +import { IssueArtifactFile } from "./IssueArtifactCard"; +import { loadArtifactCsv } from "@/lib/artifact-card-data"; + +vi.mock("@/lib/artifact-card-data", async (importOriginal) => ({ + ...(await importOriginal()), + loadArtifactCsv: vi.fn(), +})); + +describe("CSV preview consent", () => { + it("does not fetch any CSV until its preview is requested", async () => { + const load = vi + .mocked(loadArtifactCsv) + .mockResolvedValue({ + columns: ["Name"], + rows: [["Actual row"]], + truncated: false, + }); + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + try { + await act(async () => + root.render( + + {Array.from({ length: 20 }, (_, i) => ( + + ))} + , + ), + ); + expect(load).not.toHaveBeenCalled(); + const preview = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Preview data", + ); + expect(preview).toBeDefined(); + await act(async () => preview!.click()); + await vi.waitFor(() => + expect(container.textContent).toContain("Actual row"), + ); + expect(load).toHaveBeenCalledTimes(1); + expect(load).toHaveBeenCalledWith( + "/api/attachments/csv-0/content", + expect.any(AbortSignal), + ); + expect(container.textContent).toContain("View data"); + } finally { + await act(async () => root.unmount()); + client.clear(); + container.remove(); + vi.clearAllMocks(); + } + }); +}); diff --git a/ui/src/components/artifacts/RichArtifactCards.tsx b/ui/src/components/artifacts/RichArtifactCards.tsx new file mode 100644 index 0000000000..2c09febcb5 --- /dev/null +++ b/ui/src/components/artifacts/RichArtifactCards.tsx @@ -0,0 +1,733 @@ +import { useState, type ReactNode } from "react"; +import ReactMarkdown from "react-markdown"; +import { + ArrowRight, + CircleCheck, + CircleHelp, + Clock, + ExternalLink, + File, + FileText, + Film, + GitBranch, + GitCommitHorizontal, + GitMerge, + GitPullRequest, + Globe, + Image as ImageIcon, + Table2, + TriangleAlert, +} from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { Badge } from "@/components/ui/badge"; +import { artifactPreviewUrl, artifactUrl } from "@/lib/artifact-card-data"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, + DialogTrigger, +} from "@/components/ui/dialog"; + +// Renderers contain UI labels only. All artifact-specific content is passed in. +// Example values live exclusively in the individual stories' args. +export interface ArtifactIdentity { + title: string; + summary: string; + author: string; + updatedAt: string; + statusBadge?: ReactNode; +} + +function Identity({ + title, + summary, +}: Pick) { + return ( +
+

+ {title} +

+ {summary && ( +

+ {summary} +

+ )} +
+ ); +} + +function Footer({ + author, + updatedAt, + action, + statusBadge, +}: Pick & { + action: ReactNode; +}) { + return ( +
+ + {[author, updatedAt].filter(Boolean).join(" · ")} + + {statusBadge} + {action} +
+ ); +} + +function Card({ children }: { children: ReactNode }) { + return ( +
+ {children} +
+ ); +} + +function SourceLink({ url, children }: { url: string; children: ReactNode }) { + return url ? ( + + ) : ( + + ); +} + +function Viewer({ + title, + description, + action, + children, +}: { + title: string; + description: string; + action: string; + children: ReactNode; +}) { + return ( + + + + + + + {title} + {description} + + {children} + + + ); +} + +interface DiffProps { + additions?: number | null; + deletions?: number | null; + filesChanged?: number | null; +} +function Diff({ additions, deletions, filesChanged }: DiffProps) { + return ( +
+ {additions != null && ( + + +{additions.toLocaleString("en-US")} + + )} + {deletions != null && ( + + −{deletions.toLocaleString("en-US")} + + )} + {filesChanged != null && ( + + {filesChanged} {filesChanged === 1 ? "file" : "files"} + + )} +
+ ); +} + +export interface PullRequestCardProps extends ArtifactIdentity, DiffProps { + number?: number | null; + repository: string; + sourceBranch: string; + targetBranch: string; + state: "open" | "draft" | "merged" | "closed" | "unknown"; + checks: "passed" | "pending" | "failed" | "unknown"; + evidenceSource: string; + reviewSummary: string; + url: string; +} + +const checksLabel = { + passed: "Checks passed", + pending: "Checks pending", + failed: "Checks failed", + unknown: "Checks not available", +}; +const stateLabel = { + open: "Open", + draft: "Draft", + merged: "Merged", + closed: "Closed", + unknown: "State unknown", +}; + +export function PullRequestCard(props: PullRequestCardProps) { + const { + number, + repository, + sourceBranch, + targetBranch, + state, + checks, + evidenceSource, + reviewSummary, + url, + } = props; + const CheckIcon = { + passed: CircleCheck, + pending: Clock, + failed: TriangleAlert, + unknown: CircleHelp, + }[checks]; + const checkTone = { + passed: "text-(--status-task-icon-done)", + pending: "text-muted-foreground", + failed: "text-(--status-task-icon-blocked)", + unknown: "text-muted-foreground", + }[checks]; + return ( + +
+
+ + Pull request{" "} + {number != null && #{number}} + + + {state === "merged" && } + {stateLabel[state]} + +
+ +
+ {repository} + {(sourceBranch || targetBranch) && ( +
+ + + {sourceBranch} + + {sourceBranch && targetBranch && ( + + )} + + {targetBranch} + +
+ )} +
+ +
+
+ + + {checksLabel[checks]} + + {reviewSummary && ( +

{reviewSummary}

+ )} + {evidenceSource && ( +

+ Source: {evidenceSource} +

+ )} +
+