mirror of
https://github.com/dadosfera/maestro.git
synced 2026-11-12 16:39:08 +00:00
fix: PipelineExecutionGuard allows edits when pipeline has no run history
The guard read `status[status.length-1].last_status` on the pipeline_run history. For a pipeline that has never run — which is the permanent state of CDC pipelines, since they replicate continuously and never record batch runs — pipeline_run is empty, so the last element is undefined and `.last_status` threw, surfacing as "Error checking pipeline status: Cannot read properties of undefined (reading 'last_status')". This blocked Edit Objects (and delete-table) for every CDC pipeline. Treat an empty/statusless run history as "not running" and allow the edit (undefined never satisfied 'running' anyway). Also stop the catch from double-wrapping the deliberate is-running BadRequestException, so that rejection keeps its clear message; genuine status-check failures still fail closed with the wrapped message (safe default for a destructive-op gate). Adds a guard spec: empty history -> allow, statusless -> allow, not running -> allow, running -> is-running message (not wrapped), platform error -> wrapped message. Co-Authored-By: WOZCODE <contact@withwoz.com> Claude-Session: https://claude.ai/code/session_01145m1zZMfx8RSJBxAhySdg
This commit is contained in:
@@ -0,0 +1,71 @@
|
||||
import { BadRequestException } from '@nestjs/common';
|
||||
import { PipelineExecutionGuard } from './pipeline-execution.guard';
|
||||
|
||||
const logger = { info: jest.fn(), error: jest.fn() };
|
||||
|
||||
function buildGuard(proxyImpl: jest.Mock) {
|
||||
const platformApiService: any = { proxy: proxyImpl };
|
||||
return new PipelineExecutionGuard(
|
||||
{ logger } as any,
|
||||
platformApiService,
|
||||
);
|
||||
}
|
||||
|
||||
function contextWith(pipelineId = 'abc-123') {
|
||||
return {
|
||||
switchToHttp: () => ({
|
||||
getRequest: () => ({ params: { pipelineId }, user: {} }),
|
||||
}),
|
||||
} as any;
|
||||
}
|
||||
|
||||
describe('PipelineExecutionGuard', () => {
|
||||
afterEach(() => jest.clearAllMocks());
|
||||
|
||||
it('allows the edit when the pipeline has no run history (empty array)', async () => {
|
||||
const guard = buildGuard(jest.fn().mockResolvedValue([]));
|
||||
await expect(guard.canActivate(contextWith())).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it('allows the edit when the last run has no last_status', async () => {
|
||||
const guard = buildGuard(jest.fn().mockResolvedValue([{}]));
|
||||
await expect(guard.canActivate(contextWith())).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it('allows the edit when the pipeline is not running', async () => {
|
||||
const guard = buildGuard(
|
||||
jest.fn().mockResolvedValue([{ last_status: 'SUCCEEDED' }]),
|
||||
);
|
||||
await expect(guard.canActivate(contextWith())).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it('blocks with the is-running message when the pipeline is running', async () => {
|
||||
const guard = buildGuard(
|
||||
jest.fn().mockResolvedValue([{ last_status: 'RUNNING' }]),
|
||||
);
|
||||
await expect(guard.canActivate(contextWith())).rejects.toThrow(
|
||||
'Pipeline is running, cannot update input now',
|
||||
);
|
||||
});
|
||||
|
||||
it('wraps a genuine status-check failure (fail closed)', async () => {
|
||||
const guard = buildGuard(
|
||||
jest.fn().mockRejectedValue(new Error('platform down')),
|
||||
);
|
||||
await expect(guard.canActivate(contextWith())).rejects.toThrow(
|
||||
'Error checking pipeline status: platform down',
|
||||
);
|
||||
});
|
||||
|
||||
it('does not double-wrap the is-running BadRequestException', async () => {
|
||||
const guard = buildGuard(
|
||||
jest.fn().mockResolvedValue([{ last_status: 'running' }]),
|
||||
);
|
||||
await expect(guard.canActivate(contextWith())).rejects.toBeInstanceOf(
|
||||
BadRequestException,
|
||||
);
|
||||
await expect(guard.canActivate(contextWith())).rejects.not.toThrow(
|
||||
/Error checking pipeline status/,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -46,10 +46,16 @@ export class PipelineExecutionGuard implements CanActivate {
|
||||
user,
|
||||
);
|
||||
|
||||
const currentStatus = status[status.length - 1]
|
||||
const currentStatus = status?.[status.length - 1];
|
||||
|
||||
this.logger.info('Pipeline current status response:' + JSON.stringify(currentStatus));
|
||||
|
||||
|
||||
// No run history (e.g. CDC pipelines never record batch runs) means
|
||||
// nothing is executing — allow the edit rather than crash on .last_status.
|
||||
if (!currentStatus?.last_status) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (currentStatus.last_status.toLowerCase() === 'running') {
|
||||
this.logger.error('Pipeline is running, cannot update input now');
|
||||
throw new BadRequestException('Pipeline is running, cannot update input now');
|
||||
@@ -57,6 +63,11 @@ export class PipelineExecutionGuard implements CanActivate {
|
||||
return true;
|
||||
}
|
||||
} catch (error) {
|
||||
// Preserve the deliberate is-running rejection; only wrap genuine
|
||||
// status-check failures (fail closed on those for a destructive gate).
|
||||
if (error instanceof BadRequestException) {
|
||||
throw error;
|
||||
}
|
||||
this.logger.error('Error in PipelineExecutionGuard: ' + error.message);
|
||||
throw new BadRequestException('Error checking pipeline status: ' + error.message);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user