mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Much of the work means moving an opaque value somewhere else: an
agent id, a callback URL, a webhook secret, an error payload for a bug
report
> - Copy buttons all over the app do that, and most already say whether
it worked — an icon that flips to a check, a label that reads "Copied"
> - Eleven did not. They passed the promise to `.catch(() => {})` and
showed nothing at all
> - A copy that says nothing looks exactly like a copy the browser
refused, and the only way to tell is to paste somewhere and look
> - The clipboard really does refuse: over plain HTTP on a non-secure
host the async Clipboard API is unavailable, and the fallback can still
fail
> - This pull request gives every copy action an affordance, through two
shared hooks
> - The benefit is that a person can tell a copy from a failure without
leaving the page
## Linked Issues or Issue Description
**What happened?**
Eleven copy buttons gave no feedback. They called `copyTextToClipboard`
and discarded the result:
```
onClick={() => void copyTextToClipboard(robotEmail).catch(() => {})}
```
Nothing changes on screen, whether the write succeeded or failed. This
is the same surface where most other copy buttons do show a check or a
"Copied" label, so the silent ones read as broken.
**Expected behavior**
Every copy action says whether the clipboard took the value. A rejected
write says so rather than staying silent or claiming success.
**Steps to reproduce**
1. Open the connector setup flow for an app that shows a sharing email
or a callback URL.
2. Press **Copy** next to that value.
3. Before this change nothing on screen changes. Compare with the copy
button on a login panel, which flips to a check.
4. To see the failure case, open Paperclip over plain HTTP on a
non-localhost host, where the async Clipboard API is not available.
**Paperclip version or commit**
`45586170e` on `master`.
**Deployment mode**
Local.
**Agent adapter(s) involved**
Not adapter-specific (core bug).
**Additional context**
Commit `1fa36be35` moved every copy through `lib/clipboard.ts`, so the
call sites are findable. Of 62 call sites, 51 already showed feedback
and 11 did not.
## What Changed
- New `ui/src/lib/use-copy-action.ts` with two hooks:
- `useCopyAction` for a control that stays on screen. It returns
`copied` and `failed` for the inline swap the rest of the app already
uses, and resets itself.
- `useCopyToast` for a menu item, which unmounts with its menu before an
inline state could be read, so its confirmation goes to the toast
viewport.
- Both wait for the write to resolve before reporting success, and both
report a rejection as a failure. `AdapterLoginChrome.test.tsx` already
held that line for one button; the helper now makes it the default.
- The eleven silent sites now use one of the two: the connector setup
flow's sharing-email and callback-URL buttons, the runner inspector's
copy-value and copy-path buttons, the agent bubble and skill studio menu
items, annotation "Copy link" on both its hosts, both secret-error
detail buttons, the chat webhook secret, and the motion tweak panel's
export box.
- The chat webhook secret follows its own file's convention: a label
that reads "Webhook secret copied", matching the manifest buttons beside
it.
- The 51 sites that already had feedback are untouched. Converting them
would be a large diff with no visible change.
- `clipboard-usage.test.ts` gains a static check, sibling to the one
that already keeps copies on the shared helper. It fails if a new copy
site ships with no affordance.
## Verification
- `npx vitest run ui/src/lib/use-copy-action.test.tsx
ui/src/lib/clipboard-usage.test.ts ui/src/lib/clipboard.test.ts
ui/src/components/AdapterLoginChrome.test.tsx` — 18 tests pass.
- The new hook tests cover the three cases that matter: no confirmation
while the write is still in flight, a failure state on a rejected write,
and a return to rest after the reset delay. The toast hook is covered
for both tones.
- `npx vitest run ui/src/pages/apps/AppsConnect.test.tsx
ui/src/components/task-chat ui/src/components/RunnerInspector.test.tsx
ui/src/components/DocumentAnnotation` — the suites over the touched
components pass.
- `npx tsc -b ui` — clean.
## Risks
Low. Each change is additive at its own call site and the copy path
itself is unchanged. The static check is the only part that touches
future contributors: it is one assertion, and its pattern list is easy
to extend or drop.
## Model Used
Claude Opus 5 (`claude-opus-5`), 1M context, extended thinking, 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
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
82 lines
4.0 KiB
TypeScript
82 lines
4.0 KiB
TypeScript
import { readdirSync, readFileSync } from "node:fs";
|
|
import { fileURLToPath } from "node:url";
|
|
import { describe, expect, it } from "vitest";
|
|
|
|
const DIRECT_CLIPBOARD_WRITE = /navigator\.clipboard|document\.execCommand/;
|
|
/** Any affordance that tells the reader the copy landed (or didn't). */
|
|
const COPY_FEEDBACK =
|
|
/useCopyAction|useCopyToast|copyWithToast|CopyValueButton|CopyDetailsButton|setCopied|pushToast|showCopyFeedback|setCopyStatus|Copied/i;
|
|
|
|
function sourceFiles(root: URL): string[] {
|
|
return readdirSync(root, { withFileTypes: true }).flatMap((entry) => {
|
|
const url = new URL(entry.name + (entry.isDirectory() ? "/" : ""), root);
|
|
if (entry.isDirectory()) return sourceFiles(url);
|
|
if (!/\.tsx?$/.test(entry.name) || /\.test\.tsx?$/.test(entry.name)) return [];
|
|
return [fileURLToPath(url)];
|
|
});
|
|
}
|
|
|
|
describe("clipboard usage", () => {
|
|
it("routes core and first-party plugin copy actions through shared helpers", () => {
|
|
const coreSource = new URL("../", import.meta.url);
|
|
const pluginSource = new URL("../../../packages/plugins/plugin-workspace-diff/src/ui/", import.meta.url);
|
|
const violations = [...sourceFiles(coreSource), ...sourceFiles(pluginSource)]
|
|
.filter((file) => !file.replaceAll("\\", "/").endsWith("/lib/clipboard.ts"))
|
|
.filter((file) => DIRECT_CLIPBOARD_WRITE.test(readFileSync(file, "utf8")));
|
|
|
|
expect(violations).toEqual([]);
|
|
});
|
|
|
|
it("gives every copy action some way to say whether it worked", () => {
|
|
// A copy that says nothing is indistinguishable from a copy that failed,
|
|
// which is the whole complaint. Whichever affordance a site picks — the
|
|
// inline swap from `useCopyAction`, a toast from `useCopyToast`, or a
|
|
// hand-rolled `copied` state — it has to pick one.
|
|
//
|
|
// Scope, so nobody reads more into a pass than it earns: this catches
|
|
// *silence*. It cannot prove the affordance waits for the write to
|
|
// resolve — a dozen sites name their state differently enough that
|
|
// demanding a known helper would flag nine working buttons. The
|
|
// swallowed-rejection check below covers the specific way a site claims
|
|
// success it did not earn; `AdapterLoginChrome.test.tsx` and
|
|
// `use-copy-action.test.ts` hold the behavioral line per site.
|
|
const silent = sourceFiles(new URL("../", import.meta.url))
|
|
.filter((file) => !file.replaceAll("\\", "/").endsWith("/lib/clipboard.ts"))
|
|
.flatMap((file) => {
|
|
const lines = readFileSync(file, "utf8").split("\n");
|
|
return lines.flatMap((line, index) => {
|
|
if (!line.includes("copyTextToClipboard(")) return [];
|
|
if (line.includes("import") || line.includes("export")) return [];
|
|
const near = lines.slice(Math.max(0, index - 8), index + 9).join("\n");
|
|
return COPY_FEEDBACK.test(near) ? [] : [`${file}:${index + 1}`];
|
|
});
|
|
});
|
|
|
|
expect(silent).toEqual([]);
|
|
});
|
|
|
|
it("never discards a copy rejection", () => {
|
|
// The precise way a copy button lies: hand the promise an empty rejection
|
|
// handler, then show a success state the write never earned. That is the
|
|
// pattern all eleven silent sites had, and the one
|
|
// `AdapterLoginChrome.test.tsx` refuses for its own button.
|
|
const swallowed = sourceFiles(new URL("../", import.meta.url))
|
|
.filter((file) => !file.replaceAll("\\", "/").endsWith("/lib/clipboard.ts"))
|
|
.flatMap((file) => {
|
|
const source = readFileSync(file, "utf8");
|
|
const lines = source.split("\n");
|
|
return lines.flatMap((line, index) => {
|
|
if (!line.includes("copyTextToClipboard(")) return [];
|
|
if (line.includes("import") || line.includes("export")) return [];
|
|
// The handler can wrap onto the next lines, so read forward a little.
|
|
const after = lines.slice(index, index + 6).join("\n");
|
|
return /\.catch\(\s*\(\s*\)\s*=>\s*\{\s*\}\s*\)|,\s*\(\s*\)\s*=>\s*\{\s*\}\s*\)/.test(after)
|
|
? [`${file}:${index + 1}`]
|
|
: [];
|
|
});
|
|
});
|
|
|
|
expect(swallowed).toEqual([]);
|
|
});
|
|
});
|