Commit Graph
5 Commits
Author SHA1 Message Date
Theodore Li 7293b67a54 feat(custom-blocks): log and bill child runs in the publisher's workspace (#6023)
* feat(custom-blocks): log and bill child runs in the publisher's workspace

* fix(custom-blocks): sanitize every boundary failure and classify it for consumers

* fix(custom-blocks): surface a cancelled child as cancelled, not a generic failure

* fix(custom-blocks): share one large-value id list so nested blocks propagate

* fix(custom-blocks): add a durable cancel backstop to the child bridge

* fix(tools): carry Sim's own status through the tool-response boundary

* fix(custom-blocks): stop forwarding the publisher's personal quota to consumers

* fix(custom-blocks): correlate agent-tool runs to the real invoking execution

* fix(custom-blocks): plumb the invoking execution id and abort signal to agent tools

* fix(agent): forward the execution id through the provider payload

* fix(executor): stop adopting an upstream target's HTTP status as our own

* fix(custom-blocks): drain child log finalization when the parent is cancelled

* fix(custom-blocks): require curated outputs instead of exposing the whole result

* fix(custom-blocks): track the whole child run, not just its finalization
2026-07-30 21:20:58 -04:00
Theodore LiandClaude Opus 4.8 aed44024d4 fix(tables): surface real error causes on cell-execution failures (diagnostics) (#4868)
* fix(tables): retry transient DB/Redis failures in cell execution and surface error causes

Workflow-group-cell runs intermittently failed on trivial DB reads/writes
under heavy fan-out, stranding cells in `running`. Investigation showed the
PlanetScale and ElastiCache backends were healthy at the time — the failures
are transient connection-level faults that the cell (maxAttempts: 1) had no
tolerance for, and the real cause was never logged (Drizzle wraps it as
"Failed query: ..." and the driver cause lives in error.cause).

Resilience:
- Add retryTransient (lib/table/retry-transient.ts): retries only transient
  infra errors (reuses isRetryableInfrastructureError; adds an ioredis
  command-timeout match) with jittered backoff, then rethrows. Fail-fast for
  everything else.
- Wrap the cell's getTableById/getRowById reads, the terminal write
  (cell-write updateRow — idempotent via the executionId guard), and the
  Redis cascade-lock acquire.

Diagnostics:
- Add describeError (lib/core/errors/retryable-infrastructure.ts): walks the
  .cause chain and always returns the underlying driver cause (code/errno/
  syscall + causeChain), including for unclassified errors like AbortError.
- Log `cause` + a `retryable` flag (and aborted/timedOut in the cell's main
  catch) across the cell + finalization error paths, mirroring the existing
  schedule-execution pattern. Logging-only; no behavior change. This lets the
  next recurrence reveal the real cause and whether the retry applies.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(tables): address review feedback on cell retry resilience

- retryTransient: re-check the abort signal after the backoff sleep so a
  cancellation during sleep stops the next attempt (don't run/return work for
  an already-cancelled task).
- isRetryableRedisError: walk the .cause chain (mirroring the infra
  classifier) so wrapped Redis timeouts are recognized; drop "Connection is in
  subscriber mode" — that's a connection-state programming error, not a
  transient drop, and would just fail identically every retry.
- cascade-lock: stop wrapping acquireLock in retryTransient. acquireLock is a
  non-idempotent SET NX, so retrying after a timed-out-but-applied first SET
  returns false (key already ours) and yields a false `contended` that skips
  the cascade. A transient Redis blip here just fails the run before pickup
  (no stranded cell); the dispatcher re-drives it.
- Tests: cause-chain Redis match, subscriber-mode exclusion, abort-during-sleep.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(tables): drop out-of-scope abort/timeout fields from cell catch

The main catch logged `aborted`/`timedOut` from `abortSignal`/`timeoutController`,
but those are declared inside the outer try block (the inner try around
executeWorkflow is try/finally, so this catch belongs to the outer try) and are
not in scope in the catch — `next build`'s type-check failed with "Cannot find
name 'abortSignal'". Local incremental `tsc --noEmit` had skipped the file and
falsely passed; the Cursor/Greptile reviewers flagged this correctly.

Removed the two fields. Abort/timeout is still surfaced via `cause:
describeError(err)` (an aborted run shows `name: 'AbortError'` / the timeout
message), so no diagnostic signal is lost.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(tables): drop in-process retry, keep cause diagnostics only

In-process retry is the wrong layer for this path: the cell task is
maxAttempts:1 by design, retrying on a possibly-degraded worker may not help,
and it masks the very transient-failure signal we're trying to capture before
we understand the root cause. Removed retryTransient entirely (file + all
wrapping in cell-write, the cascade reads, and the lock acquire) and kept only
the diagnostic logging.

- Deleted lib/table/retry-transient.ts (+ test); cell-write and the cascade
  reads call getTableById/getRowById/updateRow directly again, fail-fast.
- Kept describeError + `cause`/`retryable` fields across the cell + finalization
  catch blocks; the cell-path `retryable` flag now sources from
  isRetryableInfrastructureError (the canonical classifier) for consistency.

Diagnostics-first: surface the real driver cause on the next recurrence, then
decide the actual fix (e.g. task-level maxAttempts, or addressing the worker-
side cause) from evidence rather than a speculative in-process retry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(schedules): log error cause on scheduled-execution failure paths

The scheduled-job failure paths logged the raw error (.message/stack only) —
its `.cause` (the real driver error behind a Drizzle "Failed query: ..."
wrapper) was never recorded, and the classified-only
`describeRetryableInfrastructureError` returns undefined for unrecognized
errors. A real failed run (same incident window as the cell failures) failed in
`applyScheduleUpdate` with exactly this unrecorded cause.

Added `cause: describeError(error)` (always-on, walks the cause chain) to the
applyScheduleUpdate catch, the early-failure catch, and the unhandled-error
catch — passed as a second arg so the existing message+stack still emit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(errors): move describeError to @sim/utils/errors

`describeError` is a general-purpose error/cause-chain helper — it didn't
belong in `lib/core/errors/retryable-infrastructure.ts` (that module is
specifically about classifying retryable infra errors, and the name read wrong
for a generic diagnostic). Moved it to `@sim/utils/errors` alongside `toError`/
`getErrorMessage`/`getPostgresErrorCode`, with its own cycle-safe cause walk.

- Added describeError + DescribedError + tests to packages/utils/src/errors.ts.
- Reverted the describeError addition from retryable-infrastructure.ts (it keeps
  only isRetryableInfrastructureError / describeRetryableInfrastructureError,
  which are accurately named and still used by the schedule retry path).
- Re-pointed all consumers (cell, logging-session, pause-persistence, schedule)
  to import describeError from @sim/utils/errors. The `retryable` classification
  flag still sources from isRetryableInfrastructureError where used.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-03 17:57:06 -04:00
Waleed 8d7bbbc670 chore(utils): migrate to shared random/ID utilities and add enforcement linting (#4623)
* chore(utils): migrate to shared random/ID utilities and add enforcement linting

- Replace all Math.random(), crypto.randomUUID(), crypto.randomBytes(), nanoid, and uuid usages with shared @sim/utils/random and @sim/utils/id helpers across 72 files
- Add new @sim/utils exports: deepClone, omit, filterUndefined (object), truncate (string), backoffWithJitter, parseRetryAfter (retry), getErrorMessage (errors)
- Sweep all getErrorMessage, sleep, deepClone callsites across 500+ files to use shared utilities
- Add Biome noRestrictedImports rule to catch nanoid, uuid, and crypto named imports at lint time
- Add scripts/check-utils-enforcement.ts to catch Math.random and crypto.* global property access
- Add check:utils script to package.json

* chore(utils): replace deepClone wrapper with structuredClone built-in

deepClone() was a one-line wrapper around structuredClone(), which is
universally available in Node 17+ and all modern browsers. Removing the
abstraction reduces indirection and means contributors don't need to
learn a project-specific name for a well-known built-in.

- Remove deepClone from packages/utils/src/object.ts and index.ts
- Replace all 17 call sites with structuredClone() directly
- Update check:utils script suggestion text
- Update CLAUDE.md and global.md docs

* fix(utils): add missing biome noRestrictedImports rule and correct truncate docs

- Add noRestrictedImports to biome.json under style — bans nanoid and uuid
  package imports at lint time (crypto.randomUUID/randomBytes are caught by
  the check:utils grep script which handles global property access)
- Correct truncate() TSDoc and parameter name: sliceLength makes it clear
  that total output length is sliceLength + suffix.length, matching the
  behavior all callers were already written to expect

* fix(utils): add missing getErrorMessage imports at 4 call sites

The sweep agents added getErrorMessage calls without the corresponding
import in 4 files, causing test failures. Added the missing imports.

* fix(utils): fix build errors from getErrorMessage sweep and retry.ts Turbopack issue

- Fix retry.ts cross-file import: Turbopack cannot resolve './random.js' for
  internal package imports; inline the jitter crypto call directly
- Add missing getErrorMessage imports to 32 files where the sweep added calls
  without the corresponding import (caught by type-check and test runs)
- Remove accidental getErrorMessage import from crowdstrike/query/route.ts
  which has its own domain-specific getErrorMessage for parsing CrowdStrike's
  JSON error format
- Fix use-sub-block-value.ts type error from structuredClone narrowing:
  add 'as T' cast at emitValue callsite (safe — valueCopy is always a
  structural copy of newValue)

* fix(tools): use toError in crowdstrike catch block instead of local getErrorMessage

The catch block was calling the local getErrorMessage function which
parses CrowdStrike API JSON responses, not JavaScript Error objects.
Use toError(error).message to correctly extract the message from a
caught value in this context.
2026-05-15 17:31:27 -07:00
Vikhyath Mondreti aee6189d14 improvement(access-control): migrate to workspace scope (#4244)
* improvement(access-control): migrate to workspace scope

* fix edge cases

* update docs

* prep merge

* regen migrations

* address comments

* add ws id, user constraint

* address more comments

* address ui comments

* address more comments
2026-04-21 15:53:17 -07:00
Waleed b5674d9ed4 improvement(codebase): centralize test mocks, extract @sim/utils, remove dead code (#4228)
* improvement(codebase): centralize test mocks, extract @sim/utils, remove dead code

* improvement(codebase): apply @sim/utils conventions to staging-introduced files
2026-04-18 14:39:03 -07:00