Skip to content
Merged
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
20 changes: 18 additions & 2 deletions src/agents/definitions/contextSteps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ export async function fetchWorkItemStep(params: FetchContextParams): Promise<Con
const provider = getPMProviderOrNull();
const limited = media.slice(0, MAX_IMAGES_PER_WORK_ITEM);

params.logWriter('INFO', 'fetchWorkItemStep: downloading work item images', {
workItemId: params.input.workItemId,
count: limited.length,
});

const { jiraClient } = await import('../../jira/client.js');
const { trelloClient } = await import('../../trello/client.js');

Expand All @@ -136,15 +141,20 @@ export async function fetchWorkItemStep(params: FetchContextParams): Promise<Con
} else {
downloaded = await trelloClient.downloadAttachment(ref.url);
}
if (!downloaded) return null;
if (!downloaded) {
params.logWriter('WARN', 'fetchWorkItemStep: image download returned null', {
url: ref.url.split('?')[0],
});
return null;
}
return {
base64Data: downloaded.buffer.toString('base64'),
mimeType: downloaded.mimeType,
altText: ref.altText,
};
} catch (err) {
params.logWriter('WARN', 'fetchWorkItemStep: failed to download image', {
url: ref.url,
url: ref.url.split('?')[0],
error: err instanceof Error ? err.message : String(err),
});
return null;
Expand All @@ -153,6 +163,12 @@ export async function fetchWorkItemStep(params: FetchContextParams): Promise<Con
);

const images = results.filter((r) => r !== null);
params.logWriter('INFO', 'fetchWorkItemStep: image download complete', {
workItemId: params.input.workItemId,
attempted: limited.length,
downloaded: images.length,
skipped: limited.length - images.length,
});
if (images.length > 0) {
injection.images = images;
}
Expand Down
2 changes: 1 addition & 1 deletion src/agents/shared/builderFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export async function createConfiguredBuilder(options: CreateBuilderOptions): Pr
builder = builder.withBudget(remainingBudgetUsd);
} catch (err) {
if (err instanceof BudgetPricingUnavailableError) {
logWriter('warn', 'Budget enforcement unavailable for model', { model });
logWriter('WARN', 'Budget enforcement unavailable for model', { model });
} else {
throw err;
}
Expand Down
57 changes: 52 additions & 5 deletions tests/unit/agents/definitions/contextSteps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,16 @@ describe('fetchWorkItemStep', () => {
expect(mockTrelloDownload).not.toHaveBeenCalled();
});

it('skips failed downloads gracefully and logs warning', async () => {
it('logs WARN and skips when download returns null, stripping query params from URL', async () => {
mockReadWorkItemWithMedia.mockResolvedValue({
text: '# Card',
media: [
{ url: 'https://trello.com/ok.png', mimeType: 'image/png', source: 'description' },
{ url: 'https://trello.com/fail.png', mimeType: 'image/png', source: 'description' },
{
url: 'https://trello.com/fail.png?key=secret&token=abc',
mimeType: 'image/png',
source: 'description',
},
],
});
mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never);
Expand All @@ -294,12 +298,25 @@ describe('fetchWorkItemStep', () => {
// Only 1 successful image
expect(result[0].images).toHaveLength(1);
expect(result[0].images?.[0].base64Data).toBe(Buffer.from('ok').toString('base64'));

// WARN for the null return, URL sanitized (no query params)
expect(params.logWriter).toHaveBeenCalledWith(
'WARN',
'fetchWorkItemStep: image download returned null',
{ url: 'https://trello.com/fail.png' },
);
});

it('logs warning when download throws an exception', async () => {
it('logs warning when download throws an exception, stripping query params from URL', async () => {
mockReadWorkItemWithMedia.mockResolvedValue({
text: '# Card',
media: [{ url: 'https://trello.com/err.png', mimeType: 'image/png', source: 'description' }],
media: [
{
url: 'https://trello.com/err.png?key=secret&token=abc',
mimeType: 'image/png',
source: 'description',
},
],
});
mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never);
mockTrelloDownload.mockRejectedValue(new Error('network failure'));
Expand All @@ -311,7 +328,37 @@ describe('fetchWorkItemStep', () => {
expect(params.logWriter).toHaveBeenCalledWith(
'WARN',
'fetchWorkItemStep: failed to download image',
expect.objectContaining({ error: 'network failure' }),
{ url: 'https://trello.com/err.png', error: 'network failure' },
);
});

it('emits INFO logs before and after download with correct counts', async () => {
mockReadWorkItemWithMedia.mockResolvedValue({
text: '# Card',
media: [
{ url: 'https://trello.com/a.png', mimeType: 'image/png', source: 'description' },
{ url: 'https://trello.com/b.png', mimeType: 'image/png', source: 'description' },
{ url: 'https://trello.com/c.png', mimeType: 'image/png', source: 'description' },
],
});
mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never);
mockTrelloDownload
.mockResolvedValueOnce({ buffer: Buffer.from('data'), mimeType: 'image/png' })
.mockResolvedValueOnce(null) // returns null
.mockRejectedValueOnce(new Error('timeout')); // throws

const params = makeParams({ workItemId: 'card-1' });
await fetchWorkItemStep(params);

expect(params.logWriter).toHaveBeenCalledWith(
'INFO',
'fetchWorkItemStep: downloading work item images',
{ workItemId: 'card-1', count: 3 },
);
expect(params.logWriter).toHaveBeenCalledWith(
'INFO',
'fetchWorkItemStep: image download complete',
{ workItemId: 'card-1', attempted: 3, downloaded: 1, skipped: 2 },
);
});

Expand Down
Loading