test(#276): UTM macOS VM 検証ハーネスを整備する#277
Conversation
.claude/rules/vm-verification.md を追加。lyra 固有の検証レーン分類 (VM で確認できること / できないこと、Dynamic Resolution の位置付け、 display hot-plug vs ScreenProvider fixture の棲み分け)を定義する。 .claude/scripts/lyra-vm-harness.sh を追加。utmctl + SSH を使って ホストから VM のライフサイクル管理・lyra のビルド/インストール/起動・ サービス状態の保存と復元・成果物収集(スクリーンショット/ログ/プロセス サンプル)を行うハーネスを提供する。スクリプトは自己完結しており、 ユーザーグローバルのスキルやルールに依存しない。 .claude/rules/dev-verification.md に VM レーンへの参照を追記。 設計方針: lyra 固有手順はプロジェクト .claude/、汎用 macOS VM 操作 パターンはユーザーグローバル ~/.config/claude/rules/utm-macos-vm.md に 切り出し。両者は同じ設計思想を共有するが依存しない。
|
Warning Review limit reached
More reviews will be available in 44 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds VM verification rules and docs, a host-side Bash harness to orchestrate UTM macOS VM lifecycle and run/capture/restore lyra in-guest, tweaks yt-dlp arguments for YouTube wallpaper downloads, and increments project version to 2.13.14. ChangesVM Verification Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4387a70c54
ℹ️ 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".
| local ip="" | ||
| log "Waiting for $vm to be reachable via SSH (timeout: ${LYRA_VM_BOOT_TIMEOUT}s)..." | ||
| while [[ $SECONDS -lt $deadline ]]; do | ||
| ip="$(vm_ip "$vm")" |
There was a problem hiding this comment.
Guard the SSH wait loop against missing IPs
When boot or reboot starts polling before UTM has published an IP address, vm_ip exits non-zero because the grep | head pipeline finds no match under set -euo pipefail; this assignment is not guarded, so the harness exits immediately instead of waiting until LYRA_VM_BOOT_TIMEOUT. Make the IP lookup non-fatal in the polling loop (or in vm_ip) so newly-started guests without an address yet can still become ready.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
|
|
||
| log "Pushing binary to guest..." | ||
| ssh_run "$ip" "mkdir -p /tmp/lyra-drop" | ||
| scp_put "$ip" "$binary" "/tmp/lyra-drop/lyra" |
There was a problem hiding this comment.
Copy SwiftPM resource bundles with the VM binary
run-lyra uploads only the executable, but this package has SwiftPM resources (VersionHandler and MediaRemoteDataSource) and the existing make install target copies the generated *.bundle directories next to the binary. Without those bundles on the guest, Bundle.module lookups return unknown for the version and cannot find media-remote-helper.swift, so VM track/daemon verification is not exercising the same installed app behavior.
Useful? React with 👍 / 👎.
| scp_put "$ip" "$binary" "/tmp/lyra-drop/lyra" | ||
|
|
||
| log "Installing on guest..." | ||
| ssh_run "$ip" "sudo install -m 755 /tmp/lyra-drop/lyra /usr/local/bin/lyra" |
There was a problem hiding this comment.
Start the binary that was just installed
On Apple-silicon Homebrew guests, the brew install from the prerequisites lives under /opt/homebrew/bin, and the repo's own executable lookup checks that path before /usr/local/bin. Since the harness installs the PR build to /usr/local/bin/lyra but later runs plain lyra daemon, the shell can resolve the preinstalled brew binary instead of the just-pushed build, invalidating VM results for the commit under test.
Useful? React with 👍 / 👎.
| prior_state="$(ssh_run "$ip" "cat ~/.lyra-vm-prior-service-state 2>/dev/null || printf 'none'")" | ||
| if [[ "$prior_state" == "started" ]]; then | ||
| log "Restoring lyra brew service on guest..." | ||
| ssh_run "$ip" "brew services start lyra" |
There was a problem hiding this comment.
Restore the overwritten brew binary before restart
On guests where Homebrew installs lyra into /usr/local/bin, run-lyra overwrites the brew-managed executable with the PR build and restore then starts the service again without reinstalling or restoring the original binary. In that setup the trap leaves the VM dirty and the supposedly restored brew service keeps running the test build after verification.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
インストール先を
/usr/local/bin/ から /tmp/lyra-vm-test/ に変更し、brew 管理バイナリを上書きしないようにしました。Intel・Apple Silicon 両方で restore 後も brew バイナリが汚染されなくなります。バンドルも同ディレクトリに配置するため Bundle.module も正常に動作します。
Fixed in e98e904.
| pid="$(ssh_run "$ip" "cat ~/.lyra-vm-daemon.pid 2>/dev/null || printf '?'")" | ||
| log "lyra daemon running on guest (PID=$pid)" |
There was a problem hiding this comment.
Fail run-lyra when the daemon exits immediately
If lyra daemon fails during startup (for example because another instance still holds the flock after brew services stop failed, or because the pushed build crashes), the SSH command still writes the background PID and run-lyra reports success after the sleep without checking that the process is alive. This makes the harness proceed to health checks and captures against no PR daemon, so validate the PID after the grace period and surface the daemon log on failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
sleep 3 後に kill -0 $pid でデーモンの生存を確認するようにしました。起動直後にクラッシュした場合はデーモンログを出力して die するようになります。
Fixed in e98e904.
Swift ファイル作業時のみロードされるよう *.swift / Package.swift に絞る。 macOS ネイティブ挙動の検証が必要になるのは Swift を書いている時であり、 マルチプラットフォームな設定・ドキュメント作業では不要なため。
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scripts/lyra-vm-harness.sh:
- Around line 111-116: Replace the fixed sleep-based checks with polling that
verifies actual VM state transitions: in the shutdown path (where ssh_run is
used then utmctl status/stop/--kill are invoked) poll utmctl status for the
"stopped" state with a short sleep loop and a configurable timeout before
escalating to utmctl stop --kill; in the reboot path (where wait_for_ssh is
used) ensure you first detect the SSH session drop by polling until SSH fails,
then poll for a new successful SSH connection (using wait_for_ssh or the same
ssh_run probe) within a timeout to confirm the guest actually rebooted; apply
these changes around the ssh_run, utmctl, and wait_for_ssh usage so you only
force-stop or treat reboot as complete after the confirmed state transitions.
- Around line 144-156: The harness installs the built artifact to
/usr/local/bin/lyra but later starts the daemon with "lyra daemon", which can
pick up a different Homebrew-provided binary; update the detached start command
used in the ssh_run that currently contains "nohup lyra daemon >
~/.lyra-vm-daemon.log 2>&1 & printf '%s\n' \$! > ~/.lyra-vm-daemon.pid" to
invoke the exact installed binary (use /usr/local/bin/lyra) so the VM uses the
artifact just copied; keep the same nohup/stdout/stderr and PID-file behavior.
- Around line 197-199: The remote tilde should be expanded before calling
scp_get because scp in SFTP mode may not expand "~"; obtain the remote home
directory via ssh (e.g., run a remote 'printf %s "$HOME"' using the existing
$ip), store it in a variable like remote_home, then call scp_get with
"$remote_home/.lyra-vm-daemon.log" instead of the literal
"~/.lyra-vm-daemon.log"; keep the existing error handling/log calls (log and
$out_dir) and update references to scp_get, ip, out_dir, and log accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7352a26e-27b1-4948-b8a5-59cba3d153b5
📒 Files selected for processing (4)
.claude/rules/dev-verification.md.claude/rules/vm-verification.md.claude/scripts/lyra-vm-harness.shSources/VersionHandler/Resources/version.txt
…rness.sh - Add `LYRA_VM_SSH_HOST` env var to `vm_ip()` so the harness works with macOS Apple Virtualization Framework backend VMs where `utmctl ip-address` returns "Operation not supported by the backend" - Convert `A && B || C` chains in `cmd_capture` to proper `if/then/else` (SC2015) - Add `shellcheck disable=SC2088` with justification for tilde in remote scp path
Safari で URL を開いて AppleScript で自動再生。 MediaRemote が認識するか lyra track でそのまま確認できる。 ゲスト側にスクリプトは置かず、ホスト側から SSH で完結。
- scp_put_r ヘルパーを追加 (ディレクトリ再帰コピー) - run-lyra: /tmp/lyra-drop を事前 sudo rm -rf してルート所有古バンドルを除去 - run-lyra: *.bundle を /usr/local/bin/ に配置 (resource bundle 必須) - run-lyra: nohup → sudo launchctl asuser + ランチャースクリプト方式に変更 SSH コンテキストから AppKit ウィンドウを表示するには GUI bootstrap namespace への inject が必要なため - capture: screencapture も GUI session が必要 (TODO コメント追加) 実証済み: VM 上で Payphone (Maroon 5) の歌詞が全画面オーバーレイ表示
- Add explicit `pgrep -x lyra | kill` step in run-lyra before launching new daemon to prevent "Another lyra daemon is already running" when re-running - Fix launcher script to use `echo "$!"` instead of `printf '%s\n' "$!"` (unquoted `\n` in sh was consumed by the shell → PID file contained `<pid>n`)
…YouTube 403
yt-dlp with the default web client triggers YouTube SABR streaming which
returns HTTP 403. Switching to `player_client=android` avoids SABR, but
video-only formats then require a GVS PO Token (unavailable), causing
"Requested format is not available".
Fix: add `best[ext=mp4][height<=maxHeight]` as fallback after the
video-only selector. This picks the combined A/V format (format 18,
360p MP4) when video-only streams are blocked — confirmed working in
VM verification. --no-audio is a no-op for combined formats.
fix(harness): use /tmp for daemon log+pid instead of $HOME
sudo resets HOME to /var/root, so "$HOME"/.lyra-vm-daemon.log and
.pid were written to /var/root/ and invisible to the SSH login user
(babu). Fixed to use /tmp/lyra-vm-daemon.{log,pid} which all users
can read.
fix(harness): add host-side UTM window fallback screenshot in capture
When SSH screencapture -x fails (no display in SSH context), falls back
to a Swift CGWindow lookup on the host to capture the largest UTM window.
Enables visual verification of the download indicator and UI state.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Closes #276
概要
UTM macOS VM を使って lyra の OS レベル検証(サービスライフサイクル、launchd KeepAlive、ゲスト再起動、ログ収集)を開発機を汚さずに行えるハーネスを整備した。
追加ファイル
.claude/scripts/lyra-vm-harness.shutmctl+ SSH ベースのハーネスシェルスクリプト。サブコマンド構成:boot <vm>shutdown <vm>reboot <vm>run-lyra <vm>capture <vm> [dir]restore <vm>exec <vm> -- <cmd>ip <vm>SSH を主経路とし、
utmctlはライフサイクル(start/stop/status/ip-address)専用。utmctl execは macOS Apple backend ゲストでは login shell をバイパスするため使わない。.claude/rules/vm-verification.mdlyra 固有の検証レーン分類:
ScreenProviderfixture テストで扱い、物理 hot-plug は最終手動 smoke に残す判断基準.claude/rules/dev-verification.md(追記)末尾に VM レーンへのポインタを追記。視覚的な確認はホスト debug-build 手順、OS レベルの副作用は VM レーンと使い分けを明示。
設計方針(棲み分け)
.claude/rules/.claude/scripts/~/.config/claude/rules/utm-macos-vm.mdlyra プロジェクトはグローバルルールに依存しない。同じ設計思想を持ちつつ自己完結している。
Summary by CodeRabbit
Documentation
New Features
Chores
Behavior