Conversation
…ction AVD selector Pixel_7_API_36 selected device "Pixel 7 API 36" but the bound-session check compared raw lowercase strings and rejected the same selector on the next open. Share matchesDeviceNameSelector from kernel between selection and session binding.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused change preserves device selection behavior and covers the session-binding fix with a regression test.
Review effort: Balanced
Findings: None
What changed in this PR
Makes session binding use the same device-name normalization as device selection, allowing consistent reuse of --device selectors.
Changes:
- Exports and reuses a shared device-name matcher.
- Adds regression coverage for underscore-separated AVD names.
| File | Description |
|---|---|
src/daemon/session-selector.ts |
Uses the shared matcher for binding checks. |
src/daemon/__tests__/session-selector.test.ts |
Tests AVD-name selection and session reuse. |
packages/kernel/src/device.ts |
Extracts the matcher without changing selection semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This PR is ready at a5d8b72. The repeated-open binding check in src/daemon/session-selector.ts now accepts the same --device spellings that device selection accepts, and I found no problem in the change. Not blocking: resolveExplicitShardDevices in packages/replay-port/src/daemon-port/session-test-shard-devices.ts still has its own copy of normalizeDeviceName and the name-equality check, so a later change to the kernel matcher will not reach the shard path. A follow-up could call matchesDeviceNameSelector from @agent-device/kernel/device and drop the local copy. Take it or leave it. There are no conflicts. Both Smoke Tests jobs were still running when I looked, so I cannot attribute a result yet. The diff only touches their route through the repeated-open binding check, and it only accepts more spellings of the same name. I did not run the unit tests locally, and I could not re-run the Pixel_7_API_36 check, which was done on the pre-rebase head. Once both Smoke Tests jobs finish green, this can merge with no code change needed. |
thymikee
added
the
ready-for-human
Summary
From e2e feedback:
--device Pixel_7_API_36selects the emulator displayed as "Pixel 7 API 36", but reusing that selector on a session already bound to it failed withINVALID_ARGS("Session ... is already bound"). Device selection normalizes names (case,_vs space, whitespace) while the session binding check compared raw lowercase strings.Kernel now exports
matchesDeviceNameSelector, and both device selection and the session binding conflict check use it, so a name that selected a device keeps matching it.3 files touched.
Validation
Tested commit: a5d8b72 (on upstream/main 9a245d0).
pnpm vitest run src/daemon/__tests__/session-selector.test.ts: 13 passed (AVD-style selector accepted on bound session; a different name still conflicts)pnpm check:affected --run --base upstream/main: all runnable checks passed (959 files / 8688 related tests, daemon wire compat unchanged protocol).open --session p7 --device Pixel_7_API_36returnedINVALID_ARGS; after, it succeeds; a different--deviceis still rejected.