From e2908fff5c4ef62496009b791bb6dca1d1b1ea68 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 30 Sep 2026 17:20:25 -0500 Subject: [PATCH 1/6] fix: enforce terminal outcomes during recovered sandbox cleanup (#14767) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runners can keep a reusable sandbox warm after successful turns. > - Failed turns must stop their sandbox before a later retry resumes it. > - Workspace recovery can release a lease outside the executor's normal teardown. > - Successful file copy-back can then retain a sandbox even when its run failed. > - This pull request checks the durable run outcome at the shared release boundary. > - Ordinary teardown and recovery now apply the same retention rule. ## Linked Issues or Issue Description **What happened?** A real Daytona verification on `5edf55d73` produced a failed native turn. The runner process exited, but workspace recovery retained the sandbox without stopping it. The recovery callback used successful workspace copy-back to select warm retention. It bypassed the terminal-state check in normal heartbeat teardown. **Expected behavior** Keep a sandbox running only after a successful run. Failed, cancelled, timed-out, and interrupted runs must convert requested warm retention to stop-and-retain. Preserve the existing ownership hold before release. **Steps to reproduce** 1. Persist a terminal failed native run whose workspace copy-back succeeds. 2. Release its environment lease from the recovery path with a stored `keep_running` disposition. 3. Observe that the provider receives `keep_running` on the base commit. 4. With this fix, the provider receives `stop_and_retain` and the lease status follows the durable run outcome. **Paperclip version or commit** Base: `5edf55d7350c7f08c9dd132c7e0f1421fa0bf2fb`. **Deployment mode** Native runner with a reusable Daytona sandbox. Related: #14747 retires unsuccessful warm runner sessions. This change closes the separate recovery lease-release path. Related search found no duplicate fix. ## What Changed - Read the durable run status at the shared lease-release boundary. - Apply the existing terminal-outcome retention rule before calling the environment runtime. - Use that same durable status for the lease-state mapping. - Add five database-backed regressions for four unsuccessful outcomes and successful warm retention. - Document the recovery rule. ## Verification - Before the fix: the four unsuccessful-outcome regressions fail; the successful case passes. - After the fix: 163 tests pass across the lease-release, native lifecycle, and explicit continuation suites. - Repository typecheck and build pass. - `pnpm test:run` encountered the existing local `native-session-resume.test.ts:1125` assertion failure; a focused rerun reproduced the same failure. This was also recorded before this follow-up with the unmodified base executor. The full command was stopped after that confirmation, so later local groups were not completed. - All 56 latest-head checks are successful or intentionally skipped (54 passed, 2 skipped), including the complete CI test groups and browser E2E suite. Greptile is 5/5 on `e7da3b3ad`, with no unresolved review comments. - Deployed exact PR head `e7da3b3adbf7a13642c0e56f5b0f0c4666adf358` to the staging workspace and independently verified the serving commit. A real failed native turn persisted `keep_running` and completed workspace finalization, reproducing the recovery-path conditions; Paperclip automatically issued stop-and-retain, and an independent Daytona read confirmed `stopped`. No manual stop was used. - Retried that failed run through the public API after correcting its temporary API-key credential. The same stopped sandbox resumed successfully. Three successful turns retained one live runner PID/start time and the same native, runner, and provider sessions. - Verified exact canonical note contents, deletion persistence, and an unchanged 8 MiB binary after every turn. Subsequent checkpoints copied/hashed only 58 and 87 bytes. After sandbox deletion, canonical files still matched. Temporary secrets were removed and agent/configuration policy restored. - Live acceptance used temporary API-key authentication. The separate managed-subscription authentication issue and browser Retry control were not tested by this campaign. ## Risks - Recovery callers can no longer use stale success status to retain a failed run's sandbox. - The existing native ownership hold still blocks release while ownership is unresolved. - Successful warm turns and explicit destroy dispositions keep their existing behavior. - No database migration, API contract, dependency, or UI change is included. ## Model Used OpenAI Codex, GPT-6, with reasoning, tool use, code execution, and test analysis. The exact served model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/DEVELOPING.md | 3 ++ ...eat-run-terminalize-before-release.test.ts | 37 +++++++++++++++++++ server/src/services/heartbeat.ts | 13 +++++-- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 40b568a6b64..7fdd364f4a5 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1246,6 +1246,9 @@ Only successful turns retain a live warm provider session. A structured failed or cancelled result must retire that session and collect its managed files before heartbeat stops the reusable sandbox. A later retry can resume the same sandbox without inheriting the stopped provider transport. +The shared lease-release boundary rechecks the durable run status for recovery +and ordinary teardown. Successful workspace copy-back alone must not keep a +failed turn's sandbox running. Run the credential-free real-process restart suite with: diff --git a/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts b/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts index 3d0ab72eec6..62555a27371 100644 --- a/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts +++ b/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts @@ -301,6 +301,43 @@ describeEmbeddedPostgres("heartbeat teardown terminalizes the run before releasi expect(releaseRunLeases).not.toHaveBeenCalled(); }); + it.each(["failed", "cancelled", "timed_out", "interrupted", "succeeded"])( + "uses the durable %s outcome when recovered workspace cleanup requests warm retention", + async (status) => { + const { companyId, agentId, issueId, runId } = await seed({ + issueStatus: status === "succeeded" ? "done" : "blocked", + runStatus: status, + }); + await db.update(heartbeatRuns).set({ + runtimeMode: "native", + nativeIssueId: issueId, + nativePhase: "committed", + finishedAt: new Date(), + }).where(eq(heartbeatRuns.id, runId)); + const releaseRunLeases = vi.fn(async () => []); + const heartbeat = heartbeatService(db, { + environmentRuntime: { releaseRunLeases } as unknown as HeartbeatEnvironmentRuntime, + }); + + // Successful workspace copy-back does not make a failed provider turn + // successful. Recovery can reach this boundary without the run's finally. + await heartbeat.releaseEnvironmentLeasesForRun({ + runId, + companyId, + agentId, + status: "succeeded", + providerResourceDisposition: "keep_running", + }); + + expect(releaseRunLeases).toHaveBeenCalledWith( + runId, + leaseReleaseStatusForRunStatus(status), + expect.any(Function), + status === "succeeded" ? "keep_running" : "stop_and_retain", + ); + }, + ); + it.each(["daytona", "local"])("destroy after failed checkpoint requires a terminal remote owner: %s", async provider => { const { companyId, agentId, runId } = await seed({ issueStatus: "blocked", runStatus: "running" }); await db.update(heartbeatRuns).set({ status: "cancelled", finishedAt: new Date() }).where(eq(heartbeatRuns.id, runId)); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 0b77ae299c8..388f1583fa8 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -10173,7 +10173,14 @@ export function heartbeatService( }) { const leaseOwnerRun = await getRun(input.runId); if (leaseOwnerRun && isNativeRunnerOwnershipHeld(leaseOwnerRun)) return; - if (input.providerResourceDisposition === "destroy") { + // Recovery can finish workspace copy-back outside the executor's finally. + // Successful copy-back does not earn warm retention for a failed turn. + const status = leaseOwnerRun?.status ?? input.status; + const providerResourceDisposition = providerResourceDispositionForTerminalRun( + input.providerResourceDisposition, + status, + ); + if (providerResourceDisposition === "destroy") { const closeResult = await ( options.closeWarmNativeSessionsForRun ?? (async ({ runId, reason }) => { @@ -10241,9 +10248,9 @@ export function heartbeatService( heartbeatRunId: input.runId, companyId: input.companyId, agentId: input.agentId, - status: leaseReleaseStatusForRunStatus(input.status), + status: leaseReleaseStatusForRunStatus(status), failureReason: input.failureReason ?? undefined, - providerResourceDisposition: input.providerResourceDisposition, + providerResourceDisposition, nativeLifecycleTelemetry: input.nativeLifecycleTelemetry, }) .catch((err) => { From 3bbb8d0f699127df9c2380423142223eb96fd490 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 30 Sep 2026 15:29:15 -0700 Subject: [PATCH 2/6] Add bounded AgentMail failure diagnostics (#14768) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - AgentMail connections create inboxes and handle email tasks. > - A failed provider request currently records only its HTTP status. > - The same 403 can mean a permission denial, a resource limit, or another provider restriction. > - This pull request records a fixed operation name and a documented, allowlisted error code. > - Operators can distinguish these failures without exposing provider payloads or changing retry behavior. ## Linked Issues or Issue Description **What happened?** An AgentMail inbox creation failure reports only `AgentMail request failed (403)`. The response body is deliberately excluded because it can contain private mail or credentials. That also discards the provider code needed to identify the cause. **Expected behavior** Keep the HTTP failure visible with a fixed operation name and a safe provider code. Never copy arbitrary error text, resource identifiers, suggested fixes, or URLs into diagnostics. **Steps to reproduce** 1. Make an inbox creation request through `agentmailApi` with a fake provider returning HTTP 403 and `code: "missing_permission"`. 2. Observe that the old error lacks the operation and provider code. 3. With this change, verify the error includes `operation=create_inbox, code=missing_permission`, preserves status 403, and excludes all other response fields. Related work: #13256 introduced the AgentMail connection. The provider documents stable codes in its [error reference](https://docs.agentmail.to/errors). ## What Changed - Add a fixed method/route-to-operation map and an allowlist of documented provider codes. - Read at most 8 KiB for diagnostics, with a one-second deadline. Cancel unread bodies and preserve the HTTP error if reading or parsing fails. - Keep the existing error prefix, status, retry delay, and failure handling. - Add regression coverage and document the diagnostic limits. ## Verification - `pnpm exec vitest run server/src/__tests__/agentmail-api.test.ts` — 44 tests passed. - `pnpm build` — passed. - `pnpm -r typecheck` — passed before the review correction. Final `pnpm --filter @paperclipai/server exec tsc --noEmit` also passed. - Full local `pnpm test:run` did not finish successfully; three company-skills-service failures were observed outside the changed module. The final-head CI server suites passed. The remaining workspaces-b CI retry covers an unrelated HTTP/2 port collision. - The diff passed a scan for configured secrets, private deployment references, and non-fixture email addresses. ## Risks - A failed request can now wait up to one extra second while reading its diagnostic code. - New, missing, malformed, or oversized provider codes report `unknown`. A future provider code needs an explicit allowlist update. - This is a diagnostics change. It does not establish or repair the cause of an existing provider denial. - No schema, credential policy, or retry behavior changes. ## Model Used OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The exact served model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 at 20853e31abacf53a32f1a63467ee208a719bbf00; the locked-body finding is fixed and its thread resolved - [x] I will address all Greptile and reviewer comments before requesting merge Final verification (September 30): all final-head GitHub checks pass at `20853e31abacf53a32f1a63467ee208a719bbf00`, including the targeted workspaces-b rerun after the unrelated EADDRINUSE failure. Greptile scored 5/5 on this head and no review threads remain unresolved. The branch is mergeable. The local full-suite run did not yield a passing completion; CI completed successfully across all suites. This public PR remains open for maintainer merge. --------- Co-authored-by: Paperclip --- doc/connections/AGENTMAIL.md | 9 ++ server/src/__tests__/agentmail-api.test.ts | 111 +++++++++++++++++++++ server/src/services/agentmail-api.ts | 77 +++++++++++++- 3 files changed, 196 insertions(+), 1 deletion(-) diff --git a/doc/connections/AGENTMAIL.md b/doc/connections/AGENTMAIL.md index 3c589d84f94..7da4ac7b252 100644 --- a/doc/connections/AGENTMAIL.md +++ b/doc/connections/AGENTMAIL.md @@ -19,6 +19,15 @@ Verified custom domains are selectable after checking the API key. Complete DNS setup in [AgentMail](https://docs.agentmail.to/custom-domains). Paperclip does not register domains or manage DNS. +Failed provider requests retain their HTTP status, a fixed operation name, and +an allowlisted [AgentMail error code](https://docs.agentmail.to/errors). For +example, `create_inbox` with `missing_permission` differs from `limit_exceeded`. +An HTTP 403 alone does not establish the cause. Missing or unrecognized codes +appear as `unknown`. Error-body inspection is limited to 8 KiB and one second; +malformed, larger, or stalled responses keep the original HTTP failure. Provider +messages, suggested fixes, URLs, inbox identifiers, and credentials are excluded +from these diagnostics. These diagnostics do not retry or suppress failures. + The setup and Permissions page warn that an unrestricted inbox can receive mail from anyone. Configure sender allowlists in AgentMail; Paperclip does not manage or verify them. AgentMail controls new-message and reply lists separately. The diff --git a/server/src/__tests__/agentmail-api.test.ts b/server/src/__tests__/agentmail-api.test.ts index c308809baf6..727a11c6f8e 100644 --- a/server/src/__tests__/agentmail-api.test.ts +++ b/server/src/__tests__/agentmail-api.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, vi } from "vitest"; import { Webhook } from "svix"; import { agentmailApi, + AgentmailApiError, agentmailMessageSchema, emailText, emailReplyRecipients, @@ -138,6 +139,116 @@ describe("AgentMail protocol boundary", () => { "AgentMail request failed (403)", ); }); + it.each(["missing_permission", "limit_exceeded", "domain_not_verified"])( + "keeps a bounded create-inbox diagnostic for %s without provider data", + async (code) => { + const privateValue = "private-address@example.test"; + const fetcher = vi.fn().mockResolvedValue(new Response(JSON.stringify({ + code, + message: privateValue, + fix: `Use credential ${privateValue}`, + docs: `https://example.test/${privateValue}`, + nested: { code: privateValue }, + }), { status: 403 })); + const error = await agentmailApi(privateValue, fetcher) + .createInbox({ username: privateValue }).catch((error: unknown) => error); + expect(error).toBeInstanceOf(AgentmailApiError); + expect(error).toMatchObject({ status: 403, operation: "create_inbox", providerCode: code }); + expect(String(error)).toBe(`Error: AgentMail request failed (403) [operation=create_inbox, code=${code}]`); + expect(JSON.stringify(error)).not.toContain(privateValue); + expect((error as Error).stack).not.toContain(privateValue); + expect(fetcher).toHaveBeenCalledTimes(1); + }, + ); + it.each([ + "private email and credentials", + JSON.stringify({ code: "private-address@example.test" }), + JSON.stringify({ code: ["missing_permission"] }), + JSON.stringify([{ code: "missing_permission" }]), + JSON.stringify({ message: "Forbidden" }), + JSON.stringify(null), + ])("does not copy unrecognized provider bodies into diagnostics (%#)", async (body) => { + const fetcher = vi.fn().mockResolvedValue(new Response(body, { status: 403 })); + await expect(agentmailApi("private-key", fetcher).whoami()).rejects.toMatchObject({ + status: 403, + operation: "inspect_key", + providerCode: "unknown", + message: "AgentMail request failed (403) [operation=inspect_key, code=unknown]", + }); + }); + it.each([ + ["GET", "/inboxes?limit=100", "list_inboxes"], + ["GET", "/inboxes/private%40example.test", "get_inbox"], + ["GET", "/domains?limit=100", "list_domains"], + ["GET", "/domains/private.example.test", "get_domain"], + ["POST", "/inboxes/private%40example.test/api-keys", "create_inbox_key"], + ["DELETE", "/inboxes/private%40example.test/api-keys/private-key", "delete_inbox_key"], + ["POST", "/inboxes/private%40example.test/webhooks", "create_webhook"], + ["DELETE", "/inboxes/private%40example.test/webhooks/private-hook", "delete_webhook"], + ["GET", "/inboxes/private%40example.test/messages?page_token=private-token", "list_messages"], + ["GET", "/inboxes/private%40example.test/messages/private-message", "get_message"], + ["GET", "/inboxes/private%40example.test/threads/private-thread", "get_thread"], + ["POST", "/inboxes/private%40example.test/messages/send", "send_message"], + ["POST", "/inboxes/private%40example.test/messages/private-message/reply", "reply_message"], + ["GET", "/inboxes/private%40example.test/messages/private-message/attachments/private-attachment", "get_attachment"], + ["POST", "/private-path?key=private-key", "request"], + ["PATCH", "/inboxes/private%40example.test", "request"], + ])("uses a fixed operation for %s route %#", async (method, path, operation) => { + const fetcher = vi.fn().mockResolvedValue(new Response(null, { status: 403 })); + const error = await agentmailApi("private-key", fetcher).request(path, method).catch((error: unknown) => error); + expect(error).toMatchObject({ operation, providerCode: "unknown" }); + expect(String(error)).not.toContain("private"); + expect(JSON.stringify(error)).not.toContain("private"); + }); + it("bounds the error body read and cancels the remaining stream", async () => { + const cancel = vi.fn(); + const response = new Response(new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(JSON.stringify({ code: "missing_permission", message: "x".repeat(8192) }))); + }, + cancel, + }), { status: 403 }); + await expect(agentmailApi("private-key", vi.fn().mockResolvedValue(response)).whoami()) + .rejects.toMatchObject({ status: 403, providerCode: "unknown" }); + expect(cancel).toHaveBeenCalledOnce(); + }); + it("keeps the HTTP status when the error body stalls or cancellation fails", async () => { + vi.useFakeTimers(); + try { + const cancel = vi.fn().mockRejectedValue(new Error("private provider error")); + const response = new Response(new ReadableStream({ cancel }), { status: 403 }); + const failure = expect(agentmailApi("private-key", vi.fn().mockResolvedValue(response)).whoami()) + .rejects.toMatchObject({ status: 403, providerCode: "unknown" }); + await vi.advanceTimersByTimeAsync(1000); + await failure; + expect(cancel).toHaveBeenCalledOnce(); + } finally { + vi.useRealTimers(); + } + }); + it("keeps retry classification when reading an error body fails", async () => { + const response = new Response(new ReadableStream({ + start(controller) { controller.error(new Error("private provider error")); }, + }), { status: 429, headers: { "retry-after": "9" } }); + const fetcher = vi.fn().mockResolvedValue(response); + await expect(agentmailApi("private-key", fetcher).send("private@example.test", {}, "private-key")) + .rejects.toMatchObject({ status: 429, retryAfterMs: 9000, providerCode: "unknown", operation: "send_message" }); + expect(fetcher).toHaveBeenCalledOnce(); + }); + it.each([403, 429])("preserves HTTP %s when the response body is already locked", async (status) => { + const response = new Response(JSON.stringify({ code: "missing_permission" }), { + status, + headers: { "retry-after": "9" }, + }); + const owner = response.body!.getReader(); + try { + await expect(agentmailApi("private-key", vi.fn().mockResolvedValue(response)) + .send("private@example.test", {}, "private-key")) + .rejects.toMatchObject({ status, retryAfterMs: 9000, providerCode: "unknown", operation: "send_message" }); + } finally { + owner.releaseLock(); + } + }); it("constructs deliberate reply-all from visible recipients, excluding self and Bcc", () => { const envelope = { from: "Sender ", diff --git a/server/src/services/agentmail-api.ts b/server/src/services/agentmail-api.ts index 7320e83e9e9..3d95934af78 100644 --- a/server/src/services/agentmail-api.ts +++ b/server/src/services/agentmail-api.ts @@ -42,13 +42,84 @@ export interface AgentmailScope { pod_id?: string; inbox_id?: string; } + +// Only these documented codes may leave the provider boundary. The other +// response fields (including `fix`) can contain addresses, URLs, or credentials. +const providerErrorCodes = [ + "missing_authorization", "invalid_token_type", "unknown_api_key", "unauthorized", + "missing_permission", "permission_escalation", "unrestricted_key_required", "forbidden", + "validation_error", "not_found", "unprocessable", "query_range_too_wide", + "already_exists", "resource_taken", "limit_exceeded", "domain_not_verified", + "conflict", "race_condition", "resource_deleting", "cannot_delete", "message_rejected", + "rate_limit_exceeded", "service_unavailable", "internal_error", +] as const; +type ProviderErrorCode = (typeof providerErrorCodes)[number] | "unknown"; +const providerOperations = [ + ["GET", /^\/auth\/me$/, "inspect_key"], + ["GET", /^\/inboxes$/, "list_inboxes"], + ["POST", /^\/inboxes$/, "create_inbox"], + ["GET", /^\/inboxes\/[^/]+$/, "get_inbox"], + ["GET", /^\/domains$/, "list_domains"], + ["GET", /^\/domains\/[^/]+$/, "get_domain"], + ["POST", /^\/inboxes\/[^/]+\/api-keys$/, "create_inbox_key"], + ["DELETE", /^\/inboxes\/[^/]+\/api-keys\/[^/]+$/, "delete_inbox_key"], + ["POST", /^\/inboxes\/[^/]+\/webhooks$/, "create_webhook"], + ["DELETE", /^\/inboxes\/[^/]+\/webhooks\/[^/]+$/, "delete_webhook"], + ["GET", /^\/inboxes\/[^/]+\/messages$/, "list_messages"], + ["GET", /^\/inboxes\/[^/]+\/messages\/[^/]+$/, "get_message"], + ["GET", /^\/inboxes\/[^/]+\/threads\/[^/]+$/, "get_thread"], + ["POST", /^\/inboxes\/[^/]+\/messages\/send$/, "send_message"], + ["POST", /^\/inboxes\/[^/]+\/messages\/[^/]+\/reply$/, "reply_message"], + ["GET", /^\/inboxes\/[^/]+\/messages\/[^/]+\/attachments\/[^/]+$/, "get_attachment"], +] as const; +type ProviderOperation = (typeof providerOperations)[number][2] | "request"; + +async function readProviderErrorCode(response: Response): Promise { + let reader: ReadableStreamDefaultReader | undefined; + let timeout: ReturnType | undefined; + try { + if (!response.body) return "unknown"; + const bodyReader = response.body.getReader(); + reader = bodyReader; + return await Promise.race([ + (async (): Promise => { + const parts: Uint8Array[] = []; + let bytes = 0; + for (;;) { + const part = await bodyReader.read(); + if (part.done) break; + bytes += part.value.length; + if (bytes > 8 * 1024) return "unknown"; + parts.push(part.value); + } + const body: unknown = JSON.parse(Buffer.concat(parts).toString("utf8")); + const code = body && typeof body === "object" && !Array.isArray(body) + ? (body as Record).code + : undefined; + return providerErrorCodes.find((known) => known === code) ?? "unknown"; + })(), + new Promise((resolve) => { + timeout = setTimeout(() => resolve("unknown"), 1000); + }), + ]); + } catch { + // A malformed, truncated, or interrupted body must not replace the HTTP error. + return "unknown"; + } finally { + clearTimeout(timeout); + void reader?.cancel().catch(() => {}); + } +} + export class AgentmailApiError extends Error { constructor( readonly status: number, readonly retryAfterMs = 1000, + readonly operation: ProviderOperation = "request", + readonly providerCode: ProviderErrorCode = "unknown", ) { // Provider bodies may contain credentials or private mail. Never log them. - super(`AgentMail request failed (${status})`); + super(`AgentMail request failed (${status}) [operation=${operation}, code=${providerCode}]`); } } export const AGENTMAIL_EVENTS = [ @@ -183,6 +254,10 @@ export function agentmailApi(apiKey: string, fetchImpl: typeof fetch = fetch) { throw new AgentmailApiError( response.status, Math.max(1000, Math.min(300_000, Number.isFinite(delay) ? delay : 1000)), + providerOperations.find(([verb, route]) => + verb === method && route.test(path.split("?")[0]), + )?.[2] ?? "request", + await readProviderErrorCode(response), ); } if (response.status === 204) return undefined as T; From 1d23cb6962f57c2f21a76d3402cec293d1a4dc76 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 30 Sep 2026 16:04:23 -0700 Subject: [PATCH 3/6] ci: pin a checkout-independent Rust cache path so PR lanes hit master's cache (#14394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip ships a native Runner binary, written in Rust, and seven CI lanes build it on every pull request > - `Canary Dry Run` is the slowest check on every green PR run, and most of its time is `cargo build --release` on third-party crates > - Master saves a Rust dependency cache for these lanes, but every PR lane logs `No cache found` and compiles every crate from zero > - The cache key matches, but GitHub also compares a hash of the absolute cache paths, and the master writer (RunsOn fleet, `/home/runner/_work/...`) and the PR readers (GitHub-hosted, `/home/runner/work/...`) hash different paths > - This pull request gives both sides a checkout-independent workspace path, so the hashes match and the PR lanes restore master's cache > - The benefit is about 2.5 minutes less wall clock per PR run and about 18 fewer runner-minutes per run ## Linked Issues or Issue Description No public issue exists for this problem. The description below follows the enhancement template. Related prior PRs on the same cache: Refs #13194, Refs #13259, Refs #13457, Refs #13459, Refs #13500, Refs #13586. None of them pins the workspace path, so none of them fixes this miss. **What existing behavior does this improve?** The `Swatinem/rust-cache` restore step in the PR workflow lanes that build the Runner: `Canary Dry Run`, `Build`, `Typecheck + Release Registry`, and the four `Verify Paperclip Runner` lanes. **Subsystem affected** CI workflows under `.github/workflows/`, their guard tests under `.github/scripts/tests/`, and `doc/RELEASE-AUTOMATION-SETUP.md`. **Current behavior** Every PR lane logs `No cache found` although master holds an entry with the exact key. Run 36424309181 computed `v0-rust-release-runner-v1-Linux-x64-c3a3ca66-a95b0328`, and master holds a 678 MB entry with that key. GitHub matches a cache entry on the key and on a version hash of the absolute paths in the cache. The master writer runs on the RunsOn fleet, where the checkout is `/home/runner/_work/paperclip/paperclip`. The PR readers run on GitHub-hosted `ubuntu-latest`, where the checkout is `/home/runner/work/paperclip/paperclip`. The stored version `5c40870d…` is the sha256 of the `_work` paths plus `zstd-without-long|1.0`. The `work` paths hash to `1656e9ee…`. The key can never match, so each lane compiles every third-party crate again. **Proposed behavior** The writer and the readers pass the same checkout-independent path to `rust-cache`. Both runner layouts then produce the same version hash, and the PR lanes restore master's cache. **Reason and benefit** `Canary Dry Run` takes 533s on a green run. 251s of that is dependency compilation that a warm cache removes. Seven lanes pay this cost in every PR run. **Breaking changes** None. This change affects CI only. ## What Changed - Add a `Pin the Runner Rust workspace path` step before `rust-cache` in the master writer (`release-verify.yml`, typecheck and runner lanes) and in all four PR readers (`pr-trusted.yml`). The step creates the symlink `$HOME/paperclip-runner-rust` → `$GITHUB_WORKSPACE/packages/paperclip-runner/runner` and passes that path to `rust-cache` as `workspaces: -> target`. `rust-cache` resolves the input with `path.resolve`, which does not follow symlinks, so both runner layouts now produce the same cache paths and the same version hash. `$HOME` is `/home/runner` on both images, which is why the `~/.cargo` paths already agreed. - Bump the shared keys `release-runner-v1` → `release-runner-v2` and `release-typecheck-v1` → `release-typecheck-v2`. The old, unreachable entries are then visibly orphaned instead of sharing a key with the new ones. - Extend the guard tests `pr-runner-rust-cache`, `release-runner-cache`, and `typecheck-rust-cache`. They now require the pin step in both workflows with identical text, placed before the cache step, and they reject a `workspaces:` value that resolves under the checkout. The `pr-runner-rust-cache` test checks all four PR reader jobs and fails if a `rust-cache` step appears in a PR job that is not in its reader list. - Update `doc/RELEASE-AUTOMATION-SETUP.md` to name the `release-runner-v2` key and to explain the pinned workspace path. ### Expected savings once merged Measured from run 36424309181. "Removed" is the dependency-compile time that a warm restore removes, minus about 18s to restore the 680 MB entry. The fleet writer's own restore shows this cost. | Lane | Today | Removed | Expected | |---|---|---|---| | Canary Dry Run | 533s | ~150s | ~380s | | Typecheck + Release Registry | 462s | ~155s | ~305s | | Verify Paperclip Runner (vitest 2/2) | 453s | ~245s | ~210s | | Verify Paperclip Runner (rust) | 400s | ~175s | ~225s | | Build | 348s | ~130s | ~220s | | Verify Paperclip Runner (static checks) | 321s | ~170s | ~150s | | Verify Paperclip Runner (vitest 1/2) | 346s | ~70s | ~275s | - Wall clock per PR run: about 533s → about 385s. That is about 2.5 minutes faster to a green check set. `Canary Dry Run` stays the longest check. The rest is the non-cargo work in `release.sh` (standalone package builds ~30s, publish-payload preview ~73s). - Runner time: about 18 runner-minutes saved per PR run across the seven lanes. - The first master push after merge compiles from zero once in the fleet writer (about 4 extra minutes on that one run) and saves the v2 entry. Later PRs hit it. When a PR changes `Cargo.lock`, the prefix restore key still gives a partial hit, as before. ## Verification - Run the guard tests for the three cache lanes: `node --test .github/scripts/tests/pr-runner-rust-cache.test.mjs .github/scripts/tests/release-runner-cache.test.mjs .github/scripts/tests/typecheck-rust-cache.test.mjs` Result: 21 pass, 0 fail. - Run the full guard suite: `node --test '.github/scripts/tests/*.test.mjs'`. Result: 376 pass, 3 fail. The 3 failures are in `docker-canary-promotion.test.mjs`. They hit a sandbox temp-file ENOENT and fail the same way on the unmodified branch. - Run `node --test scripts/__tests__/release-verify-workflow.test.mjs`. Result: 14 pass. - Local archive test: create a tar from the `_work` layout through the symlink (relative `../../../paperclip-runner-rust/target` entries, `tar -P -C $GITHUB_WORKSPACE`, the same way `@actions/cache` does). Extract it on the `work` layout. The files land in the real target directory and the symlink stays intact. - After merge, open any GitHub-hosted PR run and confirm that the seven Rust lanes log `Restored from cache key ...release-runner-v2...` in place of `No cache found`. ## Risks - Low risk. The change touches CI workflows, their tests, and one doc page. No product code changes. - If the pin step fails, `rust-cache` reports a miss and the lane compiles from zero, as it does today. The build does not break. - Both runner layouts sit four levels under `/home/runner`, so the relative `../../../` archive entries line up. The existing `~/.cargo/registry` and `~/.cargo/git` cache paths already rely on this property. A future runner image with a different `$HOME` depth would miss the cache but would not fail the job. - `rm -rf "$pinned"` acts on the symlink itself (no trailing slash), never on the checkout behind it. It only matters on a reused runner. - Squash-merge note: the branch carries commits by `Bender (Fable)`. Add `Co-Authored-By: Bender (Fable) ` to the squash body to keep that authorship. ## Model Used - Anthropic Claude Fable 5.1 (`claude-fable-5-1`), run through Claude Code inside a Paperclip agent heartbeat. Extended thinking was on. Tool use: shell, GitHub CLI, and the GitHub REST API for workflow logs, cache listings, and PR operations. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [ ] My branch name describes the change and contains no internal ticket id. The agent execution workspace fixed this branch name, so I cannot rename it. Squash-merge drops the branch name. - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 Co-authored-by: Bender (Fable) --- .../tests/pr-runner-rust-cache.test.mjs | 162 ++++++++++++++---- .../tests/release-runner-cache.test.mjs | 9 +- .../tests/typecheck-rust-cache.test.mjs | 27 ++- .github/workflows/pr-trusted.yml | 106 +++++++++++- .github/workflows/release-verify.yml | 50 +++++- doc/RELEASE-AUTOMATION-SETUP.md | 13 +- doc/cloud-build-readiness.md | 9 +- 7 files changed, 317 insertions(+), 59 deletions(-) diff --git a/.github/scripts/tests/pr-runner-rust-cache.test.mjs b/.github/scripts/tests/pr-runner-rust-cache.test.mjs index 19870724425..6089bcec638 100644 --- a/.github/scripts/tests/pr-runner-rust-cache.test.mjs +++ b/.github/scripts/tests/pr-runner-rust-cache.test.mjs @@ -5,8 +5,41 @@ import { readFileSync } from "node:fs"; const read = (name) => readFileSync(new URL(`../../workflows/${name}`, import.meta.url), "utf8"); const prWorkflow = read("pr-trusted.yml"); const releaseWorkflow = read("release-verify.yml"); -const pr = prWorkflow.split(" verify_paperclip_runner:")[1].split(" build:")[0]; -const release = releaseWorkflow.split(" verify_paperclip_runner:")[1].split(" build:")[0]; + +// Slice one job out of a workflow: from its two-space-indented key to the +// next one. Steps sit at six spaces and `with:` at eight, so only job keys +// match the boundary pattern. +const job = (workflow, name) => { + const start = workflow.indexOf(`\n ${name}:\n`); + assert.ok(start >= 0, `workflow is missing the ${name} job`); + const body = workflow.slice(start + 1); + const header = ` ${name}:\n`.length; + const next = body.slice(header).search(/\n [a-z0-9_-]+:\n/); + return next === -1 ? body : body.slice(0, header + next + 1); +}; + +const READ_STEP = " - name: Restore Runner Rust dependencies (read only)"; +const WRITE_STEP = " - name: Cache Runner Rust dependencies"; +const SELECT_STEP = " - name: Select the pinned Runner Rust toolchain"; + +// Every PR job that builds the Runner restores master's `release-runner-v2` +// entry. Each one has to agree with the writer on every key input, or that +// one lane misses and silently recompiles every third-party crate while the +// others hit. +const READER_JOBS = ["typecheck_release_registry", "verify_paperclip_runner", "build", "canary_dry_run"]; +const readers = READER_JOBS.map((name) => [name, job(prWorkflow, name)]); +const release = job(releaseWorkflow, "verify_paperclip_runner"); + +test("every rust-cache restore in the PR workflow belongs to a guarded reader job", () => { + const total = prWorkflow.match(/uses: Swatinem\/rust-cache@/g)?.length ?? 0; + const guarded = readers.filter(([, body]) => /uses: Swatinem\/rust-cache@/.test(body)).length; + assert.equal(guarded, READER_JOBS.length, "each listed reader job must restore the Rust cache"); + assert.equal(total, guarded, "a job restores the Rust cache but is not in READER_JOBS; add it so it is guarded"); + for (const [name, body] of readers) { + assert.equal(body.match(/uses: Swatinem\/rust-cache@/g).length, 1, `${name}: exactly one rust-cache step`); + assert.ok(body.includes(READ_STEP), `${name}: the rust-cache step must be the read-only restore`); + } +}); // The key is computed from these inputs. A pull request that disagrees with // the master writer on any of them misses every time and silently recompiles @@ -14,43 +47,51 @@ const release = releaseWorkflow.split(" verify_paperclip_runner:")[1].split(" // restore exists to remove. const keyInputs = [ /uses: Swatinem\/rust-cache@([0-9a-f]{40}) # v[0-9.]+/, - /workspaces: (packages\/paperclip-runner\/runner -> target)/, - /shared-key: (release-runner-v1)/, + /workspaces: (\$\{\{ steps\.runner_rust_workspace\.outputs\.path \}\} -> target)/, + /shared-key: (release-runner-v2)/, /cache-workspace-crates: (false)/, /cache-bin: (false)/, ]; -test("the PR lane restores the Rust cache under the same key the master push writes", () => { - for (const pattern of keyInputs) { - const mine = pr.match(pattern); - const theirs = release.match(pattern); - assert.ok(mine, `PR lane is missing ${pattern}`); - assert.ok(theirs, `master writer is missing ${pattern}`); - assert.equal(mine[1], theirs[1], `key input drifted from the master writer: ${pattern}`); +test("every PR reader restores the Rust cache under the same key the master push writes", () => { + for (const [name, pr] of readers) { + for (const pattern of keyInputs) { + const mine = pr.match(pattern); + const theirs = release.match(pattern); + assert.ok(mine, `${name}: PR lane is missing ${pattern}`); + assert.ok(theirs, `master writer is missing ${pattern}`); + assert.equal(mine[1], theirs[1], `${name}: key input drifted from the master writer: ${pattern}`); + } + assert.doesNotMatch(pr, /prefix-key:|cache-on-failure: true|cache-all-crates: true/, name); } - assert.doesNotMatch(pr, /prefix-key:|cache-on-failure: true|cache-all-crates: true/); }); -test("the PR lane pins the compiler before the key is computed", () => { - const select = pr.indexOf(" - name: Select the pinned Runner Rust toolchain"); - const cache = pr.indexOf(" - name: Restore Runner Rust dependencies (read only)"); - const verify = pr.indexOf(" - name: Verify Paperclip Runner\n"); - assert.ok(select >= 0 && cache > select && verify > cache); - const setup = pr.slice(select, cache); - assert.match(setup, /working-directory: packages\/paperclip-runner/); - assert.match(setup, /rustup show active-toolchain/); - assert.match(setup, /echo "RUSTUP_TOOLCHAIN=\$toolchain" >> "\$GITHUB_ENV"/); - // The gate routes to either ubuntu-latest or the public PR fleet, so a - // missing rustup must cost the cache, never the pull request. - assert.match(setup, /command -v rustup/); - assert.doesNotMatch(setup, /set -euo pipefail/); +test("every PR reader pins the compiler before the key is computed", () => { + for (const [name, pr] of readers) { + const select = pr.indexOf(SELECT_STEP); + const cache = pr.indexOf(READ_STEP); + assert.ok(select >= 0 && cache > select, `${name}: the toolchain must be selected before the cache step`); + const setup = pr.slice(select, cache); + // Nothing may sit between the pin and the restore that could change the key. + assert.equal(setup.match(/^ - name: /gm).length, 1, `${name}: no step between the toolchain pin and the restore`); + assert.match(setup, /working-directory: packages\/paperclip-runner/, name); + assert.match(setup, /rustup show active-toolchain/, name); + assert.match(setup, /echo "RUSTUP_TOOLCHAIN=\$toolchain" >> "\$GITHUB_ENV"/, name); + // The gate routes to either ubuntu-latest or the public PR fleet, so a + // missing rustup must cost the cache, never the pull request. + assert.match(setup, /command -v rustup/, name); + assert.doesNotMatch(setup, /set -euo pipefail/, name); + } }); test("a pull request never writes to or evicts the master cache entry", () => { - const step = pr.split(" - name: Restore Runner Rust dependencies (read only)")[1] - .split(" - name: Verify Paperclip Runner\n")[0]; - assert.equal(step.match(/^\s*save-if: (.+)$/m)?.[1], "false"); - assert.doesNotMatch(step, /^\s*if:/m, "the restore must not be conditional; a miss is already free"); + for (const [name, pr] of readers) { + const after = pr.split(READ_STEP)[1]; + const nextStep = after.search(/\n - name: /); + const step = nextStep === -1 ? after : after.slice(0, nextStep); + assert.equal(step.match(/^\s*save-if: (.+)$/m)?.[1], "false", `${name}: the restore must set save-if: false`); + assert.doesNotMatch(step, /^\s*if:/m, `${name}: the restore must not be conditional; a miss is already free`); + } assert.doesNotMatch(prWorkflow, /uses: Swatinem\/rust-cache@[0-9a-f]{40}[\s\S]*?save-if: (?!false)/); }); @@ -62,28 +103,75 @@ test("a pull request never writes to or evicts the master cache entry", () => { // the key matches nothing and every run recompiles. const NORMALIZE = /# rust-cache hashes every installed toolchain[\s\S]*?rustup toolchain list\n/; -test("reader and writer strip extra toolchains identically before the key is computed", () => { - const mine = pr.match(NORMALIZE); +test("every reader and the writer strip extra toolchains identically before the key is computed", () => { const theirs = release.match(NORMALIZE); - assert.ok(mine, "pr-trusted.yml must normalize the installed toolchains"); assert.ok(theirs, "release-verify.yml must normalize the installed toolchains"); - assert.equal(mine[0], theirs[0], "the normalization must be identical in both workflows"); - - for (const [name, body] of [["reader", mine[0]], ["writer", theirs[0]]]) { + for (const [name, body] of [["writer", theirs[0]]]) { // Keep the pin, drop the rest, and never fail the job over it. assert.match(body, /grep -vx "\$toolchain"/, name); assert.match(body, /xargs -n1 rustup toolchain uninstall/, name); assert.match(body, /\|\| true/, name); } + for (const [name, pr] of readers) { + const mine = pr.match(NORMALIZE); + assert.ok(mine, `${name}: pr-trusted.yml must normalize the installed toolchains`); + assert.equal(mine[0], theirs[0], `${name}: the normalization must be identical to the writer's`); + } }); test("the toolchain is stripped before the cache step, not after", () => { for (const [name, body, cacheStep] of [ - ["reader", pr, " - name: Restore Runner Rust dependencies (read only)"], - ["writer", release, " - name: Cache Runner Rust dependencies"], + ...readers.map(([name, body]) => [name, body, READ_STEP]), + ["writer", release, WRITE_STEP], ]) { const normalize = body.search(NORMALIZE); const cache = body.indexOf(cacheStep); assert.ok(normalize >= 0 && cache > normalize, `${name}: normalization must precede the cache step`); } }); + +// GitHub matches a cache entry on its key and on a version hash of the +// absolute paths being cached. rust-cache resolves the target directory under +// the checkout, and the checkout root differs by runner (/home/runner/_work on +// the RunsOn fleets that write the cache, /home/runner/work on GitHub-hosted +// runners). With every key input aligned, every GitHub-hosted pull request +// still logged "No cache found" (run 36424309181, 2026-09-28). Both workflows +// therefore hand rust-cache the same checkout-independent path, and they have +// to build it the same way or the version matches nothing. +const PIN = /# rust-cache hashes its absolute cache paths[\s\S]*?echo "path=\$pinned" >> "\$GITHUB_OUTPUT"\n/; + +test("every reader and the writer pin an identical checkout-independent Rust workspace path", () => { + const theirs = release.match(PIN); + assert.ok(theirs, "release-verify.yml must pin the Runner Rust workspace path"); + const pins = [["writer", theirs[0]]]; + for (const [name, pr] of readers) { + const mine = pr.match(PIN); + assert.ok(mine, `${name}: pr-trusted.yml must pin the Runner Rust workspace path`); + assert.equal(mine[0], theirs[0], `${name}: the pinned path must be built identically to the writer's`); + pins.push([name, mine[0]]); + } + + for (const [name, body] of pins) { + assert.match(body, /- name: Pin the Runner Rust workspace path\n\s+id: runner_rust_workspace\n/, name); + // Anchor under $HOME, which both runner images share, never under the checkout. + assert.match(body, /pinned="\$HOME\/[A-Za-z0-9._-]+"/, name); + assert.match(body, /rm -rf "\$pinned"\n\s+ln -s "\$GITHUB_WORKSPACE\/packages\/paperclip-runner\/runner" "\$pinned"/, name); + assert.match(body, /echo "path=\$pinned" >> "\$GITHUB_OUTPUT"/, name); + } +}); + +test("the workspace path is pinned before the cache step and never names the checkout", () => { + for (const [name, body, cacheStep] of [ + ...readers.map(([name, body]) => [name, body, READ_STEP]), + ["writer", release, WRITE_STEP], + ]) { + const pin = body.search(PIN); + const cache = body.indexOf(cacheStep); + assert.ok(pin >= 0 && cache > pin, `${name}: the pin must precede the cache step`); + assert.doesNotMatch(body, /workspaces: (\.|packages\/|\$\{\{ github\.workspace)/, `${name}: workspaces must not resolve under the checkout`); + } + // No rust-cache step anywhere in either workflow may point back into the checkout. + for (const [name, workflow] of [["pr-trusted.yml", prWorkflow], ["release-verify.yml", releaseWorkflow]]) { + assert.doesNotMatch(workflow, /workspaces: (\.|packages\/|\$\{\{ github\.workspace)/, `${name}: workspaces must not resolve under the checkout`); + } +}); diff --git a/.github/scripts/tests/release-runner-cache.test.mjs b/.github/scripts/tests/release-runner-cache.test.mjs index 7e35ec09c99..96bba1f3250 100644 --- a/.github/scripts/tests/release-runner-cache.test.mjs +++ b/.github/scripts/tests/release-runner-cache.test.mjs @@ -15,8 +15,13 @@ test("Runner dependency caching selects the package's pinned compiler before com assert.match(setup, /rustup show active-toolchain/); assert.match(setup, /echo "RUSTUP_TOOLCHAIN=\$toolchain" >> "\$GITHUB_ENV"/); assert.match(runner, /uses: Swatinem\/rust-cache@[0-9a-f]{40} # v[0-9.]+/); - assert.match(runner, /workspaces: packages\/paperclip-runner\/runner -> target/); - assert.match(runner, /shared-key: release-runner-v1/); + // The target path feeds the entry's version hash, so it must not resolve + // under the checkout, whose root differs between the fleet and GitHub-hosted + // runners. The pin step publishes a $HOME-anchored path to the same directory. + const pin = runner.indexOf(" - name: Pin the Runner Rust workspace path"); + assert.ok(pin >= 0 && cache > pin, "the workspace path must be pinned before the cache step"); + assert.match(runner, /workspaces: \$\{\{ steps\.runner_rust_workspace\.outputs\.path \}\} -> target/); + assert.match(runner, /shared-key: release-runner-v2/); }); test("the shared cache excludes workspace artifacts and only restores or saves the exact master-push source", () => { diff --git a/.github/scripts/tests/typecheck-rust-cache.test.mjs b/.github/scripts/tests/typecheck-rust-cache.test.mjs index c8ff43b8802..6f975cc77a1 100644 --- a/.github/scripts/tests/typecheck-rust-cache.test.mjs +++ b/.github/scripts/tests/typecheck-rust-cache.test.mjs @@ -28,11 +28,32 @@ for (const [name, overrides, ref, allowed] of [ } test("cache excludes workspace code and executable installs, and preserves full checks", () => { assert.match(cache, /uses: Swatinem\/rust-cache@[a-f0-9]{40}/); - assert.match(cache, /workspaces: packages\/paperclip-runner\/runner -> target/); - assert.match(cache, /shared-key: release-typecheck-v1/); + // The target path feeds the entry's version hash; a checkout-relative path + // hashes differently on the fleet (/home/runner/_work) and on GitHub-hosted + // runners (/home/runner/work), so the pin step publishes a $HOME-anchored one. + assert.match(cache, /workspaces: \$\{\{ steps\.runner_rust_workspace\.outputs\.path \}\} -> target/); + assert.match(cache, /shared-key: release-typecheck-v2/); assert.match(cache, /cache-workspace-crates: false/); assert.match(cache, /cache-bin: false/); + const pin = typecheck.indexOf(" - name: Pin the Runner Rust workspace path"); + assert.ok(pin >= 0 && pin < typecheck.indexOf("uses: Swatinem/rust-cache")); assert.ok(typecheck.indexOf('echo "RUSTUP_TOOLCHAIN=$toolchain"') < typecheck.indexOf("uses: Swatinem/rust-cache")); + assert.doesNotMatch(typecheck, /workspaces: (\.|packages\/|\$\{\{ github\.workspace)/); +}); +// The typecheck key is written and read only by master pushes on the fleet, so +// its version would still match if this pin drifted. Hold it to the same text +// as the Runner writer anyway: `doc/RELEASE-AUTOMATION-SETUP.md` promises one +// identical pin step before every Rust cache step, and a divergent copy here +// is the first place a later edit would quietly break that promise. +test("the typecheck pin step is identical to the Runner writer's", () => { + const PIN = /# rust-cache hashes its absolute cache paths[\s\S]*?echo "path=\$pinned" >> "\$GITHUB_OUTPUT"\n/; + const writer = workflow.split("\n verify_paperclip_runner:\n")[1]; + assert.ok(writer, "release-verify.yml is missing the verify_paperclip_runner job"); + const theirs = writer.match(PIN); + const mine = typecheck.match(PIN); + assert.ok(theirs, "the Runner writer must pin the Runner Rust workspace path"); + assert.ok(mine, "the typecheck job must pin the Runner Rust workspace path"); + assert.equal(mine[0], theirs[0], "the typecheck pin step drifted from the Runner writer's"); assert.match(typecheck, /run: pnpm -r typecheck/); - assert.match(workflow, /shared-key: release-runner-v1/); + assert.match(workflow, /shared-key: release-runner-v2/); }); diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index 7ca7bcac672..b87e5115b9c 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -407,6 +407,28 @@ jobs: # rebuilds the Runner release binary; without the shared Rust cache that # is a ~3m40s cold compile of all third-party crates (run 35036001734, # 2026-09-15). Same restore-only contract as Verify Paperclip Runner. + + # rust-cache hashes its absolute cache paths into the entry's version, + # and GitHub only serves an entry whose key and version both match. The + # target directory sits under the checkout, and the checkout root + # differs by runner: /home/runner/_work on the RunsOn fleets that write + # this cache against /home/runner/work on GitHub-hosted runners. Every + # key input agreed, yet all 25 completed pull requests on 2026-09-28 + # logged "No cache found" (run 36424309181) and cold-compiled every + # crate for ~4 minutes in four lanes. Point rust-cache at the same + # directory through a checkout-independent path so both layouts hash + # the same version. Keep this block identical in both workflows: the + # reader and the writer must agree or the version matches nothing. + - name: Pin the Runner Rust workspace path + id: runner_rust_workspace + run: | + set -euo pipefail + pinned="$HOME/paperclip-runner-rust" + # A symlink here is removed as a link, never followed into the checkout. + rm -rf "$pinned" + ln -s "$GITHUB_WORKSPACE/packages/paperclip-runner/runner" "$pinned" + echo "path=$pinned" >> "$GITHUB_OUTPUT" + - name: Select the pinned Runner Rust toolchain working-directory: packages/paperclip-runner run: | @@ -445,8 +467,8 @@ jobs: - name: Restore Runner Rust dependencies (read only) uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: - workspaces: packages/paperclip-runner/runner -> target - shared-key: release-runner-v1 + workspaces: ${{ steps.runner_rust_workspace.outputs.path }} -> target + shared-key: release-runner-v2 # Mirror the master writer: these also feed the cache key. cache-workspace-crates: false cache-bin: false @@ -766,11 +788,33 @@ jobs: # Same restore-only contract as the pnpm store above, for the Rust # dependency tree: master's post-merge verification is the sole writer - # of release-runner-v1, and PR merge refs must not save branch-scoped + # of release-runner-v2, and PR merge refs must not save branch-scoped # copies of a ~680MB target directory. Every key input below has to # match that writer in release-verify.yml exactly or each PR misses and # recompiles all 313 third-party crates in both profiles. A miss is a # slow run, never a wrong one. + + # rust-cache hashes its absolute cache paths into the entry's version, + # and GitHub only serves an entry whose key and version both match. The + # target directory sits under the checkout, and the checkout root + # differs by runner: /home/runner/_work on the RunsOn fleets that write + # this cache against /home/runner/work on GitHub-hosted runners. Every + # key input agreed, yet all 25 completed pull requests on 2026-09-28 + # logged "No cache found" (run 36424309181) and cold-compiled every + # crate for ~4 minutes in four lanes. Point rust-cache at the same + # directory through a checkout-independent path so both layouts hash + # the same version. Keep this block identical in both workflows: the + # reader and the writer must agree or the version matches nothing. + - name: Pin the Runner Rust workspace path + id: runner_rust_workspace + run: | + set -euo pipefail + pinned="$HOME/paperclip-runner-rust" + # A symlink here is removed as a link, never followed into the checkout. + rm -rf "$pinned" + ln -s "$GITHUB_WORKSPACE/packages/paperclip-runner/runner" "$pinned" + echo "path=$pinned" >> "$GITHUB_OUTPUT" + - name: Select the pinned Runner Rust toolchain working-directory: packages/paperclip-runner run: | @@ -809,8 +853,8 @@ jobs: - name: Restore Runner Rust dependencies (read only) uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: - workspaces: packages/paperclip-runner/runner -> target - shared-key: release-runner-v1 + workspaces: ${{ steps.runner_rust_workspace.outputs.path }} -> target + shared-key: release-runner-v2 # Mirror the master writer: these also feed the cache key. cache-workspace-crates: false cache-bin: false @@ -884,6 +928,28 @@ jobs: # shared Rust cache that is a ~3m40s cold compile of all third-party # crates (run 35036001734, 2026-09-15). Same restore-only contract as # Verify Paperclip Runner. + + # rust-cache hashes its absolute cache paths into the entry's version, + # and GitHub only serves an entry whose key and version both match. The + # target directory sits under the checkout, and the checkout root + # differs by runner: /home/runner/_work on the RunsOn fleets that write + # this cache against /home/runner/work on GitHub-hosted runners. Every + # key input agreed, yet all 25 completed pull requests on 2026-09-28 + # logged "No cache found" (run 36424309181) and cold-compiled every + # crate for ~4 minutes in four lanes. Point rust-cache at the same + # directory through a checkout-independent path so both layouts hash + # the same version. Keep this block identical in both workflows: the + # reader and the writer must agree or the version matches nothing. + - name: Pin the Runner Rust workspace path + id: runner_rust_workspace + run: | + set -euo pipefail + pinned="$HOME/paperclip-runner-rust" + # A symlink here is removed as a link, never followed into the checkout. + rm -rf "$pinned" + ln -s "$GITHUB_WORKSPACE/packages/paperclip-runner/runner" "$pinned" + echo "path=$pinned" >> "$GITHUB_OUTPUT" + - name: Select the pinned Runner Rust toolchain working-directory: packages/paperclip-runner run: | @@ -922,8 +988,8 @@ jobs: - name: Restore Runner Rust dependencies (read only) uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: - workspaces: packages/paperclip-runner/runner -> target - shared-key: release-runner-v1 + workspaces: ${{ steps.runner_rust_workspace.outputs.path }} -> target + shared-key: release-runner-v2 # Mirror the master writer: these also feed the cache key. cache-workspace-crates: false cache-bin: false @@ -1094,6 +1160,28 @@ jobs: # build:binary step; without the shared Rust cache that is a ~3m40s cold # compile of all third-party crates (run 35036001734, 2026-09-15). Same # restore-only contract as Verify Paperclip Runner. + + # rust-cache hashes its absolute cache paths into the entry's version, + # and GitHub only serves an entry whose key and version both match. The + # target directory sits under the checkout, and the checkout root + # differs by runner: /home/runner/_work on the RunsOn fleets that write + # this cache against /home/runner/work on GitHub-hosted runners. Every + # key input agreed, yet all 25 completed pull requests on 2026-09-28 + # logged "No cache found" (run 36424309181) and cold-compiled every + # crate for ~4 minutes in four lanes. Point rust-cache at the same + # directory through a checkout-independent path so both layouts hash + # the same version. Keep this block identical in both workflows: the + # reader and the writer must agree or the version matches nothing. + - name: Pin the Runner Rust workspace path + id: runner_rust_workspace + run: | + set -euo pipefail + pinned="$HOME/paperclip-runner-rust" + # A symlink here is removed as a link, never followed into the checkout. + rm -rf "$pinned" + ln -s "$GITHUB_WORKSPACE/packages/paperclip-runner/runner" "$pinned" + echo "path=$pinned" >> "$GITHUB_OUTPUT" + - name: Select the pinned Runner Rust toolchain working-directory: packages/paperclip-runner run: | @@ -1132,8 +1220,8 @@ jobs: - name: Restore Runner Rust dependencies (read only) uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: - workspaces: packages/paperclip-runner/runner -> target - shared-key: release-runner-v1 + workspaces: ${{ steps.runner_rust_workspace.outputs.path }} -> target + shared-key: release-runner-v2 # Mirror the master writer: these also feed the cache key. cache-workspace-crates: false cache-bin: false diff --git a/.github/workflows/release-verify.yml b/.github/workflows/release-verify.yml index 5144e6b52aa..76e5cd28bb3 100644 --- a/.github/workflows/release-verify.yml +++ b/.github/workflows/release-verify.yml @@ -42,6 +42,27 @@ jobs: node-version: 24 cache: pnpm + # rust-cache hashes its absolute cache paths into the entry's version, + # and GitHub only serves an entry whose key and version both match. The + # target directory sits under the checkout, and the checkout root + # differs by runner: /home/runner/_work on the RunsOn fleets that write + # this cache against /home/runner/work on GitHub-hosted runners. Every + # key input agreed, yet all 25 completed pull requests on 2026-09-28 + # logged "No cache found" (run 36424309181) and cold-compiled every + # crate for ~4 minutes in four lanes. Point rust-cache at the same + # directory through a checkout-independent path so both layouts hash + # the same version. Keep this block identical in both workflows: the + # reader and the writer must agree or the version matches nothing. + - name: Pin the Runner Rust workspace path + id: runner_rust_workspace + run: | + set -euo pipefail + pinned="$HOME/paperclip-runner-rust" + # A symlink here is removed as a link, never followed into the checkout. + rm -rf "$pinned" + ln -s "$GITHUB_WORKSPACE/packages/paperclip-runner/runner" "$pinned" + echo "path=$pinned" >> "$GITHUB_OUTPUT" + - name: Select the pinned Runner Rust toolchain working-directory: packages/paperclip-runner run: | @@ -56,8 +77,8 @@ jobs: if: ${{ github.repository == 'paperclipai/paperclip' && github.event_name == 'push' && github.ref == 'refs/heads/master' && inputs.ref == github.sha }} uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: - workspaces: packages/paperclip-runner/runner -> target - shared-key: release-typecheck-v1 + workspaces: ${{ steps.runner_rust_workspace.outputs.path }} -> target + shared-key: release-typecheck-v2 # Rebuild workspace code and rerun every check. Cache only compiled # dependencies; never restore installed executables from cargo/bin. cache-workspace-crates: false @@ -294,6 +315,27 @@ jobs: node-version: 24 cache: pnpm + # rust-cache hashes its absolute cache paths into the entry's version, + # and GitHub only serves an entry whose key and version both match. The + # target directory sits under the checkout, and the checkout root + # differs by runner: /home/runner/_work on the RunsOn fleets that write + # this cache against /home/runner/work on GitHub-hosted runners. Every + # key input agreed, yet all 25 completed pull requests on 2026-09-28 + # logged "No cache found" (run 36424309181) and cold-compiled every + # crate for ~4 minutes in four lanes. Point rust-cache at the same + # directory through a checkout-independent path so both layouts hash + # the same version. Keep this block identical in both workflows: the + # reader and the writer must agree or the version matches nothing. + - name: Pin the Runner Rust workspace path + id: runner_rust_workspace + run: | + set -euo pipefail + pinned="$HOME/paperclip-runner-rust" + # A symlink here is removed as a link, never followed into the checkout. + rm -rf "$pinned" + ln -s "$GITHUB_WORKSPACE/packages/paperclip-runner/runner" "$pinned" + echo "path=$pinned" >> "$GITHUB_OUTPUT" + - name: Select the pinned Runner Rust toolchain working-directory: packages/paperclip-runner run: | @@ -325,8 +367,8 @@ jobs: if: ${{ github.repository == 'paperclipai/paperclip' && github.event_name == 'push' && github.ref == 'refs/heads/master' && inputs.ref == github.sha }} uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 with: - workspaces: packages/paperclip-runner/runner -> target - shared-key: release-runner-v1 + workspaces: ${{ steps.runner_rust_workspace.outputs.path }} -> target + shared-key: release-runner-v2 # Rebuild workspace code and rerun every check. Cache only compiled # dependencies; never restore installed executables from cargo/bin. cache-workspace-crates: false diff --git a/doc/RELEASE-AUTOMATION-SETUP.md b/doc/RELEASE-AUTOMATION-SETUP.md index 5e5afc481da..25890751a4d 100644 --- a/doc/RELEASE-AUTOMATION-SETUP.md +++ b/doc/RELEASE-AUTOMATION-SETUP.md @@ -347,11 +347,22 @@ readiness can succeed. A failed lane does not cancel the other lane. Both lanes restore Cargo dependencies with the pinned Rust Cache action. The compiler comes from the Runner package's `rust-toolchain.toml` before the action computes its key. Compiler and Cargo metadata changes select a new cache. The -existing `release-runner-v1` shared key avoids separate copies for these lanes. +`release-runner-v2` shared key avoids separate copies for these lanes. Only the Rust lane saves this cache. After verification it also runs `build:rust` to warm the debug dependencies used by the protocol lane; its own tests already warm release dependencies. The cache writer is shorter than the protocol lane. +GitHub matches a cache entry on its key and on a version hash of the absolute +paths in the entry. The master writer runs on the RunsOn fleet, where the +checkout is `/home/runner/_work/paperclip/paperclip`. The trusted PR workflow +restores the same entry read-only on GitHub-hosted runners, where the checkout +is `/home/runner/work/paperclip/paperclip`. A `Pin the Runner Rust workspace +path` step in both workflows links `$HOME/paperclip-runner-rust` to the Runner +crate and passes that path to the cache action, so both layouts hash the same +paths and the PR lanes can restore master's entry. The guard tests under +`.github/scripts/tests/` require this step, with identical text, before every +Rust cache step in both workflows. + Workspace crates and installed Cargo binaries are excluded. Every run rebuilds workspace code and runs all assigned checks, including on a cache hit. Only an own-repository master-push run verifying that push's exact SHA can restore the diff --git a/doc/cloud-build-readiness.md b/doc/cloud-build-readiness.md index 2a08902fd32..df143f11eca 100644 --- a/doc/cloud-build-readiness.md +++ b/doc/cloud-build-readiness.md @@ -158,10 +158,13 @@ shared infrastructure ownership separately before removing those resources. Source verification's typecheck job builds the native Runner binary through the server's `prepare:runner-vendor` command. It restores and saves compiled Rust dependencies only for canonical master pushes that verify the event's exact SHA. -The `release-typecheck-v1` cache is separate from Runner verification because +The `release-typecheck-v2` cache is separate from Runner verification because those jobs compile different profiles. The pinned toolchain is selected before -cache lookup. Workspace crates and installed cargo binaries are excluded, and -all typechecks still execute. A missing or invalidated cache triggers compilation. +cache lookup, and the cache action receives the Runner crate through the same +checkout-independent `$HOME/paperclip-runner-rust` path as the Runner lanes +(see `RELEASE-AUTOMATION-SETUP.md`). Workspace crates and installed cargo +binaries are excluded, and all typechecks still execute. A missing or +invalidated cache triggers compilation. ### pnpm dependency store cache From f7e36ba3e2e22ef93796042e89795817d57fbc67 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:06:07 -0500 Subject: [PATCH 4/6] fix: isolate repository-free low-trust tasks in private directories (#14766) ## Thinking Path > - Paperclip manages work by agents within company boundaries. > - Email tasks can run under the low-trust review preset. > - These tasks must use an isolated workspace and a sandbox. > - The default workspace strategy assumed that the project had a Git repository. > - A project without a configured workspace failed before the agent could start. > - This change gives each such task a private directory and keeps the sandbox requirement. ## Linked Issues or Issue Description **What happened?** An inbound email assigned to a low-trust agent failed with `git_worktree_base_not_git_checkout` when its boundary project had no configured workspace. Setup had accepted the project and sandbox. **Expected behavior** The agent can process email without a repository. Its workspace stays isolated from other tasks and the shared agent home. **Steps to reproduce** 1. Select a low-trust agent with an active sandbox and a project boundary. 2. Leave the project without a configured workspace. 3. Receive an email through AgentMail. 4. Observe that startup fails before provider work starts. **Paperclip version or commit** Reproduced against `5edf55d73`. **Deployment mode** Hosted staging with sandbox execution. Related: #13256 added email tasks. #13636 fixed default isolation for projects without workspaces; the explicit isolation used by low-trust tasks still needed this path. ## What Changed - Select private task directories for low-trust sandbox tasks with no configured workspace or explicit workspace strategy. - Keep each directory scoped to its company and task. Retain files across turns and reassignment and reject symlink paths and mismatched workspace reuse. - Preserve Git validation for configured workspaces and explicit strategies, plus the existing authorization and remote gates for referenced projects. - Add a startup regression and directory isolation tests. Document the supported repository-free path. ## Verification - The startup regression failed before the fix with the same Git validation error. - 260 targeted email, workspace policy, heartbeat, referenced-project and directory tests pass. - Full `pnpm -r typecheck` and `pnpm build` pass on the latest commit. - All CI checks, including the complete sharded test suite and canary dry run, pass on `b4ccd9802b09b2e95499df72d48b4a3906b8c328`. - The final commit also passes the same server shard locally: 60 files, 1,024 passed / 6 skipped tests. The earlier all-groups local run was interrupted during follow-up edits; complete-suite verification comes from CI on the final commit. - Deployed the reviewed commit to staging and independently verified the full serving SHA. Two real Codex runs in Daytona succeeded and finalized the same private company/task workspace. The first wrote a 35-byte marker; the second read the existing file without modifying it and returned the independently verified SHA-256 `ce3bbeb44d07ca6822826d3a5945752a38d30b356d10829f3159a191e5aa92a6`. - Live runtime caveat: Codex reported a nested `bwrap` loopback permission error and used its configured escalated execution inside Daytona. The outer Daytona sandbox remained active for both runs. - The startup regression uses a real database, production trust checks, workspace persistence, sandbox lease acquisition and realization, and a fake provider. It checks reassignment and allows only an authorized referenced project. - The transfer regression runs production archive/sync-back/merge code against distinct filesystem roots: create output in one sandbox, restore it, then read and update it in a fresh sandbox. The provider I/O is emulated; live staging verification is separate. ## Risks - The new default applies only to low-trust sandbox tasks without workspace configuration. Standard agents and explicit Git strategies keep their existing behavior. - Task directories retain work across turns and consume instance storage. The change does not migrate or copy existing shared files. - No database migration or credential changes are required. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository inspection, code execution, and browser tools. The exact runtime model revision and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/LOW-TRUST-PRESETS.md | 7 + doc/connections/AGENTMAIL.md | 4 + .../heartbeat-project-repositories.test.ts | 81 ++++++++++- server/src/services/heartbeat.ts | 53 ++++++- .../services/isolated-task-directory.test.ts | 129 ++++++++++++++++++ .../src/services/isolated-task-directory.ts | 48 +++++++ 6 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 server/src/services/isolated-task-directory.test.ts create mode 100644 server/src/services/isolated-task-directory.ts diff --git a/doc/LOW-TRUST-PRESETS.md b/doc/LOW-TRUST-PRESETS.md index 91ac628904b..9581e7cfa37 100644 --- a/doc/LOW-TRUST-PRESETS.md +++ b/doc/LOW-TRUST-PRESETS.md @@ -65,6 +65,13 @@ runtime boundary: - workspace runtime-service mutations are denied unless the boundary explicitly grants the `runtime.manage` tool class +When the task's project has no configured workspace and no layer specifies a +workspace strategy, sandbox execution uses a private directory for that company +and task. The directory persists across turns and reassignment and never imports the shared +project directory or agent home. No Git repository is required for this case. +Configured workspaces and explicit Git strategies keep their existing validation; +a missing or broken checkout does not fall back to an empty directory. + The Docker workflow in `doc/UNTRUSTED-PR-REVIEW.md` remains useful for manual local review, but Paperclip-managed low-trust execution requires a sandboxed environment instead of a host-local adapter process. diff --git a/doc/connections/AGENTMAIL.md b/doc/connections/AGENTMAIL.md index 7da4ac7b252..8310cbe846b 100644 --- a/doc/connections/AGENTMAIL.md +++ b/doc/connections/AGENTMAIL.md @@ -38,6 +38,10 @@ environment selected for the agent; setup rejects an unavailable runtime. New inbound tasks request isolated execution. The trust preset itself does not sandbox filesystem or network access. Standard agents remain selectable with a warning. +A boundary project without a configured workspace can process email in a private +task directory inside the selected sandbox. It does not need a Git repository. +An explicitly configured workspace strategy still applies and must be usable. + Removing the assigned agent’s saved-connection access or revoking its credential grant stops receiving and sending. Connection creation saves the vaulted binding, human grants, and agent access in one database transaction. diff --git a/server/src/__tests__/heartbeat-project-repositories.test.ts b/server/src/__tests__/heartbeat-project-repositories.test.ts index 354bb549ecb..a619a67cef5 100644 --- a/server/src/__tests__/heartbeat-project-repositories.test.ts +++ b/server/src/__tests__/heartbeat-project-repositories.test.ts @@ -6,12 +6,14 @@ import os from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; -import { agents, companies, createDb, heartbeatRuns, issues, projects, projectWorkspaces } from "@paperclipai/db"; +import { agents, companies, createDb, environments, executionWorkspaces, heartbeatRuns, issues, projects, projectWorkspaces } from "@paperclipai/db"; import { setExpensiveWorkspaceGitExecutor } from "@paperclipai/adapter-utils/git-workspace-sync"; +import { buildProjectMentionHref } from "@paperclipai/shared"; import { createWorkspaceGitOperationScheduler, WorkspaceGitScanError } from "../services/workspace-git-operation-scheduler.js"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; import { heartbeatService } from "../services/heartbeat.ts"; import { instanceSettingsService } from "../services/instance-settings.ts"; +import { environmentRuntimeService } from "../services/environment-runtime.js"; import { drainHeartbeatRunsToQuiescence } from "./helpers/drain-heartbeat-runs.js"; const execute = vi.hoisted(() => vi.fn(async (_input: any) => ({ exitCode: 0, signal: null, timedOut: false }))); @@ -50,12 +52,89 @@ suite("task project repository provisioning", () => { afterEach(async () => { await drainHeartbeatRunsToQuiescence(db, heartbeat); setExpensiveWorkspaceGitExecutor(null); + vi.stubEnv("PAPERCLIP_MULTI_PROJECT_WORKSPACE_SYNC", "false"); await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: false, enableIsolatedWorkspacesByDefault: false, }); }); + it("isolates repository-free low-trust tasks while preserving reassignment and authorized referenced projects", async () => { + const companyId = randomUUID(), projectId = randomUUID(), agentId = randomUUID(), environmentId = randomUUID(); + const referencedProjectId = randomUUID(), deniedProjectId = randomUUID(); + vi.stubEnv("PAPERCLIP_MULTI_PROJECT_WORKSPACE_SYNC", "true"); + await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: true }); + await db.insert(companies).values({ id: companyId, name: "Email company", issuePrefix: `E${companyId.slice(0, 6)}`, defaultResponsibleUserId: "responsible-user" }); + await db.insert(projects).values({ id: projectId, companyId, name: "Onboarding" }); + for (const id of [referencedProjectId, deniedProjectId]) { + const source = path.join(root, companyId, id); + await mkdir(source, { recursive: true }); + await writeFile(path.join(source, "reference.txt"), id); + await db.insert(projects).values({ id, companyId, name: id }); + await db.insert(projectWorkspaces).values({ id: randomUUID(), companyId, projectId: id, name: "Reference", sourceType: "local_path", cwd: source, isPrimary: true }); + } + await db.insert(environments).values({ id: environmentId, name: "Email sandbox", driver: "sandbox", status: "active", config: { provider: "fake", image: "fake:test" } }); + await db.insert(agents).values({ + id: agentId, companyId, name: "Email agent", role: "engineer", status: "idle", adapterType: "codex_local", + defaultEnvironmentId: environmentId, adapterConfig: {}, runtimeConfig: {}, + permissions: { trustPreset: "low_trust_review", authorizationPolicy: { + trustPreset: "low_trust_review", trustBoundary: { mode: "low_trust_review", companyId, projectIds: [projectId, referencedProjectId] }, + } }, + }); + // Exercise the real lease and workspace lifecycle with the built-in fake + // provider, executing its setup commands in a disposable filesystem root. + const sandboxHeartbeat = heartbeatService(db, { environmentRuntime: { + ...environmentRuntimeService(db), + execute: async (input) => ({ + exitCode: 0, + stdout: execFileSync(input.command, input.args ?? [], { cwd: root, input: input.stdin, encoding: "utf8", env: { ...process.env, ...input.env } }), + stderr: "", signal: null, timedOut: false, + }), + } }); + const directories: string[] = []; + const issueIds: string[] = []; + for (let index = 0; index < 2; index += 1) { + const issueId = randomUUID(); + issueIds.push(issueId); + await db.insert(issues).values({ + id: issueId, companyId, projectId, title: "Incoming email", originKind: "chat_channel", status: "todo", assigneeAgentId: agentId, + description: [referencedProjectId, deniedProjectId].map((id) => `[@Reference](${buildProjectMentionHref(id)})`).join(" "), + executionWorkspaceSettings: { mode: "isolated_workspace" }, + }); + const run = await sandboxHeartbeat.wakeup(agentId, { source: "on_demand", triggerDetail: "manual", contextSnapshot: { issueId, projectId } }); + expect(run).not.toBeNull(); + await vi.waitFor(async () => { + const latest = await sandboxHeartbeat.getRun(run!.id); + expect({ status: latest?.status, error: latest?.error }).toEqual({ status: "succeeded", error: null }); + }, { timeout: 15_000 }); + await drainHeartbeatRunsToQuiescence(db, sandboxHeartbeat); + const [workspace] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.sourceIssueId, issueId)); + expect(workspace).toMatchObject({ companyId, projectId, mode: "isolated_workspace", strategyType: "project_primary" }); + expect(workspace.cwd).toContain(`/isolated-workspaces/${companyId}/${issueId}`); + expect(execute.mock.calls.filter(([input]) => input.runId === run!.id)).toHaveLength(1); + const call = execute.mock.calls.find(([input]) => input.runId === run!.id)![0]; + expect(call.context.paperclipEnvironment.driver).toBe("sandbox"); + expect(call.context.paperclipWorkspaces.map((workspace: { projectId: string }) => workspace.projectId)).toEqual([referencedProjectId]); + directories.push(workspace.cwd!); + await writeFile(path.join(workspace.cwd!, "email.txt"), `private email ${index}`); + } + expect(directories[0]).not.toBe(directories[1]); + expect(await readFile(path.join(directories[0], "email.txt"), "utf8")).toBe("private email 0"); + const [originalAgent] = await db.select().from(agents).where(eq(agents.id, agentId)); + const reassignedAgentId = randomUUID(); + await db.insert(agents).values({ ...originalAgent, id: reassignedAgentId, name: "Next agent", status: "idle" }); + await db.update(issues).set({ status: "todo", assigneeAgentId: reassignedAgentId }).where(eq(issues.id, issueIds[0])); + const reassignedRun = await sandboxHeartbeat.wakeup(reassignedAgentId, { source: "on_demand", triggerDetail: "manual", contextSnapshot: { issueId: issueIds[0], projectId } }); + await vi.waitFor(async () => { + const latest = await sandboxHeartbeat.getRun(reassignedRun!.id); + expect({ status: latest?.status, error: latest?.error }).toEqual({ status: "succeeded", error: null }); + }, { timeout: 15_000 }); + await drainHeartbeatRunsToQuiescence(db, sandboxHeartbeat); + const reassignedCall = execute.mock.calls.find(([input]) => input.runId === reassignedRun!.id)![0]; + expect(reassignedCall.context.paperclipWorkspace.cwd).toBe(directories[0]); + expect(await readFile(path.join(directories[0], "email.txt"), "utf8")).toBe("private email 0"); + }, 40_000); + it.each([ { scenario: "no configured workspace", configuredWorkspace: false, explicitIsolation: null }, { scenario: "configured Git workspace", configuredWorkspace: true, explicitIsolation: null }, diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 388f1583fa8..6cce1f365ea 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -324,6 +324,7 @@ import { nativeChatWorkspaceCwd, nativeChatWorkspaceMatches, } from "./native-runtime/native-chat-workspace.js"; +import { materializeIsolatedTaskDirectory, shouldUseIsolatedTaskDirectory } from "./isolated-task-directory.js"; import { trackAgentFirstHeartbeat } from "@paperclipai/shared/telemetry"; import { getTelemetryClient } from "../telemetry.js"; import { @@ -12574,9 +12575,10 @@ export function heartbeatService( opts?: { useProjectWorkspace?: boolean | null; executionEnvironmentDriver?: string | null; + anchorWorkspace?: ResolvedAnchorWorkspaceForRun; }, ): Promise { - const anchor = await resolveAnchorWorkspaceForRun( + const anchor = opts?.anchorWorkspace ?? await resolveAnchorWorkspaceForRun( agent, context, previousSessionParams, @@ -21451,6 +21453,19 @@ export function heartbeatService( ); } } + const useIsolatedTaskDirectory = issueRef !== null && shouldUseIsolatedTaskDirectory({ + trustPreset: trustPreset.kind, + environmentDriver: selectedEnvironmentForConfig?.driver ?? null, + mode: requestedExecutionWorkspaceMode, + hasProjectWorkspace: projectContext?.hasWorkspace ?? false, + projectWorkspaceId: issueRef.projectWorkspaceId, + workspaceStrategies: [ + config.workspaceStrategy, + issueAssigneeOverrides?.adapterConfig?.workspaceStrategy, + projectExecutionWorkspacePolicy?.workspaceStrategy, + issueExecutionWorkspaceSettings?.workspaceStrategy, + ], + }); const workspaceManagedConfig = buildExecutionWorkspaceAdapterConfig({ agentConfig: config, projectPolicy: projectExecutionWorkspacePolicy, @@ -21462,6 +21477,9 @@ export function heartbeatService( const mergedConfig = { ...workspaceManagedConfig, ...(issueAssigneeOverrides?.adapterConfig ?? {}), + // The base below is already task-owned. Keep directory transport while + // preserving isolated mode and the mandatory sandbox preflight. + ...(useIsolatedTaskDirectory ? { workspaceStrategy: { type: "project_primary" } } : {}), }; const configSnapshot = buildExecutionWorkspaceConfigSnapshot( mergedConfig, @@ -21743,6 +21761,39 @@ export function heartbeatService( return preflightEnvironment.driver; }, resolveWorkspace: async () => { + if (useIsolatedTaskDirectory && issueRef) { + const cwd = await materializeIsolatedTaskDirectory({ + companyId: agent.companyId, + issueId: issueRef.id, + }); + if (reusableExistingExecutionWorkspace && ( + reusableExistingExecutionWorkspace.companyId !== agent.companyId || + reusableExistingExecutionWorkspace.projectId !== issueRef.projectId || + reusableExistingExecutionWorkspace.sourceIssueId !== issueRef.id || + reusableExistingExecutionWorkspace.mode !== "isolated_workspace" || + reusableExistingExecutionWorkspace.strategyType !== "project_primary" || + reusableExistingExecutionWorkspace.cwd !== cwd + )) { + throw new WorkspaceValidationFailure("The existing execution workspace is not this task's isolated directory.", { + workspaceValidation: { reason: "isolated_task_directory_binding_mismatch", issueId: issueRef.id }, + }); + } + return resolveWorkspaceForRun(agent, context, previousSessionParams, { + executionEnvironmentDriver: selectedEnvironmentForConfig?.driver ?? null, + anchorWorkspace: { + cwd, + source: "task_session", + projectId: issueRef.projectId, + workspaceId: null, + repoUrl: null, + repoRef: null, + workspaceHints: [], + warnings: [], + baseCwdFallback: false, + materializationFailures: [], + }, + }); + } if (nativeChatWorkspaceScope && !nativeChatWorkspaceScope.projectId) { const cwd = await materializeNativeChatTaskRoot( nativeChatWorkspaceScope, diff --git a/server/src/services/isolated-task-directory.test.ts b/server/src/services/isolated-task-directory.test.ts new file mode 100644 index 00000000000..bd3d350ff9f --- /dev/null +++ b/server/src/services/isolated-task-directory.test.ts @@ -0,0 +1,129 @@ +import { cp, mkdir, mkdtemp, readFile, readdir, rm, symlink, writeFile } from "node:fs/promises"; +import { execFile as execFileCallback } from "node:child_process"; +import { promisify } from "node:util"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { materializeIsolatedTaskDirectory, shouldUseIsolatedTaskDirectory } from "./isolated-task-directory.js"; +import { prepareSandboxManagedRuntime, type SandboxManagedRuntimeClient, type SandboxSyncOperation } from "@paperclipai/adapter-utils/sandbox-managed-runtime"; + +const execFile = promisify(execFileCallback); + +const policy = { + trustPreset: "low_trust_review", + environmentDriver: "sandbox", + mode: "isolated_workspace", + hasProjectWorkspace: false, + projectWorkspaceId: null, + workspaceStrategies: [undefined, null, {}], +}; + +describe("repository-free low-trust workspace selection", () => { + it("selects a private directory for sandbox tasks without a repository", () => { + expect(shouldUseIsolatedTaskDirectory(policy)).toBe(true); + }); + + it.each([ + { trustPreset: "standard" }, + { environmentDriver: "local" }, + { environmentDriver: "ssh" }, + { mode: "shared_workspace" }, + { hasProjectWorkspace: true }, + { projectWorkspaceId: "workspace-1" }, + { workspaceStrategies: [{ type: "git_worktree" }] }, + { workspaceStrategies: [{ existingBranch: "main" }] }, + { workspaceStrategies: [{ provisionCommand: "setup" }] }, + ])("preserves configured workspace requirements: %j", (override) => { + expect(shouldUseIsolatedTaskDirectory({ ...policy, ...override })).toBe(false); + }); +}); + +describe("isolated task directories", () => { + let root: string | undefined; + afterEach(async () => { + vi.unstubAllEnvs(); + if (root) await rm(root, { recursive: true, force: true }); + }); + + async function setup() { + root = await mkdtemp(path.join(os.tmpdir(), "paperclip-isolated-task-")); + vi.stubEnv("PAPERCLIP_HOME", root); + return { companyId: "company-1", issueId: "issue-1" }; + } + + it("retains a task's files across turns without sharing them with another task or company", async () => { + const identity = await setup(); + const cwd = await materializeIsolatedTaskDirectory(identity); + await writeFile(path.join(cwd, "notes.txt"), "private task output"); + expect(await materializeIsolatedTaskDirectory(identity)).toBe(cwd); + expect(await readFile(path.join(cwd, "notes.txt"), "utf8")).toBe("private task output"); + for (const other of [{ issueId: "issue-2" }, { companyId: "company-2" }]) { + const otherCwd = await materializeIsolatedTaskDirectory({ ...identity, ...other }); + expect(otherCwd).not.toBe(cwd); + expect(otherCwd.startsWith(`${cwd}${path.sep}`)).toBe(false); + expect(await readdir(otherCwd)).toEqual([]); + } + }); + + it("rejects a symlink to another task's directory", async () => { + const identity = await setup(); + const cwd = await materializeIsolatedTaskDirectory(identity); + await symlink(cwd, path.join(path.dirname(cwd), "issue-2")); + await expect(materializeIsolatedTaskDirectory({ ...identity, issueId: "issue-2" })) + .rejects.toThrow("not a private directory"); + }); + + it("round-trips sandbox output into the task directory and stages it on the next turn", async () => { + const identity = await setup(); + const cwd = await materializeIsolatedTaskDirectory(identity); + // Only provider I/O is emulated. Production archive, ignore, sync-back and + // merge logic runs against distinct host and remote filesystem roots. + const transfer = async (operations: SandboxSyncOperation[]) => ({ + operations: await Promise.all(operations.map(async (operation) => { + for (const file of operation.files) { + await mkdir(path.dirname(file.targetPath), { recursive: true }); + await cp(file.sourcePath, file.targetPath, { recursive: true, dereference: file.followSymlinks ?? false }); + } + for (const command of operation.postUploadCommands ?? []) { + await execFile("sh", ["-c", command.command]); + } + return { operationId: operation.operationId, filesTransferred: operation.files.length, bytesTransferred: 0 }; + })), + }); + const client: SandboxManagedRuntimeClient = { + makeDir: async (target) => { await mkdir(target, { recursive: true }); }, + writeFile: async (target, bytes) => { await writeFile(target, Buffer.from(bytes)); }, + readFile: async (target) => readFile(target), + listFiles: async (target) => readdir(target), + remove: async (target) => { await rm(target, { recursive: true, force: true }); }, + run: async (command) => { await execFile("sh", ["-c", command]); }, + syncIn: transfer, + syncOut: transfer, + }; + for (let turn = 0; turn < 2; turn += 1) { + const remoteCwd = path.join(root!, `sandbox-${turn}`); + const runtime = await prepareSandboxManagedRuntime({ + spec: { transport: "sandbox", provider: "test", sandboxId: `turn-${turn}`, remoteCwd, timeoutMs: 30_000, apiKey: null }, + adapterKey: "test", + client, + workspaceLocalDir: cwd, + }); + if (turn === 0) { + await writeFile(path.join(remoteCwd, "result.txt"), "created in sandbox"); + } else { + expect(await readFile(path.join(remoteCwd, "result.txt"), "utf8")).toBe("created in sandbox"); + await writeFile(path.join(remoteCwd, "result.txt"), "continued in a new sandbox"); + } + await runtime.restoreWorkspace(); + } + expect(await readFile(path.join(cwd, "result.txt"), "utf8")).toBe("continued in a new sandbox"); + const other = await materializeIsolatedTaskDirectory({ ...identity, issueId: "issue-2" }); + expect(await readdir(other)).toEqual([]); + }); + + it("rejects path traversal identities", async () => { + const identity = await setup(); + await expect(materializeIsolatedTaskDirectory({ ...identity, issueId: "../outside" })) + .rejects.toThrow("Invalid isolated task workspace identity"); + }); +}); diff --git a/server/src/services/isolated-task-directory.ts b/server/src/services/isolated-task-directory.ts new file mode 100644 index 00000000000..be34c930c8b --- /dev/null +++ b/server/src/services/isolated-task-directory.ts @@ -0,0 +1,48 @@ +import { lstat, mkdir, realpath } from "node:fs/promises"; +import path from "node:path"; +import { parseObject } from "../adapters/utils.js"; +import { resolvePaperclipInstanceRoot } from "../home-paths.js"; + +/** A repository-free sandbox task needs isolation, but has no Git base. */ +export function shouldUseIsolatedTaskDirectory(input: { + trustPreset: string; + environmentDriver: string | null; + mode: string; + hasProjectWorkspace: boolean; + projectWorkspaceId: string | null; + workspaceStrategies: unknown[]; +}): boolean { + return input.trustPreset === "low_trust_review" + && input.environmentDriver === "sandbox" + && input.mode === "isolated_workspace" + && !input.hasProjectWorkspace + && !input.projectWorkspaceId + // Never discard an explicit repository/branch requirement or setup hook. + && input.workspaceStrategies.every((value) => Object.keys(parseObject(value)).length === 0); +} + +/** Stable across turns, separate from project roots and shared agent homes. */ +export async function materializeIsolatedTaskDirectory(input: { + companyId: string; + issueId: string; +}): Promise { + // Files belong to the task, so reassignment preserves the same workspace. + const identities = [input.companyId, input.issueId]; + if (identities.some((value) => !/^[a-zA-Z0-9_-]+$/.test(value))) { + throw new Error("Invalid isolated task workspace identity"); + } + const root = resolvePaperclipInstanceRoot(); + await mkdir(root, { recursive: true }); + let cwd = await realpath(root); + for (const segment of ["isolated-workspaces", ...identities]) { + cwd = path.join(cwd, segment); + await mkdir(cwd, { mode: 0o700 }).catch((error: NodeJS.ErrnoException) => { + if (error.code !== "EEXIST") throw error; + }); + const stat = await lstat(cwd); + if (!stat.isDirectory() || stat.isSymbolicLink() || await realpath(cwd) !== cwd) { + throw new Error("Isolated task workspace path is not a private directory"); + } + } + return cwd; +} From d6fa1fd1ef1d378aee192845b9eb8860e00496fa Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 30 Sep 2026 16:18:59 -0700 Subject: [PATCH 5/6] feat(ui): streamline account menu profile access and add Invite shortcut (#14480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Humans work in teams, so Paperclip has a multi-user system with logins, profiles, and a company sidebar. > - The account menu in the lower-left corner of the sidebar is the main entry point for a user's own settings. > - The account menu spent half of its rows on "View profile" and "Edit profile". Users open these rows rarely. > - The account menu had no fast path to invite a new member. The invite flow is different on a self-hosted instance and on Paperclip Cloud. > - This pull request removes the two profile rows, makes the header a link to the profile, adds an "Edit profile" button on the profile page, and adds an "Invite" row. > - The benefit is a shorter account menu that keeps profile access and gives users with the invite permission a one-click path to invite people. ## Linked Issues or Issue Description No public GitHub issue exists for this change. The description below follows the enhancement template. Refs #14060. That earlier pull request holds the first two commits and the first Greptile review. It closed when the branch got a new name to remove an internal ticket id. All Greptile findings from both reviews are fixed in this branch. **What existing behavior does this improve?** The account menu in the sidebar (`SidebarAccountMenu` and its `.production` variant) and the user profile page at `/u/:userSlug`. **Subsystem affected** ui/ — React + Vite board UI **Current behavior** The account menu shows the user's picture and name at the top. Below them, the menu shows "View profile" and "Edit profile" rows, then the other rows. The header is not a link. The menu has no row to invite people. On a self-hosted instance, the user must open Company settings, then Members, then the Invites tab. On Paperclip Cloud, the user must open the Members page and use the Cloud People link there. **Proposed behavior** The account menu does not show "View profile" or "Edit profile". The picture and name at the top of the menu are a link to the user's own profile page. The profile page shows an "Edit profile" button when the viewer looks at their own profile. The account menu shows an "Invite" row with the same `UserPlus` icon as the company menu. On a self-hosted instance, the row opens the Members page on the Invites tab, and shows only to boards that hold the `users:invite` grant (company owners and admins, instance admins, and local boards). On Paperclip Cloud, the row opens the People settings for the current stack, and shows only to the owner or admin of the stack, the same rule the Members page uses. **Reason and benefit** Users open their profile rarely, but the two rows took half of the menu. Inviting people is a common task, but it needed three clicks and a different path on Cloud. The new menu is shorter, keeps profile access in one tap on the header, and gives one "Invite" entry point on both hosting modes to the users who can invite. **Breaking changes** None. Routes, API responses, and settings keys do not change. The profile page and the invite pages keep their current URLs. ## What Changed - `SidebarAccountMenu.tsx` and `SidebarAccountMenu.production.tsx`: remove the "View profile" and "Edit profile" rows. Make the picture and name header a link to the user's profile. Add the "Invite" row after "Settings" in the streamlined menu and first in the production menu. - Header structure: `master` added a staging commit SHA link under the email in the same header. The profile link is now a stretched overlay behind the header content, so the SHA anchor sits beside the email and the header has no nested anchors. - `ui/src/lib/userProfileLinks.ts` (new): build the profile path from the user id first. The profile endpoint treats the id as the one unique slug, so two members with the same display name get different links. Name and email are a fallback only when the session has no id. - `ui/src/hooks/useCloudInviteUrl.ts` (new): read the Cloud stack portfolio and return the People settings URL only when the current stack role is owner or admin. This is the same rule as the Members page. - `ui/src/hooks/useCompanyInviteAccess.ts` (new): read the current board access snapshot and report whether the board may invite people to the selected company on a self-hosted instance. Local boards and instance admins pass. Other boards need an active owner or admin membership, the roles that carry `users:invite`. This follows the same client-side gate pattern as `ToolsAdminGate` and the run ledger. The server stays authoritative. - Invite row visibility: the row is hidden when the operator hides `company.members` or `company.invites`, and until the health check resolves. On self-hosted instances, the row is hidden until the board access snapshot loads and when the board cannot invite. On Cloud, the row is hidden when no People URL can be built. The in-app Invites tab is never a fallback on Cloud, because it drives a different flow. - `ui/src/pages/UserProfile.tsx`: add an "Edit profile" button that links to `/company/settings/instance/profile`. The button shows only on the viewer's own profile and follows the `instance.profile` hidden-settings gate. - Tests: extend `SidebarAccountMenu.test.tsx`; add `userProfileLinks.test.ts`, `UserProfile.test.tsx`, and `useCompanyInviteAccess.test.ts`. - No documentation references the removed menu rows, so no docs change is needed. ## Verification Run the focused tests from the `ui/` directory: ```bash pnpm vitest run src/components/SidebarAccountMenu.test.tsx src/hooks/useCompanyInviteAccess.test.ts src/lib/userProfileLinks.test.ts src/pages/UserProfile.test.tsx ``` - 52 tests pass in these four files. They cover the header link by user id, the removed rows, the header overlay with no nested anchors, the self-hosted invite target, the self-hosted permission gate (owner, admin, instance admin, and local board see the row; an operator does not, on both menu variants), the Cloud invite target with no `target="_blank"`, the menu order, the hidden-settings gate on both variants, the Cloud role gate (a plain member sees no row), the no-fallback rule when Cloud stack metadata is missing, the staging commit SHA link from `master`, and the own-profile-only "Edit profile" button. - `tsc -b` in `ui/` reports no errors in the changed files. The only errors are pre-existing `@paperclipai/plugin-sdk/ui` resolution errors in `PluginOrganizationSwitcher.tsx` from an unbuilt workspace package. Manual steps: 1. Sign in and open the account menu in the lower-left corner. Confirm the menu has no "View profile" or "Edit profile" rows. 2. Click your picture or name at the top of the menu. Confirm your profile page opens and shows an "Edit profile" button. 3. Open another user's profile. Confirm the page shows no "Edit profile" button. 4. On a self-hosted instance, as a company owner or admin, click "Invite". Confirm the Members page opens on the Invites tab. As an operator or viewer, confirm the menu shows no "Invite" row. 5. On Paperclip Cloud, as a stack owner or admin, click "Invite". Confirm the Cloud People settings page opens in the same tab. As a plain member, confirm the menu shows no "Invite" row. ## Risks - Low risk. The change is limited to the UI and touches ten files. - Users who know the "View profile" and "Edit profile" rows must learn the new header link. The header has hover and focus styles to show that it is a link. - On self-hosted instances, the "Invite" row depends on the board access snapshot from `/cli-auth/me`, which other gates in the UI already use. A member with a custom `users:invite` grant but an operator or viewer role does not see the row. That member can still use the Members page. The row is a shortcut, not the only path. - On Paperclip Cloud, the "Invite" row depends on the stack portfolio query. When that query fails or the role is unknown, the menu hides the row instead of sending the user to the wrong flow. - Both menu variants change together, so a behavior difference between them is not expected. ## Model Used - Provider: Anthropic. Model: Claude Fable 5.1 (`claude-fable-5-1`). - Run through Claude Code on the Claude Agent SDK inside a Paperclip `claude_local` agent, with extended thinking and tool use (file edits, shell, tests, GitHub API). - The model wrote the code, the tests, and this description. A human reviewed the pull request and requested the review fixes. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Bender (Fable) Co-authored-by: Claude Fable 5.1 Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com> --- .../SidebarAccountMenu.production.tsx | 112 ++++--- ui/src/components/SidebarAccountMenu.test.tsx | 287 +++++++++++++++++- ui/src/components/SidebarAccountMenu.tsx | 100 +++--- ui/src/hooks/useCloudInviteUrl.ts | 36 +++ ui/src/hooks/useCompanyInviteAccess.test.ts | 59 ++++ ui/src/hooks/useCompanyInviteAccess.ts | 46 +++ ui/src/lib/userProfileLinks.test.ts | 25 ++ ui/src/lib/userProfileLinks.ts | 34 +++ ui/src/pages/UserProfile.test.tsx | 177 +++++++++++ ui/src/pages/UserProfile.tsx | 32 +- 10 files changed, 829 insertions(+), 79 deletions(-) create mode 100644 ui/src/hooks/useCloudInviteUrl.ts create mode 100644 ui/src/hooks/useCompanyInviteAccess.test.ts create mode 100644 ui/src/hooks/useCompanyInviteAccess.ts create mode 100644 ui/src/lib/userProfileLinks.test.ts create mode 100644 ui/src/lib/userProfileLinks.ts create mode 100644 ui/src/pages/UserProfile.test.tsx diff --git a/ui/src/components/SidebarAccountMenu.production.tsx b/ui/src/components/SidebarAccountMenu.production.tsx index 005a31ad9f7..e0549e56fe7 100644 --- a/ui/src/components/SidebarAccountMenu.production.tsx +++ b/ui/src/components/SidebarAccountMenu.production.tsx @@ -5,16 +5,19 @@ import { Flag, LogOut, type LucideIcon, - UserRound, - UserRoundPen, + UserPlus, } from "lucide-react"; -import type { DeploymentMode } from "@paperclipai/shared"; +import { hidesCompanyPage, type DeploymentMode } from "@paperclipai/shared"; import { Link } from "@/lib/router"; import { authApi } from "@/api/auth"; import { queryKeys } from "@/lib/queryKeys"; import { useCloudInstance } from "@/hooks/useCloudInstance"; +import { useCloudInviteUrl } from "@/hooks/useCloudInviteUrl"; +import { useCanInviteCompanyMembers } from "@/hooks/useCompanyInviteAccess"; +import { useHiddenSettings } from "@/hooks/useHiddenSettings"; import { useSignOut } from "@/hooks/useSignOut"; import { useStagingCommit } from "@/hooks/useStagingCommit"; +import { userProfilePath } from "@/lib/userProfileLinks"; import { useSidebar } from "../context/SidebarContext"; import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; @@ -23,7 +26,7 @@ import { cn, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils"; import { ThemeToggle } from "./ThemeToggle"; import { SidebarServerInfo } from "./SidebarServerInfo"; -const PROFILE_SETTINGS_PATH = "/company/settings/instance/profile"; +const INVITES_PATH = "/company/settings/members?tab=invites"; const DOCS_URL = "https://docs.paperclip.ing/"; const FEEDBACK_URL = "https://paperclip.ing/feedback"; @@ -39,7 +42,14 @@ interface MenuActionProps { icon: LucideIcon; onClick?: () => void; href?: string; + /** Opens `href` in a new tab (docs and other off-product links). */ external?: boolean; + /** + * Leaves the app in the current tab with a full navigation. Cloud links + * must use this: the cloud harness shadows those paths on tenant hosts, so + * the in-app router can never reach them. + */ + topLevel?: boolean; } function deriveInitials(name: string) { @@ -50,21 +60,15 @@ function deriveInitials(name: string) { return name.slice(0, 2).toUpperCase(); } -function deriveUserSlug(name: string | null | undefined, email: string | null | undefined, id: string | null | undefined) { - const candidates = [name, email?.split("@")[0], email, id]; - for (const candidate of candidates) { - const slug = candidate - ?.trim() - .toLowerCase() - .replace(/['"]/g, "") - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, ""); - if (slug) return slug; - } - return "me"; -} - -function MenuAction({ label, description, icon: Icon, onClick, href, external = false }: MenuActionProps) { +function MenuAction({ + label, + description, + icon: Icon, + onClick, + href, + external = false, + topLevel = false, +}: MenuActionProps) { const className = "flex w-full items-start gap-3 rounded-xl px-3 py-3 text-left transition-colors hover:bg-accent/60"; @@ -89,6 +93,14 @@ function MenuAction({ label, description, icon: Icon, onClick, href, external = ); } + if (topLevel) { + return ( + + {content} + + ); + } + return ( {content} @@ -108,7 +120,26 @@ export function SidebarAccountMenu({ open: controlledOpen, onOpenChange, }: SidebarAccountMenuProps) { - const isCloud = Boolean(useCloudInstance()); + const cloud = useCloudInstance(); + const isCloud = Boolean(cloud); + // Invites live on the Members page (or in Cloud's People settings). Hide the + // shortcut when the hosting operator hides either surface, and until the + // health response resolves so a hidden surface never flashes. + const { hidden: hiddenSettings, loaded: hiddenSettingsLoaded } = useHiddenSettings(); + // On Cloud the shortcut exists only for the current stack's owner/admin and + // only once the stack metadata is known; the in-app Invites tab is never a + // fallback there because it drives a different invitation flow. + const cloudInviteUrl = useCloudInviteUrl(); + // Self-hosted invites need the `users:invite` grant. Offer the shortcut only + // to boards that hold it (company owner/admin, instance admins, local + // boards) so a plain member is never sent to a permission error. + const canInviteMembers = useCanInviteCompanyMembers(!isCloud); + const inviteHref = isCloud ? cloudInviteUrl : canInviteMembers ? INVITES_PATH : null; + const showInvite = + hiddenSettingsLoaded && + inviteHref !== null && + !hidesCompanyPage(hiddenSettings, "company.members") && + !hidesCompanyPage(hiddenSettings, "company.invites"); const [internalOpen, setInternalOpen] = useState(false); const { isMobile, setSidebarOpen, collapsed, peeking } = useSidebar(); const rail = collapsed && !peeking; @@ -127,7 +158,7 @@ export function SidebarAccountMenu({ const secondaryLabel = session?.user.email?.trim() || (deploymentMode === "authenticated" ? "Signed in" : "Local workspace board"); const initials = deriveInitials(displayName); - const profileHref = `/u/${deriveUserSlug(session?.user.name, session?.user.email, session?.user.id)}`; + const profileHref = userProfilePath(session?.user); function closeNavigationChrome() { setOpen(false); @@ -166,19 +197,26 @@ export function SidebarAccountMenu({ >
-
-
+ {/* The profile link is a stretched overlay so the staging SHA anchor can sit beside the email without nesting anchors. */} +
+ +
{session?.user.image ? : null} {initials}
-
+

{displayName}

{secondaryLabel}

{stagingCommit ? (
- - + {showInvite && inviteHref ? ( + + ) : null} ({ getExperimental: vi.fn(), })); const mockHealthApi = vi.hoisted(() => ({ get: vi.fn() })); +const mockGetCurrentBoardAccess = vi.hoisted(() => vi.fn()); const mockToggleTheme = vi.hoisted(() => vi.fn()); const mockSetSidebarOpen = vi.hoisted(() => vi.fn()); const mockNavigateTopLevel = vi.hoisted(() => vi.fn()); @@ -30,6 +31,18 @@ vi.mock("@/api/auth", () => ({ vi.mock("@/api/health", () => ({ healthApi: mockHealthApi })); +vi.mock("@/api/access", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + accessApi: { ...actual.accessApi, getCurrentBoardAccess: mockGetCurrentBoardAccess }, + }; +}); + +vi.mock("@/context/CompanyContext", () => ({ + useOptionalCompany: () => ({ selectedCompanyId: "company-1" }), +})); + vi.mock("@/lib/browserNavigation", () => ({ navigateTopLevel: mockNavigateTopLevel, })); @@ -78,6 +91,39 @@ async function flushReact() { }); } +/** Stack portfolio with the acme-labs stack current at the given Cloud role. */ +function cloudStacksPortfolio(role: string) { + return { + stacks: [ + { + displayName: "Acme Labs", + stackSlug: "acme-labs", + primaryHost: "acme-labs.example.test", + lifecycleState: "active", + sleepState: "awake", + role, + isCurrent: true, + }, + ], + }; +} + +/** Board access snapshot for a signed-in member of company-1 at the given company role. */ +function boardAccess( + membershipRole: "owner" | "admin" | "operator" | "viewer", + overrides: { isInstanceAdmin?: boolean; source?: string } = {}, +) { + return { + user: { id: "user-1", email: "jane@example.com", name: "Jane Example", image: null }, + userId: "user-1", + isInstanceAdmin: overrides.isInstanceAdmin ?? false, + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", membershipRole, status: "active" as const }], + source: overrides.source ?? "session", + keyId: null, + }; +} + describe("SidebarAccountMenu", () => { let container: HTMLDivElement; @@ -97,6 +143,9 @@ describe("SidebarAccountMenu", () => { enableIsolatedWorkspaces: false, }); mockAuthApi.signOut.mockResolvedValue({ success: true, redirectTo: "/cloud/logout" }); + // Default to a company owner: the self-hosted Invite shortcut needs the + // `users:invite` grant, which owners and admins hold. + mockGetCurrentBoardAccess.mockResolvedValue(boardAccess("owner")); }); afterEach(() => { @@ -347,7 +396,6 @@ describe("SidebarAccountMenu", () => { }); await flushReact(); - expect(document.body.textContent).toContain("Edit profile"); expect(document.body.textContent).toContain("Settings"); expect(document.body.textContent).not.toContain("Instance settings"); expect(document.body.textContent).toContain("Documentation"); @@ -356,6 +404,35 @@ describe("SidebarAccountMenu", () => { expect(popover?.textContent).not.toContain("Feedback"); expect(popover?.querySelector('a[href="https://paperclip.ing/feedback"]')).toBeNull(); + // Profile access moved off the action list: the header links to the + // profile, and the separate view/edit rows are gone. Editing lives on the + // profile page itself. + expect(popover?.textContent).not.toContain("View profile"); + expect(popover?.textContent).not.toContain("Edit profile"); + // The link resolves by user id, not display name: names are not unique, + // and the profile endpoint returns the first member whose slug matches. + const profileHeaderLink = popover?.querySelector('a[aria-label="View profile"]'); + expect(profileHeaderLink?.getAttribute("href")).toBe("/u/user-1"); + // The link is a stretched overlay over the header block (so the staging + // SHA anchor can live beside the email without nesting anchors), so the + // identity text is in the header block rather than inside the anchor. + const profileHeader = profileHeaderLink?.parentElement; + expect(profileHeader?.textContent).toContain("Jane Example"); + expect(profileHeader?.textContent).toContain("jane@example.com"); + expect(profileHeader?.querySelector("a a")).toBeNull(); + expect(popover?.querySelector('a[href="/company/settings/instance/profile"]')).toBeNull(); + + // Self-hosted invites open the in-app Members page on its Invites tab. + const inviteLink = popover?.querySelector( + 'a[href="/company/settings/members?tab=invites"]', + ); + expect(inviteLink?.textContent).toBe("Invite"); + expect(inviteLink?.querySelector("svg")?.classList).toContain("lucide-user-plus"); + expect(inviteLink?.hasAttribute("target")).toBe(false); + const menuOrder = popover?.textContent ?? ""; + expect(menuOrder.indexOf("Settings")).toBeLessThan(menuOrder.indexOf("Invite")); + expect(menuOrder.indexOf("Invite")).toBeLessThan(menuOrder.indexOf("Documentation")); + // Documentation still appears before the theme toggle. const menuText = popover?.textContent ?? ""; const docsPos = menuText.indexOf("Documentation"); @@ -376,7 +453,6 @@ describe("SidebarAccountMenu", () => { .not.toContain("bg-muted"); expect(document.body.textContent).not.toContain("Manage company and instance settings."); expect(document.body.textContent).not.toContain("Open your activity, task, and usage ledger."); - expect(document.body.querySelector('a[href="/company/settings/instance/profile"]')).not.toBeNull(); expect(document.body.querySelector('a[href="/company/settings"]')).not.toBeNull(); const signOutButton = Array.from(document.body.querySelectorAll("button")).find( @@ -412,6 +488,7 @@ describe("SidebarAccountMenu", () => { cloudBaseUrl: "https://cloud.example.test", }, }); + queryClient.setQueryData(queryKeys.cloud.stacks, cloudStacksPortfolio("owner")); await act(async () => { root.render( @@ -430,6 +507,20 @@ describe("SidebarAccountMenu", () => { expect(container.querySelector('a[aria-label="Share feedback"]')).toBeNull(); + // Cloud manages invitations in the stack's People settings, reached with a + // same-tab top-level navigation rather than the in-app router. + const popover = document.body.querySelector('[data-slot="popover-content"]'); + const inviteLink = Array.from(popover?.querySelectorAll("a") ?? []).find( + (anchor) => anchor.textContent?.includes("Invite"), + ); + expect(inviteLink?.getAttribute("href")).toBe( + "https://cloud.example.test/workspaces/acme-labs/settings?section=people", + ); + expect(inviteLink?.hasAttribute("target")).toBe(false); + expect(popover?.querySelector('a[href="/company/settings/members?tab=invites"]')).toBeNull(); + expect(popover?.querySelector('a[aria-label="View profile"]')?.getAttribute("href")).toBe("/u/user-1"); + expect(popover?.textContent).not.toContain("Edit profile"); + const signOutButton = Array.from(document.body.querySelectorAll("button")).find( (button) => button.textContent?.includes("Sign out"), ); @@ -448,6 +539,198 @@ describe("SidebarAccountMenu", () => { }); }); + it.each([SidebarAccountMenu, ProductionSidebarAccountMenu])("offers no cloud invite shortcut to members below stack admin (%#)", async (AccountMenu) => { + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + deploymentMode: "authenticated", + cloud: { + managed: true, + managedBy: "paperclip-cloud", + stackSlug: "acme-labs", + cloudBaseUrl: "https://cloud.example.test", + }, + }); + // Company roles can differ from Cloud roles; the Members page only offers + // the Cloud invite link to the current stack's owner/admin, so the menu + // must match rather than send a plain member to People settings. + queryClient.setQueryData(queryKeys.cloud.stacks, cloudStacksPortfolio("member")); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + + const popover = document.body.querySelector('[data-slot="popover-content"]'); + expect(popover?.textContent).not.toContain("Invite"); + expect(popover?.querySelector('a[href^="https://cloud.example.test/"]')).toBeNull(); + expect(popover?.querySelector('a[href="/company/settings/members?tab=invites"]')).toBeNull(); + expect(popover?.querySelector('a[aria-label="View profile"]')).not.toBeNull(); + + await act(async () => { + root.unmount(); + }); + }); + + it.each([SidebarAccountMenu, ProductionSidebarAccountMenu])("never falls back to in-app invites when cloud stack metadata is missing (%#)", async (AccountMenu) => { + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + // Managed instance whose health block lacks the stack slug: the portfolio + // has no current stack either, so no Cloud People URL can be built. The + // in-app Invites tab drives a different flow and must not appear instead. + queryClient.setQueryData(queryKeys.health, { + status: "ok", + deploymentMode: "authenticated", + cloud: { + managed: true, + managedBy: "paperclip-cloud", + stackSlug: null, + cloudBaseUrl: "https://cloud.example.test", + }, + }); + queryClient.setQueryData(queryKeys.cloud.stacks, { stacks: [] }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + + const popover = document.body.querySelector('[data-slot="popover-content"]'); + expect(popover?.textContent).not.toContain("Invite"); + expect(popover?.querySelector('a[href="/company/settings/members?tab=invites"]')).toBeNull(); + expect(popover?.textContent).toContain("Documentation"); + + await act(async () => { + root.unmount(); + }); + }); + + it.each([SidebarAccountMenu, ProductionSidebarAccountMenu])("hides the invite shortcut when the operator hides invites (%#)", async (AccountMenu) => { + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + deploymentMode: "authenticated", + hiddenSettings: ["company.invites"], + }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + + const popover = document.body.querySelector('[data-slot="popover-content"]'); + expect(popover?.textContent).not.toContain("Invite"); + expect(popover?.querySelector('a[href="/company/settings/members?tab=invites"]')).toBeNull(); + // The settings-independent entries stay put. + expect(popover?.querySelector('a[aria-label="View profile"]')).not.toBeNull(); + expect(popover?.textContent).toContain("Documentation"); + + await act(async () => { + root.unmount(); + }); + }); + + it.each([SidebarAccountMenu, ProductionSidebarAccountMenu])("offers no self-hosted invite shortcut to members without the invite grant (%#)", async (AccountMenu) => { + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + deploymentMode: "authenticated", + }); + // Operators and viewers lack `users:invite`; the Invites tab would only + // show them a permission error, so the menu must not offer it. + mockGetCurrentBoardAccess.mockResolvedValue(boardAccess("operator")); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + await flushReact(); + + expect(mockGetCurrentBoardAccess).toHaveBeenCalledOnce(); + const popover = document.body.querySelector('[data-slot="popover-content"]'); + expect(popover?.textContent).not.toContain("Invite"); + expect(popover?.querySelector('a[href="/company/settings/members?tab=invites"]')).toBeNull(); + expect(popover?.querySelector('a[aria-label="View profile"]')).not.toBeNull(); + expect(popover?.textContent).toContain("Documentation"); + + await act(async () => { + root.unmount(); + }); + }); + + it.each([ + ["instance admin", boardAccess("viewer", { isInstanceAdmin: true })], + ["local board", boardAccess("viewer", { source: "local_implicit" })], + ["company admin", boardAccess("admin")], + ])("offers the self-hosted invite shortcut to a %s", async (_label, access) => { + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + deploymentMode: "authenticated", + }); + mockGetCurrentBoardAccess.mockResolvedValue(access); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + await flushReact(); + + const popover = document.body.querySelector('[data-slot="popover-content"]'); + const inviteLink = popover?.querySelector( + 'a[href="/company/settings/members?tab=invites"]', + ); + expect(inviteLink?.textContent).toBe("Invite"); + + await act(async () => { + root.unmount(); + }); + }); + it("keeps sign-out hidden outside authenticated deployment mode", async () => { const root = createRoot(container); const queryClient = new QueryClient({ diff --git a/ui/src/components/SidebarAccountMenu.tsx b/ui/src/components/SidebarAccountMenu.tsx index d6552c3a509..f40959aa702 100644 --- a/ui/src/components/SidebarAccountMenu.tsx +++ b/ui/src/components/SidebarAccountMenu.tsx @@ -6,16 +6,19 @@ import { LogOut, Settings, type LucideIcon, - UserRound, - UserRoundPen, + UserPlus, } from "lucide-react"; -import type { DeploymentMode } from "@paperclipai/shared"; +import { hidesCompanyPage, type DeploymentMode } from "@paperclipai/shared"; import { Link } from "@/lib/router"; import { authApi } from "@/api/auth"; import { queryKeys } from "@/lib/queryKeys"; import { useCloudInstance } from "@/hooks/useCloudInstance"; +import { useCloudInviteUrl } from "@/hooks/useCloudInviteUrl"; +import { useCanInviteCompanyMembers } from "@/hooks/useCompanyInviteAccess"; +import { useHiddenSettings } from "@/hooks/useHiddenSettings"; import { useSignOut } from "@/hooks/useSignOut"; import { useStagingCommit } from "@/hooks/useStagingCommit"; +import { userProfilePath } from "@/lib/userProfileLinks"; import { useSidebar } from "../context/SidebarContext"; import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; @@ -24,7 +27,7 @@ import { cn, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils"; import { ThemeToggle } from "./ThemeToggle"; import { SidebarServerInfo } from "./SidebarServerInfo"; -const PROFILE_SETTINGS_PATH = "/company/settings/instance/profile"; +const INVITES_PATH = "/company/settings/members?tab=invites"; const DOCS_URL = "https://docs.paperclip.ing/"; const FEEDBACK_URL = "https://paperclip.ing/feedback"; @@ -41,7 +44,14 @@ interface MenuActionProps { icon: LucideIcon; onClick?: () => void; href?: string; + /** Opens `href` in a new tab (docs and other off-product links). */ external?: boolean; + /** + * Leaves the app in the current tab with a full navigation. Cloud links + * must use this: the cloud harness shadows those paths on tenant hosts, so + * the in-app router can never reach them. + */ + topLevel?: boolean; } function deriveInitials(name: string) { @@ -52,26 +62,13 @@ function deriveInitials(name: string) { return name.slice(0, 2).toUpperCase(); } -function deriveUserSlug(name: string | null | undefined, email: string | null | undefined, id: string | null | undefined) { - const candidates = [name, email?.split("@")[0], email, id]; - for (const candidate of candidates) { - const slug = candidate - ?.trim() - .toLowerCase() - .replace(/['"]/g, "") - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, ""); - if (slug) return slug; - } - return "me"; -} - function MenuAction({ label, icon: Icon, onClick, href, external = false, + topLevel = false, }: MenuActionProps) { const className = "flex h-(--profile-popover-row-height) w-full items-center gap-(--profile-popover-row-gap) rounded-lg px-2.5 text-left text-(length:--text-compact) font-medium leading-(--profile-popover-label-line-height) text-foreground transition-colors hover:bg-accent"; @@ -94,6 +91,14 @@ function MenuAction({ ); } + if (topLevel) { + return ( + + {content} + + ); + } + return ( {content} @@ -114,7 +119,26 @@ export function SidebarAccountMenu({ onOpenChange, forceExpanded = false, }: SidebarAccountMenuProps) { - const isCloud = Boolean(useCloudInstance()); + const cloud = useCloudInstance(); + const isCloud = Boolean(cloud); + // Invites live on the Members page (or in Cloud's People settings). Hide the + // shortcut when the hosting operator hides either surface, and until the + // health response resolves so a hidden surface never flashes. + const { hidden: hiddenSettings, loaded: hiddenSettingsLoaded } = useHiddenSettings(); + // On Cloud the shortcut exists only for the current stack's owner/admin and + // only once the stack metadata is known; the in-app Invites tab is never a + // fallback there because it drives a different invitation flow. + const cloudInviteUrl = useCloudInviteUrl(); + // Self-hosted invites need the `users:invite` grant. Offer the shortcut only + // to boards that hold it (company owner/admin, instance admins, local + // boards) so a plain member is never sent to a permission error. + const canInviteMembers = useCanInviteCompanyMembers(!isCloud); + const inviteHref = isCloud ? cloudInviteUrl : canInviteMembers ? INVITES_PATH : null; + const showInvite = + hiddenSettingsLoaded && + inviteHref !== null && + !hidesCompanyPage(hiddenSettings, "company.members") && + !hidesCompanyPage(hiddenSettings, "company.invites"); const [internalOpen, setInternalOpen] = useState(false); const { isMobile, setSidebarOpen, collapsed, peeking } = useSidebar(); const rail = collapsed && !peeking && !forceExpanded; @@ -133,7 +157,7 @@ export function SidebarAccountMenu({ const secondaryLabel = session?.user.email?.trim() || (deploymentMode === "authenticated" ? "Signed in" : "Local workspace board"); const initials = deriveInitials(displayName); - const profileHref = `/u/${deriveUserSlug(session?.user.name, session?.user.email, session?.user.id)}`; + const profileHref = userProfilePath(session?.user); function closeNavigationChrome() { setOpen(false); @@ -170,12 +194,19 @@ export function SidebarAccountMenu({ sideOffset={10} className="min-h-(--profile-popover-min-height) w-(--profile-popover-width) max-w-(--sz-calc-24) overflow-hidden rounded-xl border-border bg-popover p-0 shadow-(--shadow-profile-popover)" > -
- + {/* The profile link is a stretched overlay so the staging SHA anchor can sit beside the email without nesting anchors. */} +
+ + {session?.user.image ? : null} {initials} -
+

{displayName}

@@ -184,7 +215,7 @@ export function SidebarAccountMenu({

{stagingCommit ? ( - - + {showInvite && inviteHref ? ( + + ) : null} cloudApi.listStacks(), + enabled: isCloud, + staleTime: 30_000, + retry: false, + }); + if (!cloud || cloudStacksQuery.isError) return null; + const currentStack = cloudStacksQuery.data?.stacks.find((stack) => stack.isCurrent); + if (currentStack?.role !== "owner" && currentStack?.role !== "admin") return null; + return cloudStackInviteUrl(cloud.cloudBaseUrl, currentStack.stackSlug); +} diff --git a/ui/src/hooks/useCompanyInviteAccess.test.ts b/ui/src/hooks/useCompanyInviteAccess.test.ts new file mode 100644 index 00000000000..1939426a128 --- /dev/null +++ b/ui/src/hooks/useCompanyInviteAccess.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; +import type { CurrentBoardAccess } from "@/api/access"; +import { canInviteCompanyMembers } from "./useCompanyInviteAccess"; + +function snapshot(overrides: Partial = {}): CurrentBoardAccess { + return { + user: { id: "user-1", email: "jane@example.com", name: "Jane Example", image: null }, + userId: "user-1", + isInstanceAdmin: false, + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", membershipRole: "operator", status: "active" }], + source: "session", + keyId: null, + ...overrides, + }; +} + +describe("canInviteCompanyMembers", () => { + it("is false until the board access snapshot is known", () => { + expect(canInviteCompanyMembers("company-1", undefined)).toBe(false); + }); + + it("always passes local boards and instance admins", () => { + expect(canInviteCompanyMembers("company-1", snapshot({ source: "local_implicit" }))).toBe(true); + expect(canInviteCompanyMembers("company-1", snapshot({ isInstanceAdmin: true }))).toBe(true); + // Both bypass the company lookup, so a missing company is fine. + expect(canInviteCompanyMembers(null, snapshot({ isInstanceAdmin: true }))).toBe(true); + }); + + it.each(["owner", "admin"] as const)("passes an active company %s", (membershipRole) => { + const access = snapshot({ + memberships: [{ companyId: "company-1", membershipRole, status: "active" }], + }); + expect(canInviteCompanyMembers("company-1", access)).toBe(true); + }); + + it.each(["operator", "viewer", "member", null] as const)("rejects a company %s", (membershipRole) => { + const access = snapshot({ + memberships: [{ companyId: "company-1", membershipRole, status: "active" }], + }); + expect(canInviteCompanyMembers("company-1", access)).toBe(false); + }); + + it("ignores inactive memberships and other companies", () => { + const suspended = snapshot({ + memberships: [{ companyId: "company-1", membershipRole: "owner", status: "suspended" }], + }); + expect(canInviteCompanyMembers("company-1", suspended)).toBe(false); + const elsewhere = snapshot({ + memberships: [{ companyId: "company-2", membershipRole: "owner", status: "active" }], + }); + expect(canInviteCompanyMembers("company-1", elsewhere)).toBe(false); + expect(canInviteCompanyMembers(null, elsewhere)).toBe(false); + }); + + it("is false when the snapshot carries no memberships", () => { + expect(canInviteCompanyMembers("company-1", snapshot({ memberships: undefined }))).toBe(false); + }); +}); diff --git a/ui/src/hooks/useCompanyInviteAccess.ts b/ui/src/hooks/useCompanyInviteAccess.ts new file mode 100644 index 00000000000..7eec8933fc2 --- /dev/null +++ b/ui/src/hooks/useCompanyInviteAccess.ts @@ -0,0 +1,46 @@ +import { useQuery } from "@tanstack/react-query"; +import { accessApi, type CurrentBoardAccess } from "@/api/access"; +import { queryKeys } from "@/lib/queryKeys"; +import { useOptionalCompany } from "@/context/CompanyContext"; + +/** + * Whether a board may invite people to `companyId` on a self-hosted instance, + * judged from the current board access snapshot. + * + * Mirrors the other client-side role gates (`ToolsAdminGate`, the run + * ledger): local implicit boards and instance admins always pass; otherwise + * the active company membership must be owner or admin, the roles that carry + * the `users:invite` grant. Resolves to false while the snapshot is unknown. + * The server stays authoritative; this only decides whether to offer a + * shortcut that would otherwise end in a permission error. + */ +export function canInviteCompanyMembers( + companyId: string | null | undefined, + boardAccess: CurrentBoardAccess | undefined, +): boolean { + if (!boardAccess) return false; + if (boardAccess.source === "local_implicit" || boardAccess.isInstanceAdmin) return true; + if (!companyId) return false; + const membership = boardAccess.memberships?.find( + (item) => item.companyId === companyId && item.status === "active", + ); + return membership?.membershipRole === "owner" || membership?.membershipRole === "admin"; +} + +/** + * Whether the signed-in board may invite people to the selected company on a + * self-hosted instance. Pass `enabled: false` on Cloud, where invitations live + * in the stack's People settings and use a different rule + * (see `useCloudInviteUrl`). + */ +export function useCanInviteCompanyMembers(enabled = true): boolean { + const company = useOptionalCompany(); + const boardAccess = useQuery({ + queryKey: queryKeys.access.currentBoardAccess, + queryFn: () => accessApi.getCurrentBoardAccess(), + enabled, + retry: false, + staleTime: 30_000, + }); + return enabled && canInviteCompanyMembers(company?.selectedCompanyId, boardAccess.data); +} diff --git a/ui/src/lib/userProfileLinks.test.ts b/ui/src/lib/userProfileLinks.test.ts new file mode 100644 index 00000000000..19a5c5e891b --- /dev/null +++ b/ui/src/lib/userProfileLinks.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from "vitest"; +import { userProfilePath } from "./userProfileLinks"; + +describe("userProfilePath", () => { + it("links by user id even when a display name is available", () => { + // Two members can share a name; the endpoint returns the first slug match, + // so a name-based link could open a namesake's profile. + expect(userProfilePath({ id: "user-1", name: "Jane Example", email: "jane@example.com" })).toBe("/u/user-1"); + }); + + it("slugifies ids the same way the profile endpoint does", () => { + expect(userProfilePath({ id: "paperclip-id:TLqGPuleJpjX" })).toBe("/u/paperclip-id-tlqgpulejpjx"); + }); + + it("falls back to name, then email, when the session carries no id", () => { + expect(userProfilePath({ id: null, name: "Jane Example", email: "jane@example.com" })).toBe("/u/jane-example"); + expect(userProfilePath({ id: "", name: " ", email: "jane.doe@example.com" })).toBe("/u/jane-doe"); + expect(userProfilePath({ email: "@example.com" })).toBe("/u/example-com"); + }); + + it("uses a placeholder when nothing identifies the user", () => { + expect(userProfilePath(undefined)).toBe("/u/me"); + expect(userProfilePath({ id: "!!!", name: "'\"" })).toBe("/u/me"); + }); +}); diff --git a/ui/src/lib/userProfileLinks.ts b/ui/src/lib/userProfileLinks.ts new file mode 100644 index 00000000000..4fcc3c6cc09 --- /dev/null +++ b/ui/src/lib/userProfileLinks.ts @@ -0,0 +1,34 @@ +interface ProfileLinkUser { + id?: string | null; + name?: string | null; + email?: string | null; +} + +function slugifyUserPart(value: string | null | undefined): string | null { + const slug = value + ?.trim() + .toLowerCase() + .replace(/['"]/g, "") + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, ""); + return slug || null; +} + +/** + * In-app path of a user's profile page (`/u/:userSlug`). + * + * The profile endpoint resolves `:userSlug` against every member's name, + * email local part, email, and principal id, and returns the first member + * that matches. Display names are not unique, so a name-based slug can open a + * namesake's profile instead of the intended one. The principal id is the one + * candidate that is unique per member, so links to a known user always use + * it; name and email slugs are only a fallback for sessions with no user id. + */ +export function userProfilePath(user: ProfileLinkUser | null | undefined): string { + const candidates = [user?.id, user?.name, user?.email?.split("@")[0], user?.email]; + for (const candidate of candidates) { + const slug = slugifyUserPart(candidate); + if (slug) return `/u/${slug}`; + } + return "/u/me"; +} diff --git a/ui/src/pages/UserProfile.test.tsx b/ui/src/pages/UserProfile.test.tsx new file mode 100644 index 00000000000..3d7b8517c08 --- /dev/null +++ b/ui/src/pages/UserProfile.test.tsx @@ -0,0 +1,177 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { UserProfileResponse } from "@paperclipai/shared"; +import { queryKeys } from "../lib/queryKeys"; +import { UserProfile } from "./UserProfile"; + +const mockAuthApi = vi.hoisted(() => ({ + getSession: vi.fn(), +})); + +const mockUserProfilesApi = vi.hoisted(() => ({ + get: vi.fn(), +})); + +const mockSetBreadcrumbs = vi.hoisted(() => vi.fn()); +const mockUseParams = vi.hoisted(() => vi.fn()); + +vi.mock("../api/auth", () => ({ + authApi: mockAuthApi, +})); + +vi.mock("../api/userProfiles", () => ({ + userProfilesApi: mockUserProfilesApi, +})); + +vi.mock("../context/BreadcrumbContext", () => ({ + useBreadcrumbs: () => ({ + setBreadcrumbs: mockSetBreadcrumbs, + }), +})); + +vi.mock("../context/CompanyContext", () => ({ + useCompany: () => ({ + selectedCompanyId: "company-1", + selectedCompany: { id: "company-1", name: "Paperclip", issuePrefix: "PAP" }, + }), +})); + +vi.mock("@/lib/router", () => ({ + Link: ({ children, to, ...props }: { children: React.ReactNode; to: string }) => ( + {children} + ), + useParams: () => mockUseParams(), +})); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +async function flushReact() { + await act(async () => { + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 0)); + }); +} + +function windowStats(key: "last7" | "last30" | "all", label: string): UserProfileResponse["stats"][number] { + return { + key, + label, + touchedIssues: 4, + createdIssues: 2, + completedIssues: 3, + assignedOpenIssues: 1, + commentCount: 5, + activityCount: 9, + costCents: 1200, + inputTokens: 1000, + cachedInputTokens: 200, + outputTokens: 300, + costEventCount: 2, + }; +} + +function profileFor(userId: string): UserProfileResponse { + return { + user: { + id: userId, + slug: "jane-example", + name: "Jane Example", + email: "jane@example.com", + image: null, + membershipRole: "owner", + membershipStatus: "active", + joinedAt: new Date("2026-01-05T00:00:00Z"), + }, + stats: [windowStats("last7", "Last 7 days"), windowStats("last30", "Last 30 days"), windowStats("all", "All time")], + daily: [], + recentIssues: [], + recentActivity: [], + topAgents: [], + topProviders: [], + }; +} + +const EDIT_PROFILE_HREF = "/company/settings/instance/profile"; + +describe("UserProfile", () => { + let container: HTMLDivElement; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + mockUseParams.mockReturnValue({ userSlug: "jane-example" }); + mockAuthApi.getSession.mockResolvedValue({ + session: { id: "session-1", userId: "user-1" }, + user: { + id: "user-1", + name: "Jane Example", + email: "jane@example.com", + image: null, + }, + }); + }); + + afterEach(() => { + container.remove(); + document.body.innerHTML = ""; + vi.clearAllMocks(); + }); + + async function render(profile: UserProfileResponse, hiddenSettings: string[] = []) { + mockUserProfilesApi.get.mockResolvedValue(profile); + const root = createRoot(container); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.setQueryData(queryKeys.health, { + status: "ok", + deploymentMode: "authenticated", + hiddenSettings, + }); + + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + await flushReact(); + return root; + } + + it("offers an edit link on the viewer's own profile", async () => { + const root = await render(profileFor("user-1")); + + expect(container.textContent).toContain("Jane Example"); + const editLink = container.querySelector(`a[href="${EDIT_PROFILE_HREF}"]`); + expect(editLink).not.toBeNull(); + expect(editLink?.textContent).toContain("Edit profile"); + expect(editLink?.querySelector("svg")?.classList).toContain("lucide-user-round-pen"); + + await act(async () => root.unmount()); + }); + + it("keeps other users' profiles read-only", async () => { + const root = await render(profileFor("user-2")); + + expect(container.textContent).toContain("Jane Example"); + expect(container.querySelector(`a[href="${EDIT_PROFILE_HREF}"]`)).toBeNull(); + expect(container.textContent).not.toContain("Edit profile"); + + await act(async () => root.unmount()); + }); + + it("drops the edit link when the operator hides profile settings", async () => { + const root = await render(profileFor("user-1"), ["instance.profile"]); + + expect(container.textContent).toContain("Jane Example"); + expect(container.querySelector(`a[href="${EDIT_PROFILE_HREF}"]`)).toBeNull(); + + await act(async () => root.unmount()); + }); +}); diff --git a/ui/src/pages/UserProfile.tsx b/ui/src/pages/UserProfile.tsx index f91e33c2f98..5bca9229172 100644 --- a/ui/src/pages/UserProfile.tsx +++ b/ui/src/pages/UserProfile.tsx @@ -1,15 +1,22 @@ import { useEffect, useMemo } from "react"; import { useQuery } from "@tanstack/react-query"; -import { AlertCircle, UserRound } from "lucide-react"; -import type { UserProfileDailyPoint, UserProfileWindowStats } from "@paperclipai/shared"; +import { AlertCircle, UserRound, UserRoundPen } from "lucide-react"; +import { + hidesInstancePage, + type UserProfileDailyPoint, + type UserProfileWindowStats, +} from "@paperclipai/shared"; import { Link, useParams } from "@/lib/router"; +import { authApi } from "../api/auth"; import { userProfilesApi } from "../api/userProfiles"; import { Avatar, AvatarFallback, AvatarImage } from "../components/ui/avatar"; +import { Button } from "../components/ui/button"; import { EmptyState } from "../components/EmptyState"; import { PageSkeleton } from "../components/PageSkeleton"; import { IssueStatusBadge } from "../components/StatusBadge"; import { useBreadcrumbs } from "../context/BreadcrumbContext"; import { useCompany } from "../context/CompanyContext"; +import { useHiddenSettings } from "../hooks/useHiddenSettings"; import { queryKeys } from "../lib/queryKeys"; import { formatCents, @@ -23,6 +30,7 @@ import { } from "../lib/utils"; const NO_COMPANY = "__none__"; +const PROFILE_SETTINGS_PATH = "/company/settings/instance/profile"; function initials(name: string | null | undefined) { const value = name?.trim() || "User"; @@ -205,6 +213,18 @@ export function UserProfile() { queryFn: () => userProfilesApi.get(companyId, userSlug), enabled: !!selectedCompanyId && !!userSlug, }); + // The edit link only belongs on the viewer's own profile, and never when the + // hosting operator hides the profile settings surface the link points at. + const { data: session } = useQuery({ + queryKey: queryKeys.auth.session, + queryFn: () => authApi.getSession(), + retry: false, + }); + const { hidden: hiddenSettings } = useHiddenSettings(); + const canEditProfile = + Boolean(data?.user.id) && + data?.user.id === session?.user.id && + !hidesInstancePage(hiddenSettings, "instance.profile"); useEffect(() => { setBreadcrumbs([{ label: "Users" }, { label: data?.user.name ?? userSlug }]); @@ -280,6 +300,14 @@ export function UserProfile() { {metaParts.join(" · ")}
+ {canEditProfile ? ( + + ) : null}
From c8f874311c02eff7cdf5ab9e85dc37d80ed50147 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:22:25 -0500 Subject: [PATCH 6/6] fix(ui): hide Google connectors only on the Connections page (#14774) ## Thinking Path > - Paperclip helps people manage AI agents for work. > - The Connections page lists the apps and saved accounts that agents can use. > - Google Workspace verification is still pending. > - Google entries must be temporarily hidden from this page without removing their implementations. > - This PR filters the final page rows, including saved Google accounts, after the page resolves their provider. > - Definitions, direct setup routes, OAuth profiles, credentials, and runtime access stay intact. > - Review instances can keep the prior UI by staying on their pinned app release. ## Linked Issues or Issue Description **What existing behavior does this improve?** Temporary provider visibility on the Connections landing page. **Current behavior** The page can show Google Workspace catalog entries and saved accounts while verification is pending. **Proposed behavior** Hide all nine Google Workspace rows on this page. Keep every other connector and all Google integration code unchanged. Use an existing release pin for review instances instead of a hostname exception in the app. **Reason and benefit** Pause public discovery without disabling existing runtime tools or removing the implementation needed for verification and later re-enablement. **Breaking changes** Google accounts are no longer visible on this landing page. Direct setup and management routes remain available. This is not an access-control restriction. Related completed work: #13551 used catalog-level visibility. This change is deliberately limited to the landing page and also covers saved account rows. #14740 reduced Google scopes; this change leaves those scopes unchanged. No duplicate open PR or matching open issue was found. ## What Changed - Derive the Google app slugs from the existing Workspace profile registry. - Filter the combined catalog and saved-account rows only inside `Browse`. - Cover all nine Google entries, active/draft/disabled accounts, legacy connection metadata, mixed-provider rows, and independently identified non-Google connectors in regression tests. - Document the display-only hold, pinned review builds, and how to restore visibility after approval. ## Verification - Passed: `pnpm exec vitest run ui/src/pages/apps/Browse.test.tsx ui/src/pages/apps/AppsConnect.test.tsx` (199 tests, including the latest master changes). - Passed: `pnpm check:token-gates`. - Passed: `pnpm build`. - Passed: `pnpm -r typecheck` and `pnpm build` after merging the latest master. An earlier overlapping run hit a local runner codesign race; sequential checks passed. - Passed again after the final custom-provider fix: `pnpm --filter @paperclipai/ui typecheck` and `pnpm --filter @paperclipai/ui build`. - The full local `pnpm test:run` was started, then stopped after the full remote CI suite passed to avoid continuing duplicate long-running work on the developer machine. It is not claimed as a completed local pass. - All 54 latest-head CI checks passed. Two non-applicable Storybook jobs were skipped. One serialized server job lost its self-hosted runner connection; its single retry passed. - Greptile: 5/5 on `aeda167bf4494feed6ee0de2585960511fb02918`, with no unresolved review threads. - Confirmed in the existing review instance that all nine Google entries still appear after its current release was pinned. No new app release was deployed to that instance. - Reviewer steps: open Connections on this branch with Google catalog entries and saved Google accounts. None should appear. Non-Google connectors must remain. Direct Google setup routes must still load. ## Risks - Existing Google accounts cannot be found on this page during the hold. Their data and runtime access remain unchanged. - This is a UI-only filter, not an authorization gate. Direct routes and API access still work by design. - Review instances must not receive this UI build until the hold is removed. Their existing release pin excludes fleet app upgrades; an explicit targeted upgrade must still be avoided. - No migrations, backend changes, broker changes, or credential changes. ## Model Used OpenAI Codex (GPT-5-based coding agent), with reasoning, tool use, code execution, and browser inspection. The exact deployment model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/connections/GOOGLE-WORKSPACE.md | 14 ++++ ui/src/pages/apps/Browse.test.tsx | 125 ++++++++++++++++++++++++++-- ui/src/pages/apps/Browse.tsx | 23 ++++- 3 files changed, 154 insertions(+), 8 deletions(-) diff --git a/doc/connections/GOOGLE-WORKSPACE.md b/doc/connections/GOOGLE-WORKSPACE.md index 98b256a4d8b..ff76addd514 100644 --- a/doc/connections/GOOGLE-WORKSPACE.md +++ b/doc/connections/GOOGLE-WORKSPACE.md @@ -22,6 +22,20 @@ Google's hosted Workspace MCP servers are Developer Preview services. The app cards remain independent even when several services use the same customer-owned Google OAuth client or the same Paperclip Cloud broker deployment. +## Temporary Connections page visibility hold + +While Google OAuth verification is pending, the Connections landing page +(`ui/src/pages/apps/Browse.tsx`) hides all nine Google Workspace entries, +including their saved accounts. This is a display-only filter. App definitions, +direct setup and management routes, OAuth profiles, saved credentials, and +runtime tools remain unchanged. This is not an access-control restriction. + +Keep verification instances pinned to their pre-hold app release so reviewers +can still find and test the integrations. After approval, remove the page's +`GOOGLE_CONNECTOR_SLUGS` filter and update its visibility tests before upgrading +those instances. Do not disable the shared definitions or broker profiles to +control this page's visibility. + ## Developer Preview enrollment Google grants preview access to the specific Workspace email addresses and diff --git a/ui/src/pages/apps/Browse.test.tsx b/ui/src/pages/apps/Browse.test.tsx index 6fa85625e0b..6c3bc76dc44 100644 --- a/ui/src/pages/apps/Browse.test.tsx +++ b/ui/src/pages/apps/Browse.test.tsx @@ -200,6 +200,124 @@ describe("Connectors landing page", () => { expect(container.textContent).not.toContain("Paused"); }); + const googleSlugs = [ + "gmail", "google-drive", "google-docs", "google-sheets", "google-slides", + "google-calendar", "google-chat", "google-people", "google-workspace-search", + ]; + + it("temporarily hides all Google Workspace catalog rows without changing their definitions", async () => { + const definitions = googleSlugs.map((slug) => getAppStoreDefinition(slug)!); + listGalleryMock.mockResolvedValue({ apps: [...definitions, getAppStoreDefinition("notion")] }); + const client = await renderBrowse(); + + for (const definition of definitions) { + expect(definition.methods.length).toBeGreaterThan(0); + expect(getAppStoreDefinition(definition.slug)).toBe(definition); + expect(container.querySelector(`[data-app-slug="${definition.slug}"]`)).toBeNull(); + } + expect(container.querySelector('[data-app-slug="notion"]')).not.toBeNull(); + expect(client.getQueryData(queryKeys.apps.gallery("company-1"))).toEqual({ + apps: [...definitions, getAppStoreDefinition("notion")], + }); + expect(archiveConnectionMock).not.toHaveBeenCalled(); + }); + + it.each(["active", "draft", "disabled"])( + "hides saved Google %s accounts without disabling or removing them", + async (status) => { + const applications = googleSlugs.map((slug) => application({ + id: `app-${slug}`, name: `Saved ${slug}`, applicationKey: `app-gallery:${slug}:one`, + metadata: { sourceTemplateKey: slug }, + })); + const connections = googleSlugs.map((slug) => connection({ + id: `conn-${slug}`, applicationId: `app-${slug}`, name: `Account for ${slug}`, + status, enabled: status !== "disabled", config: { sourceTemplateKey: slug }, + })); + listGalleryMock.mockResolvedValue({ apps: googleSlugs.map(getAppStoreDefinition) }); + listApplicationsMock.mockResolvedValue({ applications }); + listConnectionsMock.mockResolvedValue({ connections }); + const client = await renderBrowse(); + + for (const slug of googleSlugs) { + expect(container.querySelector(`[data-app-slug="${slug}"]`)).toBeNull(); + expect(container.textContent).not.toContain(`Account for ${slug}`); + } + expect(client.getQueryData(queryKeys.tools.connections("company-1"))).toEqual({ connections }); + expect(client.getQueryData(queryKeys.tools.applications("company-1"))).toEqual({ applications }); + expect(archiveConnectionMock).not.toHaveBeenCalled(); + }, + ); + + it.each(["config", "transportConfig"])( + "hides a Google account identified by %s even when its gallery entry is absent", + async (sourceField) => { + listGalleryMock.mockResolvedValue({ apps: [] }); + listApplicationsMock.mockResolvedValue({ applications: [application({ + id: "legacy-google", name: "My documents", applicationKey: null, metadata: null, + }), application()] }); + listConnectionsMock.mockResolvedValue({ connections: [connection({ + id: "legacy-google-account", applicationId: "legacy-google", name: "Saved Google account", + config: {}, transportConfig: {}, [sourceField]: { sourceTemplateKey: "google-docs" }, + }), connection()] }); + await renderBrowse(); + + expect(container.textContent).not.toContain("Saved Google account"); + expect(container.textContent).toContain("Notion"); + expect(archiveConnectionMock).not.toHaveBeenCalled(); + }, + ); + + it.each(["catalog", "custom"])( + "preserves non-Google accounts in a mixed-provider %s row", + async (rowKind) => { + const notion = getAppStoreDefinition("notion")!; + listGalleryMock.mockResolvedValue({ apps: rowKind === "catalog" ? [notion] : [] }); + listApplicationsMock.mockResolvedValue({ applications: [application(rowKind === "custom" + ? { name: "My tools", applicationKey: null, metadata: null } + : {})] }); + const connections = [connection({ + id: "google-account", name: "Hidden Google account", + config: { sourceTemplateKey: "google-docs" }, + }), connection({ + id: "notion-account", name: "Visible Notion account", + config: { sourceTemplateKey: "notion" }, + })]; + listConnectionsMock.mockResolvedValue({ connections }); + const client = await renderBrowse(); + + expect(container.textContent).toContain("Visible Notion account"); + expect(container.textContent).not.toContain("Hidden Google account"); + expect(container.textContent).toContain(rowKind === "catalog" ? "Notion" : "My tools"); + expect(client.getQueryData(queryKeys.tools.connections("company-1"))).toEqual({ connections }); + expect(archiveConnectionMock).not.toHaveBeenCalled(); + }, + ); + + it.each(["metadata", "applicationKey"])( + "keeps a non-Google custom connector identified by %s when only its Google accounts are hidden", + async (sourceField) => { + listGalleryMock.mockResolvedValue({ apps: [] }); + const savedApplication = application({ + name: "My custom connector", + metadata: sourceField === "metadata" ? { sourceTemplateKey: "custom-provider" } : null, + applicationKey: sourceField === "applicationKey" ? "custom-provider" : null, + }); + listApplicationsMock.mockResolvedValue({ applications: [savedApplication] }); + const connections = [connection({ + name: "Hidden Google account", config: { sourceTemplateKey: "google-docs" }, + })]; + listConnectionsMock.mockResolvedValue({ connections }); + const client = await renderBrowse(); + + expect(container.querySelector('[data-app-slug="custom-provider"]')).not.toBeNull(); + expect(container.textContent).toContain("My custom connector"); + expect(container.textContent).not.toContain("Hidden Google account"); + expect(client.getQueryData(queryKeys.tools.applications("company-1"))).toEqual({ applications: [savedApplication] }); + expect(client.getQueryData(queryKeys.tools.connections("company-1"))).toEqual({ connections }); + expect(archiveConnectionMock).not.toHaveBeenCalled(); + }, + ); + it("hides cached memory connectors until enabled and preserves saved MCP connections", async () => { const providers = ["mem0", "zep", "supermemory", "cognee", "honcho"]; listGalleryMock.mockResolvedValue({ apps: [...providers, "notion"].map(getAppStoreDefinition) }); @@ -326,7 +444,6 @@ describe("Connectors landing page", () => { ).toEqual([ "discord", "github-code-review-bot", - "gmail", "imessage-photon", "jira", "microsoft-teams", @@ -338,11 +455,7 @@ describe("Connectors landing page", () => { expect( container.querySelector('button[aria-label="Connect Jira"]'), ).toBeTruthy(); - expect( - container.querySelector( - 'button[aria-label="Unavailable Gmail"]', - )?.disabled, - ).toBe(true); + expect(container.querySelector('[data-app-slug="gmail"]')).toBeNull(); expect(container.textContent).toContain("Connect your own tool"); const customConnect = container.querySelector( diff --git a/ui/src/pages/apps/Browse.tsx b/ui/src/pages/apps/Browse.tsx index d111e52b56b..70d79679e56 100644 --- a/ui/src/pages/apps/Browse.tsx +++ b/ui/src/pages/apps/Browse.tsx @@ -23,6 +23,7 @@ import { getAppStoreDefinition, isToolConnectionAttentionHealth, aiSubscriptionNeedsIsolatedLogin, + GOOGLE_WORKSPACE_CONNECTOR_PROFILES, } from "@paperclipai/shared"; import { useNavigate } from "@/lib/router"; import { useChatConnectorsEnabled } from "@/hooks/useChatConnectorsEnabled"; @@ -63,6 +64,7 @@ import { buildCompanyUserProfileMap } from "@/lib/company-members"; import { AppLogo } from "./AppLogo"; import { appApplicationSourceSlug, + appConnectionSourceSlug, appDefinitionDarkLogoUrl, appDefinitionDescription, appDefinitionLogoUrl, @@ -111,6 +113,13 @@ type ConnectionRemovalTarget = { }; +// Temporary, page-only hold until Google OAuth verification is approved. +// Keep definitions, direct setup/management routes, and runtime access intact. +// Remove this filter after approval; reviewer instances stay on their pinned build. +const GOOGLE_CONNECTOR_SLUGS = new Set( + Object.values(GOOGLE_WORKSPACE_CONNECTOR_PROFILES).map((profile) => profile.appSlug), +); + function chatProviderForSlug(slug: string): ChatProvider | null { const method = getAppStoreDefinition(slug)?.methods.find( (candidate) => @@ -477,8 +486,18 @@ export function Browse({ renderAccountDetails = (connection) => connection.conne const customRows: ConnectorRowModel[] = []; for (const application of activeApplications) { - const appConnections = + const applicationSlug = appApplicationSourceSlug(application); + const savedAppConnections = connectionsByApplicationId.get(application.id) ?? []; + const appConnections = savedAppConnections.filter( + (connection) => !GOOGLE_CONNECTOR_SLUGS.has(appConnectionSourceSlug(connection) ?? ""), + ); + // Hide source-only Google rows, but keep independently identified connectors. + if ( + (!applicationSlug || applicationSlug === "link") && + savedAppConnections.length > 0 && + appConnections.length === 0 + ) continue; const configuredConnectionSlug = appConnections .map( (connection) => @@ -500,7 +519,6 @@ export function Browse({ renderAccountDetails = (connection) => connection.conne : null, ) .find((value): value is string => Boolean(value)); - const applicationSlug = appApplicationSourceSlug(application); const resolvedSlug = applicationSlug && applicationSlug !== "link" && @@ -568,6 +586,7 @@ export function Browse({ renderAccountDetails = (connection) => connection.conne } return [...rowsBySlug.values(), ...customRows] + .filter((row) => !GOOGLE_CONNECTOR_SLUGS.has(row.slug)) .map((row) => ({ ...row, connections: [...row.connections].sort(