Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at 6342a18. The typed reasons change the error envelope on the physical-device Could I did not find a smaller design: the new rows reuse CI is green. GitHub reports a conflict with |
|
Reviewed at 98f2870. This follows up on the earlier review at 6342a18 (#2688 (comment)). The two live physical-device runs asked for at 6342a18 are still missing, and #2680 lists them as a done criterion. Every build-for-testing fixture in runner-startup-failure-fixtures.ts still has provenance Can you run this on a physical iPhone at the pushed head: Not blocking: the PR description says 86 files and 443 production lines, but this diff touches 11 files with +264/-43 production lines, and the earlier question about CI is green on all 5 checks. The branch conflicts with Next: post the two physical-device |
98f2870 to
85bd4de
Compare
|
At 85bd4de this is still the same rebase plus a test-helper extraction; it adds no new captures. Every build-for-testing fixture in packages/platform-apple/src/runner/tests/runner-startup-failure-fixtures.ts#L56 is still shipped-sniff-trigger or invented-shape with xcodeVersion 'unobserved', so nothing shows real Xcode output actually reaches these rows. Two rows are unproven in opposite directions: Can you run, on this pushed head, Not blocking: the rebase onto 0ebd254 (#2689) left the PR body's "declared twice" rationale stale now that BootFailureReason has one declaration in provision-kit/src/boot-diagnostics.ts#L5 with runner/host.ts only delegating, so the PR body could drop that clause and keep the vocabulary argument, but this can be taken or left. I did not run any test locally, so the claim that the moved assertions still fail without the fix rests on reading the hunks, not a run. I could not verify real Xcode wording for any rule since no capture exists in the branch. Repo Guards job 105956422148 shows CANCELLED with no failed-step log, which looks like a run superseded by the rebase push, but the diff does touch what that job exercises (a new top-level .device-evidence/ directory, a new platform-apple module, and docs), so the cancellation cannot be read as unrelated; it simply carries no result and needs a re-run on 85bd4de. Before this can merge: post the two physical-device prepare captures with reason, hint, logPath, commit SHA, and |
85bd4de to
9621905
Compare
|
Both points addressed, rebased onto The bare All three narrowed rows are PR body corrected. The Gates at the new head: Not done, and not runnable from here: the live |
|
This is a follow-up on the earlier review (85bd4de, #2688 (comment)). Reviewed at 9621905. Row 471 in https://github.com/callstack/agent-device/blob/9621905/packages/platform-apple/src/runner/runner-contract.ts#L471 only requires the substring 'expired', but toolTextIncludesAll scans the whole captured build log (execFailureDetails attaches full stdout/stderr, see packages/host-kit/src/internal/exec.ts#L703), so the pair can match across unrelated lines. A log with the benign All build-for-testing fixtures, including the three new profile rows, carry provenance 'invented-shape' or 'shipped-sniff-trigger' with xcodeVersion 'unobserved' (packages/platform-apple/src/runner/tests/runner-startup-failure-fixtures.ts#L151), so nothing at 9621905 shows real xcodebuild output reaching these rows instead of falling through to build_failed_unclassified. Can this be validated on a physical iPhone at 9621905 by running Not blocking: the signing_no_profiles_for_bundle_id row at runner-contract.ts#L438-441 spells out the same reason+hint pair as literals instead of using the PROFILE_UNUSABLE constant added at line 276, so it could point at PROFILE_UNUSABLE too, but this can be taken or left. No tests or device commands were run for this review; the claim that a profile-mentioned-while-compiling log matched the pre-delta bare row rests on reading the removed code, not on a run. The real Xcode wording for all three narrowed rows is unverified, so whether "doesn't include" arrives with an ASCII apostrophe, versus a typographic one that would fall through row 463 to the catch-all, is unknown. The author's gate list (build, format, check:*, apple-runner 503, unit-core) was not re-verified at this head. Smoke Tests failed at test/integration/ios-simulator-e2e/live-assertions.ts#L52, with id="automation-longpress" not visible after scrolling, after the runner built and 14 other assertions passed; this looks unrelated, since the head commit touches only the build-failure rule rows, fixtures and CHECKLIST, which run only when xcodebuild build-for-testing throws, and the runner-session.ts step rename plus the DevToolsSecurity preflight import return early for kind !== 'device', with no hunk touching snapshot, scroll, or selector resolution. The path to merge is narrowing row 471 to "has expired" with a cross-line negative fixture, then posting the three physical-device prepare captures at 9621905 and marking the matching fixtures captured. |
9621905 to
bcfed35
Compare
|
Both items in, rebased onto The expiry row no longer reads a bare The cross-line negative is in and it is the proof, not decoration. Constant reuse done. Fixtures stay honest: all three narrowed rows are still Gates at the new head: Still owed, and outside what I can run: the §#2680 prepare captures (no-team, foreign bundle id, missing profile, pinned-profile pairing). No fixture here says |
|
bcfed35 looks good and the code verdict is clean. Narrowing the profile rules so a name and its complaint must share one line is the right answer to the cross-line match from the last round, and it reuses the existing haystack instead of adding a second source of truth. CI is green with 20 checks passing, and the unit lane the new fixtures drive covers the changed classification path. No conflicts are known. Not blocking: One limit worth recording: the new fixture's pre-fix red state comes from reading the 9621905 rule table against the fixture output, not from a run, and the real xcodebuild wording for the |
thymikee
added
the
ready-for-human
…dule runner-session.ts is 1,181 lines, so the next behavior owed a split first. The host developer-tools security probe is a self-contained preflight: it reads a macOS setting for this machine, not the iPhone's Developer Mode toggle, and today nothing in its name or its home says which machine it speaks for. Pure move. The startup step key is renamed to say what it actually verifies, and the existing coverage in runner-session.test.ts carries unchanged because the probe consumes the same runner host port.
A `build-for-testing` failure was the one Apple failure shape with no typed reason: `runner-artifact.ts` threw COMMAND_FAILED with a hint picked by substring-matching the lowercased message plus a JSON dump of the details, and nothing downstream could switch on which signing problem it was. `AGENTS.md` keys behavior on typed reasons and details, never error text, so the build path was the exception to the rule. `classifyRunnerStartupFailure` is now the one classifier reachable from that catch, and it reads `RUNNER_ERROR_RULES` rather than adding a second table: a row either carries recovery verdicts, a `buildFailure` reason and hint, or both. The reason and the hint beside it therefore cannot disagree, and an unproven cause is never claimed -- `build_failed_unclassified` keeps the cache-recovery hint it already gave. The host `DevToolsSecurity` refusal, which had a hint and no reason at all, now publishes its own reason keyed on the typed status it read, and that reason says host so it can never be read as the device's Developer Mode toggle. `resolveRunnerBuildFailureHint` is gone, not kept as a pass-through. Reasons are recorded as fixtures carrying the tool output, the command, the Xcode, and how the line reached the file; each one is driven through the real build catch and asserted on the normalized envelope, because every case is COMMAND_FAILED and the reason is the assertion.
…ce needs it A static edge from `runner-session.ts` to the new module grew three Apple facade closures by one module each (app-lifecycle, doctor, runner-operations), which the eager-import-closure ratchet refuses: the runner subtree is eagerly evaluated to answer a simulator request, and this preflight only ever runs for a physical iOS device. Function-scoped import keeps the preflight where it belongs without paying for it on the paths that can never use it.
…ublished The startup rules read the whole `details` bag as JSON, and `execFailureDetails` puts `cmd`/`args` in that bag. A caller who pinned a profile therefore handed the profile rule its trigger: an unrelated Swift compile error classified as `signing_provisioning_profile_missing` and lost the cache-recovery hint. The same bag holds the `reason` and `hint` this classifier publishes, so a re-wrapped failure would match its own verdict. `runnerToolText` now builds the one haystack a startup rule may read — our message plus the tool's stdout and stderr — and the argv case has a fixture that proves a pinned profile is not evidence (#2680). Reading only `details` also dropped the message on the floor, so a failure the exec layer raised as a plain `Error` (which the catch wraps with `String(err)`) became unclassified while the same sentence in `stdout` classified. The message is in the haystack now, and a `message-only` fixture keeps both carriers at parity. `signing_style_conflict` is removed rather than kept as a guess: the hint named a `CODE_SIGN_STYLE` env lever that does not exist and claimed a cause nothing captured. The conflicting-settings line keeps a row so the profile row below cannot answer it with missing-profile advice, but it publishes `build_failed_unclassified` until a capture shows which setting disagrees and which lever clears it. Versioned help and the installation docs no longer list the withdrawn reason, and `.device-evidence/CHECKLIST.md` gained the capture that would let a follow-up claim it. Provenance got honest vocabulary. `OBSERVED_ON` stamped invented sentences as observed on Xcode 26.2, and one invocation was recorded as the producer of seven configurations that cannot coexist; nothing here was captured, so entries now say `shipped-sniff-trigger` (the matched substrings shipped before #2680, sentence reconstructed) or `invented-shape`, carry `xcodeVersion: 'unobserved'`, and omit `command` until a run records one.
…ure assertion body #2689 derived the runner host port from the modules it fronts, so `ExecResult` is read from `@agent-device/host-kit/command` now rather than restated in `runner/host.ts`. The per-fixture assertion body had grown past the complexity the Fallow audit allows a changed file: the envelope checks move into `assertFailureEnvelope` and the tool-output check into `assertToolOutputReachable`, so each recorded shape is still asserted through the same path and the test body reads as one call.
One bare `provisioning profile` substring was the shipped sniffer's trigger, and it is a phrase a failing build prints while talking about something else: the codesign invocation, a settings dump, a note about the profile it used. Each row now requires the profile plus the complaint Xcode attaches to it — its `IDEProvisioningErrorDomain` diagnostic, "doesn't include", "has expired" — and a mention that says nothing keeps the cache-recovery advice it already had.
A whole-log AND proves two phrases exist, not that one qualifies the other: a note about the profile the build used, three lines above an unrelated expired-certificate warning, published `signing_provisioning_profile_missing` for a profile that was fine. The profile rows now read one line at a time and the expiry row asks for Xcode's own `has expired` phrase. The rules table grows a `toolTextLineIncludesAll` axis beside `toolTextIncludesAll`, and the bundle-identifier row that also paired two phrases moves onto it so no startup row is left reading a whole log; its cross-line negative rides along. `signing_no_profiles_for_bundle_id` shares the `PROFILE_UNUSABLE` advice instead of spelling it out a third time. `.device-evidence/CHECKLIST.md` belongs to the merged #2682 lane, so this stack's capture sheet lives beside it as `CHECKLIST-runner-failures.md`.
bcfed35 to
15808ae
Compare
|
Rebased onto The one nit — the bundle-identifier row was the last whole-log AND. A cross-line negative rides along, The Gates at this head: Still owed, and unchanged in that: the physical-device captures in |
|
At What is still missing is the Phase B physical-iPhone capture, unchanged since the For that reason I have taken Two things I did not check here: the apple-runner suite was not run, so the claim that the new cross-line fixture is red under the pre-delta rule is read from the old and new match functions rather than executed; and the single-phrase whole-log rows and the profile rows themselves are outside this delta, so they keep their earlier verdict. Not blocking: the PR body still points at |
thymikee
removed
the
ready-for-human
Phase B — signing-failure captures on a physical host (coordinator-run)Host: Result: only
|
|
Thank you — these captures close the gap I held this on at Your account-level explanation for I put |
thymikee
added
the
ready-for-human
… the rest need Co-Authored-By: Apex <noreply@callstack.com>
|
Reviewed at 12de0a8. The delta does what I asked at 15808ae. The The Smoke Tests failure is in the iOS simulator scroll check (the long-press test element did not become visible after scrolling). This PR changes only runner startup-failure classification, so the failure looks unrelated. There are no conflicts, and |
1 participant
Summary
build-for-testingwas the one Apple failure shape that reached callers as prose: the catch inrunner-artifact.tsthrewCOMMAND_FAILEDwith a hint chosen by substring-matching the message plus a JSON dump of details, so nothing downstream could switch on the cause.classifyRunnerStartupFailureis now the only classifier reachable from that catch, and it extendsRUNNER_ERROR_RULESinstead of adding a third table.BootFailureReasonwas rejected: it isprovision-kit's boot vocabulary, shared with Android boot, and a build that never produced a binary has no boot to classify. One table is also the only place a row and its reason cannot disagree.Rows carry
buildFailure: { reason, hint }, so reason and advice cannot disagree —signing_no_development_team,signing_provisioning_profile_missing,bundle_identifier_already_registered,signing_unspecified,devtools_security_developer_mode_disabled, andbuild_failed_unclassified, which keepsRUNNER_CACHE_RECOVERY_HINTinstead of claiming a cause. Matching reads only what xcodebuild published — message, stdout, stderr — never the argv we were handed, and never this PR's own emitted reason.signing_style_conflictis withdrawn: conflicting provisioning settings stay unclassified until a capture names the disagreeing setting.resolveRunnerBuildFailureHintis deleted.A named profile has to earn its reason, and in the same line (#2688 review). The shipped sniffer's bare
provisioning profiletrigger is a phrase a failing build prints while talking about something else — the codesign invocation, a settings dump, a note about the profile it used — and a second phrase anywhere in the log is no better, because a profile note above an unrelated expired-certificate warning is two lines that never met. So the table gainstoolTextLineIncludesAll, the profile rows require Xcode's own complaint on the profile's line (IDEProvisioningErrorDomainnaming the profile,doesn't include,has expired), and a mention that says nothing keeps the cache-recovery advice.signing_no_profiles_for_bundle_idshares thePROFILE_UNUSABLEadvice constant rather than spelling the same hint out a third time.Closes #2680. 86 files, 443 production lines.
Validation
At
bcfed35b4c(rebased ontomainfc59620932):build,format,check:quick,check:layering,check:gate-manifest,check:command-docs,check:production-exportsandapple-runner(504) pass, as does theunit-corecli-schema, help-conformance and eager-closure ratchet lane andfallow audit --base origin/mainover the stack. Restoring whole-details matching fails 3 tests — the argv fixture, its focused negative, and self-match; reverting the expired row to a whole-loghas expiredmatch failsprofile-note-above-an-expired-certificate.check:affected --runstays with CI.Risk: live evidence pending — commands in
.device-evidence/CHECKLIST.md. Fixtures declarecaptured,shipped-sniff-triggerorinvented-shapewithxcodeVersion: unobserved, so nothing transcribed reads as observed; the three narrowed profile rows areinvented-shapeuntil Phase B records which line xcodebuild prints, and the checklist now asks for the line breaks because which line carried which phrase is the evidence.