Files
PaperClipAI/DESIGN.md
Devin FoleyandPaperclip 144083fd48 fix(interactions): wait for workspace readiness before enabling approval (#14893)
## Thinking Path

> - Paperclip lets people manage AI agents and review their work.
> - Task confirmations must use the work produced by their source run.
> - The server blocks approval while that run still needs to sync its
workspace.
> - The card currently enables approval before that check can pass, so
an ordinary click produces an error.
> - This PR exposes the existing readiness check and shows “Preparing
approval…” with acceptance disabled.
> - The card refreshes itself and enables approval when the source
workspace settles.

## Linked Issues or Issue Description

**What happened?**

A confirmation appears while its source run is still preparing or
syncing its workspace. Its enabled approval button returns a conflict
asking the user to retry after syncing.

**Steps to reproduce**

1. Run an agent in an isolated workspace.
2. Have it create a confirmation before workspace finalization
completes.
3. Click the approval button while the source workspace is still active.

**Expected behavior**

The card explains that approval is preparing. Acceptance becomes
available automatically when the same server check permits it. Reject
and revise remain available.

Related work: #10770 handles this conflict after a click with retries.
#9520 proposes changing the workspace acceptance barrier. This PR
preserves that barrier and exposes readiness before the click, including
in compact task chat. It preserves the terminal-finalize behavior from
#10099.

## What Changed

- Add an optional, read-only `acceptanceBlocker` to interaction
responses. Readiness uses the existing source-run workspace predicate,
with one check per pending source run.
- Disable acceptance and show a shared preparation notice in classic and
compact confirmation cards, including checkbox and secret-binding
confirmations.
- Refresh preparing cards every two seconds in task detail, attention,
pipelines, and Skill Studio. Restore each surface's previous polling
cadence when preparation clears.
- Preserve live tool reviews, questions, rejection, revision, and the
server acceptance barrier. No automatic acceptance occurs.
- Document the preparation state and cover readiness, terminal sync
outcomes, unrelated runs, historical cards, and automatic refresh.

## Verification

- Focused service, card, query-refresh, and helper tests: 217 passed
across five files.
- `pnpm -r typecheck`: passed.
- `pnpm build`: passed.
- `pnpm build-storybook`: passed.
- `pnpm check:token-gates`: passed.
- `pnpm test:run`: incomplete locally. Stopped after about 17 minutes
once it reproduced seven existing environment failures: two Slack tests
and two email tests lack ancestor-directory skill fixtures; three
company-skills tests fail on macOS runtime-cache staging permissions.
These are outside this change. The full CI test matrix passed.
- CI: all 53 checks passed; two optional Storybook jobs were skipped.
The branch has no conflicts with `master`.
- Greptile: 5/5 on commit `8a6f216d8d`, with no review threads.
- Reviewed added lines and new files for credentials, private URLs,
internal task references, user paths, and run artifacts. None found.

## Risks

- No database migration or change to acceptance authorization. Readiness
is advisory; the server still enforces its existing gate at acceptance.
- An open preparing card adds a read every two seconds. This cadence
stops after readiness clears; historical cards add no workspace checks.
- Failed or stale finalization retains the existing server behavior.
This PR does not change recovery policy.

## Model Used

OpenAI GPT-6 through Codex. The exact serving model ID and
context-window size are not exposed in this session. Used reasoning,
repository inspection, tool execution, and automated tests. No
sub-agents.

## 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 (217 focused tests; full
local-suite limitations are recorded above)
- [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-10-02 13:08:25 -07:00

127 lines
13 KiB
Markdown

# Paperclip Design Principles
**Status:** v0.3 — anchor document for design-language simplification. Governs structure, not brand. Brand values (color, type, iconography) are intentionally unspecified: they are being redesigned and will land as token values only. Nothing in `ui/` may hardcode them. Spacing/radius scales are likewise TBD pending the token audit (see Principle 3).
Changes from v0.2: token layer location corrected to the repo's real source (`ui/src/index.css`); existing token tiers inventoried; snapshot-coverage scope bounded for Run 1; the issue→task copy rename moved out of the zero-visual-change run.
## What this document is for
Agents and humans modifying `ui/` treat this file as the source of truth for design decisions. Storybook is the verification surface — it documents the system; it does not define it. If a change conflicts with this document, change this document first (with review) or change the code.
## Product stance
Paperclip is an operational control plane: org charts, tasks, heartbeat runs, budgets, approvals, audit logs. The user is an operator scanning state and making decisions. Every screen should answer, in order: *what is happening, does it need me, what do I do about it.* Density in service of scanning beats whitespace in service of aesthetics — but density comes from information, never from chrome.
## The token layer (where visual values live)
The single token source is **`ui/src/index.css`** (Tailwind v4; there is no tailwind config file — tokens are CSS custom properties consumed via `@theme`). Do NOT create a parallel token source such as `ui/src/tokens/` — that would produce two sources of truth. If index.css grows unwieldy, extracted values may live in a `tokens.css` **imported by index.css** so the pipeline still has one root.
Tailwind v4 gotcha: `@theme inline` bakes literal values at build time. Any token that must be runtime-tunable (theme editor, dark mode overrides) must be defined in a NON-inline block.
Existing tiers already in index.css (~80+ tokens) — extraction maps to these on **exact value match** before minting anything new:
1. **Semantic tier** — shadcn core set: `--background`, `--foreground`, `--card`, `--primary`, `--secondary`, `--muted`, `--accent`, `--destructive`, `--border`, `--input`, `--ring`, `--sidebar-*`, `--chart-1..5` (OKLCH, light/dark overrides).
2. **Brand tier** — agent gradients `--agent-1a/1b..10a/10b` (fixed hex) and status hues `--status-task-*` / `--status-agent-*` (WCAG-tuned; see inline comments).
3. **Domain tier** — match-chip tokens `--chip-match-*`, annotation highlights `--paperclip-doc-annotation-highlight-*`, plus motion/typography tokens.
## Principles
1. **One way to say each thing.** One component per job. One Button, one Card, one Badge, one Table, one EmptyState. Variants are props, not new components. Before creating a component, prove no existing one covers the job.
2. **Tokens are the only source of visual values.** All color, spacing, radius, type size/weight, shadow, and motion values come from the token layer. No hex, no raw px, no ad-hoc Tailwind arbitrary values (`p-[13px]`) in components. If a needed value doesn't exist, add a token — don't inline it. Tailwind palette classes (`bg-red-500`, `text-zinc-400`, etc.) ARE hardcoded values in spirit: they name a literal color, not a semantic role. They are in-scope debt scheduled for a dedicated future run (Run 4, cluster-by-cluster mapping to semantic tokens per doc/design/DECISION-SHEET.md B2) and are not currently gated by check-token-gates. Exception (doc/design/DECISION-SHEET.md B1 user ruling): first-party intentional one-off decoration on demo/UX-lab surfaces stays inline and allowlisted rather than minted as singleton tokens.
3. **Spacing routes through tokens; the scale comes later.** During simplification, extract every spacing and radius value verbatim into tokens — do not normalize, round, or invent a scale. The final scale is a design decision made by a human after reviewing the token audit. Structural rules apply now: vertical rhythm within a container uses one gap value, not per-element margins, and siblings never carry both margin and gap.
4. **Hierarchy through structure, not decoration.** Prefer position, size, and weight over borders, backgrounds, and dividers. Every border, divider, and background fill must justify itself; when in doubt, remove it. A screen should survive the removal of one visual layer.
5. **Status is systematic.** States like running / paused / blocked / awaiting-approval / over-budget map to a single semantic status token set used identically everywhere (badge, row, chart, log). An operator learns the vocabulary once.
6. **Machine values look machine-made.** IDs, costs, token counts, timestamps, and log output use the monospace token and consistent formatting helpers. Never format these ad hoc per screen.
7. **Words are part of the system.** One name per concept across the entire UI — the canonical term is *task* (never *issue* or *ticket* in copy, labels, or empty states). Buttons name the action ("Approve hire," not "Submit"). Errors say what happened and what to do. Empty states say what to do first. **Note:** enforcing the task rename is a visible change and is explicitly OUT of the zero-visual-change extraction run; it happens in its own follow-up run.
8. **Agent-modifiable by design.** The system must be changeable via instructions: single token source, lint rules that enforce it, and this document kept current. A correct change should be expressible as "edit tokens + run checks," not "visit 40 files."
## Form and wizard footers
Keep **Save & exit** (or Cancel/Back) and the primary Continue/Connect/Finish
action in one shared footer row, vertically centered. Put the subdued secondary
action on the left and the primary action on the right. A step owns its whole
footer: do not render Save & exit in a separate parent block below it. Check this
alignment in every step and conditional state, not just the first screen.
## Contextual feedback
Task chat shows execution errors and waits only while they remain relevant.
Completing or cancelling a task hides its old execution notices. A newer attempt by
the same agent or an explicit successor supersedes earlier run notices; an
unresolved execution hold remains visible. Historical turns keep their responses,
files, questions and inspectable activity without a Worked/Stopped status label.
Run history retains the full diagnostic record. Session reset boundaries remain
in the conversation. Time passing or a new human comment alone does not resolve
an error. Stored notices need run or recovery provenance before they can be hidden;
child-task relays and other unrelated system updates stay visible.
Do not show a toast for task or run state already visible on the current screen.
This includes descendant runs represented by the open subtree. Show local action
results in place; keep failures actionable inline. Notifications for other work
remain useful. Repeated delivery of the same run outcome must refresh cached state
without repeating its toast, including after reconnecting. A terminal outcome
delivered more than five minutes after the run finished is historical and should
refresh state silently. Expected cancellation is neutral gray, not an error. The composer's Stop action stops the current response and leaves the composer available for a new message. Pause work is a separate explicit task or subtree action. A paused task replaces the composer with an amber takeover. It says “Task is
paused.” and “Resume this task to send a message.” with a “Resume task” action.
Subtrees use “Subtree is paused.” and “Resume subtree.” The takeover cannot be
dismissed, retains drafts, and hides message inputs until the pause is released.
Confirmations whose source work is still syncing show “Preparing approval…” and
disable acceptance until the server reports readiness. Refresh that state automatically;
rejection and revision remain available. Live tool reviews keep their own approval flow.
Pending questions, confirmations, and other task-thread inputs appear in a separate
card directly above the ordinary composer. The composer stays available for new
messages while the card is open. Dismissing a card leaves a pending indicator that
can reopen it; resolving or skipping the input removes that indicator.
## Enforcement (what "compliant" means for the extraction run)
- **Zero visual change is proven, not promised:** Storybook visual snapshots are baselined before any refactor, and all snapshots match baseline after it. A change that alters rendered output must be intentional and human-approved.
- **Baseline scope for Run 1:** the shared primitives in `ui/src/components/ui/` (each gets a story if missing — there are only ~24) plus the ~46 existing stories under `ui/storybook/stories/`. Do NOT attempt a story for every feature component (~277) in this run; full coverage is a later effort.
- Mechanical rewrites (value extraction, renames) are done via committed codemod scripts in `scripts/`, not hand-edits — reviewable once, repeatable forever.
- Token layer is the single source (`ui/src/index.css`, per above) consumed via CSS variables / Tailwind theme — never values copied into components.
- Lint/grep gates pass: zero hardcoded hex values, zero arbitrary spacing values, zero raw font-size declarations in `ui/src/components/**` and `ui/src/pages/**` outside the token layer and a documented allowlist (third-party overrides, intentional opt-outs commented inline).
- `pnpm build`, `pnpm typecheck`, and `pnpm build-storybook` pass.
- AGENTS.md links here and states the token-only rule.
Aspirational (NOT gating this run): no duplicate components; every component has exactly one story covering its variants; all UI copy says "task".
## Out of scope (do not do during simplification)
No visual redesign, no new colors or typefaces, no layout restructuring, no new dependencies beyond snapshot tooling, no component consolidation/merges (audit + recommend only), no copy renames, no changes to server code or app logic. Simplification means fewer parts, same product.
## Prior art (read before auditing)
See `doc/design/PRIOR-ART.md` — a previous audit pass (PAP-280/283/284, on the `PAP-282-playground` branch, NOT on master) found that of ~220 hardcoded drift sites, only 6 were exact-value-mappable to existing tokens; expect the verbatim extraction to mint many new tokens that the human scale-collapse step later merges. It also drafted usage rules (radius tiers, CTA tiers, named type styles) that are good candidates for the post-audit scale decision.
How-to guide for day-to-day UI changes: see `doc/design/CHANGING-THE-UI.md`.
## Motion tokens (Task Chat Redesign)
The redesigned task thread (flag `enableTaskChatRedesign`) is the first surface to
tokenize motion. Principles — reasoning only; values live in `ui/src/index.css`:
- **One home, and it is `:root`, not `@theme inline`.** `@theme inline` bakes literals
at build time, so a value placed there cannot be moved at runtime. The dev tweak panel
tunes motion by writing CSS custom properties live, so every motion token must resolve
at runtime — hence `:root`.
- **Two tiers.** Primitives (`--motion-duration-*`, `--motion-ease-*`) express the app's
baseline motion feel; state/component-scoped tokens (`--motion-<state>-*`) reference the
primitives so the whole thread retunes from a few knobs. Scoped tokens exist so the
tweak panel can group controls by the state they affect.
- **Reuse the house curves.** New easing defaults point at the two curves already used
across the app rather than inventing a third feel.
- **No hardcoded timing in components.** Durations, easings, delays, and staggers used by
the redesigned thread must reference these tokens; a check script rejects raw `ms` /
`cubic-bezier` values outside `ui/src/index.css`. This discipline is what makes the
tweak panel structurally possible.
- **Values are placeholders.** The committed numbers are sensible starting points, tuned
live by a human and pasted back from the tweak panel's export — never treated as final
during the baseline build.
- **Reduced motion is honored at the token layer.** A `prefers-reduced-motion: reduce`
block collapses the duration/stagger tokens to zero, cascading to every scoped token,
in addition to each animation's own component-level guard.
Agent Chat keeps pending questions as compact “Unanswered question” entries at their original position in history. A newer user message dismisses the old question form without resolving it. Opening the history entry restores the original form and its draft; submitting later uses the same durable question response path. Actual permission reviews retain their permission checks.