fix(init): refresh iOS run device selection#594
Conversation
|
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:
📝 WalkthroughWalkthroughAdds a new CLI command Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as "Init / run-device CLI"
participant CapRun as "Capacitor CLI"
participant Parser as "Parser/Filter"
participant Prompt as "Interactive Prompt"
participant Runner as "Guarded Runner"
User->>CLI: start init or run-device
CLI->>CapRun: run 'cap run ios --list --json'
CapRun-->>CLI: JSON target list
CLI->>Parser: parseCapacitorRunTargetList(...)
Parser-->>CLI: physical / simulator lists
CLI->>Prompt: show options (devices, simulators, Refresh, Skip)
alt User chooses Refresh
User->>CLI: Refresh
CLI->>CapRun: re-run 'cap run ios --list --json'
CapRun-->>CLI: updated targets
CLI->>Prompt: update choices
else User chooses Device
User->>Prompt: select device
Prompt-->>CLI: selected target id
CLI->>CLI: resolveRunDeviceCommand(selected)
CLI->>Runner: run resolved cap run (unless --no-launch)
Runner-->>CLI: exit status / errors
CLI-->>User: success or show manual command + errors
else User chooses Skip / --no-launch
CLI-->>User: print resolved cap run command, do not launch
end
Estimated Code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/init/command.ts (1)
2517-2585: Split the physical-device selection loop into smaller helpers.This loop is already carrying the error path plus three separate selection states (
1 target,many targets,0 targets). Extracting those branches into dedicated helpers would clear the Sonar finding and make future onboarding tweaks much safer to reason about.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/init/command.ts` around lines 2517 - 2585, The long selectPhysicalIosRunTarget loop should be split into three helper functions to handle the 1-target, many-targets, and zero-target branches: extract logic for the single-target branch into a helper (e.g., handleSinglePhysicalTarget) that logs and returns getRunDeviceCommand(pm,'ios', target); extract the many-targets branch into handleMultiplePhysicalTargets that runs the pSelect flow (including pIsCancel -> exitCanceledInitOnboarding, handling iosRunTargetActions.refresh/simulator/skip, finding the selected target and returning getRunDeviceCommand or continuing/returning skipped); and extract the zero-target branch into handleNoPhysicalTargets that shows the pSelect options and returns the appropriate getRunDeviceCommand or getSkippedRunDeviceCommand (and handles pIsCancel). Keep selectPhysicalIosRunTarget as the thin orchestrator: call getCapacitorRunTargetList and getPhysicalIosRunTargets, then delegate to the new helpers, preserving use of pLog, formatError, pSelect, pIsCancel, exitCanceledInitOnboarding, iosRunTargetActions, getRunDeviceCommand, and getSkippedRunDeviceCommand so control flow/returns remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/init/command.ts`:
- Around line 2517-2585: The long selectPhysicalIosRunTarget loop should be
split into three helper functions to handle the 1-target, many-targets, and
zero-target branches: extract logic for the single-target branch into a helper
(e.g., handleSinglePhysicalTarget) that logs and returns
getRunDeviceCommand(pm,'ios', target); extract the many-targets branch into
handleMultiplePhysicalTargets that runs the pSelect flow (including pIsCancel ->
exitCanceledInitOnboarding, handling iosRunTargetActions.refresh/simulator/skip,
finding the selected target and returning getRunDeviceCommand or
continuing/returning skipped); and extract the zero-target branch into
handleNoPhysicalTargets that shows the pSelect options and returns the
appropriate getRunDeviceCommand or getSkippedRunDeviceCommand (and handles
pIsCancel). Keep selectPhysicalIosRunTarget as the thin orchestrator: call
getCapacitorRunTargetList and getPhysicalIosRunTargets, then delegate to the new
helpers, preserving use of pLog, formatError, pSelect, pIsCancel,
exitCanceledInitOnboarding, iosRunTargetActions, getRunDeviceCommand, and
getSkippedRunDeviceCommand so control flow/returns remain identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cab4244f-f6c1-40b7-8e1a-c5956b121a8a
📒 Files selected for processing (6)
README.mdpackage.jsonskills/usage/SKILL.mdsrc/init/command.tstest/test-onboarding-run-targets.mjswebdocs/init.mdx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a1523caa1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/init/command.ts (1)
2479-2481: Hoist the simulator regex to module scopeLine 2480 recompiles the regex on every call. Move it to a module-level constant.
Proposed fix
+const IOS_SIMULATOR_SUFFIX_RE = /\(simulator\)$/i + export function getPhysicalIosRunTargets(targets: CapacitorRunTarget[]): CapacitorRunTarget[] { - return targets.filter(target => !/\(simulator\)$/i.test(target.name)) + return targets.filter(target => !IOS_SIMULATOR_SUFFIX_RE.test(target.name)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/init/command.ts` around lines 2479 - 2481, The regex /\(simulator\)$/i is being recompiled on every call in getPhysicalIosRunTargets; hoist it to a module-level constant (e.g., const IOS_SIMULATOR_REGEX = /\(simulator\)$/i) and replace the inline literal in getPhysicalIosRunTargets with that constant so the pattern is compiled once and reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/init/command.ts`:
- Around line 2459-2461: The exported function parseCapacitorRunTargetList
currently calls JSON.parse directly and will throw on malformed or empty input;
update parseCapacitorRunTargetList to be defensive: trim the output, if it's
empty return []; wrap JSON.parse in a try/catch, and if parsing fails or the
parsed value is not an array return an empty array; keep the return type as
CapacitorRunTarget[] and ensure any caught errors are swallowed (or optionally
logged) but never propagated.
---
Nitpick comments:
In `@src/init/command.ts`:
- Around line 2479-2481: The regex /\(simulator\)$/i is being recompiled on
every call in getPhysicalIosRunTargets; hoist it to a module-level constant
(e.g., const IOS_SIMULATOR_REGEX = /\(simulator\)$/i) and replace the inline
literal in getPhysicalIosRunTargets with that constant so the pattern is
compiled once and reused.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/init/command.ts`:
- Around line 2520-2523: The simulator branches call getRunDeviceCommand(pm,
'ios') without a target which yields "cap run ios" and lets Capacitor re-prompt;
update the simulator path to resolve a concrete simulator target from available
targets (the same logic used for physical devices) and pass it into
getRunDeviceCommand as the target so args includes '--target', i.e., ensure the
call to getRunDeviceCommand(pm, platformName, target) (or equivalent) supplies a
resolved target object and uses target.id when building args instead of omitting
the target; alternatively remove the custom simulator branch so Capacitor
handles selection — make the change at the call sites that construct
args/command alongside pm.runner and formatRunnerCommand to always include a
target.id when available.
🪄 Autofix (Beta)
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
Run ID: 3f3e57bf-06e3-41bf-b5b6-00d25d209a70
📒 Files selected for processing (2)
src/init/command.tstest/test-onboarding-run-targets.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- test/test-onboarding-run-targets.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/init/command.ts (2)
2788-2840: ExtracttestRunDeviceCommandinto a dedicated command module.This handler is command-surface logic and would be easier to maintain/test if moved out of
src/init/command.tsinto a focused command module, then wired fromsrc/index.ts.As per coding guidelines, "When adding or changing a CLI command, prefer an exported command handler function in a dedicated module and wire it from
src/index.ts".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/init/command.ts` around lines 2788 - 2840, Move the testRunDeviceCommand function out of src/init/command.ts into a new dedicated module (e.g., src/commands/test-run-device.ts) and export it as the command handler; in the new module import its dependencies (getPMAndCommand, normalizeRunDevicePlatform, resolveRunDeviceCommand, runPackageRunnerSync, formatError, pIntro/pOutro/pSpinner/pLog/pCancel/setInitScreen, exitCanceledRunDeviceTest and any types like RunDeviceTestOptions) so the function remains self-contained; then update src/index.ts to import and register/export the handler (testRunDeviceCommand) from the new module so CLI wiring remains unchanged; ensure any relative imports and exported types are adjusted and run tests/lint to fix import paths.
2793-2807: Add an explicit non-TTY fallback before interactive iOS target selection.Line 2807 enters interactive target resolution; add a non-interactive branch so scripted usage does not depend on prompt interaction.
💡 Suggested adjustment
export async function testRunDeviceCommand(platformName?: string, options: RunDeviceTestOptions = {}) { try { const pm = getPMAndCommand() const platformNameChoice = normalizeRunDevicePlatform(platformName) + if ((!stdin.isTTY || !stdout.isTTY) && platformNameChoice === 'ios') { + pLog.info('Non-interactive mode: cannot prompt for iOS target selection.') + pLog.info(`Run manually with a concrete target, e.g. "${pm.runner} cap run ios --target <id>"`) + return + } + pIntro('Run device test')As per coding guidelines, "If a command may run in non-interactive mode, do not rely on spinner-only output; provide plain log output or a non-TTY fallback".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/init/command.ts` around lines 2793 - 2807, Before calling resolveRunDeviceCommand, add a non-interactive (non-TTY) branch that avoids the interactive target picker: detect non-interactive mode (e.g., !process.stdin.isTTY or an existing non-interactive flag), log a clear one-line status via setInitScreen/pIntro, and produce a deterministic runCommand fallback (for Android: construct the Capacitor run command; for iOS: pick a default device/UDID or return a clear error instructing the caller to supply a target) instead of entering resolveRunDeviceCommand's prompt flow; reference resolveRunDeviceCommand, exitCanceledRunDeviceTest, platformNameChoice and runCommand when adding this branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 124: The README contains a duplicate top-level heading "## Options"
causing markdownlint MD024; locate the second occurrence of the "## Options"
heading and either rename it to a distinct title or reduce its level (e.g.,
change "## Options" to "### Options") so headings are unique and hierarchical,
ensuring any internal references or anchor links still make sense after the
change.
---
Nitpick comments:
In `@src/init/command.ts`:
- Around line 2788-2840: Move the testRunDeviceCommand function out of
src/init/command.ts into a new dedicated module (e.g.,
src/commands/test-run-device.ts) and export it as the command handler; in the
new module import its dependencies (getPMAndCommand, normalizeRunDevicePlatform,
resolveRunDeviceCommand, runPackageRunnerSync, formatError,
pIntro/pOutro/pSpinner/pLog/pCancel/setInitScreen, exitCanceledRunDeviceTest and
any types like RunDeviceTestOptions) so the function remains self-contained;
then update src/index.ts to import and register/export the handler
(testRunDeviceCommand) from the new module so CLI wiring remains unchanged;
ensure any relative imports and exported types are adjusted and run tests/lint
to fix import paths.
- Around line 2793-2807: Before calling resolveRunDeviceCommand, add a
non-interactive (non-TTY) branch that avoids the interactive target picker:
detect non-interactive mode (e.g., !process.stdin.isTTY or an existing
non-interactive flag), log a clear one-line status via setInitScreen/pIntro, and
produce a deterministic runCommand fallback (for Android: construct the
Capacitor run command; for iOS: pick a default device/UDID or return a clear
error instructing the caller to supply a target) instead of entering
resolveRunDeviceCommand's prompt flow; reference resolveRunDeviceCommand,
exitCanceledRunDeviceTest, platformNameChoice and runCommand when adding this
branch.
🪄 Autofix (Beta)
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
Run ID: 4a941b6b-6e8d-4612-a779-ccc2e70162c3
📒 Files selected for processing (21)
README.mdskills/usage/SKILL.mdsrc/docs.tssrc/index.tssrc/init/command.tssrc/init/index.tswebdocs/account.mdxwebdocs/app.mdxwebdocs/build.mdxwebdocs/bundle.mdxwebdocs/channel.mdxwebdocs/doctor.mdxwebdocs/init.mdxwebdocs/key.mdxwebdocs/login.mdxwebdocs/organisation.mdxwebdocs/organization.mdxwebdocs/probe.mdxwebdocs/run-device.mdxwebdocs/star-all.mdxwebdocs/star.mdx
✅ Files skipped from review due to trivial changes (17)
- webdocs/account.mdx
- webdocs/organization.mdx
- webdocs/channel.mdx
- webdocs/organisation.mdx
- webdocs/init.mdx
- src/docs.ts
- webdocs/star-all.mdx
- webdocs/bundle.mdx
- webdocs/star.mdx
- webdocs/app.mdx
- webdocs/login.mdx
- webdocs/key.mdx
- webdocs/build.mdx
- webdocs/doctor.mdx
- webdocs/probe.mdx
- src/init/index.ts
- webdocs/run-device.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/usage/SKILL.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7439adcbb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb163217b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ice-refresh # Conflicts: # package.json # skills/usage/SKILL.md # src/init/command.ts
|
1 participant
Fixes #593
Summary
cap run ios --target <id>to avoid stale Capacitor target promptsLocal verification
bun run lintbun run buildbun run test:mcpbun run test:bundle./test/fixtures/setup-test-projects.shbun run testSummary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes