From a06a4d99ef845347413fde7f1afa52890de64bd7 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Thu, 23 Jul 2026 13:35:03 -0700 Subject: [PATCH 1/2] docs(e2e): add settings maintenance guidance Co-authored-by: Cursor --- .claude/rules/sim-settings-e2e.md | 17 ++++ .claude/rules/sim-settings-pages.md | 68 ++++++++----- .claude/skills/add-settings-page/SKILL.md | 80 ++++++++++++---- .cursor/rules/sim-settings-e2e.mdc | 15 +++ apps/sim/AGENTS.md | 1 + apps/sim/e2e/MAINTENANCE.md | 112 ++++++++++++++++++++++ apps/sim/e2e/README.md | 3 + 7 files changed, 253 insertions(+), 43 deletions(-) create mode 100644 .claude/rules/sim-settings-e2e.md create mode 100644 .cursor/rules/sim-settings-e2e.mdc create mode 100644 apps/sim/e2e/MAINTENANCE.md diff --git a/.claude/rules/sim-settings-e2e.md b/.claude/rules/sim-settings-e2e.md new file mode 100644 index 00000000000..55925020834 --- /dev/null +++ b/.claude/rules/sim-settings-e2e.md @@ -0,0 +1,17 @@ +--- +paths: + - "apps/sim/**" + - "packages/platform-authz/src/workspace.ts" + - "packages/platform-authz/src/predicates.ts" +--- + +# Settings E2E Maintenance + +When a change can affect observable settings routes, copy, visibility, +authorization, entitlements, mutations, or persisted workflows, read +`apps/sim/e2e/MAINTENANCE.md` before finishing and assess every applicable +literal Playwright contract. + +Keep acceptance expectations independent of production navigation and +authorization implementations. Behavior-preserving refactors need focused +verification, not artificial contract edits. diff --git a/.claude/rules/sim-settings-pages.md b/.claude/rules/sim-settings-pages.md index 154b45a2599..a160ed2d19c 100644 --- a/.claude/rules/sim-settings-pages.md +++ b/.claude/rules/sim-settings-pages.md @@ -1,20 +1,29 @@ --- paths: + - "apps/sim/app/account/settings/**" + - "apps/sim/app/organization/*/settings/**" - "apps/sim/app/workspace/*/settings/**" + - "apps/sim/components/settings/**" - "apps/sim/ee/**/components/**" --- # Settings Pages -The Next.js `settings/[section]/layout.tsx` owns all settings page chrome via -`SettingsHeaderShell` — a fixed header bar (a left back chip + right-aligned -action chips), a scroll region, and a centered `max-w-[48rem]` content column led -by a **title + description from navigation metadata**. The chrome stays mounted -across section navigation (it never re-renders or re-lays-out). Each section -renders through the **`SettingsPanel`** registrar -(`@/app/workspace/[workspaceId]/settings/components/settings-panel`), which feeds -the shell its header data and renders only the section body. Sections supply -**data**, never chrome. +All three settings planes share `SettingsHeaderShell` and the canonical +`SettingsPanel` implementation in `@/components/settings/settings-panel`, but +their chrome is mounted at different boundaries: + +- Account and organization `settings/layout.tsx` mount + `StandaloneSettingsShell`, which owns their sidebar, header shell, section + provider, and unload guard. +- Workspace `settings/[section]/layout.tsx` mounts the persistent header shell; + the outer workspace `settings/layout.tsx` owns its unload guard. + +The active shell owns the fixed header bar (a left back chip + right-aligned +action chips), scroll region, and centered `max-w-[48rem]` content column led by +a **title + description from the shared settings registry**. Each section +renders only its body through `SettingsPanel`; sections supply **data**, never +chrome. Do NOT hand-roll any of these in a settings page — they are owned by the layout shell (fed through `SettingsPanel`): @@ -29,6 +38,8 @@ shell (fed through `SettingsPanel`): ## Canonical page shape ```tsx +// Established section-component import; this compatibility barrel re-exports +// the canonical @/components/settings/settings-panel implementation. import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' return ( @@ -80,21 +91,29 @@ return ( - `scrollContainerRef?: React.Ref` — forwards a ref to the scroll region (e.g. programmatic scroll-to-bottom). -## Title + description live in navigation metadata +## Title + description live in the shared settings registry -`apps/sim/app/workspace/[workspaceId]/settings/navigation.ts` is the single source -of truth. Every `NavigationItem` carries a one-line `description`; `SettingsPanel` -resolves both via `getSettingsSectionMeta(section)` and the -`SettingsSectionProvider` the settings shell wraps around the active section. +`apps/sim/components/settings/navigation.ts` is the single source of truth. +Each `SettingsSectionRegistryEntry` owns a `unified` projection with the default +description and gating metadata plus optional account, organization, and +workspace plane projections. Account and organization shells provide `plane` +and `section`, so the shared `SettingsPanel` resolves metadata with +`getSettingsSectionMeta(plane, section)`. The workspace adapter at +`app/workspace/[workspaceId]/settings/navigation.ts` resolves the mandatory +unified projection with its one-argument `getSettingsSectionMeta(section)` and +the workspace renderer passes that metadata to `SettingsSectionProvider`. Adding a new settings page: -1. Add the `SettingsSection` id + a `NavigationItem` (with `label` **and** - `description`) in `navigation.ts`. Keep descriptions verb-first, one line, - ~40–55 chars, in the product voice (see `.claude/rules/constitution.md`). -2. Render the component inside the shell's `effectiveSection` switch in - `settings/[section]/settings.tsx`. -3. Build the component body inside `` — no shell, no title block. +Follow the procedure in `.claude/skills/add-settings-page/SKILL.md`. The +non-negotiable architecture invariants are: + +- Every registry entry has a mandatory unified projection, workspace renderer, + and server-side direct-route outcome. +- Every optional account, organization, or workspace plane projection has its + corresponding renderer and access/feature gate. +- The component body uses `SettingsPanel`; associated unit tests and every + applicable literal browser contract change in the same PR. ## Text-scale tokens (no literal pixel sizes) @@ -187,8 +206,9 @@ changes" modal: (from `@/app/workspace/[workspaceId]/components/credential-detail`). The in-view header **Discard** chip (via `SaveDiscardActions onDiscard`) is a *reset to original* — distinct from the back-confirm's discard, which leaves. -- **`useSettingsBeforeUnload`** is mounted **once** in the settings shell - (`settings/[section]/settings.tsx`) — never add a per-page `beforeunload`. +- **`useSettingsBeforeUnload`** is mounted once per active shell boundary: + workspace `settings/layout.tsx` and `StandaloneSettingsShell` for account or + organization. Never add a per-page `beforeunload`. - **Dirty *computation* stays local** (shapes differ: field-compare vs normalize+stringify) — only how dirty is *consumed* is shared. Derive it (a `const`/`useMemo`), never store it in `useState`. @@ -218,9 +238,9 @@ exception — it lives outside `[section]` and keeps its own `CredentialDetailLa A settings page is design-system-clean when: - [ ] Its main return is a `` (or `<>……` with modal siblings) — no hand-rolled shell/header/scroll/column. -- [ ] It renders **no** hand-rolled `

`/description title block — the title comes from nav metadata. +- [ ] It renders **no** hand-rolled `

`/description title block — the title comes from the shared settings registry. - [ ] Header chips are in `actions`; a standalone search is in the `search` prop. -- [ ] Its `NavigationItem` has an accurate, consistent-length `description`. +- [ ] Its `SettingsSectionRegistryEntry` has an accurate, consistent-length `unified.description` and only the plane projections it supports. - [ ] Detail sub-views and entitlement/loading gates keep their own chrome (intentional). - [ ] If it has editable state: Save/Discard go through `SaveDiscardActions`, dirty is wired via `useSettingsUnsavedGuard` (called before any early-return gate), and there is **no** hand-rolled Save button / `beforeunload` / "Unsaved changes" modal. - [ ] No business logic, handlers, or conditional rendering changed by the migration. diff --git a/.claude/skills/add-settings-page/SKILL.md b/.claude/skills/add-settings-page/SKILL.md index 69173332ad4..408ce9c8389 100644 --- a/.claude/skills/add-settings-page/SKILL.md +++ b/.claude/skills/add-settings-page/SKILL.md @@ -5,31 +5,62 @@ description: Add a new Sim settings page, or audit existing settings pages for d # Settings Page (add / audit) -Sim settings pages all render through the shared **`SettingsPanel`** primitive, -which owns the page chrome and renders a nav-driven title + description. The full -convention lives in `.claude/rules/sim-settings-pages.md` — read it first; this -skill is the procedure. +Sim settings pages render their bodies through the shared **`SettingsPanel`** +registrar, which publishes header metadata and actions to the active +`SettingsHeaderShell`. The shell owns and renders the page chrome, including the +registry-driven title and description. The full convention lives in +`.claude/rules/sim-settings-pages.md` — read it first; this skill is the +procedure. Key paths: -- Layout primitive: `apps/sim/app/workspace/[workspaceId]/settings/components/settings-panel/settings-panel.tsx` -- Nav metadata (titles + descriptions): `apps/sim/app/workspace/[workspaceId]/settings/navigation.ts` -- Section switch + provider: `apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx` +- Panel registrar implementation: `apps/sim/components/settings/settings-panel.tsx` +- Established section-component import: `@/app/workspace/[workspaceId]/settings/components/settings-panel` (compatibility barrel) +- Section types + registry: `apps/sim/components/settings/navigation.ts` +- Account renderer: `apps/sim/components/settings/account-settings-renderer.tsx` +- Account route gate: `apps/sim/app/account/settings/[section]/page.tsx` +- Organization renderer: `apps/sim/components/settings/organization-settings-renderer.tsx` +- Organization route gate: `apps/sim/app/organization/[organizationId]/settings/[section]/page.tsx` +- Workspace metadata adapter: `apps/sim/app/workspace/[workspaceId]/settings/navigation.ts` +- Workspace renderer + provider: `apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx` +- Workspace route gate: `apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx` - Pages: `apps/sim/app/workspace/[workspaceId]/settings/components//.tsx` and EE pages under `apps/sim/ee//components/` +- Browser acceptance maintenance: `apps/sim/e2e/MAINTENANCE.md` ## Mode A — Add a new settings page -1. **Navigation.** In `navigation.ts`: add the id to the `SettingsSection` union, - then a `NavigationItem` with `label` AND a one-line `description` (verb-first, - ~40–55 chars, product voice per `.claude/rules/constitution.md`). Place it in - the right `section` group and set any gating flags (`requiresHosted`, - `requiresEnterprise`, etc.). -2. **Wire the switch.** Add the component to the `effectiveSection` render switch - in `settings/[section]/settings.tsx` (lazy `dynamic(...)` like its siblings). -3. **Build the body inside `SettingsPanel`.** Never hand-roll the shell, header +1. **Registry.** In `apps/sim/components/settings/navigation.ts`, add the id to + `UnifiedSettingsSection` and add a `SettingsSectionRegistryEntry` to + `SETTINGS_SECTION_REGISTRY`. Its mandatory `unified` projection owns the + default description and workspace gating flags (`requiresHosted`, + `requiresEnterprise`, etc.). For each plane-specific projection the page + needs, also add the id to `AccountSettingsSection`, + `OrganizationSettingsSection`, or `WorkspaceSettingsSection` and add that + optional projection. Use plane-specific copy only when its scope genuinely + differs. Keep descriptions verb-first, one line, ~40–55 chars, in the product + voice (see `.claude/rules/constitution.md`). +2. **Always wire the unified workspace surface.** Every registry entry + participates in unified workspace navigation before its gates are applied, + whether or not it has `planes.workspace`. Render the `unified.id` in the + workspace `settings/[section]/settings.tsx` switch so an allowed route cannot + resolve to a blank page. +3. **Wire optional standalone planes.** If the entry declares `planes.account` + or `planes.organization`, add the component to + `account-settings-renderer.tsx` or `organization-settings-renderer.tsx`. + Never declare a standalone projection whose renderer cannot render it. +4. **Preserve every route gate.** In the workspace + `settings/[section]/page.tsx`, classify the unified section in + `WORKSPACE_SECTION_MAP` or `ORGANIZATION_SECTION_MAP` when it belongs to + either access plane; otherwise add an explicit direct gate when needed or + verify that host-context membership is the intended boundary. Enforce the + section's permission, deployment, plan, and entitlement outcome. For + standalone account or organization projections, also update their + `[section]/page.tsx` gate and the shared organization access/feature helpers + when applicable. Sidebar gating never replaces server authorization. +5. **Build the body inside `SettingsPanel`.** Never hand-roll the shell, header bar, scroll region, content column, or title block. Put header buttons in `actions`, a standalone search in `search={{ value, onChange, placeholder }}`, and the page content as `children`. Modals go beside the panel inside a `<>`. -4. **If the page has editable state**, wire the shared save/discard stack — put +6. **If the page has editable state**, wire the shared save/discard stack — put `SaveDiscardActions` (dirty-gated Discard+Save chips) in `actions`, and call `useSettingsUnsavedGuard({ isDirty })` **before any early-return gate**. Detail sub-views additionally route the back chip through @@ -37,7 +68,16 @@ Key paths: hand-roll a Save button, a `beforeunload`, or an "Unsaved changes" modal — they're centralized. See the "Save / Discard + unsaved-changes guard" section in `.claude/rules/sim-settings-pages.md`. -5. **Verify:** `cd apps/sim && bunx tsc --noEmit`; `bunx biome check --write `. +7. **Update tests and browser contracts.** Update the shared navigation, + workspace route/navigation, and access unit tests affected by the new + projection or gate. Then follow `apps/sim/e2e/MAINTENANCE.md`: intended + observable changes require paired literal Playwright contracts, while + behavior-preserving refactors require focused verification without + expectation churn. +8. **Verify:** From `apps/sim`, run `bunx tsc --noEmit` and + `bunx biome check --write ` on changed source files. Then run the + affected unit tests and the focused orchestrated E2E project from + `e2e/README.md`. ## Mode B — Audit existing settings pages @@ -54,8 +94,10 @@ For each page component, confirm the checklist in `.claude/rules/sim-settings-pa `.claude/rules/sim-settings-pages.md` for the token map and the row title/subtitle pairing convention): `git grep -n "text-\[1[0-8]px\]" -- 'apps/sim/**/settings/' 'apps/sim/ee/'` -4. Confirm each page imports `SettingsPanel` and that its `NavigationItem` has an - accurate `description` of consistent length with its peers. +4. Confirm each page imports `SettingsPanel` and that its + `SettingsSectionRegistryEntry` has an accurate `unified.description` of + consistent length with its peers, plus only the plane projections it + supports. - Editable pages: confirm Save/Discard go through `SaveDiscardActions` and dirty is wired via `useSettingsUnsavedGuard` (called before early-return gates) — flag any hand-rolled Save button, `beforeunload`, or unsaved modal. diff --git a/.cursor/rules/sim-settings-e2e.mdc b/.cursor/rules/sim-settings-e2e.mdc new file mode 100644 index 00000000000..7f0a26df953 --- /dev/null +++ b/.cursor/rules/sim-settings-e2e.mdc @@ -0,0 +1,15 @@ +--- +description: Keep settings browser acceptance contracts aligned with observable behavior +globs: ["apps/sim/**", "packages/platform-authz/src/workspace.ts", "packages/platform-authz/src/predicates.ts"] +--- + +# Settings E2E Maintenance + +When a change can affect observable settings routes, copy, visibility, +authorization, entitlements, mutations, or persisted workflows, read +`apps/sim/e2e/MAINTENANCE.md` before finishing and assess every applicable +literal Playwright contract. + +Keep acceptance expectations independent of production navigation and +authorization implementations. Behavior-preserving refactors need focused +verification, not artificial contract edits. diff --git a/apps/sim/AGENTS.md b/apps/sim/AGENTS.md index 6c52c2df02d..4f5a6214ab4 100644 --- a/apps/sim/AGENTS.md +++ b/apps/sim/AGENTS.md @@ -221,6 +221,7 @@ export function useEntityList(workspaceId?: string) { - Use `vi.hoisted()` + `vi.mock()` + static imports; do not use `vi.resetModules()` + `vi.doMock()` + dynamic imports except for true module-scope singletons. - Do not use `vi.importActual()`. - Prefer mocks and factories from `@sim/testing`. +- Before finishing a change that can affect observable settings routes, copy, visibility, authorization, entitlements, mutations, or persisted workflows, follow [`e2e/MAINTENANCE.md`](e2e/MAINTENANCE.md) and assess every applicable literal Playwright contract. Behavior-preserving refactors need focused verification, not artificial expectation edits. ## Utils Rules diff --git a/apps/sim/e2e/MAINTENANCE.md b/apps/sim/e2e/MAINTENANCE.md new file mode 100644 index 00000000000..42d3e6226db --- /dev/null +++ b/apps/sim/e2e/MAINTENANCE.md @@ -0,0 +1,112 @@ +# Settings E2E maintenance + +Use this guide when a product change can alter observable behavior on an account, +organization, or workspace settings surface. The browser datasets are literal +acceptance contracts, intentionally independent of production navigation and +authorization implementations. + +## Decision rule + +- An intended observable contract change requires the product code and the + matching literal E2E contract or workflow proof to change in the same PR. +- An implementation-only change that preserves the observable contract requires + focused verification, not artificial expectation churn. +- The categories below are non-exclusive. For example, a credential access + change can require both authorization and credential-workflow coverage. +- Never import from or derive expectations from `SETTINGS_SECTION_REGISTRY`, + `buildUnifiedSettingsNavigation()`, or another production implementation. + +Observable behavior includes paths, redirects, labels, headings, descriptions, +sidebar visibility, direct-route outcomes, plan and role gates, mutation +availability, accessible control names, unsaved-change behavior, and persisted +workflow results. Purely visual changes do not require a new browser case unless +they change one of those contracts. + +## Change map + +### Navigation and routing + +Update `e2e/settings/navigation/contracts.ts` and its matching specs when changing: + +- canonical sections, paths, labels, headings, descriptions, semantic readiness, + ordering, or active-sidebar behavior; +- default routes, aliases, legacy redirects, unknown sections, unavailable + states, or direct-entry outcomes; +- the complete visible set or important hidden items for a representative + persona. + +Update `e2e/settings/navigation/history.spec.ts` directly when changing browser +Back, app Back, direct-entry fallback, or return destinations. Change the +navigation contract dataset only when one of its own section, route, or +visibility expectations also changes. + +Keep the relevant production-level coverage aligned, especially: + +- `components/settings/navigation.test.ts` for the shared registry, projections, + aliases, ordering, and access metadata; +- `app/workspace/[workspaceId]/settings/navigation.test.ts` for the workspace + adapter and route-facing behavior. + +### Authorization and entitlements + +Update `e2e/settings/authorization/contracts.ts` and its matching specs when +changing: + +- direct URL access by role, permission, plane, plan, or entitlement; +- sidebar availability that is also an authorization promise; +- enabled, disabled, hidden, or upgrade-gated mutation controls. + +Update `e2e/settings/authorization/unsaved-changes.spec.ts` directly when +changing shared unsaved-change behavior. Change the authorization contract +dataset only when one of its own access or mutation-control expectations also +changes. + +Every mandatory gate needs representative allowed and denied proof. Keep +unit-level access tests, including `lib/organizations/settings-access.test.ts` +and affected consumer tests for `@sim/platform-authz` behavior, aligned with the +browser contract. + +### Credentials + +Update the affected Secrets or API key spec when changing personal/workspace +ownership, creation, editing, discard/save, deletion/revocation, binding policy, +or credential permissions. Update `e2e/settings/credentials/contracts.ts` only +when the reused authorization proof IDs or boundaries change; lifecycle +expectations live directly in the specs. Sensitive-value handling and diagnostic +suppression must remain intact. + +### Stateful workflows + +Update `e2e/settings/workflows/contracts.ts` and the affected workflow spec when +changing: + +- workspace or organization invitations, roles, grants, or removals; +- permission-group creation, assignment, enforcement, or restoration; +- SSO provider creation, editing, verification instructions, or deletion; +- data-retention defaults, overrides, or exact restoration; +- MCP domain validation, connection, discovery, editing, or deletion. + +Register cleanup before mutation and preserve exact baseline restoration. + +### Personas and scenarios + +Update `e2e/settings/personas.ts`, the relevant fixture/scenario definition, and +persona integrity or isolation proof when changing seeded roles, memberships, +plans, grants, subscriptions, or resource relationships. Do not weaken trusted +invariants to make an impossible persona pass. + +## Verification + +Run the affected unit tests and the focused orchestrated Playwright project. The +commands and project boundaries are maintained in: + +- [Settings navigation contracts](README.md#settings-navigation-contracts) +- [Settings authorization contracts](README.md#settings-authorization-contracts) +- [Settings credential workflows](README.md#settings-credential-workflows) +- [People and access-control workflows](README.md#people-and-access-control-workflows) +- [Enterprise integration workflows](README.md#enterprise-integration-workflows) + +Run browser tests only through `bun run test:e2e`; the operational and safety +requirements live in [README.md](README.md). The acceptance crosswalk and +milestone boundaries live in [STABILIZATION.md](STABILIZATION.md). Required CI +remains the authoritative complete-suite gate. diff --git a/apps/sim/e2e/README.md b/apps/sim/e2e/README.md index b3b12208892..2d7e0a7b70c 100644 --- a/apps/sim/e2e/README.md +++ b/apps/sim/e2e/README.md @@ -4,6 +4,9 @@ This directory contains the reusable full-stack Playwright harness. It runs the production Next.js app, realtime, deterministic external fakes, and a migrated per-run pgvector database. +For the cross-contract decision guide used when settings behavior changes, see +[`MAINTENANCE.md`](MAINTENANCE.md). + ## One-time setup 0. Install Node 22 and Bun. Playwright workers require Node 22; set From baed822cc816bd15369da322995ae47f5de77850 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Thu, 23 Jul 2026 13:59:35 -0700 Subject: [PATCH 2/2] docs(e2e): add settings test authoring skill Co-authored-by: Cursor --- .agents/skills/add-settings-e2e-test/SKILL.md | 181 ++++++++++++++++++ .claude/commands/add-settings-e2e-test.md | 180 +++++++++++++++++ .cursor/commands/add-settings-e2e-test.md | 175 +++++++++++++++++ apps/sim/e2e/MAINTENANCE.md | 4 + 4 files changed, 540 insertions(+) create mode 100644 .agents/skills/add-settings-e2e-test/SKILL.md create mode 100644 .claude/commands/add-settings-e2e-test.md create mode 100644 .cursor/commands/add-settings-e2e-test.md diff --git a/.agents/skills/add-settings-e2e-test/SKILL.md b/.agents/skills/add-settings-e2e-test/SKILL.md new file mode 100644 index 00000000000..81783bbbd7c --- /dev/null +++ b/.agents/skills/add-settings-e2e-test/SKILL.md @@ -0,0 +1,181 @@ +--- +name: add-settings-e2e-test +description: Add or update durable Playwright coverage for Sim settings using the correct literal contract or owning spec, existing personas, guarded orchestration, semantic assertions, cleanup, and secret-safe diagnostics. Use when asked to add a settings browser test, cover a settings regression, or update settings E2E acceptance behavior. +argument-hint: +--- + +# Add Settings E2E Test + +Add the smallest durable browser proof for the requested observable behavior. +Do not begin by copying a nearby test: first identify which acceptance dataset +or spec owns the behavior. + +## Read first + +1. `apps/sim/e2e/MAINTENANCE.md` — change type to contract/spec ownership. +2. `apps/sim/e2e/README.md` — orchestrator, project boundaries, focused commands, + diagnostics, and CI policy. +3. `apps/sim/e2e/STABILIZATION.md` — non-waivable acceptance and security + boundaries. + +Those files are canonical. This skill is the authoring procedure, not a second +copy of their inventories or commands. If this procedure ever conflicts with a +canonical document, the canonical document wins. + +## Procedure + +### 1. Define the observable contract + +Write down: + +- the user persona and resource plane (account, organization, or workspace); +- the action or direct URL; +- the exact observable result: path, semantic readiness, visible/hidden item, + access outcome, enabled/disabled control, warning, or persisted state; +- whether product behavior is intentionally changing or an implementation + change must preserve the existing contract. + +Intended behavior changes update product code and literal acceptance +expectations together. Behavior-preserving fixes add or strengthen proof without +rewriting unrelated expectations. + +### 2. Choose the owner before writing code + +- **Canonical section, route, copy, readiness, or persona visibility:** update + `apps/sim/e2e/settings/navigation/contracts.ts`; its specs generate cases from + the literal dataset, including unauthenticated redirect cases. +- **Browser Back, app Back, direct-entry fallback, or return destination:** edit + `apps/sim/e2e/settings/navigation/history.spec.ts` directly. +- **Direct access, plan/role/entitlement gate, or mutation availability:** update + `apps/sim/e2e/settings/authorization/contracts.ts`; its access and mutation + specs generate cases from that dataset. + - If an existing navigation case already owns the sidebar proof, add or reuse + an `existingNavigationProofs` entry and set + `sidebar.existingProofId`. Do not execute the same sidebar assertion again + in the authorization case. +- **Unsaved-change behavior:** edit + `apps/sim/e2e/settings/authorization/unsaved-changes.spec.ts` directly. +- **Secrets or API key lifecycle:** edit the owning credential spec directly. + Change `apps/sim/e2e/settings/credentials/contracts.ts` only when reused + authorization proof IDs or boundaries change. +- **People, access control, SSO, data retention, or MCP lifecycle:** edit the + owning workflow spec. Change `apps/sim/e2e/settings/workflows/contracts.ts` + when stable lifecycle or cross-contract proof references change. +- **Persona or seeded relationship:** update + `apps/sim/e2e/settings/personas.ts` and the relevant + scenario/factory/integrity proof. + +The categories are non-exclusive. Prefer extending an existing literal row or +owning workflow over adding a duplicate standalone test. + +### 3. Use the owning fixture wrapper + +- Browser-touching settings specs must not import runtime `test` or `expect` + from `@playwright/test`. Type-only imports such as `Page` or `Response` are + allowed. +- Navigation, authorization, persona-contract, and persona-isolation browser + specs use `apps/sim/e2e/fixtures/persona-test.ts`. +- Credential specs use the local `credential-test.ts`; workflow specs use the + local `workflow-test.ts`; authenticated or unauthenticated smoke and + browser-touching harness-level specs use + `apps/sim/e2e/fixtures/browser-test.ts`. +- Pure dataset-only contract-integrity specs and non-browser foundation policy + specs may use the Playwright test runner directly. If they begin creating a + browser, context, or page, move them to the owning wrapper first. +- Persona-based tests must create contexts through `contextForPersona` or an + owning helper that calls it. The inherited `page` and `context` fixtures on + `persona-test` are not persona-authenticated or network-guarded. +- `browser-test` guards its default context. Every manual + `browser.newContext()` must install `installBrowserNetworkGuard()` immediately, + then close the context and assert the guard even when the test fails; follow + the aggregate-cleanup pattern in authenticated smoke coverage. Do not use + `browser.newPage()`: it creates an implicit unguarded context; create and guard + an explicit context, then call `context.newPage()`. +- Reuse the owning directory's helpers. Credential tests use their local + `newPersonaPage` helper so the page is registered for failure sanitization. + Wrapper selection alone does not install every context, cleanup, attachment, + redaction, or artifact safeguard; bypassing the guarded fixture/helper path is + a test-safety bug. + +### 4. Reuse the existing world + +- Reuse an existing persona and seeded resource when it expresses the required + role, plan, entitlement, and ownership boundary. +- Add a persona or scenario edge only when no existing driver can prove the + contract without changing its meaning. +- Use run-namespaced factories and unique resources. Do not introduce shared + mutable fixture state. +- Keep sensitive fixture values browser-resident through the existing helpers. + The Playwright process must not receive database credentials, admin keys, + persona passwords, or plaintext values that the owning spec keeps in-page. + +### 5. Assert semantics, not implementation details + +- Use accessible roles, labels, names, and visible text. +- Assert exact paths and user-facing contract copy when those are the behavior + under test. +- Wait on semantic readiness, relevant same-origin responses, or explicit + authorization state—not arbitrary sleeps or CSS classes. +- For access controls, prove direct URL behavior as well as sidebar visibility. +- Do not import production navigation or authorization implementations to + generate expected values. + +### 6. Make mutations reversible + +- Register cleanup before the first mutation. +- Create uniquely named resources and restore the exact captured baseline. +- Use the production UI and same-origin APIs for behavior under test; use a + trusted database probe only where the existing suite defines one. +- Keep cleanup LIFO and safe after partial failure. Never weaken fixture + invariants merely to make a test pass. + +### 7. Respect external and diagnostic boundaries + +- Use the existing strict loopback Stripe and MCP fakes, the mail mock-success + path with provider credentials absent, and the other reviewed test + boundaries. An unexpected external request must fail rather than silently + egress. +- Credentials, tokens, certificates, verification values, storage state, and + sensitive response bodies may be used only through existing reviewed paths; + never log, attach, or retain them in diagnostics. +- The SSO workflow's public-certificate browser input is one reviewed example; + preserve its input clearing and diagnostic suppression. +- Preserve the owning spec's trace, screenshot, video, and attachment policy. + New suppression requires a security reason and corresponding safety coverage. + +### 8. Keep unit and integrity proof aligned + +- Update affected production unit tests for navigation, route gates, + authorization, billing, or business rules. +- Update the owning `contract-integrity.spec.ts` when adding IDs, references, or + a new contract axis. +- Preserve stable proof IDs; do not rerun a browser case merely because another + contract can reference its existing proof. + +### 9. Verify through the orchestrator + +- Run the affected unit and contract-integrity tests. +- Run the focused canonical project and path documented in + `apps/sim/e2e/README.md`. +- Invoke only `bun run test:e2e`; never invoke raw `playwright test`. +- Use `--reuse-build` and a single explicit project with `--no-deps` for local + iteration when the README permits it. +- Follow the retry policy in the canonical README and Playwright config. It is + currently zero; do not enable retries or change worker policy in a feature + test. Do not use `test.only`, unexplained skips, arbitrary browser sleeps, or + test-local environment bypasses. +- After focused proof passes, rely on the required complete CI suite. Run a + repeated stability gate only when the change's scope or acceptance plan calls + for one. + +## Completion checklist + +- [ ] Every observable facet has one clear owner, and all applicable owners were + updated. +- [ ] Allowed and denied outcomes exist where the feature is gated. +- [ ] Locators and readiness assertions are semantic and accessible. +- [ ] Mutations register cleanup first and restore the exact baseline. +- [ ] No secret or external-egress boundary was weakened. +- [ ] Related unit and contract-integrity tests are aligned and pass. +- [ ] Focused execution used the guarded orchestrator and complied with the + canonical retry policy. diff --git a/.claude/commands/add-settings-e2e-test.md b/.claude/commands/add-settings-e2e-test.md new file mode 100644 index 00000000000..84cf2d34e25 --- /dev/null +++ b/.claude/commands/add-settings-e2e-test.md @@ -0,0 +1,180 @@ +--- +description: Add or update durable Playwright coverage for Sim settings using the correct literal contract or owning spec, existing personas, guarded orchestration, semantic assertions, cleanup, and secret-safe diagnostics. Use when asked to add a settings browser test, cover a settings regression, or update settings E2E acceptance behavior. +argument-hint: +--- + +# Add Settings E2E Test + +Add the smallest durable browser proof for the requested observable behavior. +Do not begin by copying a nearby test: first identify which acceptance dataset +or spec owns the behavior. + +## Read first + +1. `apps/sim/e2e/MAINTENANCE.md` — change type to contract/spec ownership. +2. `apps/sim/e2e/README.md` — orchestrator, project boundaries, focused commands, + diagnostics, and CI policy. +3. `apps/sim/e2e/STABILIZATION.md` — non-waivable acceptance and security + boundaries. + +Those files are canonical. This skill is the authoring procedure, not a second +copy of their inventories or commands. If this procedure ever conflicts with a +canonical document, the canonical document wins. + +## Procedure + +### 1. Define the observable contract + +Write down: + +- the user persona and resource plane (account, organization, or workspace); +- the action or direct URL; +- the exact observable result: path, semantic readiness, visible/hidden item, + access outcome, enabled/disabled control, warning, or persisted state; +- whether product behavior is intentionally changing or an implementation + change must preserve the existing contract. + +Intended behavior changes update product code and literal acceptance +expectations together. Behavior-preserving fixes add or strengthen proof without +rewriting unrelated expectations. + +### 2. Choose the owner before writing code + +- **Canonical section, route, copy, readiness, or persona visibility:** update + `apps/sim/e2e/settings/navigation/contracts.ts`; its specs generate cases from + the literal dataset, including unauthenticated redirect cases. +- **Browser Back, app Back, direct-entry fallback, or return destination:** edit + `apps/sim/e2e/settings/navigation/history.spec.ts` directly. +- **Direct access, plan/role/entitlement gate, or mutation availability:** update + `apps/sim/e2e/settings/authorization/contracts.ts`; its access and mutation + specs generate cases from that dataset. + - If an existing navigation case already owns the sidebar proof, add or reuse + an `existingNavigationProofs` entry and set + `sidebar.existingProofId`. Do not execute the same sidebar assertion again + in the authorization case. +- **Unsaved-change behavior:** edit + `apps/sim/e2e/settings/authorization/unsaved-changes.spec.ts` directly. +- **Secrets or API key lifecycle:** edit the owning credential spec directly. + Change `apps/sim/e2e/settings/credentials/contracts.ts` only when reused + authorization proof IDs or boundaries change. +- **People, access control, SSO, data retention, or MCP lifecycle:** edit the + owning workflow spec. Change `apps/sim/e2e/settings/workflows/contracts.ts` + when stable lifecycle or cross-contract proof references change. +- **Persona or seeded relationship:** update + `apps/sim/e2e/settings/personas.ts` and the relevant + scenario/factory/integrity proof. + +The categories are non-exclusive. Prefer extending an existing literal row or +owning workflow over adding a duplicate standalone test. + +### 3. Use the owning fixture wrapper + +- Browser-touching settings specs must not import runtime `test` or `expect` + from `@playwright/test`. Type-only imports such as `Page` or `Response` are + allowed. +- Navigation, authorization, persona-contract, and persona-isolation browser + specs use `apps/sim/e2e/fixtures/persona-test.ts`. +- Credential specs use the local `credential-test.ts`; workflow specs use the + local `workflow-test.ts`; authenticated or unauthenticated smoke and + browser-touching harness-level specs use + `apps/sim/e2e/fixtures/browser-test.ts`. +- Pure dataset-only contract-integrity specs and non-browser foundation policy + specs may use the Playwright test runner directly. If they begin creating a + browser, context, or page, move them to the owning wrapper first. +- Persona-based tests must create contexts through `contextForPersona` or an + owning helper that calls it. The inherited `page` and `context` fixtures on + `persona-test` are not persona-authenticated or network-guarded. +- `browser-test` guards its default context. Every manual + `browser.newContext()` must install `installBrowserNetworkGuard()` immediately, + then close the context and assert the guard even when the test fails; follow + the aggregate-cleanup pattern in authenticated smoke coverage. Do not use + `browser.newPage()`: it creates an implicit unguarded context; create and guard + an explicit context, then call `context.newPage()`. +- Reuse the owning directory's helpers. Credential tests use their local + `newPersonaPage` helper so the page is registered for failure sanitization. + Wrapper selection alone does not install every context, cleanup, attachment, + redaction, or artifact safeguard; bypassing the guarded fixture/helper path is + a test-safety bug. + +### 4. Reuse the existing world + +- Reuse an existing persona and seeded resource when it expresses the required + role, plan, entitlement, and ownership boundary. +- Add a persona or scenario edge only when no existing driver can prove the + contract without changing its meaning. +- Use run-namespaced factories and unique resources. Do not introduce shared + mutable fixture state. +- Keep sensitive fixture values browser-resident through the existing helpers. + The Playwright process must not receive database credentials, admin keys, + persona passwords, or plaintext values that the owning spec keeps in-page. + +### 5. Assert semantics, not implementation details + +- Use accessible roles, labels, names, and visible text. +- Assert exact paths and user-facing contract copy when those are the behavior + under test. +- Wait on semantic readiness, relevant same-origin responses, or explicit + authorization state—not arbitrary sleeps or CSS classes. +- For access controls, prove direct URL behavior as well as sidebar visibility. +- Do not import production navigation or authorization implementations to + generate expected values. + +### 6. Make mutations reversible + +- Register cleanup before the first mutation. +- Create uniquely named resources and restore the exact captured baseline. +- Use the production UI and same-origin APIs for behavior under test; use a + trusted database probe only where the existing suite defines one. +- Keep cleanup LIFO and safe after partial failure. Never weaken fixture + invariants merely to make a test pass. + +### 7. Respect external and diagnostic boundaries + +- Use the existing strict loopback Stripe and MCP fakes, the mail mock-success + path with provider credentials absent, and the other reviewed test + boundaries. An unexpected external request must fail rather than silently + egress. +- Credentials, tokens, certificates, verification values, storage state, and + sensitive response bodies may be used only through existing reviewed paths; + never log, attach, or retain them in diagnostics. +- The SSO workflow's public-certificate browser input is one reviewed example; + preserve its input clearing and diagnostic suppression. +- Preserve the owning spec's trace, screenshot, video, and attachment policy. + New suppression requires a security reason and corresponding safety coverage. + +### 8. Keep unit and integrity proof aligned + +- Update affected production unit tests for navigation, route gates, + authorization, billing, or business rules. +- Update the owning `contract-integrity.spec.ts` when adding IDs, references, or + a new contract axis. +- Preserve stable proof IDs; do not rerun a browser case merely because another + contract can reference its existing proof. + +### 9. Verify through the orchestrator + +- Run the affected unit and contract-integrity tests. +- Run the focused canonical project and path documented in + `apps/sim/e2e/README.md`. +- Invoke only `bun run test:e2e`; never invoke raw `playwright test`. +- Use `--reuse-build` and a single explicit project with `--no-deps` for local + iteration when the README permits it. +- Follow the retry policy in the canonical README and Playwright config. It is + currently zero; do not enable retries or change worker policy in a feature + test. Do not use `test.only`, unexplained skips, arbitrary browser sleeps, or + test-local environment bypasses. +- After focused proof passes, rely on the required complete CI suite. Run a + repeated stability gate only when the change's scope or acceptance plan calls + for one. + +## Completion checklist + +- [ ] Every observable facet has one clear owner, and all applicable owners were + updated. +- [ ] Allowed and denied outcomes exist where the feature is gated. +- [ ] Locators and readiness assertions are semantic and accessible. +- [ ] Mutations register cleanup first and restore the exact baseline. +- [ ] No secret or external-egress boundary was weakened. +- [ ] Related unit and contract-integrity tests are aligned and pass. +- [ ] Focused execution used the guarded orchestrator and complied with the + canonical retry policy. diff --git a/.cursor/commands/add-settings-e2e-test.md b/.cursor/commands/add-settings-e2e-test.md new file mode 100644 index 00000000000..5fc43e45554 --- /dev/null +++ b/.cursor/commands/add-settings-e2e-test.md @@ -0,0 +1,175 @@ +# Add Settings E2E Test + +Add the smallest durable browser proof for the requested observable behavior. +Do not begin by copying a nearby test: first identify which acceptance dataset +or spec owns the behavior. + +## Read first + +1. `apps/sim/e2e/MAINTENANCE.md` — change type to contract/spec ownership. +2. `apps/sim/e2e/README.md` — orchestrator, project boundaries, focused commands, + diagnostics, and CI policy. +3. `apps/sim/e2e/STABILIZATION.md` — non-waivable acceptance and security + boundaries. + +Those files are canonical. This skill is the authoring procedure, not a second +copy of their inventories or commands. If this procedure ever conflicts with a +canonical document, the canonical document wins. + +## Procedure + +### 1. Define the observable contract + +Write down: + +- the user persona and resource plane (account, organization, or workspace); +- the action or direct URL; +- the exact observable result: path, semantic readiness, visible/hidden item, + access outcome, enabled/disabled control, warning, or persisted state; +- whether product behavior is intentionally changing or an implementation + change must preserve the existing contract. + +Intended behavior changes update product code and literal acceptance +expectations together. Behavior-preserving fixes add or strengthen proof without +rewriting unrelated expectations. + +### 2. Choose the owner before writing code + +- **Canonical section, route, copy, readiness, or persona visibility:** update + `apps/sim/e2e/settings/navigation/contracts.ts`; its specs generate cases from + the literal dataset, including unauthenticated redirect cases. +- **Browser Back, app Back, direct-entry fallback, or return destination:** edit + `apps/sim/e2e/settings/navigation/history.spec.ts` directly. +- **Direct access, plan/role/entitlement gate, or mutation availability:** update + `apps/sim/e2e/settings/authorization/contracts.ts`; its access and mutation + specs generate cases from that dataset. + - If an existing navigation case already owns the sidebar proof, add or reuse + an `existingNavigationProofs` entry and set + `sidebar.existingProofId`. Do not execute the same sidebar assertion again + in the authorization case. +- **Unsaved-change behavior:** edit + `apps/sim/e2e/settings/authorization/unsaved-changes.spec.ts` directly. +- **Secrets or API key lifecycle:** edit the owning credential spec directly. + Change `apps/sim/e2e/settings/credentials/contracts.ts` only when reused + authorization proof IDs or boundaries change. +- **People, access control, SSO, data retention, or MCP lifecycle:** edit the + owning workflow spec. Change `apps/sim/e2e/settings/workflows/contracts.ts` + when stable lifecycle or cross-contract proof references change. +- **Persona or seeded relationship:** update + `apps/sim/e2e/settings/personas.ts` and the relevant + scenario/factory/integrity proof. + +The categories are non-exclusive. Prefer extending an existing literal row or +owning workflow over adding a duplicate standalone test. + +### 3. Use the owning fixture wrapper + +- Browser-touching settings specs must not import runtime `test` or `expect` + from `@playwright/test`. Type-only imports such as `Page` or `Response` are + allowed. +- Navigation, authorization, persona-contract, and persona-isolation browser + specs use `apps/sim/e2e/fixtures/persona-test.ts`. +- Credential specs use the local `credential-test.ts`; workflow specs use the + local `workflow-test.ts`; authenticated or unauthenticated smoke and + browser-touching harness-level specs use + `apps/sim/e2e/fixtures/browser-test.ts`. +- Pure dataset-only contract-integrity specs and non-browser foundation policy + specs may use the Playwright test runner directly. If they begin creating a + browser, context, or page, move them to the owning wrapper first. +- Persona-based tests must create contexts through `contextForPersona` or an + owning helper that calls it. The inherited `page` and `context` fixtures on + `persona-test` are not persona-authenticated or network-guarded. +- `browser-test` guards its default context. Every manual + `browser.newContext()` must install `installBrowserNetworkGuard()` immediately, + then close the context and assert the guard even when the test fails; follow + the aggregate-cleanup pattern in authenticated smoke coverage. Do not use + `browser.newPage()`: it creates an implicit unguarded context; create and guard + an explicit context, then call `context.newPage()`. +- Reuse the owning directory's helpers. Credential tests use their local + `newPersonaPage` helper so the page is registered for failure sanitization. + Wrapper selection alone does not install every context, cleanup, attachment, + redaction, or artifact safeguard; bypassing the guarded fixture/helper path is + a test-safety bug. + +### 4. Reuse the existing world + +- Reuse an existing persona and seeded resource when it expresses the required + role, plan, entitlement, and ownership boundary. +- Add a persona or scenario edge only when no existing driver can prove the + contract without changing its meaning. +- Use run-namespaced factories and unique resources. Do not introduce shared + mutable fixture state. +- Keep sensitive fixture values browser-resident through the existing helpers. + The Playwright process must not receive database credentials, admin keys, + persona passwords, or plaintext values that the owning spec keeps in-page. + +### 5. Assert semantics, not implementation details + +- Use accessible roles, labels, names, and visible text. +- Assert exact paths and user-facing contract copy when those are the behavior + under test. +- Wait on semantic readiness, relevant same-origin responses, or explicit + authorization state—not arbitrary sleeps or CSS classes. +- For access controls, prove direct URL behavior as well as sidebar visibility. +- Do not import production navigation or authorization implementations to + generate expected values. + +### 6. Make mutations reversible + +- Register cleanup before the first mutation. +- Create uniquely named resources and restore the exact captured baseline. +- Use the production UI and same-origin APIs for behavior under test; use a + trusted database probe only where the existing suite defines one. +- Keep cleanup LIFO and safe after partial failure. Never weaken fixture + invariants merely to make a test pass. + +### 7. Respect external and diagnostic boundaries + +- Use the existing strict loopback Stripe and MCP fakes, the mail mock-success + path with provider credentials absent, and the other reviewed test + boundaries. An unexpected external request must fail rather than silently + egress. +- Credentials, tokens, certificates, verification values, storage state, and + sensitive response bodies may be used only through existing reviewed paths; + never log, attach, or retain them in diagnostics. +- The SSO workflow's public-certificate browser input is one reviewed example; + preserve its input clearing and diagnostic suppression. +- Preserve the owning spec's trace, screenshot, video, and attachment policy. + New suppression requires a security reason and corresponding safety coverage. + +### 8. Keep unit and integrity proof aligned + +- Update affected production unit tests for navigation, route gates, + authorization, billing, or business rules. +- Update the owning `contract-integrity.spec.ts` when adding IDs, references, or + a new contract axis. +- Preserve stable proof IDs; do not rerun a browser case merely because another + contract can reference its existing proof. + +### 9. Verify through the orchestrator + +- Run the affected unit and contract-integrity tests. +- Run the focused canonical project and path documented in + `apps/sim/e2e/README.md`. +- Invoke only `bun run test:e2e`; never invoke raw `playwright test`. +- Use `--reuse-build` and a single explicit project with `--no-deps` for local + iteration when the README permits it. +- Follow the retry policy in the canonical README and Playwright config. It is + currently zero; do not enable retries or change worker policy in a feature + test. Do not use `test.only`, unexplained skips, arbitrary browser sleeps, or + test-local environment bypasses. +- After focused proof passes, rely on the required complete CI suite. Run a + repeated stability gate only when the change's scope or acceptance plan calls + for one. + +## Completion checklist + +- [ ] Every observable facet has one clear owner, and all applicable owners were + updated. +- [ ] Allowed and denied outcomes exist where the feature is gated. +- [ ] Locators and readiness assertions are semantic and accessible. +- [ ] Mutations register cleanup first and restore the exact baseline. +- [ ] No secret or external-egress boundary was weakened. +- [ ] Related unit and contract-integrity tests are aligned and pass. +- [ ] Focused execution used the guarded orchestrator and complied with the + canonical retry policy. diff --git a/apps/sim/e2e/MAINTENANCE.md b/apps/sim/e2e/MAINTENANCE.md index 42d3e6226db..25e9c59a264 100644 --- a/apps/sim/e2e/MAINTENANCE.md +++ b/apps/sim/e2e/MAINTENANCE.md @@ -5,6 +5,10 @@ organization, or workspace settings surface. The browser datasets are literal acceptance contracts, intentionally independent of production navigation and authorization implementations. +For the step-by-step authoring workflow, invoke `/add-settings-e2e-test`; its +canonical source from the repository root is +`.agents/skills/add-settings-e2e-test/SKILL.md`. + ## Decision rule - An intended observable contract change requires the product code and the