diff --git a/cdk/src/handlers/get-task-events.ts b/cdk/src/handlers/get-task-events.ts index f9b2a58a..9a4cf04a 100644 --- a/cdk/src/handlers/get-task-events.ts +++ b/cdk/src/handlers/get-task-events.ts @@ -118,7 +118,16 @@ export async function handler(event: APIGatewayProxyEvent): Promise :after`` would return zero rows — silently dropping + // the rest of the event stream for a contract-valid input. + const afterValid = typeof afterRaw === 'string' && afterRaw.length === ULID_LENGTH + ? afterRaw.toUpperCase() + : undefined; // 3b. ``desc`` combined with ``after`` makes no semantic sense: ``after`` // is a forward-walking ULID cursor. Reject the combination rather than diff --git a/cdk/src/handlers/shared/create-task-core.ts b/cdk/src/handlers/shared/create-task-core.ts index 3db2ebaa..8ec057a8 100644 --- a/cdk/src/handlers/shared/create-task-core.ts +++ b/cdk/src/handlers/shared/create-task-core.ts @@ -339,7 +339,11 @@ export async function createTaskCore( parseResult.scope.startsWith('bash_pattern:') || parseResult.scope.startsWith('write_path:') ) { - const value = parseResult.scope.split(':', 2)[1] ?? ''; + // Take everything after the first colon — the value itself may + // contain colons (e.g. ``bash_pattern:git log --format=%h:%s``), so a + // ``split(':', 2)`` would truncate it and could turn a legitimate + // pattern into a degenerate-looking fragment, producing a spurious 400. + const value = parseResult.scope.slice(parseResult.scope.indexOf(':') + 1); if (isDegeneratePattern(value)) { return errorResponse( 400, diff --git a/cdk/test/handlers/get-task-events.test.ts b/cdk/test/handlers/get-task-events.test.ts index ad8c70fa..fb8771e9 100644 --- a/cdk/test/handlers/get-task-events.test.ts +++ b/cdk/test/handlers/get-task-events.test.ts @@ -250,6 +250,22 @@ describe('get-task-events handler', () => { expect(queryInput.ExclusiveStartKey).toBeUndefined(); }); + test('lower-case ?after is normalized to upper-case before the DDB query', async () => { + // ``isValidUlid`` accepts lower-case callers, but stored event_ids are + // upper-case Crockford Base32 and DDB compares raw bytes (lower-case sorts + // after upper-case). Without normalization a lower-case cursor would be + // "greater than" every stored id and silently return zero events. + const event = makeEvent({ queryStringParameters: { after: VALID_AFTER.toLowerCase() } }); + await handler(event); + + const queryInput = MockQueryCommand.mock.calls[0][0]; + expect(queryInput.KeyConditionExpression).toBe('task_id = :tid AND event_id > :after'); + expect(queryInput.ExpressionAttributeValues).toEqual({ + ':tid': 'task-1', + ':after': VALID_AFTER, // upper-cased, not the lower-case input + }); + }); + test('both after and next_token → after wins + WARN logged', async () => { const stdoutWrite = jest.spyOn(process.stdout, 'write').mockImplementation(() => true); try { diff --git a/cdk/test/handlers/shared/create-task-core.test.ts b/cdk/test/handlers/shared/create-task-core.test.ts index 8edf6220..21d59e9b 100644 --- a/cdk/test/handlers/shared/create-task-core.test.ts +++ b/cdk/test/handlers/shared/create-task-core.test.ts @@ -114,8 +114,39 @@ describe('createTaskCore', () => { expect(mockLambdaSend).toHaveBeenCalledTimes(1); }); - test('returns 400 for invalid repo', async () => { - const result = await createTaskCore({ repo: 'invalid' } as any, makeContext(), 'req-1'); + test('accepts an initial_approvals pattern whose value contains a colon', async () => { + // Regression: the degenerate-pattern guard used split(':', 2)[1], which + // truncated the value at the next colon. For "ab:cdefgh" that yields the + // 2-char fragment "ab", which isDegeneratePattern flags as degenerate — + // a spurious 400. The full value "ab:cdefgh" is not degenerate, so the + // scope must be accepted. + const result = await createTaskCore( + { + repo: 'org/repo', + task_description: 'Fix the bug', + initial_approvals: ['bash_pattern:ab:cdefgh'], + } as any, + makeContext(), + 'req-1', + ); + expect(result.statusCode).toBe(201); + }); + + test('still rejects a genuinely degenerate initial_approvals pattern', async () => { + const result = await createTaskCore( + { + repo: 'org/repo', + task_description: 'Fix the bug', + initial_approvals: ['bash_pattern:*'], + } as any, + makeContext(), + 'req-1', + ); + expect(result.statusCode).toBe(400); + expect(JSON.parse(result.body).error.code).toBe('VALIDATION_ERROR'); + }); + + test('returns 400 for invalid repo', async () => { const result = await createTaskCore({ repo: 'invalid' } as any, makeContext(), 'req-1'); expect(result.statusCode).toBe(400); expect(JSON.parse(result.body).error.code).toBe('VALIDATION_ERROR'); });