mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
fix(onboarding): retain plan approval across follow-up decisions (#19043)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Onboarding asks a person to approve a saved plan before execution tasks can start. > - A person can then approve a separate proposal for the first execution step. > - The creation gate checks only that latest card for the plan target. It loses the earlier approval of the unchanged plan. > - Decomposition and native context also omit checkbox plan confirmations. > - This pull request validates the latest execution decision and the latest plan decision together. > - The benefit is that approved work can start while human, revision, and proposal gates remain effective. ## Linked Issues or Issue Description Refs #17914, #18160, and #18283. The related-PR search found no duplicate repair. **What happened?** After a person accepted a checkbox plan card and a later execution confirmation, native task creation and REST child creation returned 403. Accepted-plan decomposition returned 422 because its lookup recognized only standard confirmations. Native task context returned no accepted plan for the checkbox decision. **Expected behavior** The latest execution proposal can use an independently approved, unchanged plan. A pending or rejected latest proposal must still stop execution. The current plan must retain valid human approval of its exact revision. **Steps to reproduce** 1. Save a plan on a server-created onboarding task. 2. Create a revision-bound checkbox confirmation and accept it as a human. 3. Create and accept a separate, untargeted confirmation for the first execution task. 4. Create the child through the native tool or REST API. 5. Separately, try accepted-plan decomposition or read native task context after step 2. **Paperclip version or commit** The workspace's registered hosted release is Core `09c05c55050fede448196637cbaa543a154da76e`. The three regressions also failed against local source `3cdb367369f29692b81cf19b514a2abdba90734f`. **Deployment mode** Hosted native Runner with ACPX Claude. Local regression proof uses actual PostgreSQL, authenticated HTTP routes, native tool authority, and synthetic task data on macOS arm64. No customer data or credentials are in the fixtures. ## What Changed - Validate the newest ordinary proposal and the newest plan proposal separately. Keep the existing human-card and conversational provenance checks for each. - Require the accepted plan target to match the current document and revision. A newer pending, rejected, invalid, or foreign plan decision cannot fall back to an older approval. - Share persisted confirmation parsing across admission, decomposition, and native context. Validate checkbox selections against the saved options and limits. - Return `acceptedPlan.approvedOptions` with the user's saved selected IDs and option text on later wakes, including revision-pinned context. Keep unchecked defaults out of scope. - Add behavioral and safety regressions. Document the approval rule. ## Verification - Three behavioral regressions failed before the repair: child creation returned 403, decomposition returned 422, and native context returned `acceptedPlan: null`. All three now pass. - Two scope-retention regressions failed before the review fix: native context omitted the selected option after a later approval, with and without a pinned plan revision. Both now pass. Unchecked defaults stay excluded. - Current admission, native-tool, and accepted-confirmation replay suites: 131 tests passed. Existing first-task completion safety checks: 17 passed, including the atomic decision recheck. - Safety cases cover missing and rejected plan approval, agent-only decisions, foreign source runs, changed revisions, invalid checkbox selections, newer pending or rejected proposals, and task-creation replay. - Full repository `pnpm -r typecheck` and `pnpm build` passed locally on final head `7c0e8ab784`. - [Full final-head CI](https://github.com/paperclipai/paperclip/actions/runs/41911580531) passed: all Vitest shards, Runner checks, eight browser E2E shards, build, release registry, and canary dry run. The required `verify` and `e2e` aggregates passed. Storybook checks were skipped for this backend change; the file-selection job was skipped because full CI ran. - Earlier local typecheck attempts exposed Copilot verification during build preparation, stale declarations across the rebase, and two test-fixture typing errors. The assets were verified, declarations rebuilt, and fixtures corrected before the complete final-head pass. - Full local Vitest attempts were interrupted for the rebase and review repair. The duplicate final local run was stopped after full Linux CI passed. No complete macOS Vitest-suite pass is claimed. - A fresh Claude-provider acceptance run remains unverified: the local Claude account is signed out and no Anthropic API credential is available. The actual native tool and HTTP paths are verified with real PostgreSQL persistence. This PR does not deploy or recover the reported production task. - Greptile: 5/5 on `7c0e8ab784`, with the review thread resolved. ## Risks A later acceptance must not revive rejected or changed scope. Admission validates both current decisions and retains source, company, human provenance, and document locks. The extra plan-proposal lookup is bounded and limited to accepted, untargeted first-task proposals. Ordinary delegation adds no new proof lookup. The native context adds an optional `approvedOptions` projection. Revision-pinned context now performs the existing scoped approval read to recover that selection. No database schema, request, provider, or background worker changes are included. ## Model Used OpenAI Codex, exact model `gpt-6.1-sol`, max reasoning effort, with repository, shell, browser, and GitHub tools. The current session metadata confirms the model and effort. The context window size is not exposed. ## 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
8002dfb06e
commit
4740d251b7
6 files changed
+239
-42
No files matched your search
@@ -34,6 +34,16 @@ approvals do not authorize the proposal. A newer pending or rejected proposal su
|
|||||||
older acceptance. A saved plan requires approval of its current document revision;
|
older acceptance. A saved plan requires approval of its current document revision;
|
||||||
checkbox decisions retain the user's actual selected scope.
|
checkbox decisions retain the user's actual selected scope.
|
||||||
|
|
||||||
|
A later accepted execution proposal can be untargeted when the unchanged saved
|
||||||
|
plan already has its own human approval. Admission validates both decisions. It
|
||||||
|
uses the latest plan proposal, so pending, rejected, invalid, or stale plan
|
||||||
|
approval cannot fall back to an older acceptance. A later pending or rejected
|
||||||
|
execution proposal still stops creation. Accepted checkbox plan confirmations
|
||||||
|
also appear in native task context and authorize normal accepted-plan decomposition.
|
||||||
|
Native context includes `acceptedPlan.approvedOptions` for checkbox decisions,
|
||||||
|
with the saved selected IDs and option text. It does not substitute the card's
|
||||||
|
defaults or unchecked options, including after a later execution confirmation.
|
||||||
|
|
||||||
Planning documents, questions and proposal cards remain available before approval.
|
Planning documents, questions and proposal cards remain available before approval.
|
||||||
Ordinary task delegation and previously committed task-creation replay retain
|
Ordinary task delegation and previously committed task-creation replay retain
|
||||||
their existing behavior, including dispatch recovery for already admitted work.
|
their existing behavior, including dispatch recovery for already admitted work.
|
||||||
|
|||||||
@@ -582,6 +582,143 @@ describe.skipIf(!support.supported)("onboarding first-task execution admission",
|
|||||||
await expect(create(f)).rejects.toThrow(/proposal.*accept/i);
|
await expect(create(f)).rejects.toThrow(/proposal.*accept/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const acceptFollowUp = async (f: Awaited<ReturnType<typeof fixture>>, p: Awaited<ReturnType<typeof proposal>>) => {
|
||||||
|
const card = await p.svc.create(p.issue, { kind: "request_confirmation",
|
||||||
|
payload: { version: 1, prompt: "Create the first read-only discovery task from the approved plan?" },
|
||||||
|
sourceRunId: f.runId,
|
||||||
|
}, { agentId: f.agentId, runId: f.runId });
|
||||||
|
await p.svc.acceptInteraction(p.issue, card.id, {}, { userId: f.responsibleUserId! });
|
||||||
|
return card;
|
||||||
|
};
|
||||||
|
|
||||||
|
it("retains approval of the current plan through a later accepted execution proposal", async () => {
|
||||||
|
const f = await fixture();
|
||||||
|
const p = await proposal(f, true);
|
||||||
|
await accept(f, p);
|
||||||
|
await acceptFollowUp(f, p);
|
||||||
|
await expect(create(f)).resolves.toMatchObject({ task: { parentId: f.issueId } });
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each(["pending", "rejected"])("keeps a later %s execution proposal behind its human gate", async status => {
|
||||||
|
const f = await fixture();
|
||||||
|
const p = await proposal(f, true);
|
||||||
|
await accept(f, p);
|
||||||
|
const card = await p.svc.create(p.issue, { kind: "request_confirmation",
|
||||||
|
payload: { version: 1, prompt: "Create the next execution task?" },
|
||||||
|
}, { agentId: f.agentId });
|
||||||
|
if (status === "rejected") await p.svc.rejectInteraction(p.issue, card.id, {}, { userId: f.responsibleUserId! });
|
||||||
|
await expect(create(f)).rejects.toThrow(/proposal.*accept/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each(["pending", "rejected", "agent-only", "invalid-selection", "revised-plan", "foreign-source"])(
|
||||||
|
"a later accepted proposal cannot replace %s plan approval", async scenario => {
|
||||||
|
const f = await fixture();
|
||||||
|
const p = await proposal(f, true);
|
||||||
|
if (scenario === "rejected") await p.svc.rejectInteraction(p.issue, p.card.id, {}, { userId: f.responsibleUserId! });
|
||||||
|
else if (scenario === "agent-only") await p.svc.acceptInteraction(p.issue, p.card.id, { selectedOptionIds: ["execute"] }, { agentId: f.agentId, runId: f.runId });
|
||||||
|
else if (scenario !== "pending") await accept(f, p);
|
||||||
|
if (scenario === "invalid-selection") await server.db.update(issueThreadInteractions)
|
||||||
|
.set({ result: { version: 1, outcome: "accepted", selectedOptionIds: ["unapproved"] } }).where(eq(issueThreadInteractions.id, p.card.id));
|
||||||
|
if (scenario === "foreign-source") {
|
||||||
|
const other = await fixture();
|
||||||
|
await server.db.update(issueThreadInteractions).set({ sourceRunId: other.runId }).where(eq(issueThreadInteractions.id, p.card.id));
|
||||||
|
}
|
||||||
|
if (scenario === "revised-plan") {
|
||||||
|
await documentService(server.db).unlockIssueDocument(f.issueId, "plan");
|
||||||
|
await documentService(server.db).upsertIssueDocument({ issueId: f.issueId, key: "plan", format: "markdown",
|
||||||
|
baseRevisionId: p.plan!.latestRevisionId, body: "Changed scope requires a new plan decision." });
|
||||||
|
}
|
||||||
|
await acceptFollowUp(f, p);
|
||||||
|
const before = await f.snapshot();
|
||||||
|
await expect(create(f)).rejects.toThrow(/proposal.*accept/i);
|
||||||
|
expect((await f.snapshot()).issues).toEqual(before.issues);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not revive an older plan approval after a newer rejected plan proposal", async () => {
|
||||||
|
const f = await fixture();
|
||||||
|
const p = await proposal(f, true);
|
||||||
|
await accept(f, p);
|
||||||
|
const replacement = await p.svc.create(p.issue, { kind: "request_confirmation", payload: {
|
||||||
|
version: 1, prompt: "Approve this plan again?", target: {
|
||||||
|
type: "issue_document", key: "plan", issueId: f.issueId,
|
||||||
|
documentId: p.plan!.id, revisionId: p.plan!.latestRevisionId!,
|
||||||
|
},
|
||||||
|
} }, { agentId: f.agentId });
|
||||||
|
await p.svc.rejectInteraction(p.issue, replacement.id, {}, { userId: f.responsibleUserId! });
|
||||||
|
await acceptFollowUp(f, p);
|
||||||
|
await expect(create(f)).rejects.toThrow(/proposal.*accept/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("decomposes a plan accepted through the onboarding checkbox card", async () => {
|
||||||
|
const f = await fixture();
|
||||||
|
const p = await proposal(f, true);
|
||||||
|
await accept(f, p);
|
||||||
|
const token = createLocalAgentJwt(f.agentId, f.companyId, "paperclip_runner", f.runId, f.responsibleUserId)!;
|
||||||
|
const response = await fetch(`${server.apiUrl}/api/issues/${f.issueId}/accepted-plan-decompositions`, {
|
||||||
|
method: "POST", headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json" },
|
||||||
|
body: JSON.stringify({ acceptedPlanRevisionId: p.plan!.latestRevisionId,
|
||||||
|
children: [{ title: "First discovery task", status: "backlog", workMode: "standard", priority: "medium", assigneeAgentId: f.agentId }] }),
|
||||||
|
});
|
||||||
|
const data = await response.json();
|
||||||
|
expect(response.status, JSON.stringify(data)).toBe(200);
|
||||||
|
expect(data.childIssueIds).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("includes a checkbox-approved plan in the native task context", async () => {
|
||||||
|
const f = await fixture();
|
||||||
|
const p = await proposal(f, true);
|
||||||
|
await accept(f, p);
|
||||||
|
await expect(f.authority.execute({ tool: "get_task_context", callId: randomUUID(), arguments: {} }))
|
||||||
|
.resolves.toMatchObject({ acceptedPlan: { revisionId: p.plan!.latestRevisionId } });
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([false, true])("retains only the selected plan scope after a later execution approval (anchored=%s)", async anchored => {
|
||||||
|
const f = await fixture();
|
||||||
|
const [issue] = await server.db.select().from(issues).where(eq(issues.id, f.issueId));
|
||||||
|
const plan = (await documentService(server.db).upsertIssueDocument({ issueId: f.issueId, key: "plan",
|
||||||
|
format: "markdown", body: "First: read-only discovery. Later: implementation, only if approved." })).document;
|
||||||
|
const svc = issueThreadInteractionService(server.db);
|
||||||
|
const selected = { id: "discovery", label: "Read-only discovery", description: "Inspect and write the architecture document." };
|
||||||
|
const card = await svc.create(issue, { kind: "request_checkbox_confirmation", payload: {
|
||||||
|
version: 1, prompt: "Which plan steps may start?", minSelected: 1, maxSelected: 2,
|
||||||
|
defaultSelectedOptionIds: ["discovery", "implementation"],
|
||||||
|
options: [selected, { id: "implementation", label: "Change the application" }],
|
||||||
|
target: { type: "issue_document", key: "plan", issueId: f.issueId, documentId: plan.id, revisionId: plan.latestRevisionId! },
|
||||||
|
} }, { agentId: f.agentId });
|
||||||
|
await svc.acceptInteraction(issue, card.id, { selectedOptionIds: [selected.id] }, { userId: f.responsibleUserId! });
|
||||||
|
await acceptFollowUp(f, { issue, plan, svc, card });
|
||||||
|
if (anchored) await server.db.update(heartbeatRuns).set({ contextSnapshot: {
|
||||||
|
issueId: f.issueId, planReviewInteraction: { acceptedTargetRevision: { revisionId: plan.latestRevisionId } },
|
||||||
|
} }).where(eq(heartbeatRuns.id, f.runId));
|
||||||
|
const context = await f.authority.execute({ tool: "get_task_context", callId: randomUUID(), arguments: {} }) as {
|
||||||
|
acceptedPlan: { revisionId: string; approvedOptions?: unknown[] };
|
||||||
|
};
|
||||||
|
expect(context.acceptedPlan.revisionId).toBe(plan.latestRevisionId);
|
||||||
|
expect(context.acceptedPlan.approvedOptions).toEqual([selected]);
|
||||||
|
const [saved] = await server.db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, card.id));
|
||||||
|
expect(saved.result).toMatchObject({ selectedOptionIds: [selected.id] });
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
{ selectedOptionIds: [] },
|
||||||
|
{ selectedOptionIds: ["unapproved"] },
|
||||||
|
{ selectedOptionIds: ["execute", "execute"] },
|
||||||
|
])("does not publish or decompose an invalid checkbox selection $selectedOptionIds", async ({ selectedOptionIds }) => {
|
||||||
|
const f = await fixture();
|
||||||
|
const p = await proposal(f, true);
|
||||||
|
await accept(f, p);
|
||||||
|
await server.db.update(issueThreadInteractions).set({ result: { version: 1, outcome: "accepted", selectedOptionIds } })
|
||||||
|
.where(eq(issueThreadInteractions.id, p.card.id));
|
||||||
|
await expect(f.authority.execute({ tool: "get_task_context", callId: randomUUID(), arguments: {} }))
|
||||||
|
.resolves.toMatchObject({ acceptedPlan: null });
|
||||||
|
const before = await f.snapshot();
|
||||||
|
await expect(issueService(server.db).decomposeAcceptedPlan(f.issueId, {
|
||||||
|
acceptedPlanRevisionId: p.plan!.latestRevisionId!, actorAgentId: f.agentId, actorRunId: f.runId,
|
||||||
|
children: [{ title: "Unapproved work", status: "backlog", workMode: "standard", priority: "medium" }],
|
||||||
|
})).rejects.toThrow(/accepted plan confirmation/);
|
||||||
|
expect((await f.snapshot()).issues).toEqual(before.issues);
|
||||||
|
});
|
||||||
|
|
||||||
it.each(["stale", "foreign-target", "empty-selection"])("does not execute a plan with %s approval", async kind => {
|
it.each(["stale", "foreign-target", "empty-selection"])("does not execute a plan with %s approval", async kind => {
|
||||||
const f = await fixture();
|
const f = await fixture();
|
||||||
const p = await proposal(f, true);
|
const p = await proposal(f, true);
|
||||||
|
|||||||
@@ -0,0 +1,36 @@
|
|||||||
|
import {
|
||||||
|
requestCheckboxConfirmationPayloadSchema,
|
||||||
|
requestCheckboxConfirmationResultSchema,
|
||||||
|
requestConfirmationPayloadSchema,
|
||||||
|
requestConfirmationResultSchema,
|
||||||
|
type RequestCheckboxConfirmationOption,
|
||||||
|
} from "@paperclipai/shared";
|
||||||
|
|
||||||
|
export function readPersistedConfirmationDecision(row: {
|
||||||
|
kind: string;
|
||||||
|
status: string;
|
||||||
|
payload: unknown;
|
||||||
|
result: unknown;
|
||||||
|
resolvedAt: Date | null;
|
||||||
|
}) {
|
||||||
|
if ((row.kind !== "request_confirmation" && row.kind !== "request_checkbox_confirmation")
|
||||||
|
|| (row.status !== "accepted" && row.status !== "rejected") || !row.resolvedAt) return null;
|
||||||
|
const checkbox = row.kind === "request_checkbox_confirmation";
|
||||||
|
const payload = checkbox ? requestCheckboxConfirmationPayloadSchema.safeParse(row.payload)
|
||||||
|
: requestConfirmationPayloadSchema.safeParse(row.payload);
|
||||||
|
const result = checkbox ? requestCheckboxConfirmationResultSchema.safeParse(row.result)
|
||||||
|
: requestConfirmationResultSchema.safeParse(row.result);
|
||||||
|
if (!payload.success || !result.success || result.data.outcome !== row.status) return null;
|
||||||
|
let approvedOptions: RequestCheckboxConfirmationOption[] | null = null;
|
||||||
|
if (checkbox && row.status === "accepted") {
|
||||||
|
if (!("options" in payload.data) || !("selectedOptionIds" in result.data)) return null;
|
||||||
|
const selected = result.data.selectedOptionIds;
|
||||||
|
const allowed = new Set(payload.data.options.map(option => option.id));
|
||||||
|
if (!Array.isArray(selected) || selected.length === 0 || selected.some(id => !allowed.has(id))
|
||||||
|
|| selected.length < payload.data.minSelected
|
||||||
|
|| (payload.data.maxSelected != null && selected.length > payload.data.maxSelected)) return null;
|
||||||
|
const options = new Map(payload.data.options.map(option => [option.id, option]));
|
||||||
|
approvedOptions = selected.map(id => options.get(id)!);
|
||||||
|
}
|
||||||
|
return { outcome: row.status, target: payload.data.target ?? null, commentId: result.data.commentId, approvedOptions } as const;
|
||||||
|
}
|
||||||
@@ -25,6 +25,7 @@ import { notifyChatRunMilestones } from "./chat-run-milestone-notifications.js";
|
|||||||
import { notifyApprovalWork } from "./approval-work-notifications.js";
|
import { notifyApprovalWork } from "./approval-work-notifications.js";
|
||||||
import { notifyDecisionTargetWork, notifyDecisionTargetIssueRemoval } from "./decision-targets.js";
|
import { notifyDecisionTargetWork, notifyDecisionTargetIssueRemoval } from "./decision-targets.js";
|
||||||
import { assertOnboardingFirstTaskCreationAllowed, assertOnboardingFirstTaskDependenciesAllowed } from "./onboarding-first-task-admission.js";
|
import { assertOnboardingFirstTaskCreationAllowed, assertOnboardingFirstTaskDependenciesAllowed } from "./onboarding-first-task-admission.js";
|
||||||
|
import { readPersistedConfirmationDecision } from "./confirmation-decision.js";
|
||||||
import { resolveConnectionIntentContinuationSource, resolveItemVerdictContinuationWake } from "./chat-interaction-continuation.js";
|
import { resolveConnectionIntentContinuationSource, resolveItemVerdictContinuationWake } from "./chat-interaction-continuation.js";
|
||||||
import { insertIssueComment } from "./issue-comment-writes.js";
|
import { insertIssueComment } from "./issue-comment-writes.js";
|
||||||
import { measureRunTransaction } from "./execution-transaction-timing.js";
|
import { measureRunTransaction } from "./execution-transaction-timing.js";
|
||||||
@@ -2421,13 +2422,17 @@ async function findAcceptedPlanDocumentInteraction(
|
|||||||
.select({
|
.select({
|
||||||
id: issueThreadInteractions.id,
|
id: issueThreadInteractions.id,
|
||||||
payload: issueThreadInteractions.payload,
|
payload: issueThreadInteractions.payload,
|
||||||
|
kind: issueThreadInteractions.kind,
|
||||||
|
status: issueThreadInteractions.status,
|
||||||
|
result: issueThreadInteractions.result,
|
||||||
|
resolvedAt: issueThreadInteractions.resolvedAt,
|
||||||
})
|
})
|
||||||
.from(issueThreadInteractions)
|
.from(issueThreadInteractions)
|
||||||
.where(
|
.where(
|
||||||
and(
|
and(
|
||||||
eq(issueThreadInteractions.companyId, input.companyId),
|
eq(issueThreadInteractions.companyId, input.companyId),
|
||||||
eq(issueThreadInteractions.issueId, input.sourceIssueId),
|
eq(issueThreadInteractions.issueId, input.sourceIssueId),
|
||||||
eq(issueThreadInteractions.kind, "request_confirmation"),
|
inArray(issueThreadInteractions.kind, ["request_confirmation", "request_checkbox_confirmation"]),
|
||||||
eq(issueThreadInteractions.status, "accepted"),
|
eq(issueThreadInteractions.status, "accepted"),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
@@ -2437,6 +2442,7 @@ async function findAcceptedPlanDocumentInteraction(
|
|||||||
);
|
);
|
||||||
|
|
||||||
for (const row of rows) {
|
for (const row of rows) {
|
||||||
|
if (!readPersistedConfirmationDecision(row)) continue;
|
||||||
const target = readAcceptedPlanConfirmationTarget(
|
const target = readAcceptedPlanConfirmationTarget(
|
||||||
row.payload,
|
row.payload,
|
||||||
input.sourceIssueId,
|
input.sourceIssueId,
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import { DOWNLOAD_ARTIFACT, DOWNLOAD_ARTIFACT_DEFINITION, downloadArtifactSchema
|
|||||||
import { notifyStatusCardIssueChange } from "../status-card-notifications.js";
|
import { notifyStatusCardIssueChange } from "../status-card-notifications.js";
|
||||||
import { nativeRunKeyScope, nativeSkillTestToolAllowed, assertNativeSkillTestTool } from "./native-skill-test-scope.js";
|
import { nativeRunKeyScope, nativeSkillTestToolAllowed, assertNativeSkillTestTool } from "./native-skill-test-scope.js";
|
||||||
import { notifyAttentionRetentionWork } from "../retention-work-notifications.js";
|
import { notifyAttentionRetentionWork } from "../retention-work-notifications.js";
|
||||||
import { requiredQualityReviewSchema } from "@paperclipai/shared";
|
import { requiredQualityReviewSchema, type RequestCheckboxConfirmationOption } from "@paperclipai/shared";
|
||||||
import { priorRecoveryApiReceipt } from "../recovery-effect-receipts.js";
|
import { priorRecoveryApiReceipt } from "../recovery-effect-receipts.js";
|
||||||
import { DELIVERY_QUEUES, notifyDeliveryWork } from "../delivery-work-notifications.js";
|
import { DELIVERY_QUEUES, notifyDeliveryWork } from "../delivery-work-notifications.js";
|
||||||
import { caseHandoffService } from "../case-handoffs.js";
|
import { caseHandoffService } from "../case-handoffs.js";
|
||||||
@@ -97,6 +97,7 @@ import { approvalService } from "../approvals.js";
|
|||||||
import { documentService } from "../documents.js";
|
import { documentService } from "../documents.js";
|
||||||
import { issueService } from "../issues.js";
|
import { issueService } from "../issues.js";
|
||||||
import { issueThreadInteractionService } from "../issue-thread-interactions.js";
|
import { issueThreadInteractionService } from "../issue-thread-interactions.js";
|
||||||
|
import { readPersistedConfirmationDecision } from "../confirmation-decision.js";
|
||||||
import { getNativeReviewAssignment, readNativeReviewAssignmentContext, type NativeReviewAssignmentContext } from "./native-review-participant.js";
|
import { getNativeReviewAssignment, readNativeReviewAssignmentContext, type NativeReviewAssignmentContext } from "./native-review-participant.js";
|
||||||
import { childReviewOutcomes } from "./child-review-outcomes.js";
|
import { childReviewOutcomes } from "./child-review-outcomes.js";
|
||||||
import { logActivity, persistActivity, publishActivity } from "../activity-log.js";
|
import { logActivity, persistActivity, publishActivity } from "../activity-log.js";
|
||||||
@@ -2468,6 +2469,7 @@ export class PaperclipRunnerToolAuthority {
|
|||||||
revisionId: string;
|
revisionId: string;
|
||||||
revisionNumber: number;
|
revisionNumber: number;
|
||||||
markdown: string;
|
markdown: string;
|
||||||
|
approvedOptions?: RequestCheckboxConfirmationOption[];
|
||||||
} | null> {
|
} | null> {
|
||||||
const acceptedTarget = record(
|
const acceptedTarget = record(
|
||||||
record(record(contextSnapshot).planReviewInteraction).acceptedTargetRevision,
|
record(record(contextSnapshot).planReviewInteraction).acceptedTargetRevision,
|
||||||
@@ -2475,7 +2477,8 @@ export class PaperclipRunnerToolAuthority {
|
|||||||
let revisionId = typeof acceptedTarget.revisionId === "string"
|
let revisionId = typeof acceptedTarget.revisionId === "string"
|
||||||
? acceptedTarget.revisionId
|
? acceptedTarget.revisionId
|
||||||
: null;
|
: null;
|
||||||
if (!revisionId) revisionId = await this.#latestAcceptedPlanRevisionId();
|
const approval = await this.#latestAcceptedPlanApproval(revisionId);
|
||||||
|
if (!revisionId) revisionId = approval?.revisionId ?? null;
|
||||||
if (!revisionId) return null;
|
if (!revisionId) return null;
|
||||||
|
|
||||||
const [revision] = await this.db.select({
|
const [revision] = await this.db.select({
|
||||||
@@ -2496,33 +2499,42 @@ export class PaperclipRunnerToolAuthority {
|
|||||||
eq(documentRevisions.companyId, this.binding.companyId),
|
eq(documentRevisions.companyId, this.binding.companyId),
|
||||||
))
|
))
|
||||||
.limit(1);
|
.limit(1);
|
||||||
if (!revision || !this.binding.museRuntime) return revision ?? null;
|
if (!revision) return null;
|
||||||
|
const acceptedRevision = { ...revision, ...(approval?.approvedOptions ? { approvedOptions: approval.approvedOptions } : {}) };
|
||||||
|
if (!this.binding.museRuntime) return acceptedRevision;
|
||||||
const document = await documentService(this.db).getIssueDocumentByKey(this.binding.issueId, "plan");
|
const document = await documentService(this.db).getIssueDocumentByKey(this.binding.issueId, "plan");
|
||||||
const [visible] = await this.#projectMuseDocumentContent(issue, [{
|
const [visible] = await this.#projectMuseDocumentContent(issue, [{
|
||||||
id: revision.revisionId, documentId: revision.documentId, body: revision.markdown,
|
id: revision.revisionId, documentId: revision.documentId, body: revision.markdown,
|
||||||
}], document ? [document] : []);
|
}], document ? [document] : []);
|
||||||
return { ...revision, markdown: visible!.body };
|
return { ...acceptedRevision, markdown: visible!.body };
|
||||||
}
|
}
|
||||||
|
|
||||||
async #latestAcceptedPlanRevisionId(): Promise<string | null> {
|
async #latestAcceptedPlanApproval(revisionId: string | null): Promise<{
|
||||||
const rows = await this.db.select({ payload: issueThreadInteractions.payload })
|
revisionId: string;
|
||||||
|
approvedOptions: RequestCheckboxConfirmationOption[] | null;
|
||||||
|
} | null> {
|
||||||
|
const rows = await this.db.select({ payload: issueThreadInteractions.payload, kind: issueThreadInteractions.kind,
|
||||||
|
status: issueThreadInteractions.status, result: issueThreadInteractions.result, resolvedAt: issueThreadInteractions.resolvedAt })
|
||||||
.from(issueThreadInteractions)
|
.from(issueThreadInteractions)
|
||||||
.where(and(
|
.where(and(
|
||||||
eq(issueThreadInteractions.companyId, this.binding.companyId),
|
eq(issueThreadInteractions.companyId, this.binding.companyId),
|
||||||
eq(issueThreadInteractions.issueId, this.binding.issueId),
|
eq(issueThreadInteractions.issueId, this.binding.issueId),
|
||||||
eq(issueThreadInteractions.kind, "request_confirmation"),
|
inArray(issueThreadInteractions.kind, ["request_confirmation", "request_checkbox_confirmation"]),
|
||||||
eq(issueThreadInteractions.status, "accepted"),
|
eq(issueThreadInteractions.status, "accepted"),
|
||||||
|
revisionId ? sql`${issueThreadInteractions.payload}->'target'->>'revisionId' = ${revisionId}` : undefined,
|
||||||
))
|
))
|
||||||
.orderBy(desc(issueThreadInteractions.resolvedAt), desc(issueThreadInteractions.createdAt));
|
.orderBy(desc(issueThreadInteractions.resolvedAt), desc(issueThreadInteractions.createdAt));
|
||||||
for (const row of rows) {
|
for (const row of rows) {
|
||||||
const target = record(record(row.payload).target);
|
const decision = readPersistedConfirmationDecision(row);
|
||||||
|
if (!decision) continue;
|
||||||
|
const target = decision.target;
|
||||||
if (
|
if (
|
||||||
target.type === "issue_document"
|
target?.type === "issue_document"
|
||||||
&& (target.issueId === undefined || target.issueId === this.binding.issueId)
|
&& (target.issueId === undefined || target.issueId === this.binding.issueId)
|
||||||
&& target.key === "plan"
|
&& target.key === "plan"
|
||||||
&& typeof target.revisionId === "string"
|
&& typeof target.revisionId === "string"
|
||||||
&& target.revisionId.length > 0
|
&& target.revisionId.length > 0
|
||||||
) return target.revisionId;
|
) return { revisionId: target.revisionId, approvedOptions: decision.approvedOptions };
|
||||||
}
|
}
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,11 +1,8 @@
|
|||||||
import { and, desc, eq, inArray, sql } from "drizzle-orm";
|
import { and, desc, eq, inArray, sql } from "drizzle-orm";
|
||||||
import { agents, documents, heartbeatRuns, issueComments, issueDocuments, issues, issueThreadInteractions, type Db } from "@paperclipai/db";
|
import { agents, documents, heartbeatRuns, issueComments, issueDocuments, issues, issueThreadInteractions, type Db } from "@paperclipai/db";
|
||||||
import { ONBOARDING_FIRST_TASK_ORIGIN_KIND, requestCheckboxConfirmationPayloadSchema, requestCheckboxConfirmationResultSchema,
|
import { ONBOARDING_FIRST_TASK_ORIGIN_KIND } from "@paperclipai/shared";
|
||||||
requestConfirmationPayloadSchema, requestConfirmationResultSchema } from "@paperclipai/shared";
|
|
||||||
import { badRequest, forbidden } from "../errors.js";
|
import { badRequest, forbidden } from "../errors.js";
|
||||||
|
import { readPersistedConfirmationDecision } from "./confirmation-decision.js";
|
||||||
const object = (value: unknown): Record<string, unknown> =>
|
|
||||||
value && typeof value === "object" && !Array.isArray(value) ? value as Record<string, unknown> : {};
|
|
||||||
|
|
||||||
const runOriginFields = {
|
const runOriginFields = {
|
||||||
nativeIssueId: heartbeatRuns.nativeIssueId,
|
nativeIssueId: heartbeatRuns.nativeIssueId,
|
||||||
@@ -97,10 +94,7 @@ export async function assertOnboardingFirstTaskDependenciesAllowed(tx: Db, input
|
|||||||
assertNotBlockedByFirstTaskSource(sources.find(source => source.isRequestedBlocker));
|
assertNotBlockedByFirstTaskSource(sources.find(source => source.isRequestedBlocker));
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Shared persisted-decision proof; creation and completion retain different authority. */
|
async function readLatestProposal(tx: Db, input: { companyId: string; issueId: string }, planOnly = false) {
|
||||||
async function readOnboardingFirstTaskDecision(tx: Db, input: {
|
|
||||||
companyId: string; issueId: string; outcomes: Array<"accepted" | "rejected">;
|
|
||||||
}): Promise<"accepted" | "rejected" | null> {
|
|
||||||
// Select the newest proposal before checking its outcome: a later pending or
|
// Select the newest proposal before checking its outcome: a later pending or
|
||||||
// rejected proposal must never fall back to an older accepted scope. Governed
|
// rejected proposal must never fall back to an older accepted scope. Governed
|
||||||
// tool/credential actions and custom completion reviews are not proposals.
|
// tool/credential actions and custom completion reviews are not proposals.
|
||||||
@@ -109,23 +103,17 @@ async function readOnboardingFirstTaskDecision(tx: Db, input: {
|
|||||||
inArray(issueThreadInteractions.kind, ["request_confirmation", "request_checkbox_confirmation"]),
|
inArray(issueThreadInteractions.kind, ["request_confirmation", "request_checkbox_confirmation"]),
|
||||||
sql`NOT (${issueThreadInteractions.payload} ?| ARRAY['toolAction', 'secretProposal', 'connectionAuthorization'])`,
|
sql`NOT (${issueThreadInteractions.payload} ?| ARRAY['toolAction', 'secretProposal', 'connectionAuthorization'])`,
|
||||||
sql`COALESCE(${issueThreadInteractions.payload}->'target'->>'type', '') <> 'custom'`,
|
sql`COALESCE(${issueThreadInteractions.payload}->'target'->>'type', '') <> 'custom'`,
|
||||||
|
planOnly ? sql`${issueThreadInteractions.payload}->'target'->>'type' = 'issue_document'
|
||||||
|
AND ${issueThreadInteractions.payload}->'target'->>'key' = 'plan'` : undefined,
|
||||||
)).orderBy(desc(issueThreadInteractions.createdAt), desc(issueThreadInteractions.id)).limit(1).for("share");
|
)).orderBy(desc(issueThreadInteractions.createdAt), desc(issueThreadInteractions.id)).limit(1).for("share");
|
||||||
if (!proposal || (proposal.status !== "accepted" && proposal.status !== "rejected") ||
|
return proposal;
|
||||||
!input.outcomes.includes(proposal.status) || !proposal.resolvedAt) return null;
|
}
|
||||||
const checkbox = proposal.kind === "request_checkbox_confirmation";
|
|
||||||
const payload = checkbox ? requestCheckboxConfirmationPayloadSchema.safeParse(proposal.payload)
|
async function readHumanDecision(tx: Db, input: { companyId: string; issueId: string },
|
||||||
: requestConfirmationPayloadSchema.safeParse(proposal.payload);
|
proposal: typeof issueThreadInteractions.$inferSelect | undefined) {
|
||||||
const result = checkbox ? requestCheckboxConfirmationResultSchema.safeParse(proposal.result)
|
if (!proposal) return null;
|
||||||
: requestConfirmationResultSchema.safeParse(proposal.result);
|
const decision = readPersistedConfirmationDecision(proposal);
|
||||||
if (!payload.success || !result.success || result.data.outcome !== proposal.status) return null;
|
if (!decision) return null;
|
||||||
if (checkbox && proposal.status === "accepted") {
|
|
||||||
const selected = object(result.data).selectedOptionIds;
|
|
||||||
if (!("options" in payload.data) || !Array.isArray(selected) || selected.length === 0) return null;
|
|
||||||
const allowed = new Set(payload.data.options.map(option => option.id));
|
|
||||||
if (selected.some(id => typeof id !== "string" || !allowed.has(id))
|
|
||||||
|| selected.length < payload.data.minSelected
|
|
||||||
|| (payload.data.maxSelected != null && selected.length > payload.data.maxSelected)) return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (proposal.sourceRunId) {
|
if (proposal.sourceRunId) {
|
||||||
const [origin] = await tx.select(runOriginFields).from(heartbeatRuns).where(and(
|
const [origin] = await tx.select(runOriginFields).from(heartbeatRuns).where(and(
|
||||||
@@ -139,7 +127,7 @@ async function readOnboardingFirstTaskDecision(tx: Db, input: {
|
|||||||
|
|
||||||
const directHumanDecision = Boolean(proposal.resolvedByUserId && !proposal.resolvedByAgentId && !proposal.resolvedByRunId);
|
const directHumanDecision = Boolean(proposal.resolvedByUserId && !proposal.resolvedByAgentId && !proposal.resolvedByRunId);
|
||||||
if (!directHumanDecision) {
|
if (!directHumanDecision) {
|
||||||
const commentId = result.data.commentId;
|
const commentId = decision.commentId;
|
||||||
if (!commentId || !proposal.resolvedByAgentId || !proposal.resolvedByRunId) return null;
|
if (!commentId || !proposal.resolvedByAgentId || !proposal.resolvedByRunId) return null;
|
||||||
const [comment] = await tx.select().from(issueComments).where(and(
|
const [comment] = await tx.select().from(issueComments).where(and(
|
||||||
eq(issueComments.id, commentId), eq(issueComments.companyId, input.companyId), eq(issueComments.issueId, input.issueId),
|
eq(issueComments.id, commentId), eq(issueComments.companyId, input.companyId), eq(issueComments.issueId, input.issueId),
|
||||||
@@ -155,20 +143,28 @@ async function readOnboardingFirstTaskDecision(tx: Db, input: {
|
|||||||
if (!resolver || runIssueId(resolver) !== input.issueId
|
if (!resolver || runIssueId(resolver) !== input.issueId
|
||||||
|| (resolver.responsibleUserId && resolver.responsibleUserId !== comment.authorUserId)) return null;
|
|| (resolver.responsibleUserId && resolver.responsibleUserId !== comment.authorUserId)) return null;
|
||||||
}
|
}
|
||||||
|
return decision;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Shared persisted-decision proof; creation and completion retain different authority. */
|
||||||
|
async function readOnboardingFirstTaskDecision(tx: Db, input: {
|
||||||
|
companyId: string; issueId: string; outcomes: Array<"accepted" | "rejected">;
|
||||||
|
}): Promise<"accepted" | "rejected" | null> {
|
||||||
|
const decision = await readHumanDecision(tx, input, await readLatestProposal(tx, input));
|
||||||
|
if (!decision || !input.outcomes.includes(decision.outcome)) return null;
|
||||||
|
|
||||||
// Refusal permits acknowledgement and closure, never execution. An agent-only
|
// Refusal permits acknowledgement and closure, never execution. An agent-only
|
||||||
// plan edit cannot turn that human refusal into permission to execute.
|
// plan edit cannot turn that human refusal into permission to execute.
|
||||||
if (proposal.status === "rejected") return "rejected";
|
if (decision.outcome === "rejected") return "rejected";
|
||||||
|
|
||||||
const target = payload.data.target;
|
const planDecision = !decision.target
|
||||||
|
? await readHumanDecision(tx, input, await readLatestProposal(tx, input, true)) : null;
|
||||||
const [plan] = await tx.select({ documentId: documents.id, revisionId: documents.latestRevisionId })
|
const [plan] = await tx.select({ documentId: documents.id, revisionId: documents.latestRevisionId })
|
||||||
.from(issueDocuments).innerJoin(documents, eq(documents.id, issueDocuments.documentId)).where(and(
|
.from(issueDocuments).innerJoin(documents, eq(documents.id, issueDocuments.documentId)).where(and(
|
||||||
eq(issueDocuments.companyId, input.companyId), eq(documents.companyId, input.companyId),
|
eq(issueDocuments.companyId, input.companyId), eq(documents.companyId, input.companyId),
|
||||||
eq(issueDocuments.issueId, input.issueId), eq(issueDocuments.key, "plan"),
|
eq(issueDocuments.issueId, input.issueId), eq(issueDocuments.key, "plan"),
|
||||||
)).for("share", { of: documents });
|
)).for("share", { of: documents });
|
||||||
// Document writers do not all lock the issue, so hold the document row too.
|
const target = decision.target ?? (plan && planDecision?.outcome === "accepted" ? planDecision.target : null);
|
||||||
// A saved plan must be accepted by revision; an untargeted card cannot stand
|
|
||||||
// in for approval of a different or subsequently edited document.
|
|
||||||
if (plan || target) {
|
if (plan || target) {
|
||||||
if (!target || target.type !== "issue_document" || target.key !== "plan"
|
if (!target || target.type !== "issue_document" || target.key !== "plan"
|
||||||
|| (target.issueId && target.issueId !== input.issueId) || !plan || plan.revisionId !== target.revisionId
|
|| (target.issueId && target.issueId !== input.issueId) || !plan || plan.revisionId !== target.revisionId
|
||||||
|
|||||||
Reference in new issue
Block a user