fix(ios): await progress observer completion instead of a 1s poll

[sambitcreate]

Summary

AidenChatTests.testProgressObservationReleasesCompletedHandleAndCanRestart failed on hosted CI for PR #272, which only touches desktop renderer code. Failing job: https://github.com/sambitcreate/aiden-agent/actions/runs/36760403852/job/110041412263

The result bundle shows the failure was in the restart phase:

• Timed out waiting for the progress observer to finish. (the wait at line 312)
• XCTAssertFalse failed at line 313
• The progressRequestCount == 2 assertion passed.

So the second observer had made its request, but it was still running when the poll gave up.

Root cause

The production code has no race here. On a capability denial, observeProgress returns, and finishProgressObservation releases the handle inside the same task body.

The problem was the test helper waitForProgressObservationToStop. It polled isProgressObservationRunning for 100 × 10 ms (about 1 s of wall-clock time), counted from when the stub URL protocol recorded the request. Several steps still happen after that point: the 403 is delivered through URLSession, the SSE Task reads the body, and the error hops back to the main actor. That runner was badly stalled; in the same run, tests that normally take milliseconds took 36 s and 39 s. Under that load the delivery took longer than the 1 s budget.

Reproduced locally: I temporarily held the second /progress/events response for 1.2 s after it was counted. The old test then failed with the same two messages as CI, and the fixed test passed.

RequestDenied: The FBSOpenApplicationServiceErrorDomain … RequestDenied at 19:19 is a separate infrastructure problem. SpringBoard refused to launch the app on a parallel-testing clone ("Clone 2"). That was four minutes after this test had already run and failed on Clone 1, and it has no causal link to the assertion. Both are symptoms of the same overloaded host.

Fix

• Add AidenChatViewModel.waitForProgressObservation(). It awaits the current observer task without cancelling it, like the existing waitForDraftPersistence(). The observer releases its handle inside the task, so after the await, isProgressObservationRunning reflects what the task body actually did, however slowly the host ran.
• The lifecycle test now awaits each observer and asserts the exact request count after each phase (1, then 2). A restart that fails to create a fresh observer still fails the test, and so does a handle that is never released.
• testRemovalCancelsAdmittedConsumerBeforeHeldEventsPublish used the same wall-clock helper and now uses the same await. The helper is removed.
• No timeouts were raised. No retries, sleeps, or skips were added, and no assertions were weakened. The upload-revocation tests and code path touched by fix(ios): join upload revocation claimed by removal cleanup #294 are unchanged.

Validation (local, Xcode beta, iPhone 17 Pro simulator)

• With the 1.2 s injected delay: the old test fails and the new one passes (5 iterations).
• The two affected tests passed 50 iterations each under -run-tests-until-failure, with 40 busy-loop processes saturating the 10-core host.
• The full AidenChatTests class passed: 226 tests, 0 failures.

Android

This is iOS test synchronization plus a read-only await seam. No shared protocol, server contract, or transcript UI changed, so Android needs no change.

🤖 Generated with Claude Code

[pullfrog]

✅ No new issues found.

Reviewed changes This review covers the awaitable progress-observer seam and the affected iOS lifecycle and removal tests.

• Observer completion seam. Adds a main-actor method that awaits the current progress task without cancelling it or starting another observer.
• Test synchronization. Replaces fixed-duration polling in the two affected tests with task completion awaits and exact progress-request count assertions, including the restart case.

  ^{｜ View workflow run ｜ Using GPT Luna ｜ 𝕏}

[very-hermes-bot]

Hermes Review Bot

Confidence: 5

Engine: agy/gemini-3.8-flash-high
Review mode: small
Head: e74d79249cf450af960e5f8a0430cc2768f8fee1
Generated: 2026-09-30T20:22:41+00:00
Reviews: 1

Summary

Eliminates a wall-clock polling flake in iOS chat progress observation tests on resource-constrained CI runners. Replaces the 100 × 10 ms (1 s) polling helper with an explicit waitForProgressObservation() await seam on AidenChatViewModel, which awaits completion of the underlying progressTask. Both testProgressObservationReleasesCompletedHandleAndCanRestart and testRemovalCancelsAdmittedConsumerBeforeHeldEventsPublish now await observer task completion directly and assert exact request counts. Maintainers should double-check that waitForProgressObservation() is only called in tests where the observer terminates on its own (such as under capability denial) to avoid indefinitely awaiting active long-lived streams.

Confidence Score: 5/5

Full trace of task creation, execution, and cleanup on @MainActor confirmed; all call sites and lifecycle states verified.

📁 Important Files Changed

• ios/AidenOnTheGo/Features/Remote/AidenChatFeature.swift: Exposes waitForProgressObservation() to await progressTask?.value without cancelling it, following the established waitForDraftPersistence() pattern.
• ios/AidenOnTheGoTests/AidenChatTests.swift: Updates two lifecycle tests to await observer completion deterministically, checks request counts after each phase, and removes the wall-clock polling helper waitForProgressObservationToStop.

Findings

No findings.

Sequence Diagram

Unchanged or not applicable for this change.

[]

---

Last reviewed commit: e74d79249cf4
Reviews (1) · Comment /hermes review to trigger a new review · /hermes review full for full re-review