diff --git a/.agents/skills/babysit/SKILL.md b/.agents/skills/babysit/SKILL.md index 83204194548..881d0117005 100644 --- a/.agents/skills/babysit/SKILL.md +++ b/.agents/skills/babysit/SKILL.md @@ -57,6 +57,12 @@ All three must hold: reported yet, and treating "not failing" as "passing" reports the PR clean before CI has had its say. Wait for it — the step-10 stop condition covers a check that never settles. +A passing design-conformance CI step can still contain warnings. Read its latest report and +triage findings using `/ship`'s [committed design check](../ship/SKILL.md#committed-design-check). +Intentional system changes and justified exceptions may remain once explained in the PR; +they do not prevent a clean review or require another fix loop. Honor decisions already made +in this session. Operational failures must be resolved before reporting the PR clean. + Do not stop early on "no new comments this round" alone — a thread can be open from an earlier round, and cubic often lands its first threads a round after Greptile's. Always check all three conditions freshly after every push. @@ -85,11 +91,14 @@ conditions freshly after every push. If `mergeable` is `CONFLICTING`, fix that first (step 2). If a check is failing, fix that too — treat it exactly like a review finding. If a check is still `pending`, do not evaluate "clean" at all: go to step 9 and wait for it. Otherwise, if Greptile is 5/5, every thread - across all pages has `isResolved: true`, and every check has finished and passed, stop — + across all pages has `isResolved: true`, every check has finished and passed, and any design + warnings have been triaged as above, stop — report the outcome (see "Reporting" below) and skip the rest of this list. 2. **If the PR has a merge conflict**, merge `origin/staging`, resolve the conflicts, run the - usual pre-push checks, push, and go to step 8 to re-trigger review. + usual pre-commit checks and commit the resolution. Run `/ship`'s + [committed design check](../ship/SKILL.md#committed-design-check) against the resulting HEAD + before pushing, then go to step 8 to re-trigger review. 3. **If no review has run yet** (fresh PR, no bot comments): both run automatically on PR open — confirm via `gh pr checks ` (look for `Greptile Review` and `cubic · AI code reviewer`) and @@ -129,7 +138,11 @@ conditions freshly after every push. migration safety, and the regenerate + audit phases. A review-fix round is still a code change and can trip any of them just as easily as the original commit did. -7. **Commit and push** the round's fixes as one commit — `--force-with-lease` whenever step 6's +7. **Commit, check and push** the round's fixes as one commit. After committing and before + every push, follow `/ship`'s [committed design check](../ship/SKILL.md#committed-design-check), + including warning triage and committing/rechecking any resulting fixes. Push only the + checked HEAD; rerun after a rebase or any other change to the comparison. + Use `--force-with-lease` whenever step 6's sync check rewrote history, which includes a plain `git rebase origin/staging` that completed with no conflicts, not only the cherry-pick rebuild path; both rewrite commits already published to the remote, so a plain `git push` can be rejected either way — then run `/ship` diff --git a/.agents/skills/emcn-design-review/SKILL.md b/.agents/skills/emcn-design-review/SKILL.md index 2f14b66c83c..3f454d64f99 100644 --- a/.agents/skills/emcn-design-review/SKILL.md +++ b/.agents/skills/emcn-design-review/SKILL.md @@ -1,84 +1,19 @@ --- name: emcn-design-review -description: Review UI code for alignment with the emcn design system — components, tokens, patterns, and conventions +description: Review product UI changes for design drift using the local conformance check, EMCN components, and global styles. argument-hint: "[scope] [fix=true|false]" --- -# EMCN Design Review - -Arguments: -- scope: what to review (default: your current changes). Examples: "diff to main", "PR #123", "src/components/", "whole codebase" -- fix: whether to apply fixes (default: true). Set to false to only propose changes. +# EMCN design review User arguments: $ARGUMENTS -## Context - -This codebase uses **emcn**, a custom component library built on Radix UI primitives with CVA variants and CSS variable design tokens. All UI must use emcn components and tokens. - -## Steps - -1. Read the emcn public barrel at `packages/emcn/src/index.ts` (re-exports components, Calendar, Table*, and icons) to know what's available; for the full icon set read `packages/emcn/src/icons/index.ts` -2. Read `apps/sim/app/_styles/globals.css` for CSS variable tokens -3. Analyze the specified scope against every rule below -4. If fix=true, apply the fixes. If fix=false, propose the fixes without applying. - ---- - -## Imports - -- Components, `cn`, and tokens from the `@sim/emcn` barrel, never component subpaths -- Icons from `@sim/emcn/icons` - -## Design Tokens - -Use CSS variable pattern (`text-[var(--text-primary)]`), never Tailwind semantics (`text-muted-foreground`) or hardcoded colors (`text-gray-500`, `#333`). - -**Text**: `--text-primary`, `--text-secondary`, `--text-tertiary`, `--text-muted`, `--text-body` (canonical value text), `--text-icon`, `--text-placeholder`, `--text-subtle`, `--text-inverse`, `--text-error` -**Surfaces**: `--bg`, `--surface-1` through `--surface-7`, `--surface-hover`, `--surface-active` -**Borders**: `--border` (`--border-1`/`--border-muted` are legacy aliases resolving to it — flag new uses) -**Brand/accent**: `--brand-secondary`, `--brand-accent` -**Z-Index**: `--z-dropdown` (100), `--z-toast` (150), `--z-modal` (200), `--z-popover` (300), `--z-tooltip` (400), `--z-takeover` (500), `--z-shell-gate` (600) -**Shadows**: `shadow-subtle`, `shadow-medium`, `shadow-overlay`, `shadow-card` -**Badges**: `--badge-*` semantic families (success/error/gray/blue/purple/orange/amber/teal/cyan/pink, each with `-bg`/`-text`) - -## Buttons - -Intent-to-variant mapping (read the actual `buttonVariants` in `packages/emcn/src/components/button/button.tsx` for the full variant set — it exposes more than listed here): - -| Action | Variant | -|--------|---------| -| Toolbar, icon-only | `ghost` | -| Create, save, submit | `primary` | -| Cancel, close | `default` | -| Delete, remove | `destructive` | -| Selected state | `active` | -| Toggle | `outline` | - -## Delete/Remove Confirmations - -`ChipModal` `size='sm'`, title "Delete/Remove {ItemType}", destructive confirm button, plain Cancel (follow the chip footer layout in `.claude/rules/emcn-components.md`). Use `text-[var(--text-error)]` for irreversible warnings. - -## Toast - -`toast.success()`, `toast.error()`, `toast()` from `@sim/emcn`. Never custom notification UI. - -## Badges - -`red`=error/failed, `gray-secondary`=metadata/roles, `type`=type annotations, `green`=success/active, `gray`=neutral, `amber`=processing, `orange`=paused, `blue`=info. Use `dot` prop for status indicators. - -## Icons - -Default: `size-[14px]`. Color: `text-[var(--text-icon)]`. Scale: 14px > 16px > 12px > 20px. Use the `size-*` shorthand — flag `h-[Npx] w-[Npx]` and `h-N w-N` pairs as refactor targets. +Interpret the arguments as the product UI scope (default: current changes) and an optional `fix=true|false` mode (default: `false`). When `fix=false`, explain proposed changes without applying them. -## Anti-patterns to flag +1. When EMCN, global styles, recipes or design ownership metadata change, run `bun run design:generate` and commit `scripts/design-conformance/contracts.generated.json` with the source. `bun run check:design-generated` checks freshness without writing. Regeneration does not hide the originating design-system finding. +2. During UI work, run `bun run check:design --base origin/staging --working-tree` from the repo root, substituting the actual PR target for `origin/staging`. After committing, use `--head HEAD` for the immutable PR comparison. Exit 1 means findings to review; exit 2 means the check failed and must be repaired or reported. CI is warning-only for findings and fails on incomplete analysis. +3. For each new finding, inspect the cited source, the applicable public EMCN export in `packages/emcn/src/index.ts`, and tokens and recipes in `apps/sim/app/_styles/globals.css`. Reuse a suitable component, prop, variant, or global token when it expresses the design intent. Avoid near-duplicate local colours or overriding EMCN chrome merely for convenience. +4. A genuinely new product treatment may remain an Extra. Explain its visual intent and why existing EMCN or global styling does not fit in the PR. The check does not decide design approval and must not be silenced by adding an arbitrary token, broad exclusion, or fake component wrapper. Ask the designer or engineer when changing a shared recipe would have broad or ambiguous effects. +5. Keep unresolved `unchecked` inputs and inspection failures separate from findings. A quiet diff means no *new detected* debt, not proof of complete visual conformance. Existing debt stays quiet; a new copy can warn. Landing and docs are out of product scope; Monaco presentation, provider branding, and block identity palettes have deliberate exclusions. See `scripts/design-conformance/README.md` for exact rule boundaries. -- Raw `