fix: Reject non-name key inputs in simulate-keyboard

[hatayama]

Summary

• simulate-keyboard --key now fails loudly on inputs that are not Input System Key names, instead of silently pressing an unrelated key.
• A rejected digit explains both valid forms (Digit3 / Numpad3) and warns that earlier runs may have pressed something else.

User Impact

Before, --key 3 returned Success: true and pressed Tab. Enum.TryParse accepted much more than key names:

Input	Old behavior
3	Success: true, pressed Tab (parsed as the enum ordinal)
+3	Success: true, same ordinal path
Space,Enter	Success: true, comma-separated names OR-ed into one value
300	Passed the Key.None guard, then threw ArgumentOutOfRangeException from the keyboard indexer

A run that pressed the wrong key still reported success, so verification results silently described input that never happened — the worst case for anyone using these results as evidence.

After this change all four inputs return Success: false with a candidate list. Rejecting a numeric key also states that bare digits used to be parsed as enum ordinals (e.g. "3" pressed Tab), so earlier results and scripts that passed digits can be re-checked rather than trusted.

Correct input is not narrowed. --key Return keeps working through the existing Return to Enter alias, and whitespace-padded names such as --key " Space " keep resolving — Enum.TryParse used to trim, so padded correct input already worked and rejecting it would break valid scripts. " 3 " still fails, because after trimming it is 3, which is not a name defined on the Key enum.

Changes

• Resolve a key only when the normalized name matches a name defined on the Key enum, using an immutable case-insensitive whitelist built from Enum.GetNames(typeof(Key)).
• Normalize once at the input entry point, trimming before the alias is applied, so padded names and the padded alias both keep working.
• Scope the digit guidance to ASCII numeric input. char.IsDigit is also true for non-ASCII digits, which Enum.TryParse never read as ordinals, so the message would otherwise claim an earlier run pressed another key when it did not.
• Keep the existing Key.None guard and route every rejection through the existing suggester error path, preserving the Invalid key name wording that tests pin.
• KeyboardKeyNameSuggester keeps its suggestion logic. Its digit suggestions were already implemented but unreachable for 1-9, because Enum.TryParse resolved those to a key before the suggester could run. 0 was the exception: it parsed to Key.None and was caught by the guard, so that one digit did reach the suggester.
• Match ASCII digits in the suggester too. char.IsDigit is true for non-ASCII digits such as ３, which would have suggested a Digit３ name that no Key value has.
• Add Digit3 to the example key names in both validation messages, so the error-prone form appears where the mistake is made.

Verification

Tests were written first and confirmed failing before the fix (4 red, plus the Return alias case green), then re-run after it.

• uloop compile — ErrorCount: 0, WarningCount: 0
• uloop run-tests --test-mode PlayMode filtered to SimulateKeyboardTests — 47 tests, 47 passed, 0 failed
• uloop run-tests --test-mode EditMode filtered to KeyboardKeyNameSuggesterTests — 3 tests, 3 passed, 0 failed
• uloop run-tests --test-mode EditMode filtered to StaticFacadeStateGuardTests — 20 tests, 20 passed, 0 failed, confirming the new static readonly lookup does not trip the mutable-static guard

Red-state evidence before the fix: --key "3" / "+3" / "Space,Enter" each failed with Expected: False But was: True, and --key "300" failed with an unhandled ArgumentOutOfRangeException, confirming both silent-success paths and the exception path.

Note on CI: repository workflows are scoped to pull_request.branches [main, v3-beta], so none of them run on a pull request targeting this integration branch. The results above are local runs; CI validation happens on the final integration pull request.

[coderabbitai]

📝 Walkthrough

Walkthrough

The Press action now validates key names against defined Input System keys, rejects numeric and comma-separated inputs, adds numeric-input guidance, and retains support for the Return alias through new play-mode tests.

Changes

Keyboard validation

Layer / File(s)	Summary
Strict key-name validation Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs	ExecuteAsync now uses a case-insensitive key-name whitelist, rejects numeric ordinals and compound inputs, expands missing-key guidance, and detects numeric-style input for error messages.
Press validation coverage Assets/Tests/PlayMode/SimulateKeyboardTests.cs	Play-mode tests verify failures for bare, signed, compound, and undefined numeric inputs, while confirming the Return alias succeeds.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• hatayama/unity-cli-loop#783: Adds related invalid-key coverage for the simulate-keyboard Press action.
• hatayama/unity-cli-loop#1516: Refactors the SimulateKeyboardUseCase.ExecuteAsync path modified here.
• hatayama/unity-cli-loop#1522: Updates ExecuteAsync validation to return readable failure responses.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly captures the main change: rejecting non-name key inputs in simulate-keyboard.
Description check	✅ Passed	The description is detailed and directly matches the implemented validation and tests.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/simulate-keyboard-numeric-key

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

Assets/Tests/PlayMode/SimulateKeyboardTests.cs (1)

637-707: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert the new numeric guidance, not only rejection.

These tests prove Success == false, but do not consistently verify the new "Digits are not key names" and prior-results warning text. Add those assertions so the remediation contract cannot regress while validation still fails.

Suggested assertions

     Assert.IsFalse(lastResponse.Success, "A bare digit must not be accepted as a Key enum ordinal");
+    StringAssert.Contains("Digits are not key names", lastResponse.Message);
+    StringAssert.Contains("re-check any earlier results", lastResponse.Message);

     Assert.IsFalse(lastResponse.Success, "A signed numeric key must not be accepted as a Key enum ordinal");
+    StringAssert.Contains("Digits are not key names", lastResponse.Message);
+    StringAssert.Contains("re-check any earlier results", lastResponse.Message);

🤖 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 `@Assets/Tests/PlayMode/SimulateKeyboardTests.cs` around lines 637 - 707,
Extend the failure assertions in
Press_WithBareDigitKey_Should_ReturnFailureSuggestingDigitAndNumpad,
Press_WithSignedNumericKey_Should_ReturnFailure,
Press_WithCommaSeparatedKeyNames_Should_ReturnFailure, and
Press_WithUndefinedNumericKey_Should_ReturnFailure to verify the response
message includes the new “Digits are not key names” guidance and the
prior-results warning text. Preserve the existing Success, candidate-name, and
validation assertions.

🤖 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.

Nitpick comments:
In `@Assets/Tests/PlayMode/SimulateKeyboardTests.cs`:
- Around line 637-707: Extend the failure assertions in
Press_WithBareDigitKey_Should_ReturnFailureSuggestingDigitAndNumpad,
Press_WithSignedNumericKey_Should_ReturnFailure,
Press_WithCommaSeparatedKeyNames_Should_ReturnFailure, and
Press_WithUndefinedNumericKey_Should_ReturnFailure to verify the response
message includes the new “Digits are not key names” guidance and the
prior-results warning text. Preserve the existing Success, candidate-name, and
validation assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d833752a-1c24-4b62-805c-54f7693d7a25

📥 Commits

Reviewing files that changed from the base of the PR and between cbbf9df and 65e73e9.

📒 Files selected for processing (2)

• Assets/Tests/PlayMode/SimulateKeyboardTests.cs
• Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs

[hatayama]

Addressed in cc6fa41 — thanks.

Applied to the three numeric cases ("3", "+3", "300"): each now asserts Digits are not key names and re-check any earlier results in addition to the existing Success, candidate-name, and validation assertions.

Not applied to Press_WithCommaSeparatedKeyNames_Should_ReturnFailure. "Space,Enter" is not numeric input, so it is rejected without the digit guidance — the message is deliberately scoped to inputs that Enum.TryParse used to accept as ordinals. Asserting that text there would fail.

Verified: PlayMode SimulateKeyboardTests 42 tests, 42 passed, 0 failed.