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
4 changes: 4 additions & 0 deletions apps/desktop/e2e-budget.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
"tests": 1,
"electron": "the folder reference has to survive a renderer reload and still agree with the Host's session record"
},
"expected-failure-feedback.spec.ts": {
"tests": 2,
"electron": "WorkHub attachment rejection crosses a native file dialog, preload and main; Session settings and Plan failures must preserve their codes through a real contextBridge IPC round trip"
},
"new-task-reload.spec.ts": {
"tests": 2,
"electron": "renderer reload must preserve an explicit new task and rebuild archived-only Host history as an empty, usable new-task surface"
Expand Down
88 changes: 88 additions & 0 deletions apps/desktop/e2e/expected-failure-feedback.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { truncate, writeFile } from 'node:fs/promises';
import path from 'node:path';
import { FAKE_HOLD_OPEN_PROMPT } from '@maka/runtime/test-only/fake-backend';
import { MAX_ATTACHMENT_BYTES } from '@maka/core/attachments';
import {
awaitSendReady,
COMPOSER_INPUT,
expect,
getWorkHubPage,
test,
withE2eWindow,
} from './fixtures';

test('WorkHub shows the main-side attachment rejection reason', async ({}, testInfo) => {
await withE2eWindow(
{ seed: true, readinessSelector: COMPOSER_INPUT, locale: 'zh-CN', showWindow: true },
async (page, { app, userDataDir }) => {
const attachmentPath = path.join(userDataDir, 'grew-after-selection.txt');
await writeFile(attachmentPath, 'small');
await app.evaluate(({ dialog }, selectedPath) => {
dialog.showOpenDialog = async () => ({ canceled: false, filePaths: [selectedPath] });
}, attachmentPath);
await page.evaluate(() => window.maka.settings.updateClient({ workHub: { enabled: true } }));
const workHub = await getWorkHubPage(app);
const addMenu = workHub.locator('.maka-composer-plus-menu button').first();
await addMenu.click();
await workHub.getByRole('menuitem', { name: '添加文件', exact: true }).click();
await expect(workHub.getByText('grew-after-selection.txt', { exact: true })).toBeVisible();

await truncate(attachmentPath, MAX_ATTACHMENT_BYTES + 1);
await workHub.locator(COMPOSER_INPUT).fill('验证附件错误提示');
await workHub.locator(COMPOSER_INPUT).press('Enter');
await expect(workHub.getByText('发送失败', { exact: true })).toBeVisible();
const screenshotPath = testInfo.outputPath('workhub-attachment-rejection.png');
await workHub.screenshot({ path: screenshotPath, animations: 'disabled' });
await testInfo.attach('WorkHub attachment rejection', {
path: screenshotPath,
contentType: 'image/png',
});
await expect(workHub.getByText('单个附件超出大小限制。', { exact: true })).toBeVisible();
},
);
});

test('setting and Plan failures retain their codes through Electron', async ({ window: page }) => {
const composer = page.locator(COMPOSER_INPUT);
await composer.fill(FAKE_HOLD_OPEN_PROMPT);
await awaitSendReady(page);
await composer.press('Enter');
await expect.poll(async () => {
const sessions = await page.evaluate(() => window.maka.sessions.list());
return sessions.some(({ status }) => status === 'running');
}).toBe(true);
const sessionId = await page.evaluate(async () => {
const sessions = await window.maka.sessions.list();
return sessions.find(({ status }) => status === 'running')!.id;
});

const result = await page.evaluate(async (id) => ({
setting: await window.maka.sessions.setPermissionMode(id, 'explore'),
plan: await window.maka.sessions.abandonPlanProposal(id, 'missing-proposal'),
}), sessionId);
expect(result.setting).toEqual({ ok: false, code: 'session_busy' });
expect(result.plan).toMatchObject({
ok: false,
error: { code: 'session_busy' },
});
await page.evaluate((id) => window.maka.sessions.stop(id), sessionId);
});
7 changes: 3 additions & 4 deletions apps/desktop/renderer-architecture.json
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@
"@maka/ui": 1
},
"importSpecifiers": 10,
"nonTriviaTokens": 3636
"nonTriviaTokens": 3618
},
"src/renderer/app-shell-chrome-actions.tsx": {
"importDeclarations": 4,
Expand Down Expand Up @@ -707,15 +707,13 @@
"window.maka.diagnostics.takePreviousMainProcessInterruption": 1,
"window.maka.notifications.runEnded": 1,
"window.maka.onboarding.setMilestone": 1,
"window.maka.sessions.abandonPlanProposal": 1,
"window.maka.sessions.compact": 1,
"window.maka.sessions.getPlanState": 1,
"window.maka.sessions.listActiveInteractions": 1,
"window.maka.sessions.listTurnLandmarks": 1,
"window.maka.sessions.promoteQueueEntry": 1,
"window.maka.sessions.reorderQueueEntries": 1,
"window.maka.sessions.retractQueueEntry": 1,
"window.maka.sessions.setCollaborationMode": 1,
"window.maka.sessions.subscribeActiveInteractions": 1,
"window.maka.sessions.updateQueueEntry": 1,
"window.maka.settings.getClient": 1,
Expand Down Expand Up @@ -856,7 +854,7 @@
"react": 1
},
"importSpecifiers": 104,
"nonTriviaTokens": 13394
"nonTriviaTokens": 13386
},
"src/renderer/use-app-shell-composer-quotes.ts": {
"importDeclarations": 2,
Expand Down Expand Up @@ -1701,6 +1699,7 @@
"unresolvedDependencies": 0,
"actionFactories": [],
"dependencyPaths": {
"@maka/core/attachments": 1,
"@maka/core/redaction": 1,
"@maka/core/ui-locale": 1
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,6 @@
* under the License.
*/

/**
* Review regression on #4457: the new-task send path runs the renderer-side
* attachment preflight BEFORE `newTasks.create`, so the main-side token
* validation never sees an over-limit request. The preflight must reject with
* the same stable `attachment_ingest:<code>` tokens main rejects with, so the
* send catch maps the real reason through the locale catalog instead of the
* generic "try again later" fallback (retrying nine attachments can never
* succeed) — and an expected rejection must not log an unexpected diagnostic.
*/

import { MAX_ATTACHMENT_COUNT } from '@maka/core/attachments';
import { strict as assert } from 'node:assert';
import { test } from 'node:test';
Expand Down Expand Up @@ -96,4 +86,42 @@ test('a nine-attachment new-task send shows the count reason, creates no session
0,
'an expected preflight rejection must not land the unexpected-error diagnostic',
);
});
});

test('a main-side attachment rejection refuses the send instead of leaving it unreconciled', async (context) => {
const errorLog = context.mock.method(console, 'error', () => undefined);
const created: string[] = [];
const toasts: Array<{ title: string; description?: string }> = [];
const restoreWindow = installWindow({
newTasks: {
create: async () => {
created.push('session-1');
return { id: 'session-1' };
},
},
sessions: {
submitMessage: async () => ({ ok: false, reason: 'attachment_blocked', code: 'item_too_large' }),
},
});
try {
const actions = createAppShellChatActions({
...createActionsDeps(),
uiLocale: 'zh-CN',
toastApi: {
error: (title, description) => {
toasts.push({ title, description });
},
info: () => undefined,
},
});
const accepted = await actions.send('hello', [fileAttachment(10, 0)]);
assert.equal(accepted, false, 'a blocked attachment is a definitive refusal, not an unknown outcome');
assert.deepEqual(toasts, [{
title: getShellCopy('zh-CN').chatActions.sendFailedTitle,
description: getShellCopy('zh-CN').sessionSettingsActions.attachmentIngestBlocked.item_too_large,
}]);
} finally {
restoreWindow();
}
assert.equal(errorLog.mock.callCount(), 0, 'an expected attachment rejection logs no diagnostic');
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { encodeIngestItems } from '../../preload/attachment-ingest-payload.js';
describe('encodeIngestItems', () => {
test('rejects more than 8 items without reading any file bytes', async () => {
const items = Array.from({ length: 9 }, (_, i) => ({ approvalId: `a${i}`, name: `f${i}.txt` }));
await assert.rejects(encodeIngestItems(items as never), /attachment_ingest:count_limit/);
await assert.rejects(encodeIngestItems(items as never), { code: 'count_limit' });
});

test('rejects a File over 50MB without calling arrayBuffer', async () => {
Expand All @@ -38,7 +38,7 @@ describe('encodeIngestItems', () => {
return new ArrayBuffer(0);
},
} as unknown as File;
await assert.rejects(encodeIngestItems([{ file: bigFile }]), /attachment_ingest:item_too_large/);
await assert.rejects(encodeIngestItems([{ file: bigFile }]), { code: 'item_too_large' });
assert.equal(arrayBufferCalls, 0, 'arrayBuffer must not be called for an oversized file');
});

Expand Down Expand Up @@ -67,7 +67,7 @@ describe('encodeIngestItems', () => {
test('rejects a raw base64 item that is neither a File nor an approval token', async () => {
await assert.rejects(
encodeIngestItems([{ name: 'forged', base64: 'AAAA' }] as never),
/attachment_ingest:items_invalid/,
{ code: 'items_invalid' },
);
});
});
32 changes: 19 additions & 13 deletions apps/desktop/src/main/__tests__/attachment-ingest-resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => (statCalls++, { size: 1 }),
}),
/attachment_ingest:count_limit/,
{ code: 'count_limit' },
);
assert.equal(statCalls, 0);
});
Expand All @@ -61,7 +61,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => (statCalls++, { size: 1 }),
}),
/attachment_ingest:(source_expired|items_invalid)/,
{ code: /^(source_expired|items_invalid)$/ },
);
assert.equal(statCalls, 0);
});
Expand All @@ -78,7 +78,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => (statCalls++, { size: 1 }),
}),
/attachment_ingest:(source_expired|items_invalid)/,
{ code: /^(source_expired|items_invalid)$/ },
);
assert.equal(statCalls, 0);
});
Expand All @@ -96,7 +96,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
stat: async () => (statCalls++, { size: 200 }),
maxBytes: 100,
}),
/attachment_ingest:item_too_large/,
{ code: 'item_too_large' },
);
assert.equal(statCalls, 1);
});
Expand All @@ -121,7 +121,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
stat: async () => (statCalls++, { size: 1 }),
maxBytes: 100,
}),
/attachment_ingest:item_too_large/,
{ code: 'item_too_large' },
);
assert.equal(statCalls, 0);
assert.equal(decodeCalls, 0, 'must reject by base64 string length before Buffer.from');
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => ({ size: 10 }),
}),
/attachment_ingest:(source_expired|items_invalid)/,
{ code: /^(source_expired|items_invalid)$/ },
);
});

Expand All @@ -165,11 +165,17 @@ describe('resolveIngestItems (pre-read validation)', () => {
for (const item of items) assert.ok(approvals.peekApproval(1, item.approvalId));
const competing = await prepareIngestItems({ ...input, items: [items[1]] });
assert.equal(competing.commit(() => 'admitted'), 'admitted');
assert.throws(() => plan.commit(() => assert.fail('must not admit an invalid plan')), /attachment_ingest:source_expired/);
assert.throws(
() => plan.commit(() => assert.fail('must not admit an invalid plan')),
{ code: 'source_expired' },
);
assert.ok(approvals.peekApproval(1, items[0]!.approvalId), 'a lost race must not burn the other approval');
const remaining = await prepareIngestItems({ ...input, items: [items[0]] });
approvals.clearSender(1);
assert.throws(() => remaining.commit(() => assert.fail('must not admit after sender teardown')), /attachment_ingest:source_expired/);
assert.throws(
() => remaining.commit(() => assert.fail('must not admit after sender teardown')),
{ code: 'source_expired' },
);
});

test('resolves a mix of approved paths and blobs into ingest files', async () => {
Expand Down Expand Up @@ -204,7 +210,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => (statCalls++, { size: 10 }),
}),
/attachment_ingest:items_invalid/,
{ code: 'items_invalid' },
);
assert.notEqual(
approvals.consumeApproval(1, issued.approvalId),
Expand All @@ -227,7 +233,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => (statCalls++, { size: 10 }),
}),
/attachment_ingest:duplicate_source/,
{ code: 'duplicate_source' },
);
assert.notEqual(
approvals.consumeApproval(1, issued.approvalId),
Expand All @@ -246,7 +252,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => ({ size: 1 }),
}),
/attachment_ingest:items_invalid/,
{ code: 'items_invalid' },
);
await assert.rejects(
() =>
Expand All @@ -256,7 +262,7 @@ describe('resolveIngestItems (pre-read validation)', () => {
approvals,
stat: async () => ({ size: 1 }),
}),
/attachment_ingest:items_invalid/,
{ code: 'items_invalid' },
);
});
});
Expand Down Expand Up @@ -440,7 +446,7 @@ describe('resolveAttachmentRefs', () => {
throw new Error('snapshot must not run');
},
}),
/attachment_ingest:item_too_large/,
{ code: 'item_too_large' },
);
assert.equal(snapshots, 0);
} finally {
Expand Down
14 changes: 5 additions & 9 deletions apps/desktop/src/main/__tests__/attachment-preflight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ describe('attachment preflight (before session create)', () => {
size: 100,
source: { type: 'file' as const, file: { size: 100 } },
}));
assert.throws(() => preflightAttachmentItems(items), {
message: 'attachment_ingest:count_limit',
});
assert.throws(() => preflightAttachmentItems(items), { code: 'count_limit' });
});

test('rejects an oversized File so no empty session is created', () => {
Expand All @@ -39,7 +37,7 @@ describe('attachment preflight (before session create)', () => {
preflightAttachmentItems([
{ size: MAX_ATTACHMENT_BYTES + 1, source: { type: 'file', file: { size: MAX_ATTACHMENT_BYTES + 1 } } },
]),
{ message: 'attachment_ingest:item_too_large' },
{ code: 'item_too_large' },
);
});

Expand All @@ -49,7 +47,7 @@ describe('attachment preflight (before session create)', () => {
preflightAttachmentItems([
{ size: MAX_ATTACHMENT_BYTES + 1, source: { type: 'approval', approvalId: 'a1' } },
]),
{ message: 'attachment_ingest:item_too_large' },
{ code: 'item_too_large' },
);
});

Expand All @@ -58,9 +56,7 @@ describe('attachment preflight (before session create)', () => {
{ size: 10, source: { type: 'approval' as const, approvalId: 'dup' } },
{ size: 10, source: { type: 'approval' as const, approvalId: 'dup' } },
];
assert.throws(() => preflightAttachmentItems(duplicate), {
message: 'attachment_ingest:duplicate_source',
});
assert.throws(() => preflightAttachmentItems(duplicate), { code: 'duplicate_source' });
});

test('passes approval tokens and files under the cap', () => {
Expand All @@ -71,4 +67,4 @@ describe('attachment preflight (before session create)', () => {
]),
);
});
});
});
Loading