4e006519de2d7d57088f6b43ea37f5c877158043
7955 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Graham Tremper
|
4e006519de |
feat(chat): expose endAndContinue to custom agents (#4647)
## Summary Raw `chat.customAgent()` loops can now call `chat.endAndContinue()` to move the Session to a fresh run. The managed loop already used the same server operation through `chat.requestUpgrade()`, but raw loops could not call it directly. Call the method between turns after detaching input listeners from the old run. Await it and return immediately. Unconsumed `.in` records stay on the Session for the continuation run. I put this on the `chat` namespace next to the other raw chat primitives. Happy to move it if maintainers prefer a different API placement. ## Testing - `pnpm exec vitest run` in `packages/trigger-sdk` (374 tests) - Focused webapp Session E2E tests (3 tests) - `pnpm run build` in `packages/trigger-sdk` - Webapp typecheck - `pnpm run format` - `pnpm run lint` ## Checklist - [x] I followed the contributing guide - [x] The PR title follows the convention - [x] I tested the change ## Changelog Allow custom chat agents to rotate to a new task version without dropping unconsumed Session input. --------- Co-authored-by: Eric Allam <eallam@icloud.com> |
||
|
Eric Allam
|
d54bcaa29c |
fix(chat): stop losing a user message that arrived mid-turn (#4795)
Follow-up to [#4644](https://github.com/triggerdotdev/trigger.dev/pull/4644), now rebased onto main so the diff is just these three commits. ## Summary Two ways a chat could lose a user message, both pre-existing and both raised while reviewing #4644. A message arriving while a turn was streaming was handed to that turn's push handler and parked in an in-memory array. The router counts a record handed to a handler as terminally decided, so it stopped holding the resume floor behind it, and the turn boundary published a cursor past a message that existed only in that process. A crash before the next turn lost it, silently. Measured: with the message at sequence 1, the boundary published `session-in-event-id: 1`, so a resume skipped it. Separately, a message the agent declined to inject was discarded with the turn. Never injected, never written to the wire buffer, never answered. That was also the documented default, since a `pendingMessages` config without `shouldInject` declines every batch. ## Design Notification and consumption are now separate concerns on the router. `observe` reports that a record arrived without taking it, so the record stays queued and keeps holding the floor. It is rejected on an `at-arrival` route: an observer there would either have to count as a listener, which would stop an unconsumed stop being discarded and bring back a wedged mailbox, or watch records it cannot affect. `take` removes exactly one queued record. The managed loop and the `chat.createSession()` iterator now only subscribe when there is a steering config to feed, and injection is the point of consumption. A declined batch never reaches the take, so its records stay queued and become later turns. Both in-memory wire buffers are gone, so a message waiting for its turn is durable rather than living in whichever worker received it. The floor doubles as the wake cursor: `awaitWake` registers with it and the server completes the waitpoint immediately if anything sits after that sequence. An over-advanced floor was therefore also a missed wake. It is now recorded on the wait span so a run that never woke can be diagnosed from its trace. ## Verification Both fixes have a red and green pair, each checked against the unmodified source rather than only observed to pass: - the resume cursor test fails on the parent branch and passes here - the declined-message test fails without the second commit and passes with it Also 8 new router tests for `observe` and `take`. Suites green at 385 for the SDK and 886 for core. ## Not addressed A `pendingMessages` config with no `chat.toStreamTextOptions()` spread still swallows messages, because nothing drains the queue at all. Same shape, different trigger, tracked separately. |
||
|
Graham Tremper
|
1065251ca7 |
fix(chat): ignore stale turn completions after reconnect (#4643)
## Summary Reloading a browser chat mid-turn can replay a completion event for an older input and close the active turn too early. This persists the last browser-owned input sequence and reuses it on reconnect, so older completion events are ignored. The sequence is cleared after the matching boundary, and reconnect avoids the settled-peek shortcut while that sequence is active. The persisted field is optional, so sessions without it keep their existing behavior. ## Testing - `pnpm --dir packages/trigger-sdk run test ./src/v3/chat.test.ts ./test/chat-turn-correlation.test.ts --run` — 67 passed - `pnpm --dir packages/trigger-sdk run test --run` — 32 files, 379 tests passed - `pnpm run build --filter @trigger.dev/sdk` - `pnpm run format` - `pnpm run lint` ## Changelog Browser chats now keep the active turn open across page reloads when older completion records are replayed. ## ✅ 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 💯 Co-authored-by: Matt Aitken <matt@mattaitken.com> |
||
|
Graham Tremper
|
c115f440bc |
feat(chat): custom agent mailbox helpers and session.in delivery fixes (#4644)
## Summary
Adds `chat.messages.hasPending()` and `chat.messages.next()` so a custom
agent
loop can inspect pending chat input without consuming it and take one
record at a
time, and fixes four ways a chat could mishandle input across a restart:
a
message silently lost, a recovered answer cut off by a stop the user had
already
pressed, a retried send answered twice, and a record the agent had no
consumer
for blocking every message queued behind it.
```ts
if (await chat.messages.hasPending()) {
const record = await chat.messages.next({ timeoutInSeconds: 0 });
if (record) handle(record.payload);
}
```
## Why the fixes came together
`session.in` carries records for consumers whose delivery needs differ.
A user
message must be delivered eventually, so it can wait arbitrarily long
for a turn
to take it. A stop only means anything to the turn that is live when it
lands.
Progress along the channel was tracked as one sequence number, and one
number
cannot say "control applied through 7, message 3 still owed" at the same
time.
Each of the bugs above is that mismatch surfacing somewhere different.
So instead of a rule per symptom, records are now classified once and
handed to
one route, and each route declares two things: whether it holds a record
when no
consumer is ready, and whether a record it never handled has to survive
into the
next boot. The resume cursor, the replay window and the
discard-the-unowned
behaviour are then derived from route state rather than maintained
beside it, and
`hasPending()` answers from the message queue instead of the head of a
buffer
shared with every other kind.
The wire is unchanged. Both cursors on the turn boundary keep their
meanings, so
existing chats resume as before and there is no webapp change.
## Behaviour worth calling out
`chat.writeTurnComplete()`'s `sessionInEventId` is the cursor that is
safe to
resume from, not the sequence of the record the turn answered. It is
held back
behind any message still waiting to be handled, so a value below the
record you
just handled is expected rather than a sign of a lost turn.
The stop fix also covers chats whose most recent turn was completed by
an older
SDK, by resolving the replay window from the channel when the boundary
does not
carry one. The trade there is deliberate: a stop that landed in the
moments
before boot and was never applied is dropped along with the replayed
ones,
because a stop the user can press again beats a stale one killing an
answer they
are waiting for.
## Verification
Thirteen reproductions against a local stack, each driving real runs
rather than
mocks, covering the documented `next()`/`hasPending()` loop, suspend and
resume, a
crash between consuming a message and writing turn-complete, a retried
send whose
idempotency claim is lost, and a continuation boot that must not replay
answered
messages. Where applicable each was also run against `main`, so the
fixes are
differences rather than assertions.
Five further legs on a deployed environment, which the earlier revisions
of this
branch did not cover at all: a message appended while the run is
genuinely
checkpointed, a message appended while the run is dead, the
stop-after-crash case
on the real crash path, and both version-skew directions (a newer worker
resuming
an older worker's turn boundary, and an older worker resuming a newer
one's).
Two of those restart fixes also have a browser-driven red and green pair
on a
deployed environment, staged identically on both sides and differing
only in the
SDK. For the lost-message fix, the unanswered message is replayed and
answered in
full here, and is never replayed at all on the released SDK. For the
stop fix,
both sides replay the message and diverge on the stop itself: it is
declined here
and the answer completes, while the released SDK re-applies it and the
recovered
answer dies before it streams.
The routing decision itself is a pure state machine, so it also has a
property
test over every interleaving of the record kinds crossed with each crash
point,
checked by mutation to confirm it fails when the cursor arithmetic or
the replay
window is broken.
## Known and not addressed here
The read of the woken record is unbounded, so a wake with nothing to
read makes
`wait()` outlive its own waitpoint. Tested and not a deadlock, since the
read
defers to the next record, but bounding it is a separate change with its
own
test.
Separately, and not caused by this branch: a run that crashes while a
message is
still queued is not replaced until the next inbound append, so that
message waits
rather than being recovered on its own. Worth its own issue.
Also not caused by this branch, but worth knowing when reading the
release note: a
chat page that stayed open across the crash keeps showing the partial
answer it
already received, so the recovered answer only appears after a reload.
The answer
itself is persisted correctly. The gap is on the client, which does not
apply a
re-delivered turn over a partial it already holds.
---------
Co-authored-by: Eric Allam <eric@trigger.dev>
Co-authored-by: Eric Allam <eallam@icloud.com>
|
||
|
wei-wei
|
c7f78e4853 |
fix(sdk): reset skipToTurnComplete when a new chat turn starts (#4744)
## ✅ 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 Reproduced with `useTriggerChatTransport` + `useChat` and the stop pattern from the ai-chat frontend docs: 1. Send a message so a turn is streaming. 2. Call `transport.stopGeneration(chatId)`, then `useChat`'s `stop()`. 3. Send another message. Before this change the second turn never renders: no parts arrive, `status` stays `streaming`, and the session stays `isStreaming: true`, so a stop button stays on screen until the page is reloaded. The run itself is fine and everything persists, so a reload shows the full response. Cause: `stopGeneration` sets `state.skipToTurnComplete = true`, and the read loop only clears that when it sees a `TURN_COMPLETE` record. The abort closes the reader before that record arrives, so the flag survives into the next turn and every record of that turn is skipped, including its own `TURN_COMPLETE`. After this change the same sequence streams the second turn normally. Verified against 4.5.11 and 4.5.12 (both affected) with the equivalent patch applied to the built SDK. --- ## Changelog Reset `skipToTurnComplete` when a new chat turn or action is sent, so a message sent after `stopGeneration` streams normally instead of leaving the chat stuck in a streaming state. --------- Co-authored-by: Devin AI <devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: nicktrn <55853254+nicktrn@users.noreply.github.com> |
||
|
Daniel Sutton
|
920892bc11 |
feat(webapp,run-store): gen-2 shard arms in read-through and idempotency (#4781)
Gives read-through and idempotency their gen-2 shard arms, so an id that names its own shard is read there and nowhere else. #4764 has landed, so this now targets `main` directly and no longer depends on an unmerged branch. It builds on what that PR supplied: `resolveShard`, `runOpsShardHandles` and the keyed router. TRI-13431 ## What changes **Read-through routes by `resolveShard`, not by the binary residency classifier.** A gen-2 id reads its own shard's replica once and probes no other store. A gen-1 v1 id still reads new only. **Callers now declare `idKind`.** A cuid gives no way to tell a run id from a waitpoint id, and the two must route differently: - a legacy-classified **run** id reads the legacy replica only — there is no cuid run migration, so the new-store probe cannot find it; - a cuid **waitpoint** keeps the new-first pair probe, which is load-bearing because a cuid waitpoint can be co-located with its run on the new store. There is no default, because a default would pick one of those arms silently. The field `runId` is renamed to `id`, since it carried both kinds already. **`ReadThroughResult` carries `found`.** `source` is an open-ended union once shards exist, so a consumer testing found-ness by listing the hit sources reads a gen-2 hit as a miss. One consumer did exactly that. Discriminating on `found` makes that class of bug a compile error rather than something a reviewer has to spot. **Idempotency resolves its client through one shard-keyed map.** Both call sites go through `clientForShardKey`, so they cannot disagree about which store owns an id. An absent key takes an explicit logged branch to the fallback, not a silent legacy default. The `classify` seam is retyped to return a `ShardKey`: `Residency` (`"NEW"`) and the reserved shard keys (`"new"`) differ only by case, and `ShardKey` collapses to `string`, so the compiler would not have caught feeding one into the other. The dead `isMigrated` branch is deleted. Nothing implemented it, and the one production comment recorded that omitting it was deliberate. **`PostgresRunStore._residency` widens to `ShardKey`.** Still unused; the store stays unaware of its siblings. ## Two behaviour fixes found while doing the above **An unconfigured shard key logs and returns not-found instead of throwing.** The waitpoint route takes the id from a URL parameter, and any base32hex core plus `[a-z0-9]` plus `"2"` parses as gen-2. The route turns a throw into a 500, so throwing here would let any authenticated client generate 500s and error logs by guessing shard chars, of which there are 36. An error-logged not-found is neither silent nor a misroute. Throwing stays correct on the router path, where ids are minted rather than received. **The two cross-seam batch hydration sites were gen-2 blind.** `hydrateRunsAcrossSeam` and `ApiBatchResultsPresenter` classified with the binary `ownerEngine`, so a gen-2 run id joined the gen-1 `new` group, missed there, and — classifying dedicated-family — never reached the legacy probe either. The id was dropped from a bulk-action page and from batch results with no error. Both now partition ids by shard key and read each configured shard once. Also: a gen-2 waitpoint that missed its shard replica fell back to the gen-1 new writer, a different database, silently disabling read-your-writes for the freshly minted token that fallback exists to serve. It now falls back to its own shard's writer. ## Merge safety Inert while `RUN_OPS_SHARDS` is unset: the shard maps are empty, so every gen-2 arm is unreachable, and gen-2 minting is not live yet. The one live change is the gen-1 run arm, and it removes work rather than adding it. `RoutingRunStore.findRun` never forwards the caller's client object — it routes by id and reads only the client's presence and replica brand — so `readRunForEvent`'s "new" closure already resolved a legacy-classified run id to the legacy store. The arm removes a duplicated read of the legacy replica. A test pins this, because a future caller passing a raw client and a run id would lose the pre-cutover 27-char case, which is new-resident but classifies legacy. ## Testing 14 tests added, testcontainers throughout, no mocks. 22 affected test files pass; typecheck, lint, format and knip are clean. Both arms were verified by neutralising them and confirming the new tests fail. The batch-results test needed rewriting after that check: the first version passed with the fix neutralised, because it used one container as both the gen-1 new client and the shard replica, so it was not testing what it claimed. Note for review: run testcontainer suites in small batches. Sixteen at once starves Docker and everything times out at 60 seconds. The run-ops legacy-guard baseline is refreshed in its own commit. The baseline is keyed by line number, so partitioning the batch-results read shifted four pre-existing entries and added one. Baselined violations in that file go from four to five, all reads; the new one is the shard read beside two gen-1 reads already there. No changeset and no `.server-changes` entry: a user notices nothing while the flag is unset. |
||
|
Oskar Otwinowski
|
4c16387426 |
fix(webapp): project integrations page — Staging gating, unreachable code, and follow-ups (#4784)
Three bugs on the project integrations page, one commit each for the two reported ones and four for the follow-ups found while fixing them. ## `chore`: remove unreachable code on the integrations page (TRI-12645) Two notification panels in `VercelSettingsPanel` could never render: 1. The **"Failed to load Vercel settings"** panel was gated on a `hasError` state whose setter is never called anywhere, so it was permanently `false`. 2. The **"connection expired"** banner *inside* the `connectedProject` branch was unreachable: `VercelSettingsPresenter` only populates `connectedProject` on its success exit, which hardcodes `authInvalid: false`, while both `authInvalid: true` exits return `connectedProject: undefined`. Removing them makes the surrounding `!showAuthInvalid` guards vacuous, and the `onboardingData?.authInvalid` disjunct redundant — the loader already folds onboarding auth state into `authInvalid` before it reaches the component. **No behaviour change.** An org with a connected project and an expired token still gets the banner, from the branch below (untouched). ## `fix`: gate Staging settings on plans without a Staging environment (TRI-12646) The ticket's premise was inverted, and I've corrected it there. In Git settings, **Preview** is the row that's correctly gated; **Staging** is the one with no gate at all: - Preview swaps its switch for an Upgrade button, and `projectSettings.server.ts` neutralises a forged `previewDeploymentsEnabled=on`. - Staging was a plain always-editable `Input`, and `validateStagingBranch` only checked the branch existed on GitHub. An org without a staging environment could type a tracking branch, hit Save, get a success toast, and have it silently do nothing. Staging and Preview environments are created together for projects on a plan that includes them, so gating one and not the other was an oversight. The Staging row now mirrors the Preview row. Server-side it ignores the submitted branch when there's no staging environment, but **preserves the stored branch rather than clearing it** — deliberately different from the Preview handling. Forcing a boolean off is harmless; forcing a *string* off would wipe a tracking branch the org had already configured the first time they saved after losing the environment. The Vercel write path had the same gap: `update-config` / `complete-onboarding` / `update-env-mapping` never re-derived available env slugs server-side, so `["stg","preview"]` could be persisted for a project with neither environment, and `createDefaultVercelIntegrationData` turned preview on unconditionally. Both now filter against the project's actual environments, via a pure `restrictConfigToAvailableEnvSlugs` helper that only touches keys present on the input. ## `fix`: show build settings when the GitHub app is disabled (TRI-13488) The page wrapped Git settings, the Vercel section **and** build settings in one `githubAppEnabled` guard, so with the GitHub app off it rendered an empty container. The Vercel section genuinely depends on GitHub — it can't sync environment variables or link deployments without a connected repo — so it stays gated. Build settings don't: they also apply to CLI deploys run with `--native-build-server`, exactly as the section's own description states. They now render regardless. ## `fix`: stop the Vercel onboarding modal spinning forever (TRI-13488) `computeInitialState` starts in `loading-projects` whenever the org has a Vercel integration but no onboarding data yet, and the effect that escapes it waits for `availableProjects !== undefined`. When `getOnboardingData` returns `null` — it does that on any thrown error, and when the org integration row is missing — nothing ever arrives. The empty-array case self-resolves (`[] !== undefined`), so this is specifically the null case. The route can tell "still loading" from "loaded nothing" because its fetcher always requests `?vercelOnboarding=true`; it now passes that down and the modal explains the failure with a retry and a link to check the integration's access on Vercel. ## `fix`: match staging and preview environments consistently (TRI-13488) The four places that ask "does this project have a staging / preview environment?" disagreed. `VercelSettingsPresenter` matched on type with no parent filter, so any preview *branch* row satisfied it — branches are `PREVIEW` rows too. `GitHubSettingsPresenter` and `ProjectSettingsService` matched on slug instead. Slug is the weaker key: it's derived at creation time and legacy rows can carry something else, which is why `memberDevelopmentEnvironmentWhere` deliberately avoids it. All four now match on `type` plus `parentEnvironmentId: null`, which excludes branches without depending on the slug being canonical. ## `fix`: explain when no Vercel environment can be mapped to Staging (TRI-13488) Reported while reviewing the branch. The Staging build settings show *"Set a Vercel environment for Staging first."* whenever the project has a staging environment and no mapping — but the control that sets the mapping only rendered when the Vercel project had at least one custom environment: ``` hint: hasStagingEnvironment && !configValues.vercelStagingEnvironment control: hasStagingEnvironment && customEnvironments.length > 0 ``` So a Vercel project with no custom environments, or one whose custom environments failed to fetch (the presenter swallows that error to `[]`), got an instruction with nothing to act on. Both conditions predate this PR. The mapping row now always renders alongside the hint and explains what to do when there's nothing to choose from, and the build-settings hint says the same thing. ## `chore`: remove the remaining dead code (TRI-13488) - The `"installing"` `OnboardingState` is unproducible — no `setState` call yields it — so its redirect effect, switch arm, `isLoadingState` conjunct and the `vercelAppInstallPath` import it was the only user of are all dead. - `(state as string) !== "completed"` sits in a branch where TypeScript has already narrowed `"completed"` out; the cast is what let it compile. - `hideSectionToggles` was only ever passed alongside `layout="settings"` but only read inside `layout="card"` blocks, so it could never take effect. Removed the prop entirely. - Unused bindings and the helpers only they referenced: `envSlugLabel`, `_formatSelectedEnvs`, `_CompleteOnboardingForm`, `_handleFinishOnboarding`, and the rest. No behaviour change in that commit. ## Not included The three overlapping modal-open effects in `settings.integrations/route.tsx` are left alone — they're defensive against a close-then-reopen race, and untangling them is a behavioural risk with no user-visible payoff. ## Verification `pnpm run typecheck --filter webapp`, `pnpm run lint` and `pnpm run knip` are clean. New `apps/webapp/test/vercelIntegrationConfig.test.ts` covers the slug restriction and the default-config seeding (both pure functions); 39 tests pass across it and the three existing Vercel/project-settings files. The new `projectId` + `slug` query is served by the existing `@@unique([projectId, slug, orgMemberId])` prefix — same access pattern as the preview check it mirrors. refs TRI-12645, TRI-12646, TRI-13488 |
||
|
Daniel Sutton
|
02e6157d12 |
feat(run-store): add an execution-snapshot store decorator behind an off-by-default dial (#4765)
## Summary Adds a `RunStore` decorator that mirrors execution snapshots into Redis alongside Postgres, plus the orphan-key sweep and the fault-injection suite that prove the write protocol converges after a crash. Nothing constructs it, so merging this changes no behaviour: the configuration, the production wiring and the Redis client all arrive in later work. The execution-state log is the hottest table in the run graph, and moving it out of Postgres has to happen without a big-bang cutover. This is the attachment point for that: a decorator that wraps the existing storage interface and intercepts only the methods that touch snapshots, so none of the many callers change. ## Design Write order is the correctness property, and the two orders differ on purpose. A transition writes Postgres first and Redis second. A crash in the gap leaves a run whose latest snapshot is stale, which is the state the heartbeat stall watchdog already heals in production today. A birth writes Redis first and Postgres second. A crash there leaves an unreachable key for a run that does not exist. Postgres first would instead leave a run with no snapshot at all, which the engine treats as a hard error, so the run would be stuck. Each order is chosen so the state a crash leaves behind is the harmless one. A lost cross-store write is never recovered by a transaction or an outbox; recovery is always the existing stall and repair job. A failed append retries, then hands the run to that job, and never rethrows, because Postgres has already committed and a throw would turn a healable gap into a caller-visible error. Inside a transaction the Redis half is staged and flushed only after the commit, so a rollback cannot leave Redis holding a transition that never happened. Reads are shape matched. Two of the snapshot reads take arbitrary Prisma arguments, and a key-value store cannot answer an arbitrary query, so the decorator recognises exactly the shapes the engine sends and delegates everything else. A miss falls back to Postgres, which is also how runs created before any cutover keep working. The sweep reaps under two rules, because neither can see what the other leaves behind. A finished run whose keyspace never received its completion expiry gets one applied. A keyspace with no run row at all, past an age threshold, is deleted; that is a crashed birth, which is non-terminal so it carries no expiry and has no run row, so the first rule can never match it. ## Inertness Three independent reasons this is a no-op if merged alone: - Nothing constructs the decorator or the Redis store outside tests. - No configuration reaches it, so the dial stays at its off position, which is a pass-through that makes no Redis call. - The existing Postgres store gains an off-by-default flag and two optional input fields. Both default to today's behaviour, and only the decorator would ever supply them. ## Notes for review The snapshot id and the creation instant are both minted by the decorator and written into both stores, so one snapshot has one identity and one timestamp wherever it is read. Without that, the two stores disagree on values that later tooling has to compare, and the cursor for a snapshot window resolved from one store misfilters the window walked in the other. Three defects in this work passed the full existing test suites before being found by review rather than by a test: the decorator wrote no wait cycle at all, the snapshot window dropped the ordering used to give each completed waitpoint its position in a batch, and the two stores stamped different creation times. The common cause was that no test drove a snapshot that actually carried waitpoints, and that the parity suite compared a timestamp against a value it had just read back from the row it was checking. Both gaps now have tests. |
||
|
Daniel Sutton
|
1801b0e80b |
feat(webapp,docker): run-ops boot interlocks and migrations at N databases (#4780)
## Summary
The run-ops boot interlocks and the migration entrypoint each assume
exactly two run-ops
databases. This generalizes them to any number, so a deployment that
configures
`RUN_OPS_SHARDS` gets the same safety guarantees it gets today with two
stores: no two stores
may point at one database, every store that owns its own database must
replicate to
ClickHouse, and every store must have its schema migrated.
With `RUN_OPS_SHARDS` unset, nothing changes. The distinctness check
over a two-element set is
the pairwise compare it replaces, replication coverage is the check it
was, and the entrypoint
runs the same two migration invocations.
A shard may declare `aliasOf: "new"`, which shares an existing store's
client by reference. An
aliased shard is not its own database, so it is exempt from the
distinctness check and needs no
replication slot of its own. Every check keys that exemption on the
declared field, never on
client object identity: two client objects can sit over one database,
which identity comparison
cannot see.
## Design
**Distinctness.** `probeDistinctDatabases` compared two URLs. It now
delegates to
`probeDistinctStores`, which reads every fingerprint in parallel and
groups them by system
identifier and database name. Any two stores under one key refuse the
boot. The old pairwise
entry point stays, so its existing container tests are the proof that
set uniqueness over one
pair gives the verdict it gave before. Fail-closed is unchanged: a probe
that cannot answer
returns not-distinct, because "distinct" is a positive claim a failed
probe cannot support.
**Co-residency.** The advisory runs once per store against the control
plane. The legacy
emission keeps its exact call shape and its untagged metric series, so
an existing dashboard
does not change. Each shard emits its own point carrying its shard key.
Every store emits
before any enforcement throw, so one offending store never costs another
store its metric.
**Replication.** `buildReplicationSources` appends one source per shard
that owns its own
database, taking the slot, publication and origin generation its
descriptor declares.
`assertReplicationCoversSplit` then requires a source per such shard.
That check also closes a hole it inherited. The descriptor parser
validates uniqueness among
shards only, so a shard could take the slot name, publication name or
origin generation of the
legacy or the new source. The replication service does validate this,
but it throws from its
constructor, and the caller reaches that constructor only after shutting
the bootstrap instance
down:
```ts
if (sources.length > 1) {
await service.shutdown(); // legacy stream stops here
service = new RunsReplicationService({ ... }); // throws: duplicate slotName
}
```
The throw was not a `SplitReplicationMisconfiguredError`, so the process
stayed up with no
replication at all, legacy included, behind one logged line. That is the
silent ClickHouse
under-count the error exists to prevent. The check now runs at the boot
gate, before anything is
torn down, and raises a subclass the existing exit path already
recognizes. A correct deployment
already satisfies it, because two consumers on one WAL slot is a data
race that cannot work.
**Migrations.** Every shard runs the identical schema, so a new shard is
the existing migrations
against a new DSN. The runner image has no `jq`, so a small node script
prints one DSN per line
and the entrypoint loops over them. The loop is a `for` and not a `while
read` pipeline: a
pipeline subshell swallows a failed migration on any iteration but the
last, which would let a
broken shard boot. Tracing stays off across the capture and the loop,
because `set -x` prints an
assignment and a DSN carries credentials.
Verified end to end against real Postgres containers for the fingerprint
probes, and against the
real shell block with a stubbed migration command: an aliased shard is
skipped, `directUrl` wins
over `url`, a failing shard stops the container on the first failure,
and a malformed descriptor
stops it before it migrates anything.
Stacked on #4764.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Saadi Myftija
|
8da393bf33 |
feat(webapp): add org slug and project name to deployment telemetry events (#4785)
Adds `$trigger.org.slug` and `$trigger.project.name` attributes to the `deployment.finished` / `deployment.initialized` events (follow-up to #4778). |
||
|
Saadi Myftija
|
38e78f8c7e |
feat(webapp): deployment lifecycle telemetry events (#4778)
Deployments currently leave little analytical trace. This PR makes every
deployment emit two analytics events to enable useful queries. It also
enables comparing deployments across build paths, CLI versions,
runtimes, and orgs.
### Where the events come from
```
trigger deploy
│
▼
initialize ─────────────────────────────▶ ✨ deployment.initialized
│ createdAt
▼
PENDING waiting for a build slot ┐
│ startedAt │ queue time
▼ ┘
INSTALLING build server installs deps ┐
│ installedAt (native paths only) │ install time
▼ ┘
BUILDING the image is built ┐
│ builtAt │ building time
▼ ┘
DEPLOYING indexing + registry push ┐
│ deployedAt / failedAt / canceledAt │ deploying time
▼ ┘
DEPLOYED · FAILED · TIMED_OUT · CANCELED
│
└───────────────────────────────────▶ ✨ deployment.finished
```
`deployment.finished` fires exactly once, whichever way the deployment
ends, and is backdated to cover the deployment's real lifetime. Not
every path visits every state (Depot deploys skip PENDING/INSTALLING,
for example) — a phase duration is simply omitted when its state was
never entered.
### What each event carries
- **Which path built it**: `depot`, `native`, or `native_local_bundle`
- **How it ended**: status, plus an error class and message when it
failed
- **How long each phase took**: queue, install, building, deploying, and
total — derived from the timestamps above
- **Who and with what**: org, project, environment, runtime, CLI
version, and how the deploy was triggered (CLI, GitHub, Vercel)
With that, one query gives failure rate per build path, duration
percentiles per phase, adoption per CLI version, or a per-org health
table.
### Fixes that ride along
- The old `deployment.outcome` span was silently dropped ~95% of the
time (it was subject to trace sampling). The new events opt out of
sampling explicitly, so every deployment is counted.
- The fail/timeout/finalize transitions were racy: a late timeout could
overwrite a successful deployment. They now use guarded writes, so
exactly one caller wins the terminal transition — and exactly one event
is emitted.
- Canceled deployments previously recorded nothing; they do now.
- The deployment's CLI version is now stored at initialization (new
nullable column), so even deploys that fail early are attributable to a
CLI release.
- Telemetry is flushed on shutdown (the last batch used to be lost on
every webapp deploy), and an optional second exporter can mirror just
these events into a dedicated dataset.
|
||
|
Daniel Sutton
|
00e3c151d4 |
feat(webapp): RUN_OPS_SHARDS config, topology and N-way store wiring (#4764)
Part of the RunOps N-way sharding work. This lets the webapp hold N run-ops stores, configured by a single `RUN_OPS_SHARDS` JSON descriptor, and routes to them through the existing keyed router. **Inert with `RUN_OPS_SHARDS` unset** — the topology, the wiring and `ROUTING_ENABLED` are byte-identical to today. ## What's here - **`RUN_OPS_SHARDS`** — a zod-validated JSON array of shard descriptors (`key`, `region`, `url`, `replicaUrl`, `directUrl`, `replication`, `knobs`, `aliasOf`), validated at boot in the `parseMachinePresetCsv` style. Unset or `[]` → no shards. - **One run-ops client factory** — `buildRunOpsWriterClient`/`buildRunOpsReplicaClient` collapse into one `buildRunOpsClient` parameterized by role and resolved pool knobs. The control-plane builders (`buildWriterClient`/`buildReplicaClient`) are a separate path and stay untouched; every resolved value matches the former builders. - **Shard loop in `selectRunOpsTopology`** — one client pair per descriptor; an `aliasOf: "new"` descriptor reuses the new store's clients by reference and opens no pool. - **N-way `buildRunStore`** — builds N dedicated stores + the keyed router via a new `RoutingRunStore.fromShards`, keeping the two-store compat router when no shards are configured. - **`UnknownShardKey`** — raised when an id resolves to an unconfigured key; never falls back to another store. `fromShards` injects `resolveShard` so a gen-2 id routes to its own shard. - **Per-shard transaction resilience** — each shard gets its own retry budget. - **Mint bound** — `computeMintShard` intersects the active mint list with the configured descriptor keys, so a key with no descriptor is never minted into. - **Boot table** — logs `key`, address fingerprint (host:port/db, no credentials), and role, only when shards are configured. ## Ordering constraint Do **not** configure a `RUN_OPS_SHARDS` descriptor in any environment until the routing-semantics change (TRI-13427) lands — three fan-out sites still truncate at N>2. Merging this PR alone is safe (inert with the var unset); configuring a descriptor is what must wait. ## Testing - Run-store corpus: green with zero test-file diffs (the bit-identical proof for the compat router). - `runOpsDbTopology.test.ts` 17/17, `runStore.server.test.ts` 4/4, `runOpsMigration` family 149/149. - New unit suites: descriptor validation, pool-knob value tables, `fromShards` routing + `UnknownShardKey`, boot-table formatter, mint bound. - typecheck (webapp + run-store), knip, lint, format: pass. ## Changelog Internal run-ops sharding infrastructure. No changeset or `.server-changes`: the change is inert with `RUN_OPS_SHARDS` unset and has no user-visible behaviour. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
DKP
|
ba57c1fc74 |
fix(webapp): disable browser autofill on environment variable inputs (#4777)
The environment variable key and value inputs did not set an autocomplete attribute, so browsers could offer to autofill or save typed values as saved credentials. This sets `autoComplete="off"` on those inputs in both the create and edit forms, matching the `autoComplete="off"` convention already used on the other credential-name inputs. `autoComplete="off"` is a best-effort hint. Browsers may still ignore it for password-typed fields, so this is defense-in-depth hardening, not a hard guarantee that a password manager cannot store the value. |
||
|
Saadi Myftija
|
6a6f0a4960 |
feat(webapp): pause deployment log auto-scroll on scroll-up (#4776)
Auto-scroll now only follows while you are at the bottom. Scrolling up pauses it; scrolling back to the bottom, or clicking the new scroll-to-bottom button in the log header, resumes it. When you are at the bottom the same button scrolls to the top. Switching to another deployment starts at the bottom again. |
||
|
Daniel Sutton
|
97d70b8906 |
feat(run-store): make the run-ops router correct at N shards (#4771)
## What
Makes `RoutingRunStore` correct when the run-ops layer routes across
more than two Postgres stores. Today it routes between a gen-1 `new`
dedicated database and a `legacy` control-plane database; this
generalizes every routing policy to N shards while keeping the two-store
behaviour byte-identical.
The change sets the four routing decisions that were implicit in code
order, and fixes one hazard that failed silently:
- **Id → shard key.** The router resolves a shard key with
`resolveShard` instead of the binary residency classifier, so a gen-2 id
reaches its own shard through the keyed map.
- **Membership vs routing.** `#distinctStores` (one entry per physical
database, aliases excluded by a declared `aliasOf`) drives every sum,
probe, and merge; `#shards` drives routing. An aliased shard can no
longer make a sum count one database twice.
- **Probe order.** A keyless lookup stays a sequential short-circuit at
two stores; above two it fans out in parallel, picks by precedence,
tolerates a single down leg, and keeps the canonical not-found throw on
the legacy leg.
- **Precedence and duplicates.** One merge helper across all four merge
sites. A duplicate id confined to `{new, legacy}` stays silent (the
known drain-mirror case); any other cross-shard duplicate increments
`runops_shard_duplicate_id_total` and logs at error level.
- **Disjoint sum (the silent hazard).** `countPendingWaitpoints` and the
waitpoint collector now partition absent ids by shard and **union by
id** rather than summing counts. A drain-mirrored waitpoint on both
gen-1 stores is counted once, so a blocked run can no longer hang
forever on a double-counted pending waitpoint.
- **Waitpoint completion.** A gen-2 waitpoint completes on its own
shard, overriding the legacy pins; a cuid waitpoint keeps its two-member
gen-1-pair probe unchanged.
- **Fail-loud creates.** A create with no shard key throws instead of
silently defaulting to `new`. An id resolving to an unconfigured shard
throws instead of being dropped.
Two new counters are exported: `runops_shard_duplicate_id_total` and
`runops_waitpoint_probe_fallback_total`.
## Why it is safe to merge
With only `{new, legacy}` configured every generalized rule reduces to
today's behaviour. `resolveShard` returns exactly what the old
classifier returned for every id shape that exists today, and no gen-2
id is minted yet. The only intentional behaviour change is the fail-loud
create throw; an enumeration of production call sites confirmed no
caller trips it.
## Testing
- New container-free algebra suite (50 cases) over probe order,
precedence, the duplicate alarm, the disjoint-sum partition, the
waitpoint probes, and the fail-loud paths.
- New `runOpsStore.nShardMatrix.test.ts` runs a four-store matrix
(legacy + new + two gen-2 shards) against real Postgres containers: the
disjoint-sum union, the alias topology, cross-tree completion,
pagination merges, and mixed-id hydration.
- New `makeNShardRunOpsPostgresTest(k)` fixture in
`@internal/testcontainers`.
- Full run-store corpus green: 71 files, 480 tests. Typecheck, lint,
format, and knip all clean.
## Notes
- Draft: opened for review; not marking ready yet.
- No changeset or `.server-changes` file: internal routing
infrastructure, no user-visible behaviour change.
- TRI-13427.
|
||
|
Saadi Myftija
|
ee29393862 |
perf(webapp): cache deployment logs across navigations (#4775)
Switching between deployments in the dashboard re-fetched the whole build log stream from record zero and re-rendered the list line by line every time. Logs are now cached per deployment for the lifetime of the tab: revisiting a deployment shows its logs immediately, and the stream is resumed from the next unread record rather than restarted. Finished deployments whose stream has been read through the `finalized` event are served entirely from the cache. ### Changes The stream/cache logic moved out of the route into a `useDeploymentLogs` hook. On each deployment switch it seeds state from the cache, resumes the S2 read session at `nextSeqNum`, and writes back on cleanup or natural session end. Completion is derived from the stream's own `finalized` event (plus a terminal deployment status), not from the session closing, so a session cut short by token expiry or a proxy cannot pin a truncated log in the cache. Memory is bounded by a small LRU (`deploymentLogsCache`): at most 20 deployments and 20,000 log lines in total, least recently viewed evicted first. The most recently viewed deployment is always kept, so a single very large log can temporarily exceed the line budget on its own. Records are batched into one state update per tick instead of one per line. |
||
|
Eric Allam
|
47ff76d727 |
feat(webapp,clickhouse): return an actionable error instead of a 500 when a runs list query is too expensive (#4773)
## Summary When a runs list query is too expensive to complete, it now fails with a clear, actionable error instead of a generic 500. Previously, a runs list query that exceeded ClickHouse resource limits threw an opaque error. On the public `runs.list` API that surfaced as a retryable 500, so a customer task calling it would keep retrying a query that could never succeed. On the dashboard it rendered as a generic error page with no hint about what to do. ## Fix The ClickHouse client now tags resource-limit failures (memory, time, rows, bytes) with their error type, and the runs repository maps those to a dedicated `RunsListQueryError` (HTTP 422). - `runs.list` API returns 422 with a message telling the user to narrow their `created_at` range, plus an `x-should-retry: false` header so the SDK does not retry it. - The dashboard runs list (and the errors, scheduled, standard-task, agents, and webhooks list views) render a shared error state with the same guidance, so a too-broad time filter is recoverable by the user. |
||
|
Saadi Myftija
|
1eda438a41 |
feat(webapp): put the admin dashboard behind an env var flag (#4774)
Adds an `ADMIN_DASHBOARD_ENABLED` env var (default: enabled) that turns the admin dashboard and user impersonation off for an entire instance. When disabled: - every admin dashboard page redirects away, and the admin navigation isn't rendered - existing impersonation cookies are ignored, and any lingering session is actively terminated with an audit record - every flow that could start an impersonation responds 404, and no impersonation tokens are minted Stopping an impersonation always works regardless of the flag, so nothing gets stuck. Machine-to-machine admin API endpoints are not affected. The variable is documented for self-hosters; instances that don't set it are unaffected. |
||
|
Daniel Sutton
|
45eaaa7bd7 |
feat(run-store,testcontainers): execution-snapshot read comparator and shared test utilities (#4772)
## Summary Adds the read comparator for the in-progress migration of the run execution-snapshot log from Postgres to Redis. The comparator samples a single read against both stores, normalizes the two results to one shape, and reports any per-field difference with a tagged metric. It never serves a read itself: the diff layer imports only types, so it cannot hold a store client, and a test enforces that by failing if any value import appears. Also adds a combined Postgres-and-Redis test fixture and two shared test utilities (a cluster-slot assertion and a generic fault-injection harness) that the parallel Redis-store work reuses. Everything here is inert. Nothing constructs the comparator, so merging changes no runtime behavior. It becomes active only when a later change turns on compare mode. ## Notes The divergence classes separate real differences (scalar, ordering, waitpoint id set, validity, missing on one side) from two expected classes that must not be driven to zero: a rotated idempotency key, and a Redis-only surplus at a since-cursor tie. The since comparison is direction sensitive: a Postgres-only entry at the cursor is always a lost write, never an expected tie. |
||
|
Oskar Otwinowski
|
036cf8d2c8 |
chore(webapp): admin endpoint to backfill Vercel deployment external ids (#4770)
Skew protection resolves a run's worker by (environmentId, externalId, status=DEPLOYED). A miss parks the run and then expires it, so deployments predating the feature — which already carry the same value in commitSHA — need externalId populated to stay reachable. Vercel instant-rollback is the sharpest case, which is why the scope is the current promotion plus a recent window rather than current alone. Follows the existing backfill shape: admin PAT, keyset cursor over environments, per-environment action results, pMap, dryRun defaulting to true. Reuses normalizeExternalDeploymentId so a backfilled id is byte-identical to what a build writes, and the update re-checks externalId IS NULL so a deploy landing mid-backfill keeps its own id. Refs TRI-13464. |
||
|
Eric Allam
|
11e1cd8174 |
feat(webapp): isolate the runs list ClickHouse read pool (#4763)
## Summary Improves the performance and reliability of the runs list and the `runs.list` API, especially for large projects and filtered views. ## What changed - **Filtered runs-list queries use `PREWHERE`.** Immutable and additive-only filters (tags, task identifier, version, queue, region, machine, and the rest) are applied in `PREWHERE` on the `task_runs_v2 FINAL` scan, so ClickHouse filters, and uses the tags skip index, before it reconciles versions and materialises the wide columns. Same results, far less memory per query. `status` stays in `WHERE`: it changes across a run's versions, so filtering it before `FINAL` could return stale rows. - **The runs-list ClickHouse pool gets per-query guardrails**, all env-configurable: a `max_execution_time` paired with the client request timeout, a per-query `max_memory_usage`, a `max_threads` cap, and `readonly`. Each bounds a single query to itself, so a heavy query can't affect other queries, and they are safe as pool-level settings only because this pool is read-only. - **Billing and bulk count reads move to the read pool**, off the write pool. Defaults are conservative for self-hosters; production values are set via env. |
||
|
nicktrn
|
2e87e93934 |
ci: run codeql on all prs via advanced setup (#4767)
Default setup doesn't run CodeQL on pull requests from forks, so external contributions are stuck on PR checks that never come. Advanced setup fixes this. Languages, categories and `main` coverage match the current default setup. The bare `pull_request` trigger (no `branches` filter) keeps stacked PRs scanned, whose base isn't `main`. Default setup has to be disabled in Settings -> Code security for these uploads to be accepted. Until it is, the CodeQL check here fails with `CodeQL analyses from advanced configurations cannot be processed when the default setup is enabled`. |
||
|
Saadi Myftija
|
f866210388 |
feat(cli): experimental --local-bundle deploy mode (#4331)
Adds an experimental `--local-bundle` flag to native build deployments: the project is installed and bundled on the local machine (exactly like in the depot path) and only the resulting build context is uploaded. The remote build then runs just the container image build. ### Design - The uploaded artifact is the same build context classic deploys produce: bundled output, a synthesized package.json with the resolved externals, build.json, and the generated Containerfile. The bundle is secret-free: build.json is deliberately scrubbed because it is copied into the image, and build-arg values never enter the bundle at all. - Build-arg values are sent with the deployment initialization request instead, stored encrypted (aes-256-gcm) in a new `WorkerDeployment.buildEnvVars` column, and cleared on every terminal status transition. They exist at rest only for the active build window, always encrypted. - A dedicated `GET /api/v1/deployments/:id/build-env-vars` endpoint returns the decrypted values to the same principals that can already read the environment's variables. It answers with an empty record for deployments without stored values or in a terminal state, keeping secret access to a single auditable route. - Size limits are enforced server side and pre-checked client side. If the server does not acknowledge storing the values, the CLI fails fast instead of letting the remote build run without them. - A `--from-bundle <dir>` mode builds a deployment image straight from such a bundle directory, skipping config loading and bundling entirely. In attach mode it fetches the stored build-arg values through the new endpoint. - Env var syncing (the `syncEnvVars` extension) happens client side, before the deployment initializes, since the remote side never sees the unscrubbed manifest. - Bundle artifacts use a distinct type and storage prefix so the server can always distinguish them from source uploads. |
||
|
Daniel Sutton
|
cc69ff4d26 |
feat(run-engine): Redis waitpoint store coordinator, Lua protocol, and waitpoint ids (#4761)
Builds the Redis-backed half of the waitpoint coordinator, beside the Postgres coordinator that #4753 extracted. Adds the coordination protocol as Lua scripts, the run-ops-format waitpoint id scheme, and the key layout. **No caller wires any of it up.** Refs TRI-13440. ## Inert by construction Merging this changes nothing observable. 3180 insertions, **zero deletions**, nine new or additively-edited files. - `WaitpointStoreCoordinator` is never constructed outside its own tests and the benchmark. - No env var, no config plumbing, no connection. It takes `redisOptions` as a constructor argument. - `waitpointSystem.ts` is untouched. Every live waitpoint operation still runs on Postgres through the coordinator merged in #4753. - No changeset and no `.server-changes` note — nothing here is user-facing yet. Deploying this needs no Redis or MemoryDB instance. That becomes a prerequisite when a later change routes traffic onto the store behind a per-organisation flag. ## What's here **Nine Lua scripts**, each atomic on one hash tag. Seven mutate state — create-if-absent, register-or-report, complete, idempotency reserve, absorb, deliver, clear. One reads state (`runReadBlockState`) and is separate because the pending, delivered and edge sets must be read as one consistent view. One discards an idempotency loser. **Two hash tags, deliberately.** `wp:{waitpointId}` holds a waitpoint's record, status, completion envelope and watcher hash. `wp:run:{runId}:*` holds one run's pending set, delivered set and edge set. A waitpoint has N watchers, so it cannot live under any single run's tag. **Waitpoint ids** reuse the run-ops body layout: a 24-char base32hex core, a type char (`r`/`b`/`d`/`m`), and version char `w`. RUN and BATCH ids derive from their anchor's core, so create-if-absent is idempotent with no lock. `parseWaitpointId` is total and never throws. **The single-slot guard.** Every script invocation goes through one private wrapper that asserts all keys share a hash tag. A single-node test server accepts what a real cluster rejects, so this assertion is the only enforcement — and it is mutation-tested: removing it fails a test. ## Measured Against the same population of real Postgres rows: | | store | postgres | |---|---|---| | pending count (the blocked/unblocked gate) | 0.13 ms p50 | 3.32 ms p50 | | full-payload read | 1.45 ms p50 | 7.70 ms p50 | Both are lower bounds: the benchmark charges Postgres a `COUNT(*)`, while the resume-time read is a join with a partial select plus filtering in JavaScript. Store-only paths, no Postgres counterpart: block+complete+deliver 0.88 ms p50; 100-watcher fan-out 13.8 ms; a 1001-edge fan-in 149.8 ms, flat at 0.15 ms per edge and round-trip bound rather than algorithmic. The benchmark lives in `*.bench.test.ts` and is excluded from the default suite. ## Review notes - **The type surfaces are not reconciled yet, on purpose.** `types.ts` (from #4753) carries the coordinator interface; `storeCoordinator.ts` declares its own operation types because this was built in parallel. The wiring change reconciles them. - **The read-time resolver is not here.** Another lane froze its contract while this was in flight, and its frozen types are not yet on main. Building a second copy would fork a just-frozen contract. - **Teardown is one-shard while registration is two-shard.** A terminal clear leaves a run registered as a watcher on the waitpoints it was blocked on, because the watcher hash is under a different tag and no script may span slots. Recorded, not fixed here — it needs a retention decision, and nothing observes it while the code is unwired. ## Verification 79 tests in the coordinator suite, 58 in the id suite. `typecheck` on run-engine and webapp, `build` on core, `knip`, `oxfmt` and `oxlint` all clean. The engine corpus passes 82/82. Every invariant is mutation-tested rather than merely asserted. A whole-branch review ran 14 mutants and killed 12; the two survivors were fixed with their own mutation checks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Daniel Sutton
|
f98e303292 |
feat(webapp): resolve which shard an environment mints run roots into (#4755)
## Summary Adds the shard-selection stage of run-id minting. `resolveMintShard(env)` returns which run-ops database an environment mints its new run roots into: the active shard list, then a fleet-wide override, then a per-environment or per-organization pin, then a rendezvous hash of the environment id. That half is inert. Nothing calls `resolveMintShard`, no deployment has any of the new flags set, and an empty active list returns the current answer without reading anything. **The other half is not inert, and it is where review effort belongs.** To stamp a grace window this needs a read-then-write under a lock, so it rewrites the global feature-flag write path that `runOpsMintKind` already depends on in production. See below. ## Placement Resolution reads the active list from a global flag, applies the grace window, and then picks: - a fleet-wide override if one is set, which is how a cutover completes without visiting each organization. `new` holds the whole fleet on the current id format. - otherwise a per-environment or per-organization pin. `new` holds one organization back while the rest move, which is how a canary works. - otherwise a rendezvous hash, so adding a shard moves only about 1/(N+1) of environments and removing one moves only its own. Two hash details are load-bearing. Scores are 64-bit `sha256(envId \0 key)`, because a 32-bit score collides at our environment count and an undetected tie would resolve by iteration order. The parsed key list is sorted, because otherwise two deployments listing the same shards in a different CSV order would place environments differently. A pin or override naming a shard that has left the active list falls through to the hash and reports once. Honouring it would leak the drain the active list exists to perform, and throwing would fail triggers whenever a pinned shard drains. ## Why the active list is a flag and not an environment variable A deploy rolls for hours, so two pods hold two different environment values at the same time. A list held in the environment therefore splits the fleet for the length of the rollout, with new pods placing an environment on one shard and old pods on another. A grace window measured in seconds cannot cover that, and the same knob times the existing mint-kind flip so it cannot simply be lengthened. An environment variable also cannot record its own flip time, and an operator cannot know a rollout's end in advance. So the list, its grace stamp and the override are global flags, written server-side against the control-plane clock under an advisory lock. This branch adds no environment variables. ## The write path, which is live Stamping generalises to any number of graced flag groups in one transaction under one lock. That has three consequences a reviewer should look at directly: - It closes a real bug. `runOpsMintKind` is an editable control on the global flags page, and that page previously wrote it with a bare upsert: no lock, no stamp. An operator flipping mint kind through the UI got an ungraced flip, so every pod crossed the cutover at a different moment. Verified against a running instance, before and after. - A graced group is all-or-nothing. Submitting its primary writes the group with a fresh stamp; omitting it deletes the primary and its stamp together, because a stamp left without its primary keeps being served and would mint into a shard just removed. - The advisory lock takes the previous id as well as the current one, in a fixed order, so writers on an older release still serialise during a rollout. The legacy id can be dropped one release after this ships. This folds with #4751 rather than replacing it: its `unlockLockedFlags` rule decides what the sweep may delete, and the graced groups keep their stamp under the lock. Both sets of tests pass. ## Notes for review Determinism is a property of the pure core for fixed inputs. The wrapper supplies the clock, the same split `effectiveMintKind` already uses. A failed read of the list falls back to the current id format rather than guessing. Six flags appear in the admin pages immediately. The two pins are per-organization, so they render read-only on the global page. The list, its stamp and the override are deployment-wide, so they render read-only in the organization dialog. Nothing bounds the active list against shards that actually exist. That is safe while nothing mints, but the change that carries a shard key into an id must land after the shard descriptors bound the list, or bound it itself. |
||
|
Daniel Sutton
|
b55fba9e06 |
feat(run-store,run-engine): freeze the completed-waitpoints record and resolver contract (#4760)
Builds on [#4754](https://github.com/triggerdotdev/trigger.dev/pull/4754), which added the store this contract belongs to. ## Why Two migrations are moving to Redis in parallel, and execution snapshots reference completed waitpoints across the boundary between them. If the record shape is agreed only once both halves are built, the correction lands mid-rollout: dual-write is live, real keys are in Redis, and changing the entry format then means two versions of the entry coexisting plus a migration for whatever was already written. Agreeing it now, while nothing writes a pointer, makes that same correction a type edit. The reserved-and-empty field is the same argument one level down. The entry format is what dual-write writes, so adding a field to it later splits the format in two. Reserving it before any write means the format never changes after writes begin. ## Summary Adds the type contract for carrying completed waitpoints alongside the Redis-backed execution-snapshot store: a `{cycleSeq, count}` pointer on the snapshot entry, the record shape that pointer resolves to, and the read-time resolver signature. Nothing constructs or reads a pointer yet, so this is inert on merge. The record shape has to reproduce `enhanceExecutionSnapshotWithWaitpoints` field for field, because that is what the executor consumes. A conformance test runs the real function against a reference resolver over an exhaustive grid of 6144 input combinations, derived from every `Waitpoint` column the function reads rather than hand-picked. ## Design `completedWaitpoints` is reserved on the entry type and always unset. `append()` rejects a set value, because the pointer's physical home is the `<snapshotId>#c` sidecar field rather than the entry JSON. The append script mints both halves after the client serializes the entry, and the entry JSON has to stay byte-identical to the Postgres row so the two can be compared during a dual-write rollout. Two rules are worth calling out, both found by making the test fail rather than by reading the code: * `records` is the authoritative waitpoint set, not `order`. Only batch waits carry an index, so `order` is empty for a single `triggerAndWait` while the Postgres join still holds the id. Comparing id sets over `order` would serve the previous wait cycle's records. * `deriveFromRun` requires a non-null `completedByTaskRunId`. `Waitpoint.completedByTaskRun` is `onDelete: SetNull`, so an orphaned RUN waitpoint keeps its output with no run left to derive from. Those records carry their output inline instead. `tsconfig.freeze-test.json` typechecks the conformance test, which the package build config excludes. Without it, renaming a field in the frozen type compiles clean and every test stays green, so the literal assertions in the test would only pin the test's own writer. ## Fixes carried along Auditing the contract surfaced three defects in the append script, each with a regression test that fails when the fix is reverted: * A new wait cycle now clears any `records` left on a reused key. A `seq` counter lost to eviction can re-mint a `cycleSeq` whose key still holds another cycle's records, and `order` and `count` are overwritten together, so the mismatch check could not see the drift. * A carry-forward now attaches a pointer only if the current keyspace incarnation actually minted that cycle. The previous key-exists check adopted a dead incarnation's records under a count that agreed with them, reporting no mismatch. * The cycle-key size metric now counts `records`, not only `order`. It reported 7 bytes for a 20 KB key, so the high-water log could never fire on the field that grows. |
||
|
nicktrn
|
d6457521cb |
fix(hosting): disable clickhouse system-log telemetry and apply profile settings via users.d (#4762)
Carries over the self-hosted ClickHouse fix from #4546 by @Leafgard, whose commits are preserved here, plus follow-up polish. Opened in-repo because the fork is org-owned, which GitHub's "Allow edits from maintainers" doesn't cover. fixes #4343 ## What was wrong Two independent problems in `hosting/docker/clickhouse/`: 1. **The `<profiles>` block never applied.** It sits in `override.xml`, mounted under `config.d` - but ClickHouse only reads profile settings from the users config tree. Verified on the pinned image: before this change `max_block_size` sat at its default `65409` with `changed=0`, so the advertised low-memory settings had never taken effect at all. 2. **Every ClickHouse system log table was enabled and unbounded.** On a sub-16GB machine their background merges outgrow the memory cap; ClickHouse's [low-RAM guide](https://clickhouse.com/docs/operations/tips) recommends disabling them. The dev stack already does this - `hosting/docker` never got it. ## What this does - `clickhouse/override.xml`: disables the high-frequency telemetry tables, and bounds the ones worth keeping with a config-level `<ttl>` - `query_log` and `part_log` at 7 days, `error_log` at 30. A config-level TTL survives log-table recreation, unlike `ALTER ... MODIFY TTL`. - New `clickhouse/users-override.xml`, mounted at `users.d/override.xml`: carries the profile settings so they actually apply, completes the sub-16GB set with `max_threads=1`, and zeroes the memory/query profilers, whose samples were the main source feeding `trace_log`. - `webapp/docker-compose.yml`: adds the `users.d` mount. ## Verification Ran `clickhouse/clickhouse-server:26.2` with these exact mounts, and `25.12` to cover the documented 25.8 floor: - All 9 profile settings report `changed=1`, and a custom `CLICKHOUSE_USER` inherits them. - `users.d` merges rather than replaces: the `default` user, its password, `access_management` and the `readonly` profile all survive, so the compose healthcheck still passes. - `remove="1"` is a clean no-op on keys absent from a given version - no empty section, no accidental table, no startup error - so pinning `CLICKHOUSE_IMAGE_TAG` to an older supported tag won't crash-loop. - TTLs land in the real DDL: `TTL event_date + toIntervalDay(7)` / `(30)`. - In-place upgrade on a populated volume: clean restart, data preserved, and ClickHouse lazily renames the pre-existing `query_log`/`error_log` to `query_log_0`/`error_log_0` as it applies the new retention. ## Notes for review - **`part_log` is kept (bounded) rather than disabled.** It appears in neither report behind this change and isn't on ClickHouse's sub-16GB list, but it's the merge history you'd need to diagnose a recurrence. Measured at ~0.18 KiB per part event under insert churn - about 10x cheaper than `text_log` over the same window - so a TTL bounds it rather than removing it. - **The profile settings go live for the first time here.** On larger machines that's a real, intended throughput change: `max_threads=1`, `max_download_threads=1`, parallel parsing and formatting off. - **Disabling a log table stops new writes but doesn't delete existing data.** Reclaiming disk on an existing deployment needs `DROP TABLE system.<name> SYNC`, including the `*_log_0` leftovers. ## Known gaps, deliberately not in this PR - The Helm chart carries the same ineffective `<profiles>` block in `values.yaml` and mounts nothing into `users.d`, so this fix isn't currently expressible there. - `background_schedule_pool_log` is enabled by default with no TTL and is disabled by neither stack. - The dev stack's disable list has drifted from this one. - The compose healthcheck still logs a query every 5 seconds. --------- Co-authored-by: Yann SEGET <yann.seget@actemium.ch> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
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. |