ahand: hub.deleteDevice compensation can delete the winner's hub registration in concurrent register race

[winrey]

Context

Surfaced while writing the persistence e2e suite for #73 (commit 93d9a02, the concurrent register collision scenario). The suite stubs AhandHubClient, so the production behaviour described below is not directly observed in tests — but the code path is reachable.

The race

AhandDevicesService.registerDeviceForUser catches Postgres UNIQUE violations (code === '23505') and compensates by calling hub.deleteDevice(input.hubDeviceId):

```ts
} catch (e) {
if ((e as { code?: string }).code === '23505') {
// Compensate hub registration since the device is already in DB
await this.hub.deleteDevice(input.hubDeviceId).catch(...);
throw new ConflictException('Device already registered');
}
...
}
```

The compensation key is the bare `hubDeviceId` — there's no per-(deviceId, externalUserId) targeting. Whether this is buggy depends on how ahand-hub keys its admin storage:

Hub keying	Behaviour	Bug?
Globally unique on `deviceId`	Loser's step 1 (`hub.registerDevice`) returns 409 → catch never reached	No
Per-(deviceId, externalUserId)	Two users can both succeed at step 1; loser's catch fires `deleteDevice(deviceId)` and also wipes the winner's hub binding	Yes

A retry path also matters: if the gateway crashed between step 1 (hub OK) and step 2 (DB OK), the retry hits step 0 → SELECT misses → step 1 → 409 from hub. So the 23505 catch is also reachable via legitimate retries when hub-DB consistency drifts.

What needs to happen

1. Confirm the hub keying. Read `ahand-hub-core` admin handler for `POST /api/admin/devices` — does it enforce UNIQUE on `deviceId` only, or on `(deviceId, externalUserId)`?
2. If globally unique: this issue is moot. Add a comment in `ahand.service.ts` documenting the invariant so future readers don't worry. Close.
3. If per-user: change the compensation strategy to one of:
   • re-SELECT the DB row, return idempotent success when owner+pubkey match (treat like step 0 idempotence catching up)
   • have the hub expose a (deviceId, externalUserId)-scoped delete so we can target only our own binding
   • drop the compensation entirely and accept orphaned hub registrations as a known leak (eventual reconciliation via hub admin)

Test coverage to add once decided

• The `concurrent register collision` scenario in `apps/server/apps/gateway/test/ahand-persistence.e2e-spec.ts` should add an assertion on the `hub.deleteDevice` mock to pin the chosen behaviour.

Acceptance

• Hub keying invariant confirmed (in code or in comment)
• Compensation strategy aligned with that invariant
• e2e suite updated to cover the chosen behaviour

Related

• Issue Phase 9 / Task 9.2 (remainder): testcontainer'd gateway e2e for ahand DB+Redis scenarios #73 (the persistence e2e that surfaced this)
• Commit 93d9a02 (the test that stubs around this code path)

[winrey]

Investigation result: Case A — issue is moot.

I cloned team9ai/aHand at 62c2f9cabde1a953ea3f13917998f5cc6efff564 and checked the three relevant artefacts:

1. Hub schema — globally unique on deviceId alone

crates/ahand-hub-store/migrations/0001_initial.sql#L2:

CREATE TABLE devices (
    id TEXT PRIMARY KEY,
    ...
);

No externalUserId column exists anywhere in the devices table. The hub enforces uniqueness on id alone.

2. POST /api/admin/devices handler

crates/ahand-hub/src/http/devices.rs, create_device, lines 60–101:

state.devices.insert(NewDevice { id: body.id.clone(), ... }).await
    .map_err(ApiError::from)?;

device_store.rs insert, lines 82–109 maps a unique violation on id → HubError::DeviceAlreadyExists.

3. Error → HTTP mapping

api_error.rs, lines 112–116:

HubError::DeviceAlreadyExists(device_id) => Self::new(
    StatusCode::CONFLICT,   // HTTP 409
    ...
)

Why the 23505 catch is unreachable in the race

In the feared concurrent-register race:

• User A calls hub.registerDevice(hubDeviceId) → hub INSERT wins → HTTP 201
• User B calls hub.registerDevice(hubDeviceId) → hub INSERT fails on PK → HTTP 409

AhandHubClient.registerDevice maps HTTP 409 → ConflictException and throws it immediately, before reaching the try block that wraps the Postgres INSERT in the gateway. User B's 23505 catch is therefore unreachable.

The only legitimate path into the 23505 catch is a crash-retry: gateway crashed after step 1 (hub OK) but before step 2 (DB OK). In that retry the hub already holds the device, the gateway retries, DB INSERT fails with 23505, and the compensation hub.deleteDevice(deviceId) fires. That is safe because deviceId is the hub's global PK — there is no other user's binding to accidentally wipe.

Action taken

PR #79 adds a comment near the 23505 catch documenting this invariant.

---

Generated by Claude Code