mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
fix(sentry): keep low-trust project denials on the task (#19531)
Keep exact pre-dispatch project access denials on their task instead of the unexpected Sentry failure stream. Validation: 301 focused tests and all applicable CI pass. Greptile scored the exact head 5/5, and no review threads remain unresolved. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
24508a8ba5
commit
91b641757b
3 files changed
+150
No files matched your search
@@ -60,6 +60,14 @@ run lacks direct-chat authority or the user lacks editing permission. The user
|
||||
can request the edit in their authorized chat or apply it directly. Creating
|
||||
another task does not authorize the instruction change.
|
||||
|
||||
When a low-trust agent's task is in a project outside its boundary, setup fails
|
||||
the run with the typed `project_access_denied` code and the denied project ID
|
||||
before provider work starts. The task notice links Manage project access. That
|
||||
exact pre-dispatch shape is an owner action, so it stays on the task instead of
|
||||
the unexpected Sentry run-failure stream. The legacy `setup_failed` code and the
|
||||
denial message alone never suppress a report, and
|
||||
`PAPERCLIP_SENTRY_RUN_FAILURES=all` still reports it.
|
||||
|
||||
## Direct-chat instruction edits
|
||||
|
||||
When an authorized user directly asks an agent in their own Agent Chat to edit
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { agents, applyPendingMigrations, closeRegisteredClients, companies, createDb, heartbeatRuns } from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "../../__tests__/helpers/embedded-postgres.js";
|
||||
|
||||
const mockCaptureRunFailure = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../../sentry.js", () => ({
|
||||
captureRunFailure: mockCaptureRunFailure,
|
||||
}));
|
||||
|
||||
import { reportRunFailure } from "../run-failure-report.js";
|
||||
import { PROJECT_ACCESS_DENIED } from "../project-access-denial.js";
|
||||
|
||||
const testDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL;
|
||||
const embeddedPostgresSupport = testDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
describeEmbeddedPostgres("reportRunFailure low-trust project denials", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
let companyId!: string;
|
||||
let agentId!: string;
|
||||
let inheritedEnv: NodeJS.ProcessEnv;
|
||||
|
||||
beforeAll(async () => {
|
||||
if (testDatabaseUrl) {
|
||||
await applyPendingMigrations(testDatabaseUrl);
|
||||
db = createDb(testDatabaseUrl);
|
||||
} else {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-run-failure-project-access-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}
|
||||
}, 20_000);
|
||||
|
||||
beforeEach(async () => {
|
||||
inheritedEnv = process.env;
|
||||
process.env = Object.fromEntries(
|
||||
["PATH", "HOME", "USER", "USERNAME", "LOGNAME", "USERPROFILE", "TMPDIR", "TEMP", "TMP"]
|
||||
.flatMap((key) => inheritedEnv[key] === undefined ? [] : [[key, inheritedEnv[key]]]),
|
||||
);
|
||||
process.env.PAPERCLIP_SENTRY_RUN_FAILURES = "unexpected";
|
||||
companyId = randomUUID();
|
||||
agentId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
defaultResponsibleUserId: "responsible-user",
|
||||
});
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "OpenCodeCoder",
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "opencode_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
vi.unstubAllEnvs();
|
||||
process.env = inheritedEnv;
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
if (testDatabaseUrl) await closeRegisteredClients(testDatabaseUrl);
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
function projectAccessDeniedRun(
|
||||
overrides: Partial<typeof heartbeatRuns.$inferSelect> = {},
|
||||
): typeof heartbeatRuns.$inferSelect {
|
||||
return {
|
||||
id: randomUUID(),
|
||||
companyId,
|
||||
agentId,
|
||||
status: "failed",
|
||||
error: "Project is outside this low-trust boundary.",
|
||||
errorCode: PROJECT_ACCESS_DENIED,
|
||||
executionStage: "preparing",
|
||||
nativeIssueId: randomUUID(),
|
||||
contextSnapshot: null,
|
||||
resultJson: {
|
||||
projectAccessDenied: { projectId: randomUUID() },
|
||||
executionRecovery: { kind: "bootstrap", providerWorkStarted: false },
|
||||
},
|
||||
...overrides,
|
||||
} as unknown as typeof heartbeatRuns.$inferSelect;
|
||||
}
|
||||
|
||||
it("keeps a low-trust project denial before dispatch on the task", async () => {
|
||||
await reportRunFailure(db, projectAccessDeniedRun(), { phase: "setup" });
|
||||
expect(mockCaptureRunFailure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ status: "timed_out" }, { errorCode: "setup_failed" }, { executionStage: "executing" }, { exitCode: 1 }, { signal: "SIGTERM" },
|
||||
{ resultJson: { projectAccessDenied: { projectId: randomUUID() } } },
|
||||
{ resultJson: { projectAccessDenied: { projectId: randomUUID() }, executionRecovery: { kind: "bootstrap", providerWorkStarted: true } } },
|
||||
{ resultJson: { projectAccessDenied: {}, executionRecovery: { kind: "bootstrap", providerWorkStarted: false } } },
|
||||
])("reports project denial markers without an exact pre-dispatch outcome: %j", async (overrides) => {
|
||||
await reportRunFailure(db, projectAccessDeniedRun(overrides), { phase: "setup" });
|
||||
expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it.each([undefined, "execute"] as const)("reports project denial markers outside the setup report: %s", async (phase) => {
|
||||
await reportRunFailure(db, projectAccessDeniedRun(), { phase });
|
||||
expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("reports project denials when every run failure is reported", async () => {
|
||||
vi.stubEnv("PAPERCLIP_SENTRY_RUN_FAILURES", "all");
|
||||
await reportRunFailure(db, projectAccessDeniedRun(), { phase: "setup" });
|
||||
expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -17,6 +17,7 @@ import { readRunFailureReportingMode, type RunFailureReportingMode } from "../se
|
||||
import { RESTRICTED_WORKSPACE_SOURCE_REASON } from "./workspace-source-restriction.js";
|
||||
import { PLUGIN_RUNNER_AI_CONNECTION_REQUIRED } from "./plugin-runner-ai-connection.js";
|
||||
import { NATIVE_PROVIDER_OVERLOADED_CODE } from "./native-runtime/native-provider-failure.js";
|
||||
import { PROJECT_ACCESS_DENIED } from "./project-access-denial.js";
|
||||
|
||||
type HeartbeatRun = typeof heartbeatRuns.$inferSelect;
|
||||
|
||||
@@ -173,6 +174,18 @@ function isPluginRunnerAiConnectionRequiredBlocker(run: HeartbeatRun, options: R
|
||||
recovery?.kind === "bootstrap" && recovery.providerWorkStarted === false;
|
||||
}
|
||||
|
||||
function isProjectAccessDenialBlocker(run: HeartbeatRun, options: RunFailureReportOptions): boolean {
|
||||
if (
|
||||
run.status !== "failed" || run.errorCode !== PROJECT_ACCESS_DENIED ||
|
||||
run.executionStage !== "preparing" || options.phase !== "setup" ||
|
||||
run.exitCode != null || run.signal != null
|
||||
) return false;
|
||||
const denial = asRecord(run.resultJson?.projectAccessDenied);
|
||||
const recovery = asRecord(run.resultJson?.executionRecovery);
|
||||
return typeof denial?.projectId === "string" &&
|
||||
recovery?.kind === "bootstrap" && recovery.providerWorkStarted === false;
|
||||
}
|
||||
|
||||
/** Only the managed-clone producer can attach this bounded pre-dispatch evidence. */
|
||||
function isGitConnectionFailure(run: HeartbeatRun, options: RunFailureReportOptions): boolean {
|
||||
const connection = readConnectionFailure(run.resultJson?.connectionFailure);
|
||||
@@ -333,6 +346,7 @@ export function reportRunFailure(db: Db, run: HeartbeatRun, options: RunFailureR
|
||||
isLocalPathWorkspaceConfigurationBlocker(run, options) ||
|
||||
isRestrictedWorkspaceSourceBlocker(run, options) ||
|
||||
isPluginRunnerAiConnectionRequiredBlocker(run, options) ||
|
||||
isProjectAccessDenialBlocker(run, options) ||
|
||||
isGitConnectionFailure(run, options) ||
|
||||
isProviderQuotaFailure(run, options) ||
|
||||
isCodexAcpModelCapacityFailure(run, options) ||
|
||||
|
||||
Reference in new issue
Block a user