mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-14 17:18:22 +02:00
fix(ui): stack confirmation prompt and hide raw custom target keys (#15895)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents ask the board for decisions with `request_confirmation`
interaction cards
> - The compact task-chat card puts the prompt and the target pill in
one `justify-between` flex row
> - An agent can send a `custom` target with only a machine `key` (for
example `github:org/repo:pull:16`) and no `label`
> - The card shows that raw key as a mono pill that does not shrink, so
the card splits about 50/50 and the prompt is squeezed into the left
half
> - This pull request stacks the prompt above the pill and hides custom
targets that have no human label
> - The benefit is a readable, full-width confirmation prompt without
meaningless identifier pills
## Linked Issues or Issue Description
**What happened?**
A pending confirmation card split its body into two columns. The prompt
used the left half. A pill with the raw text
`github:paperclipai/paperclip-desktop:pull:16` used the right half. The
line breaks in the prompt were also lost.
**Steps to reproduce**
1. Create a `request_confirmation` interaction with a multi-line
`prompt` and `target: { "type": "custom", "key":
"github:org/repo:pull:16" }` (no `label`).
2. Open the task chat for the issue.
3. See that the prompt and the key pill share one row, split about
50/50.
**Expected behavior**
The prompt uses the full card width and keeps its line breaks. A custom
target key is an identifier for idempotency and stale detection, so it
does not show as display text. A target pill shows only when the target
has a human label.
## What Changed
- `TaskChatCompactInteractionCard`: stack the prompt full width with the
target pill below it. This applies to the confirmation and the
checkbox-confirmation cards.
- `TaskChatCompactInteractionCard`: keep line breaks in the prompt
(`whitespace-pre-line`, `break-words`).
- `TaskChatCompactInteractionCard` and `IssueThreadInteractionCard`:
show no target chip for a `custom` target without a `label`. Labeled
custom targets (for example connection authorization) and
`issue_document` targets do not change. The stale-target chevron shows
only when both chips show.
- Add regression tests for the stacked layout, the hidden unlabeled key,
and the labeled link pill. The tests cover the compact confirmation and
checkbox-confirmation cards, and the thread card pending chip and
stale-target expiry (with labeled, unlabeled, and mixed custom targets,
and the chevron rule).
- `CONTRIBUTING.md`: add a "Keep Screenshots Out of the Repo" section.
Contributors attach review screenshots to the PR on GitHub. They do not
commit them to the repository.
## Verification
- `cd ui && npx vitest run
src/components/task-chat/TaskChatInteractionCard.test.tsx
src/components/IssueThreadInteractionCard.test.tsx`: 2 files, 100 tests
pass. If you revert the `isDisplayableRequestConfirmationTarget` guard,
3 of the new thread-card tests fail.
- `node scripts/check-token-gates.mjs`: all gates clean.
- `pnpm --filter @paperclipai/ui typecheck`: no errors in changed files.
The only errors come from `@paperclipai/plugin-sdk/ui`, which had no
built `dist` in the local workspace. These errors are not related to
this change.
- Manual: create a confirmation as in the steps above. The prompt is
full width and no key pill shows. Add `label: "PR #16"` and an `href`. A
`PR #16` link pill shows below the prompt.
### Screenshots
These are Storybook renders of the compact task-chat card at 720 px. The
three cards are: a confirmation with an unlabeled custom target (the
reported case), a confirmation with a labeled custom target, and a
checkbox confirmation with an unlabeled custom target.
The images load from an earlier commit of this PR. A later commit
deletes them, so they are not part of the merged change.
| Before (light) | After (light) | After (dark) |
| --- | --- | --- |
|

| 
| 
|
## Risks
- Low risk. This is a UI-only change. No schema or API change.
- Agents that send a custom target without a label lose the visible
pill. The key was never readable display text. To show a pill, agents
set `label`.
## Merge Note
The first commit was authored as Paperclip. When you squash-merge, keep
this trailer in the squash body:
Co-Authored-By: Paperclip <Paperclip-Paperclip@users.noreply.github.com>
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code agent
harness with tool use (shell, file edit). Standard context window.
## 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
- [ ] All Paperclip CI gates are green
- [ ] 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: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
This commit is contained in:
5 files changed
+183
-9
No files matched your search
@@ -94,6 +94,14 @@ git push -u origin <descriptive-name>
|
||||
git push origin --delete <old-name>
|
||||
```
|
||||
|
||||
### Keep Screenshots Out of the Repo
|
||||
|
||||
Screenshots, screen recordings, and other review evidence belong in the PR description or a PR comment, not in the repository. Do **not** commit them (for example under `docs/pr-screenshots/`). Committed review images land on `master` permanently and make every clone bigger, even though nobody needs them after the PR merges.
|
||||
|
||||
To show a visual change, drag the image into the PR description or a comment on GitHub, which hosts the file for you. If your tooling cannot upload to GitHub, leave the images out and describe what you checked under **Verification**.
|
||||
|
||||
Images that are part of the product documentation itself, such as a screenshot embedded in a guide under `doc/`, are fine to commit.
|
||||
|
||||
### Model Used (Required)
|
||||
|
||||
Every PR must include a **Model Used** section specifying which AI model produced or assisted with the change. Include the provider, exact model ID/version, context window size, and any relevant capability details (e.g., reasoning mode, tool use). If no AI was used, write "None — human-authored". This applies to all contributors — human and AI alike.
|
||||
|
||||
@@ -872,9 +872,74 @@ describe("IssueThreadInteractionCard", () => {
|
||||
expect(host.textContent).toContain("Plan v4");
|
||||
expect(targetLinks[0]?.getAttribute("href")).toContain("#document-plan");
|
||||
expect(targetLinks[1]?.getAttribute("href")).toContain("#document-plan");
|
||||
expect(host.querySelector(".lucide-chevron-right")).not.toBeNull();
|
||||
expect(host.textContent).not.toContain("Approve plan");
|
||||
});
|
||||
|
||||
describe("custom request confirmation targets", () => {
|
||||
const prKey = (pull: number) => `github:paperclipai/paperclip-desktop:pull:${pull}`;
|
||||
const unlabeled = (pull: number) => ({ type: "custom" as const, key: prKey(pull) });
|
||||
const labeled = (pull: number) => ({
|
||||
...unlabeled(pull),
|
||||
label: `PR #${pull}`,
|
||||
href: `https://github.com/paperclipai/paperclip-desktop/pull/${pull}`,
|
||||
});
|
||||
const staleTargetChange = (
|
||||
staleTarget: ReturnType<typeof unlabeled>,
|
||||
target: ReturnType<typeof unlabeled>,
|
||||
) => ({
|
||||
...staleTargetRequestConfirmationInteraction,
|
||||
payload: { ...staleTargetRequestConfirmationInteraction.payload, target },
|
||||
result: { version: 1 as const, outcome: "stale_target" as const, staleTarget },
|
||||
});
|
||||
|
||||
it("hides an unlabeled target key on a pending card", () => {
|
||||
const host = renderCard({
|
||||
interaction: {
|
||||
...pendingRequestConfirmationInteraction,
|
||||
payload: { ...pendingRequestConfirmationInteraction.payload, target: unlabeled(16) },
|
||||
},
|
||||
});
|
||||
expect(host.textContent).not.toContain("github:paperclipai");
|
||||
});
|
||||
|
||||
it("links a labeled target on a pending card", () => {
|
||||
const host = renderCard({
|
||||
interaction: {
|
||||
...pendingRequestConfirmationInteraction,
|
||||
payload: { ...pendingRequestConfirmationInteraction.payload, target: labeled(16) },
|
||||
},
|
||||
});
|
||||
const link = host.querySelector(
|
||||
'a[href="https://github.com/paperclipai/paperclip-desktop/pull/16"]',
|
||||
);
|
||||
expect(link?.textContent).toContain("PR #16");
|
||||
expect(host.textContent).not.toContain("github:paperclipai");
|
||||
});
|
||||
|
||||
it("shows no chips or chevron for an unlabeled stale-target change", () => {
|
||||
const host = renderCard({ interaction: staleTargetChange(unlabeled(15), unlabeled(16)) });
|
||||
expect(host.textContent).toContain("Expired by target change");
|
||||
expect(host.textContent).not.toContain("github:paperclipai");
|
||||
expect(host.querySelector(".lucide-chevron-right")).toBeNull();
|
||||
});
|
||||
|
||||
it("links both labeled targets across a stale-target chevron", () => {
|
||||
const host = renderCard({ interaction: staleTargetChange(labeled(15), labeled(16)) });
|
||||
expect(host.textContent).toContain("PR #15");
|
||||
expect(host.textContent).toContain("PR #16");
|
||||
expect(host.textContent).not.toContain("github:paperclipai");
|
||||
expect(host.querySelector(".lucide-chevron-right")).not.toBeNull();
|
||||
});
|
||||
|
||||
it("drops the chevron when only one side of the stale-target change is labeled", () => {
|
||||
const host = renderCard({ interaction: staleTargetChange(unlabeled(15), labeled(16)) });
|
||||
expect(host.textContent).toContain("PR #16");
|
||||
expect(host.textContent).not.toContain("github:paperclipai");
|
||||
expect(host.querySelector(".lucide-chevron-right")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it("renders a jump link for confirmations expired by comment", () => {
|
||||
const host = renderCard({
|
||||
interaction: commentExpiredRequestConfirmationInteraction,
|
||||
|
||||
@@ -1478,6 +1478,15 @@ function AskUserQuestionsCard({
|
||||
);
|
||||
}
|
||||
|
||||
/** A custom target key is a machine identifier (e.g. `github:org/repo:pull:16`),
|
||||
* not display copy, so unlabeled custom targets render no chip. */
|
||||
function isDisplayableRequestConfirmationTarget(
|
||||
target: RequestConfirmationTarget | null | undefined,
|
||||
): target is RequestConfirmationTarget {
|
||||
if (!target) return false;
|
||||
return target.type !== "custom" || Boolean(target.label);
|
||||
}
|
||||
|
||||
function requestConfirmationTargetLabel(target: RequestConfirmationTarget) {
|
||||
if (target.label) return target.label;
|
||||
const revision = target.revisionNumber ? ` v${target.revisionNumber}` : "";
|
||||
@@ -1511,7 +1520,7 @@ function RequestConfirmationTargetChip({
|
||||
target: RequestConfirmationTarget | null | undefined;
|
||||
tone?: "default" | "subtle";
|
||||
}) {
|
||||
if (!target) return null;
|
||||
if (!isDisplayableRequestConfirmationTarget(target)) return null;
|
||||
|
||||
const href = requestConfirmationTargetHref({ interaction, target });
|
||||
const className = cn(
|
||||
@@ -1652,7 +1661,8 @@ function RequestConfirmationResolution({
|
||||
target={staleTarget}
|
||||
tone="subtle"
|
||||
/>
|
||||
{staleTarget && target ? (
|
||||
{isDisplayableRequestConfirmationTarget(staleTarget)
|
||||
&& isDisplayableRequestConfirmationTarget(target) ? (
|
||||
<ChevronRight className="h-3.5 w-3.5 text-amber-700" />
|
||||
) : null}
|
||||
<RequestConfirmationTargetChip interaction={interaction} target={target} />
|
||||
|
||||
@@ -275,9 +275,11 @@ function targetLabel(
|
||||
) {
|
||||
const target = interaction.payload.target;
|
||||
if (!target) return null;
|
||||
const label =
|
||||
target.label ??
|
||||
(target.type === "issue_document" ? target.key : target.key);
|
||||
// A custom target key is a machine identifier for idempotency and stale
|
||||
// detection (e.g. `github:org/repo:pull:16`), not display copy. Only show a
|
||||
// custom target when the agent supplied a human label.
|
||||
if (target.type === "custom" && !target.label) return null;
|
||||
const label = target.label ?? target.key;
|
||||
const revision =
|
||||
target.revisionNumber == null ? null : `v${target.revisionNumber}`;
|
||||
return [label, revision].filter(Boolean).join(" · ");
|
||||
@@ -862,8 +864,8 @@ function ConfirmationCard({
|
||||
planDocument={planDocument}
|
||||
/>
|
||||
) : isPlanConfirmation && rejecting ? null : (
|
||||
<div className="flex flex-wrap items-start justify-between gap-2">
|
||||
<p className="min-w-0 flex-1 text-sm leading-5 text-foreground">
|
||||
<div className="space-y-2">
|
||||
<p className="whitespace-pre-line break-words text-sm leading-5 text-foreground">
|
||||
{isPlanConfirmation
|
||||
? "Do you accept this plan?"
|
||||
: interaction.payload.prompt}
|
||||
@@ -1152,8 +1154,8 @@ function CheckboxConfirmationCard({
|
||||
|
||||
return (
|
||||
<div>
|
||||
<div className="flex flex-wrap items-start justify-between gap-2">
|
||||
<p className="min-w-0 flex-1 text-sm leading-5 text-foreground">
|
||||
<div className="space-y-2">
|
||||
<p className="whitespace-pre-line break-words text-sm leading-5 text-foreground">
|
||||
{interaction.payload.prompt}
|
||||
</p>
|
||||
<CompactTarget interaction={interaction} />
|
||||
|
||||
@@ -370,6 +370,95 @@ describe("TaskChatInteractionCard", () => {
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("stacks the confirmation prompt full-width and hides unlabeled custom target keys", () => {
|
||||
const render = (target: RequestConfirmationInteraction["payload"]["target"]) =>
|
||||
flushSync(() => {
|
||||
root.render(
|
||||
<TooltipProvider>
|
||||
<ThemeProvider>
|
||||
<TaskChatInteractionCard
|
||||
item={interactionItem(
|
||||
createRequestConfirmation({
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "The change is open as PR #16.\n\nMay the PR merge?",
|
||||
target,
|
||||
},
|
||||
}),
|
||||
)}
|
||||
presentation="takeover"
|
||||
/>
|
||||
</ThemeProvider>
|
||||
</TooltipProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
render({ type: "custom", key: "github:paperclipai/paperclip-desktop:pull:16" });
|
||||
expect(container.textContent).not.toContain("github:paperclipai");
|
||||
const prompt = Array.from(container.querySelectorAll("p")).find((node) =>
|
||||
node.textContent?.startsWith("The change is open"),
|
||||
);
|
||||
expect(prompt?.className).not.toContain("flex-1");
|
||||
expect(prompt?.className).toContain("whitespace-pre-line");
|
||||
expect(prompt?.parentElement?.className).not.toContain("justify-between");
|
||||
|
||||
render({
|
||||
type: "custom",
|
||||
key: "github:paperclipai/paperclip-desktop:pull:16",
|
||||
label: "PR #16",
|
||||
href: "https://github.com/paperclipai/paperclip-desktop/pull/16",
|
||||
});
|
||||
const link = container.querySelector(
|
||||
'a[href="https://github.com/paperclipai/paperclip-desktop/pull/16"]',
|
||||
);
|
||||
expect(link?.textContent).toBe("PR #16");
|
||||
expect(container.textContent).not.toContain("github:paperclipai");
|
||||
});
|
||||
|
||||
it("stacks the checkbox confirmation prompt full-width and hides unlabeled custom target keys", () => {
|
||||
const render = (target: RequestConfirmationInteraction["payload"]["target"]) =>
|
||||
flushSync(() => {
|
||||
root.render(
|
||||
<TooltipProvider>
|
||||
<ThemeProvider>
|
||||
<TaskChatInteractionCard
|
||||
item={interactionItem({
|
||||
...pendingRequestCheckboxConfirmationInteraction,
|
||||
payload: {
|
||||
...pendingRequestCheckboxConfirmationInteraction.payload,
|
||||
prompt: "Pick the drafts to delete.\n\nUnchecked drafts stay.",
|
||||
target,
|
||||
},
|
||||
})}
|
||||
presentation="takeover"
|
||||
/>
|
||||
</ThemeProvider>
|
||||
</TooltipProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
render({ type: "custom", key: "github:paperclipai/paperclip-desktop:pull:16" });
|
||||
expect(container.textContent).not.toContain("github:paperclipai");
|
||||
const prompt = Array.from(container.querySelectorAll("p")).find((node) =>
|
||||
node.textContent?.startsWith("Pick the drafts"),
|
||||
);
|
||||
expect(prompt?.className).not.toContain("flex-1");
|
||||
expect(prompt?.className).toContain("whitespace-pre-line");
|
||||
expect(prompt?.parentElement?.className).not.toContain("justify-between");
|
||||
|
||||
render({
|
||||
type: "custom",
|
||||
key: "github:paperclipai/paperclip-desktop:pull:16",
|
||||
label: "PR #16",
|
||||
href: "https://github.com/paperclipai/paperclip-desktop/pull/16",
|
||||
});
|
||||
const link = container.querySelector(
|
||||
'a[href="https://github.com/paperclipai/paperclip-desktop/pull/16"]',
|
||||
);
|
||||
expect(link?.textContent).toBe("PR #16");
|
||||
expect(container.textContent).not.toContain("github:paperclipai");
|
||||
});
|
||||
|
||||
it("keeps the plan revision in the header and hides the prompt while requesting changes", async () => {
|
||||
const interaction = createRequestConfirmation({
|
||||
sourceRunId: "run-plan",
|
||||
|
||||
Reference in new issue
Block a user