From c61d99d4f1cacc976f0764c7be273b8f1243c6bd Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Wed, 19 Aug 2026 00:42:49 -0400 Subject: [PATCH 1/3] ci: build and test the iOS app on every PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The iOS app was the only ADE surface with zero CI coverage; three consecutive merges (#1117, #1120, #1121) each landed a Swift compile break, and 6 ADETests failures accumulated invisibly. New test-ios job on macos-26 (Xcode 26) builds the ADE scheme for testing and runs ADETests. It always runs — ci-pass deliberately counts "skipped" as failure — but exits success immediately on PRs that don't touch apps/ios/** or ci.yml, so non-iOS PRs pay only runner spin-up. SPM packages cached on Package.resolved. Make the suite it gates green (1494 tests, 0 failures locally): - Three PR-list tests still built 'queue' group fixtures; queue workflows were removed in 1b3d33b93 and the joins narrowed to integration groups. Fixtures now use 'integration'; the scoping/filter subjects and every other assertion are unchanged. - testFilterPullRequestListItemsMatchesStateAndSearch asserted a search for "review" returns one row, but both fixtures contain "review" in title/branch — wrong since the day it landed (4f1896050); state narrowing is covered by the following assertions. - testRosterCleanExitAndLegacyPayloadRemainCompatible expected clean exit to settle; 31bac9b8a (#951) made settle declared-only. Expect .ended and additionally pin exitCode == 0. - testRelayCandidateRuntimeIgnoresReadyBeforeAccepted raced a real 350 ms negotiation deadline against the host scheduler. The budget is now a SyncConnectionRaceBudget field (production defaults byte-identical) and the test hook widens only that window; assertions untouched. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 98 +++++++++++++++++++ .../ios/ADE/Services/SyncConnectionRace.swift | 8 +- apps/ios/ADE/Services/SyncService.swift | 14 ++- apps/ios/ADETests/ADETests.swift | 17 +++- 4 files changed, 128 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 78eafed0a..9d5f7e98c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -575,6 +575,103 @@ jobs: src/main/services/sync/syncHostService.test.ts src/main/services/sync/syncService.test.ts + # ── iOS build + unit tests ──────────────────────────────────────────── + # The iOS app is otherwise the only ADE surface with zero CI coverage, and + # three consecutive merges (#1117, #1120, #1121) each landed a Swift compile + # break that stayed invisible until someone built locally. This job keeps the + # ADE scheme compiling and ADETests green on every change that can affect + # them. + # + # It always RUNS (ci-pass treats "skipped" as failure, deliberately), but on + # pull requests that do not touch apps/ios/** or this workflow it exits + # success immediately, so non-iOS PRs pay only runner spin-up. Pushes to main + # and workflow_dispatch always execute the full build + test. + test-ios: + runs-on: macos-26 + timeout-minutes: 45 + steps: + - uses: actions/checkout@v4 + with: + # Depth 2 keeps the PR merge commit's parents available, so the + # gating step below can diff the merge against its base parent. + fetch-depth: 2 + + - name: Decide whether iOS is affected + id: gate + run: | + set -euo pipefail + if [[ "${{ github.event_name }}" != "pull_request" ]]; then + echo "run=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + # checkout@v4 checks out the PR merge commit; diffing it against its + # base parent yields exactly the changes this PR introduces. + changed=$(git diff --name-only HEAD^1 HEAD || true) + if printf '%s\n' "$changed" | grep -qE '^(apps/ios/|\.github/workflows/ci\.yml)'; then + echo "run=true" >> "$GITHUB_OUTPUT" + else + echo "run=false" >> "$GITHUB_OUTPUT" + echo "No apps/ios changes in this PR; skipping the iOS build." + fi + + - name: Select Xcode 26 + if: steps.gate.outputs.run == 'true' + run: | + set -euo pipefail + latest=$(ls -d /Applications/Xcode_26*.app 2>/dev/null | sort -V | tail -1 || true) + if [[ -n "$latest" ]]; then + sudo xcode-select -s "$latest/Contents/Developer" + fi + xcodebuild -version + + - name: Cache Swift packages + if: steps.gate.outputs.run == 'true' + uses: actions/cache@v4 + with: + path: ~/spm-packages + key: spm-v1-${{ hashFiles('apps/ios/ADE.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved') }} + restore-keys: spm-v1- + + - name: Build for testing + if: steps.gate.outputs.run == 'true' + run: | + set -euo pipefail + cd apps/ios + xcodebuild \ + -project ADE.xcodeproj \ + -scheme ADE \ + -destination 'generic/platform=iOS Simulator' \ + -derivedDataPath "$RUNNER_TEMP/ios-dd" \ + -clonedSourcePackagesDirPath ~/spm-packages \ + -skipMacroValidation \ + -skipPackagePluginValidation \ + CODE_SIGNING_ALLOWED=NO \ + build-for-testing | tail -40 + + - name: Run ADETests + if: steps.gate.outputs.run == 'true' + run: | + set -euo pipefail + cd apps/ios + dest_id=$(xcrun simctl list devices available --json \ + | jq -r '[.devices | to_entries[] | select(.key | contains("iOS")) | .value[] | select(.isAvailable and (.name | startswith("iPhone")))][0].udid') + if [[ -z "$dest_id" || "$dest_id" == "null" ]]; then + echo "::error::No available iPhone simulator on this runner image" + xcrun simctl list devices + exit 1 + fi + xcodebuild \ + -project ADE.xcodeproj \ + -scheme ADE \ + -destination "platform=iOS Simulator,id=$dest_id" \ + -derivedDataPath "$RUNNER_TEMP/ios-dd" \ + -clonedSourcePackagesDirPath ~/spm-packages \ + -skipMacroValidation \ + -skipPackagePluginValidation \ + -parallel-testing-enabled NO \ + CODE_SIGNING_ALLOWED=NO \ + test-without-building 2>&1 | tail -60 + validate-docs: needs: install runs-on: ubuntu-latest @@ -625,6 +722,7 @@ jobs: - build - build-runtime-binaries - windows-foundation + - test-ios - validate-docs runs-on: ubuntu-latest steps: diff --git a/apps/ios/ADE/Services/SyncConnectionRace.swift b/apps/ios/ADE/Services/SyncConnectionRace.swift index 89b3eabfb..db8475c65 100644 --- a/apps/ios/ADE/Services/SyncConnectionRace.swift +++ b/apps/ios/ADE/Services/SyncConnectionRace.swift @@ -54,6 +54,12 @@ enum SyncConnectionRaceTiming { struct SyncConnectionRaceBudget: Equatable, Sendable { var overallNanoseconds: UInt64 var relayReadyAfterAcceptedNanoseconds: UInt64 + /// Deadline for `accepted`. Carried on the budget rather than read straight + /// off `SyncConnectionRaceTiming` so a caller that is not racing a real + /// socket — a test feeding pre-buffered frames — can widen the window without + /// changing what either production budget waits. + var relayAcceptedNegotiationNanoseconds: UInt64 = SyncConnectionRaceTiming + .relayAcceptedNegotiationNanoseconds static let standard = SyncConnectionRaceBudget( overallNanoseconds: SyncConnectionRaceTiming.overallBudgetNanoseconds, @@ -218,7 +224,7 @@ struct SyncRelayReadyNegotiation: Equatable { var phaseBudgetNanoseconds: UInt64 { acceptedV2 ? budget.relayReadyAfterAcceptedNanoseconds - : SyncConnectionRaceTiming.relayAcceptedNegotiationNanoseconds + : budget.relayAcceptedNegotiationNanoseconds } func negotiationWindowExpired() -> SyncRelayReadyNegotiationDecision { diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index c0b7996c4..ea1bd0714 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -15806,9 +15806,10 @@ final class SyncService: ObservableObject { /// extends the budget instead of the socket being abandoned. @discardableResult private func awaitRelayCandidateReady( - mailbox: SyncConnectionRaceTextMailbox + mailbox: SyncConnectionRaceTextMailbox, + budget: SyncConnectionRaceBudget? = nil ) async throws -> SyncRelayReadyNegotiation { - var negotiation = SyncRelayReadyNegotiation(budget: connectAttemptBudget) + var negotiation = SyncRelayReadyNegotiation(budget: budget ?? connectAttemptBudget) var deadlineUptime = ProcessInfo.processInfo.systemUptime + TimeInterval(negotiation.phaseBudgetNanoseconds) / 1_000_000_000 @@ -17343,7 +17344,14 @@ final class SyncService: ObservableObject { guard let text = String(data: data, encoding: .utf8) else { continue } await mailbox.deliver(text) } - return try await awaitRelayCandidateReady(mailbox: mailbox) + // Every frame is already buffered, so the pre-`accepted` deadline is timing + // nothing real here — it only races the test host's scheduler, and a loaded + // machine can burn the 350ms production window between reading two frames + // that were delivered instantly. Widen that one window so the runtime's + // ordering rules are what the test measures. + var budget = connectAttemptBudget + budget.relayAcceptedNegotiationNanoseconds = 30_000_000_000 + return try await awaitRelayCandidateReady(mailbox: mailbox, budget: budget) } func completeCapturedRefreshRequestsForTesting() { diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index 824c3a8e6..6ccb42e34 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -6432,8 +6432,8 @@ final class ADETests: XCTestCase { ('pr-one', '2026-04-22T00:30:00.000Z'), ('pr-two', '2026-04-22T00:40:00.000Z'); insert into pr_groups(id, project_id, group_type, name, target_branch, created_at) values - ('group-one', 'project-1', 'queue', 'Project one queue', 'main', '2026-04-22T00:30:00.000Z'), - ('group-two', 'project-2', 'queue', 'Project two queue', 'main', '2026-04-22T00:40:00.000Z'); + ('group-one', 'project-1', 'integration', 'Project one integration', 'main', '2026-04-22T00:30:00.000Z'), + ('group-two', 'project-2', 'integration', 'Project two integration', 'main', '2026-04-22T00:40:00.000Z'); insert into pr_group_members(id, group_id, pr_id, lane_id, position, role) values ('member-one', 'group-one', 'pr-one', 'lane-one', 0, 'source'), ('member-two', 'group-two', 'pr-two', 'lane-two', 0, 'source'); @@ -11560,7 +11560,11 @@ final class ADETests: XCTestCase { ), ] - XCTAssertEqual(filterPullRequestListItems(items, query: "review", state: .all).map(\.id), ["pr-1"]) + // Search is a substring match over title/branches/lane/repo, so "review" + // matches pr-1 ("Improve review timeline") and pr-2 ("Draft review + // workflow") while excluding pr-3 — narrowing to one row is the job of the + // state filter, asserted below. + XCTAssertEqual(filterPullRequestListItems(items, query: "review", state: .all).map(\.id), ["pr-1", "pr-2"]) XCTAssertEqual(filterPullRequestListItems(items, query: "", state: .draft).map(\.id), ["pr-2"]) XCTAssertEqual(filterPullRequestListItems(items, query: "cleanup", state: .merged).map(\.id), ["pr-3"]) XCTAssertEqual(filterPullRequestListItems(items, query: "", state: .open).map(\.id), ["pr-1"]) @@ -13814,7 +13818,7 @@ final class ADETests: XCTestCase { try database.executeSqlForTesting(""" insert into pr_groups(id, project_id, group_type, name, target_branch, created_at) - values ('group-1', 'project-1', 'queue', 'Queue rollout', 'main', '2026-03-17T00:15:00.000Z'); + values ('group-1', 'project-1', 'integration', 'Integration rollout', 'main', '2026-03-17T00:15:00.000Z'); """) try database.executeSqlForTesting(""" insert into pr_group_members(id, group_id, pr_id, lane_id, position, role) @@ -25052,12 +25056,15 @@ final class RosterDeltaTests: XCTestCase { } """.utf8) let cleanExit = try JSONDecoder().decode(RemoteRosterChat.self, from: cleanExitData) + XCTAssertEqual(cleanExit.exitCode, 0) + // Process completion is not a lifecycle declaration: settle is declared-only + // (`settledAt`), so a clean exit with no declaration rests at `.ended`. XCTAssertEqual( workCanonicalSessionState( session: cleanExit.asTerminalSessionSummary(laneName: "Feature"), summary: nil ).phase, - .settled + .ended ) let legacyData = Data(""" From 9c2cf3bbb9b8df06bc78c9c2d01399aafe2e41a3 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Wed, 19 Aug 2026 00:57:38 -0400 Subject: [PATCH 2/3] ci: let the iOS test host sign ad-hoc; surface full failure detail CODE_SIGNING_ALLOWED=NO left the test host unsigned, so simulator keychain access failed with missing-entitlement errors in the account sign-out and DPoP proof tests (they pass locally, where the host signs ad-hoc). Also replace output truncation with -quiet and upload the .xcresult bundle on failure so CI failures are diagnosable. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9d5f7e98c..63bfaa3ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -645,8 +645,8 @@ jobs: -clonedSourcePackagesDirPath ~/spm-packages \ -skipMacroValidation \ -skipPackagePluginValidation \ - CODE_SIGNING_ALLOWED=NO \ - build-for-testing | tail -40 + -quiet \ + build-for-testing - name: Run ADETests if: steps.gate.outputs.run == 'true' @@ -669,8 +669,17 @@ jobs: -skipMacroValidation \ -skipPackagePluginValidation \ -parallel-testing-enabled NO \ - CODE_SIGNING_ALLOWED=NO \ - test-without-building 2>&1 | tail -60 + -quiet \ + test-without-building + + - name: Upload test results on failure + if: failure() && steps.gate.outputs.run == 'true' + uses: actions/upload-artifact@v4 + with: + name: ios-test-results + path: ${{ runner.temp }}/ios-dd/Logs/Test/*.xcresult + if-no-files-found: ignore + retention-days: 7 validate-docs: needs: install From 8a35120fd009d629a0a624855688a1bac97e12e3 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Wed, 19 Aug 2026 01:14:11 -0400 Subject: [PATCH 3/3] =?UTF-8?q?ci:=20address=20review=20=E2=80=94=20persis?= =?UTF-8?q?t-credentials=20off,=20timeout=20test=20keeps=20a=20real=20wind?= =?UTF-8?q?ow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test-ios checkout no longer persists the GITHUB_TOKEN into .git/config; xcodebuild runs PR-controlled build phases and needs no authenticated git. - awaitRelayCandidateReadyForTesting takes an acceptedWindowNanoseconds override; the negotiation-timeout test passes 50ms so it exercises the timeout path without sitting out the wide scheduling-safe window (SyncRecoveryPolicyTests back to ~9s). Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 3 +++ apps/ios/ADE/Services/SyncService.swift | 10 ++++++---- apps/ios/ADETests/SyncRecoveryPolicyTests.swift | 7 ++++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 63bfaa3ea..a4192b883 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -595,6 +595,9 @@ jobs: # Depth 2 keeps the PR merge commit's parents available, so the # gating step below can diff the merge against its base parent. fetch-depth: 2 + # xcodebuild executes PR-controlled build phases; it never needs + # authenticated git, so don't leave the token in .git/config. + persist-credentials: false - name: Decide whether iOS is affected id: gate diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index ea1bd0714..f950a21b5 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -17336,7 +17336,8 @@ final class SyncService: ObservableObject { } func awaitRelayCandidateReadyForTesting( - frames: [[String: Any]] + frames: [[String: Any]], + acceptedWindowNanoseconds: UInt64? = nil ) async throws -> SyncRelayReadyNegotiation { let mailbox = SyncConnectionRaceTextMailbox() for frame in frames { @@ -17347,10 +17348,11 @@ final class SyncService: ObservableObject { // Every frame is already buffered, so the pre-`accepted` deadline is timing // nothing real here — it only races the test host's scheduler, and a loaded // machine can burn the 350ms production window between reading two frames - // that were delivered instantly. Widen that one window so the runtime's - // ordering rules are what the test measures. + // that were delivered instantly. Widen that one window by default so the + // runtime's ordering rules are what the test measures; a timeout-path test + // passes a tiny window instead so it does not sit out the wide one. var budget = connectAttemptBudget - budget.relayAcceptedNegotiationNanoseconds = 30_000_000_000 + budget.relayAcceptedNegotiationNanoseconds = acceptedWindowNanoseconds ?? 30_000_000_000 return try await awaitRelayCandidateReady(mailbox: mailbox, budget: budget) } diff --git a/apps/ios/ADETests/SyncRecoveryPolicyTests.swift b/apps/ios/ADETests/SyncRecoveryPolicyTests.swift index ac0e1c6e6..3de30a350 100644 --- a/apps/ios/ADETests/SyncRecoveryPolicyTests.swift +++ b/apps/ios/ADETests/SyncRecoveryPolicyTests.swift @@ -1142,7 +1142,12 @@ final class SyncRecoveryPolicyTests: XCTestCase { } do { - try await service.awaitRelayCandidateReadyForTesting(frames: []) + // A short real window: this test IS the timeout path, so it must not sit + // out the wide scheduling-safe window the ordering tests use. + try await service.awaitRelayCandidateReadyForTesting( + frames: [], + acceptedWindowNanoseconds: 50_000_000 + ) XCTFail("A ready-v2 timeout must require a fresh legacy socket.") } catch let error as SyncRelayReadyNegotiationError { XCTAssertEqual(error, .retryLegacySocket)