Files
PaperClipAI/ui/src/lib/interaction-resolution-error.test.ts
T
DottaandPaperclip c67909cb0f fix: keep tool approvals valid through OAuth refresh and delayed review (#20340)
## Thinking Path

> - Paperclip lets people manage agent work and approve app actions.
> - An approval must bind the action that the person reviewed.
> - OAuth access tokens can expire while that person is away.
> - The old target snapshot treated each token refresh as a new
authorization. It also imposed a one-hour review deadline.
> - This pull request separates token renewal from authorization
identity and keeps the human decision durable.
> - Current access, the reviewed arguments, meaningful target changes,
task closure, and execution receipts still control dispatch.
> - People can approve later without repeating a decision after routine
connection maintenance.

## Linked Issues or Issue Description

**What happened?**

A pending app action returned `approved_tool_target_changed` after its
OAuth tokens refreshed. The tool and connection settings were unchanged.
The same card also expired after one hour. A sign-in problem could
discard an accepted decision before provider dispatch.

**Expected behavior**

The original card remains reviewable after routine OAuth refresh and a
server restart. A later approval runs the exact reviewed action once. A
connection repair preserves the decision while Paperclip verifies the
same authorization. Meaningful changes require a new review.

**Steps to reproduce**

1. Connect an OAuth MCP app and configure an action to require approval.
2. Request the action, then refresh its access and refresh tokens before
approving.
3. Approve the original card. Before this change, it returns HTTP 409
even when the authorization is unchanged.
4. Wait more than one hour. Before this change, the card expires without
a task-specific deadline.

**Paperclip version or commit**

Reproduced from master at `76254ead2229`. The final branch is rebased
onto current master.

## What Changed

- Add versioned approval target comparison. New snapshots bind
credential identity epochs, consent, permissions, ownership, connection
configuration, tool contracts, and credential references. Authenticated
OAuth refresh preserves identity. Manual credential replacement does
not.
- Remove the implicit one-hour deadline from ordinary app action
reviews. Keep explicit deadlines and terminal-task expiry. Synchronize
the nullable interaction deadline in shared contracts and UI.
- Keep accepted, undispatched actions when sign-in is required. Show a
saved-approval repair state. Resume on committed connection changes
after target and access revalidation, without an idle retry loop.
- Preserve the existing execution claim, result receipt, and
uncertain-outcome no-replay rules.
- Explain meaningful target changes without inviting the person to retry
the stale card.
- Add a production task-thread Storybook example and document
compatibility and recovery behavior.

## Verification

- Red: production-refresher invalid-grant handling and formal-approval
withdrawal during a sign-in wait also reproduced as failing regressions
before their fixes.
- Red: before implementation, the local OAuth HTTP fixture failed for
both public and confidential clients with `approved_tool_target_changed`
after token refresh.
- Green: the extended fixture passes for public and confidential clients
with personal and shared grants. It covers actual local HTTP token
exchanges, multiple refreshes, a rebuilt gateway, an ended originating
run, approval after 25 hours, duplicate approval, preserved sharing
arguments, sign-in waiting, and event-driven repair. The gateways use
the production OAuth refresher; an invalid refresh preserves the
accepted decision before provider dispatch. Omitted-scope responses
cover equivalent scope-reporting changes. The provider is an isolated
local fixture, not a deployed Notebook service.
- Negative controls refuse changed consent, scopes, tool definition,
destination, and manual credential replacement before provider
execution.
- Existing gateway service, connected-tool acceptance, secret owner,
secret service, unsigned-review queue, and connection-health suites
passed. Explicit-deadline fixtures still prove expiry behavior,
including deadlines on accepted sign-in waits. A withdrawn formal
approval cancels a waiting action before connection recovery.
- Approval card, task-chat, error-copy and target-comparison tests
passed. Server and UI typechecks passed. Token gates and Storybook
catalog checks passed.
- Inspected the new saved-approval repair card in local Storybook using
the production task thread. It shows a reconnect link and no second
approval button or false running state.
- `pnpm verify --explain` returned exit 2 and requested broad CI
coverage. [Full
CI](https://github.com/paperclipai/paperclip/actions/runs/42677855507)
passed on `eb4f1c7caa9ee6e802c314ffc5eadec475a5de0c`, including build,
typecheck/release registry, general and serialized tests, and all eight
browser shards.
- Greptile reviewed the same final head at 5/5. Both findings from the
first review are fixed and their threads resolved; no open follow-ups
remain.

## Risks

- No database migration is needed. The new comparison reuses the vault
identity epoch. Only authenticated OAuth refresh code can preserve that
epoch; HTTP credential-write schemas cannot select that behavior.
- Earlier signed requests keep their original exact-version checks and
saved deadlines. Already-expired cards are not revived. A historical
card whose credentials already changed needs a fresh review.
- A new consent, account, permission set, destination, or manually
replaced credential must not inherit the old authorization. Reconnection
continues automatically only when revalidation proves the same saved
authorization.
- The interaction enrichment now permits a null deadline. Existing
string deadlines remain valid. The web client and shared validator
change together.
- This PR does not deploy to staging or perform a live Notebook
publication.

## Model Used

OpenAI Codex, GPT-6 family, with repository tools, code execution, and
browser inspection. The runtime does not expose the exact model ID,
context-window limit, or reasoning-mode label; those values are not
asserted.

## 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>
2026-11-09 13:46:28 -06:00

191 lines
7.6 KiB
TypeScript

import { describe, expect, it } from "vitest";
import { ApiError } from "../api/client";
import {
describeInteractionResolutionFailure,
interactionResolutionErrorCode,
interactionResolutionErrorMessage,
isInteractionAudienceDenial,
} from "./interaction-resolution-error";
const addressedAudience = { shortSummary: "Only CodexCoder or the board can respond", isOpen: false };
const humanOnlyAudience = { shortSummary: "Only the board can respond", isOpen: false };
const openAudience = { shortSummary: "Anyone can respond", isOpen: true };
/** Shapes an `ApiError` the way `errorHandler` serializes an `HttpError`. */
function denial(status: number, code: string, message: string) {
return new ApiError(message, status, { error: message, code, details: { code } });
}
describe("interactionResolutionErrorCode", () => {
it("reads the top-level code the API returns", () => {
expect(interactionResolutionErrorCode(denial(403, "interaction_human_only", "human-only"))).toBe(
"interaction_human_only",
);
});
it("falls back to the code nested in details", () => {
const error = new ApiError("nope", 403, { error: "nope", details: { code: "interaction_addressee_mismatch" } });
expect(interactionResolutionErrorCode(error)).toBe("interaction_addressee_mismatch");
});
it("returns null for an error with no structured body", () => {
expect(interactionResolutionErrorCode(new Error("network down"))).toBeNull();
});
});
describe("isInteractionAudienceDenial", () => {
it("recognizes every audience denial code the evaluator can return", () => {
for (const code of [
"interaction_human_only",
"interaction_creator_excluded",
"interaction_addressee_mismatch",
"interaction_governed_action_denied",
"interaction_run_attribution_required",
"interaction_scope_denied",
]) {
expect(isInteractionAudienceDenial(denial(403, code, "denied"))).toBe(true);
}
});
it("treats an uncoded 403 as an authorization refusal", () => {
expect(isInteractionAudienceDenial(new ApiError("Forbidden", 403, { error: "Forbidden" }))).toBe(true);
});
it("does not treat a server fault or a settled card as an audience denial", () => {
expect(isInteractionAudienceDenial(new ApiError("boom", 500, { error: "boom" }))).toBe(false);
expect(isInteractionAudienceDenial(denial(409, "interaction_already_resolved", "already resolved"))).toBe(false);
});
});
describe("describeInteractionResolutionFailure", () => {
it("does not tell a person to retry a stale tool approval", () => {
const message = "The connected account, credentials, or permissions changed. A new review is required.";
expect(describeInteractionResolutionFailure(denial(409, "approved_tool_target_changed", message)))
.toMatchObject({ kind: "settled", message });
});
it("keeps the server denial reason and names who can respond", () => {
const failure = describeInteractionResolutionFailure(
denial(403, "interaction_human_only", "This issue-thread interaction is human-only"),
humanOnlyAudience,
);
expect(failure.kind).toBe("audience_denied");
expect(failure.code).toBe("interaction_human_only");
expect(failure.message).toBe(
"This issue-thread interaction is human-only. Only the board can respond.",
);
// A policy denial is permanent: never invite a retry that will fail again.
expect(failure.message).not.toMatch(/try again/i);
});
it("names the addressed responder for an addressee mismatch", () => {
expect(
interactionResolutionErrorMessage(
denial(
403,
"interaction_addressee_mismatch",
"Only the addressed agent or an authorized human may resolve this issue-thread interaction",
),
addressedAudience,
),
).toContain("Only CodexCoder or the board can respond.");
});
it("still explains a denial when no audience is known", () => {
const failure = describeInteractionResolutionFailure(
denial(403, "interaction_creator_excluded", "This issue-thread interaction requires a resolver other than its creator"),
null,
);
expect(failure.message).toBe(
"This issue-thread interaction requires a resolver other than its creator.",
);
expect(failure.message).not.toMatch(/try again/i);
});
// PAP-17289: the responder clause explains a denial, so it must never
// contradict it. An open card has no narrower audience to point at.
it("does not append the open-default clause to a refusal it would refute", () => {
const failure = describeInteractionResolutionFailure(
denial(
403,
"interaction_addressee_mismatch",
"Only the addressed agent can resolve this interaction",
),
openAudience,
);
expect(failure.kind).toBe("audience_denied");
expect(failure.message).toBe("Only the addressed agent can resolve this interaction.");
expect(failure.message).not.toMatch(/anyone can respond/i);
});
it("keeps the server's own text for an uncoded 403 instead of inventing a cause", () => {
const failure = describeInteractionResolutionFailure(
new ApiError("Forbidden", 403, { error: "Forbidden" }),
openAudience,
);
expect(failure).toMatchObject({ kind: "audience_denied", code: null, message: "Forbidden." });
expect(failure.message).not.toMatch(/resolver audience/i);
expect(failure.message).not.toMatch(/try again/i);
});
it("restates the refusal without claiming an audience cause when a 403 says nothing", () => {
const failure = describeInteractionResolutionFailure(new ApiError("", 403, null), null);
expect(failure.message).toBe("You do not have permission to respond to this card.");
expect(failure.message).not.toMatch(/resolver audience/i);
});
it("does not ask for a retry when the card has already moved on", () => {
const failure = describeInteractionResolutionFailure(
denial(409, "interaction_superseded", "This confirmation was superseded by a newer request"),
humanOnlyAudience,
);
expect(failure.kind).toBe("settled");
expect(failure.message).toBe("This confirmation was superseded by a newer request.");
expect(failure.message).not.toMatch(/try again/i);
});
it("keeps the retry prompt for a genuinely transient failure", () => {
expect(
describeInteractionResolutionFailure(new ApiError("Request failed: 503", 503, null), humanOnlyAudience),
).toMatchObject({
kind: "transient",
message: "Paperclip is temporarily unavailable. Please try again in a moment.",
});
});
it("never shows a raw gateway code", () => {
const failure = describeInteractionResolutionFailure(
new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" }),
humanOnlyAudience,
);
expect(failure.kind).toBe("transient");
expect(failure.code).toBeNull();
expect(failure.message).not.toMatch(/tenant_app_unavailable/);
expect(failure.message).toMatch(/try again/i);
});
it("does not echo a raw code for a non-transient failure", () => {
expect(
interactionResolutionErrorMessage(new ApiError("internal_error", 500, { error: "internal_error" }), null),
).toBe("Couldn't submit. Try again.");
});
it("falls back to generic copy when nothing explains the failure", () => {
expect(describeInteractionResolutionFailure(undefined, null)).toMatchObject({
kind: "transient",
message: "Couldn't submit. Try again.",
});
});
it("leaves punctuation the server already wrote alone", () => {
expect(
interactionResolutionErrorMessage(denial(403, "interaction_human_only", "Agents cannot resolve this."), null),
).toBe("Agents cannot resolve this.");
});
});