feat(sdk,webapp): add tags.delete() to remove tags from a run - #4424
Draft
claude[bot] wants to merge 1 commit into
Draft
feat(sdk,webapp): add tags.delete() to remove tags from a run#4424claude[bot] wants to merge 1 commit into
claude[bot] wants to merge 1 commit into
Conversation
Adds a per-run tag removal path, mirroring the existing tags.add() flow end to end: - `tags.delete(tag | tags)` in the SDK, plus JSDoc on both `add` and `delete` - `DELETE /api/v1/runs/:runId/tags` as a method branch on the existing route - `RemoveTagsRequestBody` and `ApiClient#removeTags` in core - `RunStore#removeTags`, a single-statement UPDATE so there is no read-modify-write window racing a concurrent pushTags - a `remove_tags` snapshot patch for runs still in the buffer Removing a tag only affects the run it's called from. Removing a tag the run doesn't have, or an empty list, is an idempotent success. Also fixes the `add` span name, which was hardcoded to "tags.set()", and an OpenAPI code sample advertising a `runs.addTags(...)` function that doesn't exist. Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 8ad4bd6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Matt Aitken · Slack thread
Before / After
Before: tags were add-only. Once a tag was on a run — set at trigger time or via
tags.add()— there was no way to take it off. A run that used tags to signal state (status_processing, thenstatus_done) accumulated both forever, and there was no public API or SDK function to remove one.After: you can remove tags from inside a run:
This removes the tags from that run only. It is not a global or org-wide delete: every other run carrying the same tag keeps it, and the tag stays available to filter by. That holds by construction — a run's tags are just strings in its own
runTagsarray, not rows in a shared table.Removing a tag the run doesn't have is an idempotent success rather than a 404, so you don't have to read the run's current tags first. An empty or blank-only list is also a no-op success. The 10-tag cap doesn't apply to a removal, since a removal can only shrink the list.
How
tags.deleteadded to thetagsnamespace inpackages/trigger-sdk/src/v3/tags.ts, mirroringaddTags' in-a-run guard. Bothaddanddeletenow carry JSDoc.RemoveTagsRequestBodyinschemas/api.ts(reusingRunTags) andApiClient#removeTagsinapiClient/index.ts.DELETE /api/v1/runs/:runId/tagswith a{ tags }body. The existing hand-rolled route (api.v1.runs.$runId.tags.ts) now switches on method instead of hard-rejecting non-POST. The POST path is unchanged: same auth, same status codes, same response shapes. Deliberately not converted tocreateActionApiRouteand no RBACauthorizationgate was added, because either would change behaviour for narrowly-scoped JWTs. The route module still exports onlyaction.RunStore#removeTags(types.ts,PostgresRunStore.ts,runOpsStore.ts). It's a singleUPDATEthat rebuilds the array withunnest, so there's no read-modify-write window for a concurrentpushTagsto be lost inside.unnestrather thanEXCEPTbecauseEXCEPTwould dedupe and reorder the surviving tags. Raw SQL doesn't fire Prisma's@updatedAt, so"updatedAt"is set explicitly and returned — realtime uses it as the read-your-writes watermark. Both thelegacyanddedicatedschema variants are handled, followingexpireRunsBatch's array-binding convention.remove_tagsSnapshotPatchand Lua branch inpackages/redis-worker/src/mollifier/buffer.ts, for runs not yet materialised into Postgres. The branch has to exist: an unhandled patch type falls into the terminalelsethat returnsbusy, which surfaces as a 2s stall then a 503. When the last tag goes, the Lua drops thetagsfield rather than writing an empty table, because cjson encodes an empty Lua table as{}(a JSON object) — an absenttagsis the same shape a run triggered without tags has, and every reader already normalises it to an empty list.No ClickHouse or replication changes are needed:
task_runs_v2is aReplacingMergeTreekeyed on_version, so theTaskRunUPDATE replicates as a replacement row correctly.Realtime limitation
On the Postgres path the route publishes a change record with the remaining tag set, the same way the add path does. That means a
subscribeToRunsWithTagsubscriber to a removed tag simply stops receiving the run — there is no "tag removed" un-subscribe signal. That's an accepted limitation of this PR.Testing
internal-packages/run-store/src/PostgresRunStore.test.ts— 8postgresTestcases forremoveTags: removes one, removes several, tag-not-present is a no-op, removing all leaves[], survivor order preserved, survivor duplicates preserved (proving noEXCEPTdedupe), a run in a differentruntimeEnvironmentIdis untouched (returnsnull), andupdatedAtis refreshed and matches what was stored.packages/redis-worker/src/mollifier/buffer.test.ts— 6redisTestcases forremove_tags, mirroring theappend_tagsset: survivor order, unknown-tag no-op, no-tags-field no-op, removing the last tag never writing a JSON object fortags,remove_tagsthenappend_tagsrebuilding a dense array, andnot_found(notbusy) for an unknown run.Verified locally:
pnpm run format,pnpm run lint,pnpm run build --filter @trigger.dev/core,--filter @trigger.dev/sdk,--filter @trigger.dev/redis-worker,pnpm run typecheck --filter webapp,pnpm run typecheck --filter @internal/run-store, andpnpm run build --filter webapp(to confirm no server-only import leaked into the client bundle) all pass.The two new test files were not executed by CI-equivalent tooling in the authoring environment, because the container images testcontainers needs could not be pulled there. Both layers were instead verified against real servers: the
removeTagsscenarios above against a locally installed PostgreSQL 16 through the realPostgresRunStore(both thelegacyanddedicatedschema variants, the latter against the run-ops client), and theremove_tagsscenarios against a local Redis 7.0 through the real builtMollifierBuffer— all passed, including a control case confirming an unhandled patch type still returnsbusy— so both files should be re-run in CI.