From b9f0a1b409135125f2de842dd06892dc2b3cafb3 Mon Sep 17 00:00:00 2001 From: scotttong Date: Mon, 13 Jul 2026 20:16:44 -0700 Subject: [PATCH] docs(design): demote per-change snapshot gate to dormant; add design-session and pr-to-green skills Per-change verification for design sessions is now token gates + typecheck + ui vitest + live before/after screenshots; the 510-snapshot Storybook suite becomes "full-rigor mode (dormant)" until the baseline archive publication flow exists (it was never a required CI check, and the committed manifest is still a placeholder). Decision recorded in doc/design/DECISION-SHEET.md; CHANGING-THE-UI.md, AGENT-SESSIONS.md, and DESIGN.md updated to the tiered policy with revival instructions kept verbatim. The session workflow is codified as two dual-exposure skills (authored in skills/, symlinked into .claude/skills/ like the existing paperclip skill, discoverable by Paperclip's project scan): - design-session: isolated worktree instance (server+UI+own DB), WORKTREE banner proof, per-change verify loop, plain-language reporting - pr-to-green: clean branch off master, template-complete draft PR, iterate to CI green + commitperclip pass + Greptile 5/5, then mark ready Co-Authored-By: Claude Fable 5 --- .claude/skills/design-session | 1 + .claude/skills/pr-to-green | 1 + DESIGN.md | 2 +- doc/design/AGENT-SESSIONS.md | 20 +++++----- doc/design/CHANGING-THE-UI.md | 37 +++++++++++++------ doc/design/DECISION-SHEET.md | 4 ++ skills/design-session/SKILL.md | 67 ++++++++++++++++++++++++++++++++++ skills/pr-to-green/SKILL.md | 58 +++++++++++++++++++++++++++++ 8 files changed, 168 insertions(+), 22 deletions(-) create mode 120000 .claude/skills/design-session create mode 120000 .claude/skills/pr-to-green create mode 100644 skills/design-session/SKILL.md create mode 100644 skills/pr-to-green/SKILL.md diff --git a/.claude/skills/design-session b/.claude/skills/design-session new file mode 120000 index 00000000000..90635b9f8c4 --- /dev/null +++ b/.claude/skills/design-session @@ -0,0 +1 @@ +../../skills/design-session \ No newline at end of file diff --git a/.claude/skills/pr-to-green b/.claude/skills/pr-to-green new file mode 120000 index 00000000000..75918101b54 --- /dev/null +++ b/.claude/skills/pr-to-green @@ -0,0 +1 @@ +../../skills/pr-to-green \ No newline at end of file diff --git a/DESIGN.md b/DESIGN.md index 6eaa1e12cf7..9a7bd598866 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -37,7 +37,7 @@ Existing tiers already in index.css (~80+ tokens) — extraction maps to these o ## 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. +- **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. **Status note (Jul 13 2026):** snapshot proof is dormant as a per-change gate pending the baseline archive publication flow — everyday design changes verify via token gates + typecheck + ui vitest + live before/after screenshots instead (see doc/design/DECISION-SHEET.md "Per-change snapshot verification demoted to dormant"). - **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. diff --git a/doc/design/AGENT-SESSIONS.md b/doc/design/AGENT-SESSIONS.md index 5d837846891..c43cb62898c 100644 --- a/doc/design/AGENT-SESSIONS.md +++ b/doc/design/AGENT-SESSIONS.md @@ -16,9 +16,9 @@ If your ask names a *feeling* ("too loud", "cramped", "inconsistent"), that's fi ## Pick the session size **Small (minutes) — a value change.** Colors, sizes, spacing, radius, one component's look. -> "In the Paperclip repo: make X look like Y. Show me before/after screenshots from the visual suite before you re-baseline anything." +> "In the Paperclip repo: make X look like Y. Show me before/after screenshots before you commit anything." -The agent should: edit token(s) → run `pnpm test:storybook-visual` → show you the diff images → only after your yes, run `test:storybook-visual:update`, publish the packed baseline archive from a trusted maintainer environment, and commit the code change + manifest update together. +The agent should: edit token(s) → show you live before/after screenshots of the affected screens → run the proof commands (`pnpm check:token-gates`, `pnpm typecheck`, `cd ui && pnpm vitest run`) → only after your yes, commit. The easiest way to start is the **`design-session` skill** — it sets up an isolated worktree instance and runs this exact loop. **Medium (an afternoon) — a retheme or a component-family restyle.** Ask for a **git worktree** so main stays untouched: > "Create a worktree off master, apply shadcn preset `` as token values only (values-only — review the CLI's diff, revert scaffolding), reconcile the Paperclip status/agent color tiers, then build me a before/after gallery of the key surfaces." @@ -26,7 +26,7 @@ The agent should: edit token(s) → run `pnpm test:storybook-visual` → show yo Review the gallery, iterate ("the dark red is too soft", "two different greens on toggles — one green"), then tell it to re-baseline and merge when you're satisfied. **Large (a day, unattended) — a bounded autonomous run.** Use `/goal` with a *measurable* finish line — the evaluator needs conditions a command can verify, not aspirations: -> Good conditions: "rg finds zero palette classes in ui/src/components", "the snapshot suite passes against the pinned external baseline", "pnpm check:token-gates reports 3/3 CLEAN". +> Good conditions: "rg finds zero palette classes in ui/src/components", "cd ui && pnpm vitest run passes with zero failures", "pnpm check:token-gates reports 3/3 CLEAN". > Bad conditions: "the UI feels cleaner", "design is more consistent". See `doc/design/GOAL-PROMPT.md` for a complete worked example (the run that built this system), including the phase structure and guardrails worth copying: work in a worktree, commit per phase, never re-baseline without human review, stop-and-report over partial application. @@ -35,16 +35,16 @@ See `doc/design/GOAL-PROMPT.md` for a complete worked example (the run that buil Hold every session to these five, regardless of size: -1. **Pictures before permanence.** Never approve on description. The suite produces before/actual/diff images for every changed story — ask for them ("show me these visually before I decide"). For subtle changes, ask for full-resolution images, not compressed thumbnails. -2. **Proof commands, not claims.** "Done" means: `pnpm check:token-gates` 3/3 CLEAN, `pnpm typecheck` green, suite result stated as a number ("510/510" or "N intentional diffs pending your review"). If the agent says done without these, ask for the outputs. -3. **Baselines ride with the change.** Intentional visual change → updated manifest metadata in the same commit, after the packed archive is reviewed and published. An agent that updates baselines to silence a failure it can't explain is the one thing you never accept. +1. **Pictures before permanence.** Never approve on description. Ask for live before/after screenshots of every affected screen ("show me these visually before I decide"). For subtle changes, ask for full-resolution images, not compressed thumbnails. +2. **Proof commands, not claims.** "Done" means: `pnpm check:token-gates` 3/3 CLEAN, `pnpm typecheck` green, `cd ui && pnpm vitest run` pass count stated as a number. If the agent says done without these, ask for the outputs. +3. **Baselines ride with the change** *(full-rigor mode only — currently dormant, see DECISION-SHEET Jul 13 2026)*. When the snapshot suite is active: intentional visual change → updated manifest metadata in the same commit, after the packed archive is reviewed and published. An agent that updates baselines to silence a failure it can't explain is the one thing you never accept. 4. **Mechanical changes via scripts.** If it's touching 20+ files with the same rewrite, it should write an idempotent codemod in `scripts/` (existing `codemod-*.mjs` files are the pattern), not hand-edit. 5. **Decisions get written down.** Anything judgment-shaped (a mapping, an exception, a deferral) goes in `doc/design/DECISION-SHEET.md` with one line of rationale. ## Reviewing like a designer -- **Contact sheet**: the diff images under `tests/storybook-visual/test-results/` are your primary review surface; ask the agent to assemble them into a browsable before/after page (or use `npx playwright show-report` from `tests/storybook-visual/`). -- **Live test drive**: for big changes, ask for a running instance from the worktree — `pnpm paperclipai worktree init` once, then `PORT=3300 pnpm dev:once` gives an isolated Paperclip (own database, own config; your real instance is untouched). Click around; real use surfaces what screenshots can't. +- **Live test drive is the primary review surface**: ask for a running instance from the worktree — `pnpm paperclipai worktree init` once, then `pnpm dev` from the repo root gives an isolated Paperclip on its own port (own database, own config; your real instance is untouched — the striped WORKTREE banner proves which one you're looking at). Click around; real use surfaces what screenshots can't. +- **Contact sheet** *(full-rigor mode)*: when the snapshot suite is active, the diff images under `tests/storybook-visual/test-results/` are a per-story review surface; ask the agent to assemble them into a browsable before/after page (or use `npx playwright show-report` from `tests/storybook-visual/`). - **Side-by-side Storybook**: old on one port, new on another (`pnpm storybook` in each checkout), flip tabs. - Trust your eyes over the agent's summary. If something looks wrong, say so plainly ("the text in the red boxes is illegible") — vague feedback is fine, the screenshots give the agent the precision. @@ -56,9 +56,9 @@ Hold every session to these five, regardless of size: ## When something goes wrong -- **Suite fails and the agent didn't change visuals** → it broke something; the diff image shows where. Don't let it re-baseline. - **A test fails asserting an old literal value** → lockstep case: the assertion updates to the token form, in the same commit as an explanation. -- **A story flakes under full-suite load** → have it re-run that story in isolation before treating it as real (three known flakes are documented in DECISION-SHEET). +- **Suite fails and the agent didn't change visuals** *(full-rigor mode)* → it broke something; the diff image shows where. Don't let it re-baseline. +- **A story flakes under full-suite load** *(full-rigor mode)* → have it re-run that story in isolation before treating it as real (three known flakes are documented in DECISION-SHEET). - **The agent is stuck or looping** → ask for a status report with the three proof commands; completed phases are committed, so restarting a session loses almost nothing. ## The short version diff --git a/doc/design/CHANGING-THE-UI.md b/doc/design/CHANGING-THE-UI.md index 49ee2a721ea..1be31d01fff 100644 --- a/doc/design/CHANGING-THE-UI.md +++ b/doc/design/CHANGING-THE-UI.md @@ -8,26 +8,30 @@ Paperclip's look lives in **three layers**, and you almost always work in the fi 1. **Tokens** — every color, text size, spacing, radius, and shadow is a named value in `ui/src/index.css`. Change a token, and every surface using it follows. 2. **Components** — consume tokens, never raw values. One component per job (one Button, one Card, one ToggleSwitch). -3. **Screenshots** — 510 baseline images (255 stories × light/dark) downloaded into `tests/storybook-visual/.snapshots/` from the pinned external archive in `tests/storybook-visual/baseline-manifest.json`. They are the proof of what the UI looks like. Any visual change shows up as a screenshot diff; no visual change proves itself the same way. +3. **Screenshots** — live before/after screenshots of the affected screens, taken on an isolated worktree instance, are the proof of what a change looks like. (A 510-image Storybook baseline system also exists but is currently dormant as a per-change gate — see "Full-rigor mode" below and the DECISION-SHEET entry of Jul 13 2026.) The rules live in [`DESIGN.md`](../../DESIGN.md) (repo root). The reasoning behind past decisions lives in [`DECISION-SHEET.md`](DECISION-SHEET.md). If you disagree with a rule, change `DESIGN.md` first (with review) — don't quietly diverge in code. -## The three commands +## The everyday verification loop + +Per-change proof for a design change (this is what the `design-session` skill runs): ```bash -pnpm check:token-gates # am I allowed to write this? (no hardcoded values) -pnpm test:storybook-visual # what does my change look like? (diff vs baseline) -pnpm test:storybook-visual:update # accept my intentional changes as the new baseline +pnpm check:token-gates # am I allowed to write this? (no hardcoded values — must be 3/3 CLEAN) +pnpm typecheck # nothing broke structurally +cd ui && pnpm vitest run # unit tests still pass ``` +…plus **live before/after screenshots** of the affected screens on the isolated worktree instance. Storybook snapshot runs are *not* part of the everyday loop — short-term drift is accepted by policy (DECISION-SHEET, Jul 13 2026) until the baseline archive publication flow exists. + ## Recipe 1 — change how something looks everywhere *"Make the corners rounder." "That amber is too loud." "Bump the smallest text size."* 1. Find the token in `ui/src/index.css` (they're named and commented: `--radius`, `--status-task-todo`, `--text-micro`, …). 2. Change the value. -3. `pnpm test:storybook-visual` — it will "fail" on every affected story. That's the point: each failure writes a before/actual/diff image triplet into `tests/storybook-visual/test-results/`. Review them (or `npx playwright show-report` from `tests/storybook-visual/` for a browsable version). -4. Happy? `pnpm test:storybook-visual:update`, review the generated bundle under `tests/storybook-visual/baseline-review/`, publish it from a trusted maintainer environment, then commit the token edit **and** the updated manifest metadata together. +3. Look at it live: screenshot the affected screens on your worktree instance before and after, and review the pair (the token's inline comments and `ui/src/lib/status-colors.ts` tell you which surfaces to check). +4. Happy? Run the everyday loop above and commit the token edit. (In full-rigor mode this step is where snapshot re-baselining happens — see below.) Notable single-knob tokens: `--radius` drives the entire corner ladder (sm→4xl are derived); the `--status-task-*` / `--status-agent-*` family is the app-wide status vocabulary (chips, charts, bars, live dots all follow it). @@ -39,7 +43,7 @@ The core palette follows the shadcn token names, so a theme built at ui.shadcn.c cd ui && pnpm dlx shadcn@latest init --preset --force --no-reinstall ``` -Then **review the git diff and keep only the CSS-variable value changes** — the CLI also tries to rewrite `components.json`, `lib/utils.ts`, and add dependencies; revert those (it once deleted 240 lines of our utils). Never use `shadcn apply --preset` (it overwrites component files). After the token diff is clean: Recipe 1 steps 3–4, plus a sanity pass on the Paperclip-specific tiers (agent gradients, WCAG-tuned status hues) for clashes. +Then **review the git diff and keep only the CSS-variable value changes** — the CLI also tries to rewrite `components.json`, `lib/utils.ts`, and add dependencies; revert those (it once deleted 240 lines of our utils). Never use `shadcn apply --preset` (it overwrites component files). After the token diff is clean: Recipe 1 steps 3–4 (a retheme touches everything, so screenshot a representative sweep: dashboard, a task list, a task detail, settings — both themes), plus a sanity pass on the Paperclip-specific tiers (agent gradients, WCAG-tuned status hues) for clashes. ## Recipe 3 — build or style a component @@ -47,12 +51,21 @@ Then **review the git diff and keep only the CSS-variable value changes** — th - **Type**: use the named ladder — `--text-nano` (10px) / `--text-micro` (11px) / Tailwind `text-xs` (12) / `--text-compact` (13) / `text-sm` (14). Letter-spacing: `--tracking-label` / `--tracking-eyebrow` / `--tracking-caps`. - **Status**: anything that means running/idle/paused/error/todo/done/blocked uses the status system (`ui/src/lib/status-colors.ts` helpers or `--status-*` tokens). Liveness is always blue. - **Primitives**: check `ui/src/components/ui/` and `doc/design/COMPONENT-INVENTORY.md` before writing a new component. Switches are `ToggleSwitch`; badges/chips route through `brandChipBadge`. -- **Give it a story.** New visual surface = new Storybook story = automatic screenshot coverage forever. +- **Give it a story.** New visual surface = new Storybook story. Stories remain the coverage map for full-rigor mode (below), so keep writing them even while snapshot gating is dormant. - `pnpm check:token-gates` before you push. If a value genuinely can't be a token (third-party config, canvas fills, intentional one-off decoration on demo pages), it goes on the allowlist **with an inline comment saying why**. -## Recipe 4 — you changed something and snapshots failed +## Full-rigor mode (currently dormant) -That's the system working. Two cases: +A 510-snapshot Storybook verification system exists (255 stories × light/dark, pixel-exact against a pinned baseline). It is **dormant as a per-change gate**: the baseline archive publication flow was never built — `tests/storybook-visual/baseline-manifest.json` is a placeholder, so the suite cannot run from a fresh checkout. Nothing was deleted; when the archive is published, the suite returns as an on-demand pre-release sweep and as the label-gated CI run (`storybook-visual` PR label). Everything below is kept verbatim for that revival. + +```bash +pnpm test:storybook-visual # what does my change look like? (diff vs baseline) +pnpm test:storybook-visual:update # accept my intentional changes as the new baseline +``` + +**Rigor-mode Recipe 1 (steps 3–4):** `pnpm test:storybook-visual` — it will "fail" on every affected story. That's the point: each failure writes a before/actual/diff image triplet into `tests/storybook-visual/test-results/`. Review them (or `npx playwright show-report` from `tests/storybook-visual/` for a browsable version). Happy? `pnpm test:storybook-visual:update`, review the generated bundle under `tests/storybook-visual/baseline-review/`, publish it from a trusted maintainer environment, then commit the token edit **and** the updated manifest metadata together. + +**Rigor-mode Recipe: snapshots failed.** That's the system working. Two cases: - **You meant it** → review the diffs (they're your design review), then `pnpm test:storybook-visual:update`, publish the packed baseline archive, and commit the manifest update with the change. A PR with visual changes but no baseline-manifest update is incomplete; baseline changes with no explanation are a red flag. - **You didn't mean it** → you broke something. The diff images show you exactly where. Do not update the baseline to make it green. @@ -63,6 +76,8 @@ Three stories are known to flake under full parallel load (they pass in isolatio (Running a session as the human? See [`AGENT-SESSIONS.md`](AGENT-SESSIONS.md) — this section is instructions for the agent itself.) +The full session workflow — isolated worktree instance, per-change verification loop, reporting rules — is codified as the **`design-session` skill** (`skills/design-session/SKILL.md`); shipping the result as a PR is the **`pr-to-green` skill** (`skills/pr-to-green/SKILL.md`). Prefer invoking those over improvising. + This system was built to be steered by instruction. "Make all running indicators blue" or "collapse these three grays into one" should land as a token edit or a small codemod plus a snapshot diff — not a manual hunt. If a change is mechanical and touches many files, write an idempotent script in `scripts/` (see `codemod-*.mjs` for the pattern) instead of hand-editing. DESIGN.md is loaded via AGENTS.md; follow it exactly, and record consequential choices in DECISION-SHEET.md. ## What's deliberately not done yet (don't fix ad hoc) diff --git a/doc/design/DECISION-SHEET.md b/doc/design/DECISION-SHEET.md index 22ea4623ec4..5e769d4043b 100644 --- a/doc/design/DECISION-SHEET.md +++ b/doc/design/DECISION-SHEET.md @@ -99,3 +99,7 @@ Executed on `design/component-convergence` (worktree focused-agnesi). Scope C2/C ## Tune session — CLOSED (Jul 6, 2026) User approved the complete new design language via gallery v4 + live test drive on the :3300 worktree instance ("ship it"). Merged origin/master (12 commits; one conflict — upstream deliberately removed the Wakes-on-confirm chip, deletion accepted). 296 snapshots re-baselined; gates 3/3 CLEAN; typecheck green; final suite verification run against the new baseline. Remaining roadmap: Run 3 (cards/pills/C11 sidebar + investigations + AgentDetail story), issue→task rename run, Run 4 (palette classes + toast), ESLint ratchet. + +## Per-change snapshot verification demoted to dormant (design-session policy, Jul 13 2026) + +**User ruling: short-term design-system drift is accepted; the Storybook visual suite is no longer a per-change gate for design sessions.** Per-change verification is now: `pnpm check:token-gates` 3/3 CLEAN + `pnpm typecheck` + `cd ui && pnpm vitest run` + live before/after screenshots of the affected screens on the session's isolated worktree instance. Rationale: (1) a full run compares ~510 snapshots (255 stories × light/dark) and burns disproportionate time and review tokens for single-token iterations; (2) the suite was never a required CI check — `.github/workflows/storybook-visual.yml` only runs on PRs carrying the `storybook-visual` label; (3) the baseline archive publication flow was never built (`tests/storybook-visual/baseline-manifest.json` is still the placeholder from the Jul 7 reconcile entry), so the suite cannot run from a fresh worktree anyway. Nothing is deleted: the `test:storybook-visual*` scripts, spec, config, and CI workflow all stay in place as **full-rigor mode (dormant)**. Revival condition: once the baseline archive is published and the manifest points at it, the suite returns as (a) an on-demand pre-release sweep and (b) the label-gated CI run for visually risky PRs. Companion changes: `doc/design/CHANGING-THE-UI.md` and `doc/design/AGENT-SESSIONS.md` rewritten to the tiered policy; the workflow itself is codified as the `design-session` skill (`skills/design-session/`), with PR shipping as the separate `pr-to-green` skill (`skills/pr-to-green/`). diff --git a/skills/design-session/SKILL.md b/skills/design-session/SKILL.md new file mode 100644 index 00000000000..ba556247322 --- /dev/null +++ b/skills/design-session/SKILL.md @@ -0,0 +1,67 @@ +--- +name: design-session +description: > + Run a design-feedback session on Paperclip's UI: the user says what looks + off, the agent makes token-first changes and proves them with live + before/after screenshots on an isolated worktree instance (server + UI + + own DB). Use to iterate on Paperclip's visual design; ship via pr-to-green. +--- + +# Paperclip design session + +You are the hands for a design-feedback session. The user describes outcomes ("that amber is too loud", often with screenshots); you translate each one into the smallest correct change — a token edit or a shared-component edit, never a one-off style — and prove it visually before calling it done. + +This skill assumes you are running inside a checkout of the Paperclip repo (usually a git worktree dedicated to the session). + +## 0. Read the rules first + +Before touching anything, read: + +1. `DESIGN.md` (repo root) — design principles and the token-only rule. +2. `doc/design/CHANGING-THE-UI.md` — the field guide: tokens → components → screenshots, recipes, and the everyday verification loop. + +Load alongside the repo's `design-guide` skill (component/token conventions). If your harness also ships a `frontend-design` skill (a Claude Code built-in, not part of this repo), load it too for visual quality; skip it if absent. + +Non-negotiables from those docs: visual values live in `ui/src/index.css` tokens; one component per job (check `ui/src/components/ui/` and `doc/design/COMPONENT-INVENTORY.md` before creating anything); no hex / raw px / arbitrary Tailwind values in components; forbidden moves are `shadcn apply --preset`, hardcoding a value because a token "doesn't fit", and ad-hoc fixes to scheduled-debt areas (palette classes etc. — see `doc/design/DECISION-SHEET.md`). + +## 1. Environment: full isolated instance, never UI-only + +The session runs on a **complete Paperclip instance** (server + UI + its own seeded database) so nothing touches the user's real data. + +1. `pnpm install` at the repo root. +2. **Only if `.paperclip/config.json` is missing in this checkout**, initialize the worktree instance: + - Pick a free server/DB port pair first: check sibling worktrees' `.paperclip/.env` files and `lsof -nP -iTCP -sTCP:LISTEN` for what's claimed, then pass explicit `--server-port` / `--db-port`. + - Normal path: `pnpm paperclipai worktree init --force --seed-mode minimal --name --server-port --db-port `. + - Gotcha: if `pnpm paperclipai` isn't wired up in the worktree, use the main checkout's CLI runner (same trick as `scripts/provision-worktree.sh`): + `node /cli/node_modules/tsx/dist/cli.mjs /cli/src/index.ts worktree init --force --seed-mode minimal --name --from-config ~/.paperclip/instances/default/config.json` +3. Start it with **`pnpm dev` from the repo root** — never `cd ui && pnpm dev`, which serves the UI only and proxies API calls to the user's primary instance (their real data). Gotcha: if tsx dies with pipe/EINVAL errors, set `TMPDIR=/tmp`. +4. **Prove isolation before accepting feedback:** open the instance in a browser and screenshot it. The striped **WORKTREE banner** across the top is the proof you're on the isolated instance. No banner → stop and fix the environment. + +Keep the server running for the whole session; hot reload shows your edits live. + +## 2. The per-change loop + +For each piece of feedback: + +1. **Screenshot the current state** of the affected screen(s) — the "before". +2. **Make one small, focused change.** Prefer, in order: existing token value → new token → shared component edit. Mechanical rewrites across many files go through an idempotent `scripts/codemod-*.mjs` script, not hand-edits. +3. **Verify — all of these, every time:** + - `pnpm check:token-gates` → must report **3/3 CLEAN** + - `pnpm typecheck` → green + - `cd ui && pnpm vitest run` → all passing (a test asserting an old literal value updates to the token form in the same change, with an explanation) +4. **Screenshot the "after"** and show the user the before/after pair. For token changes, spot-check other surfaces the token drives (its inline comment in `index.css` tells you which). +5. **Report in plain language** — what changed, where it's visible, what you verified. No code talk unless asked. Numbers, not claims ("gates 3/3, typecheck green, 2098/2098 tests"). +6. **Record real design decisions** (a mapping, an exception, a deferral — anything judgment-shaped) in `doc/design/DECISION-SHEET.md` with one line of rationale. + +**Storybook snapshots are deliberately NOT part of this loop.** Per the DECISION-SHEET entry "Per-change snapshot verification demoted to dormant (Jul 13 2026)", short-term drift is accepted; live screenshots are the visual proof. Do not run `pnpm test:storybook-visual` per change, and never re-baseline snapshots without explicit user approval. Still add a Storybook story for any new visual surface — stories are the coverage map for when full-rigor mode returns. + +## 3. Session rules + +- **User controls permanence.** Show before/after and wait for their verdict before moving on; if they say it's off, iterate — don't defend. +- **No push, no PR, no publishing** until the user explicitly asks. When they do, invoke the **`pr-to-green`** skill (`skills/pr-to-green/`). +- Commit locally in small, per-change commits so any single change can be dropped. +- If a request conflicts with `DESIGN.md`, say so and propose the compliant version (or a `DESIGN.md` change) instead of quietly diverging. + +## Installing this skill for Paperclip agents + +Claude Code picks this skill up automatically via the committed symlink in `.claude/skills/`. To make it available to Paperclip's own agents: Skills page → "Scan project workspaces" (or `POST /api/companies/:companyId/skills/scan-projects`), then enable it per agent in the agent's Skills tab. Repo skills surface under two keys (project-scan `local//...` plus adapter-native `paperclipai/paperclip/...`), so a bare slug is ambiguous when enabling via API — use the full key, e.g. `paperclipai/paperclip/design-session`. diff --git a/skills/pr-to-green/SKILL.md b/skills/pr-to-green/SKILL.md new file mode 100644 index 00000000000..06410ccc802 --- /dev/null +++ b/skills/pr-to-green/SKILL.md @@ -0,0 +1,58 @@ +--- +name: pr-to-green +description: > + Ship committed work as a draft PR and drive it to merge-ready: clean branch + off latest master, template-complete PR body, then iterate until CI checks + pass, commitperclip gates clear, and Greptile is 5/5 — only then mark ready + for review. Invoke only after the user explicitly asks to ship or push. +--- + +# PR to green + +Take the current work product to a merge-ready pull request. The finish line is exact: **all CI checks green + commitperclip gates passing + Greptile 5/5 with no open P2s, recommendations, or follow-ups → then, and only then, flip the draft to "ready for review".** + +This skill assumes you are in a checkout of the Paperclip repo with the changes committed locally, and that the user has explicitly asked to ship. + +**Related skills** (`.agents/skills/`): this is the end-to-end shipping flow for session work; it composes narrower loops rather than replacing them. `prcheckloop` is the dedicated iterate-until-checks-green loop, `check-pr` inspects an existing PR for review comments / failing checks / body gaps, and `pr-report` summarizes PR state. If you are mid-flow and only need one of those steps, invoke that skill directly. + +## 1. Build a clean branch off latest master + +1. `git fetch origin master` and branch from `origin/master` — not from the session worktree's possibly-stale fork point. +2. Apply the session's changes onto it (cherry-pick or re-apply the diff). Squash fixup noise; keep logically separate changes as separate commits. +3. **No internal identifiers anywhere public:** branch name, commit messages, and PR body must not contain internal ticket ids (PAP-123 / PAPA-123), `agent://` links, instance-local URLs, or localhost/tailnet references. Branch names describe the change (`fix/...`, `docs/...`, `design/...`). +4. Re-run the local proof before pushing: `pnpm check:token-gates` (3/3 CLEAN if UI files changed), `pnpm typecheck`, and the relevant vitest suite(s). + +## 2. Open as a DRAFT PR, template-complete + +Read `CONTRIBUTING.md` and `.github/PULL_REQUEST_TEMPLATE.md`, then fill **every** section — the commitperclip bot rejects incomplete templates: + +- **Thinking Path** — 5–8 blockquote steps from "what Paperclip is" down to this change. +- **Linked Issues or Issue Description** — either `Fixes:/Closes/Refs #NNN` (public GitHub only) or an in-PR description following the matching issue template. Search GitHub for duplicate/related PRs and link them. +- **What Changed** — one bullet per logical unit. +- **Verification** — the commands a reviewer can run, plus manual steps/screenshots for visual changes. +- **Risks** — honest; "Low risk" only if genuinely minor. +- **Model Used** — required: provider, exact model id, context window, thinking mode. +- **Checklist** — every box, truthfully. + +For visual changes while snapshot baselines are dormant: state in the PR body that baselines are intentionally not updated, citing the `doc/design/DECISION-SHEET.md` entry "Per-change snapshot verification demoted to dormant (Jul 13 2026)" — so a missing baseline-manifest update reads as policy, not an oversight. Do not add the `storybook-visual` label unless the user asks for the visual CI run. + +Open with `gh pr create --draft`. + +## 3. Drive to green + +Loop until done; report status to the user in plain language at each meaningful transition: + +1. **CI checks:** watch `gh pr checks` (the `pr.yml` jobs: policy, typecheck/release-registry, sharded general tests incl. UI vitest, build, serialized server, canary dry-run, e2e). Fix failures at the root cause and push; never weaken a test to pass it. Never hand-edit `pnpm-lock.yaml` — the policy gate blocks manual lockfile edits (regenerate via pnpm if manifests changed). +2. **commitperclip:** the bot posts one consolidated comment (template, linked-issue, dedup-search, test-coverage, lockfile, dependency gates). Fix whatever it flags until it reports all checks passing. +3. **Greptile:** it reviews automatically on open/update. Address **every** comment — fix it, or reply with a concrete reasoned justification; never ignore or resolve-without-answer. Push fixes and let it re-review until the score is **5/5 with no open P2s, recommendations, or follow-ups**. +4. Re-check 1–3 after every push (a Greptile fix can break CI and vice versa). + +If something is genuinely blocked on a human (a failing check unrelated to the change, a Greptile demand that contradicts the user's explicit decision), stop and report rather than forcing it. + +## 4. Finish + +When checks are green, commitperclip passes, and Greptile is 5/5 clean: `gh pr ready` to mark it ready for review, then give the user the PR URL and a one-paragraph plain-language summary of what shipped and what was verified. Merging stays with human maintainers. + +## Installing this skill for Paperclip agents + +Claude Code picks this skill up automatically via the committed symlink in `.claude/skills/`. To make it available to Paperclip's own agents: Skills page → "Scan project workspaces" (or `POST /api/companies/:companyId/skills/scan-projects`), then enable it per agent in the agent's Skills tab. Repo skills surface under two keys (project-scan `local//...` plus adapter-native `paperclipai/paperclip/...`), so a bare slug is ambiguous when enabling via API — use the full key, e.g. `paperclipai/paperclip/pr-to-green`.