diff --git a/packages/shared/src/validators/approval.test.ts b/packages/shared/src/validators/approval.test.ts index 15034d7001d..051d29b2740 100644 --- a/packages/shared/src/validators/approval.test.ts +++ b/packages/shared/src/validators/approval.test.ts @@ -6,6 +6,20 @@ import { } from "./approval.js"; describe("approval validators", () => { + it("passes real line breaks through unchanged", () => { + expect(addApprovalCommentSchema.parse({ body: "Looks good\n\nApproved." }).body) + .toBe("Looks good\n\nApproved."); + expect(resolveApprovalSchema.parse({ decisionNote: "Decision\n\nApproved." }).decisionNote) + .toBe("Decision\n\nApproved."); + }); + + it("accepts null and omitted optional decision notes", () => { + expect(resolveApprovalSchema.parse({ decisionNote: null }).decisionNote).toBeNull(); + expect(resolveApprovalSchema.parse({}).decisionNote).toBeUndefined(); + expect(requestApprovalRevisionSchema.parse({ decisionNote: null }).decisionNote).toBeNull(); + expect(requestApprovalRevisionSchema.parse({}).decisionNote).toBeUndefined(); + }); + it("normalizes escaped line breaks in approval comments and decision notes", () => { expect(addApprovalCommentSchema.parse({ body: "Looks good\\n\\nApproved." }).body) .toBe("Looks good\n\nApproved."); diff --git a/packages/shared/src/validators/issue.test.ts b/packages/shared/src/validators/issue.test.ts index 4f1ecf45141..4fa908e40cb 100644 --- a/packages/shared/src/validators/issue.test.ts +++ b/packages/shared/src/validators/issue.test.ts @@ -9,6 +9,24 @@ import { } from "./issue.js"; describe("issue validators", () => { + it("passes real line breaks through unchanged", () => { + const parsed = createIssueSchema.parse({ + title: "Follow up PR", + description: "Line 1\n\nLine 2", + }); + + expect(parsed.description).toBe("Line 1\n\nLine 2"); + }); + + it("accepts null and omitted optional multiline issue fields", () => { + expect(createIssueSchema.parse({ title: "Follow up PR", description: null }).description) + .toBeNull(); + expect(createIssueSchema.parse({ title: "Follow up PR" }).description) + .toBeUndefined(); + expect(updateIssueSchema.parse({ comment: undefined }).comment) + .toBeUndefined(); + }); + it("normalizes JSON-escaped line breaks in issue descriptions", () => { const parsed = createIssueSchema.parse({ title: "Follow up PR", diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index cf51095fa23..11f039af1db 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -91,13 +91,24 @@ const mockLogActivity = vi.hoisted(() => vi.fn()); const mockTrackAgentCreated = vi.hoisted(() => vi.fn()); const mockGetTelemetryClient = vi.hoisted(() => vi.fn()); const mockSyncInstructionsBundleConfigFromFilePath = vi.hoisted(() => vi.fn()); -const mockEnvironmentService = vi.hoisted(() => ({})); +const mockEnsureOpenCodeModelConfiguredAndAvailable = vi.hoisted(() => vi.fn()); +const mockEnvironmentService = vi.hoisted(() => ({ + getById: vi.fn(), +})); const mockInstanceSettingsService = vi.hoisted(() => ({ getGeneral: vi.fn(), })); function registerModuleMocks() { + vi.doMock("@paperclipai/adapter-opencode-local/server", async () => { + const actual = await vi.importActual("@paperclipai/adapter-opencode-local/server"); + return { + ...actual, + ensureOpenCodeModelConfiguredAndAvailable: mockEnsureOpenCodeModelConfiguredAndAvailable, + }; + }); + vi.doMock("@paperclipai/shared/telemetry", () => ({ trackAgentCreated: mockTrackAgentCreated, trackErrorHandlerCrash: vi.fn(), @@ -307,6 +318,8 @@ describe.sequential("agent permission routes", () => { mockGetTelemetryClient.mockReset(); mockSyncInstructionsBundleConfigFromFilePath.mockReset(); mockInstanceSettingsService.getGeneral.mockReset(); + mockEnvironmentService.getById.mockReset(); + mockEnsureOpenCodeModelConfiguredAndAvailable.mockReset(); mockSyncInstructionsBundleConfigFromFilePath.mockImplementation((_agent, config) => config); mockGetTelemetryClient.mockReturnValue({ track: vi.fn() }); mockAgentService.getById.mockResolvedValue(baseAgent); @@ -834,6 +847,232 @@ describe.sequential("agent permission routes", () => { })); }); + it("rejects creating an agent with an environment from another company", async () => { + const environmentId = "33333333-3333-4333-8333-333333333333"; + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId: "other-company", + driver: "local", + config: {}, + }); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Builder", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + defaultEnvironmentId: environmentId, + })); + + expect(res.status).toBe(422); + expect(res.body.error).toContain("Environment not found"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + }); + + it("rejects creating an agent with an unsupported default environment driver", async () => { + const environmentId = "33333333-3333-4333-8333-333333333333"; + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId, + driver: "ssh", + config: {}, + }); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Builder", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + defaultEnvironmentId: environmentId, + })); + + expect(res.status).toBe(422); + expect(res.body.error).toContain('Environment driver "ssh" is not allowed here'); + expect(mockAgentService.create).not.toHaveBeenCalled(); + }); + + const sshCapableAdapterCases = [ + { adapterType: "codex_local", name: "Codex Builder", adapterConfig: {} }, + { adapterType: "claude_local", name: "Claude Builder", adapterConfig: {} }, + { adapterType: "gemini_local", name: "Gemini Builder", adapterConfig: {} }, + { adapterType: "opencode_local", name: "OpenCode Builder", adapterConfig: { model: "opencode/gpt-5-nano" } }, + { adapterType: "cursor", name: "Cursor Builder", adapterConfig: {} }, + { adapterType: "pi_local", name: "Pi Builder", adapterConfig: { model: "openai/gpt-5.4-mini" } }, + ]; + + for (const adapterCase of sshCapableAdapterCases) { + it(`allows creating a ${adapterCase.adapterType} agent with an SSH default environment`, async () => { + const environmentId = "33333333-3333-4333-8333-333333333333"; + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId, + driver: "ssh", + config: {}, + }); + mockAgentService.create.mockResolvedValue({ + ...baseAgent, + name: adapterCase.name, + adapterType: adapterCase.adapterType, + defaultEnvironmentId: environmentId, + }); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: adapterCase.name, + role: "engineer", + adapterType: adapterCase.adapterType, + adapterConfig: adapterCase.adapterConfig, + defaultEnvironmentId: environmentId, + })); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(mockAgentService.create).toHaveBeenCalledWith( + companyId, + expect.objectContaining({ + adapterType: adapterCase.adapterType, + defaultEnvironmentId: environmentId, + }), + ); + }); + } + + it("rejects updating an agent with an unsupported default environment driver", async () => { + const environmentId = "33333333-3333-4333-8333-333333333333"; + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId, + driver: "ssh", + config: {}, + }); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ + defaultEnvironmentId: environmentId, + })); + + expect(res.status).toBe(422); + expect(res.body.error).toContain('Environment driver "ssh" is not allowed here'); + expect(mockAgentService.update).not.toHaveBeenCalled(); + }); + + for (const adapterCase of sshCapableAdapterCases) { + it(`allows updating a ${adapterCase.adapterType} agent with an SSH default environment`, async () => { + const environmentId = "33333333-3333-4333-8333-333333333333"; + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId, + driver: "ssh", + config: {}, + }); + mockAgentService.getById.mockResolvedValue({ + ...baseAgent, + adapterType: adapterCase.adapterType, + adapterConfig: adapterCase.adapterConfig, + defaultEnvironmentId: null, + }); + mockAgentService.update.mockResolvedValue({ + ...baseAgent, + adapterType: adapterCase.adapterType, + adapterConfig: adapterCase.adapterConfig, + defaultEnvironmentId: environmentId, + }); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ + defaultEnvironmentId: environmentId, + })); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentService.update).toHaveBeenCalledWith( + agentId, + expect.objectContaining({ + defaultEnvironmentId: environmentId, + }), + expect.anything(), + ); + }); + } + + it("rejects switching an agent away from an SSH-capable runtime without clearing its SSH default", async () => { + const environmentId = "33333333-3333-4333-8333-333333333333"; + mockEnvironmentService.getById.mockResolvedValue({ + id: environmentId, + companyId, + driver: "ssh", + config: {}, + }); + mockAgentService.getById.mockResolvedValue({ + ...baseAgent, + adapterType: "codex_local", + defaultEnvironmentId: environmentId, + }); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ + adapterType: "process", + })); + + expect(res.status).toBe(422); + expect(res.body.error).toContain('Environment driver "ssh" is not allowed here'); + expect(mockAgentService.update).not.toHaveBeenCalled(); + }); + it("exposes explicit task assignment access on agent detail", async () => { mockAccessService.listPrincipalGrants.mockResolvedValue([ { diff --git a/server/src/__tests__/heartbeat-retry-scheduling.test.ts b/server/src/__tests__/heartbeat-retry-scheduling.test.ts index e5b452f4067..9999c05e523 100644 --- a/server/src/__tests__/heartbeat-retry-scheduling.test.ts +++ b/server/src/__tests__/heartbeat-retry-scheduling.test.ts @@ -489,6 +489,105 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { expect(issue?.executionRunId).toBeNull(); }); + it("does not promote a scheduled retry after the issue is cancelled", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const issueId = randomUUID(); + const sourceRunId = randomUUID(); + const now = new Date("2026-04-20T15:00:00.000Z"); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + await db.insert(agents).values({ + id: agentId, + companyId, + name: "CodexCoder", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: { + heartbeat: { + wakeOnDemand: true, + maxConcurrentRuns: 1, + }, + }, + permissions: {}, + }); + + await db.insert(heartbeatRuns).values({ + id: sourceRunId, + companyId, + agentId, + invocationSource: "assignment", + triggerDetail: "system", + status: "failed", + error: "upstream overload", + errorCode: "adapter_failed", + finishedAt: now, + contextSnapshot: { + issueId, + wakeReason: "issue_assigned", + }, + updatedAt: now, + createdAt: now, + }); + + await db.insert(issues).values({ + id: issueId, + companyId, + title: "Retry promotion cancellation", + status: "todo", + priority: "medium", + assigneeAgentId: agentId, + executionRunId: sourceRunId, + executionAgentNameKey: "codexcoder", + executionLockedAt: now, + issueNumber: 1, + identifier: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}-3`, + }); + + const scheduled = await heartbeat.scheduleBoundedRetry(sourceRunId, { + now, + random: () => 0.5, + }); + expect(scheduled.outcome).toBe("scheduled"); + if (scheduled.outcome !== "scheduled") return; + + await db.update(issues).set({ + status: "cancelled", + updatedAt: now, + }).where(eq(issues.id, issueId)); + + const promotion = await heartbeat.promoteDueScheduledRetries(scheduled.dueAt); + expect(promotion).toEqual({ promoted: 0, runIds: [] }); + + const oldRetry = await db + .select({ + status: heartbeatRuns.status, + errorCode: heartbeatRuns.errorCode, + }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, scheduled.run.id)) + .then((rows) => rows[0] ?? null); + expect(oldRetry).toEqual({ + status: "cancelled", + errorCode: "issue_cancelled", + }); + + const issue = await db + .select({ executionRunId: issues.executionRunId }) + .from(issues) + .where(eq(issues.id, issueId)) + .then((rows) => rows[0] ?? null); + expect(issue?.executionRunId).toBeNull(); + }); + it("exhausts bounded retries after the hard cap", async () => { const companyId = randomUUID(); const agentId = randomUUID(); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 0387cee9433..0d2c715f4da 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -3550,6 +3550,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) const issue = await db .select({ id: issues.id, + status: issues.status, assigneeAgentId: issues.assigneeAgentId, executionRunId: issues.executionRunId, }) @@ -3557,15 +3558,18 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) .where(and(eq(issues.id, dueRunIssueId), eq(issues.companyId, dueRun.companyId))) .then((rows) => rows[0] ?? null); - if (issue && issue.assigneeAgentId !== dueRun.agentId) { - const reason = "Cancelled because the issue was reassigned before the scheduled retry became due"; + if (issue && (issue.assigneeAgentId !== dueRun.agentId || issue.status === "cancelled")) { + const issueCancelled = issue.status === "cancelled"; + const reason = issueCancelled + ? "Cancelled because the issue was cancelled before the scheduled retry became due" + : "Cancelled because the issue was reassigned before the scheduled retry became due"; const cancelled = await db .update(heartbeatRuns) .set({ status: "cancelled", finishedAt: now, error: reason, - errorCode: "issue_reassigned", + errorCode: issueCancelled ? "issue_cancelled" : "issue_reassigned", updatedAt: now, }) .where( @@ -3608,9 +3612,12 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) eventType: "lifecycle", stream: "system", level: "warn", - message: "Scheduled retry cancelled because issue ownership changed before it became due", + message: issueCancelled + ? "Scheduled retry cancelled because issue was cancelled before it became due" + : "Scheduled retry cancelled because issue ownership changed before it became due", payload: { issueId: issue.id, + issueStatus: issue.status, scheduledRetryAttempt: cancelled.scheduledRetryAttempt, scheduledRetryAt: cancelled.scheduledRetryAt ? new Date(cancelled.scheduledRetryAt).toISOString() : null, scheduledRetryReason: cancelled.scheduledRetryReason, @@ -6305,6 +6312,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) .select({ id: issues.id, companyId: issues.companyId, + status: issues.status, assigneeAgentId: issues.assigneeAgentId, executionRunId: issues.executionRunId, executionAgentNameKey: issues.executionAgentNameKey, @@ -6331,22 +6339,25 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) } const cancelStaleScheduledRetry = async (scheduledRun: typeof heartbeatRuns.$inferSelect) => { + const issueCancelled = issue.status === "cancelled"; if ( scheduledRun.status !== "scheduled_retry" || - scheduledRun.agentId === issue.assigneeAgentId + (scheduledRun.agentId === issue.assigneeAgentId && !issueCancelled) ) { return false; } const now = new Date(); - const reason = "Cancelled because the issue was reassigned before the scheduled retry became due"; + const reason = issueCancelled + ? "Cancelled because the issue was cancelled before the scheduled retry became due" + : "Cancelled because the issue was reassigned before the scheduled retry became due"; const cancelled = await tx .update(heartbeatRuns) .set({ status: "cancelled", finishedAt: now, error: reason, - errorCode: "issue_reassigned", + errorCode: issueCancelled ? "issue_cancelled" : "issue_reassigned", updatedAt: now, }) .where(and(eq(heartbeatRuns.id, scheduledRun.id), eq(heartbeatRuns.status, "scheduled_retry"))) @@ -6379,6 +6390,33 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) .where(and(eq(issues.id, issue.id), eq(issues.executionRunId, scheduledRun.id))); } + const [eventSeq] = await tx + .select({ maxSeq: sql`max(${heartbeatRunEvents.seq})` }) + .from(heartbeatRunEvents) + .where(eq(heartbeatRunEvents.runId, cancelled.id)); + + await tx.insert(heartbeatRunEvents).values({ + companyId: cancelled.companyId, + runId: cancelled.id, + agentId: cancelled.agentId, + seq: Number(eventSeq?.maxSeq ?? 0) + 1, + eventType: "lifecycle", + stream: "system", + level: "warn", + message: issueCancelled + ? "Scheduled retry cancelled because issue was cancelled before it became due" + : "Scheduled retry cancelled because issue ownership changed before it became due", + payload: { + issueId: issue.id, + issueStatus: issue.status, + scheduledRetryAttempt: cancelled.scheduledRetryAttempt, + scheduledRetryAt: cancelled.scheduledRetryAt ? new Date(cancelled.scheduledRetryAt).toISOString() : null, + scheduledRetryReason: cancelled.scheduledRetryReason, + previousRetryAgentId: cancelled.agentId, + currentAssigneeAgentId: issue.assigneeAgentId, + }, + }); + return true; }; diff --git a/ui/src/components/IssueRunLedger.tsx b/ui/src/components/IssueRunLedger.tsx index cda31a41e11..45f9bc2671f 100644 --- a/ui/src/components/IssueRunLedger.tsx +++ b/ui/src/components/IssueRunLedger.tsx @@ -319,14 +319,14 @@ function canBoardRecordWatchdogDecision( companyId: string, boardAccess: CurrentBoardAccess | undefined, ) { - if (!boardAccess) return true; + if (!boardAccess) return false; if (boardAccess.source === "local_implicit" || boardAccess.isInstanceAdmin) return true; const membership = boardAccess.memberships?.find( (item) => item.companyId === companyId && item.status === "active", ); if (!membership) return boardAccess.companyIds.includes(companyId) && !boardAccess.memberships; - return membership.membershipRole !== "viewer"; + return membership.membershipRole !== "viewer" && membership.membershipRole !== null; } function watchdogDecisionErrorMessage(error: unknown) { @@ -386,12 +386,13 @@ export function IssueRunLedger({ }, onError: (error) => { const message = watchdogDecisionErrorMessage(error); + const dedupeSuffix = error instanceof ApiError ? String(error.status) : "error"; setWatchdogDecisionError(message); pushToast({ title: "Watchdog decision not recorded", body: message, tone: "error", - dedupeKey: `watchdog-decision:${issueId}:forbidden`, + dedupeKey: `watchdog-decision:${issueId}:${dedupeSuffix}`, }); }, }); diff --git a/ui/src/components/MarkdownBody.test.tsx b/ui/src/components/MarkdownBody.test.tsx index 326527a3eda..8f64bf8f6fa 100644 --- a/ui/src/components/MarkdownBody.test.tsx +++ b/ui/src/components/MarkdownBody.test.tsx @@ -325,6 +325,7 @@ describe("MarkdownBody", () => { // icon can never be orphaned on the previous line from the URL text. expect(html).toMatch(/.*lucide-github.*?<\/svg>h<\/span>/); expect(html).toContain("ttps://github.com/paperclipai/paperclip/pull/4099"); + expect(html).not.toContain("lucide-external-link"); }); it("prefixes GitHub autolinks with the GitHub icon", () => { diff --git a/ui/src/components/MarkdownBody.tsx b/ui/src/components/MarkdownBody.tsx index 0be4eb81e2d..b155032f788 100644 --- a/ui/src/components/MarkdownBody.tsx +++ b/ui/src/components/MarkdownBody.tsx @@ -140,6 +140,8 @@ function renderLinkBody( ): ReactNode { if (!leadingIcon && !trailingIcon) return children; + // React-markdown can pass arrays/elements for styled link text; the nowrap + // splitting below is intentionally limited to plain text links. if (typeof children === "string" && children.length > 0) { if (children.length === 1) { return ( @@ -332,7 +334,7 @@ export function MarkdownBody({ const leadingIcon = isGitHubLink ? (