fix: Reject non-name key filters in record-input

.agents/skills/uloop-record-input/SKILL.md

@@ -30,7 +30,7 @@ uloop record-input --action Stop --output-path scripts/my-play.json
 |-----------|------|---------|-------------|
 | `--action` | enum | `Start` | `Start` - begin recording input, `Stop` - stop recording and save to file |
 | `--output-path` | string | auto | Save path for the recording JSON. When empty, auto-generates under `.uloop/outputs/InputRecordings/` |
-| `--keys` | string | `""` | Comma-separated key filter (for example `W,A,S,D,Space`). Empty records all common game keys |
+| `--keys` | string | `""` | Comma-separated key filter of Input System Key enum names (for example `W,A,S,D,Space`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`; a name that matches no key fails the command instead of being dropped from the filter. Empty records all common game keys |
 | `--delay-seconds` | integer | `3` | Countdown delay in seconds before recording starts (0-10). Gives time to switch focus to Game View. |
 | `--no-show-overlay` | flag | - | Hide the recording countdown and REC indicator overlay |
 

.claude/skills/uloop-record-input/SKILL.md

@@ -30,7 +30,7 @@ uloop record-input --action Stop --output-path scripts/my-play.json
 |-----------|------|---------|-------------|
 | `--action` | enum | `Start` | `Start` - begin recording input, `Stop` - stop recording and save to file |
 | `--output-path` | string | auto | Save path for the recording JSON. When empty, auto-generates under `.uloop/outputs/InputRecordings/` |
-| `--keys` | string | `""` | Comma-separated key filter (for example `W,A,S,D,Space`). Empty records all common game keys |
+| `--keys` | string | `""` | Comma-separated key filter of Input System Key enum names (for example `W,A,S,D,Space`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`; a name that matches no key fails the command instead of being dropped from the filter. Empty records all common game keys |
 | `--delay-seconds` | integer | `3` | Countdown delay in seconds before recording starts (0-10). Gives time to switch focus to Game View. |
 | `--no-show-overlay` | flag | - | Hide the recording countdown and REC indicator overlay |
 

Assets/Tests/Editor/InputRecordingKeyFilterTests.cs

@@ -0,0 +1,91 @@
+#if ULOOP_HAS_INPUT_SYSTEM
+using System.Collections.Generic;
+using NUnit.Framework;
+using UnityEngine.InputSystem;
+
+using io.github.hatayama.UnityCliLoop.FirstPartyTools;
+
+namespace io.github.hatayama.UnityCliLoop.Tests.Editor
+{
+    /// <summary>
+    /// Test fixture that verifies the record-input key filter accepts only defined key names.
+    /// </summary>
+    public sealed class InputRecordingKeyFilterTests
+    {
+        /// <summary>
+        /// Tests that named keys are accepted case-insensitively and no name is reported invalid.
+        /// </summary>
+        [Test]
+        public void ParseKeyFilter_WhenGivenKeyNames_KeepsThemAll()
+        {
+            KeyFilterParseResult result = InputRecordingFileHelper.ParseKeyFilter("space, w");
+
+            Assert.IsEmpty(result.InvalidKeyNames);
+            Assert.IsNotNull(result.Filter);
+            Assert.AreEqual(new HashSet<Key> { Key.Space, Key.W }, result.Filter);
+        }
+
+        /// <summary>
+        /// Tests that a bare digit is reported invalid instead of silently filtering the key whose
+        /// enum ordinal it happens to be.
+        /// </summary>
+        [Test]
+        public void ParseKeyFilter_WhenGivenAnOrdinal_ReportsItInvalid()
+        {
+            KeyFilterParseResult result = InputRecordingFileHelper.ParseKeyFilter("3");
+
+            Assert.AreEqual(new[] { "3" }, result.InvalidKeyNames);
+            Assert.IsNull(result.Filter);
+        }
+
+        /// <summary>
+        /// Tests that an invalid entry alongside a valid one is still reported, so a partially
+        /// applied filter is never mistaken for the requested one.
+        /// </summary>
+        [Test]
+        public void ParseKeyFilter_WhenOneEntryIsInvalid_ReportsThatEntry()
+        {
+            KeyFilterParseResult result = InputRecordingFileHelper.ParseKeyFilter("W, 3");
+
+            Assert.AreEqual(new[] { "3" }, result.InvalidKeyNames);
+        }
+
+        /// <summary>
+        /// Tests that a filter made only of empty entries is reported invalid: it would otherwise
+        /// record every key while looking like no filter was requested.
+        /// </summary>
+        [Test]
+        public void ParseKeyFilter_WhenEveryEntryIsEmpty_ReportsTheRawInputInvalid()
+        {
+            KeyFilterParseResult result = InputRecordingFileHelper.ParseKeyFilter(", ,");
+
+            Assert.AreEqual(new[] { ", ," }, result.InvalidKeyNames);
+            Assert.IsNull(result.Filter);
+        }
+
+        /// <summary>
+        /// Tests that a trailing comma is harmless once another entry names a key.
+        /// </summary>
+        [Test]
+        public void ParseKeyFilter_WhenAnEntryIsEmptyBesideANamedKey_KeepsTheKey()
+        {
+            KeyFilterParseResult result = InputRecordingFileHelper.ParseKeyFilter("W,");
+
+            Assert.IsEmpty(result.InvalidKeyNames);
+            Assert.AreEqual(new HashSet<Key> { Key.W }, result.Filter);
+        }
+
+        /// <summary>
+        /// Tests that no filter and no invalid name is reported when the parameter is omitted.
+        /// </summary>
+        [Test]
+        public void ParseKeyFilter_WhenGivenNothing_ReportsNoFilter()
+        {
+            KeyFilterParseResult result = InputRecordingFileHelper.ParseKeyFilter("");
+
+            Assert.IsEmpty(result.InvalidKeyNames);
+            Assert.IsNull(result.Filter);
+        }
+    }
+}
+#endif

Assets/Tests/Editor/KeyNameResolverTests.cs

@@ -0,0 +1,111 @@
+#if ULOOP_HAS_INPUT_SYSTEM
+using NUnit.Framework;
+using UnityEngine.InputSystem;
+
+using io.github.hatayama.UnityCliLoop.FirstPartyTools;
+
+namespace io.github.hatayama.UnityCliLoop.Tests.Editor
+{
+    /// <summary>
+    /// Test fixture that verifies key names resolve only through defined Key enum names.
+    /// </summary>
+    public sealed class KeyNameResolverTests
+    {
+        /// <summary>
+        /// Tests that a defined key name resolves regardless of the casing it was written in.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenADefinedNameInAnyCasing_ResolvesTheKey()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("space");
+
+            Assert.IsTrue(resolved);
+            Assert.AreEqual(Key.Space, key);
+        }
+
+        /// <summary>
+        /// Tests that surrounding whitespace does not stop a defined name from resolving.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenAPaddedName_ResolvesTheKey()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("  W  ");
+
+            Assert.IsTrue(resolved);
+            Assert.AreEqual(Key.W, key);
+        }
+
+        /// <summary>
+        /// Tests that "Return" keeps resolving to Enter, the alias callers already relied on.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenTheReturnAlias_ResolvesEnter()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("Return");
+
+            Assert.IsTrue(resolved);
+            Assert.AreEqual(Key.Enter, key);
+        }
+
+        /// <summary>
+        /// Tests that a bare digit is rejected instead of being read as an enum ordinal.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenAnOrdinal_IsRejected()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("3");
+
+            Assert.IsFalse(resolved);
+            Assert.AreEqual(Key.None, key);
+        }
+
+        /// <summary>
+        /// Tests that a signed ordinal is rejected: Enum.TryParse accepted it as a value too.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenASignedOrdinal_IsRejected()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("-1");
+
+            Assert.IsFalse(resolved);
+            Assert.AreEqual(Key.None, key);
+        }
+
+        /// <summary>
+        /// Tests that an ordinal outside the enum is rejected rather than producing an undefined key.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenAnUndefinedOrdinal_IsRejected()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("300");
+
+            Assert.IsFalse(resolved);
+            Assert.AreEqual(Key.None, key);
+        }
+
+        /// <summary>
+        /// Tests that comma-separated names are rejected instead of being OR-ed into one value.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenCommaSeparatedNames_IsRejected()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("Space,Enter");
+
+            Assert.IsFalse(resolved);
+            Assert.AreEqual(Key.None, key);
+        }
+
+        /// <summary>
+        /// Tests that the placeholder None value is rejected: it names no physical key.
+        /// </summary>
+        [Test]
+        public void Resolve_WhenGivenNone_IsRejected()
+        {
+            (bool resolved, Key key) = KeyNameResolver.Resolve("None");
+
+            Assert.IsFalse(resolved);
+            Assert.AreEqual(Key.None, key);
+        }
+    }
+}
+#endif

Assets/Tests/PlayMode/RecordInputKeyFilterRejectionTests.cs

@@ -0,0 +1,57 @@
+#if ULOOP_HAS_INPUT_SYSTEM
+#nullable enable
+using System.Collections;
+using System.Threading;
+using System.Threading.Tasks;
+using NUnit.Framework;
+using UnityEngine.TestTools;
+
+using io.github.hatayama.UnityCliLoop.FirstPartyTools;
+using io.github.hatayama.UnityCliLoop.ToolContracts;
+
+namespace io.github.hatayama.UnityCliLoop.Tests.PlayMode
+{
+    /// <summary>
+    /// Test fixture that verifies record-input refuses to start when the key filter names no key.
+    /// Runs in PlayMode because the rejection sits behind the PlayMode preflight.
+    /// </summary>
+    public sealed class RecordInputKeyFilterRejectionTests
+    {
+        [TearDown]
+        public void TearDown()
+        {
+            // A regression here starts a real recording, which would leak into the next test.
+            if (InputRecorder.IsRecording)
+            {
+                InputRecorder.StopRecording();
+            }
+        }
+
+        /// <summary>
+        /// Tests that a key filter naming no key fails the command instead of recording every key.
+        /// </summary>
+        [UnityTest]
+        public IEnumerator RecordInput_WhenTheKeyFilterNamesNoKey_FailsWithoutRecording()
+        {
+            RecordInputSchema request = new()
+            {
+                Action = RecordInputAction.Start,
+                Keys = "3",
+                DelaySeconds = 0,
+                ShowOverlay = false
+            };
+
+            Task<RecordInputResponse> execution = new RecordInputUseCase().RecordInputAsync(request, CancellationToken.None);
+            while (!execution.IsCompleted)
+            {
+                yield return null;
+            }
+
+            RecordInputResponse response = execution.Result;
+            Assert.IsFalse(response.Success, response.Message);
+            StringAssert.Contains("Invalid key name(s) in the keys filter: 3", response.Message);
+            Assert.IsFalse(InputRecorder.IsRecording);
+        }
+    }
+}
+#endif

Packages/src/Editor/FirstPartyTools/Common/InputRecording/InputRecordingFileHelper.cs

@@ -92,14 +92,20 @@ public static string ResolveLatestRecording(string inputPath)
             return files.OrderByDescending(f => File.GetLastWriteTimeUtc(f)).First();
         }
 
-        public static HashSet<Key>? ParseKeyFilter(string keys)
+        /// <summary>
+        /// Parses the comma-separated key filter into the keys to record. Entries that name no key
+        /// are reported rather than skipped: dropping them silently would record a different set of
+        /// keys than the caller asked for, and dropping all of them would record every key.
+        /// </summary>
+        public static KeyFilterParseResult ParseKeyFilter(string keys)
         {
             if (string.IsNullOrEmpty(keys))
             {
-                return null;
+                return new KeyFilterParseResult(null, Array.Empty<string>());
             }
 
             HashSet<Key> filter = new();
+            List<string> invalidKeyNames = new();
             string[] parts = keys.Split(',');
 
             for (int i = 0; i < parts.Length; i++)
@@ -110,17 +116,26 @@ public static string ResolveLatestRecording(string inputPath)
                     continue;
                 }
 
-                if (Enum.TryParse<Key>(trimmed, ignoreCase: true, out Key key) && key != Key.None)
-                {
-                    filter.Add(key);
-                }
-                else
+                (bool resolved, Key key) = KeyNameResolver.Resolve(trimmed);
+                if (!resolved)
                 {
-                    Debug.LogWarning($"[InputRecordingFileHelper] Unknown key name in filter: '{trimmed}'");
+                    invalidKeyNames.Add(trimmed);
+                    continue;
                 }
+
+                filter.Add(key);
+            }
+
+            if (filter.Count == 0 && invalidKeyNames.Count == 0)
+            {
+                // Every entry was empty (for example "," or " "), so the filter would fall back to
+                // recording every key while the response looked like no filter was ever given.
+                // Why not reject the empty entries themselves: a trailing comma in "W," is harmless
+                // once at least one entry names a key.
+                invalidKeyNames.Add(keys);
             }
 
-            return filter.Count > 0 ? filter : null;
+            return new KeyFilterParseResult(filter.Count > 0 ? filter : null, invalidKeyNames);
         }
     }
 }