Repository navigation
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
On an iOS simulator, open <app> --capture-network injects apple/network-capture/ADNetworkCapture.m through SIMCTL_CHILD_DYLD_INSERT_LIBRARIES. The probe observes each URLSessionTask to completion and appends one JSON line per request to the session's network.jsonl, so network dump reports every request with its own full URL, method, status, and duration, also on reused keep-alive connections, without a proxy. The dump states its source (probe or app-log); a probe that cannot be built fails the open with network-capture-probe-build-failed. ADR 0037.
…he unit budget The 375x812-vs-1080x2340 comparison spent ~0.8s of its wall clock encoding the capture with pngjs' default scanline filters, which for a solid fill buy no compression and only burn CPU under coverage instrumentation. On the Coverage lane it crossed the 2x slow-test enforce line and vitest's 5s timeout (run 39928270661). filterType 0 is a lossless choice the decoder inverts, so every compared pixel is identical.
63acf39 to
c3769fd
Compare
|
The live-run evidence in the body is useful, but I found two problems in c3769fd, and the Coverage check fails because of this change.
The Coverage job fails Not blocking, so take or leave: the dump input carries a host path that the Apple runtime uses only as a boolean, while Could the plumbing be simpler? The probe, its cache and its parser look inherent to me. The rest could shrink to one execution field instead of I did not run a simulator, so I have not verified the live run in the body; the body also does not name the SHA it ran on, and the production code at c3769fd matches the one feature commit 28470ff. Live evidence covers data tasks on iOS 27, so swizzling Before merge, please refuse |
The Coverage CRAP gate scored runIosSimulatorConsoleLaunch at 30.5 (blocks at 30): the capture-launch env arm, the capture-bound timeout branch and the rethrow arm had no test. All of them now run through the Apple tool provider seam: console+capture spawns the merged host environment with the SIMCTL_CHILD pair, console-only spawns with no environment, the 25s console bound keeps the open alive and logs the partial bytes with its diagnostic (with and without output on the error), and a non-bound failure rethrows. The fake tool provider now records the exec options so the env merge is observable at the seam. The console-capture budget moved to core/config.ts beside every other launch timeout so the test names it without duplicating 25_000.
|
Thanks for the update. The coverage-crap failure on CI is green at 2c90df4 (23 checks, none failing), and there are no conflicts. Before merge, the two issues below need to be fixed.
Not blocking, take or leave: I did not run a simulator. The PR body's live run names no SHA; production code at 2c90df4 equals 28470ff plus a constant move, so I treat that run as covering this commit, but I did not reproduce it. The Limrun finding comes from reading the device shape and the |
Review round on #3928 (thymikee), fixed at the owners: - A provider-backed iOS simulator (Limrun, TestMu) has the local simulator's device shape but opens through a lifecycle that cannot inject the probe, so its open accepted --capture-network, recorded the capture window, and dumped an empty probe source. The capability now rides the lifecycle binding: bindApplicationLifecycleInteractor derives supportsLocalSimulatorLaunch from ownership plus shape (a provider binding never claims it), and the canonical open refuses --capture-network and --launch-console without it. Contract tests drive a provider binding on a provider-shaped simulator and expect UNSUPPORTED_OPERATION with no interactor call. - One execution field: the open-option intent is execution.networkCapturePath alone; the redundant captureNetwork boolean is gone. The dump input carries a boolean mode marker and the session field stores a boolean window; the probe file path is derived from the session id at the one place that reads it. - One redaction implementation: redact() is exported and the capture-probe reader calls it instead of keeping a verbatim copy. - The console launch calls runSimulatorLaunch with an options parameter, so the env/timeout spawn exists once; the provider scenario now opens with both launchConsole and captureNetwork and asserts --console-pty plus the injected env on the same launch. - The probe masks Authorization, Cookie, Set-Cookie and their authentication siblings before writing, so a secret never enters network.jsonl; writes use writeData:error: under a 32 MB file cap, so ENOSPC cannot crash the app under test. ADR 0037 rules 1, 6 and a new rule 7 record the admission, the mask, and the crash-relaunch frozen-window limitation. - The capture reader drops a partial first line at the byte-window start, the same posture as the app-log tail, and counts it as skipped. Live evidence (ios-sim-net, this head): one open with --capture-network --launch-console produced source=probe with five distinct httpbin URLs (keep-alive reuse named individually), a 10-line console log, requestHeaders showing Authorization/Cookie as [REDACTED] with X-Trace-Id intact, and zero secret occurrences in the raw network.jsonl.
|
All findings addressed at 02addda. Live evidence re-run at this head is quoted below and in the body. 1. 2. 3. Coverage-crap on 4. Simplify the plumbing — Adopted where it didn't change behavior:
Non-blocking items, taken:
Not taken, documented instead — the crash/runner-relaunch frozen window: nothing in the daemon observes app-process relaunch, so no seam can distinguish "probe still running" from "relaunched without probe" without a new mechanism. ADR 0037 now states the limitation and the fix (reopen with On the evidence and coverage claims — fair on both: the earlier body did not name a SHA, and "all runnable checks passed" predates the Coverage lane finishing on that commit. The body now names the head every evidence block ran on. The Live evidence at 02addda (iPad-free: iPhone simulator All five keep-alive-reused requests named with their own full URLs (the gap the probe exists for); secrets masked in the dump and absent from the on-disk file; the non-sensitive Unproven, as you said — swizzling Validation: |
|
Thanks for the update. I reviewed 02addda against the earlier findings. The earlier problems are fixed, but one new problem remains. All 23 checks pass at this commit, and there are no conflicts. A refused open still changes the device first. The new owner refusal runs inside The rule should be: an open option the bound owner cannot honor is refused before the lifecycle touches the device, which covers the relaunch close, the runner stop and the runtime hints. Every lifecycle Could one admission step replace the nine Not blocking, and fine to take or leave: the new provider-refusal tests match error text instead of the typed code, and both new errors lack a typed Before #3930 lands, ADR 0037 (or the source-selection ADR both PRs cite) should fix Apple dump precedence. I would suggest probe, then Metro provider, then app-log, because the probe is an explicit open opt-in and also sees React Native fetch, which runs on URLSession on iOS. I did not run a simulator or a provider device. The live Before merge, please refuse these options before any relaunch close or runtime-hint write, with the contract test above, and settle the probe and Metro precedence with #3930. |
… device Review round on #3928 (thymikee, second pass). A refused open still changed the device first: the owner refusal ran inside invokeApplicationOpen, after the relaunch close (closeBeforeRelaunch on Limrun/webdriver), the runner stop, and the runtime-hint write. `open <app> --relaunch --capture-network` on a provider iOS simulator killed the running app and then failed. - One admission step: contracts `assertOpenAdmitted(binding, execution)` is the whole rule for `--launch-console` and `--capture-network`. It is the first call in openDirectApplication, openAndroidApplication and openAppleApplication, so an owner that cannot honor the option is refused with UNSUPPORTED_OPERATION before any device action. It replaces the nine `supportsLocalSimulatorLaunch` pass-through sites and the four stacked refusals in the canonical open; `invokeApplicationOpen` keeps only its positional, URL, and argument-shape checks. - Both refusals carry a typed reason (`launch-console-owner-unadmitted`, `network-capture-owner-unadmitted`), matching the sibling `provider-runtime-interactor-missing` posture. Their messages stay module constants of the module that raises them, clear of any function's JSDoc (the shared message home's facade must stay exhaustive, and a re-export no consumer imports is dead code). - The provider-shaped contract test now opens with `relaunch: true` against a `closeBeforeRelaunch: true` owner and asserts the typed code and reason and that neither open nor close ran. A second case pins the same typed refusal for a local binding on a non-simulator device. - A probe file at its 32 MB cap no longer reads as current traffic: the capture read reports `atSizeCap` (the reader owns the cap constant mirroring the probe's AdMaxFileBytes) and the probe-mode dump says the window stopped recording. The cap check reads the handle offset through `getOffset:error:` instead of the raising `offsetInFile`, per ADR 0037 rule 6. - ADR 0037 rule 1 now states admission-before-device-action, rule 6 the cap note and the offset API, and rule 5 fixes Apple dump precedence — probe, then provider, then app-log — as the answer to the #3930 Metro question. Live evidence re-run at this head on ios-sim-net: one open with --capture-network --launch-console produced source=probe with five distinct httpbin URLs, a 10-line console log, masked Authorization/Cookie with X-Trace-Id kept, zero secret occurrences in network.jsonl; `open --relaunch --capture-network` on the local simulator stays admitted (relaunch close, then capture, dump still probe); a non-admitted open refuses with UNSUPPORTED_OPERATION and reason network-capture-owner-unadmitted.
|
All findings from this pass addressed at 1. A refused open still changes the device first (blocking) — Fixed exactly as proposed. 2. One admission step replacing the nine pass-through sites and four stacked refusals — Adopted wholesale. 3. Non-blocking items — all taken:
4. Dump precedence with #3930 — Settled in ADR 0037 rule 5, in your order: probe, then provider (Metro), then app-log — probe because it is an explicit per-open opt-in and sees React Native What I ran / did not run — This pass I did run a simulator: the full live block re-executed at |
…licy Settles the source-selection question raised against #3930 and #3928 in one record: the platform runtime owns the order, the source enum is probe/provider/app-log, no capture installs unless an explicit operator command arms it, and the React Native provider injects only on the runtime whose device the platform named uniquely among attached devices.
…licy Settles the source-selection question raised against #3930 and #3928 in one record: the platform runtime owns the order, the source enum is probe/provider/app-log, no capture installs unless an explicit operator command arms it, and the React Native provider injects only on the runtime whose device the platform named uniquely among attached devices.
|
This PR is ready at c1ac863. The earlier finding is fixed: a refused Not blocking, take or leave: ADR 0037 rule 1 says a refused open never kills a running app and that refusal happens before every runtime-hint write. Only the owner refusal runs in I did not run a simulator or a provider device, compile |
thymikee
added
the
ready-for-human
…ouches the device Review round on #3928 (thymikee, third pass), non-blocking note taken as the stronger option. ADR 0037 rule 1 says a refused open never kills a running app, but only the owner refusal ran in assertOpenAdmitted: the URL-pair and no-app-target refusals still ran inside invokeApplicationOpen, so `open <bundle> <scheme://x> --relaunch --clear-app-state --capture-network` on a local iOS simulator closed the app and then failed with INVALID_ARGS. - One request-shape rule, `assertOpenRequestShape(positionals, execution)`, holds every launch-only check that needs no device: the positional count, the no-app-target refusals, the URL-pair refusals (first-argument, valid-URL, --launch-console, --capture-network) and the deep-link app refusals (--launch-console, --capture-network, --clear-app-state). assertOpenAdmitted enforces it, so every openApplication entry applies it before any relaunch close, runner stop, or runtime-hint write; invokeApplicationOpen keeps the same call for its folded and follow-up dispatches, and the open path now holds the one refusal that needs the platform itself (--launch-args on Linux). - Owner capability is still checked before request shape: where an option works at all is the more actionable refusal than the shape it was paired with. - Contract test: `a URL-pair relaunch carrying capture and clear-app-state is refused before any close` drives the review's exact command through a closeBeforeRelaunch local iOS-simulator binding and asserts INVALID_ARGS with no open, close, or setSetting call. Two table rows cover the deep-link app position for both launch-only options. - ADR 0037 rule 1 now states admission covers the owner capability and the request shape, so its sentence is true as written for every refusal it names.
|
Thanks — taking the note as the stronger option, at Rule 1 vs the shape refusals — The launch-only checks that depend only on positionals and execution now live in the admission step. Contract test — Code shape note — the flat version of the shape rule breached fallow's complexity threshold at the exact function the previous round moved checks into, so it is split into one helper per positional shape (device-only / URL-pair / deep-link app), which is what the two extra table rows pin. No live re-run: admitted-open behavior is unchanged — every moved refusal keeps its code and message, only its timing moved earlier; the commands the last live run admitted (capture, console, local |
|
This PR is ready at 17c933f. The change since the earlier review (c1ac863) closes the note about refusal ordering: an open that carries Not blocking, and fine to take or leave: the new doc block in I read the code at both commits but did not run the contracts tests or any gate locally, and I did not do a live device run of the new refusal ordering. Your live run at c1ac863 still covers the capture happy path. There is also no Apple or Android lifecycle test of the URL-pair refusal before close, though the ordering follows from the admission check being the first statement in both entries. I read ADR 0038 from the unmerged branch for #3930, so it can still change before that lands. |
…touches the device Extracted from #3928. assertOpenAdmitted and assertOpenRequestShape run first in openDirectApplication, openAndroidApplication and openAppleApplication, so an open option the bound owner cannot honor is refused before any relaunch close, runner stop, or runtime-hint write. On a provider iOS simulator (Limrun/TestMu shape), --relaunch --launch-console used to close the running app and only then fail. The binding now carries a supportsLocalSimulatorLaunch owner capability, and every refusal uses a typed reason.
…touches the device Extracted from #3928. assertOpenAdmitted and assertOpenRequestShape run first in openDirectApplication, openAndroidApplication and openAppleApplication, so an open option the bound owner cannot honor is refused before any relaunch close, runner stop, or runtime-hint write. On a provider iOS simulator (Limrun/TestMu shape), --relaunch --launch-console used to close the running app and only then fail. The binding now carries a supportsLocalSimulatorLaunch owner capability, and every refusal uses a typed reason.
What
open <app> --capture-networkon an iOS simulator injects a small dylib probe (apple/network-capture/ADNetworkCapture.m) into the app process throughSIMCTL_CHILD_DYLD_INSERT_LIBRARIES. The probe observes eachNSURLSessionTaskto completion and appends one JSON line per request to the session'snetwork.jsonl.network dumpthen reports every request with its own full URL, method, status, and duration — also on reused keep-alive connections — with no proxy.Design recorded in ADR 0037 (
docs/adr/0037-ios-simulator-network-capture-probe.md).How
simctl launch --terminate-running-processwith exactly twoSIMCTL_CHILD_variables (dylib path + capture file).SIMCTL_CHILD_*is purely additive — the app's own environment is unchanged. A capture launch always terminates the running instance: dyld injects only at exec, so an already-running app would activate without the probe.native-build/cache.ts), so the first capture launch builds once per toolchain and later launches serve the cached binary. ADEVELOPER_DIRswitch rebuilds. A probe that cannot be built fails the open withreason: network-capture-probe-build-failed— no silent launch-without-capture.session.networkCapture(a boolean window recorded by the store viaSessionStore.setNetworkCapture; any capture-less open clears it) selects probe mode; the dump reportssource: 'probe' | 'app-log'either way. Probe mode never consults the app-log parser. The probe file path is derived from the session id once, in the composition root.redact()with the operator'sAGENT_DEVICE_APP_LOG_REDACT_PATTERNS. The probe itself masksAuthorization/Cookie/Set-Cookieand authentication siblings before writing, and stops at a 32 MB file cap usingwriteData:error:(ENOSPC never crashes the app).assertOpenAdmitted(binding, execution)is the whole admission rule for--launch-consoleand--capture-network, called first in everyopenApplicationentry (openDirectApplication,openAndroidApplication,openAppleApplication) — so a provider-backed iOS simulator (Limrun, TestMu) or any non-simulator device is refused withUNSUPPORTED_OPERATIONand a typed reason (network-capture-owner-unadmitted,launch-console-owner-unadmitted) before any relaunch close, runner stop, or runtime-hint write; a refused open never touches the device. Only a local owner bound to an iOS simulator declares the capability. With a URL/deep-link open →INVALID_ARGS.await import+ the existinglaunch-console.tshome), so all eager-closure budgets hold at merge-base.Live evidence (ios-sim-net simulator, SpikeApp
dev.agentdevice.spikeapp, re-run at headc1ac863808)The spike app fires 5 sequential URLSession requests over one keep-alive connection; nothing is emitted to Unified Logging. The
/headersrequest carriesAuthorization,Cookieand a non-sensitiveX-Trace-Idto prove the probe-side secret mask. One launch with both options:All five requests appear with their own full paths, statuses and durations despite connection reuse — the case the app-log parser cannot name. Credential headers are
[REDACTED]in the dump and absent from the on-disk file;X-Trace-Idsurvives. The console PTY capture and probe injection share one launch.open --relaunch --capture-networkon the local simulator stays admitted (round 2's blocker: admission now runs before the relaunch close, so a local owner still closes-then-captures and the dump keeps reportingsource: probe).Contrast runs on the same simulator:
source: app-log,entries: 0(the exact gap this PR closes).source: app-log, and the plain launch carries no injected environment.open https://example.test --capture-network --platform web→UNSUPPORTED_OPERATIONwith reasonnetwork-capture-owner-unadmitted(admitted before any device action), message--capture-network is supported only for a local iOS simulator app launch; this session owner cannot inject the capture probe.Validation
count, no-app-target, URL-pair, deep-link app position) moved into the admission step too —
assertOpenAdmittednow enforces owner capability and request shape before the lifecycle touchesthe device, so
open <bundle> <scheme://x> --relaunch --clear-app-state --capture-networkisrefused with
INVALID_ARGSbefore any close. A contract test drives exactly that command througha
closeBeforeRelaunchlocal iOS-simulator binding and asserts no open/close/setSetting call.invokeApplicationOpenkeeps the same shape rule for its folded/follow-up dispatches plus the oneplatform refusal (
--launch-argson Linux). ADR 0037 rule 1 states both halves of admission.assertOpenAdmitted(binding, execution), called before the lifecycle touches the device in allthree
openApplicationentries; it replaced the ninesupportsLocalSimulatorLaunchpass-throughsites and the four stacked refusals, and both refusals carry typed reasons. The provider-shaped
contract test opens with
relaunch: trueon acloseBeforeRelaunch: trueowner and asserts thetyped code/reason and that close and open were never called. A capped probe file (32 MB) now
reports a dump note (
atSizeCapon the capture read), and the cap check usesgetOffset:error:.ADR 0037 records admission-before-device-action and fixes Apple dump precedence: probe, then
provider, then app-log.
(
supportsLocalSimulatorLaunchderives from ownership + shape; provider bindings never claim it),the redundant
captureNetworkexecution field collapsed intonetworkCapturePath's presence, theduplicated
redactCaptureTextreplaced by the exportedredact(), the console launch folded intorunSimulatorLaunch(launchArgs, { env, timeoutMs }), dump input and session field reduced to aboolean mode, the probe masks credential headers and caps its file at 32 MB via
writeData:error:, and the reader drops the byte-window's partial first line. Contract tests pinthe provider-binding refusals; the provider scenario asserts
--console-pty+ injected env onone launch with both options. ADR 0037 amended (owner admission, probe-side mask, 32 MB cap,
crash-relaunch limitation as rule 7).
pnpm check:affected --run --head 17c933f8c7→ all runnable checks passed,exactCommit=true,all runnable gates including layering, fallow, coverage, coverage-crap, depgraph, wire-compat,
xctest-selection and packaged-runner-swift. (Round-2 receipt: same result at
c1ac863808.)runIosSimulatorConsoleLaunchscored 30.5 (blocks at 30) because thecapture-env arm, the console capture-bound branch and the rethrow arm had no test. It now has
branch tests through the Apple tool provider seam (no exemption): console+capture spawns the
merged host env with the
SIMCTL_CHILD_*pair, console-only spawns with no env, the bound keepsthe open alive with its diagnostic (with and without partial bytes), a non-bound failure
rethrows. Measured with the gate's own counters: 14/14 statements, 10/10 branches, CRAP 1.0.
screenshot-diff.test.tsfull-resolution case (4.73s on main's Coverage lane, inside the2× band) crossed vitest's 5s timeout under load on this PR's first run. It now encodes the
solid fixture PNGs unfiltered (
filterType: 0, lossless — the decoder inverts it): 5.1s →~0.27s. Not a skip.
networkCaptureclassified as a store-owned SessionState field (R7/R10 ratchet holds), spawn-env merge moved to thetool-provider.tscommand boundary (R86), new eager edges moved behindawait import/ existing closure homes, probe-parser duplication shared with the app-log reader (fallow), new provider subpath declared in the R11 inventory.test/integration/provider-scenarios/ios-network-capture.test.ts): scripted build + injection env +--terminate-running-process+ cache reuse across two capture opens + probe dump (newest first) + reopen-without-capture mode switch. The probe source also compiles with-Werroragainst the real SDK in a darwin-only gate test.Not verified
proven (completed HTTP(S) data tasks). Extending the spike to
uploadTask/downloadTaskis afollow-up on request.
the probe's iOS 15 floor.
tests over the scripted provider binding, not against a real provider device.
DEVELOPER_DIRswitch is proven only against the scripted toolchain in the provider scenario.🤖 Generated with Claude Code