fix: Reject non-name key filters in record-input#2021
Merged
hatayama merged 2 commits intoJul 27, 2026
Merged
Conversation
--keys went through Enum.TryParse<Key>, which also accepts enum ordinals, so "--keys 3" silently became a filter for Key.Tab. The same hole was closed for simulate-keyboard's --key by matching defined Key enum names; this applies that rule to the filter as well. The name-to-Key whitelist moves to the shared Common/InputSystem assembly both tools already reference, so the rule lives in one place instead of being copied. Entries that name no key are now reported rather than skipped. Skipping them records a different set of keys than the caller asked for, and skipping all of them falls back to recording every key - the opposite of the request - with nothing in the response to distinguish it from omitting --keys entirely.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughKey-name resolution is centralized and rejects ordinals, undefined names, and malformed entries. Record-input parsing reports invalid names before recording starts, with tests and CLI/skill documentation updated for the accepted key formats. ChangesInput key validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant RecordInputUseCase
participant InputRecordingFileHelper
participant KeyNameResolver
participant InputRecorder
CLI->>RecordInputUseCase: Start recording with Keys
RecordInputUseCase->>InputRecordingFileHelper: ParseKeyFilter(Keys)
InputRecordingFileHelper->>KeyNameResolver: Resolve each key name
KeyNameResolver-->>InputRecordingFileHelper: Resolved keys and invalid names
InputRecordingFileHelper-->>RecordInputUseCase: KeyFilterParseResult
alt Invalid key names
RecordInputUseCase-->>CLI: Failure response
else Valid filter
RecordInputUseCase->>InputRecorder: Start recording with filter
InputRecorder-->>CLI: Success response
end
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…tract
- Run scripts/stamp-release-inputs.sh: the regenerated tool catalog is a shared
release input, so both stamp files must move with it or the release trigger
guard fails.
- Say "fails the command" rather than "is rejected" in the --keys documentation:
rejected reads as if only that name is dropped while the rest still records.
- A filter made only of empty entries ("," or " ") no longer falls through to
recording every key with a response indistinguishable from no filter at all.
A trailing comma beside a named key stays harmless.
- Add the PlayMode test for the rejection itself: without it, deleting the check
in RecordInputUseCase left every test green.
- Move normalizedKey into the failure branch that is now its only user.
hatayama
merged commit Jul 27, 2026
b87c5f9
into
feature/cli-discoverability-integration
1 of 2 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
record-input --keysnow accepts only real key names, so a filter never silently records different keys than the ones asked for.User Impact
Before:
--keyswas parsed withEnum.TryParse<Key>, which also accepts enum ordinals.uloop record-input --action Start --keys 3started a recording filtered toKey.Taband reported success, with only an Editor Console warning that an agent driving the CLI never sees. If every entry was rejected, the filter collapsed to "no filter" and the recording captured all keys — the opposite of what was asked — and the response looked identical to a run with no--keysat all.After:
--keys 3fails immediately withValid names keep working exactly as before, case-insensitively, and
Returnis accepted asEnterjust assimulate-keyboardaccepts it.Changes
KeyNameResolverin the sharedCommon/InputSystemassembly that bothsimulate-keyboardand the input-recording assembly already reference.SimulateKeyboardUseCasenow uses it instead of its own private copy, so both tools apply one rule.ParseKeyFilterreturns a result carrying the parsed keys plus every entry that named no key, andrecord-inputrejects the request when that list is non-empty.,or--keys. A trailing comma beside a named key stays harmless.--keysdocuments the name rule; the generated tool catalog, the.claude/.agentscopies, and the shared release-input stamps are regenerated from it.Verification
error CS0103: The name 'KeyNameResolver' does not exist in the current context).uloop compile: 0 errors, 0 warnings.uloop run-tests(EditMode,KeyNameResolverTests|InputRecordingKeyFilterTests): 14/14 passed. Covers ordinals (3), signed ordinals (-1), undefined ordinals (300), comma-OR-ed names (Space,Enter),None, casing, padding, theReturnalias, and the filter's mixed valid/invalid case.uloop run-tests(PlayMode,RecordInputKeyFilterRejectionTests|SimulateKeyboardTests|KeyboardKeyNameSuggesterTests): 48/48 passed — the shared resolver does not changesimulate-keyboardbehavior, and the new PlayMode test pins the rejection itself.go run ./cmd/check-release-triggers --base HEAD~2:Release trigger guard passed.after runningscripts/stamp-release-inputs.sh.