Skip to content

fix(ci): repair nightly XCTest and conformance lanes - #1989

Merged
thymikee merged 5 commits into
mainfrom
fix/ci-nightly-repairs
Aug 24, 2026
Merged

fix(ci): repair nightly XCTest and conformance lanes#1989
thymikee merged 5 commits into
mainfrom
fix/ci-nightly-repairs

Conversation

@thymikee

@thymikee thymikee commented Aug 24, 2026

Copy link
Copy Markdown
Member

Summary

  • repair the nightly XCTest expectation and make generated Maestro JSON formatting deterministic
  • make the differential oracle non-vacuous with executable fixture semantics, typed agent-device failure provenance, and fail-closed untyped Maestro exits
  • stabilize macOS replay lifecycle with confirmed app termination, a unique role=button label=About target, and enough assertion budget for one runner recycle
  • classify the typed device-claim conflict family as infrastructure in replay suites, including wrapped REPLAY_DIVERGENCE results, so test stops retrying live-owner conflicts
  • rebase onto current main and keep fix: stop replay retries after cleanup failures #2001 as the single replay-cleanup owner; the stale competing cleanup implementation and tests were removed

The differential regression runs real temporary engine processes through scenario reporting and a matching waiver. Before the fix it failed because the Maestro exit was classified as behavioral; after the fix the report is infrastructure-failed and remains red.

The device-claim regressions run through the production test --json handler path. Before the fix a typed DEVICE_CLAIM_LIVE_OWNER result retried four times and omitted the infrastructure tag; after the fix it stops after one attempt and serializes infrastructure: true. An untyped, retriable DEVICE_IN_USE result still retries four times and serializes no infrastructure tag; the classifier does not infer a claim from its code or message.

Validation

  • pnpm check:affected --run — all 34 runnable local checks passed (54 selected; GitHub-authoritative native/device checks skipped locally), including 3,323 related tests
  • pnpm maestro:conformance — 54/54 passed
  • focused device-claim, test-suite classification, and file-size ratchet tests — 42/42 passed
  • focused macOS lifecycle/replay cleanup/failure-classification tests — 103/103 passed
  • macOS command coverage contract — 7/7 passed
  • live macOS replay with CI retry settings — passed; the first attempt exposed a runner recycle exceeding the old 5s post-click budget
  • live macOS replay with suite retries disabled — passed twice consecutively in one attempt (50.8s and 46.5s)
  • planted red — the real engine-to-report waiver regression failed on pre-fix classification (behavioral instead of infrastructure)
  • planted red — the production test --json regression retried four times and omitted infrastructure before the device-claim fix
  • planted red — the direct classifier incorrectly treated untyped DEVICE_IN_USE as infrastructure before the code-wide shortcut was removed
  • exact-head CI — both coverage shards, Coverage Report, Repo Guards, integration, and all four platform smoke jobs passed

Native XCTest, differential-device, regeneration, mutation, and full-coverage lanes remain GitHub-authoritative.

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.41 MB 2.40 MB -1.3 kB
JS gzip 805.6 kB 805.7 kB +134 B
npm tarball 930.5 kB 930.1 kB -417 B
npm unpacked 3.23 MB 3.23 MB -3.1 kB

npm unpacked components

Component Base Current Diff
JS / dist source 2.56 MB 2.56 MB -3.1 kB
Apple runner source/project 570.6 kB 570.6 kB 0 B
macOS helper source 54.5 kB 54.5 kB 0 B
Android helper artifacts 0 B 0 B 0 B
Other package files 44.7 kB 44.7 kB -38 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.7 ms 27.9 ms +1.2 ms
CLI --help 81.3 ms 81.3 ms -0.0 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/session-snapshot.js -3.7 kB -1.2 kB
dist/src/snapshot.js +2.2 kB +642 B
dist/src/session2.js +177 B +235 B
dist/src/cli-help.js +305 B +125 B
dist/src/registry.js +5 B +121 B

Top changed packed files

Packed file Base Current Diff
dist/src/android-system-chrome.js 4.2 kB 401 B -3.8 kB
dist/src/session-snapshot.js 38.6 kB 34.9 kB -3.7 kB
dist/src/snapshot-chrome.js 5.1 kB 7.9 kB +2.8 kB
dist/src/snapshot.js 29.5 kB 31.8 kB +2.2 kB
dist/src/sdk-contracts.d.ts 18.1 kB 16.3 kB -1.8 kB
dist/src/snapshot-capture-annotations.js 0 B 843 B +843 B
dist/src/sdk-contracts.js 11.0 kB 10.5 kB -452 B
dist/src/snapshot-private-evidence.js 365 B 0 B -365 B
dist/src/cli-help.js 88.9 kB 89.2 kB +305 B
dist/src/app-resolution.js 6.0 kB 6.3 kB +261 B

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-24 17:33 UTC

@thymikee

Copy link
Copy Markdown
Member Author

Exact head bba433f7 reviews clean. The XCTest prefix expectation is corrected to the actual 16-character common prefix; generated Maestro JSON now goes through the repository-pinned Oxfmt via execFileSync with a focused byte-shape test; and the settle differential now taps an always-visible Settings control while preserving the 2-second timing invariant. All current PR checks are green, including Maestro conformance, Swift host XCTests, coverage, integration, and platform smoke. Scheduled nightly/regeneration lanes have not yet run this head, which is residual scheduled evidence rather than a code blocker. Ready for human review.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 24, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Request changes at 34eeb13.

  1. The exact failed macOS Smoke path is still classified as behavioral. Attempt 3 hit DEVICE_IN_USE because attempt 2 retained host-macos-local; the response preserves typed details.reason: DEVICE_CLAIM_LIVE_OWNER, but isReplayInfrastructureFailure only recognizes boot reasons, timeout_cleanup_pending, and message patterns. Add typed device-claim infrastructure classification at the owner seam (at minimum DEVICE_CLAIM_LIVE_OWNER) and a regression through the replay-suite result / test --json path. Treat this failed job as infrastructure/cleanup evidence, not Maestro behavior.

  2. Maestro ordinary nonzero exits are always called behavioral without structured provenance. That permits a future known-divergence waiver to absorb a device/tool failure that exits 1. Until Maestro provides a structured distinction, classify an unstructured nonzero result as infrastructure (still red), or obtain a structured signal; add the negative waiver-path regression.

The additive JSON tag, fail-closed behavior once tagged, and module split otherwise look sound. macOS Smoke is red and iOS Smoke remains pending, so this head is not merge-ready.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 24, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Request changes at b1a4d46.

The macOS close/cleanup delta is meaningful: it waits for observed app termination, promotes cleanup failures to typed test_cleanup_failed, and the new macOS smoke is green. It still does not resolve the two prior classifier blockers:

  1. DEVICE_CLAIM_LIVE_OWNER is absent from isReplayInfrastructureFailure, so a foreign/live-owner claim during an attempt remains behavioral/retriable. Wrapping a retained claim found during final cleanup as test_cleanup_failed does not cover that original typed attempt failure. Add the typed device-claim family at its owner seam and prove it through test --json.
  2. runEngine still calls every ordinary nonzero Maestro exit behavioral solely because spawn returned a numeric status, with no structured oracle result. Fail closed as infrastructure until structured Maestro provenance exists, and prove a nonzero tool/device failure cannot match a divergence waiver.

Current deterministic CI is green; Android/iOS/Coverage are still pending. No readiness label.

@thymikee
thymikee force-pushed the fix/ci-nightly-repairs branch from b1a4d46 to 7041e9f Compare August 24, 2026 10:06
@thymikee

Copy link
Copy Markdown
Member Author

Request changes at exact head 7041e9ff395ee31de2291743d7988e727ee6142b:

  1. P1: the changed macOS replay broadened click "role=button label=About" to click "label=About". Exact-head macOS Smoke fails all three attempts because that selector matches two distinct actionable elements. Restore a semantic selector that uniquely identifies the intended button and prove the replay on the rebuilt head; this is a PR-introduced behavioral failure, not infrastructure.

  2. P1: packages/maestro/test/conformance/differential/engine-process.ts still classifies every ordinary nonzero Maestro exit as behavioral without structured provenance. That permits a device/tool failure to satisfy a known-divergence waiver. Fail closed as infrastructure until Maestro supplies a structured distinction, and add a negative regression through the real engine/waiver path; manually injecting failureKind: infrastructure does not prove the producer.

The head is stale against current main and the PR spans several separable cleanup families. Rebase after fixing, rerun exact-head macOS and combined gates, and consider splitting generated-JSON/XCTest/CLI cleanups from the replay-classification owner change.

@thymikee
thymikee force-pushed the fix/ci-nightly-repairs branch from 7041e9f to 0eb4414 Compare August 24, 2026 15:10
@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 0eb4414880e63055b4c771600d875e884613cad8: the unique About-button selector and fail-closed Maestro nonzero classification are fixed, and all current checks are green. One blocker remains: the original failed macOS path emits code: DEVICE_IN_USE with details.reason: DEVICE_CLAIM_LIVE_OWNER, but isReplayInfrastructureFailure still does not classify that code/reason; this head adds IOS_RUNNER_OWNED_BY_OTHER_DAEMON, which comes from the runner-lease path instead. A live device-claim conflict during an attempt therefore still becomes behavioral/retriable and can be misreported to the differential oracle. Please classify the typed device-claim family (preferably the owning DEVICE_IN_USE code, or all three DEVICE_CLAIM_* reasons) and add the production-route test --json regression. No readiness label yet.

@thymikee
thymikee force-pushed the fix/ci-nightly-repairs branch from b755672 to f400450 Compare August 24, 2026 16:24
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in f400450b5.

The production test --json route now treats both direct DEVICE_IN_USE failures and wrapped, typed DEVICE_CLAIM_* reasons as infrastructure. The planted regression reproduced the original behavior before the fix: four attempts and no infrastructure tag. It now stops after one attempt and serializes infrastructure: true; a same-message failure without typed provenance remains behavioral.

Validation: focused tests 36/36; pnpm check:affected --run passed all 34 runnable local checks (54 selected, 3,321 related tests). A second full run reached the same related-test sweep but hit an unrelated Android provider timeout/worker exit; that file passed 12/12 immediately in isolation. Fresh exact-head CI is running.

@thymikee

Copy link
Copy Markdown
Member Author

Not ready: isReplayInfrastructureFailure now treats every DEVICE_IN_USE as infrastructure, even without typed claim provenance. That code also represents ordinary session/lease busy conflicts, which must remain behavioral/retriable. Classify only the three typed claim reasons through isDeviceClaimConflictReason; remove the code-wide shortcut and add a negative production-route test showing DEVICE_IN_USE without a claim reason remains non-infrastructure/retriable. The positive test --json regression for DEVICE_CLAIM_LIVE_OWNER is otherwise good.

@thymikee

Copy link
Copy Markdown
Member Author

Exact head f400450b530fa749606b7ed9fd39f0bd125c52f0 is all green. macOS, Linux, Android, and iOS smoke passed, as did coverage, integration, typecheck/package, repo guards, compatibility/provenance, lint/format, and CodeQL. The PR reports a clean merge state.

@thymikee
thymikee force-pushed the fix/ci-nightly-repairs branch from f400450 to 6b7f965 Compare August 24, 2026 16:59
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in 6b7f965ce.

  • removed the code-wide DEVICE_IN_USE shortcut; device claims now classify as infrastructure only through the three typed DEVICE_CLAIM_* reasons
  • added a direct planted-red predicate regression (true !== false before the fix) for untyped, retriable DEVICE_IN_USE
  • added the requested production test --json route regression: the untyped failure is retried four times, preserves retriable: true, and serializes no infrastructure tag
  • retained the positive typed DEVICE_CLAIM_LIVE_OWNER route regression

Validation: focused tests 38/38; typecheck and format passed; pnpm check:affected --run passed all runnable checks with 3,323 related tests. Fresh exact-head CI is running.

@thymikee

Copy link
Copy Markdown
Member Author

Re-review clean at 6b7f965c: the broad DEVICE_IN_USE shortcut is gone; only typed device-claim reasons classify as infrastructure. The negative predicate and production test --json regression prove untyped retriable busy failures keep all retries and no infrastructure tag. Code is ready for human review; remaining CI is still running.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 24, 2026
@thymikee
thymikee force-pushed the fix/ci-nightly-repairs branch from 6b7f965 to 37d7cfe Compare August 24, 2026 17:12
@thymikee

Copy link
Copy Markdown
Member Author

Coverage-ratchet follow-up is fixed in 37d7cfe02.

The new production-route regressions had pushed session-test-suite.test.ts to 1,003 lines. I split the typed/untyped infrastructure cases into session-test-suite-infrastructure.test.ts and extracted their two shared helpers into a sibling fixture module; the original suite is now 899 lines, with no ratchet pin or waiver.

The exact test-file-size-ratchet gate and focused behavior suite pass (42/42), typecheck passes, and the full affected run passed every relevant check except the known Android provider timeout/worker-exit contention; that provider file passed 12/12 immediately in isolation. Fresh exact-head CI is running.

@thymikee
thymikee merged commit 893ce4b into main Aug 24, 2026
22 checks passed
@thymikee
thymikee deleted the fix/ci-nightly-repairs branch August 24, 2026 17:29
@thymikee

Copy link
Copy Markdown
Member Author

Exact-head follow-up for 37d7cfe02957ce6c3fa70d75cf595efa68bb4404:

  • removed the code-wide DEVICE_IN_USE infrastructure shortcut; only typed reasons accepted by isDeviceClaimConflictReason classify a device claim as infrastructure
  • added a planted-red direct classifier regression plus production test --json coverage proving an untyped, retriable DEVICE_IN_USE remains non-infrastructure and performs all four attempts
  • retained the positive production-route proof that typed DEVICE_CLAIM_LIVE_OWNER stops after one attempt and serializes infrastructure: true
  • fixed the Coverage (1) file-size ratchet by extracting the two production-route cases into session-test-suite-infrastructure.test.ts and shared setup into session-test-suite.fixtures.ts; the original suite is now 899 lines, without a pin or waiver
  • focused classifier/production-route/ratchet validation: 42/42 passed
  • exact-head CI: both coverage shards, Coverage Report, Repo Guards, Integration Tests, and all four platform Smoke Tests passed; iOS included the live Settings replay, fixture E2E, gesture replay, and host-focus assertion

The worktree is clean and its HEAD exactly matches the PR head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant