diff --git a/.agents/skills/check-pr/SKILL.md b/.agents/skills/check-pr/SKILL.md new file mode 100644 index 00000000000..88d52c10f0f --- /dev/null +++ b/.agents/skills/check-pr/SKILL.md @@ -0,0 +1,379 @@ +--- +name: check-pr +description: > + Checks a GitHub, GitLab, or Perforce (p4) pull request (or merge request, or shelved changelist) + for unresolved review comments, failing status checks, and incomplete PR descriptions. Waits for + pending checks to complete, categorizes issues as actionable or informational, and optionally fixes + and resolves them. Use when the user wants to check a PR/MR/CL, address review feedback, or prepare + a change for submission. +license: MIT +compatibility: Requires git and gh (GitHub CLI), glab (GitLab CLI), or p4 (Perforce CLI) installed and authenticated. +metadata: + author: greptileai + version: "1.3" +allowed-tools: Bash(gh:*) Bash(glab:*) Bash(git:*) Bash(p4:*) +--- + +# Check PR + +Analyze a pull request (GitHub), merge request (GitLab), or shelved changelist (Perforce) for review comments, status checks, and description completeness, then help address any issues found. + +## Inputs + +- **PR/MR/CL number** (optional): If not provided, detect the PR/MR for the current branch, or the default pending changelist for p4. + +## Instructions + +### 0. Detect platform + +First check if the user is working in a Perforce depot by looking for a `.p4config` file or `P4CLIENT`/`P4PORT` environment variables: + +```bash +# Check for Perforce environment +if p4 info >/dev/null 2>&1; then + VCS="perforce" +else + # Fall back to git remote detection + REMOTE_URL=$(git remote get-url origin) + if echo "$REMOTE_URL" | grep -qi "gitlab"; then + VCS="gitlab" + else + VCS="github" + fi +fi +``` + +For self-hosted GitLab instances whose hostname doesn't contain "gitlab", the user can override by passing `--vcs gitlab` as an input. For Perforce, the user can override by passing `--vcs perforce`. + +### 1. Identify the PR/MR/CL + +If a number was provided, use it. Otherwise, detect it: + +**GitHub:** +```bash +gh pr view --json number,headRefName,headRefOid -q '{number: .number, branch: .headRefName, head: .headRefOid}' +``` + +**GitLab:** +```bash +glab mr view --output json | jq '.iid' +``` + +**Perforce:** +```bash +# List pending changelists for the current user/client +p4 changes -s pending -u $P4USER -c $P4CLIENT +``` + +Key field differences between platforms: +- GitHub: `number`, `headRefName`, `headRefOid` +- GitLab: `iid`, `source_branch`, `sha` +- Perforce: changelist number (CL), `shelved` files for in-review CLs + +### 2. Fetch PR/MR/CL details + +**GitHub:** +```bash +gh pr view --json title,body,state,reviews,comments,headRefName,headRefOid,statusCheckRollup +OWNER_REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner) +gh api "repos/$OWNER_REPO/pulls//comments" +gh api --paginate "repos/$OWNER_REPO/issues//comments?per_page=100" +``` + +GitHub PRs are also issues, so general PR comments live on the issue comments endpoint. Greptile may edit a single general PR comment on each review cycle instead of creating a new review or comment. Always inspect the latest Greptile-authored general comment by `updated_at`, including any "Prompt to fix all with AI" section, before concluding that the PR is clear. + +**GitLab:** +```bash +glab mr view --output json +# Fetch discussions (inline diff comments are type "DiffNote"; general comments have null type) +glab api "projects/:fullpath/merge_requests//discussions" +``` + +For GitLab, paginate discussions if needed (add `?per_page=100&page=N`). + +**Perforce:** +```bash +# Get changelist description, files, and status +p4 describe -s + +# Get shelved files (for in-review CLs) +p4 describe -S + +# Get the diff of the shelved changelist +p4 diff2 //...@= //...@= + +# List review comments (if using p4 review workflow) +p4 review -c +``` + +Key Perforce CL fields: +- `Change`: changelist number +- `Status`: `pending`, `submitted`, `shelved` +- `Description`: the CL description / commit message +- `Files`: list of files in the CL + +### 3. Wait for pending checks + +Before analyzing, ensure all status checks have completed. If any checks are `PENDING` or `IN_PROGRESS` (GitHub) / `running` or `pending` (GitLab), poll every 30 seconds until all checks reach a terminal state. + +**GitHub:** poll `statusCheckRollup` from `gh pr view`. + +**GitLab:** +```bash +glab api "projects/:fullpath/merge_requests//pipelines" +``` +Pipeline statuses: `running`, `pending`, `success`, `failed`, `canceled`, `skipped`. Poll until no pipeline has `running` or `pending` status. + +**Perforce:** Perforce doesn't have built-in CI checks natively. If the team uses a review tool (Swarm, etc.) or an external CI triggered by shelve events, check the relevant system. Otherwise, proceed to analysis immediately. + +### 4. Require a fresh Greptile review for the current head + +For GitHub PRs, do not treat an existing Greptile review, comment, or summary as current unless it is tied to the PR's exact current `headRefOid`. This is especially important after pushing a new commit to an existing PR: a Greptile review on an older commit is stale, even if the PR still has a Greptile comment or prior review. + +Fetch the current head SHA immediately before the Greptile gate: + +```bash +HEAD_SHA=$(gh pr view --json headRefOid -q .headRefOid) +``` + +Then inspect check-runs for that commit and require a completed Greptile run: + +```bash +OWNER_REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner) +GREPTILE_CHECKS=$(gh api "repos/$OWNER_REPO/commits/$HEAD_SHA/check-runs?per_page=100" \ + --jq '[.check_runs[] | select(.name | test("greptile"; "i"))]') + +# A run only counts as a valid fresh pass when it has completed AND concluded cleanly. +# GitHub check-run conclusions: success, neutral, skipped, failure, timed_out, +# cancelled, action_required, stale. Treat success/neutral as clean; +# everything else (especially failure and action_required) must block. +FRESH_GREPTILE_COMPLETED=$(echo "$GREPTILE_CHECKS" \ + | jq '[.[] | select(.status == "completed")] | length') +FRESH_GREPTILE_CLEAN=$(echo "$GREPTILE_CHECKS" \ + | jq '[.[] | select(.status == "completed" and (.conclusion | IN("success","neutral")))] | length') +FRESH_GREPTILE_BLOCKING=$(echo "$GREPTILE_CHECKS" \ + | jq '[.[] | select(.status == "completed" and ((.conclusion | IN("success","neutral")) | not))] | length') + +if [ "$FRESH_GREPTILE_COMPLETED" = "0" ]; then + echo "Blocked: no completed Greptile review/check is tied to current PR head $HEAD_SHA." + echo "Request a fresh Greptile review against this head before marking the PR check done." + echo "Suggested trigger: gh pr comment --body \"@greptile review\"" + exit 1 +fi + +if [ "$FRESH_GREPTILE_BLOCKING" != "0" ] || [ "$FRESH_GREPTILE_CLEAN" = "0" ]; then + echo "Blocked: Greptile completed on head $HEAD_SHA but did not conclude clean" + echo "(conclusion was failure/action_required/timed_out/cancelled/stale, or no clean run exists)." + echo "Address the findings, push, and re-run Greptile until it concludes success/clean on the new head." + exit 1 +fi +``` + +If a Greptile check exists for the current head but is still pending or in progress, wait for it with the same polling pattern used by `greploop` rather than proceeding from older review material. If no Greptile check appears for the current head after a reasonable wait, report the PR as blocked on a fresh Greptile review for `HEAD_SHA` and stop. Do not mark the check complete from PR comments, PR reviews, or Greptile summaries that cannot be associated with the current head SHA. + +For GitLab installations with Greptile integration, apply the same freshness rule against the MR's current head SHA: + +```bash +# 1. Get the MR's current head SHA +MR_SHA=$(glab mr view --output json | jq -r '.sha // .diff_refs.head_sha') + +# 2. Find the latest pipeline for that EXACT sha, then the Greptile job within it. +LATEST_PIPELINE_ID=$(glab api "projects/:fullpath/merge_requests//pipelines" \ + | jq -r --arg sha "$MR_SHA" '[.[] | select(.sha == $sha)] | sort_by(.id) | last | .id // empty') + +if [ -n "$LATEST_PIPELINE_ID" ]; then + GREPTILE_JOBS=$(glab api "projects/:fullpath/pipelines/$LATEST_PIPELINE_ID/jobs" \ + | jq --arg sha "$MR_SHA" '[.[] | select(.name | test("greptile"; "i")) + | {name, status, pipeline_sha: $sha}]') +else + GREPTILE_JOBS='[]' +fi + +GREPTILE_JOB_SUCCESS=$(echo "$GREPTILE_JOBS" \ + | jq '[.[] | select(.status == "success")] | length') +GREPTILE_JOB_BLOCKING=$(echo "$GREPTILE_JOBS" \ + | jq '[.[] | select(.status != "success")] | length') + +# 3. If Greptile integrates via MR notes instead of a CI job, require the newest +# Greptile note to reference the current head sha and report a clean review. +GREPTILE_NOTES=$(glab api "projects/:fullpath/merge_requests//discussions?per_page=100" \ + | jq --arg sha "$MR_SHA" '[.[].notes[] + | select(.author.username | test("greptile"; "i"))] + | sort_by(.updated_at // .created_at)') +GREPTILE_NOTE_CLEAN=$(echo "$GREPTILE_NOTES" \ + | jq --arg sha "$MR_SHA" 'if length == 0 then 0 + elif ((last.body // "") | contains($sha) and test("Confidence Score:[[:space:]]*5/5|Confidence:[[:space:]]*5/5|\\b5/5\\b"; "i") and (test("Prompt To Fix|blocking issue|failed|action required"; "i") | not)) then 1 + else 0 end') + +if [ "$GREPTILE_JOB_SUCCESS" = "0" ] && [ "$GREPTILE_NOTE_CLEAN" = "0" ]; then + echo "Blocked: no successful Greptile job or completed-clean current-head Greptile note is tied to MR head $MR_SHA." + exit 1 +fi + +if [ "$GREPTILE_JOB_BLOCKING" != "0" ]; then + echo "Blocked: at least one Greptile job for MR head $MR_SHA did not succeed." + exit 1 +fi +``` + +Block completion if, for `MR_SHA`, there is (a) no Greptile job or note at all (missing), (b) the newest Greptile job/note is tied to a different sha (stale), or (c) the Greptile job status is not `success`. A completed Greptile result for a different SHA is stale and must block completion. + +For Perforce installations with Greptile integration, apply the same rule using the CL's current shelved-revision identity and the Greptile webhook/review artifact tied to it; a Greptile result for an earlier shelf is stale and must block completion. + +### 5. Analyze the PR/MR + +Once all checks are complete, evaluate these areas: + +#### A. Status Checks + +- Are all CI checks passing? +- If any are failing, identify which ones and the failure reason. + +#### B. PR/MR Description + +- Is the description complete and follows team conventions? +- Are all required sections filled in? +- Are there TODOs or placeholders that need updating? + +#### C. Review Comments + +- Inline code review comments that need addressing +- Look for bot review comments (e.g. from `greptile-apps[bot]` on GitHub, or the Greptile bot user on GitLab, linters, etc.) +- Human reviewer comments +- **Perforce:** review comments from `p4 review` or external review tools + +#### D. General Comments + +- Discussion comments on the PR/MR +- For GitHub, check the issue comments endpoint and use `updated_at` to catch bot comments edited in place. Greptile's latest edited summary can contain actionable items even when there are no new inline comments. +- Bot comments (deploy previews, etc.) - usually informational +- **Perforce:** CL description should include a clear summary, affected files rationale, and testing notes + +### 6. Categorize issues + +For each issue found, categorize as: + +| Category | Meaning | +|---|---| +| **Actionable** | Code changes, test improvements, or fixes needed | +| **Informational** | Verification notes, questions, or FYIs that don't require changes | +| **Already addressed** | Issues that appear to be resolved by subsequent commits | + +### 7. Report findings + +Present a summary table: + +| Area | Issue | Status | Action Needed | +|------|-------|--------|---------------| +| Status Checks | CI build failing | Failing | Fix type error in `src/api.ts` | +| Review | "Add null check" - @reviewer | Actionable | Add guard clause | +| Description | TODO placeholder in test plan | Actionable | Fill in test plan | +| Review | "Looks good" - @teammate | Informational | None | + +### 8. Fix issues (if requested) + +If there are actionable items: + +1. Switch to the PR/MR's branch (git) or ensure files are open in the correct CL (Perforce) if not already. +2. Ask the user if they want to fix the issues. +3. If yes, make the fixes, then: + +**GitHub/GitLab:** commit and push: +```bash +git add +git commit -m "address review feedback" +git push +``` + +**Perforce:** open files for edit, make changes, and re-shelve: +```bash +p4 edit +# make changes +p4 shelve -f -c +``` + +### 9. Resolve review threads + +After addressing comments, resolve the corresponding review threads. + +**Perforce** - Perforce does not have a native "resolve thread" concept. Instead, mark comments as addressed by updating the CL description or by responding in the review tool being used (Swarm, etc.). If using `p4 review`: + +```bash +# Mark files as reviewed after addressing feedback +p4 review -c +``` + +**GitHub** - fetch unresolved thread IDs (paginate if needed - see [the GraphQL reference](references/graphql-queries.md)): + +```bash +gh api graphql -f query=' +query($cursor: String) { + repository(owner: "OWNER", name: "REPO") { + pullRequest(number: PR_NUMBER) { + reviewThreads(first: 100, after: $cursor) { + pageInfo { hasNextPage endCursor } + nodes { + id + isResolved + comments(first: 1) { + nodes { body path } + } + } + } + } + } +}' +``` + +If `hasNextPage` is true, repeat with `-f cursor=ENDCURSOR` to get remaining threads. + +Then resolve threads that have been addressed or are informational: + +```bash +gh api graphql -f query=' +mutation { + resolveReviewThread(input: {threadId: "THREAD_ID"}) { + thread { isResolved } + } +}' +``` + +Batch multiple resolutions into a single mutation using aliases (`t1`, `t2`, etc.). + +**GitLab** - fetch unresolved discussions (see [the GitLab API reference](references/gitlab-api.md)): + +```bash +glab api "projects/:fullpath/merge_requests//discussions?per_page=100" +``` + +Filter for discussions where `"resolved": false`. Collect each discussion's `id`. + +Resolve each discussion individually (GitLab has no batch resolution): + +```bash +glab api --method PUT \ + "projects/:fullpath/merge_requests//discussions/" \ + --field resolved=true +``` + +Repeat for each unresolved discussion ID. + +### 10. Multiple PRs/MRs/CLs + +If checking a chain of PRs/MRs/CLs, process them sequentially. + +**Perforce** - to check multiple changelists at once: +```bash +p4 changes -s pending -u $P4USER -c $P4CLIENT -l +``` + +## Output format + +Summarize: +- PR/MR/CL title or description and current state +- Platform detected (GitHub / GitLab / Perforce) +- Status checks summary (passing/failing/pending) - or N/A for Perforce +- Total issues found +- Actionable items with descriptions +- Items that can be ignored with reasons +- Recommended next steps diff --git a/.agents/skills/check-pr/references/gitlab-api.md b/.agents/skills/check-pr/references/gitlab-api.md new file mode 100644 index 00000000000..134fe6bd0f9 --- /dev/null +++ b/.agents/skills/check-pr/references/gitlab-api.md @@ -0,0 +1,91 @@ +# GitLab API Reference + +Useful GitLab REST API calls for working with merge request discussions, using `glab api`. + +`glab api` automatically resolves `:fullpath` to the URL-encoded project path from the local git remote. + +## Fetch MR details + +```bash +glab mr view --output json +``` + +Key fields (compared to GitHub equivalents): +- `iid` - internal MR number (use this, not `id`) +- `source_branch` - equivalent to GitHub's `headRefName` +- `sha` - HEAD commit SHA, equivalent to GitHub's `headRefOid` +- `description` - equivalent to GitHub's `body` + +## Fetch all discussions (inline + general comments) + +```bash +glab api "projects/:fullpath/merge_requests//discussions?per_page=100" +``` + +Paginate with `&page=2`, `&page=3`, etc. until the response array length is less than `per_page`. + +Each discussion object: +- `id` - discussion ID (used for resolution) +- `resolved` - `true` or `false` +- `notes` - array of note objects + +Each note object: +- `type` - `"DiffNote"` for inline diff comments, `null` for general comments +- `author.username` - author's username +- `body` - comment text +- `position.new_path` - file path (for `DiffNote` type) + +## Filter for unresolved inline diff comments + +```bash +glab api "projects/:fullpath/merge_requests//discussions?per_page=100" | \ + jq '[.[] | select(.resolved == false and (.notes[0].type == "DiffNote"))]' +``` + +## Resolve a single discussion + +```bash +glab api --method PUT \ + "projects/:fullpath/merge_requests//discussions/" \ + --field resolved=true +``` + +There is no batch resolution in GitLab - issue one PUT per discussion. + +## Fetch pipeline status for an MR + +```bash +glab api "projects/:fullpath/merge_requests//pipelines" +``` + +Pipeline statuses: `running`, `pending`, `success`, `failed`, `canceled`, `skipped`. + +## Fetch jobs for a specific pipeline + +```bash +glab api "projects/:fullpath/pipelines//jobs" +``` + +Each job has `name`, `status`, `stage`, and `web_url`. + +## Fetch MR notes (general comments and bot reviews) + +```bash +glab api "projects/:fullpath/merge_requests//notes?per_page=100" +``` + +Filter by `author.username` to find Greptile bot comments. The exact bot username depends on the Greptile installation - check the first Greptile comment to identify it. + +## Post a comment on an MR + +```bash +glab mr note --message "your message here" +``` + +Or via API: + +```bash +glab api --method POST \ + "projects/:fullpath/merge_requests//notes" \ + --field body="your message here" +``` diff --git a/.agents/skills/check-pr/references/graphql-queries.md b/.agents/skills/check-pr/references/graphql-queries.md new file mode 100644 index 00000000000..3a3889e3006 --- /dev/null +++ b/.agents/skills/check-pr/references/graphql-queries.md @@ -0,0 +1,89 @@ +# GraphQL Queries Reference + +Useful GitHub GraphQL queries for working with PR review threads. + +## Fetch unresolved review threads (with pagination) + +```graphql +query($cursor: String) { + repository(owner: "OWNER", name: "REPO") { + pullRequest(number: PR_NUMBER) { + reviewThreads(first: 100, after: $cursor) { + pageInfo { + hasNextPage + endCursor + } + nodes { + id + isResolved + comments(first: 3) { + nodes { + body + path + author { login } + createdAt + } + } + } + } + } + } +} +``` + +Pass `-f cursor=ENDCURSOR` on subsequent requests if `hasNextPage` is `true`. + +## Resolve a single review thread + +```graphql +mutation { + resolveReviewThread(input: {threadId: "THREAD_ID"}) { + thread { isResolved } + } +} +``` + +## Batch-resolve multiple threads + +Use GraphQL aliases to resolve several threads in one request: + +```graphql +mutation { + t1: resolveReviewThread(input: {threadId: "THREAD_ID_1"}) { + thread { isResolved } + } + t2: resolveReviewThread(input: {threadId: "THREAD_ID_2"}) { + thread { isResolved } + } + t3: resolveReviewThread(input: {threadId: "THREAD_ID_3"}) { + thread { isResolved } + } +} +``` + +## Fetch PR details (REST) + +```bash +gh pr view --json title,body,state,reviews,comments,headRefName,statusCheckRollup +``` + +## Fetch inline review comments (REST) + +```bash +OWNER_REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner) +gh api "repos/$OWNER_REPO/pulls//comments" +``` + +## Fetch general PR comments edited in place (REST) + +General PR comments are issue comments. Greptile may update one summary comment repeatedly, so select by `updated_at` instead of `created_at`: + +```bash +OWNER_REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner) +gh api --paginate "repos/$OWNER_REPO/issues//comments?per_page=100" \ + | jq -s 'add + | map(select(.user.login | test("greptile"; "i"))) + | sort_by(.updated_at) + | last + | {author: .user.login, updated_at, body}' +``` diff --git a/.claude/agents/codemod-runner.md b/.claude/agents/codemod-runner.md new file mode 100644 index 00000000000..3fafe19b69f --- /dev/null +++ b/.claude/agents/codemod-runner.md @@ -0,0 +1,17 @@ +--- +name: codemod-runner +description: Writes and runs codemod scripts that replace hardcoded visual values with token references in ui/src/index.css. Use for Phase 2 of the design simplification run — mechanical refactors only. +tools: Read, Write, Edit, Bash, Glob, Grep +model: sonnet +--- + +You perform mechanical refactors via scripts, never hand-edits. Follow DESIGN.md at the repo root. + +Rules: + +- The token destination is ui/src/index.css (Tailwind v4), optionally a tokens.css imported by it. NEVER create a parallel token source. Tokens that must be runtime-tunable go in a NON-inline block — `@theme inline` bakes literals at build time. +- Where a hardcoded value EXACTLY matches an existing token, replace it with that token reference. Otherwise extract the value into a new token VERBATIM — no normalizing, rounding, or inventing a scale. Ugly values stay ugly. +- Every rewrite happens through a codemod script committed to scripts/ before it is run. Scripts must be idempotent and reviewable. +- Third-party style overrides that cannot use tokens go on a documented allowlist in the token source, each with an inline comment saying why. +- Verify after every script run: rg gates (zero hardcoded hex, zero arbitrary px/bracket values in ui/src/components/** and ui/src/pages/** outside the allowlist), pnpm typecheck, and the Storybook snapshot suite. Snapshots must match the Phase 0 baseline exactly. +- If a replacement cannot be made without visual change, skip it and record it in doc/design/TOKEN-AUDIT.md under "Needs human decision". diff --git a/.claude/agents/token-auditor.md b/.claude/agents/token-auditor.md new file mode 100644 index 00000000000..07561c80520 --- /dev/null +++ b/.claude/agents/token-auditor.md @@ -0,0 +1,16 @@ +--- +name: token-auditor +description: Scans ui/src/ for hardcoded visual values, duplicate components, and shadcn replacement candidates; produces doc/design/TOKEN-AUDIT.md and doc/design/COMPONENT-INVENTORY.md. Read-only on source — never modifies component files. Use for Phase 1 of the design simplification run. +tools: Read, Grep, Glob, Bash +model: sonnet +--- + +You inventory design-system debt in this repository. Follow DESIGN.md at the repo root; read doc/design/PRIOR-ART.md first — a previous audit found only 6 of ~220 drift sites were exact-value-mappable to existing tokens, so expect most hardcoded values to need new verbatim tokens. + +Your outputs (written to the repo root): + +1. TOKEN-AUDIT.md — every hardcoded color/spacing/radius/type/shadow value in ui/src/, with frequency, file locations, and near-duplicate clusters (e.g. 13/14/15px used interchangeably). For each value, note whether it EXACTLY matches one of the ~80 existing tokens in ui/src/index.css (semantic / brand / domain tiers — see DESIGN.md). Flag clusters for human review; never merge or normalize them. Include a "Needs human decision" section. + +2. COMPONENT-INVENTORY.md — all components under ui/src/components/ (24 primitives in ui/, ~277 feature components), their variants, and suspected duplicates with evidence (similar props, similar rendered output, copy-pasted origins). Include a "shadcn candidates" section: (a) custom components duplicating an available shadcn primitive, (b) installed shadcn components that drifted from the registry (npx shadcn@latest diff where available), (c) raw Radix/plain elements where an installed shadcn wrapper exists. For each, state the recommended replacement and expected visual impact. Recommendations only — merges and swaps happen in later human-approved runs, never this one. + +Never modify source files. Bash access is for read-only commands (rg, find, npx shadcn diff) and writing the two report files only. diff --git a/.github/workflows/storybook-visual.yml b/.github/workflows/storybook-visual.yml new file mode 100644 index 00000000000..325ca62696a --- /dev/null +++ b/.github/workflows/storybook-visual.yml @@ -0,0 +1,102 @@ +name: Storybook Visual + +on: + workflow_dispatch: + inputs: + update_snapshots: + description: "Generate updated snapshots and a baseline review bundle" + required: false + type: boolean + default: false + pull_request: + branches: + - master + types: + - opened + - reopened + - synchronize + - labeled + +concurrency: + group: storybook-visual-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + visual: + name: Storybook visual regression + if: >- + github.event_name == 'workflow_dispatch' || + (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'storybook-visual')) + runs-on: ubuntu-latest + timeout-minutes: 35 + + steps: + - name: Checkout repository + uses: actions/checkout@v7 + + - name: Setup pnpm + uses: pnpm/action-setup@v6 + with: + version: 9.15.4 + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: 24 + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Install Playwright Chromium + run: npx playwright install --with-deps chromium + + - name: Cache Storybook visual baseline archive + uses: actions/cache@v5 + with: + path: tests/storybook-visual/.cache + key: storybook-visual-baseline-${{ runner.os }}-${{ hashFiles('tests/storybook-visual/baseline-manifest.json') }} + + - name: Download Storybook visual baseline + run: pnpm storybook-visual:baseline download + + - name: Verify Storybook visual baseline + run: pnpm storybook-visual:baseline verify + + - name: Build Storybook + run: pnpm build-storybook + + - name: Run Storybook visual tests + if: ${{ github.event.inputs.update_snapshots != 'true' }} + run: npx playwright test --config tests/storybook-visual/playwright.config.ts + + - name: Generate updated snapshots for review + if: ${{ github.event.inputs.update_snapshots == 'true' }} + run: npx playwright test --config tests/storybook-visual/playwright.config.ts --update-snapshots + + - name: Pack updated baseline review bundle + if: ${{ github.event.inputs.update_snapshots == 'true' }} + run: pnpm storybook-visual:baseline pack + + - name: Upload Storybook visual report + uses: actions/upload-artifact@v7 + if: always() + with: + name: storybook-visual-report-${{ github.run_id }} + path: | + tests/storybook-visual/playwright-report/ + tests/storybook-visual/test-results/ + retention-days: 30 + if-no-files-found: warn + + - name: Upload updated baseline review bundle + uses: actions/upload-artifact@v7 + if: ${{ github.event.inputs.update_snapshots == 'true' }} + with: + name: storybook-visual-baseline-review-${{ github.run_id }} + path: tests/storybook-visual/baseline-review/snapshots.tgz + retention-days: 30 + if-no-files-found: error diff --git a/.gitignore b/.gitignore index 09af52ce165..63de6b12829 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ diagnostics/ .vscode/ .claude/settings.local.json .paperclip-local/ +.paperclip-runtime/ /.idea/ /.agents/ @@ -56,6 +57,11 @@ tests/e2e/test-results/ tests/e2e/playwright-report/ tests/release-smoke/test-results/ tests/release-smoke/playwright-report/ +tests/storybook-visual/.cache/ +tests/storybook-visual/.snapshots/ +tests/storybook-visual/baseline-review/ +tests/storybook-visual/test-results/ +tests/storybook-visual/playwright-report/ .superset/ .superpowers/ .claude/worktrees/ diff --git a/AGENTS.md b/AGENTS.md index ddefe527d6a..cc341f6861c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -220,3 +220,7 @@ PR #2218 (`feat/external-adapter-phase1`) adds external adapter support. See roo - `createServerAdapter()` must include ALL optional fields (especially `detectModel`) - Built-in UI adapters can shadow external plugin parsers; external override pause/resume should restore the built-in parser. - Reference external adapters: Droid (npm); Hermes can also be tested as an override package. + +## Design system + +`DESIGN.md` at the repo root is the source of truth for UI design decisions. The token-only rule applies to all `ui/` changes: every color, spacing, radius, type, shadow, and motion value in `ui/src/components/**` and `ui/src/pages/**` comes from the token layer in `ui/src/index.css` — no hex, raw px, arbitrary Tailwind bracket values, or raw `font-size`/`fontSize` declarations in components, outside the documented allowlist in `ui/src/index.css`. Run `pnpm check:token-gates` (`scripts/check-token-gates.mjs`) before committing UI changes — it fails on any violation not covered by that allowlist. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 10849c754c6..ee22a1b7bdf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -102,6 +102,10 @@ Every PR must include a **Model Used** section specifying which AI model produce All tests must pass before a PR can be merged. Run them locally first and verify CI is green after pushing. +### Telemetry Changes + +If your change adds, removes, or modifies emitted telemetry events, update the [Telemetry Data Contract](packages/shared/src/telemetry/README.md) in the same PR. Keep clients emitting raw dimension values and avoid documenting or relying on private delivery details. + ### Paperclip Gates Must Pass All Paperclip CI gates (lint, typecheck, tests, build, and any other required checks) must be satisfied before a PR can be merged. Don't ask for a merge while gates are red — fix them first. diff --git a/DESIGN.md b/DESIGN.md new file mode 100644 index 00000000000..6eaa1e12cf7 --- /dev/null +++ b/DESIGN.md @@ -0,0 +1,58 @@ +# 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." + +## 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`. diff --git a/Dockerfile b/Dockerfile index a6631b71f51..2610730d712 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,7 +24,6 @@ COPY packages/adapter-utils/package.json packages/adapter-utils/ COPY packages/mcp-server/package.json packages/mcp-server/ COPY packages/skills-catalog/package.json packages/skills-catalog/ COPY packages/teams-catalog/package.json packages/teams-catalog/ -COPY packages/adapters/acpx-local/package.json packages/adapters/acpx-local/ COPY packages/adapters/claude-local/package.json packages/adapters/claude-local/ COPY packages/adapters/codex-local/package.json packages/adapters/codex-local/ COPY packages/adapters/cursor-cloud/package.json packages/adapters/cursor-cloud/ diff --git a/README.md b/README.md index 90a61dfda91..99ac6631435 100644 --- a/README.md +++ b/README.md @@ -412,6 +412,8 @@ Paperclip ships with opt-in OpenTelemetry auto-instrumentation for the server (t Paperclip collects anonymous usage telemetry to help us understand how the product is used and improve it. No personal information, issue content, prompts, file paths, or secrets are ever collected. Private repository references are hashed with a per-install salt before being sent. +Contributors changing emitted telemetry events should follow the [Telemetry Data Contract](packages/shared/src/telemetry/README.md). + Telemetry is **enabled by default** and can be disabled with any of the following: | Method | How | diff --git a/cli/package.json b/cli/package.json index 34f43861151..08656b56cc7 100644 --- a/cli/package.json +++ b/cli/package.json @@ -37,7 +37,6 @@ }, "dependencies": { "@clack/prompts": "^0.10.0", - "@paperclipai/adapter-acpx-local": "workspace:*", "@paperclipai/adapter-claude-local": "workspace:*", "@paperclipai/adapter-codex-local": "workspace:*", "@paperclipai/adapter-cursor-cloud": "workspace:*", diff --git a/cli/src/__tests__/company-delete.test.ts b/cli/src/__tests__/company-delete.test.ts index 8865585e751..d45fc120226 100644 --- a/cli/src/__tests__/company-delete.test.ts +++ b/cli/src/__tests__/company-delete.test.ts @@ -23,6 +23,7 @@ function makeCompany(overrides: Partial): Company { brandColor: null, logoAssetId: null, logoUrl: null, + defaultResponsibleUserId: null, createdAt: new Date(), updatedAt: new Date(), ...overrides, diff --git a/cli/src/__tests__/secrets.test.ts b/cli/src/__tests__/secrets.test.ts index 89ea9840091..295a8e5e036 100644 --- a/cli/src/__tests__/secrets.test.ts +++ b/cli/src/__tests__/secrets.test.ts @@ -46,6 +46,9 @@ function secret(partial: Partial): CompanySecret { return { id: "secret-1", companyId: "company-1", + scope: "company", + ownerUserId: null, + userSecretDefinitionId: null, key: "agent_agent-12_anthropic_api_key", name: "agent_agent-12_anthropic_api_key", provider: "local_encrypted", diff --git a/cli/src/adapters/registry.ts b/cli/src/adapters/registry.ts index 260a45d6a3b..f30e32eafb6 100644 --- a/cli/src/adapters/registry.ts +++ b/cli/src/adapters/registry.ts @@ -1,5 +1,4 @@ import type { CLIAdapterModule } from "@paperclipai/adapter-utils"; -import { printAcpxStreamEvent } from "@paperclipai/adapter-acpx-local/cli"; import { printClaudeStreamEvent } from "@paperclipai/adapter-claude-local/cli"; import { printCodexStreamEvent } from "@paperclipai/adapter-codex-local/cli"; import { printCursorStreamEvent } from "@paperclipai/adapter-cursor-local/cli"; @@ -19,11 +18,6 @@ const claudeLocalCLIAdapter: CLIAdapterModule = { formatStdoutEvent: printClaudeStreamEvent, }; -const acpxLocalCLIAdapter: CLIAdapterModule = { - type: "acpx_local", - formatStdoutEvent: printAcpxStreamEvent, -}; - const codexLocalCLIAdapter: CLIAdapterModule = { type: "codex_local", formatStdoutEvent: printCodexStreamEvent, @@ -76,7 +70,6 @@ const openclawGatewayCLIAdapter: CLIAdapterModule = { const adaptersByType = new Map( [ - acpxLocalCLIAdapter, claudeLocalCLIAdapter, codexLocalCLIAdapter, openCodeLocalCLIAdapter, diff --git a/doc/DATABASE.md b/doc/DATABASE.md index 43dfc05a8af..a979cefcaac 100644 --- a/doc/DATABASE.md +++ b/doc/DATABASE.md @@ -143,6 +143,19 @@ The database mode is controlled by `DATABASE_URL`: Your Drizzle schema (`packages/db/src/schema/`) stays the same regardless of mode. +## Migration authoring checklist + +The 0126 issue comment attribution backfill showed the failure mode this checklist is meant to prevent: each batch looked for the next rows with an unindexed predicate, so PostgreSQL repeatedly scanned the same table and the migration became O(n²) as the table grew. + +When authoring migrations or one-time backfills: + +- Create the supporting index for the batch predicate before the backfill loop runs. +- Bound batches by an indexed key, such as an id range or keyset pagination cursor. Do not use `OFFSET` pagination or a query shape that re-scans already-visited rows each batch. +- Avoid unbounded full-table `UPDATE` or `DELETE` statements. Add a selective predicate and process rows in bounded batches when table size can be large. +- Use `CREATE INDEX CONCURRENTLY` for large existing tables when the migration can run outside a transaction and must avoid long write locks. +- Split schema changes, index creation, and data backfill into separate phases so each step has clear locking and rollback behavior. +- Treat the `check:migrations` CI gate as the enforcement backstop for these rules. If it flags a migration, rewrite the migration or add a suppression comment with the indexed predicate, batch bound, and reason the remaining scan is safe. + ## Resource membership tables Paperclip stores current-user sidebar membership state in: @@ -174,11 +187,21 @@ up separately when you need full instance disaster recovery. Paperclip stores secret metadata and versions in: +- `user_secret_definitions` +- `user_secret_declarations` - `company_secrets` - `company_secret_versions` - `company_secret_bindings` - `secret_access_events` +Company secrets use `company_secrets.scope = 'company'` and are bound directly +through `company_secret_bindings`. User-specific secrets reuse the same provider +and version storage, but each value is a `company_secrets.scope = 'user'` row +with `owner_user_id` and `user_secret_definition_id` set. Definitions describe +the reusable company-level slot, declarations record where `user_secret_ref` +bindings are required, and the concrete value is selected later for the +responsible user. + Secret-aware env bindings are supported by agents, projects, and routines. Routine env lives in `routines.env`, is captured in `routine_revisions.snapshot`, and routine dispatches store `routine_runs.routine_revision_id` so runtime secret resolution uses the env snapshot that existed when the run was created. Routine secret refs bind with `target_type = 'routine'`, `target_id = routines.id`, and `config_path` values under `env.*`. For local/default installs, the active provider is `local_encrypted`: @@ -188,6 +211,10 @@ For local/default installs, the active provider is `local_encrypted`: - CLI config location: `~/.paperclip/instances/default/config.json` under `secrets.localEncrypted.keyFilePath`. - Backup/restore requires both the database metadata and the local master key file; either artifact alone is insufficient. - The server best-effort enforces `0600` key file permissions and provider health reports permission warnings. +- User-scoped values use the same local encrypted provider path. Database + backups preserve definitions, declarations, owner metadata, version metadata, + and access events, but restored user-scoped values are decryptable only when + the matching local master key is restored with the database. Optional overrides: diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 28155b9f530..e3baa0a3921 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -56,6 +56,46 @@ pnpm build-storybook These run the `@paperclipai/ui` Storybook on port `6006` and build the static output to `ui/storybook-static/`. +The Storybook visual regression suite uses external PNG baselines instead of +committed screenshots: + +```sh +pnpm test:storybook-visual +pnpm test:storybook-visual:update +``` + +`pnpm test:storybook-visual` downloads and verifies the baseline archive from +`tests/storybook-visual/baseline-manifest.json` before running Playwright. +Accepted visual changes should update the manifest metadata and publish a new +immutable archive with `pnpm storybook-visual:baseline pack` and +`pnpm storybook-visual:baseline upload`; do not commit generated PNG snapshots. + +Known limitation: Storybook visual baselines are Linux/Ubuntu-only. The manifest +pins the capture environment to `ubuntu-24.04` and the Playwright suite uses +pixel-exact comparison, so local runs on macOS, Windows, or other non-matching +platforms can report false-positive diffs from font rasterization and subpixel +rendering. Use the `Storybook Visual` GitHub Actions workflow on `ubuntu-latest` +as the source of truth, or run locally in a matching Linux environment before +accepting or updating baselines. + +PR visual checks are opt-in while the suite stabilizes. Add the +`storybook-visual` label to a PR, or run the `Storybook Visual` GitHub Actions +workflow manually, to produce downloadable Playwright report/test-result +artifacts. Normal PR visual runs use read-only repository permissions and do not +upload or mutate baseline objects. + +## UI Fonts And Screenshots + +The board UI ships its own sans-serif webfont assets in `ui/public/fonts/`. +`ui/src/index.css` declares Inter v4.1 variable regular and italic faces and wires +the Tailwind `font-sans` token to those bundled files before system fallbacks. +Linux screenshot or Storybook capture jobs should not install host Inter packages +or inject external font CSS to make Paperclip text render correctly. + +Font assets live in Vite's public directory so `pnpm --filter @paperclipai/ui build` +emits them under `ui/dist/fonts/`. The server package copies the same output into +`server/ui-dist/fonts/` through `scripts/prepare-server-ui-dist.sh`. + Inspect or stop the current repo's managed dev runner: ```sh @@ -647,6 +687,14 @@ Environment overrides: - `PAPERCLIP_DB_BACKUP_INTERVAL_MINUTES=` - `PAPERCLIP_DB_BACKUP_RETENTION_DAYS=` - `PAPERCLIP_DB_BACKUP_DIR=/absolute/or/~/path` +- `PAPERCLIP_DB_BACKUP_MAX_AGE_HOURS=` controls the `/api/health` + stale-backup warning threshold +- `PAPERCLIP_DB_BACKUP_ALERT_FILE=/path/to/failure-marker` lets external cron + wrappers surface the last failed backup in `/api/health` + +Without `PAPERCLIP_DB_BACKUP_ALERT_FILE`, health checks look for +`db-backup-to-s3.failure` in the backup directory, beside the backup directory, +and in the default sibling `health/` directory. DB backups are not full instance filesystem backups. For full local disaster recovery, also back up local storage files and the local encrypted secrets key if diff --git a/doc/PUBLISHING.md b/doc/PUBLISHING.md index 11582e9718e..451853c3529 100644 --- a/doc/PUBLISHING.md +++ b/doc/PUBLISHING.md @@ -195,14 +195,14 @@ PR CI now checks changed release-enabled package manifests against npm. That cat The first publish of a brand-new package still needs one human maintainer with npm write access. After that, trusted publishing can take over. -Example for `@paperclipai/adapter-acpx-local` from the repo root: +Example for a newly added public package from the repo root: ```bash # safe preview -pnpm run release:bootstrap-package -- @paperclipai/adapter-acpx-local +pnpm run release:bootstrap-package -- @paperclipai/new-package # one-time first publish from an authenticated maintainer machine -pnpm run release:bootstrap-package -- @paperclipai/adapter-acpx-local --publish --otp 123456 +pnpm run release:bootstrap-package -- @paperclipai/new-package --publish --otp 123456 ``` The helper script: @@ -223,7 +223,7 @@ The helper now requires `--otp ` up front for `--publish`, so it fails bef After that first publish succeeds: -1. open `https://www.npmjs.com/package/@paperclipai/adapter-acpx-local` +1. open `https://www.npmjs.com/package/@paperclipai/new-package` 2. go to `Settings` → `Trusted publishing` 3. add repository `paperclipai/paperclip` 4. set workflow filename to `release.yml` diff --git a/doc/SECRETS-AWS-PROVIDER.md b/doc/SECRETS-AWS-PROVIDER.md index c7cce82e57a..4f150dccdfe 100644 --- a/doc/SECRETS-AWS-PROVIDER.md +++ b/doc/SECRETS-AWS-PROVIDER.md @@ -4,7 +4,7 @@ Operational contract for the hosted `aws_secrets_manager` secret provider used b ## Scope -- Hosted provider for Paperclip-managed secrets when Paperclip Cloud runs on AWS. +- Hosted provider for Paperclip-managed company and user-scoped secrets when Paperclip Cloud runs on AWS. - Source of truth for secret values is AWS Secrets Manager, not Postgres. - Paperclip stores only metadata needed for ownership, bindings, version selection, audit, and runtime resolution. - AWS provider bootstrap credentials are deployment/runtime credentials, not Paperclip-managed company secrets. @@ -120,6 +120,25 @@ Tag set for Paperclip-managed secrets: - `paperclip:secret-key=` - `paperclip:environment=` +When the user-secret service creates Paperclip-managed AWS values, keep them +under the same deployment/company namespace. The secret-key segment should be +non-sensitive and collision-safe for the user-secret value, normally derived +from the definition key plus an opaque credential-owner subject. Do not put +emails, personal names, OAuth scopes, ticket identifiers, or plaintext +credential material in AWS secret names, descriptions, or tags. + +For operator-owned external references, prefer a separate approved prefix such +as: + +```text +paperclip-ext///user-secrets// +``` + +Paperclip stores the external ref and provider version as metadata. Treat those +fields as secret-adjacent: they must be redacted from comments, activity logs, +run transcripts, issue documents, and broad board views unless a route is +explicitly designed to show sanitized provider metadata. + ## IAM And KMS Assumptions Launch posture: @@ -223,6 +242,14 @@ If selected external secrets use customer-managed KMS keys, also grant permissions scoped to `paperclip//*`; do not broaden them for remote import. +The same rule applies to user-scoped external refs. Paperclip derives the +responsible user and credential owner before resolution, but AWS IAM still +decides whether the runtime role can read the selected ARN/path. If the runtime +role can read a broad external prefix, that access is broader than Paperclip's +metadata policy. Use dedicated provider vaults, accounts, Regions, prefixes, +KMS keys, or runtime roles when provider-side isolation must match a +user-secret boundary. + Safe scoping guidance: - Prefer one Paperclip runtime role per environment/account. @@ -288,16 +315,24 @@ Guidance: What must survive: - Paperclip database metadata for secret ownership, bindings, status, and provider version refs. +- User-secret definitions, declarations, `company_secrets.scope = 'user'` + value rows, owner user ids, responsible-user snapshots, access-event + metadata, and provider version refs. - AWS Secrets Manager namespace under the configured deployment prefix. +- Any operator-owned external AWS prefixes linked by user-scoped external refs. - The configured KMS key and its decrypt permissions. Restore checklist: 1. Restore Paperclip database metadata. 2. Confirm the same AWS Secrets Manager namespace still exists. -3. Confirm the Paperclip runtime role can call `GetSecretValue` on the restored prefix. -4. Confirm the role still has decrypt access to the CMK referenced by `PAPERCLIP_SECRETS_AWS_KMS_KEY_ID`. -5. Run the live smoke below or a targeted runtime secret resolution test. +3. Confirm any linked user-scoped external prefixes still exist. +4. Confirm the Paperclip runtime role can call `GetSecretValue` on the restored + managed prefix and approved external prefixes. +5. Confirm the role still has decrypt access to the CMKs referenced by + `PAPERCLIP_SECRETS_AWS_KMS_KEY_ID` and by any external user-secret values. +6. Run the live smoke below or a targeted runtime secret resolution test for + both a company secret and a user-secret value. ## Provider Outage Runbook @@ -342,6 +377,8 @@ Response steps: 1. Stop or pause affected Paperclip runs. 2. Audit recent Paperclip secret access events for impacted secret ids and consumers. + For user-scoped incidents, include `credentialOwnerUserId`, + `responsibleUserId`, and `userSecretDefinitionId` in the review. 3. Audit AWS CloudTrail for `ListSecrets`, `GetSecretValue`, `PutSecretValue`, and `DeleteSecret` calls on the relevant vault account, Region, deployment prefix, and approved external prefixes. diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 94bd2da67a1..af9c808714d 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -83,9 +83,13 @@ V1 implementation extends this baseline into a company-centric, governance-aware - Revenue/expense accounting beyond model/token costs - Knowledge base subsystem - Public marketplace (ClipHub) -- Multi-board governance or role-based human permission granularity +- Multi-board governance (multiple board UIs for a single company) - Automatic self-healing orchestration (auto-reassign/retry planners) +Role-based human permission granularity is V1 — see the `humans-and-permissions` +plan, the `principal_permission_grants` table, and the `PERMISSION_KEYS` set +in `packages/shared/src/constants.ts`. + ## 6. Architecture ## 6.1 Runtime Components diff --git a/doc/design/AGENT-SESSIONS.md b/doc/design/AGENT-SESSIONS.md new file mode 100644 index 00000000000..5d837846891 --- /dev/null +++ b/doc/design/AGENT-SESSIONS.md @@ -0,0 +1,66 @@ +# Running an AI-agent session against the design system + +A guide for the *human* driving the session. You don't need to know the codebase — the system briefs the agent for you (AGENTS.md → `DESIGN.md` → `doc/design/`). Your job is to say what you want, look at pictures, and say yes or no. This document tells you how to do that well. + +## The golden rule + +**Describe outcomes, not implementations.** The whole point of the token system is that intent maps to small, safe changes: + +- ✅ "Make all the running indicators the same blue as the status chips." +- ✅ "The smallest text in the sidebar is hard to read — bump it one step." +- ✅ "Corners feel too sharp. Round everything slightly." +- ❌ "Edit line 1899 of IssueChatThread.tsx" (you'll be wrong, and it doesn't matter — the agent finds the sites) + +If your ask names a *feeling* ("too loud", "cramped", "inconsistent"), that's fine — expect the agent to translate it into a token change and show you the before/after to confirm the translation. + +## 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." + +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. + +**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." + +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". +> 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. + +## What to demand from the agent (your checklist) + +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. +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. +- **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. + +## Safety rails (and the forbidden moves) + +- **Worktrees for anything nontrivial.** Master is never touched until an explicit merge; the scrap path is deleting the worktree. +- **Pushing is a separate, explicit step.** Nothing goes to GitHub until you say push. +- Forbidden, always: `shadcn apply --preset` (overwrites components); re-baselining snapshots to hide an unexplained diff; hardcoding a value because a token "doesn't fit" (add the token instead); "fixing" the scheduled-debt areas ad hoc (palette classes, card/pill consolidation — see DECISION-SHEET). + +## 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). +- **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 + +Say what you want in plain language → agent turns it into token/component edits → you review before/after screenshots → you say "ship it" or say what's off → repeat. Every round today's system was built with took under an hour. That loop *is* the design workflow now. diff --git a/doc/design/CHANGING-THE-UI.md b/doc/design/CHANGING-THE-UI.md new file mode 100644 index 00000000000..49ee2a721ea --- /dev/null +++ b/doc/design/CHANGING-THE-UI.md @@ -0,0 +1,74 @@ +# Changing Paperclip's UI — a field guide + +How to make visual changes now that the design system exists. Written for everyone: designers, engineers, and AI agents (AGENTS.md points here via DESIGN.md). + +## The one-minute mental model + +Paperclip's look lives in **three layers**, and you almost always work in the first one: + +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. + +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 + +```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 +``` + +## 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. + +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). + +## Recipe 2 — retheme the whole app + +The core palette follows the shadcn token names, so a theme built at ui.shadcn.com/create applies as token values: + +```bash +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. + +## Recipe 3 — build or style a component + +- **Values**: tokens only. No hex, no `text-[11px]`, no `p-[13px]`. If no token fits, **add a token** — that's a feature, not a workaround. +- **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. +- `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 + +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. + +Three stories are known to flake under full parallel load (they pass in isolation — see DECISION-SHEET). Re-run a single story with `npx playwright test --config tests/storybook-visual/playwright.config.ts -g ""` before assuming a real failure. + +## For AI-agent sessions + +(Running a session as the human? See [`AGENT-SESSIONS.md`](AGENT-SESSIONS.md) — this section is instructions for the agent itself.) + +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) + +- **Tailwind palette classes** (`bg-red-500`, ~3,100 sites) — scheduled for a dedicated cluster-by-cluster conversion pass; piecemeal fixes will collide with it. +- **Hand-rolled cards/pills → `Card`/`Badge`**, sidebar agents-section unification — queued as a component-convergence pass with per-site snapshot verification. +- **ESLint ratchet** — will eventually enforce the token rules at lint time; until then `check:token-gates` is the gate. + +See `DECISION-SHEET.md` for the full ledger. diff --git a/doc/design/COMPONENT-INVENTORY.md b/doc/design/COMPONENT-INVENTORY.md new file mode 100644 index 00000000000..9e49c042e06 --- /dev/null +++ b/doc/design/COMPONENT-INVENTORY.md @@ -0,0 +1,406 @@ +# COMPONENT-INVENTORY.md — Component Inventory (Phase 1) + +Run scope: `ui/src/components/` and `ui/src/pages/` on branch `design/token-extraction`. Read-only audit — no source files modified. + +**All consolidation, merge, and shadcn-swap recommendations in this document are RECOMMENDATIONS ONLY.** Per `GOAL-PROMPT.md` and `DESIGN.md`, no component merges, deletions, or swaps happen in this run. They become human-approved follow-up runs ("Run 2"/"Run 3"). + +## Counts + +| Area | Count | +|---|---:| +| Shared primitives (`ui/src/components/ui/`) | 24 | +| Feature components, flat (`ui/src/components/*.tsx`) | 178 | +| Feature components, nested subdirs (`access/`, `artifacts/`, `environment-variables-editor/`, `interrupt-handoff/`, `issue-output/`, `issue-properties/`, `routine-sections/`, `search/`, `timeline/`, `transcript/`) | 28 | +| **Feature components total** | **206** | +| Pages (`ui/src/pages/`, incl. `pages/secrets/`) | 73 | +| **Grand total** | **303** (roughly matches DESIGN.md's "24 + ~277") | + +--- + +## 1. Shared primitives — `ui/src/components/ui/` (24) + +All 24 checked against the live shadcn registry via `npx shadcn@latest diff` (network-available in this environment). Aggregate `diff` and per-component spot checks (`button`, `dialog`) both returned **"No updates found"** — these are currently in sync with the upstream registry source. + +| Component | File | Registry name | Variants (props) | Purpose | +|---|---|---|---|---| +| Alert Dialog | `alert-dialog.tsx` | `alert-dialog` | (Radix primitive passthrough) | Confirm/destructive-action modal | +| Avatar | `avatar.tsx` | `avatar` | `size` (sm/default), `shape` (circle/square via `data-shape`) | User/agent avatar with fallback initials | +| Badge | `badge.tsx` | `badge` | `variant` (default/secondary/destructive/outline) | Generic pill label | +| Breadcrumb | `breadcrumb.tsx` | `breadcrumb` | (Radix/plain nav passthrough) | Page breadcrumb trail | +| Button | `button.tsx` | `button` | `variant` (default/destructive/outline/secondary/ghost/link), `size` (default/sm/lg/icon/icon-sm/icon-xs) | CTA / action button, all tiers | +| Card | `card.tsx` | `card` | Header/Title/Description/Content/Footer subparts | Bordered content container | +| Checkbox | `checkbox.tsx` | `checkbox` | (Radix passthrough) | Boolean input | +| Collapsible | `collapsible.tsx` | `collapsible` | (Radix passthrough) | Expand/collapse section | +| Command | `command.tsx` | `command` | Dialog/Input/List/Item/Group/Separator/Shortcut | ⌘K palette primitive (backs `CommandPalette`) | +| Dialog | `dialog.tsx` | `dialog` | (Radix passthrough), expandable max-width transition | Modal dialog | +| Dropdown Menu | `dropdown-menu.tsx` | `dropdown-menu` | Item/CheckboxItem/RadioItem/Sub/Separator/Shortcut | Context/action menu | +| Input | `input.tsx` | `input` | (native input passthrough) | Text input | +| Label | `label.tsx` | `label` | (Radix passthrough) | Form field label | +| Popover | `popover.tsx` | `popover` | (Radix passthrough) | Floating panel | +| Radio Card | `radio-card.tsx` | **not a standard registry name** | (custom, card-shaped radio option) | Large selectable option card (onboarding/settings pickers) | +| Scroll Area | `scroll-area.tsx` | `scroll-area` | (Radix passthrough) | Styled scroll container | +| Select | `select.tsx` | `select` | (Radix passthrough) | Dropdown select | +| Separator | `separator.tsx` | `separator` | `orientation` | Divider line | +| Sheet | `sheet.tsx` | `sheet` | `side` (top/right/bottom/left) | Slide-in drawer | +| Skeleton | `skeleton.tsx` | `skeleton` | (plain div passthrough) | Loading placeholder | +| Tabs | `tabs.tsx` | `tabs` | (Radix passthrough) | Tab navigation | +| Textarea | `textarea.tsx` | `textarea` | (native passthrough) | Multi-line text input | +| Toggle Switch | `toggle-switch.tsx` | **not a standard registry name** (registry has `switch`) | (custom on/off toggle) | Boolean toggle control | +| Tooltip | `tooltip.tsx` | `tooltip` | (Radix passthrough) | Hover/focus hint | + +**Note:** `radio-card` and `toggle-switch` are not standard shadcn registry component names (the registry ships `radio-group` and `switch` respectively) — these were custom-built or heavily renamed/adapted rather than installed from the registry, so `shadcn diff` cannot check them against an upstream source. See shadcn-candidates section 4c. + +**Stock-value note:** several "arbitrary Tailwind bracket values" flagged in `TOKEN-AUDIT.md` inside these primitives (`checkbox.tsx` `rounded-[4px]`, `tooltip.tsx` `rounded-[2px]`, `command.tsx` `max-h-[300px]`, `avatar.tsx` `text-[10px]`) were verified to match the **current shadcn/ui registry source verbatim** — they are not local drift, just registry boilerplate that itself doesn't route through a token layer. Phase 2 will still need to touch them (DESIGN.md's gate is "zero arbitrary values in `ui/src/components/**`" with no carve-out for `components/ui/`), but they are not evidence of local customization. + +--- + +## 2. Feature components — `ui/src/components/` (206) + +Grouped by rough domain area. One line each; variants column is props-based where notable, blank where the component is largely propless/single-purpose. + +### 2.1 Issue / task surfaces (largest cluster) + +| Component | Purpose | +|---|---| +| `IssueRow.tsx` | Single row in a task list (status glyph, title, chips) | +| `IssuesList.tsx` | List/board container rendering many `IssueRow`s, sort/filter/group | +| `IssueColumns.tsx` | Column-layout config for the issues list/board | +| `IssueGroupHeader.tsx` | Section header when list is grouped (by status/project/assignee) | +| `IssueProperties.tsx` | 1-line re-export barrel → `issue-properties/IssueProperties.tsx` | +| `issue-properties/IssueProperties.tsx` | Full task detail properties panel (2,301 lines) — status, assignee, labels, project, dates | +| `IssueChatThread.tsx` | Task comment/chat thread (agent + human messages) | +| `IssueThreadInteractionCard.tsx` | Rich interaction card embedded in the chat thread (approvals, tool calls) | +| `IssueRecoveryActionCard.tsx` | Recovery-action prompt card in a stalled/errored task thread | +| `IssueScheduledRetryCard.tsx` | Scheduled-retry status card in task thread | +| `IssueRunLedger.tsx` | Run/cost ledger table for a task | +| `IssueMonitorActivityCard.tsx` | Monitoring/activity summary card on a task | +| `IssueBlockedNotice.tsx` | Banner when a task is blocked | +| `IssueAssignedBacklogNotice.tsx` | Banner when a backlog task gets assigned | +| `IssueDocumentAnnotations.tsx` / `issue-output/` variants | Doc-annotation highlight overlay on task documents | +| `IssueDocumentsSection.tsx` | Documents tab/section on task detail | +| `IssueAttachmentsSection.tsx` | Attachments tab/section on task detail | +| `IssuePlanDecompositionsSection.tsx` | Sub-task/decomposition list section | +| `IssueRelatedWorkPanel.tsx` | Related-issues panel | +| `IssueReferenceActivitySummary.tsx` / `IssueReferencePill.tsx` | Inline task-reference chip + activity rollup | +| `IssueSiblingNavigation.tsx` | Prev/next sibling task nav | +| `IssueLinkQuicklook.tsx` / `IssuesQuicklook.tsx` | Hover-preview popover for a linked task / task list | +| `IssueFiltersPopover.tsx` | Filter builder popover for the issues list | +| `IssueWorkspaceCard.tsx` | Card summarizing a task's execution workspace | +| `IssueContinuationHandoff.tsx` | Handoff-to-next-run card | +| `NewIssueDialog.tsx` | Create-task dialog | + +### 2.2 Agent / execution + +| Component | Purpose | +|---|---| +| `AgentCapsule.tsx` | The brand "capsule" motif — 3-state agent avatar (slot/configured/online) | +| `AgentActionButtons.tsx` | Start/stop/pause agent action row | +| `AgentBubbleActionRow.tsx` | Action row (copy/vote/timestamp/menu) under a conf-room agent chat bubble — **only one implementation found**, confirming PRIOR-ART's concern about a duplicate is resolved | +| `AgentConfigForm.tsx` | Agent configuration form (model, instructions, etc.) | +| `AgentIconPicker.tsx` | Icon picker for agent avatars | +| `AgentProperties.tsx` | Agent detail properties panel | +| `ActiveAgentsPanel.tsx` | Sidebar/dashboard panel of currently-running agents | +| `LiveRunWidget.tsx` | Live run status widget | +| `RunChatSurface.tsx` | Shared chat-surface shell used by both conf-room and task chat | +| `ClaudeSubscriptionPanel.tsx` / `CodexSubscriptionPanel.tsx` | Provider-specific subscription/quota panels — parallel, provider-specific (not a duplicate; see 3.4) | +| `ProviderQuotaCard.tsx` / `QuotaBar.tsx` | Generic quota display card / bar | + +### 2.3 Chat / composer + +| Component | Purpose | +|---|---| +| `ChatComposer.tsx` (380 lines) | Shared lean composer (conf-room + reused where a single-line/simple composer suffices) | +| `MarkdownEditor.tsx` (1,425 lines) | Rich MDX-based editor with mention autocomplete — task comment composer | +| `OnboardingChat.tsx` | Onboarding-flow chat surface | +| `CommentThread.tsx` | Generic comment-thread renderer (non-task contexts) | +| `MarkdownBody.tsx` / `WorkspaceFileMarkdownBody.tsx` | Rendered markdown output (read-only) — general vs. workspace-file-scoped | + +**KNOWN-DUPLICATES.md lead verified:** `ChatComposer` vs `MarkdownEditor`-based task composer — confirmed genuinely different in scope (380 vs 1,425 lines; ChatComposer has no mention-autocomplete/MDX machinery). Per KNOWN-DUPLICATES.md this was a deliberate non-unification (PAP-101) — **flagged in section 5, not recommended for merge.** + +### 2.4 Status / chips / badges + +| Component | Purpose | +|---|---| +| `StatusIcon.tsx` | Interactive status glyph + popover to change status (wraps `StatusGlyph`) | +| `StatusGlyph.tsx` | Pure SVG glyph renderer per status (sm/md/lg), no interactivity | +| `StatusBadge.tsx` | Custom `` pill components: `StatusBadge` (generic run/goal/approval), `AgentStatusBadge`, heartbeat capsule — **does not wrap the shadcn `Badge` primitive** (see shadcn candidates 4a) | +| `PriorityIcon.tsx` | Priority-level icon | +| `ExternalObjectStatusIcon.tsx` / `ExternalObjectStatusSummary.tsx` / `ExternalObjectPill.tsx` | External-object (linked PR/doc/etc.) status glyph, rollup summary, and inline pill — a third, deliberately separate status-presentation family | +| `BlockedReasonChip.tsx` | Chip explaining why a task is blocked | +| `SourceTrustBadge.tsx` / `SourceResolvedFoldBadge.tsx` / `SourceResolvedFoldCallout.tsx` | Trust/fold badges for external content sources | +| `ProductivityReviewBadge.tsx` | Review-status badge | + +**KNOWN-DUPLICATES.md lead verified:** StatusIcon / inline-mention chips / task chips are intentionally three separate systems (StatusIcon+StatusGlyph = task status glyph family; `ExternalObjectStatusIcon`/`Pill`/`Summary` = a second, external-object-specific family; mention chips in `lib/mention-chips.ts` + markdown CSS = a third, generic "chip in prose" family). **Documented here per instruction, not merged.** + +### 2.5 Sidebar / navigation + +| Component | Purpose | +|---|---| +| `Sidebar.tsx` / `SidebarShell.tsx` / `SecondarySidebar.tsx` / `InstanceSidebar.tsx` | Primary/secondary/instance-scoped sidebar shells | +| `SidebarSection.tsx` / `SidebarNavItem.tsx` | Sidebar section grouping + nav item row | +| `SidebarProjects.tsx` / `SidebarStarredProjects.tsx` | Project list / starred-project list in sidebar | +| `SidebarAgents.tsx` | Agent list in sidebar | +| `SidebarAccountMenu.tsx` / `SidebarCompanyMenu.tsx` | Account and company switcher menus | +| `SidebarServerInfo.tsx` | Server/instance info footer | +| `RequestCollapsedSidebar.tsx` | Collapsed-state sidebar for request/approval views | +| `RoutineSubSidebar.tsx` | Routine-scoped sub-sidebar | +| `MobileBottomNav.tsx` | Mobile bottom tab bar | +| `BreadcrumbBar.tsx` (uses `ui/breadcrumb`) | Page breadcrumb bar wrapper | +| `PageTabBar.tsx` | In-page tab bar | +| `CommandPalette.tsx` | ⌘K command palette (wraps `ui/command`) | + +**Sidebar text-size cluster** (cross-ref TOKEN-AUDIT.md 3.1): `Sidebar.tsx`, `SidebarNavItem.tsx`, `SidebarAgents.tsx`, `SidebarProjects.tsx`, `SidebarStarredProjects.tsx`, `SidebarAccountMenu.tsx` all independently use `text-[13px]` — six components implementing what looks like one shared "sidebar row label" intent as six separate arbitrary values. Flagged for the human type-scale decision, not merged here. + +### 2.6 Pipelines / routines / goals + +| Component | Purpose | +|---|---| +| `PipelineHealthWarnings.tsx` / `PipelineLivenessBanner.tsx` | Pipeline health/liveness banners | +| `PipelineItemBodyDocument.tsx` / `PipelineStageHistoryPanel.tsx` / `PipelineWorkReferences.tsx` | Pipeline stage detail panels | +| `PipelinesExperimentalGate.tsx` | Feature-flag gate wrapper for pipelines | +| `RoutineList.tsx` / `ManagedRoutinesList.tsx` | Routine list views | +| `RoutineActivityRow.tsx` / `RoutineHistoryTab.tsx` | Routine activity row + history tab | +| `RoutineRunVariablesDialog.tsx` / `RoutineVariablesEditor.tsx` | Routine run-variable input dialog/editor | +| `RoutineSaveBar.tsx` / `RoutineTriggerCard.tsx` | Routine save-bar + trigger config card | +| `routine-sections/editable-sections.tsx` (+2 more) | Editable routine-section blocks | +| `ScheduleEditor.tsx` | Cron/schedule editor | +| `GoalTree.tsx` / `GoalProperties.tsx` / `NewGoalDialog.tsx` | Goal hierarchy tree, detail panel, create dialog | + +### 2.7 Finance / budget + +| Component | Purpose | +|---|---| +| `AccountingModelCard.tsx` / `BillerSpendCard.tsx` / `FinanceBillerCard.tsx` / `FinanceKindCard.tsx` / `FinanceTimelineCard.tsx` | Various finance-dashboard cards — 5 distinct card shapes for different finance groupings (model/biller/kind/timeline); worth a human check for whether all 5 are truly distinct or 2-3 could share a base card | +| `BudgetIncidentCard.tsx` / `BudgetPolicyCard.tsx` / `BudgetSidebarMarker.tsx` | Budget incident, policy config, sidebar marker | +| `MetricCard.tsx` | Generic metric-display card | + +### 2.8 Files / documents / artifacts + +| Component | Purpose | +|---|---| +| `FileTree.tsx` | General file-tree renderer + `buildFileTree`/`FileTreeNode` model | +| `PackageFileTree.tsx` | Thin wrapper around `FileTree` (`wrapLabels` default) — reuses `FileTreeProps`, not a duplicate | +| `WorkspaceFileBrowser.tsx` | Execution-workspace file browser — **defines its own parallel tree-node model** (`WorkspaceFileTreeNode`, `buildWorkspaceFileTree`, `compareTreeNodes`) instead of reusing `FileTree`'s `FileTreeNode`/`buildFileTree` — see suspected duplicates (3.2) | +| `FileViewerSheet.tsx` | File content viewer sheet (wraps `ui/sheet`) | +| `FileTree.tsx`'s `parseFrontmatter` | Shared frontmatter parser (also exported, reused elsewhere) | +| `DocumentAnnotationLayer.tsx` / `DocumentAnnotationPanel.tsx` | Document highlight/annotation overlay + side panel | +| `DocumentDiffModal.tsx` | Document diff viewer modal | +| `DocumentFrameHeader.tsx` | Header chrome for an embedded document frame | +| `ArtifactFileChip.tsx` / `artifacts/ArtifactCard.tsx` / `artifacts/ArtifactGroupCard.tsx` / `ArtifactsPanel.tsx` | Artifact chip, card, grouped-card, and panel — internally consistent `rounded-[8px]` cluster (TOKEN-AUDIT 3.5) | +| `ImageGalleryModal.tsx` | Image gallery lightbox modal | + +### 2.9 Onboarding / access / identity + +| Component | Purpose | +|---|---| +| `OnboardingWizard.tsx` / `OnboardingWizardVariant.tsx` | Onboarding flow + an alternate variant — worth a human check on whether the variant is still live or leftover from an A/B (see 3.5) | +| `FrontDoor.tsx` | Landing/entry gate component | +| `CloudAccessGate.tsx` / `ConferenceRoomChatGate.tsx` / `PipelinesExperimentalGate.tsx` | Three separate feature-flag/access gate wrappers — same pattern (children-if-enabled), each hand-rolled per feature; candidate for a shared `FeatureGate` primitive (flagged, not built here) | +| `Identity.tsx` | Avatar + name identity chip (`deriveInitials` helper) | +| `MembershipAction.tsx` | Membership accept/decline action row | +| `access/` (2 files) | Access-request related components | +| `ReportsToPicker.tsx` / `ExecutionParticipantPicker.tsx` / `InlineEntitySelector.tsx` / `SearchableSelect.tsx` | Four distinct entity-picker components — overlapping purpose (pick a person/entity from a list); see suspected duplicates (3.3) | +| `SecretBindingPicker.tsx` / `environment-variables-editor/` (5 files) | Secrets/env-var binding UI | + +### 2.10 System / chrome / misc + +| Component | Purpose | +|---|---| +| `Layout.tsx` | App shell layout | +| `EmptyState.tsx` | **The single canonical empty-state component** — confirmed only one exists, matching DESIGN.md principle 1 | +| `SystemNotice.tsx` | Global system notice banner | +| `DevRestartBanner.tsx` | Dev-mode restart-required banner | +| `ToastViewport.tsx` + `context/ToastContext.tsx` | Fully custom toast notification system — no shadcn `sonner`/toast primitive installed (see shadcn candidates 4a) | +| `KeyboardShortcutsCheatsheet.tsx` | ⌘K-adjacent shortcuts help modal | +| `PageSkeleton.tsx` | Loading skeleton (wraps `ui/skeleton`) | +| `RouteErrorBoundary.tsx` | Route-level error boundary | +| `ThemeToggle.tsx` | Light/dark toggle | +| `CopyText.tsx` | Copy-to-clipboard text control | +| `EntityRow.tsx` | Generic entity list row (used across several list contexts) | +| `SwipeToArchive.tsx` | Mobile swipe-to-archive gesture wrapper | +| `ScrollToBottom.tsx` | Scroll-to-bottom floating button | +| `FoldCurtain.tsx` | Collapsible "show more" curtain/fade | +| `InlineEditor.tsx` | Generic inline-edit-in-place control | +| `JsonSchemaForm.tsx` | JSON-schema-driven dynamic form renderer | +| `TrustPresetSection.tsx` | Trust-level preset picker section | +| `WorktreeBanner.tsx` | Worktree-branding banner | +| `CompanyPatternIcon.tsx` | Canvas-rendered company pattern/avatar icon | +| `CompanySwitcher.tsx` / `CompanySettingsSidebar.tsx` | Company switcher + settings sidebar | +| `ProjectProperties.tsx` / `ProjectTile.tsx` / `ProjectWorkspaceSummaryCard.tsx` / `ProjectWorkspacesContent.tsx` | Project detail panel, tile, workspace summary card, workspace content | +| `ApprovalCard.tsx` / `ApprovalPayload.tsx` | Approval request card + payload renderer | +| `ExecutionWorkspaceCloseDialog.tsx` | Close-workspace confirm dialog | +| `ResponsibleUserDenialNotice.tsx` | Denial-notice banner | +| `BootstrapPendingPage.tsx` | Bootstrap-pending full-page state | +| `MissingPluginTabPlaceholder.tsx` | Placeholder when a plugin tab isn't installed | +| `StandaloneBrowserControls.tsx` | Standalone-mode browser chrome controls | +| `AsciiArtAnimation.tsx` | Decorative ASCII animation (boot/loading) | +| `OpenCodeLogoIcon.tsx` | Static logo icon | +| `StarToggle.tsx` | Star/favorite toggle button | +| `OutputFeedbackButtons.tsx` | Thumbs up/down feedback buttons on agent output | +| `KanbanBoard.tsx` | Kanban board view (alternate to `IssuesList` list view) | +| `agent-config-primitives.tsx` | Shared field primitives for agent config forms | +| `PropertiesPanel.tsx` | Generic properties-panel shell (used by Issue/Project/Agent/Goal Properties) | + +### 2.11 Nested subdirectories + +| Directory | Files | Purpose | +|---|---|---| +| `access/` | 2 | Access-request UI | +| `artifacts/` | 2 | `ArtifactCard`, `ArtifactGroupCard` | +| `environment-variables-editor/` | 5 | Env-var/secret editor rows and index | +| `interrupt-handoff/` | 1 | `InterruptHandoffViews.tsx` | +| `issue-output/` | 5 | Task output file tiles / sections | +| `issue-properties/` | 5 | Task properties panel (full impl, see 2.1) | +| `routine-sections/` | 3 | Editable routine section blocks | +| `search/` | 3 | Search result row, `HighlightedText`, `MatchSourceChip` | +| `timeline/` | 1 | `WorkTimelineChart.tsx` | +| `transcript/` | 1 | `RunTranscriptView.tsx` | + +--- + +## 3. Pages — `ui/src/pages/` (73) + +Grouped by area; one line each. + +| Page | Purpose | +|---|---| +| `Dashboard.tsx` | Home/overview dashboard | +| `IssueDetail.tsx` / `IssuesList` route pages | Task detail + list routes | +| `AgentDetail.tsx` / `Agents.tsx` | Agent detail + list | +| `ProjectDetail.tsx` / `Projects.test.tsx`-adjacent list page | Project detail + list | +| `Pipelines.tsx` / `PipelineSettings.tsx` | Pipeline list + settings | +| `Routines.tsx` | Routine list/detail | +| `Org.tsx` / `OrgChart.tsx` | Org directory + org chart visualization | +| `Costs.tsx` | Cost/budget dashboard | +| `Timeline.tsx` | Work timeline view (wraps `timeline/WorkTimelineChart`) | +| `Inbox.tsx` | Notifications/requests inbox | +| `Approvals.tsx` / `ApprovalDetail.tsx` | Approval queue + detail | +| `CompanySettings.tsx` / `CompanySkills.tsx` / `CompanyEnvironments.tsx` / `CompanyAccess.tsx` / `CompanyImport.tsx` / `CompanyExport.tsx` | Company-scoped settings sub-pages | +| `TeamCatalog.tsx` | Team/role catalog | +| `UserProfile.tsx` / `ProfileSettings.tsx` | User profile view + settings | +| `Secrets.tsx` / `secrets/*` (5 files) | Secrets management + sub-tabs (import-from-vault, definitions, my-secrets, presentation helpers, missing-banner) | +| `PluginManager.tsx` / `PluginSettings.tsx` | Plugin management + per-plugin settings | +| `AdapterManager.tsx` | Adapter (agent runtime) management | +| `CloudUpstream.tsx` / `CloudUpstreamUxLab.tsx` | Cloud-upstream connection page + its UxLab showcase twin | +| `InviteLanding.tsx` / `InviteUxLab.tsx` | Invite acceptance landing page + UxLab showcase twin | +| `BootstrapSetupUxLab.tsx` | Bootstrap-setup flow showcase | +| `ResponsibleUserDenialUxLab.tsx` | Denial-flow showcase | +| `SystemNoticeUxLab.tsx` | System-notice showcase | +| `IssueChatUxLab.tsx` / `RunTranscriptUxLab.tsx` | Chat + transcript showcases | +| `DesignGuide.tsx` | Design-system showcase/reference page | +| `BoardChat.tsx` / `BoardClaim.tsx` | Board (concierge) chat + claim flow | +| `NotFound.tsx` | 404 page | +| `CliAuth.tsx` | CLI auth handoff page | +| `JoinRequestQueue.tsx` | Join-request queue | +| `InstanceAccess.tsx` / `InstanceGeneralSettings.tsx` / `InstanceExperimentalSettings.tsx` | Instance-level settings pages | +| `ExecutionWorkspaceDetail.tsx` / `ProjectWorkspaceDetail.tsx` | Execution/project workspace detail pages | +| `Companies.tsx` | Multi-company switcher/list | +| `CompanyImport.tsx` / `CompanyExport.tsx` | Import/export company data | +| `RoutineDetail.tsx` | Routine detail route | + +**UxLab pages are real routes** (confirmed via `App.tsx` — `/ux-lab/*`, `/design-guide`), not build-excluded demo code, so all hardcoded values inside them are in-scope per DESIGN.md even though they're showcase surfaces rather than product screens a typical operator visits daily. + +--- + +## 4. Shadcn candidates + +**All items below are recommendations only — no swaps happen in this run.** + +### 4a. Custom components duplicating an available shadcn primitive + +| Custom component | Duplicates | Recommended replacement | Expected visual impact | +|---|---|---|---| +| `ToastViewport.tsx` + `context/ToastContext.tsx` | shadcn's `sonner`-based toast pattern (not currently installed) | Install `sonner`/toast primitive, migrate `useToastActions`/`useToastState` call sites to it | Low if the registry's toast is restyled to match current `toneClasses`/`toneDotClasses` tint system; the current implementation already has custom tone colors (`sky`/`emerald`/`amber`/`red`) that would need to carry over as variant props — a naive swap would look different unless those tones are preserved | +| `components/StatusBadge.tsx` (`StatusBadge`, `AgentStatusBadge`) | shadcn `Badge` primitive (installed, `ui/badge.tsx`) | Keep as a distinct component (status badges need the `.status-chip` color-mix mechanic Badge's variant system doesn't support) but consider having it render `` internally with a custom class rather than a bare ``, for consistency of base styles (focus ring, disabled states, etc.) | Low — internal implementation change only if done carefully; skip if it risks the WCAG-tuned status hues | +| Hand-rolled bordered-container `
`/`
    ` patterns (`rounded-md border bg-card`-shaped) found in ~26 files (`pages/AdapterManager.tsx`, `pages/TeamCatalog.tsx`, `pages/InstanceAccess.tsx`, `pages/BootstrapSetupUxLab.tsx`, `pages/ResponsibleUserDenialUxLab.tsx`, `pages/CliAuth.tsx`, `pages/BoardClaim.tsx`, `pages/BoardChat.tsx`, `pages/InstanceGeneralSettings.tsx`, `pages/ProjectWorkspaceDetail.tsx`, `pages/Timeline.tsx`, `pages/PluginManager.tsx`, `pages/JoinRequestQueue.tsx`, `pages/InstanceExperimentalSettings.tsx`, `App.tsx`, `components/BootstrapPendingPage.tsx`, `components/IssuePlanDecompositionsSection.tsx`, `components/BlockedInboxView.tsx`, `components/ApprovalCard.tsx`, + ~7 more) vs. shadcn `Card` (installed, only imported in 21 files) | `Card`/`CardContent` | Low-to-medium — many of these are `
      ` list wrappers, not literal card content; a swap would need per-site judgment, not a blanket codemod. Flagging the cluster, not asserting every site should change. | +| Hand-rolled pill/badge-shaped ``s (`rounded-full px-2 text-[...]`) outside `Badge` usage, ~34 files | `Badge` (installed, 35 files already use it) | `Badge` with a custom `className` for color | Low if colors are preserved as `className` overrides | +| `plugins/launchers.tsx` generic plugin-shell overlay (`role="dialog"`, hand-rolled backdrop `bg-black/45`, manual z-index math, `rounded-xl`/`rounded-2xl`) | `Dialog` / `Sheet` / `Popover` (all installed) | Case-by-case: this component multiplexes dialog/drawer/popover shell types from one plugin-host abstraction, which none of the three installed primitives do individually — a clean swap likely means keeping the multiplexer but delegating each `shellType` branch to the matching installed primitive instead of a fully custom `
      ` tree | Medium — this is the most structurally custom overlay in the codebase; recommend closer human review before treating it as a simple swap | + +### 4b. Installed shadcn components drifted from the registry + +`npx shadcn@latest diff` (run from `ui/`, network available) reported **"No updates found"** for the aggregate diff and for spot-checked `button` and `dialog` individually. **No drift detected** against the current registry for any of the 22 standard-named primitives. (`radio-card` and `toggle-switch` aren't standard registry names so `diff` cannot evaluate them — see 4c.) + +### 4c. Raw Radix/plain elements where an installed shadcn wrapper exists + +- **No raw `@radix-ui/*` imports were found outside `ui/src/components/ui/`** (`rg -l '@radix-ui' -g '*.tsx' -g '!components/ui/*'` returned zero files) — every Radix primitive in the app is properly routed through the `components/ui/` wrapper layer. This is a clean result; no action needed. +- One raw hand-rolled modal (`plugins/launchers.tsx`, `role="dialog"` on a plain `
      `) exists where `Dialog`/`Sheet` wrappers are installed — see 4a above, same finding, cross-listed here because it's also a "plain element where a wrapper exists" case. +- `radio-card.tsx` and `toggle-switch.tsx` are functioning as de facto custom primitives sitting in the `ui/` folder alongside real shadcn components, but they were not installed via the registry (no matching registry names). Recommend a human decide whether to (a) leave them as intentionally custom, documenting why the registry's `radio-group`/`switch` don't fit, or (b) evaluate swapping to the registry versions if the customization was incidental rather than deliberate. + +--- + +## 5. Suspected duplicates (with evidence) + +All of the below are **leads for human review**, not verdicts, per KNOWN-DUPLICATES.md's framing. + +### 5.1 Seed leads from KNOWN-DUPLICATES.md — verified + +| Lead | Finding | +|---|---| +| `ChatComposer` vs `MarkdownEditor`-based task composer | **Confirmed genuinely distinct** (380 vs 1,425 lines; different prop interfaces `ChatComposerProps` vs `MarkdownEditorProps`; MarkdownEditor owns mention-autocomplete machinery — `findMentionMatch`, `computeMentionMenuPosition`, `placeCaretAfterMentionAnchor` — that ChatComposer has none of). Matches KNOWN-DUPLICATES.md's note that this was a deliberate non-unification (PAP-101). **Needs human decision: re-confirm this split should remain permanent, or revisit unification now that both have matured further.** | +| `AgentBubbleActionRow.tsx` duplicate check | **Only one file found** (`components/AgentBubbleActionRow.tsx`). The concurrent-work duplicate PRIOR-ART/KNOWN-DUPLICATES.md warned about is not present on this branch — resolved. | +| `StatusIcon` vs inline-mention chips vs task chips | **Confirmed three separate, intentionally distinct systems**: (1) `StatusIcon`/`StatusGlyph` — task status glyph, drives from `--status-task-icon-*` tokens; (2) `ExternalObjectStatusIcon`/`ExternalObjectPill`/`ExternalObjectStatusSummary` — a parallel system for external (PR/doc/etc.) object status, independent color/severity model (`externalObjectStatusIcon`, `externalObjectStatusToneSeverity` in `lib/status-colors.ts`); (3) mention chips (`lib/mention-chips.ts` + `.paperclip-mention-chip` CSS in `index.css`) — generic "entity reference in prose" chip unrelated to status at all. Documented per instruction, **not merged.** | + +### 5.2 New leads found during this audit + +| Lead | Evidence | +|---|---| +| `FileTree.tsx` vs `WorkspaceFileBrowser.tsx` tree models | `FileTree.tsx` exports `buildFileTree`, `FileTreeNode`, `countFiles`, `collectAllPaths`, `parseFrontmatter`. `WorkspaceFileBrowser.tsx` independently defines its own `WorkspaceFileTreeNode`/`WorkspaceFileTreeFolderNode`/`WorkspaceFileTreeFileNode`, `buildWorkspaceFileTree`, `compareTreeNodes`, `finalizeTreeFolder` — a parallel tree-building implementation rather than reuse of `FileTree`'s exported helpers. `PackageFileTree.tsx` by contrast correctly wraps `FileTree` (reuses `FileTreeProps`). **Needs human decision**: was `WorkspaceFileBrowser`'s separate model a deliberate choice (different node shape needs — e.g. it may need workspace-specific metadata `FileTreeNode` lacks) or copy-paste-and-diverge drift? | +| `agentStatusBadge` vs `brandChipBadge` (`lib/status-colors.ts`) | Byte-for-byte identical maps for the 4 shared keys (gray/blue/amber/red — same hex, same dark-mode alpha suffixes); `brandChipBadge` additionally has `green`/`violet`. Cross-referenced in TOKEN-AUDIT.md section 1.1. **Recommend collapsing `agentStatusBadge` into `brandChipBadge`** (or making it an alias) since they are provably identical, not just similar. | +| Entity-picker family: `ReportsToPicker.tsx`, `ExecutionParticipantPicker.tsx`, `InlineEntitySelector.tsx`, `SearchableSelect.tsx` | Four components with overlapping "pick one entity from a searchable list" purpose. Not verified identical (each has domain-specific filtering — reports-to org hierarchy, execution participants, generic inline selection, generic searchable select) but the prop-surface overlap (all take an options list + selected value + onChange) is a plausible consolidation candidate. **Flagged, not verified as true duplicates** — would need a closer prop-by-prop diff in a follow-up run. | + +### 5.3 Corrections after closer inspection + +- `OnboardingWizardVariant.tsx` (10 lines) is **not** a duplicate of `OnboardingWizard.tsx` (1,786 lines) — it's a thin routing wrapper (`export function OnboardingWizardVariant() { return ; }`) left over from a since-retired experimental-flag variant (per its own doc comment: "Conference-room chat is now the only surface left behind `enableConferenceRoomChat`; onboarding stays available without that experimental flag"). Both are routed in `App.tsx`. **No action needed** — this is a naming leftover, not visual/logic duplication; candidate for a trivial rename-and-inline cleanup in a future non-visual refactor, out of scope here. +- `components/IssueProperties.tsx` is a 1-line re-export barrel (`export { IssueProperties } from "./issue-properties";`) — not a duplicate of `components/issue-properties/IssueProperties.tsx`, just a compatibility import path. + +### 5.4 Finance card family — needs closer human review + +`AccountingModelCard.tsx`, `BillerSpendCard.tsx`, `FinanceBillerCard.tsx`, `FinanceKindCard.tsx`, `FinanceTimelineCard.tsx` — five components with card-shaped, finance-dashboard purposes and naming that overlaps enough (`BillerSpendCard` vs `FinanceBillerCard`) to warrant a closer look than this audit had time for. Not verified as duplicates; flagged as a lead only. + +### 5.5 Feature-gate wrapper pattern (not a duplicate, but a repeated pattern) + +`CloudAccessGate.tsx`, `ConferenceRoomChatGate.tsx`, `PipelinesExperimentalGate.tsx` each independently implement the same "render children only if flag X is enabled, else render fallback" shape. Not byte-identical (each checks a different flag/hook), so not a strict duplicate, but a strong candidate for a shared `FeatureGate`/`ExperimentalGate` primitive that takes a flag-check function as a prop. Flagged as a recommendation, not built here. + +--- + +## 6. Needs human decision (required section) + +1. **`ChatComposer` vs `MarkdownEditor`-based composer** — KNOWN-DUPLICATES.md already flags this as deliberately unmerged (PAP-101). This audit confirms the split is real (not accidental) given the large capability gap. Human call: keep permanently split, or revisit now. **DECIDED (Run 2 review, DECISION-SHEET.md C1): keep the split, documented as deliberate — user re-confirmed after visual review.** +2. **`FileTree.tsx` vs `WorkspaceFileBrowser.tsx` independent tree models** — is `WorkspaceFileBrowser`'s separate `WorkspaceFileTreeNode` model justified by different data needs, or is it drift that should be refactored onto `FileTree`'s exported `buildFileTree`/`FileTreeNode`? Needs someone who knows both call sites' actual data shapes. **DECIDED (Run 2 review, DECISION-SHEET.md C2): investigate data-shape needs as Run 3 prep; refactor onto FileTree only if shapes align.** **RESOLVED (Run 3 investigation): KEEP SEPARATE — shapes do not align; see §7.1.** +3. **`agentStatusBadge` vs `brandChipBadge`** (`lib/status-colors.ts`) — these are provably byte-identical for their 4 shared keys. Recommend collapsing, but doing so touches every call site importing `agentStatusBadge`, so it's a human-approved Run 2/3 item, not automatic. **RESOLVED (Run 2 review, DECISION-SHEET.md A1): collapsed — `agentStatusBadge` had zero importing call sites and was deleted; `brandChipBadge` is the single map.** +4. **Entity-picker family** (`ReportsToPicker`, `ExecutionParticipantPicker`, `InlineEntitySelector`, `SearchableSelect`) — plausible consolidation candidate on prop-surface similarity alone; needs a closer prop-by-prop and behavior diff (not done in this pass) before any merge recommendation can be made with confidence. **DECIDED (Run 2 review, DECISION-SHEET.md C3): prop-by-prop diff queued as Run 3 prep; no merge without it.** **RESOLVED (Run 3 investigation): all four KEEP SEPARATE — no copy-paste drift found; see §7.2.** +5. **Finance card family** (5 components, section 5.4) — needs a domain-knowledgeable human to confirm whether all 5 are truly distinct dashboard needs or 2-3 could share a base `FinanceCard`. **DECIDED (Run 2 review, DECISION-SHEET.md C4): keep all five; revisit only if a sixth appears.** +6. **Hand-rolled card-shaped containers vs. `Card` primitive** (~26 files) and **hand-rolled pill spans vs. `Badge`** (~34 files) — both are large, low-risk-looking consolidation opportunities, but "low risk" was assessed at a glance only; a real swap pass needs per-site visual verification (this is exactly what Storybook snapshots from Phase 0 would catch if these were touched). **DECIDED (Run 2 review, DECISION-SHEET.md C5): queued for Run 3 as the shadcn-swap list, with per-site snapshot verification.** +7. **`plugins/launchers.tsx` custom multiplexed overlay** — the most structurally custom modal-like component in the app; recommend a dedicated closer look before deciding whether/how to route it through `Dialog`/`Sheet`/`Popover`, since it currently does something none of the three do alone (switch shell type per plugin action). **DECIDED (Run 2 review, DECISION-SHEET.md C6): dedicated review task; excluded from Run 3.** +8. **`radio-card.tsx` / `toggle-switch.tsx` non-standard "shadcn" primitives** — confirm whether these were deliberately custom-built (and should stay documented as such) or are stale/incidental deviations from `radio-group`/`switch` that should be swapped in a later run. **DECIDED (Run 2 review, DECISION-SHEET.md C7): deliberately custom — documented as such; no swap.** +9. **`StatusBadge`/`AgentStatusBadge` not wrapping the installed `Badge` primitive** — worth a human call on whether unifying the base markup (while keeping the custom `.status-chip` color-mix mechanic) is worth the churn, or whether the current bespoke `` approach should just be documented as an intentional, permanent exception (similar to the StatusIcon/ExternalObject/mention-chip three-way split already documented). **DECIDED (Run 2 review, DECISION-SHEET.md C8): documented as an intentional exception — the WCAG-tuned `.status-chip` mechanic stays bespoke.** +10. **Toast system has no installed shadcn primitive to compare against** — `ToastViewport`/`ToastContext` is fully custom because no `sonner`/toast component is installed at all. Human call: install one and migrate, or formally document the custom toast as the system's permanent choice (it already has a working tone/variant system). **DECIDED (Run 2 review, DECISION-SHEET.md C9): decision deferred to Run 4, when the toast's palette colors get retokenized; sonner-behind-a-pushToast-facade is the alternative to evaluate then.** + +--- + +## 7. Run 3 investigation verdicts (C2/C3) + +Investigated Jul 7 2026 on `design/component-convergence` per DECISION-SHEET.md C2/C3. Both are read-only verdicts; no merges executed (neither met the "copy-paste drift with identical data shapes" bar). + +### 7.1 C2 — WorkspaceFileBrowser vs FileTree tree models: KEEP SEPARATE + +**Deliberate divergence, not copy-paste drift.** No shared git ancestry: `git log --follow` shows FileTree.tsx originating in 3c73ed26b (plugin host surface, #5205) and WorkspaceFileBrowser.tsx independently in 468edd8b2 (workspace file viewer, #7681); no rename/copy relationship, and the only commit touching both is the mechanical token retune (c07e650cd). + +**Data shapes are distinct, not overlapping:** +- `FileTreeNode` (FileTree.tsx:18-25): `kind: "dir"|"file"`, keyed on `path`, children on every node, optional `action` string. Built by `buildFileTree(files: Record)` — a flat path→content map fed static blobs (CompanyExport.tsx:696/762/832, CompanyImport.tsx:783/903, AgentDetail.tsx:1902). +- `WorkspaceFileTreeNode` (WorkspaceFileBrowser.tsx:211-227): `kind: "folder"|"file"` (different literals), composite `key` = `kind:workspaceId:relativePath`, baked-in `depth`, folder-only `lazy` flag, and file nodes embed the entire `WorkspaceFileListFileItem` server DTO (~15 fields: capabilities, previewKind, byteSize, download URLs — packages/shared/src/types/workspace-file-resource.ts:50-94). Built from an array of server DTOs with rootPath prefix-stripping. + +**Behavior domains differ:** FileTree is an eager, static tree with tri-state checkboxes, badges/tones, frontmatter parsing, and a full roving-tabindex keyboard grid; WorkspaceFileBrowser is a server-paginated live browser with lazy folders, per-folder "load more" via `useQueries`, a debounced search combobox, and opposite sort order (folders-first vs FileTree's deliberate files-first, FileTree.tsx:96-99). Neither uses the other's load-bearing features; a merge would be a superficial shared-recursion abstraction both sides immediately override. + +### 7.2 C3 — Entity-picker family: all four KEEP SEPARATE + +**No copy-paste lineage:** four independent creation commits by different efforts — InlineEntitySelector (3709901db, Feb 26), ReportsToPicker (61f53b647, Mar 20), ExecutionParticipantPicker (be518529b, Apr 7, extracted from inline pill UI), SearchableSelect (50ae8fc65, Jun 24, #8597, born cmdk-based). `git log --follow` shows no cross-file rename/copy. + +Per-component rationale: +- **ReportsToPicker**: no search, no keyboard nav, domain `Agent[]` single-select with unique manager-state UI (terminated-manager banner, stale-ID handling, ReportsToPicker.tsx:92-104). 2 call sites. A thin bespoke widget, not a drifted copy. +- **ExecutionParticipantPicker**: the only multi-select (toggle set) and the only one with async data fetching (`useQuery` user directory); encodes selections as `agent:`/`user:` tagged strings coupled to execution-policy plumbing. Not behavior-preservingly mergeable into any single-select. +- **InlineEntitySelector vs SearchableSelect** (the only real candidate pair): different engines (hand-rolled input + manual Arrow/Enter/Tab keyboard machine with tab-advance chain driving NewIssueDialog's field flow, InlineEntitySelector.tsx:154-189/90-101 — vs cmdk `Command shouldFilter={false}`, SearchableSelect.tsx:246); different option shapes (`{id,label,searchText}` flat + synthetic none-row vs `{key,value,label,title,searchText,disabled}` grouped + generic ``); each has unique load-bearing features (recent-ordering/onConfirm vs createItem/deriveGroups/scoreOption/veto-close). ~33 call sites on InlineEntitySelector incl. the plugin bridge (plugins/bridge-init.ts:389/479). High-risk, low-reward rewrite — default keep-as-is holds. + +### 7.3 Interactive-card affordance (Run 3 review feedback) + +`Card` gained an `interactive` prop — pointer cursor, quiet hover (border→foreground/20 + shadow-md lift), focus-visible ring — used when the whole card is a click target (e.g. Companies selector). Skills tiles (CompanySkills `SkillCard`) and artifact cards (`ArtifactCard`/`ArtifactGroupCard`) apply the same recipe verbatim since they cannot render through Card (button/Link semantics). Static container Cards stay affordance-free by design. diff --git a/doc/design/DECISION-SHEET.md b/doc/design/DECISION-SHEET.md new file mode 100644 index 00000000000..8622ba7270b --- /dev/null +++ b/doc/design/DECISION-SHEET.md @@ -0,0 +1,97 @@ +# DECISION-SHEET — Run 1 human review + +Every open question from TOKEN-AUDIT.md §8 + batch logs and COMPONENT-INVENTORY.md §6, each with a recommendation, blast radius, and where it lands. Statuses: **PENDING** → APPROVED / OVERRIDDEN (with note) / DEFERRED. + +## A. Quick wins — low risk, do in this review phase + +| # | Decision | Recommendation | Blast radius | Status | +|---|---|---|---|---| +| A1 | `agentStatusBadge` vs `brandChipBadge` byte-identical maps (status-colors.ts) | Collapse to `brandChipBadge`, re-point imports | code dedup, zero pixels | APPROVED — done 5ecc0f9e4: `agentStatusBadge` had ZERO importing call sites, so it was deleted outright (no re-pointing, no alias needed); `brandChipBadge` is the single map, `AgentBadgeColor` kept (subset of `BrandChipColor`) | +| A2 | Contrast-pair triplication (color-contrast.ts / worktree-branding.ts / ThemeContext.tsx) | One shared constant IF values are truly identical; verify per-pair first, keep semantically distinct ones separate | 3 files, zero pixels if identical | APPROVED — done 383460bb2: only `#f8fafc`/`#111827` is byte-identical across files → exported as `READABLE_TEXT_LIGHT`/`READABLE_TEXT_DARK` from color-contrast.ts, imported by worktree-branding.ts; ThemeContext's `#18181b`/`#ffffff` meta-theme-color pair, the DARK_BG/LIGHT_BG rgb-object compositing backgrounds, and worktree-branding's `#000000` parse fallback are semantically distinct → untouched | +| A3 | Project-color fallbacks `#6366f1` / `#64748b` (14 sites) | Two semantic tokens (`--project-seed`, `--project-none`) — file pattern shows two intents | rename-only, zero pixels | APPROVED — done 7e3e59db2: pure rename `--hex-6366f1`→`--project-seed` (7 sites) and `--hex-64748b`→`--project-none` (9 sites incl. ActivityCharts 'backlog') + index.css definitions, values unchanged | +| A4 | Test-file hardcoded hex (56 sites) | Leave alone; update lockstep only when asserted values actually change | none | APPROVED — policy adopted, no code change | +| A5 | `FileViewerSheet` half-migrated `var(--paperclip-code-highlight-*, fallback)` | Mint the two vars in index.css at the fallback values (identical pixels today; makes the intended token real) | 1 file | APPROVED — done 7929aabeb: both vars minted in the extracted-tokens :root block at exactly the former fallback values; chose to SIMPLIFY the Batch 4 `--code-highlight-*-resolved` wrappers to plain `var(--x)` (fallbacks now redundant; nothing sets the vars at runtime); FileViewerSheet.tsx call sites unchanged | +| A6 | "Liveness blue" chat bubble reusing `--status-task-in_progress` (semantic coincidence) | Decouple: mint `--liveness-blue` with same value so a future status-hue change doesn't drag the chat bubble along | 2 sites, zero pixels | APPROVED — done b76a7955a: `--liveness-blue: #2563eb` minted; IssueChatThread.tsx bubble class + IssueChatThread.test.tsx lockstep assertion re-pointed | + +## B. Policy calls + +| # | Decision | Recommendation | Status | +|---|---|---|---| +| B1 | One-off decorative gradients/shadows (5 production + UxLab; 38 arbitrary shadows, no `--shadow-*` tokens existed) | Allowlist as documented "intentional one-off decoration" (extend allowlist criteria beyond third-party); DELETE the ~20 never-reused singleton `--gradient-extract-*` tokens back to inline, keep only reused ones. Spirit over letter of principle 2 | APPROVED — middle path, executed: 27 demo-only tokens reverted inline (19 gradients: --gradient-extract-5,6,8-24; 8 shadows: --shadow-extract-15,16,17,19-23 — all consumed solely by *UxLab.tsx pages), 35 call sites restored to original bracket literals, definitions deleted, 4 UxLab pages allowlisted + criteria doc-comment extended to first-party intentional decoration. KEPT 22 production tokens (gradients 1,2,3,4,7,25,26 + 15 shadows) — NOTE: --shadow-extract-4/5 kept contrary to the audit's first cut because ChatComposer.tsx and IssueChatThread.tsx (production) consume them, not just ChatComposer.test.tsx | +| B2 | Tailwind palette classes (`bg-red-500` etc.) — 3,115 sites / 145 files | Own future run (Run 4): cluster-by-cluster mapping to semantic tokens, starting with status-adjacent colors; NOT wholesale now, NOT permanent exemption. Update DESIGN.md principle 2 to name palette classes explicitly | APPROVED — own Run 4 later | +| B3 | Micro type cluster 9–15px (730 sites) + letter-spacing 9 values (202 sites) | Adopt PRIOR-ART named ladder (map 9→10 nano, keep 10/11/12/13/14; 15→14; tracking → 3 steps) via contact-sheet review — executed in the preset-tune session, decided now | EXECUTED (pending contact-sheet) — scripts/codemod-type-ladder.mjs (idempotent, committed): 730 font-size sites -> --text-nano (10px, incl. 9->10) / --text-micro (11px, incl. 0.7rem) / text-xs (12px) / --text-compact (13px) / text-sm (14px, incl. 15->14); 202 tracking sites -> --tracking-label (0.08em) / --tracking-eyebrow (0.14em) / --tracking-caps (0.2em); all --fs-* and --ls-* definitions deleted. NOTE: text-xs/text-sm sites also pick up Tailwind scale line-height (contact-sheet reviewable). PRIOR-ART "sm 13" tier renamed --text-compact (name collides with Tailwind text-sm=14px) | +| B4 | Radius conflict (`--radius-lg/xl` = 0px vs stock 2xl/3xl) | Defer to preset session — it's a brand question. Candidate: PRIOR-ART monotonic 6/8/10/14/16 | DEFERRED to preset session | +| B5 | Chart palette vs canonical status hues (ActivityCharts in_progress = violet, elsewhere blue) | Re-point charts at `--status-task-*` (operator learns one vocabulary — DESIGN.md P5). Visible change → contact sheet | APPROVED & CLOSED — user approved on before/after contact sheet (Jul 6); 2 chart snapshots re-baselined, suite 510/510 on new baseline. Known trade documented: To-Do amber vs priority-Medium amber adjacency, revisit at preset session | + +## C. Component calls (from COMPONENT-INVENTORY.md §6) + +| # | Decision | Recommendation | Status | +|---|---|---|---| +| C1 | ChatComposer vs MarkdownEditor split | Keep split; document as deliberate in COMPONENT-INVENTORY | APPROVED — keep split, documented deliberate (user re-confirmed after visual review) | +| C2 | FileTree vs WorkspaceFileBrowser parallel tree models | Investigate data-shape needs in Run 3; refactor onto FileTree only if shapes align | APPROVED — investigate data-shape needs in Run 3 prep | +| C3 | Entity-picker family (4 components) | Prop-by-prop diff as Run 3 prep task; no merge without it | APPROVED — prop-by-prop diff as Run 3 prep task; no merge without it | +| C4 | Finance card family (5 components) | Keep; revisit only if a 6th appears | APPROVED — keep all five; revisit only if a 6th appears | +| C5 | Hand-rolled cards (~26 files) → `Card`; pills (~34 files) → `Badge` | Run 3 shadcn-swap list, per-site snapshot verification | DONE (Run 3, Jul 7) — C5a: ~30 files converted (Card supplies skin, sites keep interior layout; interactive ` - + {adapterList.map((item) => ( ))} @@ -1635,7 +1646,7 @@ function AdapterTypeDropdown({ function ExperimentalBadge() { return ( - + Experimental ); @@ -1769,7 +1780,7 @@ function ModelDropdown({ - +
      {models.find((m) => m.id === value)?.label ?? value} - + current - + )} {detectedModel && detectedModel !== value && ( @@ -1857,9 +1868,9 @@ function ModelDropdown({ {models.find((m) => m.id === detectedModel)?.label ?? detectedModel} - + detected - + )} {detectedModelCandidates @@ -1881,13 +1892,13 @@ function ModelDropdown({ {entry?.label ?? candidate} - + config - + ); })} -
      +
      {allowDefault && ( - + {options.map((option) => (
      ) : ( @@ -208,22 +209,22 @@ export function ArtifactsPanel({ taskId, isAgentWorking, openDocKey, openDocTitl )}
      - + {wp.type.replace(/_/g, " ")} {showGenerating ? ( - + Generating... - + ) : ( - + {badge.label} - + )}
      {wp.summary && ( -

      +

      {wp.summary}

      )} @@ -298,7 +299,7 @@ function DocumentViewer({ {/* Sticky action footer */} {needsAction && (
      -

      This document needs your review.

      +

      This document needs your review.

      {parsed !== null && parsed <= incident.amountObserved ? ( -

      +

      The new budget must exceed current observed spend.

      ) : null} diff --git a/ui/src/components/BudgetPolicyCard.tsx b/ui/src/components/BudgetPolicyCard.tsx index 7834e8cbabf..d392ff0c0e2 100644 --- a/ui/src/components/BudgetPolicyCard.tsx +++ b/ui/src/components/BudgetPolicyCard.tsx @@ -23,9 +23,9 @@ function windowLabel(windowKind: BudgetPolicySummary["windowKind"]) { } function statusTone(status: BudgetPolicySummary["status"]) { - if (status === "hard_stop") return "text-red-300 border-red-500/30 bg-red-500/10"; - if (status === "warning") return "text-amber-200 border-amber-500/30 bg-amber-500/10"; - return "text-emerald-200 border-emerald-500/30 bg-emerald-500/10"; + if (status === "hard_stop") return "text-red-700 dark:text-red-300 border-red-500/30 bg-red-500/10"; + if (status === "warning") return "text-amber-700 dark:text-amber-200 border-amber-500/30 bg-amber-500/10"; + return "text-emerald-700 dark:text-emerald-200 border-emerald-500/30 bg-emerald-500/10"; } export function BudgetPolicyCard({ @@ -56,14 +56,14 @@ export function BudgetPolicyCard({ const observedBudgetGrid = isPlain ? (
      -
      Observed
      +
      Observed
      {formatCents(summary.observedAmount)}
      {summary.amount > 0 ? `${summary.utilizationPercent}% of limit` : "No cap configured"}
      -
      Budget
      +
      Budget
      {summary.amount > 0 ? formatCents(summary.amount) : "Disabled"}
      @@ -75,14 +75,14 @@ export function BudgetPolicyCard({ ) : (
      -
      Observed
      +
      Observed
      {formatCents(summary.observedAmount)}
      {summary.amount > 0 ? `${summary.utilizationPercent}% of limit` : "No cap configured"}
      -
      Budget
      +
      Budget
      {summary.amount > 0 ? formatCents(summary.amount) : "Disabled"}
      @@ -102,12 +102,12 @@ export function BudgetPolicyCard({
      @@ -116,7 +116,7 @@ export function BudgetPolicyCard({ ); const pausedPane = summary.paused ? ( -
      +
      {summary.scopeType === "project" @@ -129,7 +129,7 @@ export function BudgetPolicyCard({ const saveSection = onSave ? (
      -