mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents ask users to approve proposals and plans with confirmation cards, and users often answer in the conversation > - `resolve-from-comment` lets the responding agent record that verified reply on the card > - Native agents often create their own proposal or plan confirmation as `human_only`, because the `request_human_input` schema offers only that value > - `resolve-from-comment` then refuses with `interaction_human_only`, and the plan and onboarding gates keep the task in `in_review` or `blocked`, although the user explicitly approved > - This pull request adds one narrow relay for that case and keeps every other human-only gate on its card > - The benefit is that an explicit conversational approval works again, and governed, company-capped and quality-review approvals keep their existing controls ## Linked Issues or Issue Description Refs #19073 **What happened?** TestOps launch https://paperclip.testops.cloud/launch/2694 (GitHub run https://github.com/paperclipai/paperclip/actions/runs/41915965118) failed three Product regression cases for the same reason: `continuation/revision-preserves-approval`, `first-task/clarify-propose-accept` and `first-task/revise-accept`. In each case the agent created a `request_confirmation` with `resolverPolicy: human_only` (`effectiveResolverPolicySource: requested`). The user replied "I approve…" or "Yes, I accept that proposal". The agent's `resolve-from-comment` call returned 403 `interaction_human_only`. The task stayed `in_review` (plan gate) or became `blocked` (onboarding decision gate). **Expected behavior** When the agent itself asked for a human-only answer, the verified latest reply of the user it serves is that human answer. The agent can record it. Company-capped, governed-action, issue-review and quality-review gates still require the human to use the card. **Steps to reproduce** 1. A native agent calls `request_human_input` with `interactionKind: confirmation` and `resolverPolicy: human_only`. 2. The conversation user posts an explicit approval comment. 3. The agent calls `POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment`. Before this change it gets 403. ## What Changed - `issue-thread-interaction-resolution.ts`: the agent actor gets an optional `relayedHumanUserId`. A human-only decision is allowed as `allow_relayed_human` only when all of these are true: - the stored policy is `human_only`, its source is `requested` and its provenance is `explicit` - the resolving agent created the card (`createdByAgentId` matches) - no governed action applies - no additional restriction (issue review) applies - any named addressee user is the relayed user Company caps, the `governed_action` source and legacy rows with an unknown source are still denied. - `confirmation-comment-resolution.ts`: the service sets `relayedHumanUserId` only after its existing comment checks pass: user-authored, not agent-attributed, posted after the card, the latest user message, the same conversation session. It also requires that the author is the card's addressee, the conversation user or the run's responsible user, that the card is not a quality review, and that the company's current cap for the kind is not `human_only` (a tied cap keeps the source `requested`). The same actor goes into the locked accept/reject re-check. The activity entry adds `resolverPolicy: "human_only"` and `resolution: "relayed_human_decision"`. - `routes/issues.ts`: the route pre-check passes the candidate comment author. The service still re-verifies everything under its locks. - `issue-thread-interactions.ts`: carries the relay field into the service's own audience re-check. - Docs: the first-task skill and `api-reference.md` describe the narrow relay. The guidance still says not to add `human_only` to ordinary proposals. ## Verification - `server/src/__tests__/confirmation-comment-resolution.test.ts` (embedded Postgres): 72 pass. - Green: relay through a conversation user and through a run's responsible user. The card is `accepted`, `resolvedByAgentId` and `resolvedByRunId` are set, `resolvedByUserId` stays null, and the audit records `relayed_human_decision`. - Negative controls, each with the card still pending and no audit: agent-authored comment, earlier comment, wrong user, unbound issue/run, `company_cap` source, `governed_action` source, `qualityReview`, `toolAction`, `connectionAuthorization`, issue-review `human_only` restriction, another addressed user, human-created card, another agent's card, inherited company default, and a real capped card created through `svc.create` with a tied cap. The tied-cap test fails as expected when the cap is removed. - Existing negative controls are unchanged, including `human_only` without a bound user and the "rechecks narrowed permissions" retry. - Red check: with the three service files reverted to `master`, the two new green tests fail and the negative controls pass. - `server/src/services/issue-thread-interaction-resolution.test.ts`: 21 pass, including the pure evaluator relay rules. - `server/src/services/native-runtime/confirmation-comment-api.integration.test.ts`: 11 pass. The real route accepts the relayed reply and refuses another user's comment with 403. - Also pass: `issue-thread-interaction-routes`, `onboarding-first-task-admission`, `paperclip-runner-tool-authority`, `runner-api`, `onboarding-first-task-assets` and `company-skills-service` (2,474 tests in total). - Changed files typecheck cleanly. - Live verification: Product regression single-case runs of the three cases are pending after merge. ## Risks - This touches an approval gate. The relay is limited to cards whose creator chose `human_only`. All existing reply-evidence checks still apply, plus a bound-user requirement. Company caps, governed actions, quality reviews, issue-review restrictions and named addressees still deny the relay. - Attribution does not change: the resolver is still the agent and run. The human's comment is kept as `result.commentId` and in the audit. - Legacy rows with a null policy source are not relayed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use and code execution. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
402 lines
14 KiB
TypeScript
402 lines
14 KiB
TypeScript
import type {
|
|
IssueThreadInteractionCanonicalResolverPolicy,
|
|
IssueThreadInteractionResolverPolicy,
|
|
} from "@paperclipai/shared";
|
|
import { normalizeIssueThreadInteractionResolverPolicy } from "@paperclipai/shared";
|
|
import { HttpError } from "../errors.js";
|
|
|
|
export const ISSUE_THREAD_INTERACTION_RESOLUTION_DENIAL_CODES = [
|
|
"interaction_not_found",
|
|
"interaction_run_attribution_required",
|
|
"interaction_scope_denied",
|
|
"interaction_human_only",
|
|
"interaction_creator_excluded",
|
|
"interaction_addressee_mismatch",
|
|
"interaction_stale_target",
|
|
"interaction_superseded",
|
|
"interaction_already_resolved",
|
|
"interaction_issue_closed",
|
|
"interaction_governed_action_denied",
|
|
"review_policy_denied",
|
|
] as const;
|
|
|
|
export type IssueThreadInteractionResolutionDenialCode =
|
|
(typeof ISSUE_THREAD_INTERACTION_RESOLUTION_DENIAL_CODES)[number];
|
|
|
|
export type IssueThreadInteractionResolverActor =
|
|
| { type: "user"; userId: string }
|
|
| {
|
|
type: "agent";
|
|
agentId: string | null | undefined;
|
|
runId: string | null | undefined;
|
|
/**
|
|
* Set only by conversational confirmation resolution after it verified
|
|
* the bound user's latest comment. It never makes the agent a human.
|
|
*/
|
|
relayedHumanUserId?: string | null;
|
|
}
|
|
| { type: "system"; systemId: string };
|
|
|
|
export type IssueThreadInteractionResolverRestriction = {
|
|
policy: IssueThreadInteractionCanonicalResolverPolicy;
|
|
/**
|
|
* A server-resolved identity excluded by the binding flow in addition to the
|
|
* interaction creator. `null` means the binding requires an identity but it
|
|
* could not be resolved, so the evaluator must fail closed.
|
|
*/
|
|
excludedActor?: { type: "agent" | "user"; id: string } | null;
|
|
source?: "issue_review";
|
|
};
|
|
|
|
export type IssueThreadInteractionResolverAudienceInput = {
|
|
actor: IssueThreadInteractionResolverActor;
|
|
interaction: {
|
|
createdByAgentId?: string | null;
|
|
createdByUserId?: string | null;
|
|
sourceRunId?: string | null;
|
|
addresseeAgentId?: string | null;
|
|
addresseeUserId?: string | null;
|
|
effectiveResolverPolicy: IssueThreadInteractionResolverPolicy | string;
|
|
effectiveResolverPolicySource?: string | null;
|
|
resolverPolicyProvenance?: string | null;
|
|
};
|
|
/**
|
|
* A server-derived restriction owned by a binding flow such as issue review.
|
|
* It may only narrow the interaction's persisted effective audience.
|
|
*/
|
|
additionalRestriction?:
|
|
| IssueThreadInteractionCanonicalResolverPolicy
|
|
| IssueThreadInteractionResolverRestriction
|
|
| null;
|
|
governedAction?: boolean;
|
|
};
|
|
|
|
export type IssueThreadInteractionResolverAudienceDecision =
|
|
| {
|
|
allowed: true;
|
|
effectiveResolverPolicy: IssueThreadInteractionCanonicalResolverPolicy;
|
|
reason: "allow_anyone" | "allow_addressee" | "allow_human" | "allow_system" | "allow_human_override"
|
|
| "allow_relayed_human";
|
|
}
|
|
| {
|
|
allowed: false;
|
|
effectiveResolverPolicy: IssueThreadInteractionCanonicalResolverPolicy;
|
|
status: 403 | 422;
|
|
code: Extract<
|
|
IssueThreadInteractionResolutionDenialCode,
|
|
| "interaction_run_attribution_required"
|
|
| "interaction_human_only"
|
|
| "interaction_creator_excluded"
|
|
| "interaction_addressee_mismatch"
|
|
| "interaction_governed_action_denied"
|
|
| "review_policy_denied"
|
|
>;
|
|
message: string;
|
|
details?: Record<string, unknown>;
|
|
};
|
|
|
|
/**
|
|
* Canonicalize one stored resolver-policy value.
|
|
*
|
|
* Before provenance existed, `board_or_agents` excluded both the creator agent
|
|
* and the creating run, so a pre-migration row must stay `not_creator` rather
|
|
* than widening to canonical `anyone`. Every reader of the stored columns —
|
|
* hydration, the audience evaluator, and the attention feed — goes through here
|
|
* so none of them can disagree about a legacy row.
|
|
*/
|
|
export function canonicalizeStoredResolverPolicy(
|
|
policy: IssueThreadInteractionResolverPolicy | string,
|
|
resolverPolicyProvenance: string | null | undefined,
|
|
): IssueThreadInteractionCanonicalResolverPolicy {
|
|
if (resolverPolicyProvenance === "legacy_inherited_restriction" && policy === "board_or_agents") {
|
|
return "not_creator";
|
|
}
|
|
return normalizeIssueThreadInteractionResolverPolicy(policy as IssueThreadInteractionResolverPolicy);
|
|
}
|
|
|
|
function canonicalStoredPolicy(input: IssueThreadInteractionResolverAudienceInput["interaction"]) {
|
|
return canonicalizeStoredResolverPolicy(
|
|
input.effectiveResolverPolicy,
|
|
input.resolverPolicyProvenance,
|
|
);
|
|
}
|
|
|
|
function reviewRestrictionExcludesActor(
|
|
restriction: IssueThreadInteractionResolverRestriction | null | undefined,
|
|
actor: { type: "agent" | "user"; id: string },
|
|
) {
|
|
return restriction?.policy === "not_creator"
|
|
&& restriction.source === "issue_review"
|
|
&& (
|
|
restriction.excludedActor === null
|
|
|| (
|
|
restriction.excludedActor?.type === actor.type
|
|
&& restriction.excludedActor.id === actor.id
|
|
)
|
|
);
|
|
}
|
|
|
|
function reviewPolicyDeniedDecision(
|
|
effectiveResolverPolicy: IssueThreadInteractionCanonicalResolverPolicy,
|
|
requesterKnown: boolean,
|
|
): Extract<IssueThreadInteractionResolverAudienceDecision, { allowed: false }> {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "review_policy_denied",
|
|
message: requesterKnown
|
|
? "Review policy `not_creator` requires someone other than the writer who moved the issue into `in_review` to approve or reject it."
|
|
: "Review policy `not_creator` requires a different writer, but the review requester could not be determined.",
|
|
details: {
|
|
policy: "not_creator",
|
|
allowedActor: "writer_other_than_review_requester",
|
|
remediation: requesterKnown
|
|
? "Have another writer with issue write access submit the verdict, or change reviewPolicy to `anyone`."
|
|
: "Change reviewPolicy to `anyone`, or move the issue out of and back into `in_review` to record a requester before another writer submits the verdict.",
|
|
},
|
|
};
|
|
}
|
|
|
|
/**
|
|
* An agent may record a human-only confirmation only as the relay of a verified
|
|
* human decision, and only on a card it created with an explicit human_only
|
|
* request. Company caps (checked again at resolution by the caller), governed
|
|
* actions and binding-flow restrictions still require a direct human resolution.
|
|
*/
|
|
function relayedHumanDecisionAllowed(
|
|
input: IssueThreadInteractionResolverAudienceInput,
|
|
persistedPolicy: IssueThreadInteractionCanonicalResolverPolicy,
|
|
additionalRestriction: IssueThreadInteractionResolverRestriction | null | undefined,
|
|
) {
|
|
if (input.actor.type !== "agent") return false;
|
|
const userId = input.actor.relayedHumanUserId;
|
|
return Boolean(userId && input.actor.agentId)
|
|
&& !input.governedAction
|
|
&& persistedPolicy === "human_only"
|
|
// Only this agent's own explicit request; a company default, cap or a
|
|
// human-created card keeps its direct human resolution.
|
|
&& input.interaction.createdByAgentId === input.actor.agentId
|
|
&& input.interaction.resolverPolicyProvenance === "explicit"
|
|
&& input.interaction.effectiveResolverPolicySource === "requested"
|
|
&& !additionalRestriction
|
|
&& (!input.interaction.addresseeUserId || input.interaction.addresseeUserId === userId);
|
|
}
|
|
|
|
/**
|
|
* The single pure audience evaluator for issue-thread interaction resolution.
|
|
* Resource access, run validation, containment, and current-target checks must
|
|
* happen before this evaluator. Mutation routes must re-run it at use time.
|
|
*/
|
|
export function evaluateIssueThreadInteractionResolverAudience(
|
|
input: IssueThreadInteractionResolverAudienceInput,
|
|
): IssueThreadInteractionResolverAudienceDecision {
|
|
const persistedPolicy = canonicalStoredPolicy(input.interaction);
|
|
const additionalRestriction = typeof input.additionalRestriction === "string"
|
|
? { policy: input.additionalRestriction }
|
|
: input.additionalRestriction;
|
|
// human_only and not_creator are independent constraints, not a linear
|
|
// severity scale: their intersection means "a human other than the
|
|
// creator." Keep both predicates even though the persisted public policy
|
|
// enum can only name one of them.
|
|
const creatorExcluded =
|
|
persistedPolicy === "not_creator"
|
|
|| additionalRestriction?.policy === "not_creator";
|
|
const humanOnly =
|
|
Boolean(input.governedAction)
|
|
|| persistedPolicy === "human_only"
|
|
|| additionalRestriction?.policy === "human_only";
|
|
const effectiveResolverPolicy: IssueThreadInteractionCanonicalResolverPolicy = humanOnly
|
|
? "human_only"
|
|
: creatorExcluded
|
|
? "not_creator"
|
|
: "anyone";
|
|
|
|
if (input.actor.type === "system") {
|
|
return { allowed: true, effectiveResolverPolicy, reason: "allow_system" };
|
|
}
|
|
|
|
if (input.actor.type === "user") {
|
|
if (
|
|
input.interaction.addresseeUserId
|
|
&& input.interaction.addresseeUserId !== input.actor.userId
|
|
) {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "interaction_addressee_mismatch",
|
|
message: "Only the addressed user may resolve this issue-thread interaction",
|
|
};
|
|
}
|
|
if (
|
|
creatorExcluded
|
|
&& input.interaction.createdByUserId === input.actor.userId
|
|
) {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "interaction_creator_excluded",
|
|
message: "This issue-thread interaction requires a resolver other than its creator",
|
|
};
|
|
}
|
|
if (reviewRestrictionExcludesActor(additionalRestriction, {
|
|
type: "user",
|
|
id: input.actor.userId,
|
|
})) {
|
|
return reviewPolicyDeniedDecision(
|
|
effectiveResolverPolicy,
|
|
additionalRestriction?.excludedActor !== null,
|
|
);
|
|
}
|
|
return {
|
|
allowed: true,
|
|
effectiveResolverPolicy,
|
|
reason: input.interaction.addresseeUserId
|
|
? "allow_addressee"
|
|
: input.interaction.addresseeAgentId
|
|
? "allow_human_override"
|
|
: "allow_human",
|
|
};
|
|
}
|
|
|
|
if (!input.actor.agentId || !input.actor.runId?.trim()) {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 422,
|
|
code: "interaction_run_attribution_required",
|
|
message: "A valid authenticated agent run is required to resolve this issue-thread interaction",
|
|
};
|
|
}
|
|
|
|
if (input.governedAction) {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "interaction_governed_action_denied",
|
|
message: "This interaction is bound to a governed action that requires independent authorization",
|
|
};
|
|
}
|
|
|
|
if (effectiveResolverPolicy === "human_only") {
|
|
if (relayedHumanDecisionAllowed(input, persistedPolicy, additionalRestriction)) {
|
|
return { allowed: true, effectiveResolverPolicy, reason: "allow_relayed_human" };
|
|
}
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "interaction_human_only",
|
|
message: "This issue-thread interaction is human-only",
|
|
};
|
|
}
|
|
|
|
if (input.interaction.addresseeUserId) {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "interaction_addressee_mismatch",
|
|
message: "This issue-thread interaction is addressed to a specific user",
|
|
};
|
|
}
|
|
|
|
if (
|
|
input.interaction.addresseeAgentId
|
|
&& input.interaction.addresseeAgentId !== input.actor.agentId
|
|
) {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "interaction_addressee_mismatch",
|
|
message: "Only the addressed agent or an authorized human may resolve this issue-thread interaction",
|
|
};
|
|
}
|
|
|
|
if (
|
|
creatorExcluded
|
|
&& (
|
|
input.interaction.createdByAgentId === input.actor.agentId
|
|
|| (
|
|
Boolean(input.interaction.sourceRunId)
|
|
&& input.interaction.sourceRunId === input.actor.runId
|
|
)
|
|
)
|
|
) {
|
|
return {
|
|
allowed: false,
|
|
effectiveResolverPolicy,
|
|
status: 403,
|
|
code: "interaction_creator_excluded",
|
|
message: "This issue-thread interaction requires a resolver other than its creator or creating run",
|
|
};
|
|
}
|
|
|
|
if (reviewRestrictionExcludesActor(additionalRestriction, {
|
|
type: "agent",
|
|
id: input.actor.agentId,
|
|
})) {
|
|
return reviewPolicyDeniedDecision(
|
|
effectiveResolverPolicy,
|
|
additionalRestriction?.excludedActor !== null,
|
|
);
|
|
}
|
|
|
|
return {
|
|
allowed: true,
|
|
effectiveResolverPolicy,
|
|
reason: input.interaction.addresseeAgentId ? "allow_addressee" : "allow_anyone",
|
|
};
|
|
}
|
|
|
|
export function issueThreadInteractionResolutionError(
|
|
status: number,
|
|
code: IssueThreadInteractionResolutionDenialCode,
|
|
message: string,
|
|
details: Record<string, unknown> = {},
|
|
) {
|
|
return new HttpError(status, message, { code, ...details });
|
|
}
|
|
|
|
export function assertIssueThreadInteractionResolverAudience(
|
|
input: IssueThreadInteractionResolverAudienceInput,
|
|
) {
|
|
const decision = evaluateIssueThreadInteractionResolverAudience(input);
|
|
if (!decision.allowed) {
|
|
throw issueThreadInteractionResolutionError(
|
|
decision.status,
|
|
decision.code,
|
|
decision.message,
|
|
{ effectiveResolverPolicy: decision.effectiveResolverPolicy, ...(decision.details ?? {}) },
|
|
);
|
|
}
|
|
return decision;
|
|
}
|
|
|
|
/**
|
|
* Resolves agent-owned attention with the same audience rules used by mutation
|
|
* routes. Run attribution is deliberately represented by the interaction's
|
|
* source run for the creator and by an opaque non-source run for other agents;
|
|
* the real run is still authenticated at mutation time.
|
|
*/
|
|
export function issueThreadInteractionAttentionAgentAllowed(input: {
|
|
agentId: string;
|
|
interaction: IssueThreadInteractionResolverAudienceInput["interaction"];
|
|
additionalRestriction?: IssueThreadInteractionResolverAudienceInput["additionalRestriction"];
|
|
governedAction?: boolean;
|
|
}) {
|
|
const attentionRunId = input.interaction.createdByAgentId === input.agentId
|
|
? input.interaction.sourceRunId ?? "attention-owner-creator-run"
|
|
: "attention-owner-non-source-run";
|
|
return evaluateIssueThreadInteractionResolverAudience({
|
|
actor: { type: "agent", agentId: input.agentId, runId: attentionRunId },
|
|
interaction: input.interaction,
|
|
additionalRestriction: input.additionalRestriction,
|
|
governedAction: input.governedAction,
|
|
}).allowed;
|
|
}
|