fix(review): address latest CodeRabbit comments

This commit is contained in:
2026-03-19 23:49:55 -07:00
parent 42028d0a4d
commit 24667ad6c9
4 changed files with 92 additions and 3 deletions

View File

@@ -438,6 +438,40 @@ test('stats command aborts pending response wait when app exits before startup r
assert.equal(aborted, true); assert.equal(aborted, true);
}); });
test('stats command aborts pending response wait when attached app fails to spawn', async () => {
const context = createContext();
context.args.stats = true;
const spawnError = new Error('spawn failed');
let aborted = false;
await assert.rejects(
async () => {
await runStatsCommand(context, {
createTempDir: () => '/tmp/subminer-stats-test',
joinPath: (...parts) => parts.join('/'),
runAppCommandAttached: async () => {
throw spawnError;
},
waitForStatsResponse: async (_responsePath, signal) =>
await new Promise((resolve) => {
signal?.addEventListener(
'abort',
() => {
aborted = true;
resolve({ ok: false, error: 'aborted' });
},
{ once: true },
);
}),
removeDir: () => {},
});
},
(error: unknown) => error === spawnError,
);
assert.equal(aborted, true);
});
test('stats cleanup command aborts pending response wait when app exits before startup response', async () => { test('stats cleanup command aborts pending response wait when app exits before startup response', async () => {
const context = createContext(); const context = createContext();
context.args.stats = true; context.args.stats = true;

View File

@@ -34,6 +34,11 @@ type StatsResponseWait = {
promise: Promise<{ kind: 'response'; response: StatsCommandResponse }>; promise: Promise<{ kind: 'response'; response: StatsCommandResponse }>;
}; };
type StatsStartupResult =
| { kind: 'response'; response: StatsCommandResponse }
| { kind: 'exit'; status: number }
| { kind: 'spawn-error'; error: unknown };
const defaultDeps: StatsCommandDeps = { const defaultDeps: StatsCommandDeps = {
createTempDir: (prefix) => fs.mkdtempSync(path.join(os.tmpdir(), prefix)), createTempDir: (prefix) => fs.mkdtempSync(path.join(os.tmpdir(), prefix)),
joinPath: (...parts) => path.join(...parts), joinPath: (...parts) => path.join(...parts),
@@ -72,11 +77,19 @@ async function performStartupHandshake(
attachedExitPromise: Promise<number>, attachedExitPromise: Promise<number>,
): Promise<boolean> { ): Promise<boolean> {
const responseWait = createResponseWait(); const responseWait = createResponseWait();
const startupResult = await Promise.race([ const startupResult = await Promise.race<StatsStartupResult>([
responseWait.promise, responseWait.promise,
attachedExitPromise.then((status) => ({ kind: 'exit' as const, status })), attachedExitPromise.then(
(status) => ({ kind: 'exit' as const, status }),
(error) => ({ kind: 'spawn-error' as const, error }),
),
]); ]);
if (startupResult.kind === 'spawn-error') {
responseWait.controller.abort();
throw startupResult.error;
}
if (startupResult.kind === 'exit') { if (startupResult.kind === 'exit') {
if (startupResult.status !== 0) { if (startupResult.status !== 0) {
responseWait.controller.abort(); responseWait.controller.abort();

View File

@@ -263,6 +263,41 @@ test('KnownWordCacheManager refresh incrementally reconciles deleted and edited
} }
}); });
test('KnownWordCacheManager skips malformed note info without fields', async () => {
const config: AnkiConnectConfig = {
fields: {
word: 'Word',
},
knownWords: {
highlightEnabled: true,
},
};
const { manager, clientState, cleanup } = createKnownWordCacheHarness(config);
try {
clientState.findNotesResult = [1, 2];
clientState.notesInfoResult = [
{
noteId: 1,
fields: undefined as unknown as Record<string, { value: string }>,
},
{
noteId: 2,
fields: {
Word: { value: '猫' },
},
},
];
await manager.refresh(true);
assert.equal(manager.isKnownWord('猫'), true);
assert.equal(manager.isKnownWord('犬'), false);
} finally {
cleanup();
}
});
test('KnownWordCacheManager preserves cache state key captured before refresh work', async () => { test('KnownWordCacheManager preserves cache state key captured before refresh work', async () => {
const config: AnkiConnectConfig = { const config: AnkiConnectConfig = {
fields: { fields: {

View File

@@ -478,7 +478,14 @@ export class KnownWordCacheManager {
const notesInfoResult = (await this.deps.client.notesInfo(chunk)) as unknown[]; const notesInfoResult = (await this.deps.client.notesInfo(chunk)) as unknown[];
const chunkInfos = notesInfoResult as KnownWordCacheNoteInfo[]; const chunkInfos = notesInfoResult as KnownWordCacheNoteInfo[];
for (const noteInfo of chunkInfos) { for (const noteInfo of chunkInfos) {
if (!noteInfo || !Number.isInteger(noteInfo.noteId) || noteInfo.noteId <= 0) { if (
!noteInfo ||
!Number.isInteger(noteInfo.noteId) ||
noteInfo.noteId <= 0 ||
typeof noteInfo.fields !== 'object' ||
noteInfo.fields === null ||
Array.isArray(noteInfo.fields)
) {
continue; continue;
} }
noteInfos.push(noteInfo); noteInfos.push(noteInfo);