fix(feedback): harden recovery authorization and guidance

Redact raw recovery evidence, revoke terminal run tokens, and return actionable feedback when delivery cannot retry through a paused assignee.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-08-16 20:05:28 +00:00
1 parent 2879b35607
commit 23258922e6
18 files changed
+534 -37

No files matched your search

+7
View File
@@ -599,6 +599,13 @@ Every issue PATCH emits an `issue.updated` activity receipt containing the
actor, responsible user, run, authorization reason, and field-level before/after
changes so both agent and board edits are visible in the issue activity stream.
Local-agent run JWTs are valid only while their exact persisted heartbeat run is
`running` and has no completion timestamp. The server revalidates that lifecycle
state on every authenticated request. Moving the run to any terminal status
revokes the run JWT immediately; there is no post-terminal grace window for
agent API reads or writes. Server-owned finalization and recovery bookkeeping
uses internal authority rather than the terminated run's credential.
## 9.4 Permission Terminology and Default Visibility Rule
Paperclip V1 keeps a company-scoped visibility model as the default because centralized authorization and scoped work-object controls are not yet a core V1 control surface.
+1
View File
@@ -1011,6 +1011,7 @@ export type {
IssueExecutionDecision,
IssueComment,
IssueCommentFeedbackDelivery,
IssueFeedbackDeliveryRetryNoInvokableAssigneeReason,
IssueFeedbackDeliveryRetryResponse,
IssueCommentDerivedAuthorSource,
IssueCommentMetadata,
+1
View File
@@ -650,6 +650,7 @@ export type {
IssueExecutionDecision,
IssueComment,
IssueCommentFeedbackDelivery,
IssueFeedbackDeliveryRetryNoInvokableAssigneeReason,
IssueFeedbackDeliveryRetryResponse,
IssueCommentDerivedAuthorSource,
IssueCommentMetadata,
+14
View File
@@ -986,8 +986,22 @@ export interface IssueCommentFeedbackDelivery {
batchedCommentCount: number;
}
export type IssueFeedbackDeliveryRetryNoInvokableAssigneeReason =
| "missing"
| "paused"
| "terminated"
| "pending_approval"
| "unknown_status"
| "manager_missing"
| "manager_company_mismatch"
| "manager_terminated"
| "reporting_cycle"
| "reporting_chain_too_deep";
export interface IssueFeedbackDeliveryRetryResponse {
outcome: "queued" | "already_queued" | "not_exhausted" | "no_invokable_assignee";
/** Present only when `outcome` is `no_invokable_assignee`. */
reason: IssueFeedbackDeliveryRetryNoInvokableAssigneeReason | null;
message: string;
feedbackDelivery: IssueCommentFeedbackDelivery | null;
}
@@ -33,7 +33,14 @@ function createSelectChain(rowsForTable: (table: unknown) => unknown[]) {
function createDbState(input: {
agent: { id: string; companyId: string; status?: string };
agentKey?: { id: string; agentId: string; companyId: string; keyHash: string; responsibleUserId?: string | null };
run?: { id: string; companyId: string; agentId: string; responsibleUserId?: string | null };
run?: {
id: string;
companyId: string;
agentId: string;
status?: string;
finishedAt?: Date | null;
responsibleUserId?: string | null;
};
}) {
const activity: Array<Record<string, unknown>> = [];
const agentRow = {
@@ -57,6 +64,8 @@ function createDbState(input: {
id: input.run.id,
companyId: input.run.companyId,
agentId: input.run.agentId,
status: input.run.status ?? "running",
finishedAt: input.run.finishedAt ?? null,
responsibleUserId: input.run.responsibleUserId ?? null,
}
: null;
@@ -286,6 +295,70 @@ describe("agent auth middleware", () => {
});
});
it.each(["succeeded", "failed", "cancelled", "timed_out", "interrupted"])(
"revokes the run JWT before reads or writes when the run is %s",
async (status) => {
const companyId = randomUUID();
const agentId = randomUUID();
const runId = randomUUID();
const issueId = randomUUID();
const { db } = createDbState({
agent: { id: agentId, companyId },
run: {
id: runId,
companyId,
agentId,
status,
finishedAt: new Date(),
responsibleUserId: "user-claim",
},
});
const token = createLocalAgentJwt(agentId, companyId, "codex_local", runId, "user-claim");
const app = createApp(db);
const readRes = await request(app)
.get(`/companies/${companyId}/issues/${issueId}`)
.set("Authorization", `Bearer ${token}`)
.set("X-Paperclip-Run-Id", runId);
const writeRes = await request(app)
.patch(`/companies/${companyId}/issues/${issueId}`)
.set("Authorization", `Bearer ${token}`)
.set("X-Paperclip-Run-Id", runId)
.send({ title: "should not write" });
expect(readRes.status).toBe(401);
expect(readRes.body.error).toBe("Agent run is not active");
expect(writeRes.status).toBe(401);
expect(writeRes.body.error).toBe("Agent run is not active");
},
);
it("fails closed when a running run already has a completion timestamp", async () => {
const companyId = randomUUID();
const agentId = randomUUID();
const runId = randomUUID();
const { db } = createDbState({
agent: { id: agentId, companyId },
run: {
id: runId,
companyId,
agentId,
status: "running",
finishedAt: new Date(),
responsibleUserId: "user-claim",
},
});
const token = createLocalAgentJwt(agentId, companyId, "codex_local", runId, "user-claim");
const res = await request(createApp(db))
.get(`/companies/${companyId}/protected`)
.set("Authorization", `Bearer ${token}`)
.set("X-Paperclip-Run-Id", runId);
expect(res.status).toBe(401);
expect(res.body.error).toBe("Agent run is not active");
});
it("rejects fork-minted run JWTs before issue reads or writes reach live issue data", async () => {
const companyId = randomUUID();
const agentId = randomUUID();
@@ -0,0 +1,168 @@
import { randomUUID } from "node:crypto";
import express from "express";
import request from "supertest";
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
import {
activityLog,
agents,
agentWakeupRequests,
companies,
createDb,
issueRecoveryActions,
issues,
} from "@paperclipai/db";
import {
getEmbeddedPostgresTestSupport,
startEmbeddedPostgresTestDatabase,
} from "./helpers/embedded-postgres.js";
import { errorHandler } from "../middleware/index.js";
import { issueRoutes } from "../routes/issues.js";
import {
FEEDBACK_DELIVERY_RECOVERY_ACTION_KIND,
FEEDBACK_DELIVERY_RECOVERY_CAUSE,
} from "../services/recovery/feedback-delivery.js";
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
if (!embeddedPostgresSupport.supported) {
console.warn(
`Skipping embedded Postgres feedback-delivery retry route tests on this host: ${
embeddedPostgresSupport.reason ?? "unsupported environment"
}`,
);
}
describeEmbeddedPostgres("issue feedback-delivery retry routes", () => {
let db!: ReturnType<typeof createDb>;
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
beforeAll(async () => {
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-feedback-delivery-retry-routes-");
db = createDb(tempDb.connectionString);
}, 20_000);
afterEach(async () => {
await db.delete(issueRecoveryActions);
await db.delete(activityLog);
await db.delete(issues);
await db.delete(agentWakeupRequests);
await db.delete(agents);
await db.delete(companies);
});
afterAll(async () => {
await tempDb?.cleanup();
});
function createApp(companyId: string) {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
req.actor = {
type: "board",
userId: "board-user",
companyIds: [companyId],
memberships: [{ companyId, membershipRole: "admin", status: "active" }],
isInstanceAdmin: false,
source: "session",
};
next();
});
app.use("/api", issueRoutes(db, {} as any));
app.use(errorHandler);
return app;
}
async function seedExhaustedDelivery(input: { agentStatus?: string; withAgent?: boolean } = {}) {
const companyId = randomUUID();
const issueId = randomUUID();
const agentId = input.withAgent === false ? null : randomUUID();
const rootWakeupRequestId = randomUUID();
const sourceCommentId = randomUUID();
const issuePrefix = `F${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
await db.insert(companies).values({
id: companyId,
name: "Paperclip",
issuePrefix,
requireBoardApprovalForNewAgents: false,
});
if (agentId) {
await db.insert(agents).values({
id: agentId,
companyId,
name: "CodexCoder",
role: "engineer",
status: input.agentStatus ?? "paused",
adapterType: "codex_local",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
});
}
await db.insert(issues).values({
id: issueId,
companyId,
title: "Feedback delivery exhausted",
status: "in_review",
priority: "medium",
assigneeAgentId: agentId,
issueNumber: 1,
identifier: `${issuePrefix}-1`,
});
await db.insert(issueRecoveryActions).values({
companyId,
sourceIssueId: issueId,
kind: FEEDBACK_DELIVERY_RECOVERY_ACTION_KIND,
status: "active",
ownerType: agentId ? "agent" : "board",
ownerAgentId: agentId,
previousOwnerAgentId: agentId,
returnOwnerAgentId: agentId,
cause: FEEDBACK_DELIVERY_RECOVERY_CAUSE,
fingerprint: `feedback_delivery:${companyId}:${issueId}:${agentId ?? "missing"}:${rootWakeupRequestId}`,
evidence: {
rootWakeupRequestId,
outstandingCommentIds: [sourceCommentId],
},
nextAction: "Deliver the outstanding feedback",
attemptCount: 1,
maxAttempts: 1,
});
return { companyId, issueId };
}
it("returns a paused reason instead of leaking the wakeup 409", async () => {
const { companyId, issueId } = await seedExhaustedDelivery({ agentStatus: "paused" });
const res = await request(createApp(companyId))
.post(`/api/issues/${issueId}/feedback-delivery/retry`)
.send({});
expect(res.status, JSON.stringify(res.body)).toBe(200);
expect(res.body).toMatchObject({
outcome: "no_invokable_assignee",
reason: "paused",
message: "This delivery's agent is paused. Unpause the agent before retrying.",
});
expect(await db.select().from(agentWakeupRequests)).toHaveLength(0);
expect(await db.select().from(activityLog)).toHaveLength(0);
});
it("distinguishes a missing assignee from a paused one", async () => {
const { companyId, issueId } = await seedExhaustedDelivery({ withAgent: false });
const res = await request(createApp(companyId))
.post(`/api/issues/${issueId}/feedback-delivery/retry`)
.send({});
expect(res.status, JSON.stringify(res.body)).toBe(200);
expect(res.body).toMatchObject({
outcome: "no_invokable_assignee",
reason: "missing",
message: "This delivery has no agent to retry against. Reassign the task first.",
});
});
});
@@ -66,6 +66,79 @@ function makeRecoveryActionRow(overrides: Record<string, unknown> = {}) {
}
describe("issueRecoveryActionService", () => {
it("redacts raw failure text from existing recovery-action rows", async () => {
const existingRow = makeRecoveryActionRow({
evidence: {
sourceRunId: "run-1",
lastFailureReason: "Error: ECONNREFUSED /home/agent/.secret/token.json",
},
});
const fakeDb = {
select: vi.fn(() => ({
from() {
return this;
},
where() {
return this;
},
orderBy() {
return this;
},
limit: vi.fn(async () => [existingRow]),
})),
};
const result = await issueRecoveryActionService(fakeDb as never).getActiveForIssue(
"company-1",
"source-1",
);
expect(result?.evidence).toEqual({ sourceRunId: "run-1" });
});
it("does not persist raw failure text in new recovery actions", async () => {
let insertedValues: Record<string, unknown> | null = null;
const fakeDb = {
select: vi.fn(() => ({
from() {
return this;
},
where() {
return this;
},
orderBy() {
return this;
},
limit: vi.fn(async () => []),
})),
insert: vi.fn(() => ({
values: vi.fn((values: Record<string, unknown>) => {
insertedValues = values;
return {
returning: vi.fn(async () => [makeRecoveryActionRow({ evidence: values.evidence })]),
};
}),
})),
};
const result = await issueRecoveryActionService(fakeDb as never).upsertSourceScoped({
companyId: "company-1",
sourceIssueId: "source-1",
kind: "missing_disposition",
ownerAgentId: "agent-1",
cause: "successful_run_missing_issue_disposition",
fingerprint: "missing-disposition:fingerprint",
evidence: {
sourceRunId: "run-1",
lastFailureReason: "Process lost -- child pid 3026905 is no longer running",
},
nextAction: "Choose a valid issue disposition.",
});
expect(insertedValues).toMatchObject({ evidence: { sourceRunId: "run-1" } });
expect(result.evidence).toEqual({ sourceRunId: "run-1" });
});
it("does not reactivate an action resolved between the active read and update", async () => {
const existingRow = makeRecoveryActionRow({ id: "existing-action", attemptCount: 1 });
const createdRow = makeRecoveryActionRow({ id: "new-action", attemptCount: 1 });
@@ -1287,10 +1360,30 @@ describeEmbeddedPostgres("issue recovery actions", () => {
ownerAgentId: managerId,
cause: "successful_run_missing_issue_disposition",
fingerprint: "missing-disposition:fingerprint",
evidence: { sourceRunId: "run-1" },
evidence: {
sourceRunId: "run-1",
lastFailureReason: "Error: ECONNREFUSED /home/agent/.secret/token.json",
},
nextAction: "Choose a valid issue disposition.",
wakePolicy: { type: "wake_owner" },
});
const [storedAction] = await db
.select({ evidence: issueRecoveryActions.evidence })
.from(issueRecoveryActions)
.where(eq(issueRecoveryActions.id, action.id));
expect(storedAction?.evidence).toEqual({ sourceRunId: "run-1" });
// Emulate an action persisted before storage sanitization shipped. Every
// recovery-action read model must still remove the raw failure text.
await db
.update(issueRecoveryActions)
.set({
evidence: {
sourceRunId: "run-1",
lastFailureReason: "Process lost -- child pid 3026905 is no longer running; retrying once",
},
})
.where(eq(issueRecoveryActions.id, action.id));
const app = createApp();
const detail = await request(app).get(`/api/issues/${sourceIssueId}`).expect(200);
@@ -1300,10 +1393,14 @@ describeEmbeddedPostgres("issue recovery actions", () => {
kind: "missing_disposition",
ownerAgentId: managerId,
});
expect(detail.body.activeRecoveryAction.evidence).toEqual({ sourceRunId: "run-1" });
expect(JSON.stringify(detail.body)).not.toContain("3026905");
const list = await request(app).get(`/api/issues/${sourceIssueId}/recovery-actions`).expect(200);
expect(list.body.active).toMatchObject({ id: action.id });
expect(list.body.actions).toHaveLength(1);
expect(list.body.active.evidence).toEqual({ sourceRunId: "run-1" });
expect(JSON.stringify(list.body)).not.toContain("3026905");
});
it("projects recovery action metadata into the structured wake payload", async () => {
+44 -26
View File
@@ -46,7 +46,7 @@ function pruneCloudTenantWriteDebounce(
}
import { instanceSettingsService } from "../services/instance-settings.js";
import { ensureHumanRoleDefaultGrants } from "../services/principal-access-compatibility.js";
import { forbidden, unprocessable } from "../errors.js";
import { forbidden, unauthorized, unprocessable } from "../errors.js";
export { isCloudManagedInstance } from "../services/cloud-instance.js";
@@ -58,13 +58,18 @@ function normalizeOptionalString(value: string | null | undefined) {
return value?.trim() || null;
}
async function resolveLegacyRunResponsibleUserId(
async function loadAgentJwtRun(
db: Db,
input: { companyId: string; agentId: string; runId: string },
) {
if (!isUuidLike(input.runId)) return null;
const run = await db
.select({ responsibleUserId: heartbeatRuns.responsibleUserId })
return db
.select({
id: heartbeatRuns.id,
status: heartbeatRuns.status,
finishedAt: heartbeatRuns.finishedAt,
responsibleUserId: heartbeatRuns.responsibleUserId,
})
.from(heartbeatRuns)
.where(
and(
@@ -74,7 +79,6 @@ async function resolveLegacyRunResponsibleUserId(
),
)
.then((rows) => rows[0] ?? null);
return normalizeOptionalString(run?.responsibleUserId);
}
async function loadResponsibleUserMemberships(
@@ -301,22 +305,6 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa
return;
}
const agentRecord = await db
.select()
.from(agents)
.where(eq(agents.id, claims.sub))
.then((rows) => rows[0] ?? null);
if (!agentRecord || agentRecord.companyId !== claims.company_id) {
next();
return;
}
if (agentRecord.status === "terminated" || agentRecord.status === "pending_approval") {
next();
return;
}
const normalizedRunIdHeader = normalizeOptionalString(runIdHeader);
if (normalizedRunIdHeader && normalizedRunIdHeader !== claims.run_id) {
await auditAgentJwtRunHeaderMismatch(db, {
@@ -337,13 +325,43 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa
return;
}
const [agentRecord, jwtRun] = await Promise.all([
db
.select()
.from(agents)
.where(eq(agents.id, claims.sub))
.then((rows) => rows[0] ?? null),
loadAgentJwtRun(db, {
companyId: claims.company_id,
agentId: claims.sub,
runId: claims.run_id,
}),
]);
if (!agentRecord || agentRecord.companyId !== claims.company_id) {
next();
return;
}
if (agentRecord.status === "terminated" || agentRecord.status === "pending_approval") {
next();
return;
}
// A local-agent JWT is a capability for exactly one live heartbeat run,
// not a long-lived agent credential. Revalidate the persisted lifecycle
// on every request so terminalizing the run revokes the capability
// immediately instead of leaving it usable until the JWT's expiry.
// `finishedAt` is checked as a fail-closed guard for an inconsistent row
// whose status was not updated atomically with finalization.
if (!jwtRun || jwtRun.status !== "running" || jwtRun.finishedAt !== null) {
next(unauthorized("Agent run is not active"));
return;
}
const onBehalfOfUserId = claims.responsible_user_id !== undefined
? normalizeOptionalString(claims.responsible_user_id)
: await resolveLegacyRunResponsibleUserId(db, {
companyId: claims.company_id,
agentId: claims.sub,
runId: claims.run_id,
});
: normalizeOptionalString(jwtRun.responsibleUserId);
const onBehalfOfMemberships = await loadResponsibleUserMemberships(db, {
companyId: claims.company_id,
userId: onBehalfOfUserId,
+32
View File
@@ -113,6 +113,7 @@ import {
FEEDBACK_DELIVERY_RETRY_WAKE_REASON,
FEEDBACK_DELIVERY_WAKE_COMMENT_IDS_KEY,
} from "../services/recovery/feedback-delivery.js";
import { evaluateAgentInvokabilityFromDb } from "../services/agent-invokability.js";
import { getTelemetryClient } from "../telemetry.js";
import { isUniqueViolation } from "../db-errors.js";
import type { StorageService } from "../storage/types.js";
@@ -10826,6 +10827,7 @@ export function issueRoutes(
const states = await readStates();
res.json({
outcome: "not_exhausted",
reason: null,
message: "This task has no exhausted feedback delivery to retry.",
feedbackDelivery: states.values().next().value ?? null,
} satisfies IssueFeedbackDeliveryRetryResponse);
@@ -10849,12 +10851,40 @@ export function issueRoutes(
if (!targetAgentId || !rootWakeupRequestId) {
res.json({
outcome: "no_invokable_assignee",
reason: "missing",
message: "This delivery has no agent to retry against. Reassign the task first.",
feedbackDelivery: (await readStates()).get(outstandingCommentIds[0] ?? "") ?? null,
} satisfies IssueFeedbackDeliveryRetryResponse);
return;
}
const targetAgent = await db
.select({
id: agents.id,
companyId: agents.companyId,
name: agents.name,
reportsTo: agents.reportsTo,
status: agents.status,
})
.from(agents)
.where(and(eq(agents.id, targetAgentId), eq(agents.companyId, issue.companyId)))
.then((rows) => rows[0] ?? null);
const invokability = await evaluateAgentInvokabilityFromDb(db, targetAgent);
if (!invokability.invokable) {
const message = invokability.reason === "paused"
? "This delivery's agent is paused. Unpause the agent before retrying."
: invokability.reason === "pending_approval"
? "This delivery's agent is waiting for approval. Approve or reassign the agent before retrying."
: "This delivery has no invokable agent to retry against. Reassign the task first.";
res.json({
outcome: "no_invokable_assignee",
reason: invokability.reason,
message,
feedbackDelivery: (await readStates()).get(outstandingCommentIds[0] ?? "") ?? null,
} satisfies IssueFeedbackDeliveryRetryResponse);
return;
}
const statesBefore = await readStates();
const alreadyInFlight = outstandingCommentIds.some(
(commentId) => statesBefore.get(commentId)?.retryInFlight === true,
@@ -10862,6 +10892,7 @@ export function issueRoutes(
if (alreadyInFlight) {
res.json({
outcome: "already_queued",
reason: null,
message: "A delivery retry is already queued.",
feedbackDelivery: statesBefore.get(outstandingCommentIds[0] ?? "") ?? null,
} satisfies IssueFeedbackDeliveryRetryResponse);
@@ -10919,6 +10950,7 @@ export function issueRoutes(
const projected = latestCommentId ? statesAfter.get(latestCommentId) ?? null : null;
res.json({
outcome: "queued",
reason: null,
message: "Retrying feedback delivery.",
// Deferred behind a live run still reads as in-flight to the operator, so
// fall back to a synthetic in-flight projection rather than reporting a
-1
View File
@@ -10340,7 +10340,6 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
outstandingCommentIds: input.delivery.commentIds,
retryGeneration: input.delivery.generation,
lastFailureCode: input.run.errorCode,
lastFailureReason: input.run.error,
gateErrorCode: input.gateErrorCode ?? null,
gateReason: input.gateReason ?? null,
},
+14 -3
View File
@@ -16,6 +16,17 @@ type IssueRecoveryActionRow = typeof issueRecoveryActions.$inferSelect;
type DbTransaction = Parameters<Parameters<Db["transaction"]>[0]>[0];
type DbOrTransaction = Db | DbTransaction;
/**
* Recovery evidence is returned on company-visible issue APIs, so raw adapter
* failure text must never cross this service boundary. Keep the exact denylist
* here as defense in depth for rows written before the producer was fixed.
*/
function sanitizeEvidence(evidence: Record<string, unknown>): Record<string, unknown> {
const sanitized = { ...evidence };
delete sanitized.lastFailureReason;
return sanitized;
}
export type UpsertIssueRecoveryActionInput = {
companyId: string;
sourceIssueId: string;
@@ -64,7 +75,7 @@ function toReadModel(row: IssueRecoveryActionRow): IssueRecoveryAction {
returnOwnerAgentId: row.returnOwnerAgentId,
cause: row.cause,
fingerprint: row.fingerprint,
evidence: row.evidence,
evidence: sanitizeEvidence(row.evidence),
nextAction: row.nextAction,
wakePolicy: row.wakePolicy,
monitorPolicy: row.monitorPolicy,
@@ -195,7 +206,7 @@ export function issueRecoveryActionService(db: Db) {
returnOwnerAgentId: input.returnOwnerAgentId ?? existing.returnOwnerAgentId,
cause: input.cause,
fingerprint: input.fingerprint,
evidence: input.evidence ?? existing.evidence,
evidence: sanitizeEvidence(input.evidence ?? existing.evidence),
nextAction: input.nextAction,
wakePolicy: input.wakePolicy ?? null,
monitorPolicy: input.monitorPolicy ?? null,
@@ -237,7 +248,7 @@ export function issueRecoveryActionService(db: Db) {
returnOwnerAgentId: input.returnOwnerAgentId ?? null,
cause: input.cause,
fingerprint: input.fingerprint,
evidence: input.evidence ?? {},
evidence: sanitizeEvidence(input.evidence ?? {}),
nextAction: input.nextAction,
wakePolicy: input.wakePolicy ?? null,
monitorPolicy: input.monitorPolicy ?? null,
@@ -13,6 +13,7 @@ import {
FeedbackDeliveryFooterStatus,
FeedbackDeliveryNotice,
feedbackDeliveryAnchorId,
feedbackDeliveryRetryErrorMessage,
isIssueCommentFeedbackDelivery,
useFeedbackDeliveryTransientSuccess,
} from "./FeedbackDeliveryState";
@@ -318,6 +319,30 @@ describe("FeedbackDeliveryNotice — exhausted", () => {
).toBe(false);
});
it("shows an actionable paused-agent retry failure inline", () => {
const message = feedbackDeliveryRetryErrorMessage({
outcome: "no_invokable_assignee",
reason: "paused",
message: "server fallback",
feedbackDelivery: delivery(),
});
const node = render(
<FeedbackDeliveryNotice delivery={delivery()} onRetry={() => {}} retryError={message} />,
);
expect(node.querySelector("[data-testid='feedback-delivery-retry-error']")?.textContent).toBe(
"The assigned agent is paused. Unpause the agent before retrying.",
);
});
it("keeps missing-assignee retry guidance distinct from paused guidance", () => {
expect(feedbackDeliveryRetryErrorMessage({
outcome: "no_invokable_assignee",
reason: "missing",
message: "This delivery has no agent to retry against. Reassign the task first.",
feedbackDelivery: delivery(),
})).toBe("This delivery has no agent to retry against. Reassign the task first.");
});
it("hides retry from a viewer without the authority to trigger it", () => {
const node = render(<FeedbackDeliveryNotice delivery={delivery({ canRetry: false })} />);
expect(node.querySelector("[data-testid='feedback-delivery-retry']")).toBeNull();
+22 -4
View File
@@ -1,7 +1,10 @@
import { useEffect, useRef, useState } from "react";
import { Link } from "react-router-dom";
import { CircleCheck, RefreshCw } from "lucide-react";
import type { IssueCommentFeedbackDelivery } from "@paperclipai/shared";
import type {
IssueCommentFeedbackDelivery,
IssueFeedbackDeliveryRetryResponse,
} from "@paperclipai/shared";
import { InlineBanner } from "@/components/InlineBanner";
import { SystemNotice } from "@/components/SystemNotice";
@@ -19,6 +22,19 @@ export const FEEDBACK_DELIVERY_RETRYING_REVEAL_MS = 400;
/** How long the ephemeral "Delivered after retry" confirmation stays visible. */
export const FEEDBACK_DELIVERY_TRANSIENT_SUCCESS_MS = 5000;
export function feedbackDeliveryRetryErrorMessage(
response: IssueFeedbackDeliveryRetryResponse,
) {
if (response.outcome !== "no_invokable_assignee") return response.message;
if (response.reason === "paused") {
return "The assigned agent is paused. Unpause the agent before retrying.";
}
if (response.reason === "pending_approval") {
return "The assigned agent is waiting for approval. Approve or reassign it before retrying.";
}
return response.message;
}
const FEEDBACK_DELIVERY_STATES = [
"retrying",
"exhausted_needs_attention",
@@ -232,13 +248,13 @@ export function FeedbackDeliveryNotice({
delivery,
onRetry,
retryPending = false,
retryError = false,
retryError = null,
className,
}: {
delivery: IssueCommentFeedbackDelivery;
onRetry?: (commentId: string) => void;
retryPending?: boolean;
retryError?: boolean;
retryError?: string | boolean | null;
className?: string;
}) {
const anchorId = feedbackDeliveryAnchorId(delivery.sourceCommentId);
@@ -349,7 +365,9 @@ export function FeedbackDeliveryNotice({
</p>
{retryError ? (
<p data-testid="feedback-delivery-retry-error" className="font-medium">
Couldn&apos;t start another retry. Try again.
{typeof retryError === "string"
? retryError
: "Couldn't start another retry. Try again."}
</p>
) : null}
</div>
+11 -1
View File
@@ -222,6 +222,7 @@ interface IssueChatMessageContext {
retryingFeedbackDeliveryCommentId?: string | null;
/** Comment id whose last retry request failed, so the banner can say so inline. */
failedFeedbackDeliveryRetryCommentId?: string | null;
feedbackDeliveryRetryErrorMessage?: string | null;
onImageClick?: (src: string) => void;
onAcceptInteraction?: (
interaction:
@@ -547,6 +548,7 @@ interface IssueChatThreadProps {
onRetryFeedbackDelivery?: (commentId: string) => Promise<void> | void;
retryingFeedbackDeliveryCommentId?: string | null;
failedFeedbackDeliveryRetryCommentId?: string | null;
feedbackDeliveryRetryErrorMessage?: string | null;
onDeleteComment?: (commentId: string) => Promise<void> | void;
interruptingQueuedRunId?: string | null;
stoppingRunId?: string | null;
@@ -1479,6 +1481,7 @@ function IssueChatUserMessage({
onRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
currentUserId,
userProfileMap,
} = useContext(IssueChatCtx);
@@ -1687,7 +1690,11 @@ function IssueChatUserMessage({
delivery={feedbackDelivery}
onRetry={onRetryFeedbackDelivery}
retryPending={retryingFeedbackDeliveryCommentId === feedbackDelivery.sourceCommentId}
retryError={failedFeedbackDeliveryRetryCommentId === feedbackDelivery.sourceCommentId}
retryError={
failedFeedbackDeliveryRetryCommentId === feedbackDelivery.sourceCommentId
? feedbackDeliveryRetryErrorMessage ?? true
: false
}
/>
) : null}
</div>
@@ -4440,6 +4447,7 @@ export function IssueChatThread({
onRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId = null,
failedFeedbackDeliveryRetryCommentId = null,
feedbackDeliveryRetryErrorMessage = null,
interruptingQueuedRunId = null,
stoppingRunId = null,
onImageClick,
@@ -5021,6 +5029,7 @@ export function IssueChatThread({
onRetryFeedbackDelivery: stableOnRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
onImageClick: stableOnImageClick,
onAcceptInteraction: stableOnAcceptInteraction,
onRejectInteraction: stableOnRejectInteraction,
@@ -5053,6 +5062,7 @@ export function IssueChatThread({
stableOnRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
stableOnImageClick,
stableOnAcceptInteraction,
stableOnRejectInteraction,
+2
View File
@@ -139,6 +139,7 @@ export function TaskChatThread(props: TaskChatThreadProps) {
onRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
} = props;
const liveWorkLinks = useMemo(
@@ -687,6 +688,7 @@ export function TaskChatThread(props: TaskChatThreadProps) {
onRetryFeedbackDelivery={onRetryFeedbackDelivery}
retryingFeedbackDeliveryCommentId={retryingFeedbackDeliveryCommentId}
failedFeedbackDeliveryRetryCommentId={failedFeedbackDeliveryRetryCommentId}
feedbackDeliveryRetryErrorMessage={feedbackDeliveryRetryErrorMessage}
tail={tailRunId || bottomBlockerLinks ? (
<>
{tailRunId ? (
@@ -44,6 +44,7 @@ interface TaskChatBubbleProps {
onRetryFeedbackDelivery?: (commentId: string) => Promise<void> | void;
retryingFeedbackDeliveryCommentId?: string | null;
failedFeedbackDeliveryRetryCommentId?: string | null;
feedbackDeliveryRetryErrorMessage?: string | null;
}
function initialsForName(name: string) {
@@ -79,6 +80,7 @@ export function TaskChatBubble({
onRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
}: TaskChatBubbleProps) {
// Clicking an embedded image opens the full-screen lightbox (with download);
// arrow keys walk across the other images in the same bubble.
@@ -235,6 +237,8 @@ export function TaskChatBubble({
}
retryError={
failedFeedbackDeliveryRetryCommentId === item.feedbackDelivery.sourceCommentId
? feedbackDeliveryRetryErrorMessage ?? true
: false
}
/>
) : null}
@@ -46,6 +46,7 @@ interface TaskChatThreadViewProps {
onRetryFeedbackDelivery?: (commentId: string) => Promise<void> | void;
retryingFeedbackDeliveryCommentId?: string | null;
failedFeedbackDeliveryRetryCommentId?: string | null;
feedbackDeliveryRetryErrorMessage?: string | null;
/** Content appended inside the transcript scroller after the settled thread. */
tail?: ReactNode;
/** Optional streaming-aware key when `tail` changes without changing `items`. */
@@ -65,6 +66,7 @@ function renderItem(
onRetryFeedbackDelivery?: (commentId: string) => Promise<void> | void,
retryingFeedbackDeliveryCommentId?: string | null,
failedFeedbackDeliveryRetryCommentId?: string | null,
feedbackDeliveryRetryErrorMessage?: string | null,
) {
switch (item.kind) {
case "message": {
@@ -82,6 +84,7 @@ function renderItem(
onRetryFeedbackDelivery={onRetryFeedbackDelivery}
retryingFeedbackDeliveryCommentId={retryingFeedbackDeliveryCommentId}
failedFeedbackDeliveryRetryCommentId={failedFeedbackDeliveryRetryCommentId}
feedbackDeliveryRetryErrorMessage={feedbackDeliveryRetryErrorMessage}
attachedTurn={
item.attachedTurn ? (
<TaskChatTurn
@@ -152,6 +155,7 @@ export function TaskChatThreadView({
onRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
tail,
contentKey,
className,
@@ -176,6 +180,7 @@ export function TaskChatThreadView({
onRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
)}
</div>
))}
+12
View File
@@ -100,6 +100,7 @@ import {
type IssueChatRunFinalizationAction,
} from "../components/IssueChatThread";
import { TaskChatThread } from "../components/TaskChatThread";
import { feedbackDeliveryRetryErrorMessage } from "../components/FeedbackDeliveryState";
import type { TaskChatIssueBrief } from "../components/task-chat/TaskChatDescriptionBubble";
import { useClassicTaskInterfaceEnabled } from "../hooks/useClassicTaskInterfaceEnabled";
import { workModeMetaFor } from "../lib/work-mode-meta";
@@ -962,6 +963,7 @@ type IssueDetailChatTabProps = {
onRetryFeedbackDelivery?: (commentId: string) => void;
retryingFeedbackDeliveryCommentId?: string | null;
failedFeedbackDeliveryRetryCommentId?: string | null;
feedbackDeliveryRetryErrorMessage?: string | null;
onResolveRecoveryAction?: (outcome: import("../components/IssueRecoveryActionCard").RecoveryResolveOutcome) => void;
onReissueIsolatedRecoveryAction?: (request: import("../components/IssueRecoveryActionCard").RecoveryReissueRequest) => void;
reissueIsolatedRecoveryActionPending?: boolean;
@@ -1070,6 +1072,7 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({
onRetryFeedbackDelivery,
retryingFeedbackDeliveryCommentId,
failedFeedbackDeliveryRetryCommentId,
feedbackDeliveryRetryErrorMessage,
recoveryAction,
onResolveRecoveryAction,
onReissueIsolatedRecoveryAction,
@@ -1349,6 +1352,7 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({
onRetryFeedbackDelivery={onRetryFeedbackDelivery}
retryingFeedbackDeliveryCommentId={retryingFeedbackDeliveryCommentId ?? null}
failedFeedbackDeliveryRetryCommentId={failedFeedbackDeliveryRetryCommentId ?? null}
feedbackDeliveryRetryErrorMessage={feedbackDeliveryRetryErrorMessage ?? null}
onResolveRecoveryAction={onResolveRecoveryAction}
onReissueIsolatedRecoveryAction={onReissueIsolatedRecoveryAction}
reissueIsolatedRecoveryActionPending={reissueIsolatedRecoveryActionPending}
@@ -4196,6 +4200,8 @@ export function IssueDetail() {
useState<string | null>(null);
const [retryingFeedbackDeliveryCommentId, setRetryingFeedbackDeliveryCommentId] =
useState<string | null>(null);
const [feedbackDeliveryRetryFailureMessage, setFeedbackDeliveryRetryFailureMessage] =
useState<string | null>(null);
const retryFeedbackDelivery = useMutation({
// The commentId is carried so the banner that was pressed owns the pending
// and error state; the request itself is scoped to the issue's one open
@@ -4204,6 +4210,7 @@ export function IssueDetail() {
onMutate: (input) => {
setRetryingFeedbackDeliveryCommentId(input.commentId);
setFailedFeedbackDeliveryRetryCommentId(null);
setFeedbackDeliveryRetryFailureMessage(null);
},
onSuccess: (result, variables) => {
// The delivery state lives on the comment, so the thread is the cache that
@@ -4212,10 +4219,14 @@ export function IssueDetail() {
invalidateIssueDetail();
if (result.outcome === "queued" || result.outcome === "already_queued") return;
setFailedFeedbackDeliveryRetryCommentId(variables.commentId);
setFeedbackDeliveryRetryFailureMessage(feedbackDeliveryRetryErrorMessage(result));
pushToast({ title: "Couldn't retry delivery", body: result.message, tone: "error" });
},
onError: (err, variables) => {
setFailedFeedbackDeliveryRetryCommentId(variables.commentId);
setFeedbackDeliveryRetryFailureMessage(
err instanceof Error ? err.message : "Unable to retry feedback delivery.",
);
pushToast({
title: "Couldn't retry delivery",
body: err instanceof Error ? err.message : "Unable to retry feedback delivery.",
@@ -5263,6 +5274,7 @@ export function IssueDetail() {
onRetryFeedbackDelivery={handleRetryFeedbackDelivery}
retryingFeedbackDeliveryCommentId={retryingFeedbackDeliveryCommentId}
failedFeedbackDeliveryRetryCommentId={failedFeedbackDeliveryRetryCommentId}
feedbackDeliveryRetryErrorMessage={feedbackDeliveryRetryFailureMessage}
onResolveRecoveryAction={handleResolveRecoveryAction}
onReissueIsolatedRecoveryAction={handleReissueIsolatedRecoveryAction}
reissueIsolatedRecoveryActionPending={reissueIsolatedRecoveryAction.isPending}