Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function serializeRecordId(recordId: Array<string | number>): string {
return recordId.map(String).join('|');
}

export function deserializeRecordId(value: string): Array<string | number> {
return value.split('|');
}
Comment on lines +1 to +7
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium adapters/record-id-serializer.ts:1

serializeRecordId(["foo|bar", "baz"]) returns "foo|bar|baz", and deserializeRecordId("foo|bar|baz") returns ["foo", "bar", "baz"] — the original array structure is lost when any element contains |. Consider using an escaping scheme or structured format like JSON to preserve element boundaries.

-export function serializeRecordId(recordId: Array<string | number>): string {
-  return recordId.map(String).join('|');
-}
+export function serializeRecordId(recordId: Array<string | number>): string {
+  return JSON.stringify(recordId);
+}
 
 export function deserializeRecordId(value: string): Array<string | number> {
-  return value.split('|');
+  return JSON.parse(value);
 }
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/workflow-executor/src/adapters/record-id-serializer.ts around lines 1-7:

`serializeRecordId(["foo|bar", "baz"])` returns `"foo|bar|baz"`, and `deserializeRecordId("foo|bar|baz")` returns `["foo", "bar", "baz"]` — the original array structure is lost when any element contains `|`. Consider using an escaping scheme or structured format like JSON to preserve element boundaries.

Evidence trail:
packages/workflow-executor/src/adapters/record-id-serializer.ts (lines 1-7, REVIEWED_COMMIT) - the serialize/deserialize functions with no escaping or validation. packages/agent-client/src/record-id.ts (lines 19-25, REVIEWED_COMMIT) - the same codebase's other version that explicitly throws on `|` in elements. packages/agent-client/test/record-id.test.ts (line 37, REVIEWED_COMMIT) - test proving the guard: `expect(() => serializeRecordId(['1|abc', 2])).toThrow(...)`. packages/workflow-executor/src/http/pending-data-validators.ts (line 38, REVIEWED_COMMIT) - `deserializeRecordId` used on user-supplied input via Zod transform.

Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {

import { z } from 'zod';

import { deserializeRecordId } from './record-id-serializer';
import toStepDefinition from './step-definition-mapper';
import {
DomainValidationError,
Expand Down Expand Up @@ -149,7 +150,7 @@ export default function toAvailableStepExecution(
collectionId: run.collectionId,
baseRecordRef: {
collectionName: run.collectionName,
recordId: [run.selectedRecordId],
recordId: deserializeRecordId(run.selectedRecordId),
stepIndex: 0,
},
stepDefinition: toStepDefinition(pending.stepDefinition),
Expand Down
3 changes: 2 additions & 1 deletion packages/workflow-executor/src/http/executor-http-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import http from 'http';
import Koa from 'koa';
import koaJwt from 'koa-jwt';

import serializeStepForWire from './step-serializer';
import ConsoleLogger from '../adapters/console-logger';
import {
RunNotFoundError,
Expand Down Expand Up @@ -160,7 +161,7 @@ export default class ExecutorHttpServer {

private async handleGetRun(ctx: Koa.Context): Promise<void> {
const steps = await this.options.runner.getRunStepExecutions(ctx.params.runId);
ctx.body = { steps };
ctx.body = { steps: steps.map(serializeStepForWire) };
}

private async handleTrigger(ctx: Koa.Context): Promise<void> {
Expand Down
10 changes: 5 additions & 5 deletions packages/workflow-executor/src/http/pending-data-validators.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { z } from 'zod';

import { deserializeRecordId } from '../adapters/record-id-serializer';

// Per-step-type schemas for the userConfirmation payload sent by the front via
// POST /runs/:runId/trigger. Validated into `execution.userConfirmation`; schemas
// use .strict() to reject unknown fields.
Expand Down Expand Up @@ -30,18 +32,16 @@ const loadRelatedRecordPatchSchema = z
// The executor re-derives relatedCollectionName and displayName from FieldSchema when
// processing the confirmation.
name: z.string().min(1).optional(),
// User may override the AI-selected record; must be non-empty when provided.
// User may override the AI-selected record; pipe-separated string (e.g. 'id1|id2').
// Required when overriding the relation name — the original record ID belongs to a
// different collection and cannot be reused for the new relation.
selectedRecordId: z
.array(z.union([z.string(), z.number()]))
.min(1)
.optional(),
selectedRecordId: z.string().min(1).transform(deserializeRecordId).optional(),
})
.strict()
.refine(data => data.name === undefined || data.selectedRecordId !== undefined, {
message: 'selectedRecordId is required when overriding the relation name',
});

const guidancePatchSchema = z
.object({
userInput: z.string().optional(),
Expand Down
43 changes: 43 additions & 0 deletions packages/workflow-executor/src/http/step-serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import type { StepExecutionData } from '../types/step-execution-data';
import type { RecordRef } from '../types/validated/collection';

import { serializeRecordId } from '../adapters/record-id-serializer';

function serializeRecordRef(ref: RecordRef): unknown {
return { ...ref, recordId: serializeRecordId(ref.recordId) };
}

export default function serializeStepForWire(step: StepExecutionData): unknown {
switch (step.type) {
case 'read-record':
case 'update-record':
case 'trigger-action':
return { ...step, selectedRecordRef: serializeRecordRef(step.selectedRecordRef) };

case 'load-related-record': {
const result: Record<string, unknown> = {
...step,
selectedRecordRef: serializeRecordRef(step.selectedRecordRef),
};

if (step.pendingData) {
result.pendingData = {
...step.pendingData,
selectedRecordId: serializeRecordId(step.pendingData.selectedRecordId),
};
}

if (step.executionResult && 'record' in step.executionResult) {
result.executionResult = {
...step.executionResult,
record: serializeRecordRef(step.executionResult.record),
};
}

return result;
}

default:
return step;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { deserializeRecordId, serializeRecordId } from '../../src/adapters/record-id-serializer';

describe('serializeRecordId', () => {
it('single id → no pipe', () => {
expect(serializeRecordId(['42'])).toBe('42');
});

it('composite ids → pipe-joined', () => {
expect(serializeRecordId(['id1', 'id2'])).toBe('id1|id2');
});

it('numbers are stringified', () => {
expect(serializeRecordId([42, 99])).toBe('42|99');
});

it('mixed string and number ids', () => {
expect(serializeRecordId(['org', 42])).toBe('org|42');
});
});

describe('deserializeRecordId', () => {
it('single id → single-element array', () => {
expect(deserializeRecordId('42')).toEqual(['42']);
});

it('pipe string → multi-element array', () => {
expect(deserializeRecordId('id1|id2')).toEqual(['id1', 'id2']);
});

it('three segments', () => {
expect(deserializeRecordId('a|b|c')).toEqual(['a', 'b', 'c']);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,22 @@ describe('toAvailableStepExecution', () => {
expect(result?.runId).toBe('999');
});

it('should wrap selectedRecordId in an array for baseRecordRef', () => {
it('should deserialize selectedRecordId into an array for baseRecordRef', () => {
const run = makeRun({ selectedRecordId: 'rec-abc' });

const result = toAvailableStepExecution(run);

expect(result?.baseRecordRef.recordId).toEqual(['rec-abc']);
});

it('splits a pipe-separated selectedRecordId into a multi-element recordId array', () => {
const run = makeRun({ selectedRecordId: 'pk1|pk2' });

const result = toAvailableStepExecution(run);

expect(result?.baseRecordRef.recordId).toEqual(['pk1', 'pk2']);
});

it('should return null when workflowHistory is empty', () => {
const run = makeRun({ workflowHistory: [] });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ describe('LoadRelatedRecordStepExecutor', () => {
const context = makeContext({
agentPort,
runStore,
incomingPendingData: { userConfirmed: true, selectedRecordId: [42] },
incomingPendingData: { userConfirmed: true, selectedRecordId: '42' },
});
const executor = new LoadRelatedRecordStepExecutor(context);

Expand All @@ -867,7 +867,7 @@ describe('LoadRelatedRecordStepExecutor', () => {
selectedRecordId: [99], // AI suggestion preserved
}),
executionResult: expect.objectContaining({
record: expect.objectContaining({ collectionName: 'orders', recordId: [42] }),
record: expect.objectContaining({ collectionName: 'orders', recordId: ['42'] }),
}),
}),
);
Expand All @@ -893,7 +893,7 @@ describe('LoadRelatedRecordStepExecutor', () => {
incomingPendingData: {
userConfirmed: true,
name: 'address',
selectedRecordId: [7],
selectedRecordId: '7',
},
});
const executor = new LoadRelatedRecordStepExecutor(context);
Expand All @@ -914,7 +914,7 @@ describe('LoadRelatedRecordStepExecutor', () => {
executionParams: { name: 'address', displayName: 'Address' },
executionResult: expect.objectContaining({
relation: { name: 'address', displayName: 'Address' },
record: expect.objectContaining({ collectionName: 'addresses', recordId: [7] }),
record: expect.objectContaining({ collectionName: 'addresses', recordId: ['7'] }),
}),
}),
);
Expand Down
66 changes: 66 additions & 0 deletions packages/workflow-executor/test/http/executor-http-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,72 @@ describe('ExecutorHttpServer', () => {
expect(response.status).toBe(500);
expect(response.body).toEqual({ error: 'Internal server error' });
});

it('serializes selectedRecordRef.recordId to pipe-separated string', async () => {
const steps = [
{
type: 'update-record' as const,
stepIndex: 1,
selectedRecordRef: { collectionName: 'orders', recordId: ['pk1', 'pk2'], stepIndex: 0 },
},
];
const runner = createMockRunner({
getRunStepExecutions: jest.fn().mockResolvedValue(steps),
});

const response = await request(createServer({ runner }).callback)
.get('/runs/run-1')
.set('Authorization', `Bearer ${signToken({ id: 1 })}`);

expect(response.status).toBe(200);
expect(response.body.steps[0].selectedRecordRef.recordId).toBe('pk1|pk2');
});

it('serializes load-related-record pendingData.selectedRecordId and executionResult.record.recordId', async () => {
const steps = [
{
type: 'load-related-record' as const,
stepIndex: 2,
selectedRecordRef: { collectionName: 'customers', recordId: ['c1'], stepIndex: 0 },
pendingData: {
name: 'orders',
displayName: 'Orders',
selectedRecordId: ['o1', 'o2'],
},
executionResult: {
relation: { name: 'orders', displayName: 'Orders' },
record: { collectionName: 'orders', recordId: ['o1', 'o2'], stepIndex: 2 },
},
},
];
const runner = createMockRunner({
getRunStepExecutions: jest.fn().mockResolvedValue(steps),
});

const response = await request(createServer({ runner }).callback)
.get('/runs/run-1')
.set('Authorization', `Bearer ${signToken({ id: 1 })}`);

expect(response.status).toBe(200);
const step = response.body.steps[0];
expect(step.selectedRecordRef.recordId).toBe('c1');
expect(step.pendingData.selectedRecordId).toBe('o1|o2');
expect(step.executionResult.record.recordId).toBe('o1|o2');
});

it('passes through steps without selectedRecordRef unchanged', async () => {
const steps = [{ type: 'condition' as const, stepIndex: 0 }];
const runner = createMockRunner({
getRunStepExecutions: jest.fn().mockResolvedValue(steps),
});

const response = await request(createServer({ runner }).callback)
.get('/runs/run-1')
.set('Authorization', `Bearer ${signToken({ id: 1 })}`);

expect(response.status).toBe(200);
expect(response.body).toEqual({ steps });
});
});

describe('POST /runs/:runId/trigger', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,31 @@ describe('patchBodySchemas', () => {
expect(schema.parse({ userConfirmed: true })).toEqual({ userConfirmed: true });
});

it('accepts confirmation with selectedRecordId override only', () => {
expect(schema.parse({ userConfirmed: true, selectedRecordId: [42] })).toEqual({
userConfirmed: true,
selectedRecordId: [42],
});
it('deserializes selectedRecordId from pipe string to array', () => {
const result = schema.parse({ userConfirmed: true, selectedRecordId: 'pk1|pk2' }) as {
selectedRecordId: unknown;
};

expect(result.selectedRecordId).toEqual(['pk1', 'pk2']);
});

it('deserializes single selectedRecordId', () => {
const result = schema.parse({ userConfirmed: true, selectedRecordId: '42' }) as {
selectedRecordId: unknown;
};

expect(result.selectedRecordId).toEqual(['42']);
});

it('accepts confirmation with both name and selectedRecordId (relation override)', () => {
expect(schema.parse({ userConfirmed: true, name: 'address', selectedRecordId: [7] })).toEqual(
{ userConfirmed: true, name: 'address', selectedRecordId: [7] },
);
const result = schema.parse({
userConfirmed: true,
name: 'address',
selectedRecordId: '7',
}) as { selectedRecordId: unknown };

expect(result).toMatchObject({ userConfirmed: true, name: 'address' });
expect(result.selectedRecordId).toEqual(['7']);
});

it('rejects name override without selectedRecordId — original record ID belongs to a different collection', () => {
Expand All @@ -96,6 +110,10 @@ describe('patchBodySchemas', () => {
expect(() => schema.parse({ userConfirmed: true, name: '' })).toThrow();
});

it('rejects empty selectedRecordId string', () => {
expect(() => schema.parse({ userConfirmed: true, selectedRecordId: '' })).toThrow();
});

it('rejects unknown fields (strict schema)', () => {
expect(() => schema.parse({ userConfirmed: true, extra: 'leak' })).toThrow();
});
Expand Down
Loading