fix(native-devtools): end the restart-app ↔ tool-server cycle a skipped dylib produces - #712
Draft
latekvo wants to merge 1 commit into
Draft
Conversation
…ed dylib produces DYLD_INSERT_LIBRARIES proves the bootstrap dylib was handed to a process, never that dyld loaded it: dyld skips an inserted library silently when its slice does not match the simulator's platform, when it is unsigned, or when a dependency is missing. Such a process reads as injected, so appConnectionState alternates between the two states whose remedies undo each other — stale_process prescribes restart-app, which leaves the app younger than the listener and reads unregistered; unregistered prescribes a tool-server restart, whose new listener is younger than the app and reads stale_process — and an agent obeying both never leaves the pair. Record the relaunch hand-out per bundle for the life of the service, and once a bundle that was told to relaunch reads unregistered, report the terminal injection_failed diagnosis instead of the tool-server remedy. The flip proves the process was replaced: which side of the listener a process falls on is fixed at its exec, so the two states cannot describe one process twice. The connected-peer list localises what is left, since the launchd env is simulator-wide and this service holds one listener. Only those two states take part. connecting resolves itself as the process ages out of the grace, not_running asks for a launch, and indeterminate is the absence of a reading — it prescribes a relaunch too, but a later unregistered there may be the same process finally becoming readable. The flow launch gate keeps its re-run advice: it performed the relaunch itself, one window earlier, and a cold start produces the same reading.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap #560 left
The first half is right. The conclusion is too pessimistic: terminating the cycle never required confirming the load.
Reproduced on this base
launchctl listreports a liveUIKitApplicationrow,ps ewwrenders the bootstrap dylib and the per-udid socket in the launch environment, and nothing ever dials. Each cycle applies the remedy the surface just prescribed —restart_required→ a new process exec;service_stale→dispose()plus a fresh factory rebinding the same socket:Repeats at cycle 3. The mechanism is a time comparison:
execTime - listeningSinceis constant for a given process, sorestart-appmoves the app to the post-listener side (unregistered), and restarting the tool-server moves the listener past the app (stale_process). Each remedy is individually reasonable; together they are a pump.The repo already names this failure mode in
utils/ios-host.ts— "injecting the default IOSSIMULATOR slice causes dyld to silently skip the library and native injection never connects."Why a retry budget does not work here
A counter is the obvious fix and it is unreachable on this base. The cycle hands out exactly two remedies per tool-server lifetime, and the second one (
service_stale→ restart the tool-server) discards the counter along with the process that held it. So no budget ≥ 2 ever trips, and a budget of 1 strands an agent that merely callsnative-devtools-statusand then the feature tool it was gating.The discriminator
The
stale_process→unregisteredtransition, which is a proof rather than a heuristic. Those two states sit on opposite sides of one time-invariant comparison, so a single process observed twice cannot produce both. Seeing the flip means a relaunch demonstrably happened and the app still did not register — which is exactly the claim the terminal message needs, and it is obtainable from the process table.unregisteredturns terminal only once a relaunch has been recorded for that bundle id. The record clears on the socket handshake and on dispose.Which states are bounded, and which deliberately are not:
stale_processunregisteredconnectingindeterminateunregisteredmay be the same process finally becoming readable, which would falsify the terminal message's opening claim. On ios-remote it is the only state a running app reachesnot_runningLocating the fault without a load confirmation
DYLD_INSERT_LIBRARIESis simulator-wide, so a connected peer proves the environment and the dylib are fine and narrows the fault to this app's binary; no peer points at the simulator-wide setup.listConnectedBundleIds()already existed and was consumed only by auto-targeting.The message states only what was observed — that the app was reported uninjected across a relaunch with no connection in between — so it stays true regardless of whether the agent actually performed the restart.
Built on what #560 already has
Reuses
appConnectionState/NativeDevtoolsAppStateas the sole source of truth,buildAppStateMessagefor the non-terminal case, the precheck's state→status routing (injection_failedslots in besiderestart_required/service_stale/connect_pending), theinit_failedblock shape fornative-devtools-status, and the three-flavour recovery split the non-injectable case already uses.flowLaunchGateReason's "landed here twice" idea is made mechanical.New: a per-instance record of relaunch advice, and one seam (
adviseOnUninjectedApp) that all four surfaces emit through — the sixnative-*tools via the precheck,native-devtools-status,describe's iOS fallback, and the flow tree reader.Left alone, deliberately:
flow-run.ts's launch gate. It performed the relaunch itself one window earlier, so it has no flip to read, and its reading is exactly what a cold start slower thanLAUNCH_TO_VERDICT_MSproduces — turning it terminal would break the case its own comment defends.Verification
["restart_required", "injection_failed"]andservice_stalenever appears.git diff:keeps the tool-server remedy on a first-contact unregistered app,does not spend the relaunch remedy on repeated reads of the same state,clears the spent remedy when the app connects, plus fix(native-devtools): derive restart_required from the running process instead of asserting it #560's ownprescribes a tool-server restart that actually returnseslint .0,prettier --check .0,tsc --build0,tsc --noEmit -p tsconfig.test.json0 — unpiped, exit codes read directly.scripts/extract-tools.test.mjs46/46, so tool descriptions remain statically extractable for the SpiderShield scan.boot-device-hotbootfailures this host produces from an exportedARGENT_EMULATOR_GPU_MODE=host(a separate defect, fixed in test(tool-server): stop unit tests inheriting the developer's ARGENT_* overrides #707, not on this base).CI does not cover this PR — read this before trusting the check mark
Every workflow in
.github/workflows/is gated onpull_request: branches: [main]. This PR targets #560's branch, so onlytool-description-quality(SpiderShield) ran — 1 check, where the same file set on #560 runs 13. Lint, format, unit tests, lockfile and every E2E job were never triggered.The green tick above therefore means almost nothing. What the change has actually been through, all run locally on this branch and read by exit code, unpiped:
The 6
boot-device-hotboot.test.tsfailures on that run come from this host exportingARGENT_EMULATOR_GPU_MODE=host; that is a separate defect fixed in #707 and not present on this base.Once #560 merges, retargeting this at
mainpicks up the full matrix. It should not merge on the strength of the check above.