diff --git a/packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts b/packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts index 07e0dcf97c..783cd63a19 100644 --- a/packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts +++ b/packages/workflow-executor/src/adapters/forestadmin-client-activity-log-port.ts @@ -40,6 +40,17 @@ export default class ForestadminClientActivityLogPort implements ActivityLogPort { logger: this.logger }, ); + // Observed: server replies 200 with a null id when it declines to persist (PRD-428). + // Surface it once here; later mark-as-* calls then no-op instead of 404-looping. + if (!response.id) { + this.logger.warn('activity log not persisted by server — status updates will be skipped', { + action: args.action, + collectionId: args.collectionId, + }); + + return { id: null }; + } + return { id: response.id, index: response.attributes.index }; } catch (cause) { this.logger.error('activity log create failed', { @@ -53,6 +64,11 @@ export default class ForestadminClientActivityLogPort implements ActivityLogPort } async markSucceeded(handle: ActivityLogHandle): Promise { + // Not persisted (see createPending) → no log to update; skip the 404-retrying status call. + if (!('index' in handle)) return undefined; + + const { id, index } = handle; + return this.drainer.track(async () => { try { await withRetry( @@ -60,14 +76,14 @@ export default class ForestadminClientActivityLogPort implements ActivityLogPort () => this.service.updateActivityLogStatus({ forestServerToken: this.forestServerToken, - activityLog: { id: handle.id, attributes: { index: handle.index } }, + activityLog: { id, attributes: { index } }, status: 'completed', }), { logger: this.logger, extraRetryStatuses: [404] }, ); } catch (err) { this.logger.error('activity log mark-as-completed failed', { - handleId: handle.id, + handleId: id, error: extractErrorMessage(err), }); } @@ -75,6 +91,11 @@ export default class ForestadminClientActivityLogPort implements ActivityLogPort } async markFailed(handle: ActivityLogHandle, errorMessage: string): Promise { + // Not persisted (see markSucceeded) → skip. + if (!('index' in handle)) return undefined; + + const { id, index } = handle; + return this.drainer.track(async () => { try { await withRetry( @@ -82,14 +103,14 @@ export default class ForestadminClientActivityLogPort implements ActivityLogPort () => this.service.updateActivityLogStatus({ forestServerToken: this.forestServerToken, - activityLog: { id: handle.id, attributes: { index: handle.index } }, + activityLog: { id, attributes: { index } }, status: 'failed', }), { logger: this.logger, extraRetryStatuses: [404] }, ); } catch (err) { this.logger.error('activity log mark-as-failed failed', { - handleId: handle.id, + handleId: id, stepErrorMessage: errorMessage, error: extractErrorMessage(err), }); diff --git a/packages/workflow-executor/src/ports/activity-log-port.ts b/packages/workflow-executor/src/ports/activity-log-port.ts index 15ce3f4801..c1843ff8ca 100644 --- a/packages/workflow-executor/src/ports/activity-log-port.ts +++ b/packages/workflow-executor/src/ports/activity-log-port.ts @@ -7,10 +7,9 @@ export interface CreateActivityLogArgs { label?: string; } -export interface ActivityLogHandle { - id: string; - index: string; -} +// `{ id: null }` means the server declined to persist the log (returns 200 with a null id). +// markSucceeded/markFailed short-circuit on it — there is nothing to update. +export type ActivityLogHandle = { id: string; index: string } | { id: null }; // Per-run scoped port: token baked into the adapter's constructor. markSucceeded/markFailed // retry transient failures internally and are invoked with `void` from base-step-executor. diff --git a/packages/workflow-executor/test/adapters/forestadmin-client-activity-log-port.test.ts b/packages/workflow-executor/test/adapters/forestadmin-client-activity-log-port.test.ts index 1614b330ab..680f5d7948 100644 --- a/packages/workflow-executor/test/adapters/forestadmin-client-activity-log-port.test.ts +++ b/packages/workflow-executor/test/adapters/forestadmin-client-activity-log-port.test.ts @@ -166,9 +166,45 @@ describe('ForestadminClientActivityLogPort', () => { expect.objectContaining({ collectionName: '11' }), ); }); + + it('warns (with collectionId) and returns a not-persisted handle when the server responds 200 with id:null', async () => { + const service = makeService(); + service.createActivityLog.mockResolvedValue({ + id: null, + attributes: {}, + } as unknown as { id: string; attributes: { index: string } }); + const logger = makeLogger(); + const port = makePort(service, { logger }); + + const handle = await port.createPending({ + renderingId: 5, + action: 'action', + type: 'write', + collectionId: '11', + }); + + expect(handle).toEqual({ id: null }); + expect(service.createActivityLog).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + 'activity log not persisted by server — status updates will be skipped', + { action: 'action', collectionId: '11' }, + ); + }); }); describe('markSucceeded', () => { + it('skips the status update and drainer tracking when the handle was not persisted', async () => { + const service = makeService(); + const drainer = new ActivityLogDrainer(); + const trackSpy = jest.spyOn(drainer, 'track'); + const port = makePort(service, { drainer }); + + await port.markSucceeded({ id: null }); + + expect(service.updateActivityLogStatus).not.toHaveBeenCalled(); + expect(trackSpy).not.toHaveBeenCalled(); + }); + it('retries on 503 and eventually resolves without rethrowing', async () => { const service = makeService(); service.updateActivityLogStatus @@ -214,6 +250,18 @@ describe('ForestadminClientActivityLogPort', () => { }); describe('markFailed', () => { + it('skips the status update and drainer tracking when the handle was not persisted', async () => { + const service = makeService(); + const drainer = new ActivityLogDrainer(); + const trackSpy = jest.spyOn(drainer, 'track'); + const port = makePort(service, { drainer }); + + await port.markFailed({ id: null }, 'boom'); + + expect(service.updateActivityLogStatus).not.toHaveBeenCalled(); + expect(trackSpy).not.toHaveBeenCalled(); + }); + it('sends status: failed (no errorMessage — server schema rejects unknown fields) and retries on 503', async () => { const service = makeService(); service.updateActivityLogStatus