mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
feat: structure and humanize recovery notices (#11075)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip posts system comments when automatic run recovery cannot continue > - These comments currently mix the main event with recovery identifiers and routing details > - The task chat shell also renders these comments as large raw text blocks > - Operators need a short explanation first and inspectable evidence on demand > - This pull request emits structured recovery notices and renders them as compact humanized rows > - The benefit is a quieter task thread that keeps the full recovery evidence available ## Linked Issues or Issue Description Related prior extraction source: #11070. This pull request replaces only its structured recovery notice slice with a focused branch based on current master. **What existing behavior does this improve?** Paperclip recovery escalations and the experimental task chat system-comment renderer. **Current behavior** Recovery escalation comments put action identifiers, owner details, run details, and failure codes into the visible markdown body. The task chat shell renders the complete system comment as a large text block. **Proposed behavior** The server emits a short system notice with typed metadata sections. The task chat shell classifies known recovery families and renders one compact row. An operator can expand the row to inspect the full body and metadata. **Reason and benefit** The main thread stays readable during repeated recovery activity. Typed links and evidence remain available without exposing raw failure text in the default view. **Breaking changes** The visible recovery comment body is shorter. Recovery action deduplication now reads the structured metadata and still recognizes legacy body markers. No API schema or database migration changes. ## What Changed - Emit stranded recovery escalations with `system_notice` presentation and typed recovery, owner, run, and failure-code metadata. - Share bounded metadata row builders across recovery notice producers and preserve legacy deduplication compatibility. - Humanize known recovery notice families and render compact expandable task-chat rows. - Route system-authored comments ahead of derived agent authorship so recovery notices do not appear as agent bubbles. - Add focused server and UI regression coverage. ## Verification - `pnpm check:token-gates` — 3/3 clean. - `pnpm --filter @paperclipai/server typecheck` — passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm --filter @paperclipai/shared typecheck` — passed. - `pnpm --filter @paperclipai/shared exec vitest run src/validators/issue.test.ts` — 32 tests passed. - `pnpm --filter @paperclipai/server exec vitest run src/services/recovery/stranded-notice.test.ts src/__tests__/issue-recovery-actions.test.ts` — 57 tests passed. - `pnpm --filter @paperclipai/server exec vitest run src/services/recovery/successful-run-handoff.test.ts src/services/recovery/stranded-notice.test.ts` — 39 tests passed. - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-process-recovery.test.ts -t 'escalates an exhausted failed successful-run handoff without using generic continuation recovery first|escalates an exhausted successful handoff run that still leaves no disposition'` — 2 tests passed. - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-process-recovery.test.ts -t 'blocks assigned todo work after the one automatic dispatch recovery was already used'` — passed. - `pnpm --filter @paperclipai/ui exec vitest run src/lib/system-notice-humanizer.test.ts src/components/task-chat/TaskChatSystemNotice.test.tsx src/components/task-chat/task-chat-adapter.test.ts` — 15 tests passed. - Storybook visual baselines were not updated because this chat-shell path has no affected snapshot baseline. Focused rendering tests and token gates cover this change. ## Risks - Consumers that parse recovery action identifiers from comment markdown must move to structured metadata. Server deduplication remains backward compatible with legacy comments. - The humanizer uses stable recovery-family phrases. Unknown notices use a generic truncated first-sentence fallback. - The UI changes only the experimental task chat presentation. The stored comment body and expanded metadata remain available. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex with GPT-5, reasoning mode, repository tools, shell execution, and GitHub integration. The runtime did not expose a context-window size. ## 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
4e9a78db58
commit
cc35c3c395
25 files changed
+1166
-308
No files matched your search
@@ -116,6 +116,7 @@ import {
|
||||
SUCCESSFUL_RUN_HANDOFF_EXHAUSTED_NOTICE_BODY,
|
||||
SUCCESSFUL_RUN_HANDOFF_REQUIRED_NOTICE_BODY,
|
||||
SUCCESSFUL_RUN_MISSING_STATE_REASON,
|
||||
noticeMetadataReferencesRecoveryAction,
|
||||
} from "../services/recovery/index.ts";
|
||||
import {
|
||||
UNMANAGED_BACKGROUND_TASK_LIVENESS_REASON,
|
||||
@@ -130,6 +131,11 @@ if (!embeddedPostgresSupport.supported) {
|
||||
);
|
||||
}
|
||||
|
||||
function commentMetadataRows(comment: { metadata?: unknown } | null | undefined) {
|
||||
const metadata = comment?.metadata as { sections?: Array<{ rows?: unknown[] }> } | null | undefined;
|
||||
return (metadata?.sections ?? []).flatMap((section) => section.rows ?? []) as Array<Record<string, unknown>>;
|
||||
}
|
||||
|
||||
function spawnAliveProcess() {
|
||||
return spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], {
|
||||
stdio: "ignore",
|
||||
@@ -2489,8 +2495,11 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
});
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.body).toContain("retried continuation");
|
||||
expect(comments[0]?.body).toContain(`Recovery action: \`${recoveryAction.id}\``);
|
||||
expect(comments[0]?.body).toContain("Recovery owner: [CodexCoder]");
|
||||
expect(comments[0]?.presentation).toMatchObject({ kind: "system_notice", tone: "danger" });
|
||||
expect(noticeMetadataReferencesRecoveryAction(comments[0]?.metadata, recoveryAction.id)).toBe(true);
|
||||
expect(commentMetadataRows(comments[0]).some((row) =>
|
||||
row.type === "agent_link" && row.label === "Recovery owner" && row.name === "CodexCoder",
|
||||
)).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks failed recovery work in place during immediate terminal-run cleanup", async () => {
|
||||
@@ -2565,6 +2574,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
expect(comments[0]?.body).toContain("recovery issues do not create nested `stranded_issue_recovery` issues");
|
||||
expect(comments[0]?.body).toContain("Latest retry failure details were withheld from the issue thread");
|
||||
expect(comments[0]?.body).not.toContain("sk-test-recovery-secret");
|
||||
expect(JSON.stringify(comments[0]?.metadata)).not.toContain("sk-test-recovery-secret");
|
||||
expect(comments[0]?.presentation).toMatchObject({
|
||||
kind: "system_notice",
|
||||
tone: "warning",
|
||||
@@ -2944,7 +2954,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
authorType: "system",
|
||||
body: expect.stringContaining("Agent failed to resume after approval: `adapter_failed` — needs attention"),
|
||||
});
|
||||
expect(comments[0]?.body).toContain("Recovery action:");
|
||||
expect(commentMetadataRows(comments[0]).some((row) => row.label === "Recovery action")).toBe(true);
|
||||
|
||||
const interaction = await db
|
||||
.select({ result: issueThreadInteractions.result })
|
||||
@@ -4697,7 +4707,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, issueId));
|
||||
const recoveryComment = comments.find((comment) =>
|
||||
comment.body.includes("pending execution-review participant once") &&
|
||||
comment.body.includes(`Recovery action: \`${recoveryAction.id}\``),
|
||||
noticeMetadataReferencesRecoveryAction(comment.metadata, recoveryAction.id),
|
||||
);
|
||||
expect(recoveryComment).toBeTruthy();
|
||||
|
||||
@@ -5511,25 +5521,18 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, issueId));
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.body).toContain("retried dispatch");
|
||||
expect(comments[0]?.body).toContain("Latest retry failure details were withheld from the issue thread");
|
||||
expect(comments[0]?.body).toContain(`Recovery action: \`${recoveryAction.id}\``);
|
||||
expect(comments[0]?.body).toContain(`Recovery owner: [${longRecoveryOwnerName}]`);
|
||||
expect(comments[0]?.presentation).toMatchObject({
|
||||
kind: "system_notice",
|
||||
tone: "warning",
|
||||
title: `${`Recovery: retries exhausted — moved to blocked (owner: ${longRecoveryOwnerName})`.slice(0, 159)}…`,
|
||||
density: "compact",
|
||||
});
|
||||
expect(comments[0]?.metadata).toMatchObject({
|
||||
version: 1,
|
||||
sections: [expect.objectContaining({
|
||||
rows: expect.arrayContaining([
|
||||
expect.objectContaining({ type: "key_value", label: "Recovery action", value: recoveryAction.id }),
|
||||
expect.objectContaining({ type: "key_value", label: "Cause", value: "process_lost" }),
|
||||
expect.objectContaining({ type: "agent_link", label: "Recovery owner", name: "R".repeat(160) }),
|
||||
]),
|
||||
})],
|
||||
});
|
||||
expect(comments[0]?.body).not.toContain("sk-test-recovery-secret");
|
||||
expect(JSON.stringify(comments[0]?.metadata)).not.toContain("sk-test-recovery-secret");
|
||||
const failureSummary = commentMetadataRows(comments[0]).find((row) =>
|
||||
row.type === "key_value" && row.label === "Failure summary"
|
||||
);
|
||||
expect(failureSummary).toMatchObject({ type: "key_value", label: "Failure summary" });
|
||||
expect(failureSummary?.type === "key_value" ? failureSummary.value : "").toContain("Authorization");
|
||||
expect(comments[0]?.presentation).toMatchObject({ kind: "system_notice", tone: "danger" });
|
||||
expect(noticeMetadataReferencesRecoveryAction(comments[0]?.metadata, recoveryAction.id)).toBe(true);
|
||||
expect(commentMetadataRows(comments[0]).some((row) =>
|
||||
row.type === "agent_link" && row.label === "Recovery owner" && row.name === longRecoveryOwnerName.slice(0, 160),
|
||||
)).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks an already stranded recovery issue without creating a recovery child", async () => {
|
||||
@@ -5933,9 +5936,11 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, issueId));
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.body).toContain("retried continuation");
|
||||
expect(comments[0]?.body).toContain("Latest retry failure details were withheld from the issue thread");
|
||||
expect(comments[0]?.body).toContain(`Recovery action: \`${recoveryAction.id}\``);
|
||||
expect(comments[0]?.body).toContain("Recovery owner: [CodexCoder]");
|
||||
expect(comments[0]?.presentation).toMatchObject({ kind: "system_notice", tone: "danger" });
|
||||
expect(noticeMetadataReferencesRecoveryAction(comments[0]?.metadata, recoveryAction.id)).toBe(true);
|
||||
expect(commentMetadataRows(comments[0]).some((row) =>
|
||||
row.type === "agent_link" && row.label === "Recovery owner" && row.name === "CodexCoder",
|
||||
)).toBe(true);
|
||||
});
|
||||
|
||||
it("redacts error-code-only stranded recovery failures in issue copy", async () => {
|
||||
@@ -5966,8 +5971,15 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
|
||||
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, issueId));
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.body).toContain("Latest retry failure details were withheld from the issue thread");
|
||||
// The short structured body carries no failure details; the normalized
|
||||
// failure code surfaces only as a metadata row.
|
||||
expect(comments[0]?.body).not.toContain("adapter_exit_code");
|
||||
expect(comments[0]?.body).not.toContain("- Failure: none recorded");
|
||||
expect(commentMetadataRows(comments[0])).toContainEqual({
|
||||
type: "key_value",
|
||||
label: "Failure code",
|
||||
value: "adapter_exit_code",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps retrying transient adapter_failed continuation runs before the cap", async () => {
|
||||
@@ -6060,7 +6072,11 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.body).toContain("retried continuation");
|
||||
expect(comments[0]?.body).toContain("3× attempts");
|
||||
expect(comments[0]?.body).toContain("Latest cause: `adapter_failed`");
|
||||
expect(commentMetadataRows(comments[0])).toContainEqual({
|
||||
type: "key_value",
|
||||
label: "Failure code",
|
||||
value: "adapter_failed",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not count mixed-cause continuation failures toward the transient cap", async () => {
|
||||
@@ -6687,8 +6703,10 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.body).toContain("automatically retried continuation");
|
||||
expect(comments[0]?.body).toContain("still has no live execution path");
|
||||
expect(comments[0]?.body).toContain(`Recovery action: \`${recoveryAction.id}\``);
|
||||
expect(comments[0]?.body).toContain("Recovery owner: [CodexCoder]");
|
||||
expect(noticeMetadataReferencesRecoveryAction(comments[0]?.metadata, recoveryAction.id)).toBe(true);
|
||||
expect(commentMetadataRows(comments[0]).some((row) =>
|
||||
row.type === "agent_link" && row.label === "Recovery owner" && row.name === "CodexCoder",
|
||||
)).toBe(true);
|
||||
});
|
||||
|
||||
it("allows one productive-terminal recovery after regular continuation recovery made progress", async () => {
|
||||
|
||||
@@ -37,6 +37,7 @@ import {
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { drainHeartbeatRunsToQuiescence } from "./helpers/drain-heartbeat-runs.js";
|
||||
import { heartbeatService } from "../services/heartbeat.ts";
|
||||
import { noticeMetadataReferencesRecoveryAction } from "../services/recovery/index.ts";
|
||||
import { instanceSettingsService } from "../services/instance-settings.ts";
|
||||
import {
|
||||
WORKSPACE_WORKTREE_REQUIRES_PROJECT_CODE,
|
||||
@@ -221,7 +222,7 @@ async function waitForContainmentSideEffects(input: {
|
||||
const hasRecoveryActionComment = recoveryActionId
|
||||
? comments.some((comment) =>
|
||||
comment.issueId === input.sourceIssueId &&
|
||||
comment.body.includes(`Recovery action: \`${recoveryActionId}\``))
|
||||
noticeMetadataReferencesRecoveryAction(comment.metadata, recoveryActionId))
|
||||
: false;
|
||||
if (
|
||||
source?.status === "blocked" &&
|
||||
@@ -685,7 +686,10 @@ async function expectContainedWorkspaceBranchFailure(input: {
|
||||
}),
|
||||
});
|
||||
|
||||
expect(comments.filter((comment) => comment.issueId === input.sourceIssueId && comment.body.includes(`Recovery action: \`${action.id}\``))).toHaveLength(1);
|
||||
expect(comments.filter((comment) =>
|
||||
comment.issueId === input.sourceIssueId &&
|
||||
noticeMetadataReferencesRecoveryAction(comment.metadata, action.id),
|
||||
)).toHaveLength(1);
|
||||
expect(comments.filter((comment) => comment.issueId === input.sameWorkspaceSiblingId)).toHaveLength(0);
|
||||
expect(comments.filter((comment) => comment.issueId === input.otherWorkspaceSiblingId)).toHaveLength(0);
|
||||
}
|
||||
@@ -812,14 +816,10 @@ async function expectForwardBranchReconciled(input: {
|
||||
]),
|
||||
);
|
||||
if (resolvedRecoveryActionId) {
|
||||
expect(comments).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
authorType: "system",
|
||||
body: expect.stringContaining(`Recovery action: \`${resolvedRecoveryActionId}\``),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
expect(comments.some((comment) =>
|
||||
comment.authorType === "system" &&
|
||||
noticeMetadataReferencesRecoveryAction(comment.metadata, resolvedRecoveryActionId),
|
||||
)).toBe(true);
|
||||
}
|
||||
|
||||
const activities = await input.db
|
||||
|
||||
@@ -27,6 +27,7 @@ import { issueRoutes } from "../routes/issues.js";
|
||||
import { buildPaperclipWakePayload } from "../services/heartbeat.js";
|
||||
import { issueRecoveryActionService } from "../services/issue-recovery-actions.js";
|
||||
import { recoveryService } from "../services/recovery/service.js";
|
||||
import { noticeMetadataReferencesRecoveryAction } from "../services/recovery/successful-run-handoff.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
@@ -1143,21 +1144,14 @@ describeEmbeddedPostgres("issue recovery actions", () => {
|
||||
});
|
||||
|
||||
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, sourceIssue.id));
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.presentation).toMatchObject({
|
||||
const escalationComments = comments.filter((comment) =>
|
||||
noticeMetadataReferencesRecoveryAction(comment.metadata, actionRows[0]!.id),
|
||||
);
|
||||
expect(escalationComments).toHaveLength(1);
|
||||
expect(escalationComments[0]?.presentation).toMatchObject({
|
||||
kind: "system_notice",
|
||||
tone: "warning",
|
||||
title: "Recovery: workspace validation failed — moved to blocked (owner: CTO)",
|
||||
density: "compact",
|
||||
});
|
||||
expect(comments[0]?.metadata).toMatchObject({
|
||||
version: 1,
|
||||
sections: [expect.objectContaining({
|
||||
rows: expect.arrayContaining([
|
||||
expect.objectContaining({ type: "key_value", label: "Recovery action", value: actionRows[0]?.id }),
|
||||
expect.objectContaining({ type: "key_value", label: "Cause", value: "workspace_validation_failed" }),
|
||||
]),
|
||||
})],
|
||||
tone: "danger",
|
||||
title: "Workspace validation failed",
|
||||
});
|
||||
expect(enqueueWakeup).toHaveBeenCalledTimes(2);
|
||||
expect(enqueueWakeup).toHaveBeenCalledWith(
|
||||
@@ -1231,7 +1225,11 @@ describeEmbeddedPostgres("issue recovery actions", () => {
|
||||
|
||||
const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, sourceIssue.id));
|
||||
expect(comments).toHaveLength(1);
|
||||
expect(comments[0]?.body).toContain("Recovery action:");
|
||||
// Dedupe for structured notices is metadata-based: the short body no longer
|
||||
// carries the `Recovery action: \`id\`` marker line.
|
||||
expect(comments[0]?.body).not.toContain("Recovery action:");
|
||||
expect(noticeMetadataReferencesRecoveryAction(comments[0]?.metadata, actionRows[0]!.id)).toBe(true);
|
||||
expect(comments[0]?.presentation).toMatchObject({ kind: "system_notice", tone: "danger" });
|
||||
});
|
||||
|
||||
it("does not create nested recovery artifacts when issue-backed fallback work itself fails", async () => {
|
||||
|
||||
Reference in new issue
Block a user