fix: harden allocation journal recovery

Author: thymikee

docs/adr/0021-host-simlock-managed-device-allocation.md

@@ -110,10 +110,12 @@ authorization and attribution around the same boundary. Plain local device selec
 
 Before acquisition, agent-device durably records a non-authoritative allocation operation: the
 logical requester, idempotency key, immutable shape request, deadline, and Host attribution when
-applicable. After Simlock responds, it records the allocator handle/outcome and whether Host
-published or cleaned it. This journal exists only to recover the Host-to-Simlock handoff. It never
-mirrors Simlock's queue, provisioning, lease, cleanup, health, or capacity states, and it never
-decides whether a device is reusable.
+applicable. After Simlock responds, it records the allocator handle/outcome. Before invoking an
+external Host binding publisher, it durably records a pending publication; publication success is
+then recorded separately, and recovery conservatively cleans a pending or uncertain binding before
+releasing the allocator lease. This journal exists only to recover the Host-to-Simlock handoff. It
+never mirrors Simlock's queue, provisioning, lease, cleanup, health, or capacity states, and it
+never decides whether a device is reusable.
 
 Each logical requester is a restart-stable allocation lane; concurrent leases use distinct lanes.
 Replaying the same attempt key returns the same durable outcome, including a refusal. Disconnect,

src/daemon/__tests__/allocation-operation-decision.test.ts

@@ -59,7 +59,19 @@ test('an allocator-unknown record never gets an implicit second request', () =>
 test('grants publish first, and release cleans the binding before calling the allocator', () => {
   const record = granted();
   assert.equal(decideAllocationAction(record, 'continue').kind, 'publish');
-  const published = applyAllocationTransition(record, { kind: 'binding-published' }, NOW + 3);
+  const publishPending = applyAllocationTransition(
+    record,
+    { kind: 'binding-publish-pending' },
+    NOW + 2,
+  );
+  assert.equal(publishPending.status, 'applied');
+  assert.equal(decideAllocationAction(publishPending.record, 'continue').kind, 'blocked');
+  assert.equal(decideAllocationAction(publishPending.record, 'recover').kind, 'cleanup');
+  const published = applyAllocationTransition(
+    publishPending.record,
+    { kind: 'binding-published' },
+    NOW + 3,
+  );
   assert.equal(published.status, 'applied');
   const cleanup = decideAllocationAction(published.record, 'release');
   assert.equal(cleanup.kind, 'cleanup');

src/daemon/__tests__/allocation-operation-journal.test.ts

@@ -1,7 +1,7 @@
 import assert from 'node:assert/strict';
 import fs from 'node:fs';
 import path from 'node:path';
-import { test } from 'vitest';
+import { test, vi } from 'vitest';
 import type {
   LeaseRequestInput,
   ManagedDeviceAllocatorPort,
@@ -14,7 +14,10 @@ import {
   createAllocationOperationJournal,
   type AllocationBindingHooks,
 } from '../allocation-operation-journal.ts';
-import { createAllocationOperationStore } from '../allocation-operation-store.ts';
+import {
+  createAllocationOperationStore,
+  type AllocationOperationStore,
+} from '../allocation-operation-store.ts';
 import {
   ALLOCATION_GRANTED_STATUS,
   ALLOCATION_PENDING_STATUS,
@@ -76,7 +79,7 @@ function bindingHooks(
         const read = journal.read(binding.operation);
         assert.equal(read.status, 'found');
         assert.equal(read.record.phase.status, 'granted');
-        assert.equal(read.record.binding, 'unpublished');
+        assert.equal(read.record.binding, 'publish-pending');
       }
       published.push(binding.lease.id);
     },
@@ -108,7 +111,7 @@ test('persists intent before request and the allocator outcome before publishing
       const read = store.read(binding.operation);
       assert.equal(read.status, 'found');
       assert.equal(read.record.phase.status, 'granted');
-      assert.equal(read.record.binding, 'unpublished');
+      assert.equal(read.record.binding, 'publish-pending');
     },
     async cleanup() {},
   };
@@ -128,6 +131,66 @@ test('persists intent before request and the allocator outcome before publishing
   );
 });
 
+test('cleans a binding after publish succeeds but its durable publication state is lost', async () => {
+  const root = mkdtempForTestSync('allocation-operation-publish-recovery-');
+  const baseStore = createAllocationOperationStore({
+    allocationsDir: path.join(root, 'allocations'),
+  });
+  let failPublicationStateWrite = true;
+  const store: AllocationOperationStore = Object.freeze({
+    ...baseStore,
+    async transition(ref, expectedFence, transitionInput, nowMs) {
+      if (failPublicationStateWrite && transitionInput.kind === 'binding-published') {
+        failPublicationStateWrite = false;
+        throw new Error('binding publication state write lost');
+      }
+      return baseStore.transition(ref, expectedFence, transitionInput, nowMs);
+    },
+  });
+  const events: string[] = [];
+  const hooks: AllocationBindingHooks = {
+    async publish() {
+      events.push('publish');
+    },
+    async cleanup() {
+      events.push('cleanup');
+    },
+  };
+  const scriptedAllocator = createScriptedManagedDeviceAllocator({
+    instanceId: 'allocator-1',
+    script: { requestLease: [ALLOCATION_GRANTED_STATUS], releaseLease: [undefined] },
+  });
+  const allocator: ManagedDeviceAllocatorPort = {
+    ...scriptedAllocator,
+    async releaseLease(input) {
+      events.push('release');
+      return scriptedAllocator.releaseLease(input);
+    },
+  };
+  const journal = createAllocationOperationJournal({
+    store,
+    allocator,
+    binding: hooks,
+    now: () => NOW,
+  });
+
+  const publishResult = await journal.allocate(ALLOCATION_REQUEST);
+  assert.equal(publishResult.status, 'blocked');
+  assert.equal(
+    publishResult.status === 'blocked' ? publishResult.reason : undefined,
+    'persistence-failed',
+  );
+  assert.equal(foundRecord(journal).binding, 'publish-pending');
+
+  const released = await journal.release(ALLOCATION_REQUEST);
+  assert.equal(released.status, 'released');
+  assert.deepEqual(events, ['publish', 'cleanup', 'release']);
+  assert.deepEqual(
+    scriptedAllocator.calls.map((call) => call.method),
+    ['requestLease', 'releaseLease'],
+  );
+});
+
 test('reconciles a lost response by lookup after reconstructing the journal', async () => {
   const first = setup({ requestLease: [new Error('connection closed')] });
   const uncertain = await first.journal.allocate(ALLOCATION_REQUEST);
@@ -398,6 +461,31 @@ test('corrupt state is retained as unreadable evidence and never implies an allo
   assert.equal(first.allocator.calls.length, 0);
 });
 
+test('root journal enumeration failure blocks a new allocator attempt', async () => {
+  const setupResult = setup({
+    requestLease: [new Error('response lost'), ALLOCATION_GRANTED_STATUS],
+  });
+  assert.equal((await setupResult.journal.allocate(ALLOCATION_REQUEST)).status, 'uncertain');
+
+  const readdir = vi.spyOn(fs, 'readdirSync').mockImplementationOnce(() => {
+    throw Object.assign(new Error('allocation journal root is unreadable'), { code: 'EACCES' });
+  });
+
+  try {
+    const result = await setupResult.journal.allocate(
+      input({ attemptKey: 'attempt-2', requestGeneration: 2 }),
+    );
+    assert.equal(result.status, 'unreadable');
+    assert.equal(result.status === 'unreadable' ? result.reason : undefined, 'corrupt');
+    assert.deepEqual(
+      setupResult.allocator.calls.map((call) => call.method),
+      ['requestLease'],
+    );
+  } finally {
+    readdir.mockRestore();
+  }
+});
+
 test('cleanup uncertainty blocks release until cleanup is retried, then allocator release is retryable', async () => {
   const failing = setup({ requestLease: [ALLOCATION_GRANTED_STATUS] });
   const hooks = bindingHooks(null, async () => {

src/daemon/__tests__/allocation-operation-record.test.ts

@@ -86,7 +86,12 @@ test('exactly replaying a local transition is idempotent while a stale fence is
     () => applyAllocationTransition(dispatched, { kind: 'binding-published' }, NOW + 3),
     (error: unknown) => error instanceof AppError && error.details?.reason === 'transition-invalid',
   );
-  const published = apply(granted, { kind: 'binding-published' });
+  assert.throws(
+    () => applyAllocationTransition(granted, { kind: 'binding-published' }, NOW + 3),
+    (error: unknown) => error instanceof AppError && error.details?.reason === 'transition-invalid',
+  );
+  const publishPending = apply(granted, { kind: 'binding-publish-pending' });
+  const published = apply(publishPending, { kind: 'binding-published' });
   assert.equal(
     applyAllocationTransition(published, { kind: 'binding-published' }, NOW + 4).status,
     'already-applied',
@@ -102,7 +107,8 @@ test('cleanup and allocator release are ordered and remain retryable', () => {
       identityIncarnationId: 'incarnation-1',
     },
   });
-  const published = apply(granted, { kind: 'binding-published' });
+  const publishPending = apply(granted, { kind: 'binding-publish-pending' });
+  const published = apply(publishPending, { kind: 'binding-published' });
   const pendingCleanup = apply(published, {
     kind: 'binding-cleanup-pending',
     message: 'binding teardown was not confirmed',

src/daemon/__tests__/allocation-operation-store.test.ts

@@ -1,7 +1,7 @@
 import assert from 'node:assert/strict';
 import fs from 'node:fs';
 import path from 'node:path';
-import { test } from 'vitest';
+import { test, vi } from 'vitest';
 import { acquireProcessLock } from '@agent-device/host-kit/file';
 import { readCurrentOwnerIdentity } from '@agent-device/host-kit/process';
 import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
@@ -156,6 +156,28 @@ test('does not follow a symbolic-link lane directory', () => {
   assert.deepEqual(fs.readdirSync(outsideLane), []);
 });
 
+test('retains an unreadable lane enumeration instead of omitting its operations', () => {
+  const { store, record } = fixture();
+  assert.equal(store.create(record).status, 'created');
+  const lanePath = path.dirname(store.resolvePath(record));
+  const originalReaddirSync = fs.readdirSync;
+  const readdir = vi.spyOn(fs, 'readdirSync').mockImplementation((directory, options) => {
+    if (directory.toString() === lanePath) {
+      throw Object.assign(new Error('allocation lane is unreadable'), { code: 'EACCES' });
+    }
+    return originalReaddirSync(directory, options);
+  });
+
+  try {
+    const listed = store.list();
+    assert.equal(listed.length, 1);
+    assert.equal(listed[0]?.status, 'unreadable');
+    assert.equal(listed[0]?.status === 'unreadable' ? listed[0].reason : undefined, 'corrupt');
+  } finally {
+    readdir.mockRestore();
+  }
+});
+
 test('a terminal allocator outcome is retained rather than deleted', async () => {
   const { store, record } = fixture();
   store.create(record);

src/daemon/allocation-operation-decision.ts

@@ -92,7 +92,9 @@ function decideGrantedCleanup(
   mode: AllocationDecisionMode,
   binding: AllocationBinding,
 ): AllocationAction | undefined {
-  const recoveryCleanup = mode === 'recover' && record.binding === 'cleanup-pending';
+  const recoveryCleanup =
+    mode === 'recover' &&
+    (record.binding === 'publish-pending' || record.binding === 'cleanup-pending');
   if ((mode !== 'release' && !recoveryCleanup) || record.binding === 'cleaned') return undefined;
   return record.release === 'not-requested' ? { kind: 'cleanup', binding } : undefined;
 }
@@ -111,6 +113,12 @@ function decideGrantedBinding(
 ): AllocationAction {
   if (record.binding === 'unpublished') return { kind: 'publish', binding };
   if (record.binding === 'published') return { kind: 'terminal' };
+  if (record.binding === 'publish-pending') {
+    return blocked(
+      'not-releasable',
+      'allocation binding publication is uncertain and requires explicit release recovery',
+    );
+  }
   if (record.binding === 'cleanup-pending') {
     return blocked('not-releasable', 'allocation binding requires explicit release recovery');
   }

src/daemon/allocation-operation-journal-action-binding.ts

@@ -21,12 +21,15 @@ export async function publishBinding(
     return blocked('binding-unavailable', 'managed binding publisher is not configured', record);
   }
   if (record.binding !== 'unpublished') return options.project(record);
+  const pending = await transition(options, record, { kind: 'binding-publish-pending' });
+  if (pending.status !== 'stored') return pending;
+  if (signal?.aborted) return abandoned(pending.record);
   try {
     await options.binding.publish(action.binding);
   } catch (error) {
-    return persistCleanupFailure(options, record, error);
+    return persistCleanupFailure(options, pending.record, error);
   }
-  const published = await transition(options, record, { kind: 'binding-published' });
+  const published = await transition(options, pending.record, { kind: 'binding-published' });
   if (published.status !== 'stored') return published;
   return signal?.aborted ? abandoned(published.record) : options.project(published.record);
 }

src/daemon/allocation-operation-record-codec-state.ts

@@ -143,6 +143,7 @@ function decodeRefusal(value: unknown): LeaseRefusal | null {
 
 export function decodeBinding(value: unknown): AllocationOperationRecord['binding'] | null {
   return value === 'unpublished' ||
+    value === 'publish-pending' ||
     value === 'published' ||
     value === 'cleanup-pending' ||
     value === 'cleaned' ||

src/daemon/allocation-operation-record-transition.ts

@@ -61,13 +61,18 @@ function isAllocatorUncertaintyTransition(
   return transition.kind === 'allocator-unknown' || transition.kind === 'allocator-ambiguous';
 }
 
-function isBindingTransition(
-  transition: ResolvedAllocationTransition,
-): transition is Extract<
+function isBindingTransition(transition: ResolvedAllocationTransition): transition is Extract<
   ResolvedAllocationTransition,
-  { kind: 'binding-published' | 'binding-cleanup-pending' | 'binding-cleaned' }
+  {
+    kind:
+      | 'binding-publish-pending'
+      | 'binding-published'
+      | 'binding-cleanup-pending'
+      | 'binding-cleaned';
+  }
 > {
   return (
+    transition.kind === 'binding-publish-pending' ||
     transition.kind === 'binding-published' ||
     transition.kind === 'binding-cleanup-pending' ||
     transition.kind === 'binding-cleaned'
@@ -116,22 +121,42 @@ function applyBindingTransition(
   record: AllocationOperationRecord,
   transition: Extract<
     ResolvedAllocationTransition,
-    { kind: 'binding-published' | 'binding-cleanup-pending' | 'binding-cleaned' }
+    {
+      kind:
+        | 'binding-publish-pending'
+        | 'binding-published'
+        | 'binding-cleanup-pending'
+        | 'binding-cleaned';
+    }
   >,
   nowMs: number,
 ): AllocationTransitionResult {
   if (record.phase.status !== 'granted') return terminalOrInvalid(record, transition.kind);
+  if (transition.kind === 'binding-publish-pending') {
+    return applyPublishPendingBinding(record, nowMs);
+  }
   if (transition.kind === 'binding-published') return applyPublishedBinding(record, nowMs);
   if (transition.kind === 'binding-cleaned') return applyCleanedBinding(record, nowMs);
   return applyCleanupPendingBinding(record, nowMs);
 }
 
+function applyPublishPendingBinding(
+  record: AllocationOperationRecord,
+  nowMs: number,
+): AllocationTransitionResult {
+  if (record.binding === 'publish-pending') return alreadyApplied(record);
+  if (record.binding !== 'unpublished' || record.release !== 'not-requested') {
+    return transitionInvalid('binding-publish-pending', record.binding);
+  }
+  return applied(record, { binding: 'publish-pending' }, nowMs);
+}
+
 function applyPublishedBinding(
   record: AllocationOperationRecord,
   nowMs: number,
 ): AllocationTransitionResult {
   if (record.binding === 'published') return alreadyApplied(record);
-  if (record.binding !== 'unpublished')
+  if (record.binding !== 'publish-pending')
     return transitionInvalid('binding-published', record.binding);
   return applied(record, { binding: 'published' }, nowMs);
 }
@@ -146,6 +171,9 @@ function applyCleanupPendingBinding(
   if (record.binding === 'cleaned' || record.release !== 'not-requested') {
     return transitionInvalid('binding-cleanup-pending', record.binding);
   }
+  if (record.binding !== 'publish-pending' && record.binding !== 'published') {
+    return transitionInvalid('binding-cleanup-pending', record.binding);
+  }
   return applied(record, { binding: 'cleanup-pending' }, nowMs);
 }
 
@@ -154,7 +182,9 @@ function applyCleanedBinding(
   nowMs: number,
 ): AllocationTransitionResult {
   if (record.binding === 'cleaned') return alreadyApplied(record);
-  if (!['unpublished', 'published', 'cleanup-pending'].includes(record.binding)) {
+  if (
+    !['unpublished', 'publish-pending', 'published', 'cleanup-pending'].includes(record.binding)
+  ) {
     return transitionInvalid('binding-cleaned', record.binding);
   }
   return applied(record, { binding: 'cleaned' }, nowMs);

src/daemon/allocation-operation-record.ts

@@ -20,6 +20,7 @@ export type AllocationOperationRef = Readonly<{
 
 export type AllocationOperationBinding =
   | 'unpublished'
+  | 'publish-pending'
   | 'published'
   | 'cleanup-pending'
   | 'cleaned'
@@ -83,6 +84,7 @@ export type AllocationTransition =
   | Readonly<{ kind: 'allocator-status'; status: LeaseRequestStatus }>
   | Readonly<{ kind: 'allocator-unknown'; message: string }>
   | Readonly<{ kind: 'allocator-ambiguous'; message: string }>
+  | Readonly<{ kind: 'binding-publish-pending' }>
   | Readonly<{ kind: 'binding-published' }>
   | Readonly<{ kind: 'binding-cleanup-pending'; message?: string }>
   | Readonly<{ kind: 'binding-cleaned' }>