Skip to content

Commit e0a57d9

Browse files
d-csclaude
andcommitted
fix(webapp): mutations-layer Devin follow-ups
- metadata route was routing rootOperations to bufferedEntry.parentTaskRunId with a comment claiming PG's nil-coalesce defaults to parent. PG actually defaults to taskRun.id (self), so a buffered grandchild metadata.root.set() was silently mutating the child's metadata instead of the root's. SyntheticRun already carries rootTaskRunFriendlyId from the snapshot — use it, falling back to the run itself (matching PG) when absent. - reschedule route's PG path delegates to RescheduleTaskRunService which enforces `status !== "DELAYED"` and 422s otherwise. The buffer path had no equivalent guard, so a customer could inject delayUntil into the snapshot of an undelayed buffered run and the drainer would materialise it with an unintended delay. Added a pre-fetch through findRunByIdWithMollifierFallback and 422 when the buffered run has no delayUntil. SyntheticRun doesn't carry a "DELAYED" status enum (only QUEUED|FAILED|CANCELED) so the gate reads the snapshot's delayUntil field directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent eb2a777 commit e0a57d9

2 files changed

Lines changed: 37 additions & 5 deletions

File tree

apps/webapp/app/routes/api.v1.runs.$runId.metadata.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,19 @@ const { action } = createActionApiRoute(
159159
if (bufferedEntry) {
160160
await Promise.all([
161161
routeOperationsToRun(bufferedEntry.parentTaskRunId, body.parentOperations, env),
162-
// The snapshot doesn't carry rootTaskRunId; fall back to parent
163-
// as a rough proxy (matches the existing service's nil-coalesce
164-
// behaviour where rootTaskRun defaults to the parent). Phase D
165-
// / future work could thread rootTaskRunId through the snapshot.
166-
routeOperationsToRun(bufferedEntry.parentTaskRunId, body.rootOperations, env),
162+
// The PG service routes rootOperations to
163+
// `taskRun.rootTaskRun?.id ?? taskRun.id` — the actual root, not
164+
// the parent. The snapshot carries the root's *friendlyId*
165+
// (parentTaskRunId is an internal id; root is friendlyId because
166+
// it's what the engine passes through). Use it; if absent,
167+
// route to the run itself (matches PG's self-fallback) rather
168+
// than misrouting to the parent for grandchild → child → root
169+
// hierarchies.
170+
routeOperationsToRun(
171+
bufferedEntry.rootTaskRunFriendlyId ?? runId,
172+
body.rootOperations,
173+
env,
174+
),
167175
]);
168176
}
169177

apps/webapp/app/routes/api.v1.runs.$runParam.reschedule.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { logger } from "~/services/logger.server";
1010
import { ServiceValidationError } from "~/v3/services/baseService.server";
1111
import { RescheduleTaskRunService } from "~/v3/services/rescheduleTaskRun.server";
1212
import { mutateWithFallback } from "~/v3/mollifier/mutateWithFallback.server";
13+
import { findRunByIdWithMollifierFallback } from "~/v3/mollifier/readFallback.server";
1314
import { parseDelay } from "~/utils/delays";
1415

1516
const ParamsSchema = z.object({
@@ -49,6 +50,29 @@ export async function action({ request, params }: ActionFunctionArgs) {
4950
}
5051

5152
try {
53+
// PG-side `RescheduleTaskRunService.call` enforces
54+
// `taskRun.status !== "DELAYED"` and 422s otherwise — without an
55+
// equivalent guard the buffer path would happily inject a
56+
// `delayUntil` into the snapshot of a non-delayed buffered run, and
57+
// the drainer would materialise it with an unintended delay. The
58+
// SyntheticRun type doesn't carry a "DELAYED" enum value because
59+
// it's not a terminal status the trace API needs to express; the
60+
// buffered analogue is `delayUntil` set in the snapshot. Gate on
61+
// that. Race window between read and write is bounded: if the
62+
// drainer materialises mid-call, mutateWithFallback falls through
63+
// to the PG mutation which has its own DELAYED check.
64+
const buffered = await findRunByIdWithMollifierFallback({
65+
runId: parsed.data.runParam,
66+
environmentId: env.id,
67+
organizationId: env.organizationId,
68+
});
69+
if (buffered && !buffered.delayUntil) {
70+
return json(
71+
{ error: "Cannot reschedule a run that is not delayed" },
72+
{ status: 422 },
73+
);
74+
}
75+
5276
const outcome = await mutateWithFallback<Response>({
5377
runId: parsed.data.runParam,
5478
environmentId: env.id,

0 commit comments

Comments
 (0)