chore: modernize toolchain and harden audio workflows - #838
Conversation
Move the repository to Node 22 and Yarn 4. Refresh the React Native ecosystem and native project settings. Regenerate Nitro artifacts for the updated toolchain.
Serialize Android player replacement and ignore stale callbacks. Use the modern Bluetooth HFP audio-session option on iOS.
Cover native instance creation and the web recorder/player lifecycle. Add guarded Maestro flows and rebuild the example web bundle.
Use the pinned Node and Yarn toolchain across workflows. Preserve Xcode failures and generate robust changelog entries.
Document the verified platform baselines and scoped permissions. Clarify the defensive iOS recorder mitigation and AI context.
Share canonical Codex workflows with lightweight Claude wrappers. Add guarded review, release, rebase, and audio E2E procedures.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds AI maintenance commands and skills, updates CI and toolchain configuration, hardens Android media-player lifecycle handling, changes iOS audio-session options, and adds native, web, and Maestro audio validation. ChangesAI workflows and validation
Audio runtime and application validation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Ensure the published tag includes the generated changelog commit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
example/src/screens/SoundScreen.tsx (1)
175-196: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle rejections from
pauseRecorderandresumeRecorder.Both handlers return a floating promise.
pauseRecorderrejects on Android below API 24 and when no recorder exists, which produces an unhandled rejection during the Maestro flow.example/src/screens/CompatibilityScreen.tsxalready attaches.catch(() => {})to the same calls.🛡️ Proposed fix
- onPress={() => soundRef.current.pauseRecorder()} + onPress={() => { + soundRef.current.pauseRecorder().catch(() => {}); + }} @@ - onPress={() => soundRef.current.resumeRecorder()} + onPress={() => { + soundRef.current.resumeRecorder().catch(() => {}); + }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/src/screens/SoundScreen.tsx` around lines 175 - 196, Update the onPress handlers for the pause and resume controls in SoundScreen so the promises returned by soundRef.current.pauseRecorder() and resumeRecorder() are explicitly handled with a rejection catch, matching the established pattern in CompatibilityScreen. Keep the existing button state, disabled conditions, and recorder calls unchanged..github/workflows/ci-ios.yml (1)
26-29: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRun the generated Swift fix in the iOS workflow.
scripts/fix-nitrogen-swift.shreplacesstd::optionalpointee access innitrogen/generated/ios/swift/AudioSet.swift, but.github/workflows/ci-ios.ymlnow calls onlyyarn preparebefore the build. Add a post-yarn preparestep that runs the script so the simulator build compiles the generated Swift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci-ios.yml around lines 26 - 29, Add a step immediately after the “Build library + generate nitrogen code” step in the iOS workflow to execute scripts/fix-nitrogen-swift.sh before selecting the Xcode version or building, ensuring generated AudioSet.swift receives the required fix.
🧹 Nitpick comments (4)
example/src/screens/CompatibilityScreen.tsx (1)
65-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
requestPermissionshelper across four example screens. The example app has no shared permission module, so each screen repeats the identical microphone request logic. Extract one helper, for exampleexample/src/utils/permissions.ts, and import it in each screen.
example/src/screens/CompatibilityScreen.tsx#L65-L71: replace the local helper with the shared import.example/src/screens/SoundHookStatesScreen.tsx#L46-L52: replace the local helper with the shared import.example/src/screens/SoundHookScreen.tsx#L76-L82: replace the local helper with the shared import.example/src/screens/SoundScreen.tsx#L36-L42: replace the local helper with the shared import.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/src/screens/CompatibilityScreen.tsx` around lines 65 - 71, Extract the duplicated requestPermissions helper into example/src/utils/permissions.ts, preserving its Android microphone permission behavior and non-Android return value. Replace the local helper with an import in example/src/screens/CompatibilityScreen.tsx lines 65-71, example/src/screens/SoundHookStatesScreen.tsx lines 46-52, example/src/screens/SoundHookScreen.tsx lines 76-82, and example/src/screens/SoundScreen.tsx lines 36-42.example/src/screens/HomeScreen.tsx (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse single-quoted JSX attributes.
These new
testIDvalues use double quotes. Use single quotes to match the TypeScript formatting rule.As per coding guidelines,
**/*.{ts,tsx}must use single quotes.Proposed fix
- testID="e2e-home-hook" + testID='e2e-home-hook' - testID="e2e-home-hook-states" + testID='e2e-home-hook-states' - testID="e2e-home-direct" + testID='e2e-home-direct'Also applies to: 48-48, 57-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/src/screens/HomeScreen.tsx` at line 39, Update the testID JSX attributes in the affected HomeScreen elements to use single-quoted values, including the attributes near e2e-home-hook and the other two test IDs, preserving their existing values.Source: Coding guidelines
.github/actions/setup/action.yml (1)
10-17: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winEnable Corepack before resolving the Yarn cache in every workflow.
actions/setup-noderesolves the Yarn cache directory during cache setup, and that command can use the runner’s default Yarn instead of the project toolchain when Corepack is enabled afterward. Movecorepack enablebefore the firstactions/setup-node@v4step that usescache: 'yarn', or remove the setup-node Yarn cache input and add explicit caching after Corepack is enabled. This applies to.github/actions/setup/action.yml,.github/workflows/deploy-web.yml, and.github/workflows/publish-package.yml.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/setup/action.yml around lines 10 - 17, Enable Corepack before any actions/setup-node@v4 step that uses cache: 'yarn' in .github/actions/setup/action.yml (10-17), .github/workflows/deploy-web.yml (20-27), and .github/workflows/publish-package.yml (49-57), or replace those cache inputs with explicit caching configured after Corepack is enabled..codex/skills/nitro-sound-workflows/SKILL.md (1)
16-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover all native implementation files in the workflow checks.
The current list checks only
ios/*.swiftandandroid/src/main/**/*.kt. It omits nested Swift files and the Android C++ adapter and headers. The parity rule also omits C++. A workflow can miss a native bridge change and still report complete review coverage.Use recursive native paths and include C++ behavior in the parity check. As per coding guidelines, Android native bridge code includes
android/**/*.{kt,cpp,h,hpp}.Proposed fix
-- Native behavior: `ios/*.swift` and `android/src/main/**/*.kt` +- Native behavior: `ios/**/*.swift` and `android/**/*.{kt,cpp,h,hpp}` ... -- Keep TypeScript, Web, Swift, and Kotlin behavior aligned unless the public +- Keep TypeScript, Web, Swift, Kotlin, and C++ behavior aligned unless the public documentation explicitly identifies a platform limitation.Also applies to: 53-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.codex/skills/nitro-sound-workflows/SKILL.md around lines 16 - 18, Update the native implementation and parity-check path lists in the workflow guidance to recursively cover Android bridge files matching android/**/*.{kt,cpp,h,hpp} and nested Swift files under ios, ensuring both native implementation checks and parity checks include C++ behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/commands/commit.md:
- Line 220: Update the Markdown fence opening the dependency-bump commit example
in commit documentation to specify the text language, changing the untagged
fence to a text-tagged fence while leaving the example content unchanged.
In @.claude/commands/compile-knowledge.md:
- Line 35: Update the generated-template instructions near the source collection
and enumeration steps to reference the new TypeScript entry points, especially
src/index.tsx and src/index.web.tsx, instead of the stale src/index.ts path;
keep the collected source list and generated enumeration instructions
consistent.
In @.codex/scripts/install-skills.sh:
- Around line 10-20: Validate that each canonical skill source under
.codex/skills exists before creating its link in the installation loop. If a
source is missing, report the skill installation failure and exit nonzero; only
run ln and the success echo for valid sources.
In @.codex/skills/ios-audio-e2e/SKILL.md:
- Around line 60-61: Update the device-backed iOS E2E instructions near the
installation and launch step to require a physical iPhone exclusively; remove
simulator support from this workflow and direct simulator-only recorder/player
regression runs to the $simulator-audio-e2e skill.
In @.codex/skills/simulator-audio-e2e/references/flow-contract.md:
- Around line 9-13: Update the canonical direct recorder and hook recorder flows
to enforce their documented timing requirements: assert direct recording reaches
at least one second, and assert hook recording reaches four callback-driven
seconds with increasing numeric callback values. Use stable numeric state
assertions rather than only waiting for “Record Progress: Ready”; alternatively
remove those thresholds from the contract if the flows cannot validate them.
- Around line 16-17: Update the Android recorder-stop flow contract so the
success alert itself asserts the saved path while asserting “Recording Stopped,”
before dismissing “OK”; alternatively, revise the documented contract to match
the evidence if the alert does not contain the path. Keep the iOS behavior
unchanged.
In @.codex/skills/simulator-audio-e2e/scripts/preflight.sh:
- Around line 17-23: Update the Android SDK fallback used by the preflight
discovery logic around android_sdk, replacing the macOS-only default with
OS-aware handling that checks standard macOS and Linux SDK locations, or
requires ANDROID_SDK_ROOT/ANDROID_HOME when no supported default exists. Ensure
adb and emulator discovery use the resolved SDK path before reporting Android
targets.
- Around line 4-5: Update the repository-root initialization near BASH_SOURCE[0]
to first resolve the script’s physical directory with pwd -P, then derive
repo_root by ascending from that resolved directory. Keep the subsequent cd
behavior unchanged so package.json and other repository lookups use the actual
repository when invoked through a symlink.
In @.codex/skills/simulator-audio-e2e/SKILL.md:
- Around line 20-21: Update the simulator-audio-e2e workflow instructions to
make recording deletion mandatory: verify the recording is absent from both the
app container and artifact directory before reporting RUNTIME PASS, and report
RUNTIME BLOCKED if cleanup fails or cannot be confirmed. Apply the same
requirement to the corresponding cleanup guidance referenced elsewhere in the
document.
In @.github/workflows/publish-package.yml:
- Around line 49-54: Remove the cache: 'yarn' setting from the Setup Node.js
action in the publish workflow, while preserving node-version-file,
registry-url, and the existing yarn install --immutable step.
In @.nvmrc:
- Line 1: Update the .nvmrc runtime pin from the major-only value to Node
22.21.0, matching the package.json engines.node minimum and ensuring NVM and CI
select a compliant patch version.
In `@android/src/main/java/com/margelo/nitro/audiorecorderplayer/Sound.kt`:
- Around line 371-494: Refactor the player setup around the visible start flow
so mediaPlayerLock is never held during setDataSource() or prepare(); configure
and prepare the new player outside the lock, preferably using prepareAsync()
with an on-prepared callback. Publish the prepared player under a short
synchronized section only after verifying its generation is still current,
releasing it when superseded. Remove lock acquisition from the handler.post
start block and retain only a brief generation/player identity check before
player.start(), ensuring listener callbacks and playback controls cannot block
on preparation.
In `@CONTRIBUTING.md`:
- Around line 62-64: Update the ios:pod script in example/package.json to invoke
CocoaPods through Bundler using bundle exec pod install --project-directory=ios
--repo-update, matching the CI command and the repository-pinned dependency
requirements.
In `@example/src/App.tsx`:
- Around line 40-43: Update the root layout in AppInner to apply the bottom
safe-area inset by adding paddingBottom using insets.bottom, while preserving
the existing top inset handling; alternatively, replace the root view with a
safe-area view that applies both edges.
In `@package.json`:
- Line 47: Update the package.json nitrogen script so it runs
scripts/fix-nitrogen-swift.sh after generating bindings, preserving the existing
generation command and ensuring yarn nitrogen applies the Swift repair
automatically.
---
Outside diff comments:
In @.github/workflows/ci-ios.yml:
- Around line 26-29: Add a step immediately after the “Build library + generate
nitrogen code” step in the iOS workflow to execute scripts/fix-nitrogen-swift.sh
before selecting the Xcode version or building, ensuring generated
AudioSet.swift receives the required fix.
In `@example/src/screens/SoundScreen.tsx`:
- Around line 175-196: Update the onPress handlers for the pause and resume
controls in SoundScreen so the promises returned by
soundRef.current.pauseRecorder() and resumeRecorder() are explicitly handled
with a rejection catch, matching the established pattern in CompatibilityScreen.
Keep the existing button state, disabled conditions, and recorder calls
unchanged.
---
Nitpick comments:
In @.codex/skills/nitro-sound-workflows/SKILL.md:
- Around line 16-18: Update the native implementation and parity-check path
lists in the workflow guidance to recursively cover Android bridge files
matching android/**/*.{kt,cpp,h,hpp} and nested Swift files under ios, ensuring
both native implementation checks and parity checks include C++ behavior.
In @.github/actions/setup/action.yml:
- Around line 10-17: Enable Corepack before any actions/setup-node@v4 step that
uses cache: 'yarn' in .github/actions/setup/action.yml (10-17),
.github/workflows/deploy-web.yml (20-27), and
.github/workflows/publish-package.yml (49-57), or replace those cache inputs
with explicit caching configured after Corepack is enabled.
In `@example/src/screens/CompatibilityScreen.tsx`:
- Around line 65-71: Extract the duplicated requestPermissions helper into
example/src/utils/permissions.ts, preserving its Android microphone permission
behavior and non-Android return value. Replace the local helper with an import
in example/src/screens/CompatibilityScreen.tsx lines 65-71,
example/src/screens/SoundHookStatesScreen.tsx lines 46-52,
example/src/screens/SoundHookScreen.tsx lines 76-82, and
example/src/screens/SoundScreen.tsx lines 36-42.
In `@example/src/screens/HomeScreen.tsx`:
- Line 39: Update the testID JSX attributes in the affected HomeScreen elements
to use single-quoted values, including the attributes near e2e-home-hook and the
other two test IDs, preserving their existing values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ceb34baa-b8d8-4a81-8c4b-86b88517fcea
⛔ Files ignored due to path filters (13)
.yarn/plugins/@yarnpkg/plugin-interactive-tools.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-workspace-tools.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-3.6.1.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-4.18.0.cjsis excluded by!**/.yarn/**example/Gemfile.lockis excluded by!**/*.lockexample/dist/bundle.7eed44cf7c19972c4ca6.jsis excluded by!**/dist/**example/dist/bundle.7eed44cf7c19972c4ca6.js.LICENSE.txtis excluded by!**/dist/**example/dist/bundle.afe9605342637d687c15.jsis excluded by!**/dist/**example/dist/index.htmlis excluded by!**/dist/**example/ios/Podfile.lockis excluded by!**/*.lockexample/ios/SoundExample/Images.xcassets/AppIcon.appiconset/Logo.pngis excluded by!**/*.pngnitrogen/generated/ios/swift/AudioSet.swiftis excluded by!**/generated/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (80)
.claude/commands/audit-code.md.claude/commands/commit.md.claude/commands/compile-knowledge.md.claude/commands/e2e-tests.md.claude/commands/release.md.claude/commands/resolve-issue.md.claude/commands/review-pr.md.claude/commands/upgrade-deps.md.claude/commands/verify-all.md.claude/skills/android-audio-e2e/SKILL.md.claude/skills/funding-steward/SKILL.md.claude/skills/generate-doc/SKILL.md.claude/skills/ios-audio-e2e/SKILL.md.claude/skills/nitro-sound-workflows/SKILL.md.claude/skills/rebase-main/SKILL.md.claude/skills/review-self/SKILL.md.claude/skills/simulator-audio-e2e/SKILL.md.codex/scripts/install-skills.sh.codex/skills/android-audio-e2e/SKILL.md.codex/skills/android-audio-e2e/agents/openai.yaml.codex/skills/funding-steward/SKILL.md.codex/skills/funding-steward/agents/openai.yaml.codex/skills/generate-doc/SKILL.md.codex/skills/generate-doc/agents/openai.yaml.codex/skills/ios-audio-e2e/SKILL.md.codex/skills/ios-audio-e2e/agents/openai.yaml.codex/skills/nitro-sound-workflows/SKILL.md.codex/skills/nitro-sound-workflows/agents/openai.yaml.codex/skills/rebase-main/SKILL.md.codex/skills/rebase-main/agents/openai.yaml.codex/skills/review-self/SKILL.md.codex/skills/review-self/agents/openai.yaml.codex/skills/simulator-audio-e2e/SKILL.md.codex/skills/simulator-audio-e2e/agents/openai.yaml.codex/skills/simulator-audio-e2e/references/flow-contract.md.codex/skills/simulator-audio-e2e/scripts/preflight.sh.codex/skills/simulator-audio-e2e/scripts/run-maestro.sh.github/actions/setup/action.yml.github/workflows/ai-dispatch.yml.github/workflows/ci-ios.yml.github/workflows/deploy-web.yml.github/workflows/publish-package.yml.gitignore.nvmrc.yarnrc.ymlAGENTS.mdCHANGELOG.mdCONTRIBUTING.mdREADME.mdandroid/gradle.propertiesandroid/src/main/AndroidManifest.xmlandroid/src/main/java/com/margelo/nitro/audiorecorderplayer/Sound.kte2e/maestro/audio-android.yamle2e/maestro/audio-ios.yamlexample/Gemfileexample/android/app/src/main/AndroidManifest.xmlexample/android/app/src/main/java/sound/example/MainApplication.ktexample/android/gradle/wrapper/gradle-wrapper.propertiesexample/ios/SoundExample.xcodeproj/project.pbxprojexample/ios/SoundExample/Info.plistexample/jest.config.jsexample/package.jsonexample/src/App.tsxexample/src/components/SafeAreaContext.tsxexample/src/screens/CompatibilityScreen.tsxexample/src/screens/HomeScreen.tsxexample/src/screens/SoundHookScreen.tsxexample/src/screens/SoundHookStatesScreen.tsxexample/src/screens/SoundScreen.tsxios/Sound.swiftknowledge/_claude-context/context.mdnitro.jsonpackage.jsonscripts/fix-nitrogen-swift.shscripts/generate-release-notes.jssrc/__tests__/index.test.tsxsrc/__tests__/index.web.test.tssrc/index.web.tsxsrc/specs/Sound.nitro.tsturbo.json
💤 Files with no reviewable changes (2)
- example/ios/SoundExample/Info.plist
- android/src/main/AndroidManifest.xml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-package.yml:
- Around line 115-118: Update the release flow around the tag recreation command
and the subsequent yarn prepare step so preparation runs before recreating the
annotated $NEW_VERSION tag. Ensure any tracked generated changes from
preparation are committed before tagging, or fail the workflow when preparation
leaves an uncommitted tracked diff, so the tag and published package reference
the same tree.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e584c9a-a1c5-4ea0-b054-18d092545383
📒 Files selected for processing (1)
.github/workflows/publish-package.yml
hyochan
added
dependencies
Synchronize Claude and Codex review adapters. Add five-minute polling and review-self fallback rules.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.codex/skills/review-self/SKILL.md (1)
49-66: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep fallback mode to one round.
The fallback contract in
.codex/skills/review-pr/SKILL.mdLines 85-88 requires$review-selfto leave polling with the caller. This section allows five-minute rechecks when monitoring was requested, so a fallback invocation can create a second monitor. Add an explicit fallback-mode guard that disables the stability-recheck section.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.codex/skills/review-self/SKILL.md around lines 49 - 66, Add an explicit fallback-mode guard around the Optional Stability Rechecks section, using the skill’s fallback-for-$review-pr condition, so fallback execution performs exactly one round and never starts monitoring or rechecks. Preserve the existing recurring-monitoring behavior for non-fallback invocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.codex/skills/review-pr/SKILL.md:
- Around line 77-83: Update the reviewer-unavailable fallback logic in the
review workflow so successful terminal reviews are excluded. Only mark the
reviewer unavailable when there is no terminal result or the terminal status
explicitly indicates unavailability, while preserving the existing quota,
billing, rate-limit, permission, service-availability, diff-limit, and two-poll
handling.
- Around line 90-92: Update the fallback coverage cache key to represent the
complete review target, including repository, PR number, base SHA, head SHA,
feedback IDs, and acceptance-criteria fingerprint. Replace the current
reviewer-failure-set/head-SHA-only identity in the fallback coverage logic,
while preserving invalidation when any review target component changes.
- Around line 16-17: Update .codex/skills/review-pr/SKILL.md lines 16-17 to load
AGENTS.md, workflow skills, and path rules only from the trusted base or
installed revision. In .codex/skills/review-pr/SKILL.md lines 30-35, prohibit
commands or permission changes based solely on PR-controlled evidence. Apply the
same trusted-source rule in .codex/skills/review-self/SKILL.md lines 26-27; all
three locations require direct changes.
---
Outside diff comments:
In @.codex/skills/review-self/SKILL.md:
- Around line 49-66: Add an explicit fallback-mode guard around the Optional
Stability Rechecks section, using the skill’s fallback-for-$review-pr condition,
so fallback execution performs exactly one round and never starts monitoring or
rechecks. Preserve the existing recurring-monitoring behavior for non-fallback
invocations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5bfade9-2d0a-4406-9477-c8b9561b0905
📒 Files selected for processing (10)
.claude/commands/review-pr.md.claude/skills/nitro-sound-workflows/SKILL.md.claude/skills/review-pr/SKILL.md.codex/scripts/install-skills.sh.codex/skills/nitro-sound-workflows/SKILL.md.codex/skills/review-pr/SKILL.md.codex/skills/review-pr/agents/openai.yaml.codex/skills/review-self/SKILL.mdAGENTS.mdknowledge/_claude-context/context.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .codex/scripts/install-skills.sh
- .codex/skills/nitro-sound-workflows/SKILL.md
- knowledge/_claude-context/context.md
- AGENTS.md
Remove blocking MediaPlayer preparation from the shared lock. Harden release CI, E2E cleanup, examples, and synchronized review skills.
|
Review follow-up for eb77046:\n\n- Fixed the outside-diff SoundScreen pause/resume promise handling.\n- Added the explicit one-round fallback guard to review-self.\n- Consolidated microphone permission handling and expanded native workflow path coverage.\n- Kept JSX testID attributes in double quotes because that is the repository's Prettier output; yarn lint passes.\n- Did not add a duplicate Swift-repair step to ci-ios: yarn prepare invokes Bob's custom yarn nitrogen step, and yarn nitrogen now runs the repair script. A clean local yarn prepare verified this path.\n- Also removed stale partial CocoaPods cache restores that caused the previous iOS CI failure. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.codex/skills/simulator-audio-e2e/references/flow-contract.md (1)
1-22: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep
.codex/workflow artifacts local.These files add internal workflow-only content to the PR. Remove them from the committed change unless the user explicitly requests a commit, push, PR, or merge.
.codex/skills/simulator-audio-e2e/references/flow-contract.md#L1-L22: Keep the simulator flow contract local..codex/skills/simulator-audio-e2e/scripts/run-maestro.sh#L6-L127: Keep the Maestro runner local.As per coding guidelines, “Keep internal workflow-only changes under
.codex/local unless the user explicitly requests commit, push, PR, or merge.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.codex/skills/simulator-audio-e2e/references/flow-contract.md around lines 1 - 22, Remove the internal workflow-only files from the committed change: delete .codex/skills/simulator-audio-e2e/references/flow-contract.md (lines 1-22) and .codex/skills/simulator-audio-e2e/scripts/run-maestro.sh (lines 6-127), while leaving them available locally. No source-code changes are required.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.codex/skills/simulator-audio-e2e/references/flow-contract.md:
- Around line 1-22: Remove the internal workflow-only files from the committed
change: delete .codex/skills/simulator-audio-e2e/references/flow-contract.md
(lines 1-22) and .codex/skills/simulator-audio-e2e/scripts/run-maestro.sh (lines
6-127), while leaving them available locally. No source-code changes are
required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4838656a-9cce-485b-9b87-ec9170a44b0d
📒 Files selected for processing (27)
.claude/commands/commit.md.claude/commands/compile-knowledge.md.codex/scripts/install-skills.sh.codex/skills/ios-audio-e2e/SKILL.md.codex/skills/nitro-sound-workflows/SKILL.md.codex/skills/review-pr/SKILL.md.codex/skills/review-self/SKILL.md.codex/skills/simulator-audio-e2e/SKILL.md.codex/skills/simulator-audio-e2e/references/flow-contract.md.codex/skills/simulator-audio-e2e/scripts/preflight.sh.codex/skills/simulator-audio-e2e/scripts/run-maestro.sh.github/actions/setup/action.yml.github/workflows/ci-ios.yml.github/workflows/deploy-web.yml.github/workflows/publish-package.yml.nvmrcandroid/src/main/java/com/margelo/nitro/audiorecorderplayer/Sound.kte2e/maestro/audio-android.yamle2e/maestro/audio-ios.yamlexample/package.jsonexample/src/App.tsxexample/src/screens/CompatibilityScreen.tsxexample/src/screens/SoundHookScreen.tsxexample/src/screens/SoundHookStatesScreen.tsxexample/src/screens/SoundScreen.tsxexample/src/utils/permissions.tspackage.json
💤 Files with no reviewable changes (2)
- .github/actions/setup/action.yml
- .github/workflows/deploy-web.yml
🚧 Files skipped from review as they are similar to previous changes (15)
- .nvmrc
- example/src/screens/CompatibilityScreen.tsx
- .claude/commands/compile-knowledge.md
- example/src/App.tsx
- e2e/maestro/audio-android.yaml
- .codex/skills/nitro-sound-workflows/SKILL.md
- package.json
- .codex/skills/review-self/SKILL.md
- android/src/main/java/com/margelo/nitro/audiorecorderplayer/Sound.kt
- e2e/maestro/audio-ios.yaml
- .codex/skills/simulator-audio-e2e/SKILL.md
- .claude/commands/commit.md
- .codex/skills/simulator-audio-e2e/scripts/preflight.sh
- .codex/scripts/install-skills.sh
- example/src/screens/SoundScreen.tsx
|
Disposition for CodeRabbit review 4878841568: the workflow-artifact finding is not applicable here. The maintainer explicitly requested Codex/Claude skill synchronization and authorized committing, pushing, and updating this PR, which satisfies the repository's exception for .codex/ and related workflow changes. The simulator contract and guarded runner are part of that requested synchronized workflow, so removing them would leave the committed skills incomplete. No code change is needed. |
Use the legacy HFP option spelling on Swift toolchains before 6.2.
|
CI follow-up: fixed the Xcode 16.1 compile failure in fc0787e. AVAudioSession's HFP category option now uses the legacy allowBluetooth spelling on Swift toolchains before 6.2 and allowBluetoothHFP on newer SDKs. This preserves the same HFP route behavior across the repository's supported Xcode range. The Xcode 26.6 simulator build passes locally; the new CI run will verify the Xcode 16.1 branch. No microphone runtime claim is being made. |
|
Follow-up pushed in c8a34e1. The Xcode 16.1 jobs reached a Swift 6.0.2 IR-generation compiler crash after the compatibility computed property was introduced. I removed that helper and now use the SDK legacy |
|
Review loop stopped at the repeated-finding gate on c8a34e1. Both Xcode 16.1 iOS jobs failed again with the same Swift 6.0.2 IR-generation crash and identical ConstantAggregateBuilder/ReflectionMetadataBuilder backtrace after two corrective commits (fc0787e and c8a34e1). All Android, Web, lint, test, and library checks pass; CodeRabbit is successful; unresolved review-thread count is zero. No third speculative fix was attempted. Maintainer direction is required on whether to raise the CI/toolchain baseline or pursue a separate Xcode 16.1 compiler-workaround change. Physical-device microphone runtime E2E remains unapproved and unclaimed. |
|
Maintainer decision implemented in 104aacb: move the supported iOS build baseline to Xcode 26.0+. CI now runs on macOS 15 with Xcode 26.3, README/FAQ/maintainer context are aligned, and the Swift source uses the modern allowBluetoothHFP spelling. Local verification passed with Xcode 26.6 Simulator, typecheck, 7 Jest tests, YAML parsing, and git diff checks; lint has zero errors (only three pre-existing ignored coverage-report warnings). New-head CI is running. Physical-device microphone runtime E2E remains unapproved and is not claimed. |
|
Final diff review found one stale contributor requirement after the Xcode baseline decision. 0a3097e aligns CONTRIBUTING.md with the documented Xcode 26.0 minimum. This is a documentation-only change; the commit hook typecheck and diff checks passed. |
|
Simulator runtime follow-up in 1b0eb9e. On iPhone 17 Pro Max / iOS 26.3 (3C5103EB-0C0B-45E4-9896-DE53FA853951), Xcode 26.6 produced a Debug BUILD PASS and the guarded Maestro lane completed 1/1 in 82 seconds. Direct and useSound flows both covered record, pause/resume, stop, playback, pause/resume, and stop. The flow now persists the early callback milestone and records long enough for reliable iOS state observation. Cleanup was also hardened to terminate sound.example before deleting recordings; the final run left no app process or audio file in the app container/artifacts. Process-scoped error/fault logs were empty. Bluetooth, interruptions, backgrounding, route changes, and audible fidelity remain outside simulator coverage. |
1 participant
Summary
MediaPlayerreplacement against stale callbacks and concurrent state transitions, and use the modern iOS Bluetooth HFP audio-session optionValidation
yarn install --immutableyarn typecheckyarn lintyarn test --maxWorkers=2 --coverage(2 suites, 7 tests)yarn prepareyarn build:webbundle exec pod install --project-directory=iosRuntime note
The guarded iOS Simulator and Android emulator audio flows require explicit microphone-capture approval. Compilation and unit coverage pass; device-backed recording/playback runtime verification remains pending that approval.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation