Skip to content

Commit 050237e

Browse files
committed
test(daemon): assert the retained session's in-memory close action, not just the durable event
Review feedback on #1392 (thymikee): the durable-event assertion alone doesn't catch a reintroduced session.actions.length = actionsBeforeClose rollback, because that event is queued (and durable) before the write even attempts — a regression there would leave the assertion passing while silently reintroducing the in-memory/durable mismatch. Retain the session object past handleCloseCommand (store.delete only drops the map entry, not the object a local variable still points at) and assert its actions array contains exactly one close entry, matching the durable event count. Verified by temporarily reintroducing the old rollback locally: this assertion fails (0 !== 1) where the prior durable-only check did not, then reverted.
1 parent c839ee4 commit 050237e

1 file changed

Lines changed: 19 additions & 2 deletions

File tree

src/daemon/handlers/__tests__/session-device-claims.test.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { SessionStore } from '../../session-store.ts';
3535
import { handleCloseCommand } from '../session-close.ts';
3636
import { handleOpenCommand } from '../session-open.ts';
3737
import type { DeviceInfo } from '../../../kernel/device.ts';
38+
import type { SessionState } from '../../types.ts';
3839
import { AppError } from '../../../kernel/errors.ts';
3940

4041
const mockDispatch = vi.mocked(dispatchCommand);
@@ -258,15 +259,20 @@ test('#1391: a close-time script save failure still clears the advisory claim an
258259
assert.ok(acquired.ownership);
259260
const targetPath = path.join(stateDir, 'already-published.ad');
260261
fs.writeFileSync(targetPath, 'pre-existing\n');
261-
store.set('close-save-script-failure', {
262+
// Retained directly (not just looked up via `store`) so it stays inspectable
263+
// after `handleCloseCommand` deletes it from the store — `store.delete` only
264+
// drops the map entry, it doesn't touch the object this variable still
265+
// points at.
266+
const session: SessionState = {
262267
name: 'close-save-script-failure',
263268
device: android,
264269
deviceClaim: acquired.ownership,
265270
createdAt: Date.now(),
266271
actions: [],
267272
recordSession: true,
268273
saveScriptPath: targetPath,
269-
});
274+
};
275+
store.set('close-save-script-failure', session);
270276
mockDispatch.mockResolvedValue({});
271277

272278
// Like the platform-close-error tests above, a failed close-time save is
@@ -323,4 +329,15 @@ test('#1391: a close-time script save failure still clears the advisory claim an
323329
closeEvent,
324330
'expected a durable action.recorded:close event to survive the failed save',
325331
);
332+
// Same assertion, in-memory: reinstating `session.actions.length =
333+
// actionsBeforeClose` would still pass the durable-event check above (that
334+
// event was already queued before the write failed) while silently
335+
// dropping this to zero — assert directly on the retained session object,
336+
// not just the store, so that specific regression is caught.
337+
const inMemoryCloseActions = session.actions.filter((action) => action.command === 'close');
338+
assert.equal(inMemoryCloseActions.length, 1);
339+
assert.equal(
340+
inMemoryCloseActions.length,
341+
events.filter((event) => event.command === 'close').length,
342+
);
326343
});

0 commit comments

Comments
 (0)