## Code Review — Remozio §19 rev 36 changed clauses Reviewed lines 1674, 1676, 1678, 1692, 1702 against the five focus areas. Findings below, ordered by severity. --- ### Finding 1 — Teardown/response race: "first durable outcome wins" has no mandated serialization mechanism (lines 1676, 1678) Line 1678: *"At teardown, close an install as unknown only if it is still open; an earlier acknowledged response remains acknowledged."* Line 1676: *"The first durable terminal outcome wins."* The close is specified as check-then-act ("only if it is still open"), and there are at least two writers of the Mac-resident durable state: the response-processing path (line 1676: *"A completed, authenticated ADB install response closes that operation..."*) and the teardown/startup orphan-close path. Line 1676's atomicity requirement binds only the *transition + its audit event* as a pair ("Persist each terminal transition and its metadata-only audit event atomically") — it does not require the competing closes themselves to be mutually exclusive (single-writer, compare-and-set, or conditional update). Without that, both writers can observe "open" and durably write different terminal outcomes, and the subsequent audit replay (which replays "from durable terminal state") could reconstruct an audit event for the loser. The acceptance test in line 1702 (*"teardown racing an acknowledged response"*) is not deterministically passable until the clause mandates an atomic conditional close. **Fix direction:** require the terminal close to be a single atomic compare-and-set on the operation record ("close as X iff state == open"), with the loser forbidden from writing. ### Finding 2 — Evidence retention expiry contradicts audit-replay and later-verification obligations (lines 1676 vs. 1678) Line 1676 caps retention: *"after expiry, keep the durable unknown status and package/operation identifiers."* Two conflicts: - **Replay vs. expiry:** the audit-replay fallback ("replay the missing audit event from durable terminal state before admission") is lazy — it only executes when a later install is admitted. If detailed evidence (expected signing identity, version, artifact digest, timestamps) expires *before* any subsequent admission, the durable remainder (status + identifiers only) is insufficient to reconstruct the metadata-only audit event, voiding the "atomically **or** replay" guarantee precisely in the crash case it exists for. - **Line 1678's unconditional comparison:** *"before each later install to the same package... independently read the currently installed package metadata"* and compare against *"the requested artifact"* — impossible once the expected digest/version has been evicted. Line 1678's only degradation path is *"If package verification is unavailable, say so,"* which addresses platform unavailability, not self-inflicted expiry. The two clauses never reconcile the post-expiry side-by-side display. **Fix direction:** pin the fields required for audit replay and for the line-1678 comparison (expected version, signing identity, digest) to the lifetime of the durable terminal status, or explicitly specify degraded behavior when they are gone. ### Finding 3 — Admission is not serialized; the orphan-close gate is itself a check-then-act race (line 1676) *"On command/service startup, first close any orphaned open install operation as unknown before another install can start."* Nothing grants mutual exclusion between concurrently starting command/service instances on the same Mac: two processes can both observe no orphan (or both close the same orphan) and both admit installs, producing two open operations. Relatedly, the clause gates only *startup*; it does not state that a new install is refused while a non-orphaned operation is still open within a running session. The "one open operation" invariant the rest of the design assumes (first-outcome-wins, side-by-side display) is therefore unenforced at admission. **Fix direction:** require an admission lock or a single-owner service, and explicitly refuse (or queue) installs while any operation is open. ### Finding 4 — Audit replay is lazy and does not cover the start event (lines 1676, 1702) Two gaps in *"replay the missing audit event from durable terminal state before admission"*: - **Laziness:** if no further install is ever admitted, the audit log permanently lacks the terminal event. Audit completeness becomes conditional on future user activity; any audit review conducted before the next admission silently shows a gap. (Compounds with Finding 2's expiry problem.) - **Scope:** replay is keyed on "durable **terminal** state" and replays "the missing audit event" (the terminal one). A crash after dispatch but before the *start* audit event is persisted leaves a missing start event with no replay path — yet line 1702 requires verifying *"metadata-only start/stop audit."* The open-operation store is not required to be atomic with its start audit event, only terminal transitions are. **Fix direction:** persist the open record and start audit event atomically (or extend replay to synthesize both from the durable record), and replay on service startup/recovery rather than only at next admission. ### Finding 5 — Unauthenticated or malformed install response is unclassified (line 1676) Line 1676 closes an operation on *"a completed, authenticated ADB install response"* (acknowledged success/failure) or if the response is *"lost, interrupted, or times out"* (unknown). A response that arrives but **fails authentication or fails to parse** matches neither branch. Given the hostile-adjacent scenarios line 1702 anticipates (racing local processes, same-user key holders), this gap should be closed: an unauthenticated/malformed response must be treated as unknown (or explicitly defined otherwise). ### Finding 6 — Minor: retention clause preserves only *unknown* statuses (line 1676) *"after expiry, keep the durable unknown status..."* — acknowledged success/failure terminal states appear fully evictable under the bounded count/period. After eviction, "the first durable terminal outcome wins" has no durable record for those operations, and startup logic cannot distinguish "no prior operation" from "evicted acknowledged operation." Likely benign, but the clause should state that terminal status (of any kind) plus identifiers is the non-evictable floor, not unknown-status alone. --- ### Areas reviewed with no substantive flaw found - **Terminal status after Mac crash (core rule):** storing the open operation durably *before* dispatch, closing orphans as unknown at startup, and refusing to infer success from dropped socket or later package state (1674, 1676) is sound and conservative — including the case where `pm install` completes phone-side after the Mac dies. - **Never-reclassify rule** (1676/1678) and the refusal to treat version/signing match as artifact-equality proof absent a trustworthy platform digest (1678) are correctly stated. - **Session-admission policy deferral** (1692) is an explicit, testable open decision wired consistently into 1678 and 1702, not a contradiction. - **Self-update teardown** reporting unknown (1674, 1678) is conservative-by-design; losing an in-flight success response when the bridge-hosting app dies is acceptable under the stated policy once Finding 1's atomic close is mandated. **Summary:** six findings — the teardown/response race lacks a mandated atomic conditional close (F1); retention expiry breaks both the audit-replay fallback and the mandatory pre-install comparison (F2); admission lacks serialization (F3); audit replay is lazy and start-event-blind (F4); unauthenticated responses are unclassified (F5); and the retention floor is defined only for unknown statuses (F6).