fix(fileservice): don't hang on ListDirectory failures; don't leave zero-byte files on failed pulls (iOS 26.5.x app-group, #784)

[danielpaulus]

Problem

On iOS 26.5.x, file operations against App Group containers (--app-group) fail device-side with RemoteServices error 11007 "File paths cannot contain '..'." even though the paths contain no .. (#784). That is an Apple daemon bug — reproduced identically with Apple's own xcrun devicectl on the same devices — but go-ios failed badly around it:

• ios file ls --app-group=… hung forever with no output.
• A failed ios file pull left a zero-byte local file behind, which scripted pipelines can mistake for a successfully pulled empty file.

Root cause

• ListDirectory sends RetrieveDirectoryList and then blocks in ReceiveOnClientServerStream() with no timeout. Every other control response (CreateSession, RetrieveFile, ProposeFile) arrives on the server→client stream — when the device reports a RetrieveDirectoryList failure there instead of sending the listing, the error was silently lost and the call blocked indefinitely.
• cmd_device_files.go ran os.Create(localPath) before PullFile, so any pull failure (including the device-side 11007 rejection, which happens before any data streaming) left a truncated 0-byte destination file.

Fix

• ios/fileservice: ListDirectory now receives on both streams concurrently under a receive timeout (default 30s) and surfaces failures as typed errors: *fileservice.DeviceError (carries the device's EncodedError + LocalizedDescription) and fileservice.ErrTimeout. A pending control-stream read left by a successful listing is tracked on the Connection and consumed by the next control receive (createSession/PullFile/PushFile), so the control stream never has two concurrent readers. extractError now returns *DeviceError everywhere (same message format as before).
• cmd_device_files.go: the pull path is extracted into a testable pullToLocalFile wrapper that removes the local file when the download fails — no zero-byte/partial files remain. Success behavior (including pulling genuinely empty files) is unchanged.
• Interface seam: Connection.conn is now a minimal controlConnection interface (satisfied by *xpc.Connection) so the control protocol is unit-testable without a device. No change to the wire protocol or ios/xpc.
• Docs: ios file ls help text in main.go now notes the iOS 26.5.x App Group limitation and the --app workaround.

Options considered

1. Timeout-only on the list stream, check control stream after expiry — simplest, but the device's 11007 error would only surface after the full timeout instead of immediately, and the error could still be lost. Rejected.
2. Concurrent receive on both streams with a pending-read handoff (chosen) — surfaces the device error immediately, keeps a single reader per stream, and reduces to the old behavior on the happy path. Slightly more state (pendingControl), fully covered by tests.
3. Modify ios/xpc to support deadlines/contexts — cleaner long-term, but a much larger blast radius across all XPC consumers; out of scope for a targeted hardening.
4. For the pull: create the local file only after the control response (needs restructuring PullFile's writer contract and breaks empty-file pulls) vs. unlink on error (chosen) — minimal, and also cleans up partially-written files on mid-stream failures.

Test plan

• New device-free unit tests that hang (and fail via a 5s test deadline) without the fix:
  • ios/fileservice/fileservice_test.go against a fake XPC connection: control-stream EncodedError → typed *DeviceError, no hang; device never responds → ErrTimeout with short injected timeout; happy-path listing; pending control-read handoff to PullFile.
  • cmd_device_files_test.go: erroring puller (immediate and mid-stream) → no local file remains; successful pull writes content and returns size.
• Verified the ListDirectory tests fail against the pre-fix receive logic (both hang), and pass with the fix.
• go build ./..., go test ./..., go test -race on changed packages, gofmt -l clean.

Improves #784 — the go-ios-side hang and zero-byte-file behaviors are fixed here, but the underlying App Group 11007 rejection is an Apple iOS 26.5.x daemon bug (Apple's devicectl fails identically), so the issue stays open to track that limitation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk

[danielpaulus]

/test-devices

[github-actions]

🧪 Running real-device tests on PR #802 — run.

[github-actions]

❌ Real-device tests failed — see run.

[danielpaulus]

Adversarial review (with a second-opinion pass via codex). Verdict: the core design is sound; I pushed one follow-up commit for a real cleanup gap and dismissed the rest as false positives or documented-by-contract.

Fixed (commit 932e266)

pullToLocalFile left a partial file behind on Stat/Close failure. The wrapper documents that a failed pull never leaves a zero-byte/partial file, but it only removed the file when the pull callback errored. If pull succeeded yet Stat or Close failed, it returned an error while leaving the incomplete file on disk. Close is precisely where buffered write errors (e.g. a full disk) surface, so this was the case most likely to leave a truncated file a pipeline mistakes for success. Cleanup now runs via a deferred, error-guarded os.Remove on every post-os.Create failure path, plus a regression test for the finalize-failure path.

Reviewed and dismissed

• Goroutine leak / protocol desync when the control stream wins or on timeout. Real in the abstract, but the losing goroutine only leaks until Close(), and both ErrTimeout and the ListDirectory doc mandate closing the connection after an error/timeout. The sole caller (cmd_device_files.go) creates a fresh connection per CLI invocation and always defers Close(), so the reuse-after-error desync never materializes. Not a bug given the documented contract.
• Data race on pendingControl / conn. No race: all pendingControl access is under c.mu; conn is set once in New and never reassigned (only Close()d). go test -race ./ios/fileservice/ is clean.
• TOCTOU on os.Remove / truncating a pre-existing destination. The "another process swaps the path between Close and Remove" scenario is not a realistic threat for a user-specified CLI download target, and os.Create truncating an existing destination matches the prior behavior on main and every download tool. No change warranted.

Test status

go build ./..., go test ./..., and go test -race ./ios/fileservice/ all pass; gofmt -l clean.

[danielpaulus]

Thanks for tackling the #784 fileservice hang! Heads-up from a CI triage pass: the real-device e2e is failing here for a reason that looks like it needs another look at this change, not an infra flake.

TestFileLsCrash and TestFileLsTemp fail identically on both the macOS and Linux runners (two different healthy iOS 17 devices — the file-roundtrip push/pull subtest passes on both, so the devices are fine), with:

ListDirectory: no response within 30s: timed out waiting for a response from the device

That 30s timeout is the new code path this PR introduces in ios/fileservice/fileservice.go — the concurrent list-stream + control-stream reader racing the receiveTimeout. It looks like the normal (success) path of ListDirectory now regresses: neither the list channel nor the control channel fires, so it always hits the timeout branch. The most likely culprit is the concurrent control-stream reader (or a cached pendingControl read) consuming/stealing the directory-list response the main read is waiting for.

So the fix for the hang-on-failure case seems to have introduced a hang on the happy path. Could you take another look at the ListDirectory rewrite so a normal successful listing still returns? Happy to help verify on the device farm once updated. 🙏