Skip to content

fix(cli): reject an unknown --device name - #728

Merged
LargeModGames merged 1 commit into
LargeModGames:mainfrom
devtechedge:fix-cli-unknown-device-name
Oct 8, 2026
Merged

fix(cli): reject an unknown --device name#728
LargeModGames merged 1 commit into
LargeModGames:mainfrom
devtechedge:fix-cli-unknown-device-name

Conversation

@devtechedge

@devtechedge devtechedge commented Oct 8, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

--device with a name that matches nothing used to keep going. set_device wrote an id only on an exact match, then returned success, so a typo or an empty device list still played on the saved device.
--transfer already stopped with no device with name '…' and a non-zero exit. --device now does that before any playback command runs.

device_id_by_name returns the id of the first device with that exact name. It errors when nothing matches, the list is empty, or that device has no id. If devices() is None, the old no device available error is unchanged.

The old loop had no break, so two devices with the same name kept the last id. The helper keeps the first. --transfer already stopped at the first match, and it now uses this helper so both paths share the message.

The tests call the helper only. They never build a CliApp, because set_device_id writes the real client.yml.

  • a_device_name_resolves_to_its_id
  • an_unknown_device_name_is_an_error
  • a_device_name_with_no_id_is_an_error
  • a_device_name_with_no_devices_is_an_error

Checked locally:

  • cargo test --no-default-features --features telemetry,tui device_name: 4 passed, 1457 filtered out
  • cargo fmt --all -- --check
  • cargo clippy --no-default-features --features telemetry,tui -- -D warnings
  • cargo test --no-default-features --features telemetry,tui: 1461 passed, 0 failed

Fixes #669

Summary by CodeRabbit

  • Bug Fixes
    • The play and playback commands now report an error and exit with a non-zero status when the requested Connect device cannot be found, instead of continuing on the saved device.
    • When multiple devices have the requested name, playback uses the first match. If that device has no ID, the command reports that no device with the name was found.

@devtechedge
fix(cli): reject an unknown --device name
b2eab21 
An unmatched --device name used to fall through and keep the saved device. Resolve the name first and return the same error --transfer already uses.
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LargeModGames/spotatui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b33fa3d8-7f24-4088-949d-236ee0c18204
📥 Commits

Reviewing files that changed from the base of the PR and between 7bf7d10 and b2eab21.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • src/cli/cli_app.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

The CLI now resolves requested device names to IDs and reports an error when no exact match has an ID. Both playback --device and play --device stop instead of continuing with a saved device. When names are duplicated, lookup uses the first match.

Changes

CLI device-name lookup

Layer / File(s) Summary
Shared device-name lookup
src/cli/cli_app.rs		 A shared helper returns the ID of the first exact name match. It reports the same name-specific error for an unknown name, a matching device without an ID, or an empty list. Tests cover these cases and a successful lookup.
CLI device selection
src/cli/cli_app.rs, CHANGELOG.md		 set_device and transfer_playback use the shared lookup. The changelog records the non-zero exit behavior for unmatched names and the first-match behavior for duplicate names.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to b2eab

The device-name lookup appears to meet the stated behavior; no merge-blocking issue is identified.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses the permitted conventional-commit prefix fix(cli): and concisely describes the change to reject unknown --device names.
Linked Issues check ✅ Passed Issue [#669] requirements are met. set_device now uses device_id_by_name before playback handling. The helper uses an exact, case-sensitive match, returns the first matching ID, and returns `no de…
Out of Scope Changes check ✅ Passed The changes stay within issue [#669]. The shared helper and transfer_playback update provide the requested common behavior. The unit tests verify the requested behavior. The changelog documents the …
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
✨ Simplify code
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.18750% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/cli/cli_app.rs 67.1% 21 Missing ⚠️

📢 Thoughts on this report? Let us know!

@LargeModGames
LargeModGames merged commit 5bb415d into LargeModGames:main Oct 8, 2026
31 checks passed
@devtechedge

devtechedge commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

I appreciate the quick approval and merge, LargeModGames.

--device was the path that still played on the saved device when the name matched nothing, while --transfer already stopped.

Sharing device_id_by_name so a miss, an empty list, and a device with no id all fail the same way was the piece I wanted to get right.

Genuinely fun to trace, and I learned a lot from how those two commands had drifted!

@chenrui333 chenrui333 mentioned this pull request Oct 12, 2026
Merged
@LargeModGames LargeModGames mentioned this pull request Oct 22, 2026
Merged
LargeModGames added a commit that referenced this pull request Oct 22, 2026
@LargeModGames
docs: add six contributors to the list (#920)
3e8698e 
# Summary

Adds @mvanhorn, @devtechedge, @montionugera, @psymariux, @uznog and
@zaydmulani09 to the contributor list for code, for #857, #728, #729,
#893, #536, #677 and #592. One PR for all six, so the entries do not
conflict with each other the way separate bot PRs do.

# Testing

- `npx [email protected] add <login> code` for each login,
which also regenerated the README table.

# Additional notes

Replaces #918, #919 and the other bot PRs from the requests on #872.

---
<sub>💬 Questions or want to chat with other contributors? Join the
[spotatui Discord](https://spotatui.com/discord).</sub>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LargeModGames LargeModGames LargeModGames approved these changes
@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CLI --device with an unknown device name is silently ignored

2 participants

@devtechedge @LargeModGames