73dd30d4e32279492d1ce6978b992bc0a74758d4
7933 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Claude
|
73dd30d4e3 |
Merge remote-tracking branch 'origin/main' into fix/waitpoint-completion-db-error-mislabel
# Conflicts: # internal-packages/run-engine/src/engine/systems/waitpointSystem.ts |
||
|
Daniel Sutton
|
0205feda39 |
refactor(run-engine): extract a WaitpointCoordinator seam around the Postgres waitpoint implementation (#4753)
Extracts every Postgres waitpoint and edge operation out of
`WaitpointSystem` into a `WaitpointCoordinator` seam with one Postgres
implementation, so a different coordination backend can be plugged in
later without any caller changing.
Pure refactor. Zero behaviour change, and zero test-file diffs — the
existing engine corpus is the characterisation test.
## What moved
`WaitpointCoordinator` (`waitpointCoordinator/types.ts`, declared with
`type`) has nine members: `clearRunBlockState`, `readRunBlockState`,
`registerBlocks`, `registerBlocksLockless`, `complete`,
`createDateTimeWaitpoint`, `createManualWaitpoint`,
`mintAssociatedWaitpointData`, `createAssociatedWaitpoint`.
`LegacyPostgresWaitpointCoordinator` implements them against the run-ops
store. Its dependencies are `{ runStore, prisma, logger }` only, so it
structurally cannot reach the run lock, the worker, or the event bus —
orchestration stays in `WaitpointSystem`, which keeps all ten public
signatures, all six `worker.enqueue` sites, the racepoints, the snapshot
transitions, and the event emissions.
Two register methods rather than one with a flag, so "the batch path
issues no extra query" is structural instead of conditional. Both share
one private edge-write helper.
## Six notes for reviewers — please read before "simplifying" any of
these
1. **`nanoid(24)` is called twice with different values on purpose**, in
each create path: once for the upsert `where` key, once for
`create.data`. Hoisting either to a shared constant makes the where-key
match the create-key, turning a guaranteed-miss upsert into a possible
update. In `createManualWaitpoint` both calls plus
`WaitpointId.generate()` stay *inside* the retry loop so each attempt
tries a fresh key.
2. **The two enqueue conditions are deliberately asymmetric.** DATETIME
enqueues `finishWaitpoint` unconditionally after a non-cached create,
with `availableAt: completedAfter`. MANUAL enqueues only when `timeout`
is set. That is existing behaviour, not an oversight. The coordinator
returns a discriminated union on `kind` rather than a boolean so the
enqueue is structurally unreachable on the cached path.
3. **One false clause was deleted from a moved comment.** The old
comment on the full-clear delete claimed the caller's `tx` is not
forwarded. The code does forward it, and `PostgresRunStore` uses `tx ??
this.prisma`, so a single store joins the caller's transaction — only
the routing store strips it. The rest of that comment is unchanged.
4. **The MANUAL timeout enqueue now sits outside the P2002 retry loop.**
Safe because the worker is Redis-backed and cannot raise
`Prisma.PrismaClientKnownRequestError`, so the loop never retried on it.
**If a Postgres-backed enqueue is ever swapped in, that equivalence
breaks silently.**
5. **The coordinator caches `runStore`/`prisma`/`logger` at
construction**, where the old code read `this.$.*` per call. Equivalent
only because nothing reassigns them: one assignment at
`engine/index.ts`, and the `resources` object is a `const` that is never
mutated.
6. **Two comments in other files are now stale and were left alone** —
`engine/index.ts` and `completeWaitpointCrossSeamGuard.test.ts` both
describe routing as the first statement of
`waitpointSystem.completeWaitpoint`. Both tests still pass, because that
guard sits in `index.ts` before the delegation. Left untouched to keep
this diff to three files.
## Preserved verbatim
The `unnest` edge CTE rather than a `Waitpoint` join; the pending count
as a separate statement after the edge write (READ COMMITTED needs its
own snapshot); completion's `findWaitpointOnPrimary` re-read through the
*resolved handle* while the blocked-run fan-out goes back through the
*router*; the residency and colocate hints, with colocation objects
built only in the Postgres arm and the count keeping its `runId`
argument; `ON CONFLICT DO NOTHING` and the `(taskRunId, waitpointId,
batchIndex)` multi-index edge semantics; the unread `batchId` select,
which rides inside two `logger.debug` payloads.
`internal-packages/run-store/` is untouched, so the CTE and the conflict
semantics never moved.
## Verification
| Check | Result |
| --- | --- |
| Engine corpus | 61/61 files, 353 passed, 1 skipped, **0 failed**
(baseline: 352 passed, 1 failed) |
| Test-file diffs | **empty** |
| `run-engine` typecheck | `tsc --noEmit -p tsconfig.build.json` exits 0
|
| `webapp` typecheck | 146 errors on this branch, **146 identical errors
at baseline** — pre-existing, none added |
The webapp typecheck does not pass. The failures are pre-existing
(`PrismaPg` not assignable to `never`; missing `@trigger.dev/rbac`
exports) and the sorted error lists are byte-identical to the merge
base, so this branch adds none — but the criterion is genuinely unmet
and needs a separate fix.
No changeset and no `.server-changes` note: internal refactor with no
user-visible change.
## Follow-ups this surfaced
- The dominant RUN waitpoint is still created outside the seam —
`buildRunAssociatedWaitpoint` now mints through the coordinator, but the
row is inserted nested inside `createRun`/`createFailedRun`. That needs
its own packet before a second backend lands, or the commonest waitpoint
gets split across two of them.
- `clearRunBlockState` overloads opposite outcomes on `undefined` versus
`[]`: `undefined` clears every edge, `[]` clears none. Both callers are
correct today; worth splitting when the file is next touched.
- A stray non-`.sql` entry in `internal-packages/clickhouse/schema/`
breaks every `containerTest` in the repo, because the testcontainers
migration reader `readFile`s every `readdir` entry without filtering
despite a comment claiming it filters. Hit this during setup; unrelated
to this change and left for a separate fix.
|
||
|
Daniel Sutton
|
73f86c7af1 |
fix(webapp): stop saving global flags from unsetting the locked ones (#4751)
## Summary
On a self-hosted instance, saving anything on the global admin feature
flags page also deleted the two read-only flags,
`defaultWorkerInstanceGroupId` and `taskEventRepository`. Losing the
first one leaves deployed runs with no default worker group. Neither
deletion showed up in the confirm dialog, so the flags disappeared
silently.
## Root cause
The page submits only the flags its UI is managing, and strips the
read-only ones from the payload unless "Unlock read-only flags" is
ticked. The action treated every catalog key absent from that payload as
"the admin unset this", and protected the locked keys only when the
instance was managed cloud. Anywhere else, both locked rows fell
straight into the delete sweep.
The protection now keys off what the client says it was editing rather
than off the deployment:
```ts
const canDeleteLocked = params.unlockLockedFlags && !params.isManagedCloud;
...
} else if (canDeleteLocked || !GLOBAL_LOCKED_FLAGS.includes(key)) {
keysToDelete.push(key);
}
```
Exactly one case changes: a locked flag, on a non managed-cloud
instance, with the flags not unlocked, is now kept instead of deleted.
Managed cloud behaviour is bit for bit identical, and ticking the unlock
box still gives a self-hosted instance full control. The write moves
into `replaceGlobalFeatureFlags` so it can be driven directly in tests
against a real Postgres.
|
||
|
nicktrn
|
b082e44389 |
fix(webapp): write-path and appearance-control fixes for the theme work (#4756)
Fixes found while reviewing #4547, stacked on that branch so they can be reviewed on their own and merged into it. One commit per fix. ## Write-path correctness **Refuse account writes while impersonating.** The five `dashboardPreferences` writers already no-op for an impersonating admin, but the three profile writers added next to them did not, and `requireUserId` returns the impersonated user's id. Both gates now refuse up front and say so, rather than the preference writers silently no-opping while the page reports success. **Preserve unknown keys on a full-blob write.** `mutateDashboardPreferences` parses the JSON column, hands the result to a mutator and persists the whole object back. zod strips keys it does not declare, so a deploy that predates a preference field drops it on the next write through that path — and `updateCurrentProjectEnvironmentId` sits on the navigation hot path. `preserveUnknownKeys` re-attaches them at the write. Note this cannot help deploys already running, so it makes this the last release able to strip rather than retroactively protecting the fields added in #4547. **Scope hidden-sidebar writes to what was shown.** The customize dialog builds its hidden map from the sections it can see and the write replaced `hiddenItems` wholesale. The profile page has no org in scope, so it resolves sections from the most-recently-updated project's org: confirming there dropped hidden ids belonging to sections that org's flags exclude. The payload now carries the ids the dialog rendered and the write only replaces those. Submissions without the list stay authoritative. **Consider both addresses when checking email ownership.** The check only looked at the address the user already had; it now considers the current and submitted address together, so an org managing either one governs the change. Validation moved ahead of the check, and `emailDomainOf` splits on the last `@`. ## Interaction **Revert unsaved themes, debounce contrast saves.** The theme and system-theme selects stamp `data-theme` before the write lands. When it fails, the loader returns the value it always had — so `useSystemThemeSync`'s effect deps are unchanged and React's vdom diff sees no change either, and nothing rewrites the attribute. The page kept rendering a theme that was never stored while the select showed the stored one. The stored pair is now re-applied explicitly, as the side menu's switcher already did. The contrast slider is debounced because Radix commits on every arrow keypress, so a keyboard user crossing the range fired one write per step. **Tick More options for themes outside the short list.** The appearance submenu offers System, Light and Dark; Black and White live on the profile page. With one of those stored, every row read as unselected. ## Subtraction **Drop the profile update rate limiter.** It covered one of four paths that write the same column — `resources.preferences.sidemenu` and `.favorites` take unlimited authenticated writes and go through the locked read-modify-write, which is more expensive than the single narrow `jsonb_set` this capped. It was also what made the contrast slider unusable by keyboard. If preference writes want limiting, it belongs in one place covering all of them. **Resolve email ownership when the dialog opens.** It fans out one SSO status lookup per organization the user belongs to and ran in the profile loader on every page view, purely to pick which body the dialog renders. The action re-derives it before writing either way, so the check that guards the write now has one call site instead of two. ## Testing `typecheck --filter webapp` and `lint` clean. New unit tests for `preserveUnknownKeys`, `mergeHiddenItems` and `emailDomainOf`; `themePreference`, `mergeHiddenItems` and `ssoManagedIdentity` suites pass locally (26 tests). The rest of the webapp suite needs testcontainers and is left to CI. No changeset or `.server-changes` entry: everything here fixes code on the parent branch that has not shipped. The one exception worth a maintainer's call is `mergeHiddenItems`, which also touches the side menu's own customize path. |
||
|
James Ritchie
|
4c5237ca4a |
feat(webapp): themes refinement, new black & white themes, 2 accessibility toggles (#4547)
## What this does Rounds out the theme work behind the existing `hasThemeSwitcher` flag. **Two new themes.** Black and White sit alongside Dark and Light. They inherit their neighbour's whole token set and only pin their surfaces flat, so sections are separated by grid lines rather than layered fills. **`System` is now configurable at both ends.** You choose which theme the OS light setting lands on (Light or White) and which the dark setting lands on (Dark or Black). **Two accessibility toggles.** - *Stronger colors* — swaps tinted status chips for solid fills, drops decorative icon accents to monochrome, and darkens chart series that didn't clear 3:1 on a white plot. - *Underline links* — underlines body-text links, so an underline always means the preference is on rather than being a hover style. **Contrast slider.** Stores a 0–100 position within the active theme's own range rather than a shared scale, so 35% stays 35% when you switch themes. Each theme maps it in CSS, which keeps `system` working before hydration. **Appearance in the account popover.** A submenu listing the themes with a check against the current one, plus a link through to the full set on your profile. Picking one applies immediately rather than waiting for the write to round-trip. **Profile page.** Each row now saves on its own — no submit button. Name and email show their value inline with an edit button; the email row is read-only when an identity provider owns the address. **A `/storybook/colors` audit page.** Renders every colour-carrying pattern in the app once per theme plus once under Stronger colors, and measures contrast ratios off the live DOM rather than a hard-coded table, so it can't go stale. --- ## Demo https://github.com/user-attachments/assets/d56cd4d8-719f-4ec5-a990-e04cdb98def1 --- ## Compatibility The stored preference shape is unchanged (`version: "1"`), and the four new fields are all optional. The retired `classic` theme falls back to Dark, whose palette at contrast 0 is what Classic shipped. One deliberate change worth knowing: the default contrast moves from 50 to 0, so existing users who never touched the slider will see slightly less contrast than before. That's what makes 0 mean "the base palette". --- ## Testing Switched between every theme from both the account popover and the profile page, in the expanded and collapsed rail, checking `data-theme` follows and survives a reload. Dragged the contrast slider in each theme and confirmed the percentage label tracks the handle and resnaps if a save fails. Checked both accessibility toggles across the `/storybook/colors` page, which is also where the contrast ratios were read from. Confirmed the Appearance entry stays hidden for a non-admin while the flag is off. <!-- conductor-workspace-link --> --- [Open workspace in Conductor](https://app.conductor.build/workspace/fee50611-7623-4422-bada-ed1cba317ed1) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Daniel Sutton
|
dd3a1c0c54 |
feat(run-store): Redis-backed store for the run execution-state log (#4754)
Adds `RedisSnapshotStore` to `@internal/run-store`: a Redis-backed,
append-only store for a run's execution-state log, as an alternative to
keeping that log in Postgres.
Nothing constructs it. No existing code path can reach it, so merging
this changes no behaviour. The store, the wiring that would use it, and
the switch that would enable it are deliberately separate changes.
## Design
Four keys per run, plus one key per wait cycle, all sharing a `{runId}`
hash tag. Every mutation for a run therefore lands in one cluster slot,
and each operation is a single Lua script.
No script mints a key name. Dynamic keys are derived from `KEYS[1]` by
string surgery, because ioredis applies `keyPrefix` only to the KEYS
array: a key built inside Lua would be unprefixed while the client wrote
a prefixed one.
Retention is keyed to run completion. A non-terminal run's keys carry no
expiry at all, since a suspended run can wait indefinitely with nothing
left to refresh a TTL. The terminal transition sets the completion
expiry once, and a write arriving after completion re-applies that same
expiry rather than a live one, so a stale client cannot resurrect a key.
Entry JSON round-trips byte for byte. No script calls `cjson`, and the
values the store assigns itself live in their own hash fields instead of
being patched into the caller's document.
Sizes are observed, never enforced. Entry and cycle-key bytes are
recorded, with a warning above a configurable mark. Nothing rejects,
truncates, or spills.
`append` takes an optional expected-current-snapshot argument. Left out,
it advances the pointer unconditionally, matching the Postgres behaviour
it replaces. Supplied, it advances only on a match and otherwise reports
the conflict without writing.
Covered by 48 tests against a real Redis container, including the
retention transitions, the single-slot guarantee under a key prefix, and
tenant-scoped reads.
|
||
|
Oskar Otwinowski
|
910011d44e |
feat(vercel): automatic version skew protection at connect + atomic deployments deprecation (#4741)
Connecting a Vercel project now writes TRIGGER_AUTOMATIC_SKEW_VERSION_PROTECTION=1 (plain, create-if-absent only - an existing value, including "0", is never touched; presence is target-containment aware, branch-scoped records do not count, a truncated env listing skips the write). The onboarding wizard no longer offers automatic atomic deployments (default off); the settings row is labelled Deprecated and enabling it requires confirming a dialog that points to task version skew protection and the docs (TRI-13001). |
||
|
Oskar Otwinowski
|
b98cceb8fb |
docs: task version skew protection, --external-id, and the atomic deployments deprecation (#4742)
New deployment/version-skew-protection page: the skew problem, the --external-id primitive and its reuse behaviour, runtime discovery (call option, configure(), TRIGGER_EXTERNAL_DEPLOYMENT_ID, and the gated platform/CI/generic commit-SHA variables with the build-time caveat), the manual any-platform recipe, waiting/expiry semantics, precedence, and automatic skew protection on Vercel. Deprecation callouts on the atomic deployments page and the Vercel integration page; --external-id/--force added to the CLI deploy reference; redirect from deployment/vercel-skew-protection so existing webapp links resolve (TRI-13002). |
||
|
Eric Allam
|
32bf745c02 |
feat(webapp): customizable runs list with columns and smart columns (#4652)
## Summary Makes the runs list customizable. A new **Display** control lets you show, hide, and reorder columns, and add **smart columns** that pull a single value out of a run's payload, metadata, or output by JSON path (e.g. `$.failed`, `$.order.total`). Column choices live in the page URL, so a view can be bookmarked or shared. Applies to the global runs list and every per-task / scheduled / agent / webhook / error list, which all share one table. ID, Task, and Status can be reordered but not hidden. Smart columns are display-only (no sort or filter, which would defeat the ClickHouse sort key and cursor). ## How it works Columns come from a shared registry; the Postgres `select` is derived from the visible columns, so a run's large payload/output are only hydrated when a smart column actually references them. All JSON parsing for smart columns happens client-side, respecting the packet content type, parsed once per source per row. Offloaded (too-large) values and paths that aren't present render distinct placeholders rather than fetching per row. The live poll carries the same sources so smart-column values update in place. Scalar columns stay always-selected for now: the shared list presenter has a fixed output shape consumed by several routes and the live poll, and narrowing individual scalar fields would add no real query cost benefit on a single-row read. The select derivation is already column-driven, so tightening this later is a one-line change. ## Screenshots <img width="590" height="1028" alt="CleanShot 2026-08-21 at 16 48 17@2x" src="https://github.com/user-attachments/assets/86b39856-bfcc-47c0-85ed-ee6ccddc3590" /> <img width="1924" height="1528" alt="CleanShot 2026-08-21 at 16 48 27@2x" src="https://github.com/user-attachments/assets/6c766249-6d5b-45be-9330-c6caa75af7f7" /> <!-- conductor-workspace-link --> --- [Open workspace in Conductor](https://app.conductor.build/workspace/d6911080-2140-4de1-b88a-1b0623593caa) --------- Co-authored-by: James Ritchie <james@trigger.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Daniel Sutton
|
aa9b888988 |
refactor(run-store): hold RoutingRunStore's stores in a keyed shard map (#4752)
## What `RoutingRunStore` held two named store fields, `#new` and `#legacy`, and took its routing policy from the order the statements happened to run in. It now holds a `Map<ShardKey, RunStore>`, and the three policies that were implicit are readable data: - **`#probeOrder`** (`new` → `legacy`) — the sequential probe for a lookup with no routable id. The first non-null result wins, and the *last* entry owns the canonical not-found throw. - **`#precedence`** (`legacy` → `new`) — ascending authority for a merge, so the highest-authority shard wins a duplicate id. - **`#idlessRouteShard`** (`new`) and **`#idlessWaitpointShard`** (`legacy`) — the two id-less defaults, which differ by role and were previously two unrelated literals in unrelated methods. The two orders are the **reverse of each other**, which is why they are separate fields rather than one ordering. Nine sites observe the result-array order and must iterate `#probeOrder`; five decide a value by which shard wins a duplicate and must iterate `#precedence`. Five more sum counts and are order-independent, because addition commutes. Four helpers absorb the twenty-six hand-written fan-outs — `#probeFirst`, `#fanOut(order, fn)`, `#fanOutPartitioned`, `#shardsExcept` — and `#shardKeyOf` replaces the inline residency-to-store ternaries. `#fanOut` takes its order as an argument so every call site states which policy it uses. The constructor keeps its exact options type. No union arm, no `shards` member: that would loosen the excess-property check and silently retire the `@ts-expect-error onLegacyRead` lock in the test corpus. N-way construction is a later change. ## One behaviour change `findManyTaskRunWaitpoints` merged its edge rows NEW-first into a last-wins dedupe, so a duplicate edge id resolved to the **legacy** row — the opposite of the rule the other four merges follow, and the opposite of what `dedupeEdgesById`'s own comment claimed. No test pinned it in either direction. It now resolves NEW-wins, consistent with every sibling merge, and a new test pins the winner so it cannot drift back silently. Reaching this case needs one edge id present on both stores at the same time, with no routable `taskRunId`. That only arises from drain mirroring. The drain seam is removed (`runOpsStore.test.ts`, "fan-out spans NEW+LEGACY with no drain seam"), so **no new duplicates can be created** — but removing the code does not delete rows it previously wrote, and this class still carries comments treating mirrored rows as a live data condition. Whether any historical duplicate edge rows persist is an empirical question about production data, not something this diff settles. If such a row is hit, the two copies either agree — in which case the winner is immaterial — or they have diverged, in which case NEW is the authoritative copy by the router's own precedence rule. So the corrected behaviour is at least as correct as the old one in every reachable case. Everything else is behaviour-preserving. ## How it was verified - **`internal-packages/run-store`: 69 files, 379 tests pass.** The corpus is the regression gate for this refactor. 67 of the 68 pre-existing test files are byte-identical; the one that differs (`runOpsStore.mixedResidency.test.ts`) changes only `//` comments. - **`internal-packages/run-engine`: 12 files, 69 tests pass** — every file that constructs the router, exercised at runtime. - **The `@ts-expect-error onLegacyRead` lock still fires.** `tsconfig.build.json` excludes `*.test.ts`, so a green typecheck does not cover it. A scratch probe confirmed `tsc` still reports `TS2353` for `onLegacyRead` and no error for the three real options. - **All 48 construction sites outside the package compile unchanged.** `tsconfig.check.json` also excludes `*.test.ts`, so the 25 webapp test files were checked with the test exclusion dropped and compared against the same check on the base commit: 614 errors before, 614 after, zero present in one and not the other. Those 614 are pre-existing in never-typechecked test files. - `typecheck` passes for `run-store`, `run-engine` and `webapp`. `knip` reports nothing in `run-store`. ## Also Refreshes the sixteen stale `runOpsStore.ts` line references in `runOpsStore.mixedResidency.test.ts`, each verified against the symbol it names. ## Notes for the reviewer - The riskiest possible mistake in this diff is a fan-out passing the wrong order — the compiler cannot catch it, because both orders are `readonly ShardKey[]`. The five `#precedence` sites are `#findRunsOpen`, `findRunsByIdempotencyKeys`, `#collectManyWaitpoints`, `findManyTaskRunWaitpoints` and `findManyWaitpointTags`. Those are the lines worth the closest read. - Four sites previously derived "the other store" by object identity (`home === this.#new ? ...`). They now compare keys. The two are equivalent: in single-database mode both keys map to the same store object, and when the stores are distinct, identity and key comparison agree. - No changeset and no `.server-changes` note: the package is internal and the one behaviour change is unreachable in production, so a release note would tell a user nothing. - Two CI checks fail for reasons that predate this branch and reproduce on the base commit: `lint` (~16 unknown `react/*` rules make `.oxlintrc.json` fail to parse, which disables oxlint entirely — including the two `trigger-runops` fences) and `knip` (`unrun`, an unused devDependency on the default branch). Both want their own fix. |
||
|
Daniel Sutton
|
c5c2ea92ca |
feat(core): add shard-routable run-ops id format and resolveShard (#4750)
## Summary
Adds a second generation of run-ops id, plus the resolver that reads a
store key straight out of an id. A gen-2 id keeps the existing
26-character layout, but the character at index 24 becomes a routing
shard key instead of a region code, and the version character at index
25 becomes `"2"`. Nothing mints gen-2 ids yet, so this is inert on
merge.
## Design
The version character is a single character, so the gen-1 and gen-2
shape checks can never both match. That is what makes the two
generations provably disjoint rather than disjoint by convention.
```ts
resolveShard(id) // gen-2 body -> its shard key, [a-z0-9]
// gen-1 v1 body -> "new"
// anything else -> "legacy"
```
`resolveShard` is total: it returns a key for any input string,
including an empty or malformed one, and never throws.
`classifyResidency` keeps its signature and its two values, and now
reports gen-2 ids as part of the dedicated family, so existing consumers
of that boolean are unaffected.
The body stays 26 characters rather than 27 deliberately. The older
27-character format is still in the wild and has to keep resolving to
legacy, and a longer gen-2 shape would need probabilistic disambiguation
against it. A rare misroute is not an acceptable property for a routing
key.
The one behavior change is that a 26-character body ending in `"2"` now
routes by its shard key instead of falling back to legacy. Two test
assertions pinned the old result and are updated here. A repository-wide
search confirms they are the only two of their kind.
Verified against the full run-store corpus (68 files, 370 tests) with no
test-file changes there, plus the run-engine residency and waitpoint
suites. No changeset: the new surface has no caller, so a version bump
would tell a user nothing.
|
||
|
Eric Allam
|
60d71da90e |
perf(webapp,run-engine): cut CPU on the engine-facing worker-action routes (#4746)
Cuts CPU on the `engine/v1/worker-actions/*` routes a managed supervisor calls, and adds the benchmark harness the numbers come from. Measured on a local stack: **on-CPU per completed run 9.07ms → 6.59ms (−27%)**, busy fraction 45.6% → 33.8%, with every worker-action p50 down 23–27%. Load was 5,000 runs / 24 virtual supervisors / 90s window / 30,120 requests / 0 errors. Query-count work from the same investigation is deliberately **not** here — it will follow as a separate PR. ## The three changes **1. Split the event-loop monitor in two (~14% of on-CPU, plus ~5pp of GC).** `eventLoopMonitor.server.ts` installs a global `async_hooks` hook: `init` writes a `Map` entry for *every* async resource the process creates, `before` calls `process.hrtime()` and `context.active()` on every one. Enabling any async hook also puts V8 on the slow path for promise instrumentation process-wide. `EVENT_LOOP_MONITOR_ENABLED` defaulted to `"1"`, so this was the shipping configuration. The blocked-loop detector is now opt-in (`EVENT_LOOP_MONITOR_ENABLED`, default `0`). The event-loop *utilization* gauge — a single interval timer with no per-request cost — moves to its own flag (`EVENT_LOOP_UTILIZATION_MONITOR_ENABLED`, default `1`) and stays on, so the useful half survives without the expensive half. A/B under identical load: | | monitor on | monitor off | change | |---|---|---|---| | on-CPU per run | 9.08ms | 7.25ms | −20% | | GC self time | 9.80% | 5.05% | −4.75pp | | dequeue p50 | 76.6ms | 62.8ms | −18% | | attempts/start p50 | 56.3ms | 43.5ms | −23% | **2. Bucket route matching by first static path segment (10.4% → 3.9% of on-CPU).** `patches/@remix-run__router@1.23.3.patch` already memoized flattened branches and compiled path regexes. What remained was the linear scan: `matchRouteBranch` walked the ranked branch list calling `matchPath` per branch across 521 route files, so every worker-action request paid a scan proportional to the whole route table. Branches are now indexed by their lowercased leading segment, with one always-considered list for branches whose leading segment is dynamic, splat or optional (and for root/pathless paths). A request walks only its own bucket merged with that list. Route-matching self time dropped 64% (3.6s → 1.3s over a 90s window). Ordering is preserved exactly: both lists hold indexes into the already rank-sorted branch array and are walked in ascending-index order, so the first match found is the same branch the full scan would have found. Bucketing lowercases on both sides, so case-insensitive matching still resolves and `caseSensitive: true` routes are still rejected by `matchPath` itself. A pathname whose own leading segment can't be bucketed falls back to the full scan. Verified equivalent to the unpatched matcher over 20,050 pathnames (literal, dynamic, splat, optional, case variants, basenames, percent-encoded) with zero mismatches. `apps/webapp/test/routeMatchingPatch.test.ts` pins the matching semantics rather than the optimisation, so it still passes without the patch. **3. Demote per-heartbeat and per-dequeue `info` logs to `debug`.** These are the two highest-rate engine calls and each wrote a synchronous structured log line on every request. Synchronous `console` writes can block the loop when stdout backs up, which costs more than the ~1.3% CPU share suggests. ## The harness Two benchmarks, neither in the default suite (they run for minutes, attach the V8 profiler, and report numbers rather than assert on them). See `apps/webapp/test/bench/README.md`. - `apps/webapp/test/bench/engineHttp.bench.test.ts` — spawns a real webapp against throwaway Postgres/Redis containers, seeds a production environment with a promoted managed deployment, and drives a closed-loop supervisor pool through the full lifecycle. Profiling runs over CDP rather than `--cpu-prof` so it covers only the measured window instead of being swamped by boot, and `performance.eventLoopUtilization()` is sampled *inside* the webapp process. - `internal-packages/run-engine/src/engine/bench/runEngineLifecycle.bench.test.ts` — drives `RunEngine` directly, profiling enqueue and lifecycle separately so engine cost isn't mixed with request-stack overhead. - `apps/webapp/test/bench/analyzeProfile.ts` — dependency-free `.cpuprofile` analyzer that symbolicates through the build's source maps and ranks CPU by package, self time and total time. Percentages are shares of on-CPU time (V8's `(idle)`/`(program)` excluded). `startWebapp` gains `overrideEnv`, applied after the worker-disable defaults, so the HTTP bench can re-enable the run engine worker that drains the master queue into the worker queues a supervisor dequeues from. The local OTel collector gains a traces pipeline. It only defined a metrics pipeline, so pointing `INTERNAL_OTEL_TRACE_EXPORTER_URL` at it locally failed and the webapp silently fell back to the console span logger. ## Configuration For operators upgrading: - `EVENT_LOOP_MONITOR_ENABLED` (now defaults to `0`) — the per-async-resource blocked-loop detector. Set to `1` to restore the previous behaviour and keep emitting `event-loop-blocked` spans. - `EVENT_LOOP_UTILIZATION_MONITOR_ENABLED` (new, defaults to `1`) — the `nodejs.event_loop.utilization` gauge. Unchanged in behaviour; it just has its own flag now so it survives turning the detector off. ## Notes for review - `pnpm-lock.yaml` changes only because the router patch content changed, which changes its patch hash. - One thing the profile ruled out: with a real OTLP collector receiving spans, tracing costs ~1.7% of on-CPU at 100% sampling and ~0.8% at the production rate. Span shipping is not a hidden cost, so nothing here touches it. - Caveats on the numbers: a laptop, not production hardware, so DB and Redis *latency* are unrepresentative (client-side CPU is what's ranked); single webapp process; throughput varies ~5% run to run, which is why the claims rest on on-CPU per run rather than req/s. ## Verification - 20,050-pathname router equivalence check vs the unpatched matcher, zero mismatches - `apps/webapp/test/routeMatchingPatch.test.ts` (12 cases) passes - webapp e2e smoke suite (68 tests) passes through the patched router - run-engine suites covering the snapshot/attempt paths pass - `typecheck`, `format`, `lint`, `knip` clean |
||
|
claude[bot]
|
4953128c10 |
chore: vouch wuweiweiwu (#4748)
Adds `wuweiweiwu` to the vouched-contributors list. Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Daniel Sutton <45313566+d-cs@users.noreply.github.com> |
||
|
James Ritchie
|
2efb07e0b1 |
Toggle switch feels nicer to toggle (#4749)
Two small tweaks to the `Switch` primitive, so every variant and call site picks them up: 1. **Track is 2px shorter.** `large` 44 → 42px, `medium` 32 → 30px, `small` 24 → 22px. The checked thumb travel drops by the same 2px so the thumb stays flush at both ends. 2. **Holding the switch down stretches the thumb into an oval** pointing the way it's about to travel — rightwards when off, leftwards when on. Pure CSS via `group-active:`, no new state or handlers. The thumb's `transition` shorthand doesn't cover `width`, so it's now `transition-[translate,width,background-color]` (same 150ms duration/easing as before). `size-N` on the thumb became `h-N w-N` so the press rule overrides the same `width` utility. Verified in headless Chrome across all five variants in both states: correct widths at rest, thumb flush at both ends, stretch grows the right direction, and no overflow of the track. <img width="266" height="108" alt="CleanShot 2026-08-21 at 10 16 14" src="https://github.com/user-attachments/assets/ee95a399-0a40-48c4-a325-a1166b3bd88a" /> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- conductor-workspace-link --> --- [Open workspace in Conductor](https://app.conductor.build/workspace/c1ce8d0f-9ed2-4fbc-8084-a3989484cc53) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DKP
|
d04467018e |
feat(webapp,database): save platform notifications as drafts and publish later (#4743)
## Summary The platform notifications admin page can now save a notification as a draft without committing to a schedule, then publish it later by entering start and end dates. Drafts stay hidden from the webapp panel, the CLI, and the "What's new" changelog until they are published. ## Design A draft is an `isDraft` flag on `PlatformNotification`, not nullable dates, so the existing index and every read query stay intact. All three reader queries filter on the flag, so a draft can never surface regardless of its placeholder dates. Publishing writes the real start and end dates and clears the flag; the publish dialog validates the range and shows inline errors. Editing a draft keeps it a draft, with the schedule fields hidden until publish. Also folds in a small tweak: the "Send preview to me" test button now appears when editing a notification, not just when creating one. |
||
|
Eric Allam
|
1034b618a4 |
fix(webapp): let an impersonating admin preview the Queue Metrics UI (#4736)
## Summary The Queue Metrics dashboard UI is gated by a per-org feature flag, so there was no way to look at it for a real org without turning it on for every member of that org. An admin impersonating into an org now sees the metrics UI there regardless of the flag, so it can be checked against real data before anyone else in the org sees it. Nothing changes for a normal session: a member of an org whose flag is off still gets the classic Queues page, and the gated sub-routes still 404. ## Design The gate had no request and only resolved the org flag. It now takes the request and resolves impersonation itself, rather than each caller computing a boolean and passing it in, so the rule lives in one place and a new call site cannot forget it. Seven call sites gate on this, which is exactly why. Two things narrow the bypass: - It keys on **impersonation**, not `user.admin`. Impersonation is scoped to one org and is deliberate; keying on admin status would silently hand every admin the preview in their own day-to-day orgs. - It yields to the **view-as-user** toggle. That toggle exists so an impersonating admin can see what the member sees, and unreleased UI leaking through it would make it lie. Suppressing a read-only view there stays inside the display-only contract in `hasAdminDisplayAccess` (added in #4421). The bypass also stays behind the gate's existing org-membership lookup. Since the acting user id is the impersonation target, that lookup is what keeps the preview confined to the org actually being impersonated into. Verified end-to-end against a running instance across the matrix: member with the flag off gets the classic view and 404s; the same org under impersonation gets the metrics view and a 200; flipping view-as-user returns it to the member's exact experience and back; and the flag-on path is unchanged. An admin who is merely a member, not impersonating, still gets the classic view. One thing worth flagging: a few route comments say that with the flag off no metrics reads fire. That remains true for every member session and for the org as a whole, but an admin actively previewing does exercise that org's real Redis and ClickHouse reads. That is inherent to previewing, and bounded to one admin session. |
||
|
Eric Allam
|
9baebbd1a6 |
fix(webapp): keep the dashboard agent's tool calls on the user's instance (#4740)
## Summary Follow-up to #4738. Splits the dashboard agent's base URL into two: the instance that hosts the agent project (used for sessions), and the instance the agent acts against as the user (used by its read-tools). #4738 only needed the first, but moved the second along with it, which breaks the tools when the agent runs on a different instance than the webapp. ## Root cause The agent's read-tools call the API as the logged-in user via a delegated user-actor token. The webapp signs that token with its own `SESSION_SECRET`, scoped to its own `userId` and `environmentId`, so it can only be verified by, and only resolves the user's data on, that same instance. #4738 routed the injected `apiOrigin` those tools use to the agent's host instance, so the token no longer verifies and the data isn't there. ## Fix `dashboardAgentApiOrigin()` stays the agent's host instance (sessions, task triggers, realtime, the `in` forward). A new `dashboardAgentUserApiOrigin()` returns the webapp's own origin (`API_ORIGIN ?? APP_ORIGIN`) and is injected into the run metadata the tools use. Same-instance deployments resolve both to the same host, so behavior is unchanged there. |
||
|
Eric Allam
|
56f875680c |
fix(webapp): let the dashboard agent use a configurable base URL (#4738)
## Summary Lets the dashboard agent point at a specific Trigger instance instead of assuming it runs on the same instance as the webapp. Adds an optional `DASHBOARD_AGENT_BASE_URL`; when unset it falls back to the SDK default. ## Root cause The agent's session start, token mint, head start, in-proxy and the client transport all built the agent's base URL from the webapp's own origin (`API_ORIGIN ?? APP_ORIGIN`). That only holds when the agent project runs on the same instance as the webapp. When it runs elsewhere, `DASHBOARD_AGENT_SECRET_KEY` belongs to that other instance, so the webapp's own API rejects it with an "Invalid API key" and the chat can't start. ## Fix `dashboardAgentApiOrigin()` now returns `DASHBOARD_AGENT_BASE_URL` or the SDK default, never the webapp origin. A concrete default (rather than an unset value) keeps it independent of `TRIGGER_API_URL`, which a webapp may point at a different host. Every server call site already routes through that helper; the client transport reads the value from the root loader via a new `useDashboardAgentBaseUrl` hook. |
||
|
claude[bot]
|
19eae515fd | fix: rename the Projects org settings URL to /settings/projects (#4739) | ||
|
Chris Arderne
|
4392e79ce2 | chore: adopt stable React Compiler lint rules (#4737) | ||
|
github-actions[bot]
|
ce40d0259f | chore: release v4.5.12 (#4610) helm-v4.5.12 v.docker.4.5.12 v4.5.12 | ||
|
Chris Arderne
|
06f99aeb31 | fix: security release 2026-08-12 (#4735) | ||
|
claude[bot]
|
518978bc52 |
fix(core): don't assume a 64-character idempotency key is pre-hashed on reset (#4626)
<!-- ccr-slack-attribution --> _Requested by **Matt Aitken** · [Slack thread](https://triggerdotdev.slack.com/archives/C045W9WM3E1/p1786741966214949?thread_ts=1786741966.214949&cid=C045W9WM3E1)_ `idempotencyKeys.reset()` now honours an explicitly passed `scope` even when the key material happens to be 64 characters long. **Before:** `resetIdempotencyKey` treated *any* 64-character string as an already-computed hash and sent it to the API verbatim. That short-circuit ran before the scope logic, so if your key material is itself a 64-character digest (a common pattern when you hash your own dedup identity) the `scope` you passed was silently discarded and the un-hashed material went on the wire. The server stores the hash, so the reset matched no run and returned 404 every single time. Key material of any other length worked fine, which made this look arbitrary. **After:** a 64-character key with an explicit `scope` is sent verbatim first and, only when that attempt comes back a definitive not-found, retried as the derived scope hash. Every call that worked before behaves identically, and the previously impossible case now resolves on the fallback. ## How A 64-character string is forwarded unchanged, exactly as before, when: - the idempotency key catalog recognises it (it came from `idempotencyKeys.create()` in this process), or - no `scope` was passed, so there is nothing to derive a hash from, or - the scope hash cannot be derived (e.g. `scope: "run"` outside a task context with no `parentRunId`). Otherwise the key is ambiguous: it may be raw material the caller wants hashed with the scope, or it may already be the stored hash. Reset sends the verbatim value first because that is what every previous version sent, so anything that resolved before still resolves with the same single request, the same target run, and the same errors. The derived hash is the new behaviour, so it only runs once the verbatim attempt has failed with a 404, a definitive "no run under this key". Any other error (a 503, a connection error) leaves the verbatim key's state unknown, and resetting a different key on unknown state would be an untargeted write the caller never asked for, so those errors surface unchanged. That has an honest cost: when the endpoint answers 503 for a miss it cannot confirm, the caller sees the 503 and retries rather than silently falling through to the derived key. When both attempts miss, the verbatim attempt's 404 is surfaced, again matching what previous versions threw. A side benefit of this order: a key from `idempotencyKeys.create()` reset with a `scope` from a cold process resolves in a single request, because the created key is itself the stored value. `isIdempotencyKey` is deliberately left alone: it applies the same length rule on the trigger path, but it is self-consistent there, and changing it would invalidate already-stored keys. The `attachedOptions?.key` / `attachedOptions?.scope` fallbacks below the old guard were unreachable (every catalog entry is a 64-character digest, so it always hit the short-circuit first) and re-deriving from them produces the identical hash anyway. They are removed rather than left as dead code. --- ## ✅ Checklist - [x] I have followed every step in the [contributing guide](https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md) - [x] The PR title follows the convention. - [x] I ran and tested the code works --- ## Testing Tests in `packages/core/src/v3/idempotencyKeys.test.ts` drive the real `resetIdempotencyKey` against a local HTTP server and assert on the exact values that reach the wire, in order. Nothing is mocked. They cover: - 64-character material + explicit `scope` derives the global- and run-scoped hash once the verbatim key misses (fails without this change) - the verbatim key wins when runs exist under both the verbatim value and the derived hash, so the pre-existing target is preserved - keys from `idempotencyKeys.create()` are forwarded unchanged: catalog hit, no scope, and scope with a cold catalog (the last now a single request) - a transient failure of the verbatim attempt surfaces its error without ever touching the derived key - error surfacing: a double miss reports the key the caller passed, and a non-404 from the fallback is not swallowed - ordinary short material is still hashed, and underivable run/attempt scopes still send a 64-character key verbatim while still throwing for shorter material ``` pnpm run test ./src/v3/idempotencyKeys.test.ts --run # 18 passed pnpm run build --filter @trigger.dev/core # clean pnpm run format && pnpm run lint # clean ``` --- ## Changelog `idempotencyKeys.reset()` now works when your idempotency key is itself 64 characters long. Previously any 64-character key was assumed to be already hashed, so passing one along with a `scope` silently ignored the scope and the reset never found a matching run. --- ## Follow-ups (not in this PR) - `docs/idempotency.mdx` describes the `idempotencyKey` parameter of `reset()` as "the 64-character hash string" in one place while showing raw material plus `{ scope: "global" }` a few lines later. Worth reconciling. - No surface currently exposes the stored hash that the reset endpoint matches on: `ctx.run.idempotencyKey`, the run page and the `idempotency_key` query column all show the user-provided key. That is what leads people to send a value reset cannot match. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Matt Aitken <matt@mattaitken.com> |
||
|
Chris Arderne
|
c668b72c3f |
chore(webapp): enforce React Compiler lint (#4732)
## Summary Enforces `react/react-compiler` as an error for the webapp now that all reported compiler diagnostics are fixed or narrowly scoped. Removes the unused lazy-ref helper made obsolete by the ref initialization cleanup. |
||
|
Chris Arderne
|
2b2b047089 |
chore(webapp): scope imperative route refs (#4731)
## Summary Scopes React Compiler diagnostics to route statements where refs intentionally coordinate virtualized views, live reload state, transport lifecycles, and deferred callbacks. Other compiler diagnostics remain active in those routes. |
||
|
Chris Arderne
|
4d040e13be |
chore(webapp): scope imperative component refs (#4730)
## Summary Scopes React Compiler diagnostics to component and hook statements where refs intentionally coordinate editors, animations, polling, deferred callbacks, and other imperative integrations. Other compiler diagnostics remain active in those components. |
||
|
Chris Arderne
|
a89ce5a709 |
refactor(webapp): replace render-time ref initialization (#4729)
## Summary Replaces render-time ref initialization with lazy state for frozen form defaults, the tooltip's virtual positioning element, and the side menu's first-paint visuals. Editable alert fields now update immutable state snapshots. |
||
|
Chris Arderne
|
7ab437c8ad |
chore(webapp): scope route effect synchronization (#4728)
## Summary Scopes React Compiler diagnostics for route effects that intentionally synchronize loader data, navigation, submissions, polling, streams, and transient UI state. Each suppression remains attached to the reported synchronization call. |
||
|
Chris Arderne
|
7673c46a02 |
chore(webapp): scope component effect synchronization (#4727)
## Summary Scopes React Compiler diagnostics for component and hook effects that intentionally synchronize with navigation, submissions, browser APIs, streams, timers, or authoritative server values. Each suppression stays on the reported synchronization call rather than disabling analysis for the component. |
||
|
Chris Arderne
|
101883c41c |
refactor(webapp): derive controlled UI state during render (#4726)
## Summary Derives controlled tab, tag, and checkbox values directly during render instead of copying them through effects. Modal drafts now reset from their open event, and the route-backed alert dialog renders open immediately without a mount-time state update. |
||
|
Chris Arderne
|
00149675ac |
chore(webapp): scope intentional draft synchronization (#4725)
## Summary Scopes state synchronization that intentionally resets editable drafts from authoritative server values, deployment state, or programmatic filter changes. These values cannot be derived during render without removing user control between resets. |
||
|
Chris Arderne
|
f723e5a1b8 |
refactor(webapp): simplify manual memoization (#4722)
## Summary Removes manual memoization where derived values are already rebuilt each render, narrows the dashboard watch callback to a stable chat identifier, and scopes two intentional memoization patterns that protect local edits and serialized synchronization. |
||
|
Chris Arderne
|
cf96204c7f |
chore(webapp): scope memo dependency diagnostics (#4721)
## Summary Makes stable dashboard history refs explicit memo inputs and scopes the remaining compiler diagnostics to callbacks whose local handlers or lifetime-stable values cannot be represented accurately in dependency arrays. |
||
|
Chris Arderne
|
7682a215db |
fix(webapp): stabilize time-sensitive UI renders (#4720)
## Summary Captures chat-history age when the menu opens so rerenders cannot change labels mid-view. The waitpoint deadline form also reuses one intentional wall-clock snapshot for all calculations in a render. |
||
|
Chris Arderne
|
e394b5acf5 |
fix(webapp): timestamp live metric responses (#4719)
## Summary Records when live metric responses arrive and uses that timestamp to evaluate gauge freshness and waiting duration. Cached or failed responses remain untrusted until revalidated, while rendered values stay stable between polling updates. |
||
|
Chris Arderne
|
34211e6649 |
fix(webapp): derive expiry status from loader time (#4718)
## Summary Derives session and API key expiry states from a timestamp captured by each route loader. Every status on a page now uses one consistent point in time instead of changing according to when an individual component rerenders. |
||
|
Chris Arderne
|
11ea1f8ba9 |
fix(webapp): use stable chart bucket timestamps (#4717)
## Summary Uses explicit bucket timestamps when rendering usage charts instead of anchoring missing timestamps to the current render time. Tooltips now remain stable across rerenders, and examples use a deterministic timestamp. |
||
|
Chris Arderne
|
7ea02716fc |
fix(webapp): avoid mutating render inputs (#4716)
## Summary Keeps render inputs and shared regular expressions immutable. Grouped selects now compute each section's shortcut offset directly from preceding sections, which also makes numeric shortcuts follow the displayed item order reliably. |
||
|
Chris Arderne
|
176fb6daf4 |
fix(webapp): call hooks directly and unconditionally (#4715)
## Summary Calls dashboard hooks directly instead of passing them as ordinary callback values, and subscribes to optional Ariakit stores through an unconditional hook. This keeps hook ordering stable while preserving the existing behavior when a provider is absent. |
||
|
Chris Arderne
|
6dfc54b75b |
chore(webapp): scope unsupported React Compiler diagnostics (#4713)
## Summary Adds targeted lint suppressions for components built around libraries that React Compiler intentionally declines to memoize, plus one unsupported function-reference pattern. Each suppression is scoped to the affected component so other compiler diagnostics remain actionable. |
||
|
Chris Arderne
|
9dca03f682 |
chore: enforce exhaustive React hook dependencies (#4712)
## Summary Enables exhaustive React Hook dependency checking and resolves the existing violations across the dashboard and React hooks package. Effects and callbacks now track current values without introducing request, subscription, or render loops. ## Design Dependencies are included directly when the hook lifecycle should follow them. Timers, Remix fetchers, and realtime subscriptions use stable callbacks or latest-value refs where restarting work would change behavior. Unnecessary memoization was removed where ordinary derivation is clearer. Full lint and typechecks for the webapp and React hooks package pass. |
||
|
Oskar Otwinowski
|
adaa8e9e30 |
fix(clickhouse): renumber the external deployment id migration to 041 (#4734)
## Summary `goose up` against `internal-packages/clickhouse/schema` panics on `main` today, so ClickHouse migrations cannot be applied from a fresh checkout. Renumbering the external deployment id migration from 040 to 041 clears it. ## Root cause Two migrations claim version 40. [#4615](https://github.com/triggerdotdev/trigger.dev/pull/4615) added `040_create_task_events_search_v2.sql`, and [#4661](https://github.com/triggerdotdev/trigger.dev/pull/4661) added `040_add_task_runs_v2_external_deployment_id.sql` a day later. #4661 was opened before #4615 merged, so 040 was genuinely free at branch time, and because the two files have different names there is no textual conflict for git or a rebase to surface. Both merged green, and no workflow in this repo runs `goose`, so the collision only shows up the first time someone actually migrates. goose parses the numeric filename prefix as the version and refuses duplicates: ``` panic: goose: duplicate version 40 detected: .../040_create_task_events_search_v2.sql .../040_add_task_runs_v2_external_deployment_id.sql ``` It aborts while collecting the directory, before executing any SQL, so nothing was half applied and there is no migration state to repair. This migration gets renumbered rather than the `task_events_search_v2` one because goose keys on the version number and not the filename: version 40 is already recorded wherever 040 has been applied, so renaming that file would re-run an applied migration. Verified with a full `goose up` against ClickHouse 26.2.19.43 (the image pinned in `internal-packages/testcontainers`): migrations apply cleanly through version 41, and `task_runs_v2.external_deployment_id` lands as `String DEFAULT ''`. |
||
|
Chris Arderne
|
19908436b8 |
perf(ci): speed up webapp test execution (#4709)
## Summary Speeds up webapp test jobs by balancing measured work across runners, reducing repeated container setup, and ensuring test workers release shutdown resources promptly. Unit tests run across 24 duration-aware shards, while E2E tests run across two balanced shards. ## Design `RunEngine` shutdown now closes processing resources before support resources, continues cleanup if one close fails, and reuses one shutdown promise for concurrent callers. Redis workers clear completed shutdown deadlines so finished tests no longer wait on idle timers. Container-heavy suites are split only where it improves parallelism, and repeated replication and engine fixtures are consolidated where one end-to-end case provides coverage. Timing weights are refreshed for all affected files. Dependency installation overlaps container pulls, and both workflows use WarpBuild's Node setup action. |
||
|
claude[bot]
|
447471843c |
fix(webapp): keep the branches list query string when archiving a branch (#4724)
<!-- ccr-slack-attribution --> _Requested by **Iss** · [Slack thread](https://triggerdotdev.slack.com/archives/C045W9WM3E1/p1787161814493949)_ **Before:** archiving a branch dropped the query string on the way back to the branches list, so the list reset to page 1. Working down a long list meant re-navigating to the page you were on after every archive. **After:** you land back on the exact page you archived from, with `page`, `search` and `showArchived` intact. The archive action now redirects to the page the request came from instead of rebuilding a bare branches path. ## How The archive dialog already submits the page it was opened from as a hidden `redirectPath` field (`${location.pathname}${location.search}`), and the failure path already redirected to it — only the success path ignored it and rebuilt the path with `branchesPath`/`branchesDevPath`, which have no query string. Both paths now redirect to the submitted path, run through the existing `sanitizeRedirectPath` helper to keep the redirect same-origin (the same idiom used by `resources.batches.$batchId.check-completion`). ## ✅ Checklist - [x] I have followed every step in the [contributing guide](https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md) - [x] The PR title follows the convention. - [x] I ran and tested the code works --- ## Testing Three files change: - `apps/webapp/app/routes/resources.branches.archive.tsx` — the fix. - `apps/webapp/test/archiveBranchRedirect.test.ts` — new test that drives the archive action and asserts the redirect `Location`: the query string survives on both success and failure, and an off-origin `redirectPath` falls back to `/`. Reverting the fix makes two of the three cases fail, so the test covers the regression. - `.server-changes/archive-branch-keeps-list-page.md` — release-note entry, since this is a user-facing server-only change. Also ran `pnpm run typecheck` and `oxlint` for `apps/webapp` — both clean. --- ## Changelog Archiving a branch now returns you to the same page of the branches list instead of resetting it to page 1. --- ## Screenshots _None — no visual change._ --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Eric Allam
|
aa17c4d706 |
chore(ci): deploy the dashboard agent dormant, drop the reviewer gate, add a ref input (#4710)
## Problem Every merge to main touching the agent queued a gated `staging`+`prod` deploy that sat `pending` on a reviewer approval nobody grants routinely. Because the gated runs never completed, they never drained the concurrency queue and cancelled each other, so the Actions tab filled with never-completing runs and the agent only ever actually deployed via a manual dispatch + approval. The reviewer gate bought nothing here: the agent deploys with `--skip-promotion`, so a deploy lands **dormant** and nothing goes live until the consuming webapp flips `DASHBOARD_AGENT_VERSION`. Promotion is already a deliberate act (the env-var flip); gating the dormant deploy on top of that just created the pile-up. ## Change - **Remove the reviewer gate** by dropping the required-reviewers rule on the `dashboard-agent-*` environments (repo-settings change, done). The `environment:` key **stays** so the per-environment scoped deploy token still resolves — no secret migration. - **`workflow_dispatch` `ref` input** — deploy a specific commit SHA, branch, or tag; defaults to the ref the run launches from. Checkout uses `github.event.inputs.ref || github.sha`. - **Require the ref to be an ancestor of `main`.** Constrains which commit gets deployed to merged code only. A push is always main's tip (passes trivially); a dispatched unmerged ref is rejected before the deploy step. Because an explicit `ref:` checkout doesn't create remote-tracking branches, `origin/main` is fetched explicitly before `git merge-base --is-ancestor`. - **`cancel-in-progress: false`** (kept). Cancelling the runner wouldn't stop the remote build (it finishes server-side), and a superseding concurrent deploy would race the same project's indexer. With the gate gone, deploys are short, so a brief queue can't pile up. - `max-parallel: 1` stays (parallel deploys of the same project race at the indexer). ## Owner actions (repo settings — not in the diff) 1. **Remove required-reviewers** on `dashboard-agent-staging` and `dashboard-agent-prod` — done. 2. **Add a deployment branch policy** on both environments restricting deployments to `main`. This is the authoritative token guard: `workflow_dispatch` runs the workflow file from the selected ref, so the in-file ancestor check alone can't protect `TRIGGER_ACCESS_TOKEN` (a branch could edit the check out). GitHub enforces the branch policy server-side against `GITHUB_REF` regardless of file contents. With it in place, the workflow only runs (and the token is only exposed) when dispatched from `main`, and the in-file check then constrains the independent `ref` input to merged commits. ## Pile-up root cause The stacking was caused by the **reviewer gate** (runs waited forever, so the queue never drained), not by `cancel-in-progress`. Removing the gate is what fixes it; `cancel-in-progress` stays `false`. |
||
|
Oskar Otwinowski
|
967dedcebc |
fix(run-engine): correct park deadline and snapshot state for debounced parked runs (#4708)
Two defects that surface when a run parked on an external deployment id
gets pushed by a debounce key. Both were reproduced against a local
instance before being fixed.
## 1. The run is expired before it is due
```
now | status | statusReason | delayUntil | expiredAt
13:57:06 | EXPIRED | EXTERNAL_DEPLOYMENT_NOT_FOUND | 14:01:37 | 13:57:02
```
Killed 4m35s before its own scheduled start, blaming a missing
deployment.
**Why.** The park deadline is armed **once**, when the run is first
parked, from `max(now, delayUntil) + deadline`. Debounce pushes
`delayUntil` out afterwards and nothing re-arms it:
- `rescheduleDelayedRun` reschedules `enqueueDelayedRun:<id>`, not
`expireParkedExternalDeploymentRun:<id>`
- the redis-worker reschedule is an update-only `ZADD … XX`, and a
parked run has no `enqueueDelayedRun` job, so that call is a silent
no-op
Repeat triggers on one key walk `delayUntil` away from a deadline that
no longer moves. Once it crosses, the run dies while parked and not yet
due.
**Fix.** The expiry job already loads `delayUntil`, so it re-arms from
the current value and returns instead of expiring a run that is not due.
The guard lives in the expiry job rather than the debounce path
deliberately: it covers **every** caller that moves `delayUntil`, so a
future call site can't reintroduce this by forgetting to re-arm. It
stays bounded by the debounce max-duration contract, so a hot key can't
postpone expiry indefinitely.
## 2. The run reports itself as delayed while it is parked
```
RUN_CREATED | PENDING_VERSION | Run is waiting for a deployment of 'debounce-test-2'
DELAYED | DELAYED | Delayed run was rescheduled to a future date ← after one debounce push
```
The row stays `PENDING_VERSION`; the latest snapshot claims `DELAYED`,
so the run page describes a parked run as delayed. Happens on the
*first* push.
**Fix.** `rescheduleRun` hardcoded `DELAYED`/`DELAYED`. The snapshot
statuses are now supplied by the caller and **default to `DELAYED`**, so
the ordinary delayed path is byte-identical, and `rescheduleDelayedRun`
passes the parked statuses through when the run is parked.
## Reproducing
Repeated triggers on one debounce key against an id that hasn't landed:
```bash
curl … -d '{"options":{"externalDeploymentId":"x","debounce":{"key":"k","delay":"5m"}}}'
```
Three triggers correctly fold into one parked run; the defects show up
on the pushes.
## Testing
Two tests, each verified red before green and failing alone:
- a run whose delay was pushed past the deadline stays `PENDING_VERSION`
instead of expiring
- a debounce push on a parked run leaves a
`RUN_CREATED`/`PENDING_VERSION` snapshot, not `DELAYED`
`56 passed` across parking, pendingVersion, delayedRunSystem and
debounce; `43 passed` in `PostgresRunStore`. Typecheck, lint, format
clean.
## Notes
- Stacks on #4665, so it lands after the whole external-deployment-id
series.
- No changeset: this fixes unreleased behaviour introduced by the stack
below it, so no user has seen it.
- Both found by Devin's review on #4664, and both confirmed end to end
on a local instance before fixing.
|
||
|
Oskar Otwinowski
|
cde8919861 |
feat(webapp): show the external deployment id on deployments and runs (#4665)
Deployments page: an always-visible External ID column after Deployed by, and an External ID row in the deployment inspector under Worker type, both showing an en dash when a deploy carried no id. The Vercel Linked column now renders before Git, still only when a Vercel integration is connected. Also corrects the blank-row colSpan, which was already off by one before this column existed. Run inspector: an External deployment ID row between Version and SDK version, read from the run annotations, so an operator can see which id a run was pinned to - including a run that expired before its deployment ever arrived, where the locked version is empty but the id is the whole story. Buffered runs read the id from the same annotations rather than reporting none. Long ids are head-truncated with the full value behind the copy button: a commit SHA is meaningful in its prefix, and the inspector panel can be narrowed to 250px, where an unbroken 40-character SHA would otherwise scroll the properties list sideways and push the copy button off-panel (TRI-12923, TRI-13000). |
||
|
Oskar Otwinowski
|
8b0385c429 |
feat(run-engine): trigger tasks pinned to an external deployment id (#4664)
The SDK discovers an external deployment id at runtime (explicit TRIGGER_EXTERNAL_DEPLOYMENT_ID always; platform commit-SHA variables and generic fallbacks when TRIGGER_AUTOMATIC_SKEW_VERSION_PROTECTION=1) and sends it alongside lockToVersion; the server resolves precedence (version > external id > current). An id held by a deployed deployment pins the run to that worker; an in-flight or unknown id parks the run in PENDING_VERSION with the id in TaskRun.annotations, wakes it pinned when a deployment carrying the id finalizes (ClickHouse candidates, Postgres authoritative), and expires it after a deadline that re-checks Postgres before acting. Parking outranks delaying and preserves delayUntil. The id is projected to ClickHouse task_runs_v2.external_deployment_id during replication. Redis cache for id-to-worker resolution, guarded version-aware writes. Ids are not unique. Several deployments can hold one id - a --force rebuild is the ordinary way to get there - so resolution always picks the highest version among the candidates, never the newest by timestamp. The rule is applied identically on both paths that can bind a run to a worker: resolveExternalDeployment at trigger time, and PendingVersionSystem when a landing deployment wakes a parked run. Version comparison is numeric on the counter half, so 20260807.10 outranks 20260807.9. A run whose id never lands expires at the deadline with EXTERNAL_DEPLOYMENT_NOT_FOUND and an error naming the id it waited for, which is what a failed build or a typo looks like from the caller. Default deadline is one hour (EXTERNAL_DEPLOYMENT_PARK_DEADLINE_MS). Debounce registration happens in both the parked and the delayed branch through one helper, so a debounced run that parks still binds its debounce key; without it every later trigger for the same key created another parked run, and all of them executed when the deployment landed. The two DELAYED-only status checks in DebounceSystem also accept PENDING_VERSION, without which the lock-contention fallback would rethrow a 5xx the SDK retries and amplifies, and the fast path would push every trigger on a parked key through the redlock. Resolution is skipped in development. A dev environment cannot hold a WorkerDeployment - trigger dev registers a BackgroundWorker with nothing behind it, and deploy --env refuses dev - so an external deployment id there could only ever park, and the parked run then expired against the dev TTL while a connected dev worker sat idle. The id is still annotated so the dashboard shows what the app sent (TRI-13000). |
||
|
Oskar Otwinowski
|
6bfce6387d |
feat(deploy): --external-id and --force for deploy idempotency (#4663)
A deploy can carry an opaque external id (commit SHA, CI run id, release tag). Repeating an id that already deployed returns the existing version as a no-op instead of rebuilding; an id with a build in flight is rejected with 409 naming that version; a failed id rebuilds freely. --force is non-destructive to deployments that already succeeded - both persist and the higher version wins - but cancels a build still in flight, so one id never has two live builds racing to define it. Cancelling writes a terminal status and appends a finalized event, which aborts a build the platform drives; a build it does not drive keeps running but can never land, and the CLI says so. Ids are deliberately not unique - reuse is resolved in application code by highest version, never timestamps. The no-op path mints no build credentials and no event stream (TRI-12923). What that means for callers: a --force rebuild leaves two deployments holding one id, and runs triggered with it go to the higher version once the rebuild lands, so the takeover needs no separate promotion. Until a successful build exists for an id, runs triggered with it park and then expire rather than falling back to current - a failed build is therefore visible to the caller as expired runs, not as runs on the wrong release. |
||
|
Oskar Otwinowski
|
689538d327 |
feat(core): external deployment id wire contract (#4662)
An external deployment id is an opaque, caller-chosen name for a release - a commit SHA, a CI run id, a release tag. This adds the shared contract that both halves of the feature read, and nothing else: no deploy writes one yet and no trigger sends one. ExternalDeploymentId is defined once and reused by InitializeDeploymentRequestBody.externalId and TriggerTaskRequestBody.options.externalDeploymentId, so a value accepted by one half can never be rejected by the other. A value that is blank once trimmed is treated as absent rather than rejected, so an unset CI variable expanding to an empty string is not a 400. The 128 character limit fits a SHA-256 commit hash with room for composite ids, and EXTERNAL_DEPLOYMENT_ID_MAX_LENGTH is the single source of truth that the request schemas and the CLI both read. RunAnnotations.externalDeploymentId records the request, not the outcome: lockedToVersionId and taskVersion are overwritten when a run locks, whereas this stays true forever, and it can carry the pin for a run parked before its deployment exists. Also lands the runtime discovery helpers as pure functions over an environment reader: the explicit TRIGGER_EXTERNAL_DEPLOYMENT_ID variable, the platform and CI commit-SHA table, and the TRIGGER_AUTOMATIC_SKEW_VERSION_PROTECTION gate. Nothing calls them yet. refs TRI-13000 |