diff --git a/.agents/skills/add-block/SKILL.md b/.agents/skills/add-block/SKILL.md index 2361233e08a..24a0366b91f 100644 --- a/.agents/skills/add-block/SKILL.md +++ b/.agents/skills/add-block/SKILL.md @@ -902,16 +902,16 @@ Every block declares a one-line prose summary that replaces its card's field row ``` Slack ← header (already names the block) -Posts ⟨Ship it πŸš€βŸ© to ⟨#eng⟩ ← the sentence; βŸ¨β€¦βŸ© are live value chips +Post ⟨Ship it πŸš€βŸ© to ⟨#eng⟩ ← the sentence; βŸ¨β€¦βŸ© are live value chips ``` Write one `byOperation` entry per operation dropdown option (or a single `default` when the block has no operation dropdown). -**The full authoring contract β€” voice, structure, and the two mistakes that break +**The full authoring contract β€” voice, structure, and the four mistakes that break cards silently β€” is `apps/sim/blocks/AGENTS.md` β†’ "Canvas sentences". Read it -before writing any.** The two failures worth repeating here, because both are -invisible at runtime: +before writing any.** Two of those four are worth repeating here, because both +are invisible at runtime: 1. A clause naming only one member of a `canonicalParamId` pair drops the sentence for every advanced-mode user. List all members: diff --git a/.agents/skills/add-column-type/SKILL.md b/.agents/skills/add-column-type/SKILL.md index 4462dacb060..1ac05a8c9ec 100644 --- a/.agents/skills/add-column-type/SKILL.md +++ b/.agents/skills/add-column-type/SKILL.md @@ -148,13 +148,13 @@ Registering the *type* is compiler-enforced. Registering its *metadata* is not, - [ ] Icon added, centered on the family's optical center, exported alphabetically - [ ] `migrateCellsTo` / `migrateCellsFrom` added if the stored bytes change - [ ] New metadata keys added to `TYPE_SPECIFIC_COLUMN_KEYS` + `FOREIGN_METADATA_VERB` -- [ ] Unit tests for `coerce` / `isCompatibleWith` round-trips, verified to fail without the code +- [ ] Unit tests for `coerce` / `isCompatibleWith` round-trips only if they pass the `test-audit` authoring gate, verified to fail without the code - [ ] Docs row added to `apps/docs/content/docs/tables/index.mdx` ## Final Validation (Required) 1. **`cd apps/sim && bun run type-check`** β€” must be clean. If any file *outside* `column-types/` errors, that file has a hardcoded type list; fix it to read the registry. 2. **Grep for leaks** β€” `grep -rnE "(===|!==) '{id}'|case '{id}':" apps/sim --include='*.ts' --include='*.tsx' | grep -v column-types/`. (All three forms: a plain `!==` and a `case` are how half of `currency`'s real branches are written.) Hits are expected; judge each. A hit is fine when it mounts a specific React component or encodes a genuinely one-off behavior (`json`'s mono textarea, `date`'s timezone-aware parsing). A hit is a **leak** when it restates something the registry could answer β€” an icon, a label, a colour, an operator set, a cast, a coercion. Leaks get a registry field, not a new branch. -3. **Run the suite** β€” `bunx vitest run lib/table 'app/workspace/[workspaceId]/tables' lib/api app/api/table app/api/v1 lib/copilot/tools/server/table`. Existing tests must pass **unchanged**; needing to edit one means you changed behavior for the other types. -4. **`bun run lint:check`, `bun run check:api-validation`, `bun run check:client-boundary`** from the repo root. +3. **Run the suite** β€” `bun run --cwd apps/sim test lib/table 'app/workspace/[workspaceId]/tables' lib/api app/api/table app/api/v1 lib/copilot/tools/server/table`. Existing tests must pass **unchanged**; needing to edit one means you changed behavior for the other types. +4. **`bun run lint`, `bun run check:api-validation`, `bun run check:client-boundary`** from the repo root. 5. **Exercise it in the running app** on a table with one column of every type: create, edit inline / in the expanded popover / in the row modal, paste from a spreadsheet, filter, sort, convert to and from other types, export CSV, undo a column delete. diff --git a/.agents/skills/add-feature-flag/SKILL.md b/.agents/skills/add-feature-flag/SKILL.md index bb415d8585f..a5e1abf0e41 100644 --- a/.agents/skills/add-feature-flag/SKILL.md +++ b/.agents/skills/add-feature-flag/SKILL.md @@ -83,9 +83,9 @@ Critically, **none of this is expressible in code** β€” gating (especially `admi 4. **(Prod) configure in AppConfig.** The infra `feature-flags` profile schema is permissive, so a new flag needs **no infra change**. Operators add the flag to the hosted `feature-flags` document using `enabled` for global rollout or only the selected `workspaceIds`/`orgIds`/`userIds`/`adminEnabled` clauses for scoped rollout, then start a `sim--fast` deployment (see the AppConfig runbook in the infra README β€” same flow as `access-control`). The fallback secret only applies when AppConfig is disabled. -5. **Test.** Add a case to `apps/sim/lib/core/config/feature-flags.test.ts` that matches the chosen granularity. For a global flag, exercise `isFeatureEnabled('')` with an AppConfig `enabled` rule and toggle the fallback secret for the off-AppConfig path. For scoped rollout, cover only the selected clauses and mock `isPlatformAdmin` when testing `adminEnabled`. +5. **Test only new evaluation logic.** A flag that reuses the existing clauses is already covered by `apps/sim/lib/core/config/feature-flags.test.ts`; add no per-flag case. When you change how flags evaluate (a new clause kind, a new fallback path), add a case there that passes the `test-audit` authoring gate. -6. **Clean up after rollout.** When the feature ships to everyone, delete the flag's entry from `FEATURE_FLAGS`, the `` env entry, the AppConfig document, the call sites, and the test. Leaving dead flags around is the main failure mode of flag systems. +6. **Clean up after rollout.** When the feature ships to everyone, delete the flag's entry from `FEATURE_FLAGS`, the `` env entry, the AppConfig document, and the call sites. Leaving dead flags around is the main failure mode of flag systems. ## Notes diff --git a/.agents/skills/add-managed-cli/SKILL.md b/.agents/skills/add-managed-cli/SKILL.md index 906d99e5257..0917a55a34b 100644 --- a/.agents/skills/add-managed-cli/SKILL.md +++ b/.agents/skills/add-managed-cli/SKILL.md @@ -99,7 +99,7 @@ Do not special-case a CLI in those layers unless the registry contract cannot ex ## 6. Test the Addition -Extend tests when the new entry introduces behavior not already covered: +Extend tests only when the new entry introduces behavior not already covered and the test passes the `test-audit` authoring gate: - For every upgrade, add a regression proving the old ID and recipe remain resolvable but non-selectable, while the replacement ID is selectable. - Add important executable aliases to the table-driven search assertion. @@ -110,17 +110,13 @@ Never commit downloaded artifacts or credentials. ## Required Validation -From `apps/sim`: - ```bash -bunx vitest run \ +bun run --cwd apps/sim test \ lib/execution/remote-sandbox/cli-tools.test.ts \ lib/execution/remote-sandbox/cli-tools-boundary.test.ts \ lib/execution/remote-sandbox/sandbox-spec.test.ts \ lib/execution/remote-sandbox/resolve.test.ts \ - lib/api/contracts/sandboxes.test.ts \ - 'app/workspace/[workspaceId]/settings/components/sandboxes/utils.test.ts' \ - 'app/workspace/[workspaceId]/settings/components/sandboxes/components/sandbox-editor.test.tsx' + 'app/workspace/[workspaceId]/settings/components/sandboxes/utils.test.ts' ``` From the repository root: diff --git a/.agents/skills/add-permission-group-item/SKILL.md b/.agents/skills/add-permission-group-item/SKILL.md index e413adcd70f..43ff11e4106 100644 --- a/.agents/skills/add-permission-group-item/SKILL.md +++ b/.agents/skills/add-permission-group-item/SKILL.md @@ -210,7 +210,7 @@ bun run check:permission-group-enforcement bun run check:application-graph bun run check:capability-subject cd apps/sim && bun run type-check -cd apps/sim && bunx vitest run lib/permission-groups +bun run --cwd apps/sim test lib/permission-groups ``` Also `bun run check:api-validation` if you touched a contract or the group routes. `bun run check:audits` runs all of these; it derives its list from the `check:*` scripts in `package.json`, so a new audit is opted *out* deliberately rather than opted in. diff --git a/.agents/skills/add-selector/SKILL.md b/.agents/skills/add-selector/SKILL.md index ec5fc589737..3c4f0099f9d 100644 --- a/.agents/skills/add-selector/SKILL.md +++ b/.agents/skills/add-selector/SKILL.md @@ -122,7 +122,7 @@ list, manifest/registry exhaustiveness plus an existing provider primitive test Run the smallest relevant set, then: ```bash -bunx vitest run +bun run --cwd apps/sim test bun run --cwd apps/sim type-check bun run check:fork-dependent-coverage bun run check:client-boundary diff --git a/.claude/skills/add-settings-page/SKILL.md b/.agents/skills/add-settings-page/SKILL.md similarity index 100% rename from .claude/skills/add-settings-page/SKILL.md rename to .agents/skills/add-settings-page/SKILL.md diff --git a/.agents/skills/add-tools/SKILL.md b/.agents/skills/add-tools/SKILL.md index 85d34b491cd..39fbb963050 100644 --- a/.agents/skills/add-tools/SKILL.md +++ b/.agents/skills/add-tools/SKILL.md @@ -256,7 +256,7 @@ Hard rules: provider responses, filenames, URLs, and errors remain unchanged when Sim did not resolve a secret into them. -Add focused tests covering named projection, ordinary identical text without provenance, nested and +Run the `test-audit` authoring gate, then cover these risks at the boundary that owns them: named projection, ordinary identical text without provenance, nested and serialized shape handling, unchanged ordinary external inputs, malformed/incomplete private metadata failing closed, headerless legacy requests, and absence of private metadata in the public tool result. For durable sinks, also cover legacy `NULL` markers, exact-empty new writes, tracked secret writes, diff --git a/.agents/skills/babysit/SKILL.md b/.agents/skills/babysit/SKILL.md index d7bf60a75e4..83204194548 100644 --- a/.agents/skills/babysit/SKILL.md +++ b/.agents/skills/babysit/SKILL.md @@ -121,15 +121,13 @@ conditions freshly after every push. ``` 6. **Before pushing, re-run the full sync check from `/ship` step 2** β€” not just the log command, - the whole check-and-recover flow (stash WIP if needed, rebase, verify the rebase didn't just + the whole check-and-recover flow (stash WIP pinned by SHA as `/ship` step 2 shows, rebase, verify the rebase didn't just cleanly replay stray commits, cherry-pick rebuild if it did or if it conflicted). A babysit loop spanning a long session is exactly the scenario where a branch can drift, and pushing review fixes on top of undetected drift is how an oversized PR happens even after the branch - was fixed once. Then run the repo's pre-ship checks the same way `/ship` does before - committing β€” not just lint/typecheck/boundary-validation, but also the conditional `/cleanup` - (if this round's fix touched UI code) and `/db-migrate` (if it touched schema/migrations) - gates from `/ship` steps 4 and 5. A review-fix round is still a code change and can trip - either gate just as easily as the original commit did. + was fixed once. Then run `/ship` steps 4–6 on this round's diff β€” the cleanup and test gates, + 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 sync check rewrote history, which includes a plain `git rebase origin/staging` that completed diff --git a/.agents/skills/cleanup/SKILL.md b/.agents/skills/cleanup/SKILL.md index ddd4aa7ef9c..e957a1a64f7 100644 --- a/.agents/skills/cleanup/SKILL.md +++ b/.agents/skills/cleanup/SKILL.md @@ -16,9 +16,9 @@ User arguments: $ARGUMENTS Parse `$ARGUMENTS` into `scope` and `fix`: extract the `fix=true|false` token wherever it appears in the string and strip it from `scope`; defaults are the current changes and `fix=true`. `fix` is consumed by Step 3 only β€” the passes below always run `fix=false`. -Spawn all nine passes concurrently as subagents in a **single message** (multiple Agent tool calls). Each runs its skill on the parsed `scope` with `fix=false` β€” analysis and proposals ONLY, no edits. Instruct each agent to return its findings as a structured list: for every proposed change, the file path, line range, a one-line description of the change, and the exact before/after so the orchestrator can apply it without re-deriving. +Spawn up to nine passes concurrently as subagents in a **single message** (multiple Agent tool calls); pass 9 runs only when its condition holds. Each runs its skill on the parsed `scope` with `fix=false` β€” analysis and proposals ONLY, no edits. Instruct each agent to return its findings as a structured list: for every proposed change, the file path, line range, a one-line description of the change, and the exact before/after so the orchestrator can apply it without re-deriving. -Run these nine in parallel on the parsed `scope`: +Run these in parallel on the parsed `scope`: 1. `/you-might-not-need-an-effect fix=false` 2. `/you-might-not-need-a-memo fix=false` @@ -28,7 +28,7 @@ Run these nine in parallel on the parsed `scope`: 6. `/emcn-design-review fix=false` 7. `/you-might-not-need-url-state fix=false` 8. `/you-might-not-need-a-comment fix=false` -9. `/test-audit audit ` β€” read-only; only when the scope adds or changes test files (`*.test.ts(x)`, `*.integration.ts`, `e2e/**`). It applies the authoring gate to every new or changed test and proposes deleting the ones that fail it. +9. `/test-audit audit ` β€” read-only; only when the scope adds or changes test files (`*.test.ts(x)`, `*.integration.ts`, `**/e2e/**`, `apps/sim/scripts/test-*-e2e.ts`). First resolve a free-form scope to the concrete list of added or changed test paths (`git diff --name-only` against the scope's base) and pass those paths. It applies the authoring gate to every new or changed test and proposes deleting the ones that fail it. ## Step 2 β€” Converge @@ -52,7 +52,7 @@ Comments apply after every structural pass, on purpose: that pass operates on wh 2. If the `old_string` still matches verbatim, apply it β€” a content-anchored edit is safe even if its line moved. 3. If it no longer matches (an earlier pass altered that region), do **not** force the stale patch. Re-derive the change from the current code by re-applying that pass's rule to the construct, or drop it if a prior pass already made it moot. Never apply a proposal against text it wasn't computed from. -After all edits, run `bun run lint:check` (it runs `turbo run lint:check` across the repo β€” there is no per-file target, so run the full check). +After all edits, run `bun run lint` from the repo root (it autofixes formatting across the repo; there is no per-file target). ## Step 4 β€” Summary @@ -60,4 +60,4 @@ Output a summary across all passes that ran: what each found, what was applied v ## Boundary findings -Never resolve a boundary finding by adding a `// boundary-raw-fetch` / `// double-cast-allowed` annotation β€” fix the call (adopt the contract + `requestJson`, or narrow the type). Annotations are only for the documented exceptions in CLAUDE.md β†’ Boundary annotations. +Never resolve a boundary finding by adding a `// boundary-raw-fetch` / `// double-cast-allowed` annotation β€” fix the call (adopt the contract + `requestJson`, or narrow the type). Annotations are only for the documented exceptions in `.claude/rules/sim-api-contracts.md` β†’ Boundary annotations. diff --git a/.agents/skills/migrate-application-operation/SKILL.md b/.agents/skills/migrate-application-operation/SKILL.md index d8be0eaf32c..47ea0d2b618 100644 --- a/.agents/skills/migrate-application-operation/SKILL.md +++ b/.agents/skills/migrate-application-operation/SKILL.md @@ -80,7 +80,7 @@ Preserve behavior unless the task explicitly changes it. Stop and report a decis ## Freeze observable behavior before editing -Treat the legacy route or tool as an ordered program, not merely a bag of business logic. Before moving code, write a compact baseline for every in-scope entry point and add focused characterization tests for behavior not already pinned down. +Treat the legacy route or tool as an ordered program, not merely a bag of business logic. Before moving code, write a compact baseline for every in-scope entry point. Pin behavior no existing test covers with a characterization test only where it passes the `test-audit` gate; otherwise record it in the baseline and verify it by hand after the move. Capture all of these when they apply: @@ -315,18 +315,16 @@ Do not force these through an ordinary JSON migration: Stop and report a missing design rather than weakening identity, authorization, limits, or errors. -## Test the complete matrix +## Test each risk at one boundary -Add focused tests for every migrated surface and principal kind allowed by the operation: +Run the `test-audit` authoring gate before writing any test. Own each risk at exactly one boundary: + +- Application use-case tests own authorization, principal-kind rejection before canonical loading, workspace assertion mismatch, delegated scope, not found, conflict, no-op, audit derived from authoritative results, and infrastructure failures (storage, rate-limit, provider, or database errors raised by delegated services) propagating as 5xx-mapped errors β€” never converted to not-found or forbidden. +- One `*.integration.ts` owns repository semantics: canonical active lookup, workspace-predicated writes, archived resources, authoritative affected rows, and database error propagation. +- Add a surface test only for a surface-specific risk (for example, a v2 envelope or rate header, a Copilot forged-scope rejection, or a legacy redirect/cookie behavior the characterization baseline pinned). Do not restate the operation registry or the shared builders' auth-before-parse behavior per surface. + +Risks that usually earn a test when the change introduces them: -- Application: allowed and disallowed roles, principal-kind rejection before canonical loading, workspace assertion mismatch, delegated scope, not found, conflict, no-op, and infrastructure propagation. -- Operation registry: role/workspace-key/principal-kind/delegated-service consistency and fail-fast rejection of invalid definitions. -- Repository: canonical active lookup, workspace-predicated writes, archived resources, authoritative affected rows, and database error propagation. -- Internal API: authentication before parsing, exact contract, typed errors, and surface analytics only after success. -- Public API: personal and workspace keys, rate behavior, concealment, exact external envelope, and rate headers. -- Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. -- Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. -- Compatibility characterization: legacy normalization, exact response/redirect/cookie behavior, concealment, error subclass precedence, and branch-specific output. - Failure sequencing: inject a failure after each independently committing step and assert persisted state plus audit, analytics, and notification effects. - Concurrency: overlap stateful browser or provider flows and prove each callback consumes only its own state and return destination. - Rendering boundaries: exercise hostile values for every newly connected input that reaches HTML, inline JavaScript, URLs, logs, or provider requests. @@ -334,7 +332,7 @@ Add focused tests for every migrated surface and principal kind allowed by the o Run at minimum: ```bash -bunx vitest run +bun run --cwd apps/sim test bunx biome check bunx turbo run type-check --filter=@sim/app --filter=@sim/auth bun run check:api-validation:strict diff --git a/.agents/skills/react-query-best-practices/SKILL.md b/.agents/skills/react-query-best-practices/SKILL.md index 99f96b259ce..0e426b8bd34 100644 --- a/.agents/skills/react-query-best-practices/SKILL.md +++ b/.agents/skills/react-query-best-practices/SKILL.md @@ -26,7 +26,7 @@ Read these before analyzing: ## Rules to enforce ### Query keys and hooks -Enforce CLAUDE.md "React Query" and `.claude/rules/sim-queries.md` (key factory with `all` + plural prefixes, `signal` forwarding, named `staleTime` constants reused by prefetches, `keepPreviousData` only on variable keys, `requestJson` boundary). Additionally: +Enforce `.claude/rules/sim-queries.md` (key factory with `all` + plural prefixes, `signal` forwarding, named `staleTime` constants reused by prefetches, `keepPreviousData` only on variable keys, `requestJson` boundary). Additionally: - Key factories live next to their hooks β€” except a factory, standalone fetcher/mapper, or `staleTime` constant that a server module (a `prefetch.ts`, route, block, trigger) imports, which must live in a non-`'use client'` module under `hooks/queries/utils/` per `.claude/rules/sim-queries.md` (a `'use client'` export called from the server crashes SSR) - Use `enabled` to prevent queries from running without required params - Warm data for hover/focus intent with `queryClient.prefetchQuery` and shared `queryOptions`; never temporarily enable a mounted hidden observer, which can remain active after focus restoration and refetch data for closed UI @@ -37,7 +37,7 @@ Enforce CLAUDE.md "React Query" and `.claude/rules/sim-queries.md` (key factory - Server prefetches must call the authorized use case, apply the route presenter/response schema, and reuse the client's exact key, mapper, and stale time. Keep all fallible auth/read/parse work inside `queryFn` so an optional warm cannot fail the page, and never bypass a route that redacts fields. ### Mutations -Enforce CLAUDE.md "Mutation Hooks" (targeted invalidation, `onMutate`/`onError` rollback, mutation objects out of `useCallback` deps). Additionally: +Enforce `.claude/rules/sim-queries.md` "Mutation Hook" (targeted invalidation, `onMutate`/`onError` rollback, mutation objects out of `useCallback` deps). Additionally: - Plain mutations invalidate in `onSuccess`; optimistic mutations reconcile in `onSettled` (fires on success and error) with rollback in `onError` β€” see `.claude/rules/sim-queries.md` "Mutation Hook" / "Optimistic Updates" ### Server state ownership diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index b866ee9a149..c4c6372ff73 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -15,7 +15,15 @@ When the user runs `/ship`: 1. **Check git status** - See what files have changed 2. **Sync check**: `git fetch origin staging && git log --oneline origin/staging..HEAD`. The list must contain ONLY commits you can attribute to this session (recognizable subjects/SHAs) β€” a worktree/branch cut from a stale local `staging` silently drags in unrelated commits. - If it shows commits you don't recognize, fix it now, **before** staging/committing any new work (step 7 hasn't run yet): - - If the working tree has uncommitted changes, stash them first β€” `git stash push -u -m ship-sync-fix` β€” so the rebase below isn't blocked by dirty state. Restore with `git stash pop` once the branch is fixed. + - If the working tree has uncommitted changes, stash them first so the rebase below isn't blocked by dirty state, and pin the entry by SHA β€” the stash list is shared across every worktree of the repo, so `stash@{0}` and `git stash pop` can grab another session's entry: + ```bash + git stash push -u -m ship-sync-fix && SHIP_STASH=$(git rev-parse 'stash@{0}') + # once the branch is fixed (`git stash drop` rejects a raw SHA, so resolve the pinned + # entry's current stash@{n} and drop only that; an empty lookup drops nothing): + git stash apply "$SHIP_STASH" && + SHIP_STASH_REF=$(git stash list --format='%gd %H' | awk -v s="$SHIP_STASH" '$2==s{print $1}') && + { [ -z "$SHIP_STASH_REF" ] || git stash drop "$SHIP_STASH_REF"; } + ``` - Try `git rebase origin/staging` first. - **A rebase finishing without conflicts does NOT by itself mean the branch is clean** β€” it can replay stray commits onto the new base with no conflict at all. After the rebase (clean or not), re-run `git log --oneline origin/staging..HEAD` and re-check the commit list against what you recognize. - If the rebase conflicted on unrecognized commits, OR finished cleanly but the log still shows them, abandon it (`git rebase --abort` if mid-rebase) and rebuild, in this exact order: @@ -32,7 +40,8 @@ When the user runs `/ship`: - Keep it concise 4. **Run the cleanup and test gates** - If the diff modifies UI code (any non-test `.tsx` file, or anything under `apps/sim/components/`, `apps/sim/hooks/`, or `apps/sim/stores/`), run `/cleanup`. It fans out the React/UI passes (effects, memo, callbacks, state, React Query, emcn, url-state), the comment pass, and the test-audit pass, and applies fixes so they land in this commit. - - Otherwise, if the diff adds or changes tests (`*.test.ts(x)`, `*.integration.ts`, `e2e/**`), run `/test-audit audit ` on its own. Every new or changed test must pass the authoring gate; delete the ones that don't rather than shipping them. + - Otherwise, if the diff adds or changes tests (`*.test.ts(x)`, `*.integration.ts`, `**/e2e/**`, `apps/sim/scripts/test-*-e2e.ts`), run `/test-audit audit ` on its own. Every new or changed test must pass the authoring gate; delete the ones that don't rather than shipping them. + - Then run the test files the diff adds or changes, plus the existing tests beside changed source files, with `bun run --cwd test ` (`bun run --cwd apps/sim test ` for the app; `*.integration.ts` needs the setup in `.claude/rules/sim-testing.md`). A failing test aborts ship. 5. **Run migration safety** β€” only if the diff touches `packages/db/migrations/**` or `packages/db/schema.ts`: - Run `/db-migrate` to review the migration for zero-downtime safety (expand/contract phasing, backward-compatibility with the deployed app version). - `bun run check:migrations origin/staging` must pass (staging is the PR base). Do not silence a flagged statement with a `-- migration-safe:` annotation unless `/db-migrate` confirmed the old code no longer depends on it; otherwise split the destructive change into a later deploy. @@ -139,7 +148,7 @@ Use this exact template in the user's voice (concise, bullet points): - [x] Bug fix (or appropriate type) ## Testing -Tested manually (or describe testing) +Describe the checks, tests, and E2E artifacts run ## Checklist - [x] Code follows project style guidelines @@ -169,6 +178,6 @@ gh pr create --base staging --title "COMMIT_MESSAGE" --body "PR_BODY" - Short, direct bullet points - No unnecessary explanation -- "Tested manually" is acceptable for testing section; include lint, boundary validation, and (when migrations changed) `check:migrations` results when run +- Testing section names what actually ran: the test files, lint, `check:audits`, (when migrations changed) `check:migrations`, and any E2E artifacts - Checkboxes filled in appropriately - No screenshots section unless UI changes diff --git a/.agents/skills/test-audit/SKILL.md b/.agents/skills/test-audit/SKILL.md index bd685aa6f20..31c3f924a03 100644 --- a/.agents/skills/test-audit/SKILL.md +++ b/.agents/skills/test-audit/SKILL.md @@ -73,6 +73,8 @@ boundary covers the bug; do not replay it at every layer it crosses. - negative controls that pass for an unrelated reason (a different guard short-circuits first); - names or fixtures that promise more than the input exercises; - dead production code or exports whose only callers are tests. +- a hand-rolled `vi.mock` factory for a module `vitest.setup.ts` or `@sim/testing` already mocks, or a + local copy of a `@sim/testing` helper (`bun run check:test-patterns` fails on these). ## Retention bar @@ -133,10 +135,7 @@ For a whole subsystem or the whole repo: Never edit source or tests while Vitest is running in the same checkout. -1. Run the touched and sibling test files. From `apps/sim`: - `../../node_modules/.bin/vitest run ` (never `bunx vitest`, which fetches a different - Vitest). Other workspaces: run from the workspace directory. Never pipe the runner through - `grep`/`tail` where the pipe hides its exit code. +1. Run the touched and sibling test files (`.claude/rules/sim-testing.md` β†’ Running). 2. If production code changed: `bun run type-check` in that workspace. 3. `bun run check:audits` from the repo root (some audits list test files by path). 4. `bun run lint`, then `git diff --check`. diff --git a/.agents/skills/v2-api-conventions/SKILL.md b/.agents/skills/v2-api-conventions/SKILL.md index 0b77773666a..b177330a771 100644 --- a/.agents/skills/v2-api-conventions/SKILL.md +++ b/.agents/skills/v2-api-conventions/SKILL.md @@ -87,7 +87,7 @@ Use the shared sets in `contracts/v2/openapi/shared.ts` β€” `RESOURCE_ERRORS`, ` ## Rule 3 β€” a collection that returns `nextCursor` must accept `limit` + `cursor`, and must apply them -Every list returns `{ data, nextCursor }`. Whether it *pages* is a separate, pinned decision β€” see `lib/api/contracts/v2/__tests__/list-pagination.test.ts`, which enumerates both sets and fails when a new list is in neither. +Every list returns `{ data, nextCursor }`. Whether it *pages* is a separate, pinned decision β€” see `lib/api/contracts/v2/list-pagination.test.ts`, which enumerates both sets and fails when a new list is in neither. Build the query slice from the shared helper, never by hand: @@ -106,13 +106,13 @@ Both take the same two stamps: `cursorSortKey(sortBy, sortOrder)` for the orderi The third is **per-domain**: a list whose read predates the shared codecs, or whose page boundary is not expressible as one, mints its own β€” a bare `encodeCursor({ version })` on `GET /workflows/{id}/versions` and `encodeCursor({ email })` on the workspace member list, the local codecs in `lib/audit-logs/query.ts`, `lib/logs/list-logs.ts`, and `lib/table/rows/cursor.ts`, and a usage-event id passed straight through by `GET /billing/logs`. Those tokens stay opaque and untouched, but a domain-minted cursor on a list a caller can re-filter is wrapped at the surface with `encodeScopedCursor(cursorScopeKey(cursorRoute(contract, pathParams), {...}), token)` and unwrapped with `readScopedCursor`, so it carries the same binding as the shared schemes. **A new list picks one of the two shared schemes.** Do not add a fourth. -Every paged list's binding is declared in `lib/api/contracts/v2/__tests__/list-pagination.test.ts` and checked against what the contract actually accepts, in both directions. A new list, or a new filter on an existing one, fails that test until its binding is declared or the param is explicitly recorded as unable to change the sequence. +Every paged list's binding is declared in `lib/api/contracts/v2/list-pagination.test.ts` and checked against what the contract actually accepts, in both directions. A new list, or a new filter on an existing one, fails that test until its binding is declared or the param is explicitly recorded as unable to change the sequence. **A keyset's key list must end in a unique column (`id`).** A non-unique trailing key cannot separate tied rows, so the page boundary either repeats or drops them. `lib/api/list-keyset-paging.test.ts` demonstrates the failure. Return `nextCursor: null` on the last page and only then. Never construct a cursor client-side. -**Ordering is `sortBy` + `sortOrder`, except where there is nothing to sort by.** Nearly every paged list takes the pair; `CURSOR_BINDINGS` in `contracts/v2/__tests__/list-pagination.test.ts` is the authoritative set. Exactly one β€” `GET /workflows/{workflowId}/runs` β€” has a single sortable column (start time), so there is no `sortBy` to pair with and the direction rides on a single `order` param; `sortBy`/`sortOrder` are not accepted there. That is the *only* sanctioned deviation, and it is documented in its contract. A new list picks the pair. Do not "fix" it by accepting `sortOrder` as an alias: an alias is a second spelling of one thing with undefined precedence when both arrive, which is its own inconsistency. +**Ordering is `sortBy` + `sortOrder`, except where there is nothing to sort by.** Nearly every paged list takes the pair; `CURSOR_BINDINGS` in `contracts/v2/list-pagination.test.ts` is the authoritative set. Exactly one β€” `GET /workflows/{workflowId}/runs` β€” has a single sortable column (start time), so there is no `sortBy` to pair with and the direction rides on a single `order` param; `sortBy`/`sortOrder` are not accepted there. That is the *only* sanctioned deviation, and it is documented in its contract. A new list picks the pair. Do not "fix" it by accepting `sortOrder` as an alias: an alias is a second spelling of one thing with undefined precedence when both arrive, which is its own inconsistency. Before documenting a second `order`-style exception, check every other endpoint on the same collection: if one of them already sorts those rows more than one way, the "exactly one sortable column" premise is false β€” fix the premise rather than documenting the exception. diff --git a/.agents/skills/validate-permission-group-item/SKILL.md b/.agents/skills/validate-permission-group-item/SKILL.md index 3488765cdf2..9422ddbe6c5 100644 --- a/.agents/skills/validate-permission-group-item/SKILL.md +++ b/.agents/skills/validate-permission-group-item/SKILL.md @@ -109,7 +109,8 @@ For an allowlist the three states must be tested separately β€” `null` permits e bun run check:permission-group-enforcement bun run check:application-graph bun run check:capability-subject -cd apps/sim && bun run type-check && bunx vitest run lib/permission-groups +cd apps/sim && bun run type-check +bun run --cwd apps/sim test lib/permission-groups ``` All three are inside `check:audits`, which derives its list from the `check:*` scripts in `package.json` β€” a new audit is opted *out* deliberately. Read the output, not the exit codes. Success-line shapes (the counts must include the item under audit): diff --git a/.agents/skills/you-might-not-need-a-comment/SKILL.md b/.agents/skills/you-might-not-need-a-comment/SKILL.md index 788bbbda174..602ba36cb49 100644 --- a/.agents/skills/you-might-not-need-a-comment/SKILL.md +++ b/.agents/skills/you-might-not-need-a-comment/SKILL.md @@ -16,7 +16,7 @@ User arguments: $ARGUMENTS A comment must add information the code cannot express itself. Code says *what* and *how*; a comment earns its place only by explaining *why* β€” a non-obvious constraint, a workaround, a decision, a gotcha. If deleting the comment loses no information a competent reader wouldn't recover from the code in seconds, delete it. -This codebase's convention: **TSDoc for documentation, no non-TSDoc comments, no `====` separators.** Genuine documentation belongs in a `/** ... */` block on the declaration; everything that survives as an inline `//` comment must be a real *why*, kept terse. +This codebase's convention: **TSDoc for documentation; an inline `//` only for a terse non-obvious why or a script-enforced annotation; no `====` separators.** Genuine documentation belongs in a `/** ... */` block on the declaration; everything that survives as an inline `//` comment must be a real *why*, kept terse. ## Anti-patterns to detect diff --git a/.claude/rules/emcn-components.md b/.claude/rules/emcn-components.md index f02ff3946e5..92f95238590 100644 --- a/.claude/rules/emcn-components.md +++ b/.claude/rules/emcn-components.md @@ -15,7 +15,7 @@ Never hand-roll the chip pill from raw class strings (they go stale). Compose fr - **Surface, typography + content tokens:** `chip/chip-chrome.ts` β€” `chipFilledSurfaceTokens`, `chipFieldSurfaceClass`, `chipFieldTextClass` (text fields and the dropdown search box build on these), plus the chip-content chrome `chipContentGap`, `chipGeometryClass`, `chipContentIconClass`, `chipContentLabelClass`, `cellIconNodeClass` (non-chip surfaces that must visually match chip content, e.g. resource table cells), and the row-state pair `chipHoverSurfaceClass` / `chipActiveSurfaceClass` (hover vs. selected β€” mutually exclusive, so a selected row holds its surface through hover; every hand-rolled row imports these rather than restating the literals). All are re-exported from the `@sim/emcn` barrel β€” no subpath import needed. - **Pill geometry:** `chip/chip.tsx` β€” `chipVariants` (30px tall, `rounded-lg`, `px-2`, icon↔text `gap-1.5`). Every pill-shaped trigger (`ChipDropdown`, `ChipSelect`, `ChipSwitch`) reuses it for visual parity. -Canonical look: normal font-weight (never `font-medium`/`font-semibold`), value text `--text-body`, icons `--text-icon` at `size-[14px]`, placeholder `--text-muted`, `transition-colors`, **no focus ring** (the caret marks focus). Filled surface is `--surface-5` light / `--surface-4` dark with a `--border-1` border. +Canonical look: normal font-weight (never `font-medium`/`font-semibold`), value text `--text-body`, icons `--text-icon` at `size-[14px]`, placeholder `--text-muted`, `transition-colors`, **no focus ring** (the caret marks focus). Filled surface is `--surface-5` light / `--surface-4` dark with a `--border` border (`chip-chrome.ts` still spells it through the legacy alias `--border-1`; new code writes `--border`). The menu surface intentionally diverges from the pill: `dropdown-menu.tsx` items use `text-small` and `gap-2` (a menu convention, not the chip pill). Keep them distinct. @@ -27,7 +27,7 @@ The menu surface intentionally diverges from the pill: `dropdown-menu.tsx` items - **`ChipTextarea`** β€” multi-line sibling. `error`, `resizable` (off by default), `viewOnly` (read-only at full opacity with the default cursor β€” the multi-line counterpart of `ChipCopyInput`). - **`ChipDropdown`** β€” pill that opens a menu. Single OR multi-select via the discriminated `multiple` prop (one component, not two). Owns its trailing chevron β€” no `rightIcon`. - **`ChipSelect` / `ChipCombobox`** β€” `Combobox`-backed pickers with search, groups, multi-select; for richer lists than `ChipDropdown`. -- **`ChipModal` + `ChipModalField`** β€” declarative compact modal. The field's `type` (`input` | `email` | `textarea` | `dropdown` | `copy` | `file` | `emails` | `custom`) picks the control and **owns all chrome** β€” consumers describe intent, never pass `variant`/`className`/`id` to the inner control. `custom` is the escape hatch. **Every body field MUST be a `ChipModalField`** β€” never hand-roll a field row (raw `
` + hand-rolled `

`/`

` per page, in Hero only β€” never add another. -- Strict heading hierarchy: H1 (Hero) β†’ H2 (section titles) β†’ H3 (feature names). -- Every section: `
`. +- One `

` per page, in the hero only β€” never add another. The brand carries in the title tag, the meta description, and the hero's `sr-only` summary, so the H1 is free to lead with the non-brand keywords people search ("AI workspace", "AI agents") rather than "Sim is the". +- Strict heading hierarchy: H1 (hero) β†’ H2 (section titles) β†’ H3 (items within a section). Never skip a level. +- Semantic landmarks: `
`, `
`, `