mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
fix(inbox): separate automatic recovery cleanup from user participation
Keep exact workspace owner verification holds stable and let sandbox agents inspect authorized issue activity. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
4b958ecbd8
commit
697e132ca4
13 files changed
+194
-8
No files matched your search
@@ -126,6 +126,33 @@ describe("activity routes", () => {
|
||||
mockAccessService.canUser.mockResolvedValue(false);
|
||||
});
|
||||
|
||||
it("lets an agent read activity for an authorized issue", async () => {
|
||||
mockIssueService.getByIdentifier.mockResolvedValue({
|
||||
id: "issue-1", companyId: "company-1", projectId: null, parentId: null,
|
||||
assigneeAgentId: "agent-1", assigneeUserId: null, status: "done",
|
||||
});
|
||||
mockActivityService.forIssue.mockResolvedValue([{ action: "issue.recovery_action_resolved", actorType: "system" }]);
|
||||
const app = await createApp({ type: "agent", companyId: "company-1", agentId: "agent-1" });
|
||||
const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/issues/TEST-1/activity"));
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({
|
||||
action: "issue:read", resource: expect.objectContaining({ companyId: "company-1", issueId: "issue-1" }),
|
||||
}));
|
||||
expect(mockActivityService.forIssue).toHaveBeenCalledWith("issue-1");
|
||||
expect(res.body).toEqual([{ action: "issue.recovery_action_resolved", actorType: "system" }]);
|
||||
});
|
||||
|
||||
it.each(["boundary", "company"])("denies agent activity reads outside the %s scope", async (kind) => {
|
||||
const issue = { id: "issue-1", companyId: kind === "company" ? "company-2" : "company-1" };
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueService.getByIdentifier.mockResolvedValue(issue);
|
||||
mockAccessService.decide.mockResolvedValue({ allowed: false });
|
||||
const app = await createApp({ type: "agent", companyId: "company-1", agentId: "agent-1" });
|
||||
const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/issues/issue-1/activity"));
|
||||
expect(res.status).toBe(kind === "company" ? 404 : 403);
|
||||
expect(mockActivityService.forIssue).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns redacted all-actors rows to a basic company reader", async () => {
|
||||
mockAgentActionAuditService.list.mockResolvedValue({
|
||||
items: [{
|
||||
|
||||
@@ -38,6 +38,8 @@ import { deliverReconciledExecutions } from "../services/execution-recovery-reso
|
||||
import { workspaceOperationService, isNativeWorkspaceFinalizationOperationActive } from "../services/workspace-operations.js";
|
||||
import { issueRecoveryActionService } from "../services/issue-recovery-actions.js";
|
||||
import { recoveryService } from "../services/recovery/service.js";
|
||||
import { issueService } from "../services/issues.js";
|
||||
import { withNativeWorkspaceFinalizationOwnership } from "../services/native-runtime/native-workspace-finalization-ownership.js";
|
||||
import { noticeMetadataReferencesRecoveryAction } from "../services/recovery/successful-run-handoff.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
@@ -2479,6 +2481,51 @@ describeEmbeddedPostgres("issue recovery actions", () => {
|
||||
trigger: "read_projection",
|
||||
recoveryActionId: action.id,
|
||||
});
|
||||
expect(activityRows.find((row) => row.action === "issue.recovery_action_resolved")).toMatchObject({
|
||||
actorType: "system", actorId: "recovery", agentId: null,
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["done", "cancelled"])("keeps one workspace owner hold across reads and sweeps of a %s task", async (status) => {
|
||||
const { companyId, coderId, sourceIssueId } = await seedCompany();
|
||||
const runId = randomUUID();
|
||||
const owner = { token: randomUUID(), hostname: "old-controller.invalid", pid: 42, processStartedAt: new Date(0).toISOString() };
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: runId, companyId, agentId: coderId, nativeIssueId: sourceIssueId,
|
||||
runtimeMode: "native", status: "failed", finishedAt: new Date(),
|
||||
runnerProfileJson: { nativeWorkspaceFinalizationOwner: owner },
|
||||
});
|
||||
await db.update(issues).set({ status }).where(eq(issues.id, sourceIssueId));
|
||||
const userId = "board-reader";
|
||||
const app = createApp({ type: "board", source: "local_implicit", userId });
|
||||
const copyback = vi.fn();
|
||||
const recovery = recoveryService(db, { enqueueWakeup: vi.fn(async () => null) });
|
||||
let firstAction: typeof issueRecoveryActions.$inferSelect | undefined;
|
||||
for (let tick = 0; tick < 3; tick += 1) {
|
||||
expect(await withNativeWorkspaceFinalizationOwnership({ db, companyId, runId }, copyback)).toEqual({ acquired: false });
|
||||
const detail = await request(app).get(`/api/issues/${sourceIssueId}`).expect(200);
|
||||
expect(detail.body.activeRecoveryAction).toMatchObject({
|
||||
cause: "native_workspace_finalization_owner_unverified", status: "active", attemptCount: 1,
|
||||
});
|
||||
await request(app).get(`/api/companies/${companyId}/issues?view=compact`).expect(200);
|
||||
await request(app).get(`/api/issues/${sourceIssueId}/recovery-actions`).expect(200);
|
||||
await recovery.reconcileStrandedAssignedIssues();
|
||||
const actions = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, sourceIssueId));
|
||||
expect(actions).toHaveLength(1);
|
||||
firstAction ??= actions[0]!;
|
||||
expect(actions[0]).toEqual(firstAction);
|
||||
}
|
||||
expect(copyback).not.toHaveBeenCalled();
|
||||
expect(await issueService(db).list(companyId, { touchedByUserId: userId })).toEqual([]);
|
||||
expect(await db.select().from(activityLog).where(eq(activityLog.entityId, sourceIssueId))).toHaveLength(0);
|
||||
|
||||
// Only clearing the exact physical owner receipt makes this hold stale.
|
||||
await db.update(heartbeatRuns).set({ runnerProfileJson: {} }).where(eq(heartbeatRuns.id, runId));
|
||||
const cleared = await request(app).get(`/api/issues/${sourceIssueId}`).expect(200);
|
||||
expect(cleared.body.activeRecoveryAction).toBeNull();
|
||||
const rows = await db.select().from(activityLog).where(eq(activityLog.entityId, sourceIssueId));
|
||||
expect(rows).toEqual([expect.objectContaining({ actorType: "system", actorId: "recovery", action: "issue.recovery_action_resolved" })]);
|
||||
expect(await issueService(db).list(companyId, { touchedByUserId: userId })).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps active recovery visible when a plain comment does not create a live path", async () => {
|
||||
|
||||
@@ -668,6 +668,39 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("ignores historical reader-attributed recovery cleanup without hiding explicit recovery decisions", async () => {
|
||||
const companyId = await seedAssignableAgentCompany();
|
||||
const userId = "board-user";
|
||||
const old = new Date("2026-03-26T10:00:00Z");
|
||||
const recent = new Date("2026-03-26T15:00:00Z");
|
||||
const variants = [
|
||||
{ id: randomUUID(), details: { source: "source_revalidation", trigger: "read_projection" }, counts: false },
|
||||
{ id: randomUUID(), details: null, counts: true },
|
||||
{ id: randomUUID(), details: { outcome: "restored" }, counts: true },
|
||||
{ id: randomUUID(), details: { source: "source_revalidation", trigger: "issue_update" }, counts: true },
|
||||
];
|
||||
await db.insert(issues).values(variants.map(({ id }) => ({
|
||||
id, companyId, title: "Recovery audit", status: "done", updatedAt: old,
|
||||
})));
|
||||
await db.insert(activityLog).values(variants.map(({ id, details }) => ({
|
||||
companyId, actorType: "user", actorId: userId, action: "issue.recovery_action_resolved",
|
||||
entityType: "issue", entityId: id, details, createdAt: recent,
|
||||
})));
|
||||
await db.insert(issueComments).values(variants.map(({ id }) => ({
|
||||
companyId, issueId: id, authorUserId: "other-user", body: "An older update", createdAt: old,
|
||||
})));
|
||||
|
||||
const expectedIds = variants.filter(v => v.counts).map(v => v.id).sort();
|
||||
expect((await svc.list(companyId, { touchedByUserId: userId })).map(v => v.id).sort()).toEqual(expectedIds);
|
||||
expect(await svc.countUnreadTouchedByUser(companyId, userId)).toBe(expectedIds.length);
|
||||
const rows = await svc.list(companyId, {});
|
||||
for (const variant of variants) {
|
||||
expect(rows.find(v => v.id === variant.id)?.lastActivityAt).toEqual(variant.counts ? recent : old);
|
||||
}
|
||||
// Audit evidence is preserved, including the wrongly attributed old row.
|
||||
expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(variants.length);
|
||||
});
|
||||
|
||||
function agentRow(companyId: string, input: {
|
||||
id: string;
|
||||
name: string;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { parseObject } from "../adapters/utils.js";
|
||||
import { enqueueFastResponse } from "../services/fast-responses.js";
|
||||
import { hasRequiredWorkspaceRecovery, LEGACY_WORKSPACE_RECOVERY_SCHEMA } from "../services/workspace-restore-recovery-state.js";
|
||||
import { hasUnverifiedNativeWorkspaceFinalizationOwner } from "../services/native-runtime/native-workspace-finalization-recovery-state.js";
|
||||
import { monitorPoliciesEqual, applyActorMonitorScheduledBy, assertCanManageIssueMonitor, summarizeIssueMonitor } from "../services/issue-monitors.js";
|
||||
import type { IssuePrivacyConstraints } from "@paperclipai/shared";
|
||||
import { canActorReadHeartbeatRun } from "../services/heartbeat-run-privacy.js";
|
||||
@@ -4343,6 +4344,10 @@ export function issueRoutes(
|
||||
: input.activeRecoveryAction;
|
||||
if (!activeRecoveryAction) return null;
|
||||
|
||||
if (await hasUnverifiedNativeWorkspaceFinalizationOwner(db, activeRecoveryAction)) {
|
||||
return activeRecoveryAction;
|
||||
}
|
||||
|
||||
const resolutionNote = await classifySourceRecoveryRevalidation(input);
|
||||
if (!resolutionNote) return activeRecoveryAction;
|
||||
|
||||
@@ -4356,13 +4361,12 @@ export function issueRoutes(
|
||||
});
|
||||
if (!resolved) return activeRecoveryAction;
|
||||
|
||||
const actor = input.actor;
|
||||
await logActivity(db, {
|
||||
companyId: input.issue.companyId,
|
||||
actorType: actor?.actorType ?? "system",
|
||||
actorId: actor?.actorId ?? "system",
|
||||
agentId: actor?.agentId ?? null,
|
||||
runId: actor?.runId ?? null,
|
||||
actorType: "system",
|
||||
actorId: "recovery",
|
||||
agentId: null,
|
||||
runId: input.actor?.runId ?? null,
|
||||
action: "issue.recovery_action_resolved",
|
||||
entityType: "issue",
|
||||
entityId: input.issue.id,
|
||||
|
||||
@@ -2834,6 +2834,16 @@ const ISSUE_USER_PARTICIPATION_ACTIVITY_ACTIONS = [
|
||||
"issue.work_product_updated",
|
||||
] as const;
|
||||
|
||||
// Old read projections attributed this automatic cleanup to the reader. Keep
|
||||
// the audit rows, but do not count them as participation or issue activity.
|
||||
function automaticReadRecoveryCleanupCondition() {
|
||||
return sql<boolean>`(
|
||||
${activityLog.action} = 'issue.recovery_action_resolved'
|
||||
AND COALESCE(${activityLog.details}->>'source', '') = 'source_revalidation'
|
||||
AND COALESCE(${activityLog.details}->>'trigger', '') = 'read_projection'
|
||||
)`;
|
||||
}
|
||||
|
||||
function touchedByUserCondition(companyId: string, userId: string) {
|
||||
return sql<boolean>`
|
||||
(
|
||||
@@ -2847,6 +2857,7 @@ function touchedByUserCondition(companyId: string, userId: string) {
|
||||
AND ${activityLog.companyId} = ${companyId}
|
||||
AND ${activityLog.actorType} = 'user'
|
||||
AND ${activityLog.actorId} = ${userId}
|
||||
AND NOT ${automaticReadRecoveryCleanupCondition()}
|
||||
AND ${activityLog.action} IN (${sql.join(
|
||||
ISSUE_USER_PARTICIPATION_ACTIVITY_ACTIONS.map(
|
||||
(action) => sql`${action}`,
|
||||
@@ -2952,6 +2963,7 @@ function issueLatestLogAtExpr(companyId: string) {
|
||||
WHERE ${activityLog.companyId} = ${companyId}
|
||||
AND ${activityLog.entityType} = 'issue'
|
||||
AND ${activityLog.entityId} = ${issues.id}::text
|
||||
AND NOT ${automaticReadRecoveryCleanupCondition()}
|
||||
AND ${activityLog.action} NOT IN (${sql.join(
|
||||
ISSUE_LOCAL_INBOX_ACTIVITY_ACTIONS.map((action) => sql`${action}`),
|
||||
sql`, `,
|
||||
@@ -5127,6 +5139,7 @@ async function lastActivityStatsForIssues(
|
||||
eq(activityLog.companyId, companyId),
|
||||
eq(activityLog.entityType, "issue"),
|
||||
inArray(activityLog.entityId, issueIdChunk),
|
||||
sql`NOT ${automaticReadRecoveryCleanupCondition()}`,
|
||||
sql`${activityLog.action} NOT IN (${sql.join(
|
||||
ISSUE_LOCAL_INBOX_ACTIVITY_ACTIONS.map(
|
||||
(action) => sql`${action}`,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { issueRecoveryActionService } from "../issue-recovery-actions.js";
|
||||
import { nativeSha256 } from "./canonical.js";
|
||||
import { NATIVE_WORKSPACE_OWNER_UNVERIFIED } from "./native-workspace-finalization-recovery-state.js";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import os from "node:os";
|
||||
import { and, eq, sql } from "drizzle-orm";
|
||||
@@ -74,11 +75,18 @@ export async function withNativeWorkspaceFinalizationOwnership<T>(
|
||||
const prior = readOwner(rawPrior);
|
||||
if (rawPrior != null) {
|
||||
const requireStopVerification = async () => {
|
||||
if (run.issueId) await issueRecoveryActionService(input.db).upsertSourceScoped({
|
||||
const fingerprint = nativeSha256({ runId: input.runId, owner: rawPrior });
|
||||
const recovery = issueRecoveryActionService(input.db);
|
||||
const existing = run.issueId ? await recovery.getActiveForIssue(input.companyId, run.issueId) : null;
|
||||
// Rechecking the same physical owner is not a new recovery attempt.
|
||||
if (existing?.cause === NATIVE_WORKSPACE_OWNER_UNVERIFIED && existing.fingerprint === fingerprint) {
|
||||
return { acquired: false } as const;
|
||||
}
|
||||
if (run.issueId) await recovery.upsertSourceScoped({
|
||||
companyId: input.companyId, sourceIssueId: run.issueId,
|
||||
kind: "active_run_watchdog", ownerType: "board", returnOwnerAgentId: run.agentId,
|
||||
cause: "native_workspace_finalization_owner_unverified",
|
||||
fingerprint: nativeSha256({ runId: input.runId, owner: rawPrior }),
|
||||
cause: NATIVE_WORKSPACE_OWNER_UNVERIFIED,
|
||||
fingerprint,
|
||||
evidence: { runId: input.runId, owner: rawPrior },
|
||||
nextAction: "Verify the previous controller and its workspace-copyback processes have stopped, then release only this exact workspace owner receipt using doc/native-workspace-finalization-recovery.md. Resume workspace finalization without another provider turn.",
|
||||
wakePolicy: null, maxAttempts: 1, supersedeOnIdentityChange: true,
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { heartbeatRuns, type Db } from "@paperclipai/db";
|
||||
import { isUuidLike, type IssueRecoveryAction } from "@paperclipai/shared";
|
||||
import { nativeSha256 } from "./canonical.js";
|
||||
|
||||
export const NATIVE_WORKSPACE_OWNER_UNVERIFIED = "native_workspace_finalization_owner_unverified";
|
||||
|
||||
/** A terminal task does not prove that an older controller's copyback stopped. */
|
||||
export async function hasUnverifiedNativeWorkspaceFinalizationOwner(
|
||||
db: Db,
|
||||
action: Pick<IssueRecoveryAction, "companyId" | "sourceIssueId" | "cause" | "fingerprint" | "evidence">,
|
||||
): Promise<boolean> {
|
||||
if (action.cause !== NATIVE_WORKSPACE_OWNER_UNVERIFIED) return false;
|
||||
const runId = action.evidence?.runId;
|
||||
if (typeof runId !== "string" || !isUuidLike(runId)) return false;
|
||||
const [run] = await db.select({ profile: heartbeatRuns.runnerProfileJson }).from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, action.companyId),
|
||||
eq(heartbeatRuns.id, runId),
|
||||
eq(heartbeatRuns.nativeIssueId, action.sourceIssueId),
|
||||
eq(heartbeatRuns.runtimeMode, "native"),
|
||||
)).limit(1);
|
||||
const owner = run?.profile?.nativeWorkspaceFinalizationOwner;
|
||||
return owner != null && action.fingerprint === nativeSha256({ runId, owner });
|
||||
}
|
||||
@@ -93,6 +93,7 @@ import { emitAgentTaskRun } from "../agent-task-run-telemetry.js";
|
||||
import { budgetService } from "../budgets.js";
|
||||
import { unadmittedChatWakeupCondition } from "../durable-chat-wakeup.js";
|
||||
import { issueRecoveryActionService } from "../issue-recovery-actions.js";
|
||||
import { hasUnverifiedNativeWorkspaceFinalizationOwner } from "../native-runtime/native-workspace-finalization-recovery-state.js";
|
||||
import {
|
||||
legacyExecutionNeedsReconciliationWithEvidence,
|
||||
terminalizeLegacyExecution,
|
||||
@@ -3428,6 +3429,10 @@ export function recoveryService(
|
||||
issueIds: [] as string[],
|
||||
};
|
||||
for (const { action, issue } of rows) {
|
||||
if (await hasUnverifiedNativeWorkspaceFinalizationOwner(db, action)) {
|
||||
result.skipped += 1;
|
||||
continue;
|
||||
}
|
||||
const wakePolicy = parseObject(action.wakePolicy);
|
||||
const wakePolicyType = readNonEmptyString(wakePolicy.type);
|
||||
if (
|
||||
|
||||
Reference in new issue
Block a user