Repository navigation
fix(ios): prepare snapshot helpers within a shared readiness budget - #4181
andrewfree wants to merge 11 commits into
Conversation
|
Someone is attempting to deploy a commit to the Callstack Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks for the change. The code at 34236db looks sound, but it is device-facing and has no live run yet, so it is not ready to merge. This change reorders native helper startup (runtime init before bind) and moves helper creation out of the capture into a detached 10 s attempt (lifecycle.ts#L231). The PR body says live qualification on the current head is pending, and unit fakes cannot show the main risk. If the cold AX cost sits in the app's first Not blocking, and fine to take or leave: (a) with The only failing check is Vercel, which needs a team member to authorize the fork deploy. The diff touches no website or docs-site files, so this looks unrelated. No GitHub Actions job ran on this fork head, so there is no unit or gate evidence yet. Before merge, the live cold-Simulator run above is needed, followed by a full |
|
Initial simulator evidence for Commands used Observed phase evidence:
The native cache manifest matches the six current source inputs: source fingerprint The full affected gate passes all 41 runnable checks at this SHA; 25 skipped lanes are not passes. Draft retained: exclusively primary cold acceptance, the earlier separate application-server/foreground failure, and the wider runtime matrix remain unproven. The fallback remains a workaround. No capture deadline or behavioral assertion was weakened. Verification: OpenAI Codex, parent gpt-6-astra; independent source review gpt-6.1-sol. Follow-up at On the preceding head with a prebuilt bridge and a freshly rebooted simulator, one helper PID served all captures. Preparation took 145 ms. Open encountered Nonblocking refactors (shared constants, required readiness deadline, shared cleanup ownership, README placement, and typed native startup-protocol cleanup) are deferred. The suggested adapter-boundary timing test is a coverage gap; current source already passes the same readiness deadline to preparation and manager. The unreachable runtime-unavailable response is a diagnostic cleanup, not a newly observed capture defect. No unrelated refactor was included. Final correction head The current-head cleanup probe observed the real prebuilt helper with no socket yet, then requested A subsequent fresh current-head daemon captured Settings through primary AX (754 ms and a 188 ms stabilization acquisition), navigated by General's fresh ref, then captured General through primary AX in 719 ms. Source and T3 General PNGs were saved and inspected. The 8.235-second click included 6.458 seconds of XCTest runner readiness preflight; it was not a fast-click benchmark. Open still reported the transient foreground-owner refusal before primary recovery. Final source session/daemon cleanup passed, with owned helper/daemon PIDs and socket absent. This closes the pending-startup cleanup evidence gap, while the exact requested cold pending/fallback trajectory and runtime matrix remain unqualified. |
|
I reviewed 312cdcf. The live evidence I asked for at 34236db is now in, so that part is resolved. One defect remains in the new readiness path. With The rule the code must satisfy: every helper startup failure is delivered to exactly one capture, and it retires a generation only when that capture belongs to the generation whose capture started the attempt. A capture of any other generation starts a fresh attempt. Not blocking, take or leave: the new regression at lifecycle.test.ts:83 throws a hand-built There are no conflicts. The only failing check is Vercel, which waits for a team member to authorize the fork deploy. The diff touches only |
|
Fixed and pushed at The manager now retains a failed startup with its starting generation. The next capture of that generation receives and consumes the failure; a different generation discards it and starts a fresh helper. A readiness waiter that observes the original rejection clears the retained failure, so it is not replayed. The regression drives a real fixture process exit after the first 2-second wait through bridgeExitOrClose. Crash and 10-second startup expiry both cover same-generation delivery, consumption without replay, and direct generation-2 recovery. The expiry test now requires timeout/bridge-startup-deadline for the next same-generation capture because its previous expectation skipped failure delivery. Cleanup and fresh-helper assertions are retained. No deadline was widened. The final tests on c7d87ea produce 4 expected failures, 2 passes and 31 filtered skips; on the new head all 6 cases pass with 31 filtered skips. Focused lifecycle, adapter, preparation, route and route-failure-policy tests pass 99/99. The full affected gate passes all 41 runnable checks with exactCommit=true; 25 lanes are skipped, not passes. Broad unit results: 17,112 passed, 1 skipped. The optional direct disabledGenerations assertion was not added; existing route/failure-policy tests were included. No new live-device run was performed. |
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
At 76ce032, the manager now retains the previously requested readiness state, and I found no new blocking defect since the earlier review. The review is static; I did not run tests or device commands. The exact cold-AX recovery trajectory remains unobserved, as previously accepted. The null fixture thread and two lower-priority fixture threads still apply. The binary path thread does not apply because production capture uses one fixed binary per runtime. The only failing check is the Vercel deployment authorization, which is not a code problem. There are no known conflicts. Before merge, please address or reply to the three fixture threads. |
|
This conflicts with main now. Please rebase it so review can continue. The three fixture threads from the review at 76ce032 still apply. |
Cover a helper crash observed inside the starting capture's wait: the next same-generation capture starts a fresh helper instead of receiving the failure again. Require close to stay pending while adopted startup is paused instead of checking one microtask. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…exit codes The fixture identity cases still create real directories and sockets, so a 100 ms deadline could fail them before identity validation on a loaded runner. The readiness fixture now reports fork and waitpid failures with their own exit codes instead of reusing the behavioral failure code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
76ce032 to
bfbead4
Compare
|
Rebased on main |
|
At bfbead4, the code looks good. The rebase is in, there are no conflicts, and production code is unchanged from the revision that had live device evidence, so I am not asking for a new live run. The fixture threads are fixed and can be resolved: null return, exit codes, pid mismatch. The binary path thread does not apply, since adapter.ts prepares one binary per runtime. Coverage failed at bfbead4, but its log stops at "building macOS helper" and names no failing test. The same cut-off log appeared on another PR and later passed, and main Coverage passed at 4abff15, so it looks unrelated. Please re-run Coverage so there is a green result or a complete log that names a test. One design question, since the change adds about 800 production lines: could the adapter be the single owner of the readiness allowance, with one named constant and Not blocking, take or leave: the 10 s startup ceiling is also a literal twice at lifecycle.ts:297 and :299. |
Summary
Cold Simulator AX startup can consume a capture deadline. Preparation and connection share two seconds; detached startup has a ten-second ceiling. Pending startup permits XCTest fallback without retiring the generation. Initialize the native runtime before listener readiness and await owned work on close.
Retain late startup failures for their starting generation without replay to new generations. Fix three fixture issues. 13 files change; startup notes move to
apple/snapshot-bridge/README.md. No public API change or linked issue.Rebased on main
4abff15e, after #4172 and #4217. The readiness fixture now links #4172'sSnapshotBridgeSurfaceHosts.m, and adapter helpers moved toadapter.fixtures.tsto keepadapter.test.tsunder the line limit. Production diffs are unchanged by the rebase.Validation
Head
bfbead48f2c9b02bcac1cf5d94b8ee6756bc7afb; base4abff15edaf7a2634f1b2a00edd742a14e808630.pnpm check:affected --run --head HEAD: 41 checks passed, exact commit; 25 lanes skipped, not passes. Broad unit: 17,364 passed, 1 skipped.cache-native-runtime.test.ts(included): 51/51, 0 skipped, including both readiness scenarios.AI-assisted under Andrew's direction using OpenAI Codex and Claude.