-
Notifications
You must be signed in to change notification settings - Fork 103
Description
I was working on an experimental Cloudflare World implementation and I was going off by world-local and world-postgres as reference. Then I noticed an issue within storage.ts for world-postges.
The Storage contract allows steps.get(runId?: string, stepId: string), but the postgres backend always prepares the statement with both placeholders. Passing undefined for runId yields run_id = NULL, so lookups fall through and return 404 even though the step exists.
Where
packages/world-postgres/src/storage.ts#L336-L346
In here, there's a prepared statement that always requires both parameters:
const get = drizzle
.select()
.from(steps)
.where(
and(
eq(steps.stepId, sql.placeholder('stepId')),
eq(steps.runId, sql.placeholder('runId')) // crashes when runId is undefined
)
)
.limit(1)
.prepare('workflow_steps_get');
return {
async get(runId, stepId) {
const [value] = await get.execute({ stepId, runId }); // passes undefined to runId
// ...
}
}Fix
Similar to how world-local does it, there should be a conditional WHERE clause so postgres matches the Storage contract
packages/world-local/src/storage.ts#L252-L266:
The fix would be:
async get(runId, stepId) {
const whereClause = runId
? and(eq(steps.stepId, stepId), eq(steps.runId, runId))
: eq(steps.stepId, stepId);
const [value] = await drizzle
.select()
.from(steps)
.where(whereClause)
.limit(1);
if (!value) throw new WorkflowAPIError(`Step not found: ${stepId}`, { status: 404 });
}I’ll open a PR that drops the prepared statement. Would love to hear the Workflows Team's input on this too 🖤