Commit Graph

5023 Commits

Author SHA1 Message Date
Katia Bulatova 643ea3907f fix(dashboard-agent): read the report's untrustworthy reason under its current name
`curateReport` still read `facts.staleReason`, renamed to `untrustworthyReason`
in dc3b50260 and split into telemetry_stale / telemetry_absent / flow_unmeasured.
The read had been undefined since, so the agent got "untrustworthy" with no why,
and the prompt still told it every such case was stale telemetry.

`facts` is `z.record(z.unknown())`, so nothing typechecked the key. The new test
goes presenter -> reports route JSON -> curateReport without naming a facts key
on the way in, and asserts curation carries every key the presenter emits.
2026-08-08 17:03:53 +00:00
Katia Bulatova ad2698bdcc feat(webapp): render the cards the flows already emit
The prompt requires render_view and the schema union allows actions, investigation
and report blocks, but ViewBlocks only had cases for diagnosis and chart, so those
three rendered as an empty div. Adds the three renderers, the block-envelope
latest-wins resolution the switch keys on, and a contract test that reads the block
types off viewBlockSchema, so a new union member fails until it has a renderer.
2026-08-08 17:03:52 +00:00
Katia Bulatova 3355b6813e fix(webapp): stop an unmeasured queue depth from silencing a measured flow finding
An unmeasurable depth made the whole flow finding unassessable, so a
critical start latency measured off runs showed as crit in format=json
while the rendered report said 'Flow unknown ... nothing to do' and
dropped the row that earned it. The depth still buys no cause, no
attribution and no drain ETA, but the measured symptom is now reported.
2026-08-08 16:23:04 +00:00
Katia Bulatova a66034930c fix(webapp): make each report caveat discount only the input it names
A missing telemetry feed said the run aggregates were a point-in-time
snapshot; they are measured over the window either way, and what is
actually unknown is how current they are. An unmeasured queue depth
blamed throughput, which the report does measure, and contradicted its
own headline. Tests pin the claim rather than the wording.
2026-08-08 16:23:03 +00:00
Katia Bulatova d0f06d5c5e fix(webapp,dashboard-agent): address a branch environment by name and branch
The agent could not read anything on a preview or dev branch. Its environment name
is derived from the environment's type, and every branch shares its parent's type —
so the name identifies a family, not a row. The API's env routes are name-addressed,
so a bare "preview" or "dev" resolved to the parent, and the delegated token's
`environmentId` claim then correctly refused it. The guard was the detector, not the
defect: the exchange was never given enough identity to resolve the environment the
dashboard had selected.

Name and branch are now resolved together into one address, so no caller can take
the name without the branch, and both mint sites share the one type map instead of
keeping a copy each. The address travels to the JWT exchange and to the three
delegated-token reads that resolve by name (list_tasks, correlate_version, the repo
snapshot). The env-JWT reads address the environment by id and are untouched.

Second, and why nobody saw the first: the exchange reported the same "no
environment" for a genuine absence and for any failure, and cached the failure for
the whole turn. Following the queue live-read precedent, only a missing environment
is stated as one; anything else says the read didn't land and carries its status.
2026-08-08 16:21:48 +00:00
Katia Bulatova a4ba0271b0 fix(webapp): refuse a delegated token at the entrance of the PAT-only auth helper
The helper checks no scopes and no capability context, so a read-only user-actor token
reached an alert-channel write and every admin route behind requireAdminApiRequest.
Actor-aware routes are unaffected: they authenticate the token through the route
builders, which enforce its claims.
2026-08-08 16:19:12 +00:00
Katia Bulatova 314a1d795d perf(webapp): skip the global feature-flag query when a per-org override resolves
flag() queried the FeatureFlag row before looking at the caller-supplied overrides, so a
per-org hit still paid a round-trip. Check the override first and only fall through to the
query when it fails the schema, which keeps today's resolution order intact.
2026-08-08 13:59:06 +00:00
Katia Bulatova c3f0d62d62 fix(webapp): undo a new agent chat only when its start never got anywhere
A start that rejects dispatched no handover and sent no message, so the chat
row is taken back. A failed access-token mint is left alone: the session is
live by then and removing the chat would hide a running agent.
2026-08-08 13:46:01 +00:00
Katia Bulatova 14d70bef9a docs(webapp): say that the agent is off until a flag turns it on 2026-08-08 13:45:53 +00:00
Katia Bulatova e2704eaa84 fix(webapp): keep identityOnly off action routes in the type
identityOnly waives the contextless refusal, which is only sound for reads.
The action options intersected the loader options, so the type allowed it.
2026-08-08 11:52:37 +00:00
Katia Bulatova de4cdad61b fix(webapp): list the preview branch an agent token is scoped to
A token signed for a branch child listed nothing: the scope filter and the
base-environments-only filter could never hold together. Unscoped callers
still see base environments only.
2026-08-08 11:52:36 +00:00
Katia Bulatova b38c5186ac fix(webapp): stop a failed agent chat start leaving an empty chat behind
The environment lookup, repo lookup and token mint now all run before the chat
row is created, so a 404 or a mint failure can't orphan a chat in the history.
2026-08-08 11:52:35 +00:00
Katia Bulatova f598f96142 fix(webapp): refuse a dashboard agent turn whose token mint failed
The catch around the turn body tolerated a JSON.parse failure, but it also
swallowed a rejected mint and forwarded the turn with no credential.
2026-08-08 11:52:34 +00:00
Katia Bulatova 4c422300c8 fix(webapp): stop treating a ClickHouse unknown identifier as a rollout gap 2026-08-08 09:22:48 +00:00
Katia Bulatova d0be659457 test(webapp): assert the queue depth trend fills its bucket grid 2026-08-08 09:22:47 +00:00
Katia Bulatova 0db1cf0d23 fix(webapp): say why a report's numbers can't be trusted
Absent telemetry and an unmeasured flow were both labelled stale data, so every
snapshot-based report claimed staleness it could not have measured. Choose the
badge and caveat from the reason instead.
2026-08-08 08:29:47 +00:00
Katia Bulatova 1ee6704d53 fix(webapp): describe how far a report metric fell
A fall's multiplier rounds to 0 or 1, so every drop rendered as "flat" — a metric
that collapsed from 100 to 5 read as unchanged. Measure the fall against the
baseline instead, and show a bare arrow when it collapsed to nothing.
2026-08-08 08:29:46 +00:00
Katia Bulatova 05505d5741 fix(webapp): carry a queue's depth forward across empty buckets
The per-queue metrics route mapped ClickHouse rows straight to an array, so a
bucket with no sample shortened the trend and shifted every later point in time.
Fill a fixed-width grid the way the two sibling callers do.
2026-08-08 08:29:46 +00:00
Katia Bulatova 2d23ddec17 fix(webapp): keep the retired chats.messages column, and allow Google SSO avatars
Dropping the column was irreversible and blocked on a production row count; retiring it is not. The avatar host is an exact origin the app already knows, like the GitHub one.
2026-08-08 08:29:43 +00:00
Katia Bulatova 5126bae044 fix(webapp): gate a run's commit metadata on reading deployments
The route served a deployment's git blob — commit message, author, branch,
PR title — with no ability check, while the deployments list serves the same
blob behind read on deployments. Apply that check here too.
2026-08-07 23:50:04 +00:00
Katia Bulatova cb0bcc92b9 fix(webapp): refuse an environment-scoped token on a route that names nothing
assertUserActorScope returned early whenever the passed scope carried no
org, project or environment, and the route builder passes {} for any route
that declares no context — so the guard was a no-op there. api.v1.orgs's
action is such a route and has no authorization block either, letting a
read-only agent token create an organization.

Fail closed instead, with an explicit identityOnly opt-in for the two
contextless loaders that answer with the caller's own identity, and give
org creation the gate its siblings have.
2026-08-07 23:50:02 +00:00
Katia Bulatova f89158477c fix(webapp): stub what the env JWT act-claim test's route actually calls
The test mocked the old preamble, so the route hit real rbac and a logger without
`info`, failing with a 403 and an uncaught type error.
2026-08-07 23:31:56 +00:00
Katia Bulatova bd357fc837 revert(webapp): put the JWT exchange back to how main had it
The scope ceiling rewrite landed here and was reverted two PRs up, leaving the
stack asserting both directions. The exchange intersects requested scopes with the
token's cap again, a capless token passes through like a PAT, and the route keeps
only the environment claim check and the acting client.
2026-08-07 23:31:55 +00:00
Katia Bulatova df2227e1a3 fix(webapp): resolve a test's route paths from the test, not the repo root
The queue-JWT test read its route sources through repo-root-relative paths, so it
never resolved from the webapp's own working directory.
2026-08-07 23:31:55 +00:00
Katia Bulatova 8adf2b2523 Merge remote-tracking branch 'origin/main' into feat/dashboard-agent-flows 2026-08-07 23:03:15 +00:00
Katia Bulatova c23660197f feat(webapp): let an environment JWT read a queue, as it already reads its metrics
The dashboard agent asks for a queue's live row — paused, depth, limit — through the environment JWT it exchanges for. The metrics route has accepted that JWT all along; the retrieve route answered 401, so the agent saw no queue at all.
2026-08-07 22:27:07 +00:00
Katia Bulatova 711b79ee8a fix(dashboard-agent): apply the token's cap as a second ceiling, and finalise only this turn's messages
Review of #4418: the cloud path builds the ability from the user's role, so a read-only delegated token could exchange it for a write JWT; and the finalisable set was the whole replayed transcript rather than what the turn produced.
2026-08-07 19:32:35 +00:00
Eric Allam c526528d8f feat(webapp,database): bound Prisma list filter arity (#4480)
⚒️ Publish Worker (v4) / build (supervisor) (push) Has been cancelled
## Summary

Prisma expands `in` / `notIn` into one bind parameter per element, so
every distinct list
length is a separate prepared statement. Where the length tracks data
volume (a batch size,
a run-graph fan-out, a prior query's id set) one call site can mint
hundreds of them. Each
is used about once, but inserting it evicts an entry that was being
reused, so the cost
lands on unrelated queries sharing the pooler's statement cache. An
unbounded list also
risks the 65535 bind-parameter ceiling.

`boundedIn()` pads a filter list to the next power of two by repeating
its last element.
`IN` and `NOT IN` ignore duplicates, so results are unchanged, and a
call site drops from
one statement per length to at most `log2(cap)`. Applied to all existing
sites.

## Enforcement

Two oxlint rules require the helper: a list filter must be an inline
array literal or a
`boundedIn()` call.

- The first covers filters reached through `where` / `having` /
`cursor`, and deliberately
never descends into `data`, `create`, `update`, `set` or `equals`. A key
named `in` in
those positions is user data, not a predicate, and rewriting it would
corrupt what gets
  stored or compared.
- The second covers bare filter objects passed to where-building
helpers, which the first
cannot see. It found five sites in the run-graph batch loaders that were
otherwise
  invisible.

Both rules follow filters through the shapes they are actually written
in: conditional
expressions, logical-and objects, spread-conditional properties,
computed keys, and call
arguments. An array literal only counts as fixed-arity when nothing
spreads into it, since
`[...new Set(ids)]` has a runtime length. Twelve sites were hidden
behind those shapes
until the rules handled them.

Scoped to `in` and `notIn`. The scalar-list filters `hasSome` and
`hasEvery` compile to
`&& $1` and `@> $1`, passing the whole array as a single bind parameter,
so their arity never
reaches the statement text and there is nothing to bound.

Both rules are `error`, so new call sites fail CI. That ratchet has
already caught four
sites added by other PRs while this one was in review.

## Notes

`boundedIn` pads by repeating rather than with null: `x NOT IN (a, b,
NULL)` is never true,
so null-padding a `notIn` filter would silently return no rows. Lists
above 32768 are
returned unchanged so padding can never push a query past the parameter
limit.

Route modules reach the helper through `~/db.server` rather than
importing the database
barrel directly, since a value import of that barrel into a module that
also exports a React
component is only safe while dead-code elimination prunes it.

Measured on a local rig: 300 distinct list lengths produce 300 prepared
statements
unpadded, 10 padded. Verified end-to-end against a local stack with the
full task-suite
sweep, which surfaced no regressions.
2026-08-07 16:39:58 +01:00
Eric Allam 63176a6d69 fix(webapp): stop api inheriting inbound sampled traceparents so trace sampling applies (#4532)
## What

The internal tracing `ParentBasedSampler` in `tracer.server.ts` left
`remoteParentSampled` at its default of `AlwaysOn`. Any request arriving
with a `traceparent` whose sampled flag was set got recorded in full,
bypassing `INTERNAL_OTEL_TRACE_SAMPLING_RATE` entirely. Because the SDK
propagates its (always-sampled) trace context on calls back to the
platform from inside running tasks, the large majority of API server
spans inherited a sampled parent and ignored the divisor. The sampling
knob was effectively inert on the busiest service.

This registers a custom propagator
(`NonInheritingTraceContextPropagator`) that stops adopting the inbound
trace as the parent:

- `inject` still delegates to the standard W3C trace + baggage
propagators, so outbound propagation is unchanged.
- `extract` drops the parent span (`trace.deleteSpan`) while preserving
baggage, so every incoming request roots its own trace and the ratio
sampler applies uniformly.

`remoteParentSampled` is also set to the ratio sampler as a
belt-and-suspenders fallback, in case an inbound sampled parent ever
reaches the sampler another way.

Two effects: the divisor becomes effective on the API server, and the
API no longer stitches onto (and inflates) the propagated task-run
traces, which is where the very large, un-thinnable trace chains came
from. Rooting each request removes those chains rather than only
diluting them.

Only the internal APM trace pipeline
(`INTERNAL_OTEL_TRACE_EXPORTER_URL`) is affected. The user-facing
run-trace pipeline (`otel.v1.traces` -> ClickHouse) is a separate path
and is untouched. The only consumer of the global propagator's `extract`
is the OTel HTTP/Express auto-instrumentation, so the blast radius is
inbound-request trace shape.

## Evidence (local full-stack red/green, divisor 10)

A local OTLP/JSON sink counting spans; a driver fires N requests at a
real endpoint, each carrying a distinct sampled `traceparent`, then
counts how many spans/traces carry that run's marker.

| run | code | sent | kept traces | kept fraction |
| --- | --- | --- | --- | --- |
| before | unmodified | 500 | 500 | 1.00 |
| after | this PR | 500 | 67 | 0.134 |
| after | this PR | 2000 | 213 | 0.1065 |

Before: 100% of inherited-sampled requests kept, divisor ignored. After:
~10% kept (the divisor), converging on it at larger N. In every
after-run each kept request is a single self-rooted trace (kept spans ==
kept distinct traces), confirming the inherited chains are gone, not
just thinned. `typecheck` passes.

## Rollout / rollback

No flag. Behavior stays governed by the existing
`INTERNAL_OTEL_TRACE_SAMPLING_RATE`. Rollback is a straight revert with
no data migration.

## Notes

Internal dashboards that count raw span or request volume from this
pipeline will read lower once this ships. That is expected: those counts
were inflated by the bypass, not a real drop in traffic.
Latency/percentile monitors retain plenty of samples at the current
divisor.

refs TRI-13031
2026-08-07 15:13:42 +01:00
Katia Bulatova c025bbfcb4 fix(dashboard-agent): keep the finished answer in the transcript, not the mid-flight one
A turn stores its messages before the model finishes, so the completed bodies arrived against ids that already existed and were skipped. Reopening a chat then replayed a tool call that never ends.
2026-08-07 13:46:12 +00:00
Katia Bulatova 798fdf94b7 refactor(webapp): split the dashboard agent's UI out of the first PR
The system — contracts, storage, auth, the agent package and its webapp routes — lands first; the panel, the page-context marks and the entry points follow in their own PR.
2026-08-07 12:34:06 +00:00
Eric Allam 7246f677db fix(webapp): strip null bytes from idempotency and debounce keys at trigger (#4527)
## What

A trigger request carrying a Unicode NUL (`U+0000`) in the **idempotency
key** or **debounce key** reached `prisma.taskRun.create()` and failed
the insert, so the caller got an opaque 500 and the run was never
created.

These two keys are stored in `jsonb` columns (`idempotencyKeyOptions`,
`debounce`), and Postgres rejects a NUL inside a `jsonb` value with
`SQLSTATE 22P05` ("unsupported Unicode escape sequence ... cannot be
converted to text"). This fix strips the NUL from both keys at the
single trigger-input chokepoint (`#buildEngineTriggerInput`), which
every trigger path flows through (single, batch item, mollified, and
drainer replay).

Stripping matches the existing precedent for run errors and task events.
It does not change dedup behaviour: the idempotency **dedup identity**
is the hashed key (a clean 64-char digest), computed independently of
the raw key we clean, so dedup keeps working exactly as before. For
debounce the key is used directly, so the cleaned key also becomes the
grouping key, an acceptable change for input that is already malformed.

## Why not payload / metadata / tags

Those are `text` columns fed by `JSON.stringify`, which escapes a NUL to
a safe escape sequence, so they do not hit this failure on the normal
JSON path. (A raw NUL in a `text` column throws a different code,
`22021`, and is not what triggers this issue.) The observed failures are
the `jsonb` `22P05` variant, which is only reachable via the two key
fields.

## Evidence

Red then green (containerTest, real Postgres): with the fix reverted,
triggering through the real service with a NUL in
`idempotencyKeyOptions.key` / `debounce.key` fails with the exact
`22P05` signature; with the fix, the run is created and the stored key
has the NUL removed.

Full-stack e2e (isolated stack, real HTTP): `POST
/api/v1/tasks/:taskId/trigger` with a NUL inside
`idempotencyKeyOptions.key` (`"acme<NUL>inc"`) and, separately,
`debounce.key` (`"grp<NUL>1"`):

- both returned `HTTP 200` with a created run (previously `500`)
- stored `idempotencyKeyOptions` = `{ "key": "acmeinc", "scope": "run"
}` (7 chars, NUL removed)
- stored `debounce.key` = `"grp1"` (4 chars, NUL removed)
- both runs render in the dashboard

Unit tests cover the helper (strip, no-op fast path, object-reference
reuse, null/undefined pass-through).

## Rollout / rollback

Server-only webapp change, no flag. Zero behaviour change for clean
input; only affects inputs that previously 500'd. Rollback is a straight
revert, no data migration.

## Known limitation

A raw NUL in a plain-string idempotency key (not created via
`idempotencyKeys.create()`) lands in a `text` column and throws `22021`
instead. That variant is not addressed here because stripping it would
change the dedup identity, so it warrants a separate decision. Not
observed in practice.

refs TRI-13030
2026-08-07 13:28:52 +01:00
claude[bot] dc529414df feat(webapp): add /_/* redirect route (#4523) 2026-08-07 13:21:07 +01:00
Katia Bulatova dfcee8bf33 fix(webapp): settle the API keys route after merging main 2026-08-07 11:54:59 +00:00
Katia Bulatova db9f9a2502 Merge remote-tracking branch 'origin/main' into feat/dashboard-agent-flows
# Conflicts:
#	apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.apikeys/route.tsx
2026-08-07 11:50:37 +00:00
Katia Bulatova 9c6f14872f chore(webapp): keep Ask AI in the tree, deprecated and unmounted
Its old keystroke now opens Ask Trigger instead of nothing.
2026-08-07 11:40:29 +00:00
Chris Arderne 0a44b88b39 fix: security release 2026-07-21 (#4528) 2026-08-07 12:25:40 +01:00
Katia Bulatova 0d3d21659f feat(webapp): drop the agent button from the deploy blank states 2026-08-07 10:47:00 +00:00
Katia Bulatova c88a4483da refactor(webapp): rename the ask-ai button variant to ask-trigger 2026-08-07 10:43:34 +00:00
Katia Bulatova 5edfd78f80 feat(webapp): name the dashboard agent Ask Trigger everywhere
The launcher spelled it out while every other surface read it from the shared label.
2026-08-07 10:36:11 +00:00
Katia Bulatova c37bdf81cb test(webapp): pin the delegated token's scope ceiling on the RBAC fallback
Also collapses the environment guard's per-function docs into one file-level note.
2026-08-07 10:30:26 +00:00
Katia Bulatova 9fad66554e test(webapp): cover the dashboard agent's eval-policy gate, and require its token's environment
The mint's environment is what every environment-bound endpoint reads off the token, so it is now a required argument rather than an optional one.
2026-08-07 10:17:19 +00:00
Eric Allam db67a856fe perf(webapp,database): index the newest-task-version lookup (#4518)
📦 Preview packages (pkg.pr.new) / Build and publish previews (push) Has been cancelled
📚 Publish docs / publish (push) Has been cancelled
Implementing PlanetScale Insights improvement.

## Summary

Validating a schedule (creating or updating one through the API or the
dashboard, and deploying a project that declares schedules) looks up the
newest version of a task by slug. That lookup reads *every* version of
the task and sorts them to return one. A project gains a row per task on
every deploy, so the work grows with the project's age: the oldest
projects pay the most, and dev-mode redeploys make it worse. This was
picked because it was the largest single consumer of database time on
the schedules path, and the fix is a sort key with no index behind it.

## Fix

`BackgroundWorkerTask` is indexed on `(projectId, slug)`, which serves
the equality but not the `ORDER BY createdAt DESC`. Postgres seeks the
index, then bitmap-scans and top-N sorts the whole group to produce a
single row. Adding `createdAt` to the index lets it scan backward and
stop at the first row.

The same call site also selected all 21 columns, including five JSON
blobs, to read one field (`triggerSource`), so it now selects that field
alone.

## Benchmark

Local Postgres 17, 997,000 seeded rows / 748 MB, group sizes chosen to
match the distribution seen in production.

| Group size | Before | After |
| --- | --- | --- |
| 15,000 versions of one task | 11.118 ms, 1,510 buffers, 15,000 rows
scanned | 0.027 ms, 4 buffers, 1 row |
| 2,000 versions of one task | 2.081 ms, 1,455 buffers, 2,000 rows
scanned | 0.022 ms, 4 buffers, 1 row |

```
before:  Limit -> Sort (top-N heapsort) -> Bitmap Heap Scan
after:   Limit -> Index Scan Backward using BackgroundWorkerTask_projectId_slug_createdAt_idx
```

An ascending index scanned backward is enough here, so no descending
index is needed.

## Impact and risk

Real-world gain lands between the two rows above and scales with how
many deploys a project has accumulated. Projects with few deploys will
see little change, since there is barely anything to sort.

The new index costs noticeably more than the existing two-column one: 43
MB against 7.3 MB on the benchmark rig. Adding `createdAt` makes every
key unique, which defeats btree deduplication, so this is a real disk
and write cost rather than a rounding error. Writes to this table happen
at deploy time, not on the run path, so the write amplification is
acceptable. The existing `(projectId, slug)` index is now a redundant
prefix and could be dropped, but this PR keeps it so index usage can be
observed before removing it.

Behavior is unchanged: same predicate, same ordering, same row returned.
The narrowed select is the only code change, and the field it keeps is
the only one the caller read.

Deploy note: the migration is
`20260806100000_add_background_worker_task_project_id_slug_created_at_index`
and uses `CREATE INDEX CONCURRENTLY IF NOT EXISTS`, so it can be
pre-applied by hand before the deploy.
2026-08-07 11:17:10 +01:00
Eric Allam 6c6e58e6ff perf(webapp): batch declarative schedule cleanup queries (#4522)
## Summary

`syncDeclarativeSchedules` runs on every background-worker creation
(every deploy, and every file save during `trigger dev`). It issued one
instance-delete per declarative schedule the current worker no longer
declares, in a loop, and the overwhelming majority of those deletes
matched zero rows. This collapses the loop into at most two set-based
statements and skips the instance delete entirely when the current
environment owns no instance of the schedule.

## Why so many, and mostly no-op

The loop runs once per entry in `missingSchedules`, which starts as
every DECLARATIVE schedule for the whole project across all its
environments (the query filters only by `projectId`). A schedule leaves
that set only when a declared task matches it by `taskIdentifier`
**and** the schedule already has an instance in the current environment.

That last clause is the amplifier. When a task's schedule has no
instance in the current environment, the create branch inserts a
brand-new `TaskSchedule` row with an instance for this environment
rather than adding an instance to the existing row. So the same
scheduled task, once it has run in dev and been deployed to prod, exists
as two separate schedule rows: one carrying a dev instance, one carrying
a prod instance.

On a dev worker sync of that project:

- the dev-instance row matches the declared task and is removed from the
set
- the prod-instance row has the same `taskIdentifier` but no dev
instance, so it stays in the set and gets `deleteMany(taskScheduleId =
prodRow, environmentId = dev)`, which matches zero rows

So every declarative task that has been synced in another environment
contributes one guaranteed no-op delete per sync, and the count scales
with (declarative tasks x environments), plus any leftover rows from
renamed or removed tasks. A project does not need to have dropped a
schedule to generate these; it just needs the same declarative tasks
present in more than one environment, which is the normal
develop-in-dev, deploy-to-prod case.

## Fix

The candidate schedules are already loaded with their instances, so the
branch is decided in memory:

- schedules with no instances (or only current-environment instances)
are removed in a single `taskSchedule.deleteMany`
- schedules that still have another environment's instance have only the
current environment's instance detached, in a single
`taskScheduleInstance.deleteMany`, and only when such an instance
actually exists

Behavior is unchanged (cascade delete still removes the instances of a
deleted schedule); the difference is statement count. A zero-row delete
writes no WAL and creates no dead tuples, so the removed work was pure
query and commit overhead.

Verified with a testcontainer test (red before, green after) counting
the emitted deletes across the no-op, batched-detach, and
schedule-delete cases, and end to end through `trigger dev`: three
declarative schedules created, surviving a re-sync, then two removed in
a single batched delete with the third preserved.
2026-08-07 10:27:39 +01:00
Matt Aitken 04f9c4e1a5 fix(webapp,run-engine,core): drop the hidden debounce ceiling, fail fast on an unusable maxDelay (#4521)
Debouncing with a `delay` longer than an hour did nothing at all.

The engine applied a server-side ceiling on how long a debounced run
could be pushed back, measured from the run's `createdAt` and defaulting
to one hour. A run is only pushed back while its new execution time
stays inside that ceiling, so a `delay` at or above it could never push
anything: the waiting run was released, the trigger started its own run,
and the next trigger repeated it. A `delay: "12h"` produced one run per
trigger, each correctly delayed by 12h, with no error raised and nothing
on the run to show the debounce key had been ignored.

The ceiling is now unset by default. A debounce key with no `maxDelay`
keeps collapsing triggers for as long as they keep arriving, which is
what the docs have always described. Self-hosters who want a bound can
still set `RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS`.

That has a consequence worth stating plainly, so the docs now carry a
warning for it: with no `maxDelay`, a continuously triggered key never
executes. Set `maxDelay` when the work has to happen eventually.

**Failing fast on an unusable `maxDelay`.** A caller who sets `maxDelay`
no longer than their `delay` hits exactly the dead end described above,
so that pair is now rejected at trigger time instead of silently
behaving as if no debounce were set:

```
debounce.maxDelay (1h) must be longer than debounce.delay (12h). A debounced run is only
pushed back while it stays inside maxDelay, so with these values every trigger would create
its own run.
```

An unparseable `maxDelay` is rejected too, rather than quietly falling
back to no bound at all, and so is a `delay` given as a date rather than
a duration, which could never work because the value is re-applied on
every push.

The same check runs against a configured server ceiling, so a
self-hosted deployment that sets
`RUN_ENGINE_MAXIMUM_DEBOUNCE_DURATION_MS` gets the error rather than the
silent failure this PR is about. With no `maxDelay` and no configured
ceiling, which is the default, there is nothing to conflict with and
nothing is rejected.

The docs, the `TriggerOptions` JSDoc and the engine option all now state
that the room available to push is the gap between `delay` and
`maxDelay`. The run engine suite gains the case that motivated this:
four triggers on one key with a 12h delay now collapse to a single run.
2026-08-07 07:55:35 +00:00
Katia Bulatova bb036f92e8 feat(webapp): split Watch out of the dashboard agent's first PR
The agent ships Chat and Investigate here; Watch — telling the user later —
follows in its own PR. The whole user-facing feature leaves: the watch card,
chips, wake banner and toast, the unread-wake badge and its poll, the watch
routes, checks, batches and sweeps, the watch alert email and its channel type,
and the watchMaintenance cron.

The agent no longer promises it either: schedule_watch and the alert tools are
gone from the tool set, and the Watches section is out of the system prompt.
Leaving that text in would have had the agent refuse to poll for something it
could no longer offer.

The datastore's watch tables stay. The migrations ship in this PR, so the drizzle
schema that describes them has to ship too — a schema that no longer matched the
migrated tables would make the next generate emit a drop.
2026-08-06 18:35:38 +00:00
Katia Bulatova 3d3c96296b fix(webapp): keep a malformed message's own text out of the error, and pin a finalisation to its body id
The malformed-message error carried 200 characters of the payload, which can be user text or tool output. It now names the shape only. A finalisation also verifies that the body's id is the row it targets, so the stored key and the payload cannot name different messages. The legacy-column guard missed a schema-qualified update, and now self-tests both spellings.
2026-08-06 17:19:20 +00:00
Katia Bulatova f159c69c46 test(webapp): guard against a raw-SQL reference to the dropped chats.messages
The transcript moved out of `chats.messages` into `chat_messages`. TypeScript
already rejects a reference through the Drizzle schema, but a raw-SQL reference
compiles fine and only fails at runtime, and the earlier guard test went away
with the column. Zero hits today is the point: the test exists so a
reintroduction is caught rather than deployed.
2026-08-06 16:29:24 +00:00
Katia Bulatova d8fda075da fix(webapp): stop an ordinary transcript write from rewriting a stored message
`storeChatMessages` ended in `onConflictDoUpdate`, so `persistMessages` and
`persistTurn` — which are handed a whole snapshot — treated any differing body
under an existing message id as a deliberate finalisation. A stale snapshot
carrying `wake:watch_1:fired`, the watch consent record, the deterministic
confirmation or an investigation settlement card with a different body would
overwrite the durable row that was already recorded. The proxy caps body size
and metadata but does not rewrite message ids, so this was not an
internal-bug-only exposure. The same clause updated only the `message` JSONB and
never the `role` column, so `chat_messages.role` could end up disagreeing with
`message.role` — and the UI reads one while the quota query reads the other.

Ordinary transcript writes are now insert-only. Changing a stored message is its
own operation, `finalizeChatMessage`, guarded on chat id, message id and role.
`role` is verified rather than updated, and verified on both sides: the stored
column must match `expectedRole` and so must the incoming body's own `role`, so
the two cannot drift. A finalisation that matches nothing returns false; one
whose body contradicts `expectedRole` throws.

No production caller depended on the implicit finalisation. Every existing
finalisation-shaped path already writes through an insert-only append:
`settleInvestigationAndCloseCard`, `settleInvestigationStateAndCloseCard` and
the watch request/confirmation/refusal records all use
`appendChatMessageOnce(ByChatId)`.

Also: re-sending a snapshot no longer reserves positions for messages that are
already stored. The chat row is held, the missing ids are read under that lock,
and only those get slots. A 40-message chat grown one turn at a time used to
burn 1+2+…+40 = 820 slots for its 40 rows; it now burns 40. Deltas would be the
proper fix, but that reaches into the agent's turn hooks and is a larger change
than this pass.

Two smaller repairs in the same file: `messageIdOf`/`messageRoleOf` now fail
fast and name the chat and the offending message instead of casting unchecked
and surfacing a `NOT NULL` violation from the driver; and a batch carrying the
same message id twice throws instead of silently keeping the first, since that
is an impossible state and a silent pick is how the upstream bug would stay
invisible.

The comment on `reserveMessagePositions` claiming the row lock is "released with
the statement" was wrong — Postgres holds it to commit — and now says what is
true.
2026-08-06 16:29:24 +00:00
Chris Arderne 088f68b373 feat(webapp): share rate limit bucket across additional API keys per environment (#4508)
## What

Rate-limit the API by **environment** rather than per API key.

Previously the limiter keyed its bucket on the hash of the full
`Authorization` header — one bucket per key. With additional environment
API keys (`tr_*_sk_*`), an environment can mint many keys and each got
its own full bucket, so more keys = higher effective rate limit. This
collapses all of an environment's keys onto a single shared
per-environment bucket, so the ceiling is exactly the configured limit
regardless of key mix.

## How

- `authorizationRateLimitMiddleware` now lets the override return `{
config?, identifier? }`. `identifier`, when present, is the rate limit
bucket key; otherwise it falls back to the hashed `Authorization` header
(unchanged legacy behavior, still used by `engineRateLimiter` and any
unauthenticated fallthrough).
- `apiRateLimiter`'s override resolves the environment id and uses it as
the identifier:
- **Additional keys** (`isAdditionalApiKey`) resolve via a new
`resolveAdditionalApiKeyRateLimitScope()` — a **scope-agnostic** keyHash
→ (environmentId, org limiter config) lookup. It is deliberately
permissive (restricted keys resolve too) because it's used **only for
bucketing, never as an auth decision** — request auth still goes through
the RBAC bearer controller, which enforces scopes. Revoked/expired keys
are excluded so they can't hold a bucket warm.
- **Root/legacy keys** reuse the environment already resolved by
`authenticateAuthorizationHeader` and key on `environment.id` too.
- The identifier is always the stable environment id, never the secret
key (which can rotate and would split the bucket).
- The whole override result is cached per key by the existing SWR cache,
so **no extra per-request lookup and no separate Redis mapping** is
added.

## Behavior notes

- Root + additional keys of the same environment now share one bucket
(ceiling = configured limit, not a multiple of it). Restricted
additional keys are included — they were the biggest gap, since they
authenticate via the RBAC controller and previously fell back to per-key
buckets.
- **Public JWTs** keep their existing fixed-window, per-token bucketing.
- One-time bucket reset on deploy (bucket keys change); harmless.

## Tests

- New: two tokens resolving to the same identifier share one bucket.
- New: with no identifier, bucketing stays per-key (legacy behavior
preserved).
- Updated existing override tests to the new `{ config }` return shape.

Base: `feat/multi-keys-surface`. Closes TRI-12888.
2026-08-06 16:05:27 +01:00