Skip to content

Commit 0721012

Browse files
committed
fix: address CodeRabbit review feedback
- Reject requests exceeding 500 runs with 400 instead of silently truncating - Propagate finalizeRun through bulk action params so large batches also skip PENDING_CANCEL - Inherit parent process.env when spawning watchdog (TLS/proxy settings) - Add PID file prefix (trigger-watchdog:) to prevent killing unrelated processes on PID reuse
1 parent 562e88a commit 0721012

File tree

4 files changed

+35
-17
lines changed

4 files changed

+35
-17
lines changed

apps/webapp/app/routes/engine.v1.dev.disconnect.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,14 @@ const { action } = createActionApiRoute(
4646
);
4747
}
4848

49-
// Cap the number of runs
50-
const runFriendlyIds = body.runFriendlyIds.slice(0, MAX_RUNS);
49+
if (body.runFriendlyIds.length > MAX_RUNS) {
50+
return json(
51+
{ error: `A maximum of ${MAX_RUNS} runs can be cancelled per request` },
52+
{ status: 400 }
53+
);
54+
}
55+
56+
const { runFriendlyIds } = body;
5157

5258
if (runFriendlyIds.length === 0) {
5359
return json({ cancelled: 0 }, { status: 200 });
@@ -56,7 +62,6 @@ const { action } = createActionApiRoute(
5662
logger.info("Dev disconnect: cancelling runs", {
5763
environmentId,
5864
runCount: runFriendlyIds.length,
59-
capped: body.runFriendlyIds.length > MAX_RUNS,
6065
});
6166

6267
// For small numbers of runs, cancel inline
@@ -150,7 +155,7 @@ async function createBulkCancelAction(
150155
environmentId,
151156
name: "Dev session disconnect",
152157
type: BulkActionType.CANCEL,
153-
params: { runId: runFriendlyIds },
158+
params: { runId: runFriendlyIds, finalizeRun: true },
154159
queryName: "bulk_action_v1",
155160
totalCount: runFriendlyIds.length,
156161
completionNotification: BulkActionNotificationType.NONE,

apps/webapp/app/v3/services/bulk/BulkActionV2.server.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,13 @@ export class BulkActionService extends BaseService {
138138
}
139139

140140
// 2. Parse the params
141+
const rawParams = group.params && typeof group.params === "object" ? group.params : {};
142+
const finalizeRun = "finalizeRun" in rawParams && (rawParams as any).finalizeRun === true;
141143
const filters = parseRunListInputOptions({
142144
organizationId: group.project.organizationId,
143145
projectId: group.projectId,
144146
environmentId: group.environmentId,
145-
...(group.params && typeof group.params === "object" ? group.params : {}),
147+
...rawParams,
146148
});
147149

148150
const runsRepository = new RunsRepository({
@@ -199,6 +201,7 @@ export class BulkActionService extends BaseService {
199201
cancelService.call(run, {
200202
reason: `Bulk action ${group.friendlyId} cancelled run`,
201203
bulkActionId: bulkActionId,
204+
finalizeRun,
202205
})
203206
);
204207
if (error) {

packages/cli-v3/src/dev/devSupervisor.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ class DevSupervisor implements WorkerRuntime {
222222
detached: true,
223223
stdio: "ignore",
224224
env: {
225+
...process.env,
225226
WATCHDOG_PARENT_PID: process.pid.toString(),
226227
WATCHDOG_API_URL: this.options.client.apiURL,
227228
WATCHDOG_API_KEY: this.options.client.accessToken ?? "",
@@ -254,8 +255,12 @@ class DevSupervisor implements WorkerRuntime {
254255
// Also try via PID file in case the process reference is stale
255256
if (this.watchdogPidPath) {
256257
try {
257-
const pid = parseInt(readFileSync(this.watchdogPidPath, "utf8"), 10);
258-
process.kill(pid, "SIGTERM");
258+
const content = readFileSync(this.watchdogPidPath, "utf8");
259+
const prefix = "trigger-watchdog:";
260+
if (content.startsWith(prefix)) {
261+
const pid = parseInt(content.slice(prefix.length), 10);
262+
if (pid) process.kill(pid, "SIGTERM");
263+
}
259264
} catch {
260265
// Already dead or no file
261266
}

packages/cli-v3/src/dev/devWatchdog.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,23 +43,28 @@ if (!existsSync(dir)) {
4343
mkdirSync(dir, { recursive: true });
4444
}
4545

46+
const PID_FILE_PREFIX = "trigger-watchdog:";
47+
4648
// Single instance: kill any existing watchdog
4749
try {
48-
const existingPid = parseInt(readFileSync(pidFilePath, "utf8"), 10);
49-
if (existingPid && existingPid !== process.pid) {
50-
try {
51-
process.kill(existingPid, 0); // Check if alive
52-
process.kill(existingPid, "SIGTERM"); // Kill it
53-
} catch {
54-
// Already dead
50+
const pidFileContent = readFileSync(pidFilePath, "utf8");
51+
if (pidFileContent.startsWith(PID_FILE_PREFIX)) {
52+
const existingPid = parseInt(pidFileContent.slice(PID_FILE_PREFIX.length), 10);
53+
if (existingPid && existingPid !== process.pid) {
54+
try {
55+
process.kill(existingPid, 0); // Check if alive
56+
process.kill(existingPid, "SIGTERM"); // Kill it
57+
} catch {
58+
// Already dead
59+
}
5560
}
5661
}
5762
} catch {
58-
// No PID file
63+
// No PID file or invalid format
5964
}
6065

61-
// Write our PID
62-
writeFileSync(pidFilePath, process.pid.toString());
66+
// Write our PID with prefix so we can verify ownership later
67+
writeFileSync(pidFilePath, `${PID_FILE_PREFIX}${process.pid}`);
6368

6469
function cleanup() {
6570
try {

0 commit comments

Comments
 (0)