diff --git a/.agents/skills/add-enrichment/SKILL.md b/.agents/skills/add-enrichment/SKILL.md index fcb83525ab9..35ef4596b24 100644 --- a/.agents/skills/add-enrichment/SKILL.md +++ b/.agents/skills/add-enrichment/SKILL.md @@ -22,7 +22,7 @@ Because enrichments run on Sim's hosted keys by default, **every provider tool y ## Architecture (what you're plugging into) -- **`enrichments/types.ts`** — `EnrichmentConfig { id, name, description, icon, inputs, outputs, providers }` and `EnrichmentProvider { id, label, toolId, buildParams, projectFailure, mapOutput }` — `toolProvider` fills `projectFailure` with the standard HTTP projection; override it only for a provider whose failure shape is nonstandard (see `enrichments/provider-failures/`). Providers are **plain data** (no `@/tools` import) so the catalog stays client-safe. +- **`enrichments/types.ts`** — `EnrichmentConfig { id, name, description, icon, inputs, outputs, providers }` and `EnrichmentProvider { id, label, toolId, buildParams, projectFailure, mapOutput, isVerified? }` — `toolProvider` fills `projectFailure` with the standard HTTP projection; override it only for a provider whose failure shape is nonstandard (see `enrichments/provider-failures/`). Providers are **plain data** (no `@/tools` import) so the catalog stays client-safe. - **`enrichments/providers.ts`** — `toolProvider(...)` (typed passthrough) plus shared input helpers: `str(v)`, `normalizeDomain(v)`, `firstNonEmpty(arr)`, `splitName(fullName)`, and `projectEnrichmentProviderFailure`. - **`enrichments/run.ts`** — the server-only cascade runner. Calls `executeTool(provider.toolId, { ...params, _context: { workspaceId, userId } })`, accumulates hosted-key cost, returns the first non-empty mapped result. **You do not edit this** — it works for any registry entry. - **`enrichments/registry.ts`** — `ENRICHMENT_REGISTRY` / `ALL_ENRICHMENTS` / `getEnrichment`. Register new entries here. @@ -37,7 +37,9 @@ For each output the enrichment produces, decide which existing tool provides it. - Its `params` accept what you can derive from table columns (read the tool's `params`). - Its `outputs` / `transformResponse` actually expose the field you need (read the real output shape — don't assume). -Order providers **cheapest / most-likely-to-hit first**; the cascade stops at the first non-empty result. Apollo and LinkedIn APIs are not hosted-safe (ToS) — never call them as providers. +Order providers by **accuracy first, then price per hit**. Order decides which answer wins when two providers would disagree, so a cheaper but less accurate provider ahead of a better one trades quality for cost; reorder for price only with benchmark evidence. Among similarly accurate providers, put the ones that bill on a miss or charge a high price per hit after the cheaper, free-on-miss ones (check each vendor's billing docs and the tool's `hosting.pricing`); never move a more accurate provider behind a less accurate one to save cost. The cascade stops at the first non-empty, verified result. Apollo and LinkedIn APIs are not hosted-safe (ToS) — never call them as providers. + +Prefer a vendor's own verified-only option (e.g. Prospeo `only_verified_email`) when it turns an unverified result into a free miss. Otherwise, when a provider can return an unconfirmed hit (a catch-all guess, a `risky` status), declare `isVerified(output)`: an unconfirmed hit is held while later providers look for a confirmed one, and fills the cell only if none does. `isVerified: false` marks a provider that never confirms its hits (a database record); it runs only while nothing is held. ## Step 2: Verify hosted-key support — chain to `/add-hosted-key` if missing @@ -136,7 +138,8 @@ export const ENRICHMENT_REGISTRY: EnrichmentRegistry = { - [ ] Each output mapped to a real tool field (verified against the tool's `params`/`outputs`) - [ ] **Every provider tool has a `hosting` block — ran `/add-hosted-key` for any that didn't** -- [ ] Providers ordered cheapest / most-likely-first; Apollo/LinkedIn not used +- [ ] Providers ordered accuracy-first, then price per hit (billing on a miss checked against vendor docs); Apollo/LinkedIn not used +- [ ] Providers that can return an unconfirmed hit declare `isVerified` - [ ] Enrichment file is client-safe (no `@/tools` import); uses `toolProvider` + shared helpers - [ ] `buildParams` returns `null` on insufficient inputs; `mapOutput` returns `null` on a miss - [ ] Registered in `enrichments/registry.ts` diff --git a/.claude/rules/sim-testing.md b/.claude/rules/sim-testing.md index 74e32a99cbd..8684de09d8d 100644 --- a/.claude/rules/sim-testing.md +++ b/.claude/rules/sim-testing.md @@ -33,7 +33,7 @@ contracts, and demonstrated regressions. | Suffix | Needs | Run with | In CI | |--------|-------|----------|-------| | `*.test.ts(x)` | nothing; global mocks from `vitest.setup.ts` | `bun run --cwd test` | `test` jobs | -| `*.integration.ts` | real PostgreSQL (`TEST_DATABASE_URL`), optionally Redis (`TEST_REDIS_URL`) | `bun run --cwd test --mode integration` | `integration` jobs, by glob (sharded per provisioning path) | +| `*.integration.ts` | real PostgreSQL (`TEST_DATABASE_URL`), Redis (`TEST_REDIS_URL`; optional locally, required in CI) | `bun run --cwd test --mode integration` | `integration` jobs, by glob (sharded per provisioning path) | | `*.live.test.ts` | provider APIs, hosted sandboxes, local runtimes, or sibling checkouts | `bun run --cwd apps/sim test --mode live ` | never | | `apps/desktop/e2e/*.spec.ts` | the packaged Electron app | Playwright | desktop E2E workflow | | `apps/sim/scripts/test-*-e2e.ts` | a running app over HTTP | its `package.json` script when one exists (`bun run test:scim:e2e`; `test:workflow-version-compare:e2e` adds `--no-env-file`), else `bun scripts/test--e2e.ts` from apps/sim | `e2e` jobs (`.github/scripts/http-e2e.sh`) | @@ -48,8 +48,8 @@ contracts, and demonstrated regressions. those checks. Isolate with a unique schema or generated IDs, and clean up in `afterAll`. - Integration files run one at a time against one shared database. A new `*.integration.ts` is picked up by CI with no workflow change, and the run writes the JSON report configured in - `vitest.shared.ts`, which CI uploads. Never add a passing suite to the quarantine list in - `apps/sim/vitest.config.ts`. + `vitest.shared.ts`, which CI uploads. Never exclude a failing suite from the run in + `apps/sim/vitest.config.ts`: fix it or delete it. - `bun run test:integration` starts disposable Postgres and Redis containers, provisions the schema, and runs both workspaces; pass filenames to narrow the `apps/sim` run. @@ -169,6 +169,7 @@ The suite's wall time is bound by the single Vite server thread that serves ever 3. Mock heavy graphs a test does not need and the setup does not already mock: `@/blocks`, `@/triggers/registry`, `@/tools/generated/*`. 4. No real timers: `vi.useFakeTimers()`, `flushMicrotasks()`, or `flushMacrotask()`. + `check:test-patterns` (`real-wait`) fails on a new wall-clock wait in a unit test. 5. Absolute imports only. ### Running diff --git a/.cursor/rules/sim-testing.mdc b/.cursor/rules/sim-testing.mdc index 69dc1bd9497..f8e665e0c90 100644 --- a/.cursor/rules/sim-testing.mdc +++ b/.cursor/rules/sim-testing.mdc @@ -31,7 +31,7 @@ contracts, and demonstrated regressions. | Suffix | Needs | Run with | In CI | |--------|-------|----------|-------| | `*.test.ts(x)` | nothing; global mocks from `vitest.setup.ts` | `bun run --cwd test` | `test` jobs | -| `*.integration.ts` | real PostgreSQL (`TEST_DATABASE_URL`), optionally Redis (`TEST_REDIS_URL`) | `bun run --cwd test --mode integration` | `integration` jobs, by glob (sharded per provisioning path) | +| `*.integration.ts` | real PostgreSQL (`TEST_DATABASE_URL`), Redis (`TEST_REDIS_URL`; optional locally, required in CI) | `bun run --cwd test --mode integration` | `integration` jobs, by glob (sharded per provisioning path) | | `*.live.test.ts` | provider APIs, hosted sandboxes, local runtimes, or sibling checkouts | `bun run --cwd apps/sim test --mode live ` | never | | `apps/desktop/e2e/*.spec.ts` | the packaged Electron app | Playwright | desktop E2E workflow | | `apps/sim/scripts/test-*-e2e.ts` | a running app over HTTP | its `package.json` script when one exists (`bun run test:scim:e2e`; `test:workflow-version-compare:e2e` adds `--no-env-file`), else `bun scripts/test--e2e.ts` from apps/sim | `e2e` jobs (`.github/scripts/http-e2e.sh`) | @@ -46,8 +46,8 @@ contracts, and demonstrated regressions. those checks. Isolate with a unique schema or generated IDs, and clean up in `afterAll`. - Integration files run one at a time against one shared database. A new `*.integration.ts` is picked up by CI with no workflow change, and the run writes the JSON report configured in - `vitest.shared.ts`, which CI uploads. Never add a passing suite to the quarantine list in - `apps/sim/vitest.config.ts`. + `vitest.shared.ts`, which CI uploads. Never exclude a failing suite from the run in + `apps/sim/vitest.config.ts`: fix it or delete it. - `bun run test:integration` starts disposable Postgres and Redis containers, provisions the schema, and runs both workspaces; pass filenames to narrow the `apps/sim` run. @@ -167,6 +167,7 @@ The suite's wall time is bound by the single Vite server thread that serves ever 3. Mock heavy graphs a test does not need and the setup does not already mock: `@/blocks`, `@/triggers/registry`, `@/tools/generated/*`. 4. No real timers: `vi.useFakeTimers()`, `flushMicrotasks()`, or `flushMacrotask()`. + `check:test-patterns` (`real-wait`) fails on a new wall-clock wait in a unit test. 5. Absolute imports only. ### Running diff --git a/.github/scripts/ci-changes.sh b/.github/scripts/ci-changes.sh new file mode 100755 index 00000000000..c506a2149bf --- /dev/null +++ b/.github/scripts/ci-changes.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Prints which checks a pull request's changes can affect, as `=true|false` lines for +# $GITHUB_OUTPUT. A flag is `false` only when every changed file is on an allowlist whose jobs +# fully cover it. A file on no allowlist sets every flag but `desktop_live` (which keeps its own +# skip list) to `true`; a change under .github/, an empty diff, or any failure to work out the +# diff sets all of them. +# +# full integration, e2e, e2e-build, e2e-mobile: any file not allowlisted below +# test the Vitest shards and test-types: content, docs app, and desktop changes, +# plus `full` +# build the app's `next build`: content changes, plus `full` +# desktop_live the live Electron suite: any file outside its own skip list +# +# lint always runs and is not gated. +# +# Usage: ci-changes.sh +set -u + +# Posts, author profiles, and the public assets only posts reference. The test shards read them +# (lib/content/og-image.test.ts) and `next build` prerenders every post with its MDX components, +# which check:library-content compiles but never renders. Components under content/ are code. +content='^apps/sim/content/([^/]+/)*[^/]+\.mdx?$|^apps/sim/content/authors/[^/]+\.json$|^apps/sim/public/(authors|blog|changelog|library)/' +# Its own workspace tests, its type-check and docs-manifest audit in lint, and apps/sim tests +# that read its pages (tools/netsuite/netsuite.test.ts). +docs='^apps/docs/' +# Nothing outside the Electron app imports it; the desktop bridge and protocol packages are +# consumed by apps/sim and stay unlisted. +desktop='^apps/desktop/' +# Agent guidance, read only by the lint job's audits. +guidance='^(\.agents|\.claude|\.cursor)/|^([^/]+/)*(AGENTS|CLAUDE)\.md$|^README\.md$' +# The live desktop suite drives apps/desktop against the app, so it skips more than the rest. +desktop_live_skip="$guidance"'|^(apps/docs/|apps/pii/|apps/sim/content/|apps/sim/public/(authors|blog|changelog|library)/|packages/(python-sdk|ts-sdk)/)|\.mdx?$|(^|/)LICENSE$' + +emit() { + printf 'full=%s\ntest=%s\nbuild=%s\ndesktop_live=%s\n' "$1" "$2" "$3" "$4" +} +run_all() { + emit true true true true + echo "Running every check: $1" >&2 + exit 0 +} +matches() { printf '%s\n' "$names" | grep -qE "$1"; } +outside() { printf '%s\n' "$names" | grep -qvE "$1"; } + +base=${1:-} +[ -n "$base" ] || run_all 'no base commit' +git fetch --quiet --depth=1 origin "$base" || run_all "could not fetch $base" +names=$(git diff --no-renames --name-only "$base" HEAD) || run_all "could not diff against $base" +[ -n "$names" ] || run_all 'no changed files listed' +if matches '^\.github/'; then run_all 'a CI file changed'; fi + +desktop_live=false +if outside "$desktop_live_skip"; then desktop_live=true; fi +if outside "$content|$docs|$desktop|$guidance"; then + emit true true true "$desktop_live" + unlisted=$(printf '%s\n' "$names" | grep -vE "$content|$docs|$desktop|$guidance" | head -n 1) + echo "Running every check (desktop_live=$desktop_live): $unlisted is not on an allowlist" >&2 + exit 0 +fi + +test=false build=false +if matches "$content|$docs|$desktop"; then test=true; fi +if matches "$content"; then build=true; fi +emit false "$test" "$build" "$desktop_live" +echo "Scoped checks: test=$test build=$build desktop_live=$desktop_live" >&2 diff --git a/.github/scripts/ci-gate.sh b/.github/scripts/ci-gate.sh new file mode 100755 index 00000000000..c5303551763 --- /dev/null +++ b/.github/scripts/ci-gate.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# The `ci` job's verdict: the single required check, so it must never pass a run whose checks did +# not all pass. Reads the job's `needs` context (`toJSON(needs)`) from $NEEDS and exits non-zero, +# naming each offending job, unless every job succeeded or was skipped as `changes` intended. +# +# A skip passes only for a gated job whose `changes` flag is exactly `false`: +# +# integration, e2e, e2e-build, e2e-mobile full +# test, test-types test +# build build +# desktop-live desktop_live +# +# Every other skip fails: an ungated job (lint, `changes` itself), a gated job whose flag is +# `true`, missing, or anything else (a failed or skipped `changes` leaves no outputs). A failure +# or a cancellation always fails. +# +# Usage: NEEDS='' ci-gate.sh +set -euo pipefail + +needs=${NEEDS:-} +if ! jq -e 'type == "object"' <<< "$needs" > /dev/null 2>&1; then + echo "::error::The needs context is not a JSON object: '$needs'" + exit 1 +fi + +failed="$(jq -r ' + {"integration": "full", "e2e": "full", "e2e-build": "full", "e2e-mobile": "full", "test": "test", + "test-types": "test", "build": "build", "desktop-live": "desktop_live"} as $gate + | (.changes.outputs // {}) as $flags + | to_entries[] + | select(.value.result != "success") + | select(.value.result != "skipped" or $gate[.key] == null or $flags[$gate[.key]] != "false") + | "\(.key): \(.value.result)" +' <<< "$needs")" +if [ -n "$failed" ]; then + echo "::error::Checks did not pass:" + echo "$failed" + exit 1 +fi +echo "All checks passed." diff --git a/.github/scripts/desktop-live-changes.sh b/.github/scripts/desktop-live-changes.sh deleted file mode 100755 index d42cc2b2e70..00000000000 --- a/.github/scripts/desktop-live-changes.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/usr/bin/env bash -# Prints `changed=false` only when every file a pull request changes is clearly unrelated to the -# live desktop suite, and `changed=true` otherwise, including when the diff cannot be worked out. -# -# Usage: desktop-live-changes.sh -set -u - -unrelated='^(apps/docs/|apps/pii/|apps/sim/content/|packages/(python-sdk|ts-sdk)/)|\.mdx?$|(^|/)LICENSE$' - -base=${1:-} -run() { - echo "changed=true" - echo "Running the live desktop suite: $1" >&2 - exit 0 -} -[ -n "$base" ] || run 'no base commit' -git fetch --quiet --depth=1 origin "$base" || run "could not fetch $base" -names=$(git diff --no-renames --name-only "$base" HEAD) || run "could not diff against $base" -[ -n "$names" ] || run 'no changed files listed' -if printf '%s\n' "$names" | grep -qvE "$unrelated"; then - run 'a change may affect it' -fi -echo "changed=false" -echo 'Skipping the live desktop suite: every change is unrelated to it' >&2 diff --git a/.github/scripts/http-e2e.sh b/.github/scripts/http-e2e.sh index ef9c500bd1d..bccb9491102 100755 --- a/.github/scripts/http-e2e.sh +++ b/.github/scripts/http-e2e.sh @@ -1,14 +1,16 @@ #!/usr/bin/env bash -# Runs one end-to-end suite group over real HTTP, each against its own `next dev` app. +# Runs one end-to-end suite group over real HTTP, each against its own app: `next dev`, except +# mobile, which serves the production build the `e2e-build` job uploads with `next start` (the +# job extracts it into apps/sim/.next first). # # Usage: http-e2e.sh (run from apps/sim) # # The job provides DATABASE_URL, BETTER_AUTH_SECRET and ENCRYPTION_KEY; each group sets the rest of # its app's environment here. Reports and server logs land in $RUNNER_TEMP/e2e. # -# The first request cold-compiles the app under Turbopack, which takes 42-150s on CI runners, so -# the readiness deadline only has to catch a hung boot: an exited server fails immediately, and -# either way the server log tail lands in the job log. +# Under `next dev` the first request cold-compiles the app under Turbopack, which takes 42-150s on +# CI runners, so the readiness deadline only has to catch a hung boot: an exited server fails +# immediately, and either way the server log tail lands in the job log. # # Each app starts from an empty Turbopack dev cache: a cache written under other NEXT_PUBLIC_* # values, by a server that `next dev` SIGKILLs 100ms after SIGTERM, can panic Turbopack or wedge a @@ -26,19 +28,43 @@ server_pid='' app_tag='' server_log='' status_log='' +realtime_pid='' +realtime_tag='' +realtime_log='' +browsers_pid='' +# `dev` or `start`; a group sets it before start_app. +app_server=dev finish() { local status=$? if [ -n "$server_pid" ]; then bash "$GITHUB_WORKSPACE/.github/scripts/stop-session.sh" "$server_pid" "$app_tag" || status=1 wait "$server_pid" 2>/dev/null || true - if [ -n "$status_log" ]; then + if [ -n "$status_log" ] && [ "$app_server" = start ]; then + # `next start` logs no requests; RouteHandler's closing line for each API request carries + # its status (an unhandled error's line is a 500 without one) and duration. + jq -Rr 'fromjson? | objects | select(.module == "RouteHandler" and .duration != null + and ((.path // "") | startswith("/api/"))) + | "\(.method) \(.path) \(.status // 500) in \(.duration)ms"' \ + "$server_log" > "$status_log" + elif [ -n "$status_log" ]; then awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$status_log" fi if [ "$status" -ne 0 ]; then tail -n 200 "$server_log" fi fi + if [ -n "$realtime_pid" ]; then + bash "$GITHUB_WORKSPACE/.github/scripts/stop-session.sh" "$realtime_pid" "$realtime_tag" || status=1 + wait "$realtime_pid" 2>/dev/null || true + if [ "$status" -ne 0 ]; then + tail -n 100 "$realtime_log" + fi + fi + if [ -n "$browsers_pid" ]; then + kill -- "-$browsers_pid" 2>/dev/null || true + wait "$browsers_pid" 2>/dev/null || true + fi exit "$status" } trap finish EXIT @@ -54,24 +80,54 @@ start_app() { export NEXT_PUBLIC_APP_URL="http://127.0.0.1:$port" export BETTER_AUTH_URL="$NEXT_PUBLIC_APP_URL" export DISABLE_TELEMETRY=true NEXT_TELEMETRY_DISABLED=1 - rm -rf .next/dev - E2E_APP="$app_tag" setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 \ - --port "$port" > "$server_log" 2>&1 & + if [ "$app_server" = start ]; then + if [ ! -f .next/BUILD_ID ]; then + echo "::error::No production build in apps/sim/.next for next start." + exit 1 + fi + # Production logs only errors by default; INFO writes the RouteHandler lines the status log + # reads. + E2E_APP="$app_tag" LOG_LEVEL=INFO setsid node ../../node_modules/next/dist/bin/next start \ + --hostname 127.0.0.1 --port "$port" > "$server_log" 2>&1 & + else + rm -rf .next/dev + E2E_APP="$app_tag" setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 \ + --port "$port" > "$server_log" 2>&1 & + fi server_pid=$! + wait_ready "$server_pid" "$NEXT_PUBLIC_APP_URL/api/health" "$label app" +} + +# start_realtime : the Socket.IO server the app's browser clients connect to, so pages +# render connected rather than under a "Reconnecting..." notification. Start after start_app, which +# exports the app URL the server allows as an origin; NEXT_PUBLIC_SOCKET_URL must already be set +# when the app starts. +start_realtime() { + local name=$1 port=$2 + realtime_tag="$name-realtime-$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT-$$" + realtime_log="$report_dir/$name-realtime.log" + (cd ../realtime && PORT="$port" E2E_APP="$realtime_tag" exec setsid bun run start \ + > "$realtime_log" 2>&1) & + realtime_pid=$! + wait_ready "$realtime_pid" "http://127.0.0.1:$port/health" "$name realtime server" +} +# wait_ready