fix(ios): respect devicectl minimum timeout for device probes

[samwize]

Summary

• Clamp the CoreDevice details CLI timeout to the accepted 5-second minimum while preserving the independent host deadline. No retry or transport-policy changes.
• Define the minimum once in IOS_DEVICECTL_MIN_TIMEOUT_SECONDS, shared by details and hinge-angle probes. Leave xcdevice timeouts unchanged.
• Three files changed: shared configuration, probe calculation and five boundary regression cases using the existing anonymized fixture.

Validation

• Commit: c2ed74a9ff3bf133d2872b7401bc854c2a7d8003.
• pnpm check:affected --run: all runnable gates passed, including 444 test files / 3,210 tests. Coverage and native CI lanes remain separate.
• Focused suites: 28 CoreDevice + 3 hinge-angle tests passed. Restoring the old clamp made the shortened CoreDevice suite fail (1 failed / 27 passed); restoring the fix passed all 28.
• Earlier direct live resolver checks at 15343c425, unchanged 1,000 ms budget: unpatched source returned no address twice (49/48 ms); patched source resolved it three times (641/456/498 ms). Not repeated for this constant-only refactor.
• Full CLI idle-gap behavior remains unverified. USB-first commands may bypass this path, so no broader runner-reliability claim is made. Fork CI requires maintainer approval; current-head CI is not claimed green.

[samwize]

Could a maintainer approve the fork workflows and review this for merging once the required checks pass?

An independent agent review of 15343c425 found no blocking code or test issues. The local affected gates passed, and the direct live CoreDevice resolver changed from failing to succeeding within the same 1,000 ms budget.

Full CLI idle-gap behavior is still unverified. USB-first commands can bypass this CoreDevice path, so this PR only claims to correct the invalid timeout argument, not to resolve every runner failure. The validation limits are recorded in the description.

[cubic-dev-ai]

No issues found across 2 files

_{Re-trigger cubic}

[thymikee]

Code-quality review (head 15343c4). The fix is right and well scoped. One structural change before merge, plus two smaller points.

1. The devicectl floor is now a duplicated fact. Name it once (blocking)

packages/platform-apple/src/core/config.ts already records this constraint:

/**
 * The smallest `--timeout` devicectl accepts. ...
 */
export const IOS_HINGE_ANGLE_STREAM_SECONDS = 5;

This PR adds a second, unnamed copy as the literal 5 in runCoreDeviceDetails. A reader of Math.max(5, …) cannot tell that the 5 is a devicectl contract and not a tuning choice. The two copies can also drift apart. Suggested change:

// config.ts
/** The smallest `--timeout` (seconds) devicectl accepts; smaller values exit 64 before any device query. */
export const IOS_DEVICECTL_MIN_TIMEOUT_SECONDS = 5;
/** The hinge-angle stream never ends on its own, so one read costs exactly the devicectl minimum ... */
export const IOS_HINGE_ANGLE_STREAM_SECONDS = IOS_DEVICECTL_MIN_TIMEOUT_SECONDS;

// physical-device-coredevice.ts
const timeoutSeconds = Math.max(IOS_DEVICECTL_MIN_TIMEOUT_SECONDS, Math.ceil(timeoutMs / 1000));

After this change, the constraint has one owner, and both devicectl --timeout call sites read from it. (xcdevice wait --timeout= in physical-device-control.ts uses a different tool, so leave it as is.)

2. Below 5 s, the CLI --timeout cannot take effect. Say so where the deadline is set

For a budget under 5 s, devicectl's own timeout can no longer fire first. The host process deadline (timeoutMs + commandTimeoutBufferMs) kills the process, so the probe gets the killed-process result and never gets a devicectl-written failure payload. That is acceptable, because the old value was rejected with exit 64 anyway. But it is a non-obvious constraint, so it needs one short line at the clamp, for example "devicectl rejects sub-minimum values; shorter budgets are enforced by the host deadline alone". The test name already states this behavior ("keeps a %i ms host budget"). The code should state it too.

3. Test table: trim to the boundary cases (nit)

The fake that copies devicectl's exit-64 validation is a good choice: it encodes the real contract, not an argv snapshot. Seven rows is more than the boundary needs. 250 (sub-second), 4999, 5000, 5001, and 10000 cover every branch of max/ceil. 1000 and 4000 add nothing. Also, if (1) lands, the fake should use the constant (Number(observedCliTimeout) < IOS_DEVICECTL_MIN_TIMEOUT_SECONDS), so that the test and the code share the fact.

No file-size, layering, or branching concerns: the change is one expression in the correct module.

[thymikee]

The probe change at 15343c4 is close, but one reuse issue needs a fix before merge. The devicectl minimum --timeout is already named in config.ts as IOS_HINGE_ANGLE_STREAM_SECONDS, documented as the smallest value devicectl accepts. The PR adds a second, unnamed copy as the literal 5 in Math.max(5, ...) at physical-device-coredevice.ts. Behavior is correct today. But the constraint now has two owners, and a reader of the literal cannot tell it is a devicectl contract and not a tuning choice. Could you make every devicectl --timeout call site read one named floor? Add IOS_DEVICECTL_MIN_TIMEOUT_SECONDS = 5 in config.ts, define IOS_HINGE_ANGLE_STREAM_SECONDS from it, and use it in runCoreDeviceDetails. The only other devicectl --timeout site is hinge-angle.ts, which already reads the hinge constant. Please leave the xcdevice --timeout= sites alone, since that is a different tool. Not blocking, and you can take or leave it: a one-line note at the clamp would help, saying devicectl rejects sub-minimum values and shorter budgets are enforced by the host deadline alone. The only check on this PR is green, but fork workflows still await maintainer approval, so the full matrix may not have run. The diff touches only the CoreDevice details probe and its test, so a later failure would need to overlap packages/platform-apple/src/core. I did not run the tests or reproduce the live device run, so the live-run numbers are yours and no log is attached. I also did not trace the USB-first bypass claim, and the exit 64 behavior for --timeout below 5 comes from the mocked error text, not from running devicectl. There are no conflicts. Before merge, the shared constant needs to land and the gates need a re-run.

[samwize]

Addressed in c2ed74a.

Added IOS_DEVICECTL_MIN_TIMEOUT_SECONDS and reused it through both the details probe and hinge-angle stream constant. The xcdevice timeout sites are unchanged. Added the short host-deadline note and reduced the table to the five suggested boundary cases; the fake now reads the shared floor while expected CLI values remain literal.

All runnable checks from pnpm check:affected --run passed on this head: 444 files / 3,210 tests. The focused suites passed 28 CoreDevice and 3 hinge-angle tests. Reinstating the old clamp still makes the shortened regression suite fail.

The description now records the current validation and attributes the earlier live timings to their original commit. Coverage/native CI and full CLI idle-gap verification remain separate.

[thymikee]

The delta at c2ed74a fixes the issues from the earlier review (#3085 (comment)), and I have no new findings. The change stays inside packages/platform-apple/src/core: the probe clamp, one config constant and a unit test.

CI shows 1 check with none failing. The fork matrix may not have run yet, so a maintainer needs to approve CI for the fork before merge.

I did not run the tests or the pnpm check:affected gate, so the pass counts and the old-clamp mutation result in the PR are the author's own. I did not run devicectl live either. The exit 64 behavior below 5 s comes from the mocked error text in the test, and the clamp itself is a plain numeric floor. There are no conflicts.