Conversation
…the daemon serves requests The restore now runs inside the configureForDaemonLock startup phase, before the servers open and before daemon.json is published. Its diagnostics are buffered with the other startup diagnostics and written to daemon.log after publication. The migration-specific lifecycle method and its daemon wrapper are deleted. A restore failure is reported as a diagnostic, not thrown. A simulator_set_destination_not_found error now names the Xcode version that this process's runner cache decision already read, and never probes the toolchain; without that value it says "(version unreadable)". Docs say that a symlinked XCTestDevices is not supported and is removed at daemon startup, and that the Xcode version can be unreadable. Tests pin exact failure messages per fixture and cover the real daemon startup path. Captured fixtures use placeholder home, checkout and temp paths.
|
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/platform-apple/src/runner/runner-device-set.ts">
<violation number="1" location="packages/platform-apple/src/runner/runner-device-set.ts:124">
P3: With `memoizedRunnerXcodeVersion` never probing, the `(version unreadable)` fallback now fires whenever the fingerprint hasn't been memoized in this process (adopted session, expired memo TTL), not only when the toolchain is genuinely unreadable. That message misleads a user into investigating a toolchain problem when the version was simply never read. Since the diagnostic knows the reason is 'not memoized', distinguish it (e.g. omit the Xcode clause or say 'version not read yet') instead of reporting the version as unreadable.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| const simulatorSetPath = runnerSimulatorSetPath(device); | ||
| if (simulatorSetPath === undefined) return {}; | ||
| const xcodeVersion = readRunnerXcodeVersion(device); | ||
| const xcodeVersion = memoizedRunnerXcodeVersion(device); |
There was a problem hiding this comment.
P3: With memoizedRunnerXcodeVersion never probing, the (version unreadable) fallback now fires whenever the fingerprint hasn't been memoized in this process (adopted session, expired memo TTL), not only when the toolchain is genuinely unreadable. That message misleads a user into investigating a toolchain problem when the version was simply never read. Since the diagnostic knows the reason is 'not memoized', distinguish it (e.g. omit the Xcode clause or say 'version not read yet') instead of reporting the version as unreadable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/runner/runner-device-set.ts, line 124:
<comment>With `memoizedRunnerXcodeVersion` never probing, the `(version unreadable)` fallback now fires whenever the fingerprint hasn't been memoized in this process (adopted session, expired memo TTL), not only when the toolchain is genuinely unreadable. That message misleads a user into investigating a toolchain problem when the version was simply never read. Since the diagnostic knows the reason is 'not memoized', distinguish it (e.g. omit the Xcode clause or say 'version not read yet') instead of reporting the version as unreadable.</comment>
<file context>
@@ -94,14 +114,14 @@ type RunnerSimulatorSetFailureDetails = { simulatorSetPath?: string; xcodeVersio
const simulatorSetPath = runnerSimulatorSetPath(device);
if (simulatorSetPath === undefined) return {};
- const xcodeVersion = readRunnerXcodeVersion(device);
+ const xcodeVersion = memoizedRunnerXcodeVersion(device);
return xcodeVersion === undefined ? { simulatorSetPath } : { simulatorSetPath, xcodeVersion };
}
</file context>
|
At 785f61b the fix moves the legacy XCTestDevices redirect restore into Not blocking: the three near-duplicate diagnostic shapes ( Is reusing one startup-diagnostic record type for both the daemon buffer and the Apple restore the smaller design here, rather than the two new types this PR adds? Folding the restore into The PR doesn't say how a concurrent daemon on an older version, in a different state dir, would leave its own in-flight redirect if this daemon's restore runs first — that behavior is unconditional on main today and this PR only moves it earlier, so it's an existing gap, not a new one, but it's worth a line in the body since the timing changed. No conflicts. CI is green across 21 checks. This is evidence-pending: the fix itself reads correct, but I'd want the darwin startup-cost question answered, or a note that it's negligible, before calling it ready. Restoring the daemon-level "failed restore doesn't stop the daemon" test would close the remaining gap; everything else here can be taken or left at merge. |
This branch has not been deployed
1 participant
Summary
Follow-up to #2963 (its last review round landed after merge). Refs #2935.
configureForDaemonLocknow callsrestoreLegacyXctestDeviceSetRedirectRuntimeafter the daemonlock and before the servers open or
daemon.jsonis written, so a staleXCTestDevicesredirectis repaired before the daemon starts serving requests, not after.
PlatformOwnerLifecycle.restoreLegacyXctestDeviceSetRedirectand
restoreLegacyXctestDeviceSetForDaemonStartupare deleted; restore diagnostics go through thesingle
startupDiagnosticsbuffer andflushDaemonStartupDiagnosticswrites them todaemon.log.Also closes the Option C gap from the r2 findings: an
XCTestDevicessymlink is now unconditionallyremoved at daemon startup regardless of what it points at (no ownership check, no refusal path), and
the toolchain-version probe on the diagnostic report path is replaced by a memoized read
(
memoizedRunnerXcodeVersion) so producing the "version unreadable" message costs no xcrun probes.12 files touched, net production lines (excluding tests/docs): +78 / −81.
Validation
Tested SHA:
785f61bd76(branch =origin/main78c0ca4b84+ this commit).pnpm check:affected --run-> EXIT=0: format, lint, typecheck, layering, fallow (--base origin/main),build, vitest (432 files / 3057 tests), command-docs (1 file / 12 tests) all passed. 4 tests ran
over the 2.5s budget but inside the 2x load band; no failures, no reruns needed.
pnpm check:production-exports-> EXIT=0: 68 pre-existing unused exports (unchanged from the priorround); the only touched-package entry (
runner-operations-facade.ts) is pre-existing, not asymbol this branch changed.
open com.apple.Preferenceson a throwaway iPhone 16 / iOS 26.2 simulator (own daemoninstance,
daemon.jsoncodeOrigin=checkout, version 0.21.15) succeeded in 67s;snapshot -ireturned 17 nodes over the XCTest path;
closesucceeded.daemon.logcarried nolegacy_xctest/restore diagnostics, as expected since~/Library/Developer/XCTestDeviceswas areal directory (not a symlink) before and after — the restore is a no-op on this host. The scoped
symlink-removal and toolchain-probe-avoidance paths are covered by
daemon-runtime-xctest-device-set.test.ts,runner-device-set.test.ts, andrunner-early-exit-diagnosis.test.ts, including a mutation check that removing the daemon-startuprestore call site fails the pinning test.
Remaining risk: the symlink-removal live path (the actual data-loss scenario from #2935) was not
exercised against a live symlinked
XCTestDevicesin this round — verified by test only. Apre-existing unrelated fixture path (
runner-startup-failure-fixtures.ts:194, from #2680) stillembeds
/Users/thymikee/.t3/...and is out of scope here.