Conversation
…nch probe and the capture route resolveLaunchedTarget and resolveTargetForObservation each looped on simulator-target-discovery-pending themselves, differing only in what ends the retry: one gives up after any non-pending failure, the other also gives up once a live runner can answer instead. Both now call resolveSimulatorTargetJoiningDiscovery in snapshot-target.ts, passing their own continuation rule; isSimulatorTargetDiscoveryPending drops its export since the shared loop is now its only caller.
Size Report
Startup median (7 runs, lower is better):
|
Member
Author
|
Closing: an adversarial review found this adds more indirection than it removes (net +30 lines to share about 4). Reasoning is on #2838. |
|
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
1 participant
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.
Summary
Follow-up on #2838's review note:
resolveLaunchedTarget(snapshot-observability.ts) andresolveTargetForObservation(snapshot-route.ts) each looped onsimulator-target-discovery-pendingthemselves, differing only in their exit rule — one returnsundefinedon any other failure, the other also gives up once a live runner can answer instead. Both now call a singleresolveSimulatorTargetJoiningDiscoveryloop added tosnapshot-target.ts(which already owns the discovery-pending reason), passing their own continuation rule as a parameter.isSimulatorTargetDiscoveryPendingdrops itsexportsince the shared loop is now its only caller (flagged byfallow).Behavior is unchanged for both callers, including cancellation via the request signal (each caller keeps its own signal check exactly where it had it) and the discovery's own deadline (untouched, in
resolveSimulatorSnapshotTarget). No new tests: the existing suites already exercise both exit rules through production resolvers (snapshot-observability.test.ts's pending-vs-non-pending table,snapshot-route.test.ts'shasLiveRunnerSessionscenarios).Touched files: 3.
Validation
Tested at
90dc3cadb8e9a646af47355813ec2edbed6b02c0.pnpm check:affected --runpasses clean (264 files / 1706 tests, format/lint/typecheck/layering/fallow/build all green). Two earlier runs on this host hit unrelated flakes (a provider-integration timeout and a tmpdirENOTEMPTYcleanup race) that reproduced only under load and passed in isolation and on a clean re-run — consistent with documented host-contention flakiness, not a regression from this change.No device run: this is a behavior-preserving internal refactor with no observable-surface or command-semantics change.