Add comprehensive test suite and fix i18n/config issues#47
Add comprehensive test suite and fix i18n/config issues#47alexander-acker wants to merge 2 commits into
Conversation
…add comprehensive tests Bug fixes: - Replace hardcoded Chinese strings in Sidebar, MessageCard with i18n t() calls - Replace hardcoded Chinese strings in config-store PROVIDER_PRESETS with English - Add missing memory_fts FTS5 virtual table creation in database schema - Fix pendingPermissions memory leak on session deletion in SessionManager New tests (9 files, 117 new tests): - config-store, credentials-store, database, i18n-completeness, logger, memory-manager, session-manager-lifecycle, session-update, store-actions All 37 test files (213 tests) passing. https://claude.ai/code/session_01EEkUmHuGJ1mJgB7mqoqTDK
85f68d9 to
8e1242e
Compare
There was a problem hiding this comment.
Findings
-
[Blocker]
tis referenced insideContentBlockViewwithout being defined in that component, so this change will fail the renderer build as soon as the new tooltip line is compiled. Evidencesrc/renderer/components/MessageCard.tsx:167.
Suggested fix:function ContentBlockView(...) { const { t } = useTranslation(); ... }
-
[Major]
deleteSession()now resolves and deletes every entry inpendingPermissions, not just the ones that belong to the session being removed. Deleting one chat can therefore deny tool prompts that are still waiting in other active sessions. Evidencesrc/main/session/session-manager.ts:614, with the session id only available at insertion time insrc/main/session/session-manager.ts:751.
Suggested fix:private pendingPermissions = new Map< string, { sessionId: string; resolve: (result: PermissionResult) => void } >(); this.pendingPermissions.set(toolUseId, { sessionId, resolve }); for (const [toolUseId, pending] of this.pendingPermissions) { if (pending.sessionId !== sessionId) continue; pending.resolve('deny'); this.pendingPermissions.delete(toolUseId); }
Summary
- Review mode: initial
- 2 issues found: a renderer build break in
MessageCardand cross-session permission cancellation inSessionManager. - Residual testing gap: the new tests do not cover
ContentBlockViewrendering or a multi-session pending-permission cleanup case.
Testing
- Not run (automation)
Open Cowork Bot
| }} | ||
| className={getFileLinkButtonClassName()} | ||
| title="在文件夹中定位" | ||
| title={t('messageCard.locateInFolder')} |
There was a problem hiding this comment.
[BLOCKER] t is only defined in MessageCard, but this new tooltip lives inside ContentBlockView, so the file no longer compiles once this line is added. Please initialize useTranslation() in ContentBlockView (or pass t down explicitly) before using it here.
Suggested fix:
function ContentBlockView(...) {
const { t } = useTranslation();
...
}| } | ||
|
|
||
| // Clean up pending permissions for this session | ||
| for (const [toolUseId, resolver] of this.pendingPermissions.entries()) { |
There was a problem hiding this comment.
[MAJOR] This loop now denies every pending permission in the process, not only the ones for sessionId. Because requestPermission() stores only the resolver today, deleting one session will also deny tool prompts that are still pending in other live sessions.
Suggested fix:
private pendingPermissions = new Map<
string,
{ sessionId: string; resolve: (result: PermissionResult) => void }
>();
this.pendingPermissions.set(toolUseId, { sessionId, resolve });
for (const [toolUseId, pending] of this.pendingPermissions) {
if (pending.sessionId !== sessionId) continue;
pending.resolve('deny');
this.pendingPermissions.delete(toolUseId);
}
Summary
This PR adds a comprehensive test suite covering core functionality and fixes several issues with internationalization and configuration management.
Key Changes
Test Suite (New Files)
Source Code Fixes
useTranslation()hook for proper i18n supportsidebar.deleteAllConfirmtranslation key in both English and ChineseNotable Implementation Details
https://claude.ai/code/session_01EEkUmHuGJ1mJgB7mqoqTDK