Skip to content

Commit 4f69c43

Browse files
authored
feat(supervisor): reclaim a run's checkpoint storage when it finishes (#4493)
When a run reaches a terminal state, ask the checkpoint service to reclaim the storage its checkpoints occupied. Storage for finished runs is not otherwise reclaimed, so nothing frees it today. **Off by default** behind `DELETE_CHECKPOINTS_ON_COMPLETION`, and the service-side handler ships separately, so merging this changes no behaviour. ## Where the tenancy comes from Addressing a run's checkpoints needs org, project, environment, deployment version and run id. All five are already in hand at `attempt.complete`, and three are **signed** by the deployment token: | Value | Source | Trust | | -- | -- | -- | | org | claim `org_id` | signed | | environment | claim `environment_id` | signed | | deployment version | claim `deployment_version` | signed | | project ref | `x-trigger-workload-project-ref` header | runner-supplied | | run | route param | runner-supplied | `authorizeWorkloadRequest` previously returned only `environment_id`, and only in enforce mode, so it now also returns the verified `claims`. That difference is deliberate and documented on the method: claims are used to address a run's **own** resources locally, never to scope the platform, which is why `environmentId` stays enforce-only. The two runner-supplied values are safe because the signed ones are outermost - a runner lying about either can only name something inside its own org and environment, and a project ref that doesn't pair with its signed environment matches nothing. The run id is read from `params.runFriendlyId`, the same value the platform just validated, rather than from the body or a header. Where both a claim and a header exist (`deployment_version`), the claim wins. ## Placement The call sits after `reply.json(...)`, so the runner sees no added latency - the same shape the suspend route already uses. The service enqueues and returns 202, so it is one fast local hop. Terminal means `RUN_FINISHED` **or `RUN_PENDING_CANCEL`** - a run cancelled mid-execution never restores, and skipping it would leave its storage behind. Retries are excluded deliberately: reclamation is per-run, so a retry is covered by the final completion. Also gated on `!snapshotService`, so it stays inert where checkpoints aren't the kind this reclaims. ## Observability `checkpoint_delete_requests_total{result}` counts `sent` **and every reason we decide not to send**: `disabled`, `not_terminal`, `no_claims`, `no_project_ref`, `http_error`. The negative labels are the point - without them, "no requests are happening" looks identical to the feature being switched off. `no_claims` is reachable even under enforcement, since enforce only rejects a *present-but-invalid* token; an absent or legacy id still passes with no claims attached. ## Notes for review - **No changeset**: `CheckpointClient` is `core/v3/serverOnly`, an internal service-to-service API rather than customer-facing surface. - **No `.server-changes/` note**: there is nothing a dashboard user would notice here. Happy to add one if you disagree. - `pnpm run typecheck` can't complete in my checkout - `@trigger.dev/database` fails to build on a missing `tsc` in the pnpm store, unrelated to this diff. Verified with `tsc --noEmit` against the supervisor project instead: **zero errors in `apps/supervisor/src`**. Worth noting it caught a real bug here - the completion response is wrapped, so the status is `data.result.attemptStatus`. refs TRI-12789
1 parent e8398d1 commit 4f69c43

3 files changed

Lines changed: 134 additions & 1 deletion

File tree

apps/supervisor/src/env.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export const Env = z
2222
// also reject invalid tokens.
2323
WORKLOAD_TOKEN_SECRET: z.string().optional(),
2424
WORKLOAD_TOKEN_ENFORCEMENT: z.enum(["disabled", "log", "enforce"]).default("disabled"),
25+
DELETE_CHECKPOINTS_ON_COMPLETION: BoolEnv.default(false), // irreversible; enable per cluster
2526
// Absolute expiry for minted deployment tokens. Deterministic (no wall-clock issued-at) so every
2627
// pod of a deployment carries an identical token; bump before this date. Must outlive any run.
2728
WORKLOAD_TOKEN_EXP: z.string().datetime().default("2032-01-01T00:00:00.000Z"),
@@ -326,6 +327,14 @@ export const Env = z
326327
path: ["WORKLOAD_TOKEN_SECRET"],
327328
});
328329
}
330+
if (data.DELETE_CHECKPOINTS_ON_COMPLETION && data.WORKLOAD_TOKEN_ENFORCEMENT === "disabled") {
331+
ctx.addIssue({
332+
code: z.ZodIssueCode.custom,
333+
message:
334+
"DELETE_CHECKPOINTS_ON_COMPLETION needs WORKLOAD_TOKEN_ENFORCEMENT set to log or enforce: the tenancy it deletes by comes from the deployment token, so with tokens disabled it would silently reclaim nothing",
335+
path: ["DELETE_CHECKPOINTS_ON_COMPLETION"],
336+
});
337+
}
329338
if (
330339
data.TRIGGER_DEQUEUE_BACKPRESSURE_ENABLED &&
331340
!data.TRIGGER_DEQUEUE_BACKPRESSURE_REDIS_HOST

apps/supervisor/src/workloadServer/index.ts

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,16 @@ import EventEmitter from "node:events";
2323
import type { IncomingMessage, ServerResponse } from "node:http";
2424
import { type Namespace, Server, type Socket } from "socket.io";
2525
import { z } from "zod";
26+
import { tryCatch } from "@trigger.dev/core/utils";
27+
import { Counter } from "prom-client";
2628
import { env } from "../env.js";
2729
import { register } from "../metrics.js";
2830
import {
2931
verifyDeploymentIdHeader,
3032
workloadTokenEnforced,
3133
workloadTokensEnabled,
3234
} from "../workloadToken.js";
35+
import type { WorkloadDeploymentTokenClaims } from "@trigger.dev/core/v3";
3336
import {
3437
ComputeSnapshotService,
3538
type RunTraceContext,
@@ -50,6 +53,13 @@ interface DefaultEventsMap {
5053
[event: string]: (...args: any[]) => void;
5154
}
5255

56+
const checkpointDeleteRequests = new Counter({
57+
name: "checkpoint_delete_requests_total",
58+
help: "Checkpoint delete requests attempted at run completion, by outcome",
59+
labelNames: ["result"],
60+
registers: [register],
61+
});
62+
5363
const WorkloadActionParams = z.object({
5464
runFriendlyId: z.string(),
5565
snapshotFriendlyId: z.string(),
@@ -181,10 +191,15 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
181191
* environment_id to forward upstream. The env id is only forwarded in enforce mode: in log mode
182192
* we still verify + record metrics but attach no header (so the platform never scopes). Only
183193
* enforce fails a request, and only for a present-but-invalid token; absent and legacy ids pass.
194+
*
195+
* `claims` are returned on any valid token, for local use only - never to scope the platform,
196+
* which is why environmentId stays gated on enforce.
184197
*/
185198
private async authorizeWorkloadRequest(
186199
req: IncomingMessage
187-
): Promise<{ ok: true; environmentId?: string } | { ok: false }> {
200+
): Promise<
201+
{ ok: true; environmentId?: string; claims?: WorkloadDeploymentTokenClaims } | { ok: false }
202+
> {
188203
if (!workloadTokensEnabled) {
189204
return { ok: true };
190205
}
@@ -201,9 +216,73 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
201216
workloadTokenEnforced && result.outcome === "jwt_valid"
202217
? result.claims.environment_id
203218
: undefined,
219+
claims: result.outcome === "jwt_valid" ? result.claims : undefined,
204220
};
205221
}
206222

223+
/**
224+
* reclaimCheckpoints asks the checkpoint service to delete a finished run's checkpoint storage.
225+
* Must be called after the reply is sent: it never delays the runner.
226+
*/
227+
private async reclaimCheckpoints(
228+
req: IncomingMessage,
229+
runFriendlyId: string,
230+
attemptStatus: string,
231+
claims: WorkloadDeploymentTokenClaims | undefined
232+
): Promise<void> {
233+
if (!env.DELETE_CHECKPOINTS_ON_COMPLETION) {
234+
checkpointDeleteRequests.inc({ result: "disabled" });
235+
return;
236+
}
237+
238+
if (!this.checkpointClient) {
239+
checkpointDeleteRequests.inc({ result: "no_client" });
240+
return;
241+
}
242+
243+
if (this.snapshotService) {
244+
checkpointDeleteRequests.inc({ result: "not_applicable" });
245+
return;
246+
}
247+
248+
if (attemptStatus !== "RUN_FINISHED" && attemptStatus !== "RUN_PENDING_CANCEL") {
249+
checkpointDeleteRequests.inc({ result: "not_terminal" });
250+
return;
251+
}
252+
253+
if (!claims) {
254+
checkpointDeleteRequests.inc({ result: "no_claims" });
255+
return;
256+
}
257+
258+
const projectRef = this.projectRefFromRequest(req);
259+
if (!projectRef) {
260+
checkpointDeleteRequests.inc({ result: "no_project_ref" });
261+
this.logger.error("Cannot reclaim checkpoints without a project ref", { runFriendlyId });
262+
return;
263+
}
264+
265+
const [error, accepted] = await tryCatch(
266+
this.checkpointClient.deleteCheckpoints({
267+
runFriendlyId,
268+
body: {
269+
orgId: claims.org_id,
270+
envId: claims.environment_id,
271+
deploymentVersion: claims.deployment_version,
272+
projectRef,
273+
},
274+
})
275+
);
276+
277+
if (error || !accepted) {
278+
checkpointDeleteRequests.inc({ result: "http_error" });
279+
this.logger.error("Failed to request checkpoint reclaim", { runFriendlyId, error });
280+
return;
281+
}
282+
283+
checkpointDeleteRequests.inc({ result: "sent" });
284+
}
285+
207286
/**
208287
* Sets common route meta on the wide-event state from URL params.
209288
*/
@@ -364,6 +443,13 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
364443
}
365444

366445
reply.json(completeResponse.data satisfies WorkloadRunAttemptCompleteResponseBody);
446+
447+
await this.reclaimCheckpoints(
448+
req,
449+
params.runFriendlyId,
450+
completeResponse.data.result.attemptStatus,
451+
auth.claims
452+
);
367453
return;
368454
}
369455
),

packages/core/src/v3/serverOnly/checkpointClient.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,42 @@ export class CheckpointClient {
119119

120120
return true;
121121
}
122+
123+
/**
124+
* Ask the checkpoint service to reclaim a finished run's checkpoint storage. Best-effort: the
125+
* service enqueues and returns 202, so a `true` here means accepted, not deleted.
126+
*/
127+
async deleteCheckpoints({
128+
runFriendlyId,
129+
body,
130+
}: {
131+
runFriendlyId: string;
132+
body: {
133+
orgId: string;
134+
envId: string;
135+
projectRef: string;
136+
deploymentVersion: string;
137+
};
138+
}): Promise<boolean> {
139+
const res = await fetch(
140+
new URL(`/api/v1/runs/${runFriendlyId}/checkpoints/delete`, this.opts.apiUrl),
141+
{
142+
method: "POST",
143+
headers: {
144+
"Content-Type": "application/json",
145+
},
146+
body: JSON.stringify(body),
147+
}
148+
);
149+
150+
if (!res.ok) {
151+
this.logger.error("[CheckpointClient] Delete checkpoints request failed", {
152+
runFriendlyId,
153+
status: res.status,
154+
});
155+
return false;
156+
}
157+
158+
return true;
159+
}
122160
}

0 commit comments

Comments
 (0)