Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The developer wanted to diagnose why iOS real-device runs with recording enabled in a test YAML produced no video artifact, then fix the underlying issue. They confirmed a solution that separates the app target device identifier from the recording device identifier, pairs iOS physical devices by exact name when IDs differ, and uses the recorder’s actual device selector for video capture while keeping Flutter/app launch on the Flutter UDID. They then requested implementation with staged commits, including a prepared iOS capture lifecycle where recording hardware is opened before app launch and the video segment starts only after the runtime is ready. After the fix was committed in phases, they asked for a standalone document teaching another AI the iOS “preheated recording” solution, and explicitly required that the document not include project-specific code context or terminology.
What Changed
Risk Assessment
Testing
I inspected the recording-focused change, ran the targeted Flutter Pilot and screen_recorder Dart tests successfully, attempted a real prepared physical-iOS recording for end-user video evidence, and captured the setup-level failure showing the iPhone is visible to Flutter/xctrace but unavailable to screen_recorder discovery; no linters, formatters, or static analysis were run.
Evidence: Flutter Pilot targeted recording tests transcript
Evidence: screen_recorder prepared capture tests transcript
Evidence: Physical iOS recorder focused tests transcript
Evidence: Live physical iOS prepared recording attempt
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
⏭️ **Review** - skipped
packages/screen_recorder/lib/src/service/screen_recorder_service.dart:243-dispose()drops a prepared capture without handling an active session. For backends that returnbackendPrepared == false(Android and iOS simulator through the composite prepared path), a session started from that prepared capture is a normal backend recording, so cleanup during interruption or early session close can leave the recording process/device capture running and the output untracked. Dispose should stop/discard anyactiveSessionIdbefore removing the prepared state, or the controller should discard its active_sessionbefore disposing the capture.packages/screen_recorder/lib/src/backend/ios_physical_recording_backend.dart:116- If the physical iOS helper starts but never emits the expectedreadyevent,prepare()leaves the process in_preparedCapturesand rethrows.ScreenRecorderService.prepare()only records the capture after backend prepare succeeds, so the caller has no handle to dispose and the helper can keep the device capture open. Wrap the readiness wait with cleanup that removes the state and terminates/drains the helper before rethrowing.🔧 Fix: Fix prepared recording cleanup leaks
2 issues (1 error, 1 warning) still open:
packages/screen_recorder/lib/src/backend/ios_physical_recording_backend.dart:77- Standalone physical iOSstartRecordcan leak the prepared helper whenstartRecord(capture, session, ...)fails afterprepare(capture)succeeds. The service rolls back the active session, but the caller never receives a session or capture handle, while_standaloneCaptures/_preparedCapturesstill own the serve process. Wrap this call in cleanup that removes the standalone mapping and disposes the capture before rethrowing; apply the same ownership-preserving cleanup pattern to standalone stop/discard failures where the mapping is removed before the helper is shut down.lib/src/execution/test_execution_session.dart:350-close()now letsRecordingExceptionfromrecordingController.dispose()escape directly. InDefaultTestCommandExecutorandDefaultProjectRunExecutor, this runs fromfinally, so a dispose failure can mask a successful run, hide the original launch/scenario failure, and bypass the existingTestCommandException/ environment-failure rendering paths. Convert dispose failures toTestExecutionRecordingExceptionor suppress/report them without overriding the primary result.🔧 Fix: Preserve recording cleanup ownership
3 issues (2 errors, 1 warning) still open:
packages/screen_recorder/lib/src/service/screen_recorder_service.dart:264-dispose()marks the capture disposed and removes it from_preparedCapturesbefore backend shutdown succeeds. If physical iOS helper shutdown throws or times out, the public capture handle can no longer be retried through this service, leaving backend-owned capture state/processes orphaned. Keep the prepared state owned until backenddisposesucceeds, and apply the same remove-after-success ownership rule in the composite backend.lib/src/recording/screen_recorder_recording_controller.dart:105- The recording controller clears_captureand sets_disposedbefore_recorder.dispose(capture)succeeds. A close-time dispose failure therefore makes subsequent cleanup attempts no-op at the controller layer, which is especially risky for prepared iOS helpers. Only clear the capture / mark disposed after the recorder dispose completes successfully.lib/src/execution/test_command_executor.dart:108-completedNormallyis set for any returnedScenarioRunReport, including failed reports, so a recording dispose failure infinallystill replaces the Scenario failure with aTestCommandException. Match the project-run behavior by only reporting close/dispose failure when the returned report passed; failed Scenario reports should remain the primary result.🔧 Fix: Preserve recording cleanup ownership
1 warning still open:
packages/screen_recorder/lib/src/backend/ios_physical_recording_backend.dart:390-_waitForEvent()decodes the helper protocol line before entering the existing unexpected-event error path. If the helper writes any non-JSON stdout line while serving, this leaks a rawFormatExceptionout ofstartRecord/stopRecordinstead of aScreenRecorderException, which breaks the backend contract and bypasses the existing user-facing diagnostics. Catch JSON parse failures here and convert them to the samefailureCodewith the raw line attached.⏭️ **Test** - skipped
xcrun xctrace list devicesandflutter devices --machine, but the screen_recorder physical-iOS discovery returned no Recording Devices, so prepared capture failed withScreenRecorderErrorCode.deviceNotFoundbefore a movie could be recorded. The fake-command tests cover the intended ID/name pairing and preheated lifecycle, but a reviewer still needs either a retry on hardware where the physical-iOS capture helper lists the device or explicit acceptance of the simulated evidence for this validation pass.dart test test/target/target_device_resolver_test.dart test/execution/test_command_executor_test.dart test/execution/test_execution_session_test.dart test/recording/screen_recorder_recording_controller_test.dart --reporter expandeddart test packages/screen_recorder/test/ios_physical_recorder_test.dart packages/screen_recorder/test/ios_simulator_recorder_test.dart packages/screen_recorder/test/android_recorder_test.dart packages/screen_recorder/test/screen_recorder_test.dart --reporter expandeddart test packages/screen_recorder/test/ios_physical_recorder_test.dart --reporter expandedxcrun xctrace list devicesflutter devices --machinedart --packages=.dart_tool/package_config.json /var/folders/1b/xbh8qb0d0j11fzyfm0qs0pf40000gn/T/no-mistakes-evidence/01KXD3BTQY6R7BHSD5M5939HBF/physical_ios_preheated_recording.dart✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.