[leonardoxr]

Closes #1112

Status

Implement: done Review: fix-check passed (round 1, round 2, fix-check), 5 fixed, 3 decisions taken Hardware: n/a Gate: running

• The Tests lines pass. — CI on the reviewed head, plus src/tunnel/worker.test.ts, src/daemon/tunnel-lease.e2e.test.ts and src/drivers/android/index.test.ts run locally in the fix runs
• tunnelModule is in the registry, off by default; simlock status shows the tunnel capability off until enabled. — module registry test and the status capability test, checked by the spec review
• The configuration reference lists capabilities.tunnel.enabled; the errors reference lists the nine codes; the configuration reference says the worker reserves loopback ports 47000-47999 for tunnels; the remote-agents page describes the tunnel socket on a lone worker, says other leases' devices on the same worker can reach a tunnelled port while it is open, and says a tunnel rule a crash leaves on a phone stays until the phone reboots; ARCHITECTURE has the module's row. — configuration.mdx, errors.mdx, remote-agents.mdx and ARCHITECTURE.md, checked by the claims review

Assumptions

• Assumption: a device connection past 256 open on a socket — closed at once with no frame, because a reverse connection is opened by the worker and a RESET would name an id the caller never saw.
• Assumption: wording of the BAD_REQUEST messages — worded by the module (a frame came before HELLO, HELLO came twice, unknown frame tag <n>, DATA is past the granted window, connection <n> was never opened, the window is above 2147483648).
• Assumption: start sweep on leased devices — reads their rules and reserves the ports instead of removing them, to keep safety rule 2; reclaim removes them.

Review

Spec review: 0 blocking, 0 fixed, 1 notes. Code review: 3 blocking, 3 fixed, 0 notes. Claims review: 2 blocking, 2 fixed, 0 notes.
Rounds: 2. Tokens: 31.5M.
Runs: 14. Cost USD: 9.2006. Agent seconds: 3630.

Rejected:

• spec: src/core/AGENTS.md:35 omits the new core files and DriverCatalog methods — incomplete, not false [nonblocking]
• code: src/drivers/android/device-tcp.ts:148-149 the start sweep only reserves ports on a leased device — follows the Tests line and safety rule 2 [intentional]

Assessment: complete

Written by an agent.

[leonardoxr]

@coderabbitai review

Written by an agent.

[leonardoxr]

Review ledger

Round 1: ef0194e
Round 2: 181af2d
Round fix-check: 42f31b5

Runs: 14. Cost USD: 9.2006. Agent seconds: 3630.
Tokens input 596, output 178035, cache read 17984039, cache write 1080289.
- fix implementer: claude-opus-5-5 834 s $1.9721
- review 1 reviewer: claude-sonnet-5-5 609 s $0.4403
- review 1 reviewer spec: claude-sonnet-5-5 107 s $0.515
- review 1 reviewer both: claude-opus-5-5 442 s $2.1195
- review 1 reviewer claims: claude-sonnet-5-5 73 s $0.3357
- fix 1 implementer: claude-sonnet-5-5 374 s $0.8522
- review 2 reviewer: claude-sonnet-5-5 444 s $0.4453
- review 2 code both: claude-opus-5-5 330 s $1.3248
- review 2 spec: claude-sonnet-5-5 18 s $0.1264
- review 2 claims: claude-sonnet-5-5 29 s $0.1427
- fix 2 implementer: claude-sonnet-5-5 68 s $0.2033
- review fix-check reviewer: claude-sonnet-5-5 178 s $0.2117
- review fix-check code behaviour: claude-opus-5-5 108 s $0.4234
- review fix-check claims: claude-sonnet-5-5 16 s $0.0882
Finding outcomes: {"duplicate":1,"fixed":5,"intentional":1,"nonblocking":1}. Classes: {"cleanup deferred to a reclaim that skips phones":1,"comment says reclaim removed rules; phone reclaim removes none":1,"doc says reclaim removes rules; phones excluded":1,"doc says refused with codes never sent":1,"documentation 7":1,"hook returns before in-flight rule removal":1,"spec vs rule":1,"testing 10":1}.

{"findings":[{"id":"R1","role":"code","class":"hook returns before in-flight rule removal","outcome":"fixed","path":"src/tunnel/sessions.ts","round":"1","fix":"22f1d60d0"},{"id":"R2","role":"code","class":"testing 10","outcome":"fixed","path":"src/daemon/tunnel-lease.e2e.test.ts","round":"1","fix":"5ee15c2a6"},{"id":"R3","role":"claims","class":"doc says refused with codes never sent","outcome":"fixed","path":"website/src/content/docs/docs/reference/errors.mdx","round":"1","fix":"181af2d5c"},{"id":"R4","role":"spec","class":"documentation 7","outcome":"nonblocking","path":"src/core/AGENTS.md","round":"1"},{"id":"R5","role":"code","class":"spec vs rule","outcome":"intentional","path":"src/drivers/android/device-tcp.ts","round":"1"},{"id":"R6","role":"code","class":"cleanup deferred to a reclaim that skips phones","outcome":"fixed","path":"src/tunnel/sessions.ts","round":"2","fix":"42f31b54b"},{"id":"R6c","role":"claims","class":"doc says reclaim removes rules; phones excluded","outcome":"duplicate","canonicalId":"R6","path":"src/tunnel/AGENTS.md","round":"2"},{"id":"R7","role":"claims","class":"comment says reclaim removed rules; phone reclaim removes none","outcome":"fixed","path":"src/drivers/android/index.ts","round":"2","fix":"42f31b54b"}]}

Written by an agent.

[V3RON]

Decision taken

The ADR review (round 1, R1 and R2) found two questions that the spec left open. Both are decided here, pending the maintainer. #1112's body now says the same, with Tests lines that pin each one.

R2: tunnel rules on enrolled phones. Reclaim and the start sweep ran reverse --remove-all and removed forward rules on phones, through the host's shared adb server. Safety rule 1 and ADR 0022 §3 allow only one destructive act on a physical device.

• (a, taken) Reclaim and the start sweep act on emulators only. On a phone, Simlock removes only the reverse rules its own tunnels set, when their socket closes. What a user sees: after a daemon crash, a tunnel rule can stay on a phone until it reboots, and the docs say so.
• (b) Keep the phone cleanup and amend safety rule 1 and ADR 0022 to allow removing adb rules. What a user sees: no stale rules on phones, but Simlock now changes phone state it didn't make.

Why (a): it keeps the safety rule as it is, it is the smaller change, and it is easy to widen later. Reverting means option (b): a rule amendment, plus putting the phone branch back in device-tcp.ts.

R1: the loopback listener accepts any local connection. While a tunnel is open, another lease's device on the same worker can reach the holder's tunnelled port: an emulator through 10.0.2.2, or any device through an adb reverse set with device.exec. A local process can reach it too.

• (a, taken) Add no check. The ADR's Consequences and the remote-agents page state the exposure. The module stays off by default, and an operator opts in.
• (b) Listen on a Unix socket (adb reverse tcp:<d> localfilesystem:<path>, mode 0600), and refuse device.exec reverse rules that point there. What a user sees: no exposure across leases, at the cost of more code in The tunnel module: reverse a leased Android device's port over a lease WebSocket on a worker #1112.

Why (a): it is what the code does today, and the exposure is opt-in. Reverting means option (b), which would be a follow-up task under #1110.

Written by an agent.

[V3RON]

Decision taken

The ADR review (round 2, R5): a tunnel rule left on a phone (after a daemon crash or a failed removal) keeps pointing at a port in 47000-47999. If nothing reserves that port, a later tunnel on another lease could get it, and would then receive that phone's connections for a different holder.

• (a, taken) Keep the port reserved. At start, the worker reads reverse --list on every enrolled phone and changes nothing there. Each tunnel port that a rule points to stays reserved. A phone's reclaim reads its rules again and frees only the ports that no rule points to. What a user sees: nothing, unless every tunnel port is used up by stale phone rules, which takes 1000 of them.
• (b) Allow the reuse, and state that the operator accepts it. What a user sees: one holder's tunnel may carry another lease's phone traffic.

Why (a): it closes a path from one lease into another, and it only reads phone state, so safety rule 1 holds. Reverting means dropping the read of phone rules at start and at reclaim.

Written by an agent.

[V3RON]

Decision taken

Review round 2 (R6): a lease end whose signal is already aborted (nuke, a daemon stop) skips rule removal and leaves it to reclaim. A phone's reclaim removes no rules, so the tunnel's rule stayed on the phone, and the next holder's tunnel to that device port was refused with TUNNEL_PORT_IN_USE.

• (a, taken) On a phone, an aborted end still removes the tunnel's own rules. A phone's reclaim never touches rules, so the removal cannot race it. Emulators keep the skip, because their reclaim removes the rules. What a user sees: after nuke or a stop, the phone is clean for the next holder. A removal that fails leaves its rule, and the rule's port stays reserved (D3).
• (b) A phone's reclaim removes the rules whose host side is a tunnel port the daemon still holds. What a user sees: the same, and failed removals get retried too. But it contradicts ADR 0054 Decision 6, which says phone reclaim removes no rules.

Why (a): it keeps ADR 0054 as reviewed and removes only what Simlock set. Reverting means putting the abort skip back on phones.

Written by an agent.

[V3RON]

Decisions confirmed

The maintainer confirmed the delivery decisions on 2026-11-01:

• Phones: reclaim and the start sweep clean adb rules on emulators only. On a phone, Simlock removes only the rules its own tunnels set. Ports of stale phone rules stay reserved, and an aborted lease end on a phone still removes that phone's tunnel rules.
• Loopback listener: the exposure stays documented for now. A follow-up task under A remote caller's leased device reaches ports on the caller's machine, and back #1110 adds a Unix-socket listener.

Written by an agent.