Skip to content

fix: resolve hardware verification findings B1, B2, D1, D2, D3 - #91

Closed
V3RON wants to merge 2 commits into
feat/owned-device-roots-5-doctorfrom
claude/test-failures-bug-fixes-5a9ffd
Closed

V3RON wants to merge 2 commits into
feat/owned-device-roots-5-doctorfrom
claude/test-failures-bug-fixes-5a9ffd

Conversation

@V3RON

@V3RON V3RON commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Stacked on #84. Fixes the defects found by the Phase 1-4 hardware verification run against feat/owned-device-roots-5-doctor.

B1 — daemon stop left the process alive

Two handles kept the event loop open after "Daemon stopped":

  • The Android emulator was spawned with piped stdio and never unref'd, so the daemon lived as long as any emulator did. It is now spawned with stdio: "ignore" and unref(), the same treatment the adb server gets: nothing read those pipes, and the emulator is designed to outlive the daemon (a restart re-attaches it).
  • Detached-lease TTL timers were never cancelled on shutdown. LeaseEngine.dispose() now disposes the expiry scheduler too; the leases stay in the registry and their timers are restored on the next start.

B2 — rejected root + interrupted reclaim crashed the whole daemon

Startup convergence assumed every platform in the registry had a driver; the resulting NoDriverError was fatal and took the healthy platform down. StartupConverger now takes a driver-availability port and skips devices of a platform without a driver for both interrupted-reclaim recovery and excess-capacity shutdown. Those devices stay as found (the documented "dark platform keeps its inventory" behaviour) and doctor still reports the rejection.

D1 — socket path length

resolveDaemonSocketPath validates daemon.sock against the kernel's sun_path limit (104 bytes on macOS, 108 on Linux) and throws a message naming SIMLOCK_HOME as the fix. CLI, MCP server, and daemon all derive the path through it; the CLI reports it as USAGE. Documented in CLI.md.

D2 — Android boot estimate

Cold-boot estimate raised from 31s to the measured 70s (measurement recorded in the comment). Affects the doctor stall threshold and ETA reporting only.

D3 — stale address on quarantined devices

transition() drops address on entry to shutdown or quarantined, since nothing runs there and Android reuses the console port. The next makeReady supplies a fresh one.

Not changed

  • D4 (recovery-budget exhaustion time) is bounded by the driver readiness timeout per attempt, by design; a shorter recovery timeout would be a product decision.
  • D5 was a test-harness sed mismatch.

Verification

  • pnpm run typecheck, typecheck:e2e, lint, format:check, fallow: clean
  • pnpm run test: 960 passed (new tests for the converger skip, expiry-timer disposal, address clearing, socket-path check)
  • pnpm run test:e2e (fake driver): 40 passed, 1 expected fail
  • The slow real-hardware lane was not run; slow-android-smoke.test.ts is the test that exercises B1 for real.

claude and others added 2 commits September 2, 2026 11:30
A device inside a validly-marked root with no registry record is an
orphan: almost always a daemon that died between creating a device and
writing it down, and until now permanently unreclaimable, because
registry-only destruction cannot reach what the registry has never heard
of. `doctor --purge-orphans` is the one opt-in exception to that rule
(safety rule 1), and everything about how it is wired is meant to keep it
exceptional: its own flag rather than part of `--fix`, so a `doctor
--fix` already running unattended does not acquire a destructive
behaviour on upgrade; a confirmation unless `--yes`; and no reachability
from the reaper, a cleanup rule, or an idle tier.

The reason it must stay opt-in is structural rather than stylistic. Every
central safety filter is written over registry records, and an orphan has
none, so an orphan proposal bypasses the safety net rather than passing
through it -- including, in the provision-then-register window, a device
this very daemon is about to write down.

Ownership is proven once, at startup, and then trusted for the life of
the process. That is fine for reporting and not fine for destroying: a
daemon up for days is one `mv` away from a root that now holds the user's
own simulators. So every root a purge would touch is re-proven first, and
a refusal aborts the whole run rather than one platform.

Devices left in the pre-root locations are reported as `legacy-device`
rather than as vanished, and `--fix` destroys them through the old path
they were recorded with. That is the one destruction that reaches outside
an owned root, and it is permitted because the device is in the registry:
registry-only destruction is satisfied by the record, not by the root.

The docs stop describing a future. ADR 0001 is Accepted, its three events
are implemented, and the pitfalls that said "planned" now say what the
code does -- including a new one for the startup-proof lifetime above,
which the existing accident-boundary entry did not cover.

Refs: #73, #70

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X
- daemon stop: spawn the emulator detached (ignored stdio, unref) and cancel
  lease expiry timers on dispose, so the process exits once "Daemon stopped"
- startup: skip devices of a platform whose driver was refused instead of
  failing convergence with NoDriverError and taking the healthy platform down
- socket path: validate daemon.sock length against the kernel limit up front
  and name SIMLOCK_HOME as the fix
- android: raise the cold-boot estimate to the measured 70s
- registry: drop a device's address on shutdown/quarantine
@V3RON
V3RON force-pushed the feat/owned-device-roots-5-doctor branch from b55240d to bcdefd6 Compare September 4, 2026 05:49
V3RON added a commit that referenced this pull request Sep 5, 2026
…01) (#111)

Implements ADR 0001. Ownership stops being a guess about a device's name and
becomes a fact about its location.

- A device root is proven, not assumed. `ensureOwnedRoot` creates a root only
  when it creates it empty itself, and refuses one that is unmarked, marked for
  another instance, symlinked, or wrongly owned or permissioned. The root is
  assembled in a staging sibling and published with a single `rename`, so
  creation is atomic with its marker.
- Both drivers answer `listManaged` from root membership. A `simlock-` name
  prefix survives only as a cosmetic label; it is no longer evidence of
  ownership, so an identically-named user device is never adopted.
- A refused root costs that platform and nothing else -- never a fallback to
  the default device location; reported as `driver.root-rejected` at startup
  and as a `doctor` finding on every run after.
- A grant carries a driver-built `environment` the core never reads,
  `--export-env` prints it for `eval`, and the scoped `simlock simctl` /
  `simlock adb` wrappers let a lease holder reach a contained device without
  handing back the capability containment removed.
- `doctor --purge-orphans` is the single opt-in exception to registry-only
  destruction, behind its own flag and a confirmation, with every root
  re-proven before anything is destroyed.

Integrates the five reviewed PRs #80-#84 and resolves them against ADR 0003's
typed-contract daemon. Two ADR corrections were made during implementation:
#82 amends decision 4 (`ADB_EMU=0` is what actually contains adb's emulator
scan; raising `ADB_LOCAL_TRANSPORT_MAX_PORT` alone widens a sweep that always
starts at 5555), and #84 amends `docs/ARCHITECTURE.md`, which claimed Simlock
cannot address anything outside a root -- `destroyLegacy` breaks that by
design, per the ADR's migration paragraph.

Known gap: the hardware-verification fixes in #91 are not included. B1 landed
independently via #90, but B2 (a rejected root plus interrupted-reclaim state
crashes the daemon at startup), D1 (no socket-path length check), D2 (Android
boot estimate too low) and D3 (stale address on quarantined devices) are still
outstanding and land next. The slow real-hardware lanes have not been run.

Refs #73, #70
@V3RON

V3RON commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favour of #113, which restacks this work onto main.

This PR could not reach main as it stood: its base is feat/owned-device-roots-5-doctor, and the stack was squash-merged, so that branch is not an ancestor of anything on main. Merging here would have updated a dead branch without moving the fixes anywhere, and retargeting the base would have carried a duplicate of #84's already-landed commit.

One correction while restacking: B1 is already fixed on main, having landed independently as #90 with more thorough coverage than the version here. #113 therefore carries B2, D1, D2 and D3 only, and drops this PR's duplicate B1 test (whose LeaseRequestOptions signature had also gone stale).

The conflicts against current main were resolved rather than replayed — defaultCliEnvironment is now buildCliEnvironment, cliErrorCode routes through isSimlockError instead of DaemonClientError, and the new SIMLOCK_HOME paragraph needed to move under its own heading. #113 is green on pnpm run check and all five CI checks.

@V3RON V3RON closed this Sep 5, 2026
V3RON added a commit that referenced this pull request Sep 5, 2026
Restack of #91 onto main, which it could not reach: its base was the
squash-merged stack branch `feat/owned-device-roots-5-doctor`. B1 is not here --
it landed independently as #90 with stronger coverage -- so this carries the
four findings still missing after #111.

- **B2**: a rejected root took the whole daemon down at startup. Convergence
  called into a driver for every device it found, including devices of a
  platform refused at discovery, and the resulting `NoDriverError` aborted
  convergence and stopped the process -- Android's bad root took iOS with it,
  the inverse of ADR 0001's "a refused root costs that platform and nothing
  else". Such devices are now left alone and reported by `doctor`.
- **D1**: the kernel caps a Unix socket path at 104 bytes on macOS, 108 on
  Linux, and `daemon.sock` sits under `SIMLOCK_HOME`. A deeper home failed on
  connect with a bare `EINVAL`; it now reports one `USAGE` line naming the
  limit, in both the CLI and MCP.
- **D2**: the Android cold-boot estimate was 31s against emulators measured at
  60-71s, so `lease` quoted an ETA it could not meet and `Doctor` flagged
  healthy boots. Raised to 70s.
- **D3**: a device kept a stale `address` through `shutdown`. Dropped there,
  where `makeReady` re-supplies it on the way back.

Also carries the emulator `stdio: "ignore"` + `unref()` that is the other half
of B1's daemon-stop fix, now with a test -- argv cannot show whether a spawned
child released the event loop, and `stdio: "ignore"` alone does not.

An adversarial review of this branch found two regressions it had introduced,
fixed here before merge: D1's error never reached its handler (resolved in
`runCli`'s default parameter, which runs before the try/catch, so it escaped as
an uncaught rejection and took `simlock --help` with it), and D3's drop extended
to `quarantined`, where it was permanent -- `ReclaimResult` carries no address
for `recoverFromQuarantine` to restore, leaving a device grantable with no
serial its holder could reach.

Known limit, documented in `StartupConverger` rather than implied: a refused
platform's devices still count toward capacity, so a large dark inventory can
make the healthy platform look over budget and get its warm device evicted.
Pre-existing, and reachable now that convergence no longer crashes first;
fixing it means deciding whether an undrivable device should consume capacity
at all.

Refs #73, #70
@V3RON
V3RON deleted the claude/test-failures-bug-fixes-5a9ffd branch September 5, 2026 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants