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 4a286d701ed..e957a1a64f7 100644 --- a/.agents/skills/cleanup/SKILL.md +++ b/.agents/skills/cleanup/SKILL.md @@ -1,6 +1,6 @@ --- name: cleanup -description: Run all code quality skills — effects, memo, callbacks, state, React Query, emcn design review, url-state, and comments — analyzing in parallel, then applying fixes sequentially +description: Run all code quality skills — effects, memo, callbacks, state, React Query, emcn design review, url-state, comments, and test-audit — analyzing in parallel, then applying fixes sequentially argument-hint: "[scope] [fix=true|false]" --- @@ -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 eight 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 eight 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,10 +28,11 @@ Run these eight 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/**`, `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 -Collect all findings into one list, **keeping each proposal tagged with the pass that produced it** — do NOT collapse a file's proposals into a single unlabeled patch, because Step 3 applies in pass order and needs those labels. Detect overlaps where two passes touch the same region (common: a state pass and an effect pass on the same block, or a memo and callback pass on the same component). Reconcile only genuine same-region conflicts, and drop proposals a sibling pass has made moot; a reconciled change inherits the pass label of whichever of its passes comes first in the Step 3 dependency order (effects → state → memo → callback → React Query → url-state → emcn → comments), so it is applied at the earliest safe point. Non-overlapping proposals stay as-is with their own labels. The output is a per-pass list of surviving changes, not a per-file patch. +Collect all findings into one list, **keeping each proposal tagged with the pass that produced it** — do NOT collapse a file's proposals into a single unlabeled patch, because Step 3 applies in pass order and needs those labels. Detect overlaps where two passes touch the same region (common: a state pass and an effect pass on the same block, or a memo and callback pass on the same component). Reconcile only genuine same-region conflicts, and drop proposals a sibling pass has made moot; a reconciled change inherits the pass label of whichever of its passes comes first in the Step 3 dependency order (effects → state → memo → callback → React Query → url-state → emcn → comments → tests), so it is applied at the earliest safe point. Non-overlapping proposals stay as-is with their own labels. The output is a per-pass list of surviving changes, not a per-file patch. ## Step 3 — Sequential apply @@ -39,11 +40,11 @@ If `fix=false`, skip this step — just report the proposals from Step 2. Otherwise apply the surviving changes yourself (in the main context, not delegated), iterating **pass by pass** in this dependency order so earlier structural changes settle before later passes build on them: -1. effects → 2. state → 3. memo → 4. callback → 5. React Query → 6. url-state → 7. emcn design → 8. comments +1. effects → 2. state → 3. memo → 4. callback → 5. React Query → 6. url-state → 7. emcn design → 8. comments → 9. tests For each pass in turn, apply all of that pass's changes, then move to the next pass. A file touched by several passes is therefore edited once per pass, in this order — not once as a merged patch. This is what makes the ordering real: a single merged-per-file patch would collapse all passes into one edit and lose it. -Comments apply last, on purpose: that pass operates on whatever the earlier structural passes settled the code into, so it never edits lines a sibling pass is about to delete or rewrite. +Comments apply after every structural pass, on purpose: that pass operates on whatever the earlier passes settled the code into, so it never edits lines a sibling pass is about to delete or rewrite. Tests apply last because they only touch test files; in Step 2, drop any other pass's proposal on a test file the tests pass deletes. **Treat every Step 1 proposal as snapshot-relative, not authoritative.** All passes analyzed the *original* files in parallel, so a proposal's line ranges and before/after text describe the code as it was *before* any edits — once an earlier pass has run, a later pass's snippet may no longer match. So for each change, before applying: @@ -51,12 +52,12 @@ Comments apply last, on purpose: that pass operates on whatever the earlier stru 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 -Output a summary across all eight passes: what each found, what was applied vs. skipped-as-redundant, and any proposals that need a human decision. +Output a summary across all passes that ran: what each found, what was applied vs. skipped-as-redundant, and any proposals that need a human decision. ## 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 943ea393f87..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: @@ -30,8 +38,10 @@ When the user runs `/ship`: - Types: `fix`, `feat`, `improvement`, `chore` - Scope: short identifier (e.g., `undo-redo`, `api`, `ui`) - Keep it concise -4. **Run the cleanup pass** — only if the diff modifies UI code (any `.tsx` file, or anything under `apps/sim/components/`, `apps/sim/hooks/`, or `apps/sim/stores/`): `/cleanup` - - `/cleanup` fans out the React/UI passes (effects, memo, callbacks, state, React Query, emcn, url-state) plus the comment pass; skip it when no UI was touched. When it runs, it applies fixes so they land in this commit. +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/**`, `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. @@ -138,12 +148,12 @@ 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 - [x] Self-reviewed my changes -- [ ] Tests added/updated and passing +- [ ] Tests added/updated and passing (new tests pass the `test-audit` authoring gate) - [x] No new warnings introduced - [x] I confirm that I have read and agree to the terms outlined in the [Contributor License Agreement (CLA)](./CONTRIBUTING.md#contributor-license-agreement-cla) ``` @@ -168,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 new file mode 100644 index 00000000000..31c3f924a03 --- /dev/null +++ b/.agents/skills/test-audit/SKILL.md @@ -0,0 +1,147 @@ +--- +name: test-audit +description: Invoke whenever writing, changing, reviewing, or sweeping tests. Authoring gate for new tests, plus an audit workflow for low-value, implementation-coupled, or duplicative tests and the test-only production seams they demand. +argument-hint: "[author | audit | campaign ]" +--- + +# Test Audit + +Three modes, one value bar. **Authoring** gates every new or changed test at write time. +**Audit** runs a focused sweep of existing tests that re-assert source, duplicate stronger proof, +couple to implementation, or keep test-only production seams alive. **Campaign** prunes one +subsystem's whole test surface in parallel lanes. Optimize for confidence, not deletion count — +but a test that cannot name the bug it catches is cost, not coverage. + +Read `.claude/rules/sim-testing.md` first: it defines the test layers, file naming, and the +mechanics (global mocks, `@sim/testing`, performance rules). + +## The three rules + +1. **Never write unit tests after you write code.** A test written to describe code that already + exists restates the implementation, passes on the first run, and proves nothing. If the change + needs proof, prove it end to end. +2. **Prefer E2E tests; use them to verify complex features.** Exercise the real boundary — real + Postgres/Redis (`*.integration.ts`), a running app over real HTTP, or the packaged desktop app + (Playwright). Every E2E run ends with a **verifiable, repeatable artifact**: a JSON report of + checks with pass/fail and durations, an HTTP status log, a trace, or a screenshot, written to a + path the caller controls (`_REPORT_PATH`) and uploaded by CI on failure. + `apps/sim/scripts/test-scim-e2e.ts` is the reference shape. +3. **If you must test a system in isolation, write the failure modes down first, then write the + code.** List every way the unit can fail (bad input, boundary, concurrency, partial failure, + permission denial, resource cap). Each listed mode becomes one test that fails before the code + exists. A mode you cannot list is not a test you should write. + +## Authoring gate + +Before adding any test, answer all four. A missing answer means do not add it. + +1. What observable behavior, invariant, or independent contract does it protect? +2. What credible regression makes it fail? +3. Why does existing coverage not already catch that failure? Type-check, `next build`, + `bun run check:audits`, and the integration/E2E suites are coverage too. Each contract has one + primary owner at the strongest boundary; another layer needs its own distinct risk. Prefer + extending an existing table-driven case over a near-duplicate test. +4. Does it need a production seam (export, flag, wrapper, injection hook) that no production + caller needs? If yes, test at the real boundary instead. + +Then check it against every junk pattern below. A match fails the gate unless the retention bar +names the contract it independently guards. A test that would break under behavior-preserving +refactoring asserts implementation, not behavior. + +**Regression tests** must fail on the pre-fix code for the intended reason. Revert each guard of +the fix separately and watch the test named for that guard go red, then restore. A regression +test that never demonstrably failed proves the mock, not the fix. One regression at the owner +boundary covers the bug; do not replay it at every layer it crosses. + +## Junk patterns + +- assertion-free or `toBeDefined()`-only tests; "renders without crashing"; +- restating declarations: block/tool/trigger/provider config (subBlock ids, params, outputs, URL + templates, header maps), constant tables, registries, enums, export lists — type-check and + `check:audits` own these; +- route/handler tests that mock every collaborator and assert `toHaveBeenCalledWith` on the mocks, + or re-assert a mock's canned return; +- mocks that implement the asserted behavior, or one mock standing in for different APIs; +- Zod contract tests proving a schema accepts a valid object or rejects an obviously invalid one; +- React tests of text, class names, aria presence, snapshots, "calls onClick"; +- hook tests asserting query keys or fetch URLs; store tests of trivial setters; +- tests of test infrastructure (mocks, factories, builders testing themselves); +- source-text or import greps (`readFileSync(src)` + `toContain`); +- expected values produced by the helper under test; +- duplicate invocations of the same contract, or provider-local replays of a shared helper; +- fixtures that supply the ordering, receipt, or callback the owner should produce; +- 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 + +Keep a test when it independently enforces one of: + +- **security** — authn/authz denial, tenant/workspace isolation, SSRF/URL validation, secret + redaction, encryption, signature verification, path traversal, injection, rate limits; +- **money and data integrity** — billing/usage math, metering, quotas, idempotency, migrations, + persistence semantics, concurrency/locking/leases, outbox, retries; +- **executor semantics** — DAG traversal, loops/parallels, conditions/routers, reference + resolution, streaming, pause/resume, run-from-block, cancellation; +- **real algorithms with edge cases** — chunkers, parsers, query builders, cron, diff/merge, + pagination, encoding, date math, ranking; +- **cross-process wire contracts** — realtime protocol, desktop bridge/IPC, CLI/SDK wire, provider + webhooks, MCP — that type-check cannot see; +- **a regression with a credible repeat**, shown red on the pre-fix code. + +Also keep call ordering when order is observable, and a source inspection when it is the cheapest +independent guard of a user-facing byte, key, or path. A retained test that fails on the baseline +is a possible product bug: reproduce it and fix the owner rather than deleting it. Static or slow +is not a deletion reason. + +## Audit mode + +Keep discovery read-only and report evidence before editing. Before judging a candidate, read the +complete test and its production owner, callers, sibling implementations, overlapping tests, CI +routing (CI discovers `*.integration.ts` by glob; `.github/workflows/*.yml` names a few scripts and +files by path), and relevant history +(`git log --format='%h %s' -5 -- `). + +Record for every deletion candidate: the test and location; the failure it can actually detect; +non-test callers of the seam it covers; the stronger remaining proof (or why none is needed); the +production or test-support code its deletion unlocks; and the focused validation command. + +**Edit shape.** One coherent owner-boundary batch per PR. When pruning inside a file, also delete +now-unused imports, mocks, fixtures, and helpers. Delete test-only exports and dead production +paths instead of preserving aliases (`rg -n '' --glob '!**/*.test.*'` must show no other +reference, including string and dynamic-import references; never delete route files, registry +entries, or generated files). Prefer net-negative production LOC. Do not add replacement tests +that restate the same implementation. + +## Campaign mode + +For a whole subsystem or the whole repo: + +1. Partition test files into lanes of ~150–350 files by owning directory, and list the protected + set (every `*.integration.ts`, `*.live.test.ts`, `__integration__/**`, `apps/desktop/e2e/**`, + and every path named in `.github/workflows/*.yml`). +2. Give each lane its own git worktree and branch (`git worktree add -b `, + then `bun install --frozen-lockfile` inside it). Lanes never share a checkout, never symlink + `node_modules`, and never use `git stash` — the stash is shared across worktrees. +3. Each lane commits once and writes a report: counts, categories removed with examples, notable + keeps and why, production seams removed with grep evidence, and the commands it ran. +4. Merge lane branches, then sweep orphaned shared test support (`packages/testing/**`, fixtures, + helpers) that no remaining test imports. + +## Validation + +Never edit source or tests while Vitest is running in the same checkout. + +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`. +5. Report `git diff --shortstat` with production and test changes counted separately. + +## Handoff + +Report the categories removed, production simplifications, retained false positives and why they +stay, the validation actually run, production vs test LOC, and named follow-ups. 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: `
`, `
`, `
`, `

') - expect(html).toContain('>Self-hosting

') - expect(html).not.toContain('Open source') - }) - - it('shifts every mark off the 64-grid origin so its stroke sits flush with the text', () => { - const html = renderToStaticMarkup() - const viewBoxes = [...html.matchAll(/viewBox="([^"]+)"/g)].map(([, value]) => value) - - expect(viewBoxes).toHaveLength(6) - for (const viewBox of viewBoxes) { - const [minX] = viewBox.split(' ').map(Number) - expect(minX).toBeGreaterThan(12) - } - }) - - it('carries no heading of its own', () => { - const html = renderToStaticMarkup() - - expect(html).not.toContain('How the workspace runs') - expect(html).not.toContain(' ({ - ...(await importOriginal()), - ChipLink: ({ - variant: _variant, - ...props - }: AnchorHTMLAttributes & { variant?: string }) => , - cn: (...values: Array) => values.filter(Boolean).join(' '), -})) - -import { Security } from '@/app/(landing)/components/security/security' - -describe('Security', () => { - it('renders the governance intro and white certification blocs', () => { - const html = renderToStaticMarkup() - - expect(html).toContain('id="security"') - expect(html).not.toContain('min-h-[100dvh]') - expect(html).toContain('>Central governance for enterprise AI') - expect(html).toContain( - 'Sim is one place to control who can build agents, what they can use, and how the workspace runs.' - ) - expect(html).toContain('>SOC 2 Type II') - expect(html).toContain('>ISO 27001') - expect(html).toContain('>GDPR') - expect(html).toContain('href="https://trust.sim.ai/"') - expect(html).toContain('bg-[var(--surface-2)]') - expect(html).toContain('aspect-[5/6]') - expect(html).toContain('rounded-[10px]') - expect(html).not.toContain('rounded-[12px]') - expect(html).not.toContain('How the workspace runs') - expect(html).not.toContain('Self-hosting') - expect(html).not.toContain('SOC2 compliant') - expect(html).not.toContain('HIPAA') - expect(html).not.toContain('CCPA') - }) -}) diff --git a/apps/sim/app/(landing)/components/shared/logs-run-graph/logs-run-graph.test.tsx b/apps/sim/app/(landing)/components/shared/logs-run-graph/logs-run-graph.test.tsx deleted file mode 100644 index bfd6eb520dd..00000000000 --- a/apps/sim/app/(landing)/components/shared/logs-run-graph/logs-run-graph.test.tsx +++ /dev/null @@ -1,115 +0,0 @@ -/** - * @vitest-environment jsdom - */ -import { act } from 'react' -import { createRoot, type Root } from 'react-dom/client' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { CoreFeatureCard } from '@/app/(landing)/components/features/components/core-feature-card' -import { LogsRunGraph } from '@/app/(landing)/components/shared/logs-run-graph' -import { RunTraceGraphic } from '@/app/(landing)/logs/components/feature-graphics' - -let root: Root -let host: HTMLDivElement - -beforeEach(() => { - vi.useFakeTimers() - ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true - host = document.createElement('div') - document.body.append(host) - root = createRoot(host) -}) - -afterEach(() => { - act(() => root.unmount()) - host.remove() - vi.restoreAllMocks() - vi.useRealTimers() -}) - -function mount() { - act(() => root.render()) - const graph = host.querySelector('button') - if (!graph) throw new Error('Graph not rendered') - return graph -} - -function pointer(element: Element, type: string, pointerType = 'touch', x = 40, y = 300) { - const event = new MouseEvent(type, { bubbles: true, clientX: x, clientY: y }) - Object.defineProperty(event, 'pointerType', { value: pointerType }) - act(() => element.dispatchEvent(event)) -} - -describe('LogsRunGraph', () => { - it('keeps tapped details visible and dismisses them on outside interaction', () => { - const graph = mount() - const bar = graph.querySelectorAll('[data-run-count]')[1] - pointer(bar, 'pointerdown') - pointer(bar, 'pointerup') - pointer(graph, 'pointerout') - expect(document.querySelector('[role="tooltip"]')?.textContent).toContain('5 succeeded') - expect(graph.getAttribute('aria-label')).toContain('23–22 hours ago') - pointer(document.body, 'pointerdown') - expect(document.querySelector('[role="tooltip"]')).toBeNull() - }) - - it.each(['pointercancel', 'swipe'])('does not open details after a %s gesture', (gesture) => { - const graph = mount() - pointer(graph, 'pointerdown') - if (gesture === 'pointercancel') pointer(graph, 'pointercancel') - pointer(graph, 'pointerup', 'touch', gesture === 'swipe' ? 120 : 40) - expect(document.querySelector('[role="tooltip"]')).toBeNull() - }) - - it.each(['scroll', 'resize'])( - 'dismisses details when the viewport changes through %s', - (event) => { - const graph = mount() - pointer(graph, 'pointerdown') - pointer(graph, 'pointerup') - expect(document.querySelector('[role="tooltip"]')).not.toBeNull() - act(() => (event === 'resize' ? window : host).dispatchEvent(new Event(event))) - expect(document.querySelector('[role="tooltip"]')).toBeNull() - } - ) - - it('supports keyboard exploration and Escape dismissal', () => { - const graph = mount() - act(() => graph.focus()) - act(() => graph.dispatchEvent(new KeyboardEvent('keydown', { key: 'End', bubbles: true }))) - expect(graph.getAttribute('aria-label')).toContain('Last hour: 7 succeeded') - act(() => - graph.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowLeft', bubbles: true })) - ) - expect(graph.getAttribute('aria-label')).toContain('2–1 hours ago: 6 succeeded') - act(() => graph.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))) - expect(document.querySelector('[role="tooltip"]')).toBeNull() - }) - - it('keeps first-hour details inside a narrow viewport', () => { - const graph = mount() - vi.spyOn(window, 'innerWidth', 'get').mockReturnValue(320) - pointer(graph, 'pointerdown') - pointer(graph, 'pointerup') - const tooltip = document.querySelector('[role="tooltip"]') - expect(tooltip?.style.translate).toContain('calc(192px - 100%)') - expect(tooltip?.className).toContain('w-[176px]') - }) - - it('keeps the interactive chart outside the route link and decorative wrappers', () => { - act(() => - root.render( - } - interactiveVisual - /> - ) - ) - const graph = host.querySelector('[data-run-overview-graph]') - expect(graph).not.toBeNull() - expect(graph?.closest('a, [aria-hidden="true"]')).toBeNull() - expect(host.querySelector('a')?.getAttribute('href')).toBe('/logs') - }) -}) diff --git a/apps/sim/app/(landing)/components/shared/product-window/product-window.test.tsx b/apps/sim/app/(landing)/components/shared/product-window/product-window.test.tsx deleted file mode 100644 index 18264f65db0..00000000000 --- a/apps/sim/app/(landing)/components/shared/product-window/product-window.test.tsx +++ /dev/null @@ -1,31 +0,0 @@ -/** - * @vitest-environment jsdom - */ -import { renderToStaticMarkup } from 'react-dom/server' -import { describe, expect, it, vi } from 'vitest' - -vi.mock('@sim/emcn', async (importOriginal) => ({ - ...(await importOriginal()), - cn: (...values: Array) => values.filter(Boolean).join(' '), -})) -vi.mock('@/app/(landing)/components/shared/product-preview', () => ({ - ProductPreview: () => ( - <> -