Allow a new human intent to request tool review after a decline (#18742)

## Thinking Path

> - Paperclip manages work for companies of AI agents.
> - App writes can require a human review for each invocation.
> - Decline and Always allow are separate controls.
> - The gateway currently reuses a declined action across later human
requests with the same arguments.
> - A new request can therefore receive the old decline instead of a new
review.
> - This change uses the saved run and human comment to distinguish a
new intent.
> - The new intent still follows the current policy and approval gates.

## Linked Issues or Issue Description

**What happened?**

After a person declines an Ask first app write, a later request from
that person for the same write receives `action_declined`. The gateway
matches company, task, agent, tool and arguments across runs. It does
not distinguish the later human request.

**Expected behavior**

A distinct later request from the same responsible person can open a new
Ask first review. It must not execute the write or reuse approval. A
retry of the declined intent, including its rejection continuation, must
remain declined.

**Steps to reproduce**

1. Ask an agent to perform an app write that requires approval.
2. Decline the proposed action.
3. Send a new request for the same action.
4. Observe that the gateway reuses the old rejection and creates no new
pending review.

**Paperclip version or commit**

The behavior is present on master `7b8aa60b6d` and in a retained native
journey. The focused parent control reproduces the matching error. No
related open fix was found in the scoped PR search.

## What Changed

- Only a rejected match can be released for a later human intent.
- Require the active run, company, task, agent and responsible person to
match the saved direct-human comment. Reject derived, deleted, untrusted
or system-authored input and invalid chronology.
- Preserve all other replay states, signed argument checks, current
policy, unresolved-effect guards, approval and execution owners.
- Document the distinction between decline and a standing policy change.

## Verification

- Actual matching-function control: parent fails; this change passes.
The control uses the unchanged owner body with synthetic database rows.
It does not claim real PostgreSQL or provider verification.
- 52 focused pure controls pass. These cover same-intent retries,
rejection continuations, foreign scope and actor, invalid authorship,
chronology and other action states.
- TypeScript 7.0.2 checks pass for the new helper and its owning tests.
- `node scripts/check-maps.mjs` and `git diff --check` pass.
- The existing gateway service suite has a new signed-action integration
test. It covers ordinary and reopened human requests. It requires the
old rejection plus one new pending request, a stable retry, and no
completed call. Each leaf prints one fixed success label after its
assertions so CI can distinguish execution from a platform skip. No
local PostgreSQL, app, model or provider execution was run. Current-head
normal CI41594912212 completed with48 successful jobs and1 expected
skip. Both real PostgreSQL leaves executed and emitted their
post-assertion markers; the owning job passed. Full typecheck/build
checks passed in CI.

## Risks

This adds one indexed run/comment read only after a rejected matching
action. Missing identity or ambiguous provenance keeps the old
rejection. Legacy and external comments without direct-human provenance
do not reset it. A new human intent is not consent to execute. This core
PR remains unmerged. Candidate native journey qualification is separate;
the original failed journey is retained.

## Model Used

OpenAI `gpt-6.1-sol` with reasoning, source inspection and tool use. The
host did not expose the 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
#123` / `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:
DottaandPaperclip authored and GitHub committed 2026-11-03 06:28:36 -06:00
1 parent 8bd45b5b30
commit a2a6129ed5
6 files changed
+223 -1

No files matched your search

+1 -1
View File
@@ -313,7 +313,7 @@ Gaps: Agent invocation is denied by the stated policy. The authorized human or o
API: `present` — `POST /api/tool-gateway/action-requests/{id}/decline`; JSON REST.
Agent access: `restricted` — existing human setup or approval surface; agent can identify the saved setup and inspect the result.
Policy: Human consent, identity confirmation, action approval, or trusted-host administration. This endpoint does not delegate human identity or self-approval to an agent.
Policy: Human consent, identity confirmation, action approval, or trusted-host administration. This endpoint does not delegate human identity or self-approval to an agent. A distinct later direct-human comment, bound to the active source run and the same responsible person, can ask for a new review after that person's rejection. It grants no execution authority. Same-intent retries and rejection continuations retain the rejection; all current policy and approval checks still apply.
Evidence: `source-only` — [route](../../server/src/routes/tool-gateway.ts); reviewed in the working tree based on the source commit above.
Verification: `not-run` — individual operation not executed in this audit; shared authorization and representative family flows are exercised by the authenticated integration suite. Follow the route schema, inspect persistence/audit, and assert owner, scope and revoked-authority denial.
Gaps: Agent invocation is denied by the stated policy. The authorized human or operator must perform this step; this entry does not claim delegated execution.
+7
View File
@@ -20,6 +20,13 @@ Agent access: [companion](../agent-access-map/app-permissions.md).
- `test-action`: `app-permissions/test-action` — choose an actor and inputs and inspect a real action result.
- `account-maintenance`: `app-permissions/account-maintenance` — inspect accounts, reconnect/revoke, and review connection activity.
Declining an Ask first action declines that invocation. It does not change the
connection's policy. A later, distinct request from the same responsible person
can open a new review for the same action and arguments. The agent must obtain
that new approval before execution. Retries of the declined intent and its
rejection continuation remain declined. Missing or untrusted human provenance
does not open another review.
Changing the connection's agent list preserves profile assignments for agents
that remain installed. Saving the list must not restore a removed app-profile
assignment. New installations retain the current app-profile defaults.
@@ -2725,6 +2725,48 @@ describeEmbeddedPostgres("tool gateway service", () => {
})).rejects.toMatchObject({ reasonCode: "action_declined" });
});
it.each(["issue.comment", "issue.comment.reopen"])("opens a fresh review only for a later trusted human intent after a decline (%s)", async source => {
const { company, agent, issue, run } = await createRunFixture(db);
const human = "board-user";
await db.insert(companyMemberships).values({ companyId: company.id, principalType: "user", principalId: human, membershipRole: "owner" });
const [firstComment] = await db.insert(issueComments).values({ companyId: company.id, issueId: issue.id,
authorType: "user", authorUserId: human, body: "Update this note." }).returning();
await db.update(heartbeatRuns).set({ responsibleUserId: human, startedAt: new Date(),
contextSnapshot: { issueId: issue.id, source: "issue.comment", commentId: firstComment!.id } }).where(eq(heartbeatRuns.id, run.id));
await initializeRunIdentity(db, { companyId: company.id, runId: run.id, issueId: issue.id,
responsibleUserId: human, messageIds: [firstComment!.id], cause: "instruction" });
await db.insert(toolPolicies).values(reviewNotePolicyRow(company));
const gateway = createTestToolGatewayService(db);
const original = await gateway.createSession(runSessionInput(company, agent, run));
const call = { tool: "mcp-remote-fixture:update_note", parameters: { noteId: "n1", body: "same proposed write" } };
await expect(gateway.executeTool({ ...call, sessionToken: original.token })).rejects.toMatchObject({ reasonCode: "approval_required" });
const [first] = await db.select().from(toolActionRequests).where(eq(toolActionRequests.companyId, company.id));
await gateway.declineActionRequest({ companyId: company.id, actionRequestId: first!.id, actor: { userId: human } });
await expect(gateway.executeTool({ ...call, sessionToken: original.token })).rejects.toMatchObject({ reasonCode: "action_declined" });
await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, run.id));
const [nextComment] = await db.insert(issueComments).values({ companyId: company.id, issueId: issue.id,
authorType: "user", authorUserId: human, body: "Please request my review for that update again." }).returning();
// Make the relevant strict ordering explicit, without timer sleeps or a new clock.
const rejectedAt = new Date(nextComment!.createdAt.getTime() - 1);
await db.update(toolActionRequests).set({ decidedAt: rejectedAt, resolvedAt: rejectedAt }).where(eq(toolActionRequests.id, first!.id));
const [nextRun] = await db.insert(heartbeatRuns).values({ companyId: company.id, agentId: agent.id, status: "running",
responsibleUserId: human, startedAt: new Date(), invocationSource: "on_demand",
contextSnapshot: { issueId: issue.id, source, commentId: nextComment!.id } }).returning();
await initializeRunIdentity(db, { companyId: company.id, runId: nextRun!.id, issueId: issue.id,
responsibleUserId: human, messageIds: [nextComment!.id], cause: "instruction" });
const next = await gateway.createSession(runSessionInput(company, agent, nextRun!));
await expect(gateway.executeTool({ ...call, sessionToken: next.token })).rejects.toMatchObject({ reasonCode: "approval_required" });
await expect(gateway.executeTool({ ...call, sessionToken: next.token })).rejects.toMatchObject({ reasonCode: "approval_required" });
const requests = await db.select().from(toolActionRequests).where(eq(toolActionRequests.companyId, company.id));
expect(requests).toHaveLength(2);
expect(requests.find(row => row.id === first!.id)?.status).toBe("rejected");
expect(requests.find(row => row.id !== first!.id)).toMatchObject({ status: "pending", requestedByAgentId: agent.id });
const invocations = await db.select().from(toolInvocations).where(eq(toolInvocations.companyId, company.id));
expect(invocations.map(row => row.status).sort()).toEqual(["awaiting_approval", "denied"]);
expect(await db.select().from(toolAccessAuditEvents).where(and(eq(toolAccessAuditEvents.companyId, company.id), eq(toolAccessAuditEvents.action, "tool_gateway.call_completed")))).toHaveLength(0);
console.info(`Verified fresh tool review after decline: ${source}`);
});
it("expires a stale identical request and creates a fresh approval", async () => {
const { company, agent, run } = await createRunFixture(db);
await db.insert(toolPolicies).values(reviewNotePolicyRow(company));
@@ -0,0 +1,82 @@
import { afterEach, expect, it, vi } from "vitest";
import { isNewHumanIntentAfterToolRejection } from "./tool-action-replay-intent.js";
afterEach(() => vi.useRealTimers());
function fixture() {
vi.useFakeTimers(); vi.setSystemTime(10_000);
return {
session: { companyId: "company", issueId: "task", agentId: "agent", runId: "new-run", responsibleUserId: "person" },
rejection: { companyId: "company", issueId: "task", status: "rejected", requestedByAgentId: "agent",
resolvedByUserId: "person", decidedByUserId: "person", resolvedByAgentId: null, decidedByAgentId: null,
resolvedAt: new Date(2_000), decidedAt: new Date(2_000) },
invocation: { companyId: "company", issueId: "task", agentId: "agent", runId: "original-run" },
run: { id: "new-run", companyId: "company", agentId: "agent", status: "running", responsibleUserId: "person", startedAt: new Date(4_000),
issueId: "task", nativeIssueId: "task", contextSnapshot: { source: "issue.comment", issueId: "task", taskId: "task", commentId: "new-human" } },
comment: { id: "new-human", companyId: "company", issueId: "task", authorType: "user", authorUserId: "person", authorAgentId: null, createdByRunId: null,
derivedAuthorAgentId: null, derivedCreatedByRunId: null, derivedAuthorSource: null, deletedAt: null, sourceTrust: null, presentation: null,
createdAt: new Date(3_000), body: "Please try that update again and let me review it." },
} satisfies Parameters<typeof isNewHumanIntentAfterToolRejection>[0];
}
it("distinguishes a new authenticated human request after rejection without approving it", () => {
const f = fixture(); expect(isNewHumanIntentAfterToolRejection(f)).toBe(true);
expect(f.rejection.status).toBe("rejected");
});
it("accepts the ordinary direct-human reopen source with the same persisted intent checks", () => {
const f = fixture(); f.run.contextSnapshot.source = "issue.comment.reopen";
expect(isNewHumanIntentAfterToolRejection(f)).toBe(true);
});
it.each(["automatic", "derived", "forged comment"])("does not treat %s reopen input as a new human intent", fault => {
const f = fixture(); f.run.contextSnapshot.source = "issue.comment.reopen";
if (fault === "automatic") f.run.contextSnapshot.source = "automation.issue.comment.reopen";
else if (fault === "derived") Object.assign(f.comment, { derivedCreatedByRunId: "agent-run" });
else f.run.contextSnapshot.commentId = "unrelated-human-comment";
expect(isNewHumanIntentAfterToolRejection(f)).toBe(false);
});
const invalid: Array<[string, (f: ReturnType<typeof fixture>) => void]> = [
["same intent retry", f => { f.comment.createdAt = new Date(1_000); }],
["same run retry", f => { f.invocation.runId = f.run.id; }],
["rejection continuation", f => { f.run.contextSnapshot.source = "tool_action_review"; }],
["automatic continuation", f => { f.run.contextSnapshot.source = "chat_task_completed"; }],
["foreign session company", f => { f.session.companyId = "foreign"; }],
["foreign request company", f => { f.rejection.companyId = "foreign"; }],
["foreign request task", f => { f.rejection.issueId = "foreign"; }],
["foreign invocation actor", f => { f.invocation.agentId = "foreign"; }],
["foreign invocation task", f => { f.invocation.issueId = "foreign"; }],
["foreign invocation company", f => { f.invocation.companyId = "foreign"; }],
["foreign run company", f => { f.run.companyId = "foreign"; }],
["foreign run actor", f => { f.run.agentId = "foreign"; }],
["stopped run", f => { f.run.status = "cancelled"; }],
["foreign context source", f => { f.run.contextSnapshot.issueId = "foreign"; }],
["contradictory task binding", f => { f.run.contextSnapshot.taskId = "foreign"; }],
["foreign native task", f => { f.run.nativeIssueId = "foreign"; }],
["foreign stored run task", f => { f.run.issueId = "foreign"; }],
["wrong canonical comment", f => { f.run.contextSnapshot.commentId = "old-human"; }],
["foreign comment task", f => { f.comment.issueId = "foreign"; }],
["foreign comment company", f => { f.comment.companyId = "foreign"; }],
["another human", f => { f.comment.authorUserId = "another-person"; }],
["wrong responsible user", f => { f.run.responsibleUserId = "another-person"; }],
["another person's rejection", f => { f.rejection.decidedByUserId = "another-person"; }],
["unknown rejection author", f => { f.rejection.decidedByUserId = ""; f.rejection.resolvedByUserId = ""; }],
["agent-attributed comment", f => { Object.assign(f.comment, { authorAgentId: "agent" }); }],
["agent-created comment", f => { Object.assign(f.comment, { createdByRunId: "old-run" }); }],
["derived agent comment", f => { Object.assign(f.comment, { derivedAuthorAgentId: "agent" }); }],
["derived run comment", f => { Object.assign(f.comment, { derivedCreatedByRunId: "run" }); }],
["derived attribution", f => { Object.assign(f.comment, { derivedAuthorSource: "run_log" }); }],
["deleted human comment", f => { Object.assign(f.comment, { deletedAt: new Date(5_000) }); }],
["untrusted external source", f => { Object.assign(f.comment, { sourceTrust: { disposition: "quarantined" } }); }],
["system notice", f => { Object.assign(f.comment, { presentation: { kind: "system_notice" } }); }],
["non-user author", f => { f.comment.authorType = "agent"; }],
["empty comment", f => { f.comment.body = " "; }],
["new-session command", f => { f.comment.body = "/new"; }],
["equal rejection time", f => { f.comment.createdAt = new Date(2_000); }],
["comment after run started", f => { f.comment.createdAt = new Date(5_000); }],
["future run", f => { f.run.startedAt = new Date(11_000); }],
["invalid decision time", f => { f.rejection.decidedAt = new Date(NaN); }],
["invalid comment time", f => { f.comment.createdAt = new Date(NaN); }],
["invalid start time", f => { f.run.startedAt = new Date(NaN); }],
];
it.each(invalid)("preserves the prior rejection for %s", (_name, mutate) => {
const f = fixture(); mutate(f); expect(isNewHumanIntentAfterToolRejection(f)).toBe(false);
});
it.each(["pending", "approved", "executing", "executed", "failed", "cancelled"])("never bypasses %s action state", status => {
const f = fixture(); f.rejection.status = status; expect(isNewHumanIntentAfterToolRejection(f)).toBe(false);
});
@@ -0,0 +1,60 @@
/** A fresh human request may ask for another review, never inherit approval.
* These values come only from persisted run/comment rows, not tool arguments. */
interface ReplaySession {
companyId: string; agentId: string | null; issueId: string | null; runId: string | null;
responsibleUserId?: string | null;
}
interface ReplayRun {
id: string; companyId: string; agentId: string; status: string; responsibleUserId: string | null;
startedAt: Date | null; issueId?: string | null; nativeIssueId?: string | null;
contextSnapshot: Record<string, unknown> | null;
}
interface HumanComment {
id: string; companyId: string; issueId: string; authorType: string | null;
authorUserId: string | null; authorAgentId: string | null; createdByRunId: string | null;
derivedAuthorAgentId: string | null; derivedCreatedByRunId: string | null; derivedAuthorSource: string | null;
deletedAt: Date | null; sourceTrust: unknown; presentation: { kind?: string } | null;
createdAt: Date; body: string;
}
interface RejectedAction {
companyId: string; issueId: string | null; status: string; requestedByAgentId: string | null;
resolvedByUserId: string | null; decidedByUserId: string | null;
resolvedByAgentId: string | null; decidedByAgentId: string | null;
resolvedAt: Date | null; decidedAt: Date | null;
}
interface Invocation {
companyId: string; issueId: string | null; agentId: string | null; runId: string | null;
}
export function isNewHumanIntentAfterToolRejection(input: {
session: ReplaySession; rejection: RejectedAction; invocation: Invocation;
run: ReplayRun; comment: HumanComment;
}): boolean {
const { session, rejection, invocation, run, comment } = input;
if (rejection.status !== "rejected" || !session.runId || !session.issueId || !session.agentId || !session.responsibleUserId
|| rejection.companyId !== session.companyId || rejection.issueId !== session.issueId || rejection.requestedByAgentId !== session.agentId
|| invocation.companyId !== session.companyId || invocation.issueId !== session.issueId || invocation.agentId !== session.agentId
|| !invocation.runId || invocation.runId === session.runId
|| run.id !== session.runId || run.companyId !== session.companyId || run.agentId !== session.agentId
|| run.status !== "running" || run.responsibleUserId !== session.responsibleUserId) return false;
const context = run.contextSnapshot;
if (!context || (context.source !== "issue.comment" && context.source !== "issue.comment.reopen") || context.commentId !== comment.id
|| context.issueId !== session.issueId
|| (context.taskId != null && context.taskId !== session.issueId)
|| (run.issueId != null && run.issueId !== session.issueId)
|| (run.nativeIssueId != null && run.nativeIssueId !== session.issueId)) return false;
if (comment.companyId !== session.companyId || comment.issueId !== session.issueId
|| comment.authorType !== "user" || comment.authorUserId !== session.responsibleUserId
|| comment.authorAgentId || comment.createdByRunId || comment.derivedAuthorAgentId || comment.derivedCreatedByRunId
|| comment.derivedAuthorSource || comment.deletedAt || comment.sourceTrust != null
|| comment.presentation?.kind === "system_notice" || !comment.body.trim() || comment.body.trim() === "/new") return false;
// A later request by a different person cannot override this person's decline.
const deciders = [rejection.decidedByUserId, rejection.resolvedByUserId].filter(value => value != null);
if (!deciders.length || deciders.some(value => value !== session.responsibleUserId)
|| rejection.resolvedByAgentId || rejection.decidedByAgentId) return false;
const decisions = [rejection.decidedAt, rejection.resolvedAt].filter((value): value is Date => value != null);
const times = decisions.map(value => value.getTime());
const requestedAt = comment.createdAt.getTime(), startedAt = run.startedAt?.getTime();
return times.length > 0 && times.every(Number.isFinite) && Number.isFinite(requestedAt) && Number.isFinite(startedAt)
&& Math.max(...times) < requestedAt && requestedAt <= startedAt! && startedAt! <= Date.now();
}
+31
View File
@@ -1,3 +1,4 @@
import { isNewHumanIntentAfterToolRejection } from "./tool-action-replay-intent.js";
import { notifyChatRunMilestones } from "./chat-run-milestone-notifications.js";
import { writeToolConnectionHealthChange } from "./tool-connection-health-notifications.js";
import { writeApprovalChange } from "./approval-work-notifications.js";
@@ -71,6 +72,7 @@ import {
documents,
heartbeatRuns,
issueApprovals,
issueComments,
issueDocuments,
issueThreadInteractions,
issues,
@@ -8597,6 +8599,35 @@ export function createToolGatewayService(
} else if (match.actionRequest.canonicalArgumentsHash !== input.argumentsHash) {
return null;
}
// Rejection applies to that intent, not every later human request with the
// same arguments. A new trusted request still passes the normal policy and
// Ask first path below; it receives no execution or approval authority here.
if (match.actionRequest.status === "rejected" && input.session.runId && input.session.responsibleUserId) {
const [intent] = await db.select({
run: {
id: heartbeatRuns.id, companyId: heartbeatRuns.companyId, agentId: heartbeatRuns.agentId,
status: heartbeatRuns.status, responsibleUserId: heartbeatRuns.responsibleUserId,
startedAt: heartbeatRuns.startedAt, issueId: heartbeatRuns.issueId, nativeIssueId: heartbeatRuns.nativeIssueId,
contextSnapshot: sql<Record<string, unknown>>`jsonb_build_object(
'source', ${heartbeatRuns.contextSnapshot}->'source',
'issueId', ${heartbeatRuns.contextSnapshot}->'issueId',
'taskId', ${heartbeatRuns.contextSnapshot}->'taskId',
'commentId', ${heartbeatRuns.contextSnapshot}->'commentId')`,
},
comment: issueComments,
}).from(heartbeatRuns).innerJoin(issueComments, and(
eq(issueComments.companyId, heartbeatRuns.companyId),
eq(issueComments.issueId, input.session.issueId),
eq(issueComments.id, sql`case when ${heartbeatRuns.contextSnapshot}->>'commentId'
~* '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$'
then (${heartbeatRuns.contextSnapshot}->>'commentId')::uuid end`),
)).where(and(
eq(heartbeatRuns.id, input.session.runId), eq(heartbeatRuns.companyId, input.session.companyId),
eq(heartbeatRuns.agentId, input.session.agentId),
)).limit(1);
if (intent && isNewHumanIntentAfterToolRejection({ session: input.session, rejection: match.actionRequest,
invocation: match.invocation, ...intent })) return null;
}
if (match.actionRequest.status === "failed" && match.invocation.errorCode !== "provider_interaction_required") return null;
// The gateway builds an ask-first request in two steps inside one call: it
// inserts the row with a null signature and a null expiry, then signs the