From f99c55abf08c30a9fdc65ba0397299f68d9ba2f2 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 01:37:43 +0900 Subject: [PATCH 01/13] Add --expect to pause-point-status A hit can be inspected either by awaiting it or by querying it later, but only await-pause-point could assert on the captured values. Callers that queried an already-recorded hit had to re-read CapturedVariables by hand. pause-point-status now accepts --expect and reports Expectations / AllExpectationsPassed under the same field names await-pause-point uses, so one response shape works for both commands. Expectations are evaluated before the --captured-variable-names filter and the --captured-variables mode run, since those can narrow the response or strip values that an expectation targets. A failed expectation leaves the exit code at 0: whether the query succeeded and whether the state matched are separate questions. Also guards both runner-owned commands against help/parser drift: every flag their --help advertises is now asserted to be accepted by their own parser. --- .../projectrunner/native_command_help.go | 1 + .../pause_point_cli_options_contract_test.go | 47 +++++ .../pause_point_status_expect_test.go | 195 ++++++++++++++++++ .../projectrunner/pause_point_types.go | 13 ++ .../projectrunner/pause_point_wait.go | 20 +- 5 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go diff --git a/cli/project-runner/internal/projectrunner/native_command_help.go b/cli/project-runner/internal/projectrunner/native_command_help.go index e2b83e0570..b36efe92e6 100644 --- a/cli/project-runner/internal/projectrunner/native_command_help.go +++ b/cli/project-runner/internal/projectrunner/native_command_help.go @@ -39,6 +39,7 @@ var runnerNativeCommandOptions = map[string][]string{ "--" + PausePointIDFlagName, "--" + tooldocs.PausePointCapturedVariablesFlagName, "--" + tooldocs.PausePointCapturedVariableNamesFlagName, + "--" + tooldocs.PausePointExpectFlagName, }, } diff --git a/cli/project-runner/internal/projectrunner/pause_point_cli_options_contract_test.go b/cli/project-runner/internal/projectrunner/pause_point_cli_options_contract_test.go index 57f35940a4..2405418b28 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_cli_options_contract_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_cli_options_contract_test.go @@ -3,6 +3,7 @@ package projectrunner import ( "testing" + "github.com/hatayama/unity-cli-loop/common/clicore" "github.com/hatayama/unity-cli-loop/common/tooldocs" ) @@ -118,3 +119,49 @@ func TestPausePointSharedHelpOptionsAreAcceptedByTheWaitParser(t *testing.T) { } } } + +// runnerNativeCommandSampleArgs supplies one accepted argv form per flag advertised by a +// runner-owned native command's --help. A flag added to runnerNativeCommandOptions without an entry +// here fails the contract test below rather than silently going unchecked. +var runnerNativeCommandSampleArgs = map[string][]string{ + "--" + PausePointIDFlagName: {"--id", "marker"}, + "--" + PausePointTimeoutFlagName: {"--timeout-seconds", "5"}, + "--" + PausePointLogsMaxCountFlagName: {"--matching-logs-max-count", "3"}, + "--" + tooldocs.PausePointCapturedVariablesFlagName: {"--captured-variables", "names"}, + "--" + tooldocs.PausePointCapturedVariableNamesFlagName: {"--captured-variable-names", "score"}, + "--" + tooldocs.PausePointExpectFlagName: {"--expect", "score=1"}, + "--" + tooldocs.PausePointTriggerFlagName: {"--trigger", "focus-window"}, + "--" + tooldocs.PausePointResumePlayFlagName: {"--resume-play"}, +} + +// Verifies every flag pause-point-status advertises in its --help output is actually accepted by its +// own parser. The status parser is a separate switch from the await one, so a flag added to the help +// table alone would be advertised and then rejected as unknown on use. +func TestPausePointStatusHelpOptionsAreAcceptedByTheStatusParser(t *testing.T) { + for _, option := range runnerNativeCommandOptions[clicore.PausePointStatusUserCommandName] { + args, ok := runnerNativeCommandSampleArgs[option] + if !ok { + t.Fatalf("no sample argv for %s: add one to runnerNativeCommandSampleArgs", option) + } + + statusArgs := append([]string{"--" + PausePointIDFlagName, "marker"}, args...) + if _, err := parsePausePointStatusOptions(statusArgs); err != nil { + t.Errorf("pause-point-status parser rejected advertised option %s: %v", option, err) + } + } +} + +// Verifies the same for await-pause-point, whose help table is the larger of the two. +func TestPausePointAwaitHelpOptionsAreAcceptedByTheWaitParser(t *testing.T) { + for _, option := range runnerNativeCommandOptions[clicore.PausePointAwaitCommandName] { + args, ok := runnerNativeCommandSampleArgs[option] + if !ok { + t.Fatalf("no sample argv for %s: add one to runnerNativeCommandSampleArgs", option) + } + + waitArgs := append([]string{"--" + PausePointIDFlagName, "marker"}, args...) + if _, err := parseWaitForPausePointOptions(waitArgs); err != nil { + t.Errorf("await-pause-point parser rejected advertised option %s: %v", option, err) + } + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go b/cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go new file mode 100644 index 0000000000..1fe91211f5 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go @@ -0,0 +1,195 @@ +package projectrunner + +import ( + "bytes" + "context" + "encoding/json" + "strings" + "testing" + + "github.com/hatayama/unity-cli-loop/common/clicore" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// pausePointStatusExpectPayload decodes only the expectation fields the CLI adds on top of the +// Unity status response, so these tests assert the wire names --expect callers actually read. +type pausePointStatusExpectPayload struct { + Status string `json:"Status"` + CapturedVariables []pausePointCapturedVariable `json:"CapturedVariables"` + Expectations []pausePointExpectationResult `json:"Expectations"` + AllExpectationsPassed *bool `json:"AllExpectationsPassed"` +} + +func stubPausePointStatusHitWithSpeed(t *testing.T, speed string) { + t.Helper() + + originalQuery := queryPausePointStatus + t.Cleanup(func() { + queryPausePointStatus = originalQuery + }) + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Success: true, + Id: id, + Status: pausePointStatusHit, + IsEnabled: true, + IsHit: true, + HitCount: 1, + CapturedVariables: []pausePointCapturedVariable{ + {Name: "speed", Scope: "Local", TypeName: "System.Int32", Value: pausePointVariableValue(speed)}, + }, + }, nil + } +} + +func runPausePointStatusForExpect(t *testing.T, args []string) (int, string) { + t.Helper() + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runPausePointStatusCommand( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + args, + &stdout, + &stderr) + if stderr.Len() > 0 { + t.Logf("stderr: %s", stderr.String()) + } + return code, stdout.String() +} + +func decodePausePointStatusExpectPayload(t *testing.T, output string) pausePointStatusExpectPayload { + t.Helper() + + var payload pausePointStatusExpectPayload + if err := json.Unmarshal([]byte(output), &payload); err != nil { + t.Fatalf("stdout is not valid JSON: %v\n%s", err, output) + } + return payload +} + +// Verifies pause-point-status --expect reports each expectation and the aggregate verdict using the +// same field names await-pause-point already emits, so one query shape works for both commands. +func TestRunPausePointStatusEvaluatesExpectations(t *testing.T) { + stubPausePointStatusHitWithSpeed(t, "5") + + code, output := runPausePointStatusForExpect(t, []string{"--id", "jump", "--expect", "speed=5"}) + + if code != 0 { + t.Fatalf("expected success, got %d: %s", code, output) + } + payload := decodePausePointStatusExpectPayload(t, output) + if len(payload.Expectations) != 1 { + t.Fatalf("expected 1 expectation, got %#v", payload.Expectations) + } + expectation := payload.Expectations[0] + if expectation.Name != "speed" || expectation.Expected != "5" || expectation.Actual != "5" || + !expectation.Found || !expectation.Passed { + t.Fatalf("expectation mismatch: %#v", expectation) + } + if payload.AllExpectationsPassed == nil || !*payload.AllExpectationsPassed { + t.Fatalf("AllExpectationsPassed mismatch: %#v", payload.AllExpectationsPassed) + } +} + +// Verifies a failed expectation still exits 0: querying the hit succeeded, and the expectation +// verdict is reported in the payload rather than through the process exit code. +func TestRunPausePointStatusFailedExpectationKeepsExitCodeZero(t *testing.T) { + stubPausePointStatusHitWithSpeed(t, "5") + + code, output := runPausePointStatusForExpect(t, []string{"--id", "jump", "--expect", "speed=9"}) + + if code != 0 { + t.Fatalf("expected exit code 0 for a failed expectation, got %d: %s", code, output) + } + payload := decodePausePointStatusExpectPayload(t, output) + if len(payload.Expectations) != 1 || payload.Expectations[0].Passed { + t.Fatalf("expected a failing expectation, got %#v", payload.Expectations) + } + if payload.AllExpectationsPassed == nil || *payload.AllExpectationsPassed { + t.Fatalf("AllExpectationsPassed must be present and false: %#v", payload.AllExpectationsPassed) + } +} + +// Verifies expectations are evaluated before --captured-variables names strips values, so the +// requested value is still compared even though the response itself reports names only. +func TestRunPausePointStatusEvaluatesExpectationsBeforeNamesModeStripsValues(t *testing.T) { + stubPausePointStatusHitWithSpeed(t, "5") + + code, output := runPausePointStatusForExpect( + t, []string{"--id", "jump", "--captured-variables", "names", "--expect", "speed=5"}) + + if code != 0 { + t.Fatalf("expected success, got %d: %s", code, output) + } + payload := decodePausePointStatusExpectPayload(t, output) + if len(payload.Expectations) != 1 || !payload.Expectations[0].Passed || + payload.Expectations[0].Actual != "5" { + t.Fatalf("expectation must be evaluated against the unfiltered value: %#v", payload.Expectations) + } + if len(payload.CapturedVariables) != 1 || payload.CapturedVariables[0].Value != nil { + t.Fatalf("names mode must still strip Value: %#v", payload.CapturedVariables) + } +} + +// Verifies expectations are evaluated before the --captured-variable-names filter narrows the +// response, so an --expect target that was not also requested by name is not reported as missing. +func TestRunPausePointStatusEvaluatesExpectationsBeforeNameFilter(t *testing.T) { + stubPausePointStatusHitWithSpeed(t, "5") + + code, output := runPausePointStatusForExpect( + t, []string{"--id", "jump", "--captured-variable-names", "health", "--expect", "speed=5"}) + + if code != 0 { + t.Fatalf("expected success, got %d: %s", code, output) + } + payload := decodePausePointStatusExpectPayload(t, output) + if len(payload.Expectations) != 1 || !payload.Expectations[0].Found || + !payload.Expectations[0].Passed { + t.Fatalf("expectation must survive the name filter: %#v", payload.Expectations) + } +} + +// Verifies a status query without --expect emits neither expectation field, so callers that never +// asked for expectations see no schema change. +func TestRunPausePointStatusOmitsExpectationFieldsWithoutExpectFlag(t *testing.T) { + stubPausePointStatusHitWithSpeed(t, "5") + + code, output := runPausePointStatusForExpect(t, []string{"--id", "jump"}) + + if code != 0 { + t.Fatalf("expected success, got %d: %s", code, output) + } + if strings.Contains(output, "Expectations") || strings.Contains(output, "AllExpectationsPassed") { + t.Fatalf("expectation fields must be omitted without --expect: %s", output) + } +} + +// Verifies an invalid --expect value is rejected by pause-point-status the same way +// await-pause-point rejects it, instead of being reported as an unknown option. +func TestRunPausePointStatusRejectsInvalidExpectValue(t *testing.T) { + stubPausePointStatusHitWithSpeed(t, "5") + + code, output := runPausePointStatusForExpect(t, []string{"--id", "jump", "--expect", "speed"}) + + if code == 0 { + t.Fatalf("expected failure for an --expect value without '=': %s", output) + } +} + +// Verifies pause-point-status --help advertises --expect, so the flag is discoverable from the +// command that accepts it. +func TestPausePointStatusHelpAdvertisesExpect(t *testing.T) { + var stdout bytes.Buffer + printNativeCommandHelp(clicore.PausePointStatusUserCommandName, &stdout) + + if !strings.Contains(stdout.String(), "--expect") { + t.Fatalf("pause-point-status help must list --expect: %s", stdout.String()) + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index 874ca1174b..eeb3b96fb1 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -60,6 +60,19 @@ type pausePointStatusResponse struct { ResumePlayResult *pausePointResumePlayResult `json:"ResumePlayResult,omitempty"` } +// pausePointStatusResult wraps a status response with the CLI-evaluated --expect verdicts. +// pause-point-status marshals the Unity response directly, so it needs this wrapper to carry the +// two extra fields; the names match pausePointWaitResult's so one query shape reads both commands. +type pausePointStatusResult struct { + pausePointStatusResponse + + // Both fields are omitted unless --expect was passed, and AllExpectationsPassed is a pointer + // for the same reason as on pausePointWaitResult: to distinguish "no --expect given" from + // "the given expectations failed". + Expectations []pausePointExpectationResult `json:"Expectations,omitempty"` + AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` +} + type pausePointEditorState struct { IsPlaying bool `json:"IsPlaying"` IsPaused bool `json:"IsPaused"` diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index 2ed2c8ec31..4e759cab21 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -61,6 +61,7 @@ type pausePointStatusOptions struct { id string capturedVariablesMode pausePointCapturedVariablesMode capturedVariableNames []string + expectations []pausePointExpectation } func normalizePausePointStatusResponse(response pausePointStatusResponse) pausePointStatusResponse { @@ -171,10 +172,21 @@ func runPausePointStatusCommand( } response = normalizePausePointStatusResponse(response) response = filterPausePointCapturedVariableHistory(response) + // Evaluated against the raw CapturedVariables, before the filters below can narrow or strip + // values, for the same reason as on the await path (runWaitForPausePoint): otherwise an --expect + // target not also requested via --captured-variable-names, or whose value names mode stripped, + // would be reported as missing or failing. + expectations := evaluatePausePointExpectations(response.CapturedVariables, options.expectations) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) - result, err := json.Marshal(response) + // Expectation verdicts never change the exit code: whether the query succeeded and whether the + // captured state matched are separate questions, as on await-pause-point. + result, err := json.Marshal(pausePointStatusResult{ + pausePointStatusResponse: response, + Expectations: expectations, + AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), + }) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ ProjectRoot: connection.ProjectRoot, @@ -397,6 +409,12 @@ func parsePausePointStatusOptions(args []string) (pausePointStatusOptions, error options.capturedVariablesMode = mode case tooldocs.PausePointCapturedVariableNamesFlagName: options.capturedVariableNames = parsePausePointCapturedVariableNames(value) + case tooldocs.PausePointExpectFlagName: + expectation, parseErr := parsePausePointExpectFlagValue(value) + if parseErr != nil { + return pausePointStatusOptions{}, parseErr + } + options.expectations = append(options.expectations, expectation) default: return pausePointStatusOptions{}, pausePointUnknownOptionError(clicore.PausePointStatusUserCommandName, name) } From d1b4991fb9eb3e6024bd56833c0117594fb85bdc Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 01:43:40 +0900 Subject: [PATCH 02/13] Name the real owner of a misplaced pause-point flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Passing an enable-pause-point flag to a query command used to be answered with "the installed project runner may be older than the docs — update the CLI". That hint fired exactly when it was false: the flag does exist in this build, just on another command. The one case it was written for (documentation ahead of the binary) is not reachable in practice, because the package and the runner are pinned together and each runner-owned command renders its own --help from the same table its parser reads. Unknown flags are now split in two. A flag another pause-point command accepts names that command, and an enable-time setting whose value Unity reports back on every later response also says it does not need to be passed again. A flag that exists nowhere is reported as a plain unknown option pointing at --help. The owning command is resolved through a fixed search order rather than map iteration, so a flag several commands accept always reports the same owner. --- .../projectrunner/native_command_help.go | 19 --- .../pause_point_unknown_option.go | 112 ++++++++++++++++ .../pause_point_unknown_option_test.go | 122 ++++++++++++++++++ .../projectrunner/pause_point_wait_test.go | 18 +-- 4 files changed, 243 insertions(+), 28 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_unknown_option.go create mode 100644 cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go diff --git a/cli/project-runner/internal/projectrunner/native_command_help.go b/cli/project-runner/internal/projectrunner/native_command_help.go index b36efe92e6..b35957333b 100644 --- a/cli/project-runner/internal/projectrunner/native_command_help.go +++ b/cli/project-runner/internal/projectrunner/native_command_help.go @@ -1,12 +1,9 @@ package projectrunner import ( - "fmt" "io" "sort" - clierrors "github.com/hatayama/unity-cli-loop/common/errors" - "github.com/hatayama/unity-cli-loop/common/clicore" "github.com/hatayama/unity-cli-loop/common/tooldocs" ) @@ -88,22 +85,6 @@ func printNativeCommandHelp(command string, stdout io.Writer) { } } -// pausePointUnknownOptionError reports an unrecognized flag for a runner-owned native -// command. The hint calls out an outdated installed project runner as the likely cause when -// the flag is documented in the skill but this runner build predates it, rather than leaving -// the caller to guess between a typo and a stale binary. -func pausePointUnknownOptionError(command string, name string) *clierrors.ArgumentError { - return &clierrors.ArgumentError{ - Message: fmt.Sprintf( - "Unknown option %q for %s. If the skill documentation mentions this option, the installed "+ - "project runner may be older than the docs — check 'uloop --version' and update the CLI.", - "--"+name, command), - Option: "--" + name, - Command: command, - NextActions: []string{fmt.Sprintf("Run `uloop %s --help` to inspect supported options.", command)}, - } -} - func sortedNativeCommandOptions(options []string) []string { result := append([]string{}, options...) sort.Strings(result) diff --git a/cli/project-runner/internal/projectrunner/pause_point_unknown_option.go b/cli/project-runner/internal/projectrunner/pause_point_unknown_option.go new file mode 100644 index 0000000000..4f88374f27 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_unknown_option.go @@ -0,0 +1,112 @@ +package projectrunner + +import ( + "fmt" + "strings" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + + "github.com/hatayama/unity-cli-loop/common/clicore" + "github.com/hatayama/unity-cli-loop/common/tooldocs" +) + +// pausePointFlagOwnerSearchOrder fixes the order in which an unknown flag's owning command is +// resolved. Several pause-point commands accept the same flag name (--id and --timeout-seconds +// exist on more than one), so a map-order search would report a different owner from run to run. +// enable-pause-point comes first because it owns the largest flag set and is the command whose +// flags are most often reached for while inspecting an already-armed marker. +var pausePointFlagOwnerSearchOrder = []string{ + pausePointEnableCommandName, + clicore.PausePointAwaitCommandName, + clicore.PausePointStatusUserCommandName, +} + +// pausePointCarriedOverEnableFlagNames are the enable-pause-point flags whose values Unity reports +// back on every later status response (as Mode, MaxHistory, MaxPreviewElements and TimeoutSeconds). +// Passing one of these to a query command is not just misplaced, it is unnecessary — which is the +// part a caller cannot infer from "wrong command" alone. +var pausePointCarriedOverEnableFlagNames = []string{ + "mode", + "max-history", + "max-preview-elements", + PausePointTimeoutFlagName, +} + +// pausePointUnknownOptionError reports an unrecognized flag for a runner-owned native command. +// A flag that belongs to another pause-point command is reported as such, since naming the real +// owner (and, for enable-time settings, saying the value is already in this response) is what lets +// the caller recover without a second round trip. A flag that exists nowhere is reported as a plain +// unknown option: it cannot be a case of documentation running ahead of this build. +func pausePointUnknownOptionError(command string, name string) *clierrors.ArgumentError { + message := fmt.Sprintf("Unknown option %q for %s.", "--"+name, command) + if owner, ok := pausePointFlagOwnerCommand(name); ok && owner != command { + message = fmt.Sprintf("--%s is an %s option, not a %s one.", name, owner, command) + if owner == pausePointEnableCommandName && isPausePointCarriedOverEnableFlag(name) { + message += " The value passed to " + pausePointEnableCommandName + + " is already applied to the response of this command, so it does not need to be passed again here." + } + } + + return &clierrors.ArgumentError{ + Message: message, + Option: "--" + name, + Command: command, + NextActions: []string{fmt.Sprintf("Run `uloop %s --help` to list the accepted options.", command)}, + } +} + +// pausePointFlagOwnerCommand reports which pause-point command accepts the flag, searching in a +// fixed order so the answer never depends on map iteration. +func pausePointFlagOwnerCommand(name string) (string, bool) { + for _, command := range pausePointFlagOwnerSearchOrder { + for _, flagName := range pausePointCommandFlagNames(command) { + if flagName == name { + return command, true + } + } + } + return "", false +} + +// pausePointCommandFlagNames lists the flag names a pause-point command accepts, without the "--" +// prefix. Both sources are the same tables the commands' own --help output is built from, so a flag +// added to a command becomes recognizable here without a second registration. +func pausePointCommandFlagNames(command string) []string { + if command == pausePointEnableCommandName { + return pausePointEnableFlagNames() + } + + names := make([]string, 0, len(runnerNativeCommandOptions[command])) + for _, option := range runnerNativeCommandOptions[command] { + names = append(names, strings.TrimPrefix(option, "--")) + } + return names +} + +// pausePointEnableFlagNames lists enable-pause-point's CLI-only flags plus the ones derived from its +// Unity schema, since the misuse this message exists for (--max-preview-elements on a query command) +// is a schema-derived flag. +func pausePointEnableFlagNames() []string { + names := make([]string, 0) + for _, option := range tooldocs.PausePointEnableCLIOnlyOptions() { + names = append(names, option.FlagName) + } + + tool, ok := clicore.FindTool(clicore.LoadDefaultTools(), pausePointEnableCommandName) + if !ok { + return names + } + for propertyName, property := range tool.EffectiveInputSchema().Properties { + names = append(names, tooldocs.OptionNameForProperty(tool.Name, propertyName, property)) + } + return names +} + +func isPausePointCarriedOverEnableFlag(name string) bool { + for _, flagName := range pausePointCarriedOverEnableFlagNames { + if flagName == name { + return true + } + } + return false +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go b/cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go new file mode 100644 index 0000000000..340871bc24 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go @@ -0,0 +1,122 @@ +package projectrunner + +import ( + "encoding/json" + "strings" + "testing" +) + +// Verifies a flag that belongs to enable-pause-point names its real owner and states that the value +// given at enable time already shows up here, which is the round trip the original message cost: +// the flag exists, so "your runner may be outdated" was never the answer. +func TestParsePausePointStatusUnknownOptionNamesEnableAsTheOwner(t *testing.T) { + _, err := parsePausePointStatusOptions([]string{"--id", "jump", "--max-preview-elements", "5"}) + + if err == nil { + t.Fatal("expected error for an enable-pause-point flag passed to pause-point-status") + } + message := err.Error() + if !strings.Contains(message, "--max-preview-elements is an enable-pause-point option, not a pause-point-status one.") { + t.Errorf("owner sentence missing: %s", message) + } + if !strings.Contains( + message, + "The value passed to enable-pause-point is already applied to the response of this command, "+ + "so it does not need to be passed again here.") { + t.Errorf("carry-over sentence missing: %s", message) + } + if strings.Contains(message, "older than the docs") { + t.Errorf("stale-runner hint must be gone: %s", message) + } +} + +// Verifies an enable-pause-point flag whose value is not carried into this command's response names +// the owner without claiming a carry-over that does not happen. +func TestParsePausePointStatusUnknownOptionOmitsCarryOverForNonCarriedFlags(t *testing.T) { + _, err := parsePausePointStatusOptions([]string{"--id", "jump", "--line", "42"}) + + if err == nil { + t.Fatal("expected error for --line passed to pause-point-status") + } + message := err.Error() + if !strings.Contains(message, "--line is an enable-pause-point option, not a pause-point-status one.") { + t.Errorf("owner sentence missing: %s", message) + } + if strings.Contains(message, "already applied to the response") { + t.Errorf("carry-over sentence must not be claimed for --line: %s", message) + } +} + +// Verifies a flag owned by another runner-owned command names that command, so a flag borrowed from +// await-pause-point is not reported as an enable-pause-point one. +func TestParsePausePointStatusUnknownOptionNamesAwaitAsTheOwner(t *testing.T) { + _, err := parsePausePointStatusOptions( + []string{"--id", "jump", "--" + PausePointLogsMaxCountFlagName, "3"}) + + if err == nil { + t.Fatal("expected error for an await-pause-point flag passed to pause-point-status") + } + if !strings.Contains( + err.Error(), + "--"+PausePointLogsMaxCountFlagName+" is an await-pause-point option, not a pause-point-status one.") { + t.Errorf("owner sentence missing: %s", err.Error()) + } +} + +// Verifies the owner reported for a flag several commands accept is fixed rather than dependent on +// map iteration order, so the same misuse always produces the same message. +func TestPausePointUnknownOptionOwnerIsDeterministic(t *testing.T) { + const flagName = PausePointTimeoutFlagName + + first, ok := pausePointFlagOwnerCommand(flagName) + if !ok { + t.Fatalf("--%s must resolve to an owning command", flagName) + } + if first != pausePointEnableCommandName { + t.Errorf("owner of --%s must be the first command in the fixed search order, got %q", flagName, first) + } + for attempt := 0; attempt < 20; attempt++ { + owner, _ := pausePointFlagOwnerCommand(flagName) + if owner != first { + t.Fatalf("owner of --%s changed between calls: %q then %q", flagName, first, owner) + } + } +} + +// Verifies the carry-over sentence is only claimed for the enable-time settings the status response +// actually reports back, so the message never promises evidence the response cannot show. +func TestPausePointCarriedOverEnableFlagsAreVisibleInTheStatusResponse(t *testing.T) { + response, err := json.Marshal(pausePointStatusResponse{ + Mode: "continuous", + MaxHistory: 20, + MaxPreviewElements: 5, + TimeoutSeconds: 30, + }) + if err != nil { + t.Fatalf("failed to marshal status response: %v", err) + } + + carriedOverFields := map[string]string{ + "mode": "Mode", + "max-history": "MaxHistory", + "max-preview-elements": "MaxPreviewElements", + "timeout-seconds": "TimeoutSeconds", + } + if len(carriedOverFields) != len(pausePointCarriedOverEnableFlagNames) { + t.Fatalf("carry-over flag list changed: %v", pausePointCarriedOverEnableFlagNames) + } + for _, flagName := range pausePointCarriedOverEnableFlagNames { + field, ok := carriedOverFields[flagName] + if !ok { + t.Errorf("--%s is described as carried over but has no known status response field", flagName) + continue + } + if !strings.Contains(string(response), `"`+field+`"`) { + t.Errorf("status response does not report %s for --%s", field, flagName) + } + if owner, ok := pausePointFlagOwnerCommand(flagName); !ok || owner != pausePointEnableCommandName { + t.Errorf("--%s is listed as a carried-over enable flag but resolves to owner %q (found=%v)", + flagName, owner, ok) + } + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go index 55eb131e5a..275927d1e3 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -1126,27 +1126,27 @@ func TestRunProjectLocalPausePointStatusRespectsToolSettings(t *testing.T) { } } -// Verifies an unrecognized flag on await-pause-point/pause-point-status carries a hint -// that the installed project runner may be older than the skill docs. -func TestParseUnknownOptionErrorsIncludeOutdatedRunnerHint(t *testing.T) { - wantHint := "Unknown option \"--bogus-flag\" for await-pause-point. If the skill documentation mentions this option, the installed project runner may be older than the docs — check 'uloop --version' and update the CLI." +// Verifies a flag that exists nowhere is reported as a plain unknown option: no stale-runner hint, +// since the flag being absent from every command means the docs cannot be ahead of this build. +func TestParseUnknownOptionErrorsOmitStaleRunnerHint(t *testing.T) { + wantMessage := `Unknown option "--bogus-flag" for await-pause-point.` _, err := parseWaitForPausePointOptions([]string{"--id", "jump", "--bogus-flag", "value"}) if err == nil { t.Fatal("expected error for unknown flag") } - if err.Error() != wantHint { - t.Fatalf("await-pause-point hint mismatch: %v", err) + if err.Error() != wantMessage { + t.Fatalf("await-pause-point message mismatch: %v", err) } - wantStatusHint := "Unknown option \"--bogus-flag\" for pause-point-status. If the skill documentation mentions this option, the installed project runner may be older than the docs — check 'uloop --version' and update the CLI." + wantStatusMessage := `Unknown option "--bogus-flag" for pause-point-status.` _, statusErr := parsePausePointStatusOptions([]string{"--id", "jump", "--bogus-flag", "value"}) if statusErr == nil { t.Fatal("expected error for unknown flag") } - if statusErr.Error() != wantStatusHint { - t.Fatalf("pause-point-status hint mismatch: %v", statusErr) + if statusErr.Error() != wantStatusMessage { + t.Fatalf("pause-point-status message mismatch: %v", statusErr) } } From 576ccbd92ad32cd527b97868f94cfc5732d47c53 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 01:45:13 +0900 Subject: [PATCH 03/13] Report which requested captured-variable names matched nothing --captured-variable-names only ever showed the names that did match, so a typo in one of several names looked identical to a clean partial capture: the caller had to diff the flag value against the response by hand. CapturedVariableNamesNotFound now lists the requested names that matched no captured variable, in the order they were requested. CapturedVariableNameFilterNoMatch keeps its exact meaning and stays in place, so nothing that reads it changes; both fields appear when nothing matched at all. A name matched in any history frame counts as found, matching what the filter itself considers a match. The shared status-response contract fixture is deliberately left alone: like CapturedVariableNameFilterNoMatch and TriggerResult, this field is produced by the CLI, and the fixture pins the Unity-side contract only. --- ...se_point_captured_variable_names_filter.go | 27 +++++++++++++ ...int_captured_variable_names_filter_test.go | 39 +++++++++++++++++++ .../projectrunner/pause_point_types.go | 7 ++++ 3 files changed, 73 insertions(+) diff --git a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go index 5978bb45e5..4c3c22e28e 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go +++ b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go @@ -38,13 +38,17 @@ func filterPausePointCapturedVariablesByName( nameSet[name] = struct{}{} } + matchedNames := map[string]struct{}{} + filteredCurrent, currentMatchCount := filterCapturedVariablesByNameSet(response.CapturedVariables, nameSet) + collectCapturedVariableNames(filteredCurrent, matchedNames) response.CapturedVariables = filteredCurrent totalMatchCount := currentMatchCount history := make([]pausePointCapturedHistoryFrame, len(response.CapturedVariableHistory)) for index, frame := range response.CapturedVariableHistory { filteredFrame, frameMatchCount := filterCapturedVariablesByNameSet(frame.CapturedVariables, nameSet) + collectCapturedVariableNames(filteredFrame, matchedNames) frame.CapturedVariables = filteredFrame totalMatchCount += frameMatchCount history[index] = frame @@ -52,9 +56,32 @@ func filterPausePointCapturedVariablesByName( response.CapturedVariableHistory = history response.CapturedVariableNameFilterNoMatch = totalMatchCount == 0 + response.CapturedVariableNamesNotFound = unmatchedCapturedVariableNames(names, matchedNames) return response } +// unmatchedCapturedVariableNames lists the requested names that matched nothing, keeping the order +// they were requested in so the report reads back against the flag value the caller wrote. A name +// matched anywhere — current variables or any history frame — counts as found. +func unmatchedCapturedVariableNames(names []string, matchedNames map[string]struct{}) []string { + notFound := make([]string, 0, len(names)) + for _, name := range names { + if _, ok := matchedNames[name]; !ok { + notFound = append(notFound, name) + } + } + if len(notFound) == 0 { + return nil + } + return notFound +} + +func collectCapturedVariableNames(variables []pausePointCapturedVariable, names map[string]struct{}) { + for _, variable := range variables { + names[variable.Name] = struct{}{} + } +} + func filterCapturedVariablesByNameSet( variables []pausePointCapturedVariable, nameSet map[string]struct{}, diff --git a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go index 6d5d5f7956..2904c8f72f 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go @@ -72,6 +72,45 @@ func TestFilterPausePointCapturedVariablesByName(t *testing.T) { } }) + t.Run("reports which requested names matched nothing, in the requested order", func(t *testing.T) { + result := filterPausePointCapturedVariablesByName( + baseResponse(), []string{"shield", "velocity", "armor"}) + if len(result.CapturedVariableNamesNotFound) != 2 || + result.CapturedVariableNamesNotFound[0] != "shield" || + result.CapturedVariableNamesNotFound[1] != "armor" { + t.Fatalf("expected the unmatched names in request order: %#v", result.CapturedVariableNamesNotFound) + } + if result.CapturedVariableNameFilterNoMatch { + t.Fatal("a partial match must not set CapturedVariableNameFilterNoMatch") + } + }) + + t.Run("a name matched only in history is not reported as missing", func(t *testing.T) { + response := baseResponse() + response.CapturedVariables = nil + result := filterPausePointCapturedVariablesByName(response, []string{"health"}) + if len(result.CapturedVariableNamesNotFound) != 0 { + t.Fatalf("a history-only match must count as found: %#v", result.CapturedVariableNamesNotFound) + } + }) + + t.Run("all names missing sets both the list and the no-match flag", func(t *testing.T) { + result := filterPausePointCapturedVariablesByName(baseResponse(), []string{"shield", "armor"}) + if len(result.CapturedVariableNamesNotFound) != 2 { + t.Fatalf("expected both names reported missing: %#v", result.CapturedVariableNamesNotFound) + } + if !result.CapturedVariableNameFilterNoMatch { + t.Fatal("expected CapturedVariableNameFilterNoMatch to stay true when nothing matches") + } + }) + + t.Run("every name matching leaves the missing list empty", func(t *testing.T) { + result := filterPausePointCapturedVariablesByName(baseResponse(), []string{"velocity", "health"}) + if result.CapturedVariableNamesNotFound != nil { + t.Fatalf("expected no missing names: %#v", result.CapturedVariableNamesNotFound) + } + }) + t.Run("empty names list leaves the response unchanged", func(t *testing.T) { original := baseResponse() result := filterPausePointCapturedVariablesByName(original, nil) diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index eeb3b96fb1..67b9007381 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -51,6 +51,13 @@ type pausePointStatusResponse struct { // CapturedVariables array for "nothing was captured at this hit". CapturedVariableNameFilterNoMatch bool `json:"CapturedVariableNameFilterNoMatch,omitempty"` + // CapturedVariableNamesNotFound is set by the CLI, not Unity: the requested + // --captured-variable-names that matched no captured variable, in the order they were + // requested. Without it a partial match is indistinguishable from a full one, since the + // response only carries the names that did match and CapturedVariableNameFilterNoMatch covers + // the all-or-nothing case. Both are emitted when nothing matched at all. + CapturedVariableNamesNotFound []string `json:"CapturedVariableNamesNotFound,omitempty"` + // TriggerResult is set by the CLI, not Unity, only when --trigger was passed. It is omitted // entirely otherwise, so callers that never use --trigger see no schema change at all. TriggerResult *pausePointTriggerResult `json:"TriggerResult,omitempty"` From 5e2cafa9e588d1f1225eef4dbc831a9bef12d361 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 01:53:52 +0900 Subject: [PATCH 04/13] Report the pause point that refused a paused-PlayMode call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A simulate or record call made while a pause point holds PlayMode paused is refused before it does anything, and until now the only trace of which pause point caused that was inside the message text. Callers that need to tell "refused by the marker I am waiting on" from "refused for some other reason" had no option but to match on the wording — and PausePointId stays null here, because that field reports a marker hit *during* the call, not a refusal. Every response that shares this preflight — simulate-keyboard, simulate-mouse-input, simulate-mouse-ui, record-input and replay-input — now carries RejectedByActivePausePointId. The preflight returns its own result type instead of the general-purpose ValidationResult, which has no place to put an id and is used by argument validation that must not grow the field. The paused branches are unreachable from EditMode tests, since PlayMode never runs there, so the decision is split into Evaluate(isPlaying, isPaused, activePausePointId, ...) with the editor-state read left in the thin wrapper. --- .../PausePointRejectionResponseFieldTests.cs | 67 ++++++++++++++++ .../PlayModeToolPreflightResultTests.cs | 79 +++++++++++++++++++ .../PlayModeToolPreflightServiceTests.cs | 5 +- .../Preflight/PlayModeToolPreflightResult.cs | 54 +++++++++++++ .../PlayModeToolPreflightResult.cs.meta | 11 +++ .../Preflight/PlayModeToolPreflightService.cs | 47 ++++++++--- .../RecordInput/RecordInputResponse.cs | 8 ++ .../RecordInput/RecordInputUseCase.cs | 5 +- .../ReplayInput/ReplayInputResponse.cs | 8 ++ .../ReplayInput/ReplayInputUseCase.cs | 5 +- .../SimulateKeyboardResponse.cs | 8 ++ .../SimulateKeyboardUseCase.cs | 5 +- .../SimulateMouseInputResponse.cs | 8 ++ .../SimulateMouseInputUseCase.cs | 5 +- .../MouseUiSimulationResponseFactory.cs | 19 +++++ .../MouseUiSimulationValidator.cs | 4 +- .../SimulateMouseUiResponse.cs | 8 ++ 17 files changed, 320 insertions(+), 26 deletions(-) create mode 100644 Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs create mode 100644 Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs.meta diff --git a/Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs b/Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs new file mode 100644 index 0000000000..f65719dadc --- /dev/null +++ b/Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs @@ -0,0 +1,67 @@ +using Newtonsoft.Json; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that pins the wire name of the pause-point rejection field on every tool response + /// a --trigger can dispatch, because the CLI matches on that exact name to detect a trigger that + /// was refused before it ran. + /// + public class PausePointRejectionResponseFieldTests + { + private const string ExpectedJsonFragment = "\"RejectedByActivePausePointId\":\"marker\""; + + [Test] + public void SimulateKeyboardResponse_WhenRejectedByPausePoint_SerializesTheRejectionField() + { + // Verifies simulate-keyboard's rejection response carries the field under the name the CLI reads. + string json = JsonConvert.SerializeObject( + new SimulateKeyboardResponse { Success = false, RejectedByActivePausePointId = "marker" }); + + Assert.That(json, Does.Contain(ExpectedJsonFragment)); + } + + [Test] + public void SimulateMouseInputResponse_WhenRejectedByPausePoint_SerializesTheRejectionField() + { + // Verifies simulate-mouse-input's rejection response carries the field under the name the CLI reads. + string json = JsonConvert.SerializeObject( + new SimulateMouseInputResponse { Success = false, RejectedByActivePausePointId = "marker" }); + + Assert.That(json, Does.Contain(ExpectedJsonFragment)); + } + + [Test] + public void SimulateMouseUiResponse_WhenRejectedByPausePoint_SerializesTheRejectionField() + { + // Verifies simulate-mouse-ui's rejection response carries the field under the name the CLI reads. + string json = JsonConvert.SerializeObject( + new SimulateMouseUiResponse { Success = false, RejectedByActivePausePointId = "marker" }); + + Assert.That(json, Does.Contain(ExpectedJsonFragment)); + } + + [Test] + public void RecordInputResponse_WhenRejectedByPausePoint_SerializesTheRejectionField() + { + // Verifies record-input reports the same structured rejection, since it shares the preflight that refuses it. + string json = JsonConvert.SerializeObject( + new RecordInputResponse { Success = false, RejectedByActivePausePointId = "marker" }); + + Assert.That(json, Does.Contain(ExpectedJsonFragment)); + } + + [Test] + public void ReplayInputResponse_WhenRejectedByPausePoint_SerializesTheRejectionField() + { + // Verifies replay-input reports the same structured rejection: it is a realistic --trigger target. + string json = JsonConvert.SerializeObject( + new ReplayInputResponse { Success = false, RejectedByActivePausePointId = "marker" }); + + Assert.That(json, Does.Contain(ExpectedJsonFragment)); + } + } +} diff --git a/Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs b/Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs new file mode 100644 index 0000000000..3a3dc95d3c --- /dev/null +++ b/Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs @@ -0,0 +1,79 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies the preflight result carries the active pause point's id as a + /// structured field, not only inside the human-readable rejection message. + /// + public class PlayModeToolPreflightResultTests + { + private const string PausedActionDescription = "simulating keyboard input"; + + [Test] + public void Evaluate_WhenPlayModeIsNotActive_ReportsNoPausePointId() + { + // Verifies a not-active rejection has nothing to do with a pause point, so the structured field stays empty. + PlayModeToolPreflightResult result = PlayModeToolPreflightService.Evaluate( + isPlaying: false, + isPaused: false, + activePausePointId: "marker", + pausedActionDescription: PausedActionDescription); + + Assert.That(result.IsValid, Is.False); + Assert.That(result.ErrorMessage, Is.EqualTo(PlayModeToolPreflightService.PlayModeNotActiveMessage)); + Assert.That(result.RejectedByActivePausePointId, Is.Null); + } + + [Test] + public void Evaluate_WhenPausedByPausePoint_ReportsThatPausePointId() + { + // Verifies a pause-point-owned pause reports the id in a field a caller can compare, since the message alone forces string matching. + PlayModeToolPreflightResult result = PlayModeToolPreflightService.Evaluate( + isPlaying: true, + isPaused: true, + activePausePointId: "marker", + pausedActionDescription: PausedActionDescription); + + Assert.That(result.IsValid, Is.False); + Assert.That(result.RejectedByActivePausePointId, Is.EqualTo("marker")); + Assert.That( + result.ErrorMessage, + Is.EqualTo(PlayModeToolPreflightService.FormatPausePointPausedMessage("marker", PausedActionDescription))); + } + + [Test] + public void Evaluate_WhenPausedWithoutPausePoint_ReportsNoPausePointId() + { + // Verifies a manual pause is not attributed to a pause point, so a caller cannot mistake it for one of its own markers. + PlayModeToolPreflightResult result = PlayModeToolPreflightService.Evaluate( + isPlaying: true, + isPaused: true, + activePausePointId: string.Empty, + pausedActionDescription: PausedActionDescription); + + Assert.That(result.IsValid, Is.False); + Assert.That(result.RejectedByActivePausePointId, Is.Null); + Assert.That( + result.ErrorMessage, + Is.EqualTo(PlayModeToolPreflightService.FormatPausedMessage(PausedActionDescription))); + } + + [Test] + public void Evaluate_WhenPlayingAndNotPaused_Succeeds() + { + // Verifies the success path stays a plain success with no rejection details attached. + PlayModeToolPreflightResult result = PlayModeToolPreflightService.Evaluate( + isPlaying: true, + isPaused: false, + activePausePointId: "marker", + pausedActionDescription: PausedActionDescription); + + Assert.That(result.IsValid, Is.True); + Assert.That(result.ErrorMessage, Is.Empty); + Assert.That(result.RejectedByActivePausePointId, Is.Null); + } + } +} diff --git a/Assets/Tests/Editor/PlayModeToolPreflightServiceTests.cs b/Assets/Tests/Editor/PlayModeToolPreflightServiceTests.cs index b7b47dde9e..aa2b2e8599 100644 --- a/Assets/Tests/Editor/PlayModeToolPreflightServiceTests.cs +++ b/Assets/Tests/Editor/PlayModeToolPreflightServiceTests.cs @@ -1,7 +1,6 @@ using NUnit.Framework; using io.github.hatayama.UnityCliLoop.FirstPartyTools; -using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.Tests.Editor { @@ -17,7 +16,7 @@ public class PlayModeToolPreflightServiceTests public void RequireActive_WhenEditModeIsNotPlaying_ReturnsNotActiveFailure() { // Verifies the active-only preflight fails with the exact wire-visible not-active message. - ValidationResult result = PlayModeToolPreflightService.RequireActive(); + PlayModeToolPreflightResult result = PlayModeToolPreflightService.RequireActive(); Assert.That(result.IsValid, Is.False); Assert.That(result.ErrorMessage, Is.EqualTo(ExpectedNotActiveMessage)); @@ -27,7 +26,7 @@ public void RequireActive_WhenEditModeIsNotPlaying_ReturnsNotActiveFailure() public void RequireActiveAndNotPaused_WhenEditModeIsNotPlaying_ReturnsNotActiveFailure() { // Verifies the paused-aware preflight also fails with the exact not-active message when PlayMode is inactive. - ValidationResult result = PlayModeToolPreflightService.RequireActiveAndNotPaused(RecordInputUseCase.PausedActionDescription); + PlayModeToolPreflightResult result = PlayModeToolPreflightService.RequireActiveAndNotPaused(RecordInputUseCase.PausedActionDescription); Assert.That(result.IsValid, Is.False); Assert.That(result.ErrorMessage, Is.EqualTo(ExpectedNotActiveMessage)); diff --git a/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs new file mode 100644 index 0000000000..6a7e908832 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs @@ -0,0 +1,54 @@ +#nullable enable +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Outcome of a PlayMode preflight check: whether the tool may run, the wire-visible rejection + /// message, and — when a pause point is what refused the call — that pause point's id as a + /// separate field. The id exists on its own because callers (the CLI's --trigger diagnosis + /// above all) have to tell "refused by the marker I am waiting on" from "refused for some other + /// reason", and the message text is too brittle to match on. + /// + public class PlayModeToolPreflightResult + { + public bool IsValid { get; } + + /// Rejection message, empty on success. Never null, so callers can assign it to a + /// tool response's non-nullable Message without a null check. + public string ErrorMessage { get; } + + /// Id of the pause point holding PlayMode paused, null for every other outcome. + public string? RejectedByActivePausePointId { get; } + + private PlayModeToolPreflightResult(bool isValid, string errorMessage, string? rejectedByActivePausePointId) + { + IsValid = isValid; + ErrorMessage = errorMessage; + RejectedByActivePausePointId = rejectedByActivePausePointId; + } + + /// Creates the success outcome. + public static PlayModeToolPreflightResult Success() + { + return new PlayModeToolPreflightResult(true, string.Empty, null); + } + + /// Creates a rejection that has no pause point behind it. + public static PlayModeToolPreflightResult Failure(string errorMessage) + { + Debug.Assert(!string.IsNullOrEmpty(errorMessage), "errorMessage must not be null or empty"); + return new PlayModeToolPreflightResult(false, errorMessage, null); + } + + /// Creates a rejection caused by an active pause point, recording which one. + public static PlayModeToolPreflightResult FailureRejectedByPausePoint( + string errorMessage, + string pausePointId) + { + Debug.Assert(!string.IsNullOrEmpty(errorMessage), "errorMessage must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(pausePointId), "pausePointId must not be null or empty"); + return new PlayModeToolPreflightResult(false, errorMessage, pausePointId); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs.meta b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs.meta new file mode 100644 index 0000000000..1a1047e344 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightResult.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b6c9bb51e0b3d4cd4b1d88aa86b681b2 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs index c09fa2e183..4300b40430 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs @@ -19,39 +19,60 @@ public static class PlayModeToolPreflightService /// /// Fails when PlayMode is not active. Use for tools that tolerate paused PlayMode. /// - public static ValidationResult RequireActive() + public static PlayModeToolPreflightResult RequireActive() { if (!EditorApplication.isPlaying) { - return ValidationResult.Failure(PlayModeNotActiveMessage); + return PlayModeToolPreflightResult.Failure(PlayModeNotActiveMessage); } - return ValidationResult.Success(); + return PlayModeToolPreflightResult.Success(); } /// /// Fails when PlayMode is not active, or is active but paused. The paused-message suffix /// describes the blocked action in the caller's vocabulary (for example "recording input"). /// - public static ValidationResult RequireActiveAndNotPaused(string pausedActionDescription) + public static PlayModeToolPreflightResult RequireActiveAndNotPaused(string pausedActionDescription) + { + return Evaluate( + EditorApplication.isPlaying, + EditorApplication.isPaused, + UloopPausePointRegistry.GetActivePausePointId(), + pausedActionDescription); + } + + /// + /// Decides a paused-aware preflight outcome from already-read editor state. Separated from + /// RequireActiveAndNotPaused so the paused branches — the only ones that produce a pause + /// point id — are reachable from EditMode tests, where PlayMode is never actually running. + /// + public static PlayModeToolPreflightResult Evaluate( + bool isPlaying, + bool isPaused, + string activePausePointId, + string pausedActionDescription) { Debug.Assert(!string.IsNullOrEmpty(pausedActionDescription), "pausedActionDescription must not be null or empty"); - if (!EditorApplication.isPlaying) + if (!isPlaying) + { + return PlayModeToolPreflightResult.Failure(PlayModeNotActiveMessage); + } + + if (!isPaused) { - return ValidationResult.Failure(PlayModeNotActiveMessage); + return PlayModeToolPreflightResult.Success(); } - if (EditorApplication.isPaused) + if (string.IsNullOrEmpty(activePausePointId)) { - string activePausePointId = UloopPausePointRegistry.GetActivePausePointId(); - string message = string.IsNullOrEmpty(activePausePointId) - ? FormatPausedMessage(pausedActionDescription) - : FormatPausePointPausedMessage(activePausePointId, pausedActionDescription); - return ValidationResult.Failure(message); + return PlayModeToolPreflightResult.Failure(FormatPausedMessage(pausedActionDescription)); } - return ValidationResult.Success(); + return PlayModeToolPreflightResult.FailureRejectedByPausePoint( + FormatPausePointPausedMessage(activePausePointId, pausedActionDescription), + activePausePointId); } /// diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputResponse.cs b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputResponse.cs index b0f770acab..ed1eee1ac0 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputResponse.cs @@ -14,5 +14,13 @@ public class RecordInputResponse : UnityCliLoopToolResponse public string? OutputPath { get; set; } public int? TotalFrames { get; set; } public float? DurationSeconds { get; set; } + + /// + /// Id of the pause point that refused this call before it ran anything, null otherwise. A + /// refusal means nothing was started, so a caller reading only Success would miss that the + /// action never happened. The CLI's --trigger diagnosis compares this against the marker it + /// awaits. + /// + public string? RejectedByActivePausePointId { get; set; } } } diff --git a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs index 44b1b66f3b..2ca4bc94f9 100644 --- a/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RecordInput/RecordInputUseCase.cs @@ -98,14 +98,15 @@ private static async Task ExecuteStartAsync( RecordInputSchema request, CancellationToken ct) { - ValidationResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); + PlayModeToolPreflightResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) { return new RecordInputResponse { Success = false, Message = preflight.ErrorMessage, - Action = RecordInputAction.Start.ToString() + Action = RecordInputAction.Start.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId }; } diff --git a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs index 7b50abd38f..773aec61e8 100644 --- a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs @@ -16,5 +16,13 @@ public class ReplayInputResponse : UnityCliLoopToolResponse public int? TotalFrames { get; set; } public float? Progress { get; set; } public bool? IsReplaying { get; set; } + + /// + /// Id of the pause point that refused this call before it ran anything, null otherwise. A + /// refusal means nothing was started, so a caller reading only Success would miss that the + /// action never happened. The CLI's --trigger diagnosis compares this against the marker it + /// awaits. + /// + public string? RejectedByActivePausePointId { get; set; } } } diff --git a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs index 9e3ce27179..d23f144e15 100644 --- a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs @@ -94,14 +94,15 @@ public async Task ReplayInputAsync( #if ULOOP_HAS_INPUT_SYSTEM private static ReplayInputResponse ExecuteStart(ReplayInputSchema request) { - ValidationResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); + PlayModeToolPreflightResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) { return new ReplayInputResponse { Success = false, Message = preflight.ErrorMessage, - Action = ReplayInputAction.Start.ToString() + Action = ReplayInputAction.Start.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId }; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs index 52d1151b42..b9ff6b3c7c 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs @@ -15,6 +15,14 @@ public class SimulateKeyboardResponse : UnityCliLoopToolResponse public string Action { get; set; } = ""; public string? KeyName { get; set; } public bool InterruptedByPausePoint { get; set; } + /// + /// Id of the pause point that refused this call before it ran anything, null otherwise. + /// Distinct from PausePointId, which reports a marker hit *during* the call: a refusal means + /// no input was injected at all, so a caller reading only Success would miss that the action + /// never happened. The CLI's --trigger diagnosis compares this against the marker it awaits. + /// + public string? RejectedByActivePausePointId { get; set; } + public string? PausePointId { get; set; } public int? PausePointHitCount { get; set; } public List? PausePointHits { get; set; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index 6b7601e988..3520090e25 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -49,7 +49,7 @@ public async Task ExecuteAsync( // ReleaseAll must work while paused so agents can recover stuck device state after a // pause-point interruption without first resuming PlayMode. - ValidationResult preflight = parameters.Action == UnityCliLoopKeyboardAction.ReleaseAll + PlayModeToolPreflightResult preflight = parameters.Action == UnityCliLoopKeyboardAction.ReleaseAll ? PlayModeToolPreflightService.RequireActive() : PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) @@ -58,7 +58,8 @@ public async Task ExecuteAsync( { Success = false, Message = preflight.ErrorMessage, - Action = parameters.Action.ToString() + Action = parameters.Action.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId }; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs index 72982359eb..71cd0e8ab9 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs @@ -26,6 +26,14 @@ public class SimulateMouseInputResponse : UnityCliLoopToolResponse public float? InjectedUnityPositionY { get; set; } public string CoordinateConversionFormula { get; set; } = ""; public bool InterruptedByPausePoint { get; set; } + /// + /// Id of the pause point that refused this call before it ran anything, null otherwise. + /// Distinct from PausePointId, which reports a marker hit *during* the call: a refusal means + /// no input was injected at all, so a caller reading only Success would miss that the action + /// never happened. The CLI's --trigger diagnosis compares this against the marker it awaits. + /// + public string? RejectedByActivePausePointId { get; set; } + public string? PausePointId { get; set; } public int? PausePointHitCount { get; set; } public List? PausePointHits { get; set; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs index 9bc0fa565d..ff007ac80f 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs @@ -46,14 +46,15 @@ public async Task ExecuteAsync( #else string correlationId = UnityCliLoopConstants.GenerateCorrelationId(); - ValidationResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); + PlayModeToolPreflightResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) { return new SimulateMouseInputResponse { Success = false, Message = preflight.ErrorMessage, - Action = parameters.Action.ToString() + Action = parameters.Action.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId }; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs index 6e739c7c0c..356eead17d 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs @@ -24,6 +24,25 @@ internal static SimulateMouseUiResponse CreateFailure( }; } + /// + /// Creates the failure response for a rejected PlayMode preflight, carrying the rejecting + /// pause point's id through to the wire. Separate from CreateFailure (rather than an optional + /// argument on it) so the other failure sites cannot accidentally claim a pause-point cause. + /// + internal static SimulateMouseUiResponse CreatePreflightFailure( + MouseUiSimulationCommand parameters, + PlayModeToolPreflightResult preflight) + { + Debug.Assert(!preflight.IsValid, "CreatePreflightFailure must only be called for a rejected preflight"); + return new SimulateMouseUiResponse + { + Success = false, + Message = preflight.ErrorMessage, + Action = parameters.Action.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId + }; + } + internal static SimulateMouseUiResponse CreateFrameTimeoutResult( MouseAction action, Vector2 position, diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs index e3d85abcec..6c9035e143 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs @@ -16,11 +16,11 @@ internal static class MouseUiSimulationValidator EventSystem? eventSystem, string pausedActionDescription) { - ValidationResult playModeResult = + PlayModeToolPreflightResult playModeResult = PlayModeToolPreflightService.RequireActiveAndNotPaused(pausedActionDescription); if (!playModeResult.IsValid) { - return MouseUiSimulationResponseFactory.CreateFailure(parameters, playModeResult.ErrorMessage); + return MouseUiSimulationResponseFactory.CreatePreflightFailure(parameters, playModeResult); } if (eventSystem == null) diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs index 66fd34b283..8da0c6270a 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs @@ -19,6 +19,14 @@ public class SimulateMouseUiResponse : UnityCliLoopToolResponse public float? EndPositionX { get; set; } public float? EndPositionY { get; set; } public bool InterruptedByPausePoint { get; set; } + /// + /// Id of the pause point that refused this call before it ran anything, null otherwise. + /// Distinct from PausePointId, which reports a marker hit *during* the call: a refusal means + /// no input was injected at all, so a caller reading only Success would miss that the action + /// never happened. The CLI's --trigger diagnosis compares this against the marker it awaits. + /// + public string? RejectedByActivePausePointId { get; set; } + public string? PausePointId { get; set; } public int? PausePointHitCount { get; set; } public List? PausePointHits { get; set; } From 84d1bc7e3406606d52130751d683aa36b8aba5da Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 01:54:24 +0900 Subject: [PATCH 05/13] Stop dropping Unity's warning from an await-pause-point hit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the Unity status response and the CLI's hit payload expose a field named Warning, and the CLI's outer field shadows the embedded Unity one. Any hit where the CLI had something to say therefore silently discarded Unity's enable-time diagnostic — the physics-callback dispatch warning among them. The outer Warning is now the join of Unity's text and the CLI's, the same treatment the enable path already gives it. The failed-log-fetch branch gained a Warning field for the same reason: it had none at all, so Unity's warning was the only one that could ever appear there, and only by accident of shadowing. --- .../projectrunner/pause_point_wait.go | 27 +++++---- .../pause_point_warning_join_test.go | 55 +++++++++++++++++++ 2 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_warning_join_test.go diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index 4e759cab21..ac445a31fb 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -232,29 +232,32 @@ func runWaitForPausePoint( // Best-effort: a hit must stay a success even if Unity is busy while paused. // On fetch failure MatchingLogs is omitted entirely, so an empty array always // means "the fetch succeeded and no matching log exists". - var payload any = response logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) - switch { - case logsErr == nil: + var payload any + if logsErr == nil { payload = pausePointWaitResult{ pausePointStatusResponse: response, MatchingLogs: logs.Logs, - Warning: buildPausePointWarning(logs, response.HitCount), - Expectations: expectations, - AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), + Warning: joinPausePointWarnings( + response.Warning, + buildPausePointWarning(logs, response.HitCount)), + Expectations: expectations, + AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), } - case len(expectations) > 0: - // Best-effort: a failed log fetch must not also drop --expect results, since that is - // the only evidence a caller asked for by name in this branch. Uses an anonymous - // struct (not pausePointWaitResult) so MatchingLogs is omitted entirely rather than - // serialized as an empty array, preserving "empty array only means a successful - // fetch with no matches". + } else { + // Best-effort: a failed log fetch must not also drop the CLI-side evidence — the + // --expect results a caller asked for by name, or a warning about the hit itself. Uses + // an anonymous struct (not pausePointWaitResult) so MatchingLogs is omitted entirely + // rather than serialized as an empty array, preserving "empty array only means a + // successful fetch with no matches". payload = struct { pausePointStatusResponse + Warning string `json:"Warning,omitempty"` Expectations []pausePointExpectationResult `json:"Expectations,omitempty"` AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` }{ pausePointStatusResponse: response, + Warning: response.Warning, Expectations: expectations, AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), } diff --git a/cli/project-runner/internal/projectrunner/pause_point_warning_join_test.go b/cli/project-runner/internal/projectrunner/pause_point_warning_join_test.go new file mode 100644 index 0000000000..4f7dbb09f3 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_warning_join_test.go @@ -0,0 +1,55 @@ +package projectrunner + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +func runAwaitWithoutTrigger(t *testing.T) string { + t.Helper() + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runWaitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + matchingLogsMaxCount: 5, + }, &stdout, &stderr) + if code != 0 { + t.Fatalf("expected the hit to be a success, got %d with stderr %s", code, stderr.String()) + } + return stdout.String() +} + +// Verifies Unity's own warning reaches the caller on a hit. The CLI's Warning field shadows the +// embedded Unity one — both serialize as "Warning" — so Unity's text is dropped unless joined in. +func TestRunWaitForPausePointKeepsUnityWarningOnAHit(t *testing.T) { + stubPausePointHit(t, "Unity-side enable warning.") + stubPausePointMatchingLogs(t, nil) + + result := decodePausePointWaitResult(t, runAwaitWithoutTrigger(t)) + + if !strings.Contains(result.Warning, "Unity-side enable warning.") { + t.Errorf("Unity's warning was dropped: %q", result.Warning) + } +} + +// Verifies Unity's warning also survives the failed-log-fetch branch, which builds a different +// payload shape and previously had no Warning field at all. +func TestRunWaitForPausePointKeepsUnityWarningWhenTheLogFetchFails(t *testing.T) { + stubPausePointHit(t, "Unity-side enable warning.") + stubPausePointMatchingLogs(t, errors.New("unity busy")) + + result := decodePausePointWaitResult(t, runAwaitWithoutTrigger(t)) + + if !strings.Contains(result.Warning, "Unity-side enable warning.") { + t.Errorf("Unity's warning was dropped on the failed-fetch branch: %q", result.Warning) + } +} From ad4cf851b2938a559b8f32271651b20a7e602ff0 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 01:54:54 +0900 Subject: [PATCH 06/13] Surface a trigger that never ran on an await-pause-point hit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A marker armed on a line that runs every frame is hit before --trigger's input reaches Unity, so the trigger is refused for running while PlayMode is paused and injects nothing. The wait still reports Success:true and Status:Hit, with the refusal three levels down in TriggerResult.Response — where a reader looking at the top-level verdict never sees it, and the hit passes for input-driven evidence it is not. TriggerFailed is now promoted to the top level whenever a completed trigger reports Success:false or its dispatch failed, and a refusal by the very marker being awaited also produces a warning explaining how to arm the marker so the input is actually in effect. The id must match the awaited marker: a PlayMode paused by something else is a different problem. InterruptedByPausePoint is deliberately not part of the predicate. It marks the working case — the marker hit while the input was being applied — so an earlier draft that keyed on it warned about exactly the runs that behaved correctly. Known gap: a trigger that has not reported back within the join grace window has no known outcome, so it is neither warned about nor marked failed. Treating an unfinished trigger as failed would misreport every long-running hold. --- .../pause_point_trigger_diagnosis.go | 75 +++++ .../pause_point_trigger_diagnosis_test.go | 268 ++++++++++++++++++ .../projectrunner/pause_point_types.go | 7 + .../projectrunner/pause_point_wait.go | 12 +- 4 files changed, 358 insertions(+), 4 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go create mode 100644 cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go new file mode 100644 index 0000000000..39861450bb --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go @@ -0,0 +1,75 @@ +package projectrunner + +import "encoding/json" + +// pausePointTriggerResponseView is the part of a triggered command's response this diagnosis reads. +// Success is a pointer so a response that omits it is treated as "unknown", not as a failure. +type pausePointTriggerResponseView struct { + Success *bool `json:"Success"` + RejectedByActivePausePointId string `json:"RejectedByActivePausePointId"` +} + +// pausePointTriggerRefusalWarning warns about the case where the marker was hit before the trigger +// ran, so Unity refused the trigger for being called while PlayMode was paused and no input reached +// the game at all. The hit still reports success, which makes this indistinguishable from a real +// input-driven hit unless it is called out. +// +// The refusing pause point's id must match the marker being awaited: a PlayMode paused by some other +// marker is a different problem, and the advice below would be wrong for it. InterruptedByPausePoint +// is deliberately not consulted — it marks the working case, where the marker was hit while the +// input was being applied. +func pausePointTriggerRefusalWarning(result *pausePointTriggerResult, awaitedPausePointID string) string { + response, ok := decodePausePointTriggerResponse(result) + if !ok || response.Success == nil || *response.Success { + return "" + } + if response.RejectedByActivePausePointId != awaitedPausePointID { + return "" + } + + return "The marker was hit before the trigger ran, so Unity refused the trigger for running " + + "while PlayMode was paused and no input reached the game. This hit is not evidence about the " + + "trigger's input: hold the key down with a separate simulate-keyboard KeyDown call before " + + "arming the marker, or move the marker to a line reached after the input is applied. A marker " + + "placed after the input would have paused with the input already in effect." +} + +// pausePointTriggerFailedPointer reports whether the trigger failed, or nil when there is nothing to +// report: no trigger ran, or it never finished so its outcome is unknown. +func pausePointTriggerFailedPointer(result *pausePointTriggerResult) *bool { + if !pausePointTriggerFailed(result) { + return nil + } + failed := true + return &failed +} + +// pausePointTriggerFailed reports whether a completed trigger is known to have failed, either +// because its dispatch failed outright (Error) or because it ran and reported Success:false. +func pausePointTriggerFailed(result *pausePointTriggerResult) bool { + if result == nil || !result.Completed { + return false + } + if result.Error != "" { + return true + } + + response, ok := decodePausePointTriggerResponse(result) + return ok && response.Success != nil && !*response.Success +} + +// decodePausePointTriggerResponse reads the fields this diagnosis needs out of the triggered +// command's raw response, leaving TriggerResult.Response itself untouched. An unfinished trigger or +// an unparseable response yields no view: neither can be diagnosed, and guessing either way is worse +// than reporting nothing. +func decodePausePointTriggerResponse(result *pausePointTriggerResult) (pausePointTriggerResponseView, bool) { + if result == nil || !result.Completed || len(result.Response) == 0 { + return pausePointTriggerResponseView{}, false + } + + view := pausePointTriggerResponseView{} + if err := json.Unmarshal(result.Response, &view); err != nil { + return pausePointTriggerResponseView{}, false + } + return view, true +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go new file mode 100644 index 0000000000..9af4f3edaf --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go @@ -0,0 +1,268 @@ +package projectrunner + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "strings" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// pausePointRejectedTriggerResponse is the response Unity produces when a simulate command is +// refused before it runs anything because a pause point is holding PlayMode paused. Only Success and +// RejectedByActivePausePointId matter to the diagnosis; the rest is kept for realism. +func pausePointRejectedTriggerResponse(rejectedByID string) string { + return `{"Success":false,` + + `"Message":"PlayMode is paused because pause point '` + rejectedByID + `' is active ...",` + + `"InterruptedByPausePoint":false,"PausePointId":null,` + + `"RejectedByActivePausePointId":"` + rejectedByID + `"}` +} + +func stubPausePointHit(t *testing.T, unityWarning string) { + t.Helper() + + originalQuery := queryPausePointStatus + t.Cleanup(func() { + queryPausePointStatus = originalQuery + }) + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Success: true, + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + Warning: unityWarning, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } +} + +func stubPausePointMatchingLogs(t *testing.T, fetchError error) { + t.Helper() + + originalFetch := fetchMatchingLogs + t.Cleanup(func() { + fetchMatchingLogs = originalFetch + }) + + fetchMatchingLogs = func( + ctx context.Context, + connection unityipc.Connection, + searchText string, + maxCount int, + ) (pausePointMatchingLogsResult, error) { + if fetchError != nil { + return pausePointMatchingLogsResult{}, fetchError + } + return pausePointMatchingLogsResult{ + SearchText: searchText, + TotalCount: 1, + DisplayedCount: 1, + MaxCount: maxCount, + Logs: []pausePointMatchingLog{{Type: "Log", Message: "[jump] hit"}}, + }, nil + } +} + +// stubPausePointTriggerDispatch makes the triggered command produce triggerStdout, which the wait +// path turns into TriggerResult.Response exactly as a real dispatch would. +func stubPausePointTriggerDispatch(t *testing.T, triggerStdout string) { + t.Helper() + + originalDispatch := dispatchPausePointTriggerCommand + t.Cleanup(func() { + dispatchPausePointTriggerCommand = originalDispatch + }) + + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + _, _ = stdout.Write([]byte(triggerStdout)) + return 0 + } +} + +func runAwaitWithStubbedTrigger(t *testing.T) (int, string) { + t.Helper() + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runWaitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + matchingLogsMaxCount: 5, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press", "--key", "W"}, + }, &stdout, &stderr) + if stderr.Len() > 0 { + t.Logf("stderr: %s", stderr.String()) + } + return code, stdout.String() +} + +func decodePausePointWaitResult(t *testing.T, output string) pausePointWaitResult { + t.Helper() + + result := pausePointWaitResult{} + if err := json.Unmarshal([]byte(output), &result); err != nil { + t.Fatalf("stdout parse failed: %v from %s", err, output) + } + return result +} + +// Verifies a trigger refused by the very marker being awaited is called out end to end: the hit +// still reads as a success, so without this the refusal stays buried in TriggerResult.Response. +func TestRunWaitForPausePointWarnsWhenTheTriggerWasRefusedByThisMarker(t *testing.T) { + stubPausePointHit(t, "") + stubPausePointMatchingLogs(t, nil) + stubPausePointTriggerDispatch(t, pausePointRejectedTriggerResponse("jump")) + + code, output := runAwaitWithStubbedTrigger(t) + + if code != 0 { + t.Fatalf("expected the hit to stay a success, got %d: %s", code, output) + } + result := decodePausePointWaitResult(t, output) + if !strings.Contains(result.Warning, "refused") { + t.Errorf("expected a refusal warning: %q", result.Warning) + } + if result.TriggerFailed == nil || !*result.TriggerFailed { + t.Errorf("TriggerFailed must be promoted to the top level: %#v", result.TriggerFailed) + } +} + +// Verifies the refusal warning still reaches the caller when the matching-log fetch fails, the +// branch that used to build a payload with no Warning field at all. +func TestRunWaitForPausePointKeepsTheRefusalWarningWhenTheLogFetchFails(t *testing.T) { + stubPausePointHit(t, "") + stubPausePointMatchingLogs(t, errors.New("unity busy")) + stubPausePointTriggerDispatch(t, pausePointRejectedTriggerResponse("jump")) + + _, output := runAwaitWithStubbedTrigger(t) + + if strings.Contains(output, `"MatchingLogs"`) { + t.Errorf("a failed fetch must omit MatchingLogs entirely: %s", output) + } + result := decodePausePointWaitResult(t, output) + if !strings.Contains(result.Warning, "refused") { + t.Errorf("expected the refusal warning despite the failed log fetch: %q", result.Warning) + } + if result.TriggerFailed == nil || !*result.TriggerFailed { + t.Errorf("TriggerFailed must survive the failed log fetch: %#v", result.TriggerFailed) + } +} + +// Verifies Unity's own warning survives alongside the CLI's. Both use the JSON name "Warning", where +// the CLI's outer field shadows the embedded Unity one, so Unity's text is lost unless joined in. +func TestRunWaitForPausePointKeepsUnityWarningAlongsideCliWarnings(t *testing.T) { + stubPausePointHit(t, "Unity-side enable warning.") + stubPausePointMatchingLogs(t, nil) + stubPausePointTriggerDispatch(t, pausePointRejectedTriggerResponse("jump")) + + _, output := runAwaitWithStubbedTrigger(t) + + result := decodePausePointWaitResult(t, output) + if !strings.Contains(result.Warning, "Unity-side enable warning.") { + t.Errorf("Unity's warning was dropped: %q", result.Warning) + } + if !strings.Contains(result.Warning, "refused") { + t.Errorf("the CLI warning was dropped: %q", result.Warning) + } +} + +// Verifies the refusal is only blamed on this wait when the refusing marker is the one being +// awaited, so a pause owned by some other marker does not produce advice about this one. +func TestPausePointTriggerRefusalWarningRequiresTheAwaitedMarker(t *testing.T) { + refusedByThisMarker := &pausePointTriggerResult{ + Completed: true, + Response: json.RawMessage(pausePointRejectedTriggerResponse("jump")), + } + if pausePointTriggerRefusalWarning(refusedByThisMarker, "jump") == "" { + t.Error("expected a warning when this marker refused the trigger") + } + + refusedByAnotherMarker := &pausePointTriggerResult{ + Completed: true, + Response: json.RawMessage(pausePointRejectedTriggerResponse("other-marker")), + } + if warning := pausePointTriggerRefusalWarning(refusedByAnotherMarker, "jump"); warning != "" { + t.Errorf("a refusal by another marker must not be warned about here: %q", warning) + } +} + +// Verifies a marker hit while the trigger's input was being applied produces no warning: that is +// the normal, working case, and the earlier draft predicate warned on exactly it. +func TestPausePointTriggerRefusalWarningIgnoresAMidExecutionInterruption(t *testing.T) { + interrupted := &pausePointTriggerResult{ + Completed: true, + Response: json.RawMessage( + `{"Success":true,"InterruptedByPausePoint":true,"PausePointId":"jump"}`), + } + + if warning := pausePointTriggerRefusalWarning(interrupted, "jump"); warning != "" { + t.Errorf("a mid-execution interruption is the normal case: %q", warning) + } +} + +// Verifies a trigger that never reported back within the grace window is neither warned about nor +// called failed: its outcome is unknown, and claiming failure would be as wrong as claiming success. +func TestPausePointTriggerDiagnosisTreatsAnIncompleteTriggerAsUnknown(t *testing.T) { + incomplete := &pausePointTriggerResult{Completed: false} + + if warning := pausePointTriggerRefusalWarning(incomplete, "jump"); warning != "" { + t.Errorf("an unfinished trigger cannot be diagnosed: %q", warning) + } + if pausePointTriggerFailed(incomplete) { + t.Error("an unfinished trigger has no known outcome") + } +} + +// Verifies the two failure shapes a completed trigger can have are both promoted, and a plain +// success is not. +func TestPausePointTriggerFailedCoversBothFailureShapes(t *testing.T) { + failedResponse := &pausePointTriggerResult{ + Completed: true, + Response: json.RawMessage(`{"Success":false,"Message":"no keyboard device found"}`), + } + if !pausePointTriggerFailed(failedResponse) { + t.Error("a completed trigger reporting Success:false has failed") + } + + failedDispatch := &pausePointTriggerResult{ + Completed: true, + Error: `{"Error":{"ErrorCode":"UNITY_NOT_REACHABLE"}}`, + } + if !pausePointTriggerFailed(failedDispatch) { + t.Error("a trigger whose dispatch failed has failed") + } + + succeeded := &pausePointTriggerResult{ + Completed: true, + Response: json.RawMessage(`{"Success":true}`), + } + if pausePointTriggerFailed(succeeded) { + t.Error("a successful trigger must not be reported as failed") + } + + if pausePointTriggerFailed(nil) { + t.Error("no trigger at all cannot have failed") + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index 67b9007381..4594e41f54 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -65,6 +65,13 @@ type pausePointStatusResponse struct { // ResumePlayResult is set by the CLI, not Unity, only when --resume-play was passed. It is // omitted entirely otherwise, matching TriggerResult's omit-when-unused contract. ResumePlayResult *pausePointResumePlayResult `json:"ResumePlayResult,omitempty"` + + // TriggerFailed is set by the CLI, not Unity, only when --trigger was passed and the trigger is + // known to have failed. It repeats at the top level what TriggerResult already carries three + // levels down, because the loss it guards against is a caller reading Success:true / Status:Hit + // and never opening TriggerResult at all. A pointer so the field is absent — rather than a + // misleading false — when no trigger ran or its outcome is unknown. + TriggerFailed *bool `json:"TriggerFailed,omitempty"` } // pausePointStatusResult wraps a status response with the CLI-evaluated --expect verdicts. diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index ac445a31fb..7b5f57d8b9 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -226,6 +226,7 @@ func runWaitForPausePoint( response.TriggerResult = triggerResult response.ResumePlayResult = resumeResult + response.TriggerFailed = pausePointTriggerFailedPointer(triggerResult) response = filterPausePointCapturedVariableHistory(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) @@ -240,7 +241,8 @@ func runWaitForPausePoint( MatchingLogs: logs.Logs, Warning: joinPausePointWarnings( response.Warning, - buildPausePointWarning(logs, response.HitCount)), + buildPausePointWarning(logs, response.HitCount), + pausePointTriggerRefusalWarning(triggerResult, options.id)), Expectations: expectations, AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), } @@ -257,9 +259,11 @@ func runWaitForPausePoint( AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` }{ pausePointStatusResponse: response, - Warning: response.Warning, - Expectations: expectations, - AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), + Warning: joinPausePointWarnings( + response.Warning, + pausePointTriggerRefusalWarning(triggerResult, options.id)), + Expectations: expectations, + AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), } } result, marshalErr := json.Marshal(payload) From b36f9c4c827f1fafb0665625f9e5315f8808e537 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 02:03:26 +0900 Subject: [PATCH 07/13] Add Unity meta files for the new preflight EditMode tests Unity generated these after the test scripts were added; without them the scripts have no stable GUID for other checkouts. --- .../PausePointRejectionResponseFieldTests.cs.meta | 11 +++++++++++ .../Editor/PlayModeToolPreflightResultTests.cs.meta | 11 +++++++++++ 2 files changed, 22 insertions(+) create mode 100644 Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs.meta create mode 100644 Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs.meta diff --git a/Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs.meta b/Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs.meta new file mode 100644 index 0000000000..4ac8bac88b --- /dev/null +++ b/Assets/Tests/Editor/PausePointRejectionResponseFieldTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 48fc9d8273cd94ee585a666e36a3e4e9 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs.meta b/Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs.meta new file mode 100644 index 0000000000..3fa024dc77 --- /dev/null +++ b/Assets/Tests/Editor/PlayModeToolPreflightResultTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: eb33fa476a1a34b47ba86f1626768877 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From f8970906f1aa0616cd9446b8345071be31f68a7d Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 02:03:37 +0900 Subject: [PATCH 08/13] Report a refused trigger from enable-pause-point --await too The trigger diagnosis (TriggerFailed plus the refusal warning) was wired only into await-pause-point, while enable-pause-point --await builds its hit payload in a second place. A real run of the documented form `enable-pause-point --await --trigger "simulate-keyboard ..."` therefore still reported a plain success with no warning, which is the exact scenario the diagnosis exists for. Rather than duplicating the wiring, both hit paths now build their payload through one buildPausePointHitPayload helper, so a field added for one command cannot silently stay missing from the other. The warning join also drops repeats, because the same Unity text can reach the payload from both the enable response and the status poll that observed the hit. --- .../projectrunner/pause_point_enable.go | 53 ++++------ ...pause_point_enable_await_diagnosis_test.go | 98 +++++++++++++++++++ .../projectrunner/pause_point_logs.go | 56 +++++++++++ .../projectrunner/pause_point_wait.go | 44 ++------- 4 files changed, 184 insertions(+), 67 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index c9a7e61202..50c78da18d 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "io" + "slices" "strings" "time" @@ -407,35 +408,18 @@ func runPausePointWaitAfterEnable( response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) - var payload any = response logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) - switch { - case logsErr == nil: - payload = pausePointWaitResult{ - pausePointStatusResponse: response, - MatchingLogs: logs.Logs, - Warning: joinPausePointWarnings(enableFields.Warning, buildPausePointWarning(logs, response.HitCount)), - Expectations: expectations, - AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), - } - case enableFields.Warning != "" || len(expectations) > 0: - // Best-effort like the plain await path: a failed log fetch must not also drop the - // enable-time warning or --expect results, since those are the only evidence left in - // this branch. Uses an anonymous struct (not pausePointWaitResult) so MatchingLogs is - // omitted entirely rather than serialized as an empty array, preserving "empty array - // only means a successful fetch with no matches". - payload = struct { - pausePointStatusResponse - Warning string `json:"Warning,omitempty"` - Expectations []pausePointExpectationResult `json:"Expectations,omitempty"` - AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` - }{ - pausePointStatusResponse: response, - Warning: enableFields.Warning, - Expectations: expectations, - AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), - } - } + // Unity's warning can come from either the enable response or the status poll that observed + // the hit, so both are passed; the join drops the repeat when they carry the same text. + payload := buildPausePointHitPayload(pausePointHitPayloadInputs{ + response: response, + logs: logs, + logsErr: logsErr, + unityWarning: joinPausePointWarnings(enableFields.Warning, response.Warning), + triggerResult: triggerResult, + awaitedPausePointID: options.id, + expectations: expectations, + }) result, marshalErr := json.Marshal(payload) if marshalErr != nil { clierrors.WriteClassifiedError(stderr, marshalErr, clierrors.ErrorContext{ @@ -477,12 +461,17 @@ func runPausePointWaitAfterEnable( return 1 } +// joinPausePointWarnings concatenates the warnings that apply to one response, dropping empty ones +// and repeats. Repeats are possible because the same text can reach a hit payload from two sources — +// the enable response and the status poll that observed the hit — and printing it twice reads as two +// separate problems. func joinPausePointWarnings(warnings ...string) string { - nonEmpty := make([]string, 0, len(warnings)) + unique := make([]string, 0, len(warnings)) for _, warning := range warnings { - if warning != "" { - nonEmpty = append(nonEmpty, warning) + if warning == "" || slices.Contains(unique, warning) { + continue } + unique = append(unique, warning) } - return strings.Join(nonEmpty, " ") + return strings.Join(unique, " ") } diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go b/cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go new file mode 100644 index 0000000000..ac876ab4aa --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go @@ -0,0 +1,98 @@ +package projectrunner + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// runEnableAwaitWithStubbedTrigger drives the enable-pause-point --await hit path, the second hit +// payload builder, with the same stubs the plain await path's tests use. +func runEnableAwaitWithStubbedTrigger(t *testing.T, enableWarning string) (int, string) { + t.Helper() + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runPausePointWaitAfterEnable( + context.Background(), + unityipc.Connection{}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + matchingLogsMaxCount: 5, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press", "--key", "W"}, + }, + enablePausePointPropagatedFields{Warning: enableWarning}, + &stdout, + &stderr, + ) + if stderr.Len() > 0 { + t.Logf("stderr: %s", stderr.String()) + } + return code, stdout.String() +} + +// Verifies enable-pause-point --await diagnoses a refused trigger exactly as await-pause-point does: +// the two commands build their hit payloads separately, so a diagnosis wired into only one is +// invisible to callers of the other, which is the form this project's own checklist exercises. +func TestRunPausePointWaitAfterEnableWarnsWhenTheTriggerWasRefusedByThisMarker(t *testing.T) { + stubPausePointHit(t, "") + stubPausePointMatchingLogs(t, nil) + stubPausePointTriggerDispatch(t, pausePointRejectedTriggerResponse("jump")) + + code, output := runEnableAwaitWithStubbedTrigger(t, "") + + if code != 0 { + t.Fatalf("expected the hit to stay a success, got %d: %s", code, output) + } + result := decodePausePointWaitResult(t, output) + if !strings.Contains(result.Warning, "refused") { + t.Errorf("expected a refusal warning: %q", result.Warning) + } + if result.TriggerFailed == nil || !*result.TriggerFailed { + t.Errorf("TriggerFailed must be promoted to the top level: %#v", result.TriggerFailed) + } +} + +// Verifies the enable-time warning survives next to the CLI's refusal warning, and that the +// refusal warning also survives a failed matching-log fetch. +func TestRunPausePointWaitAfterEnableKeepsEnableWarningWithTheRefusalWarning(t *testing.T) { + stubPausePointHit(t, "") + stubPausePointMatchingLogs(t, errors.New("unity busy")) + stubPausePointTriggerDispatch(t, pausePointRejectedTriggerResponse("jump")) + + _, output := runEnableAwaitWithStubbedTrigger(t, "Enable-time warning.") + + if strings.Contains(output, `"MatchingLogs"`) { + t.Errorf("a failed fetch must omit MatchingLogs entirely: %s", output) + } + result := decodePausePointWaitResult(t, output) + if !strings.Contains(result.Warning, "Enable-time warning.") { + t.Errorf("the enable-time warning was dropped: %q", result.Warning) + } + if !strings.Contains(result.Warning, "refused") { + t.Errorf("the refusal warning was dropped: %q", result.Warning) + } +} + +// Verifies a warning reported by both the enable response and the status poll is printed once: +// repeating identical text reads as two separate problems. +func TestRunPausePointWaitAfterEnableReportsARepeatedUnityWarningOnce(t *testing.T) { + stubPausePointHit(t, "Same Unity warning.") + stubPausePointMatchingLogs(t, nil) + stubPausePointTriggerDispatch(t, `{"Success":true}`) + + _, output := runEnableAwaitWithStubbedTrigger(t, "Same Unity warning.") + + result := decodePausePointWaitResult(t, output) + if strings.Count(result.Warning, "Same Unity warning.") != 1 { + t.Errorf("expected the repeated warning exactly once: %q", result.Warning) + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_logs.go b/cli/project-runner/internal/projectrunner/pause_point_logs.go index 9a1206ba75..48f2919ef4 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_logs.go +++ b/cli/project-runner/internal/projectrunner/pause_point_logs.go @@ -52,6 +52,62 @@ type pausePointWaitResult struct { AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` } +// pausePointHitPayloadInputs gathers everything a hit payload is built from. Both hit paths +// (await-pause-point and enable-pause-point --await) share one builder because they had drifted +// apart before: a field added to one silently stayed missing from the other. +type pausePointHitPayloadInputs struct { + response pausePointStatusResponse + + // logs / logsErr come straight from fetchMatchingLogs. A failed fetch omits MatchingLogs + // entirely rather than emitting an empty array, so "empty array" keeps meaning "the fetch + // succeeded and nothing matched". + logs pausePointMatchingLogsResult + logsErr error + + // unityWarning is Unity's own warning for this hit: the status response's on the plain await + // path, the enable response's on the enable --await path. + unityWarning string + + triggerResult *pausePointTriggerResult + awaitedPausePointID string + expectations []pausePointExpectationResult +} + +// buildPausePointHitPayload assembles the JSON payload for a hit, folding the CLI-side diagnosis +// (trigger outcome, warnings, --expect verdicts) into the Unity response. +func buildPausePointHitPayload(inputs pausePointHitPayloadInputs) any { + response := inputs.response + response.TriggerFailed = pausePointTriggerFailedPointer(inputs.triggerResult) + triggerWarning := pausePointTriggerRefusalWarning(inputs.triggerResult, inputs.awaitedPausePointID) + + if inputs.logsErr != nil { + // Best-effort: a failed log fetch must not also drop the CLI-side evidence — the warnings + // or the --expect results, which are the only evidence left in this branch. + return struct { + pausePointStatusResponse + Warning string `json:"Warning,omitempty"` + Expectations []pausePointExpectationResult `json:"Expectations,omitempty"` + AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` + }{ + pausePointStatusResponse: response, + Warning: joinPausePointWarnings(inputs.unityWarning, triggerWarning), + Expectations: inputs.expectations, + AllExpectationsPassed: pausePointAllExpectationsPassedPointer(inputs.expectations), + } + } + + return pausePointWaitResult{ + pausePointStatusResponse: response, + MatchingLogs: inputs.logs.Logs, + Warning: joinPausePointWarnings( + inputs.unityWarning, + buildPausePointWarning(inputs.logs, response.HitCount), + triggerWarning), + Expectations: inputs.expectations, + AllExpectationsPassed: pausePointAllExpectationsPassedPointer(inputs.expectations), + } +} + // pausePointAllExpectationsPassedPointer returns nil when no --expect was given, and otherwise // a pointer to whether every expectation passed. func pausePointAllExpectationsPassedPointer(results []pausePointExpectationResult) *bool { diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index 7b5f57d8b9..e8f29efc75 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -226,46 +226,20 @@ func runWaitForPausePoint( response.TriggerResult = triggerResult response.ResumePlayResult = resumeResult - response.TriggerFailed = pausePointTriggerFailedPointer(triggerResult) response = filterPausePointCapturedVariableHistory(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) // Best-effort: a hit must stay a success even if Unity is busy while paused. - // On fetch failure MatchingLogs is omitted entirely, so an empty array always - // means "the fetch succeeded and no matching log exists". logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) - var payload any - if logsErr == nil { - payload = pausePointWaitResult{ - pausePointStatusResponse: response, - MatchingLogs: logs.Logs, - Warning: joinPausePointWarnings( - response.Warning, - buildPausePointWarning(logs, response.HitCount), - pausePointTriggerRefusalWarning(triggerResult, options.id)), - Expectations: expectations, - AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), - } - } else { - // Best-effort: a failed log fetch must not also drop the CLI-side evidence — the - // --expect results a caller asked for by name, or a warning about the hit itself. Uses - // an anonymous struct (not pausePointWaitResult) so MatchingLogs is omitted entirely - // rather than serialized as an empty array, preserving "empty array only means a - // successful fetch with no matches". - payload = struct { - pausePointStatusResponse - Warning string `json:"Warning,omitempty"` - Expectations []pausePointExpectationResult `json:"Expectations,omitempty"` - AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` - }{ - pausePointStatusResponse: response, - Warning: joinPausePointWarnings( - response.Warning, - pausePointTriggerRefusalWarning(triggerResult, options.id)), - Expectations: expectations, - AllExpectationsPassed: pausePointAllExpectationsPassedPointer(expectations), - } - } + payload := buildPausePointHitPayload(pausePointHitPayloadInputs{ + response: response, + logs: logs, + logsErr: logsErr, + unityWarning: response.Warning, + triggerResult: triggerResult, + awaitedPausePointID: options.id, + expectations: expectations, + }) result, marshalErr := json.Marshal(payload) if marshalErr != nil { clierrors.WriteClassifiedError(stderr, marshalErr, clierrors.ErrorContext{ From 2de13da219d0d30726c58109dfaf9a6f17a5b098 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 02:13:57 +0900 Subject: [PATCH 09/13] Report a repeated captured-variable name as missing once Self-review follow-up. A name passed twice in --captured-variable-names was listed twice in CapturedVariableNamesNotFound, reading as two separate missing variables; the list answers which names have no value, not how often each was asked for. Also pins what --expect reports for a marker that is armed but not yet hit, since that is the state a polling caller queries most. --- ...se_point_captured_variable_names_filter.go | 17 ++++++++--- ...int_captured_variable_names_filter_test.go | 8 +++++ .../pause_point_status_expect_test.go | 29 +++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go index 4c3c22e28e..52eaa599aa 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go +++ b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go @@ -1,6 +1,9 @@ package projectrunner -import "strings" +import ( + "slices" + "strings" +) // parsePausePointCapturedVariableNames splits the comma-separated --captured-variable-names // value into individual names, trimming surrounding whitespace and dropping empty entries. @@ -62,13 +65,19 @@ func filterPausePointCapturedVariablesByName( // unmatchedCapturedVariableNames lists the requested names that matched nothing, keeping the order // they were requested in so the report reads back against the flag value the caller wrote. A name -// matched anywhere — current variables or any history frame — counts as found. +// matched anywhere — current variables or any history frame — counts as found. A name requested +// twice is reported once: the list answers "which names have no value", not "how many times each +// was asked for". func unmatchedCapturedVariableNames(names []string, matchedNames map[string]struct{}) []string { notFound := make([]string, 0, len(names)) for _, name := range names { - if _, ok := matchedNames[name]; !ok { - notFound = append(notFound, name) + if _, ok := matchedNames[name]; ok { + continue } + if slices.Contains(notFound, name) { + continue + } + notFound = append(notFound, name) } if len(notFound) == 0 { return nil diff --git a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go index 2904c8f72f..383ebdfb1c 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go @@ -104,6 +104,14 @@ func TestFilterPausePointCapturedVariablesByName(t *testing.T) { } }) + t.Run("a name requested twice is reported missing once", func(t *testing.T) { + result := filterPausePointCapturedVariablesByName( + baseResponse(), []string{"shield", "shield"}) + if len(result.CapturedVariableNamesNotFound) != 1 { + t.Fatalf("expected the repeated name once: %#v", result.CapturedVariableNamesNotFound) + } + }) + t.Run("every name matching leaves the missing list empty", func(t *testing.T) { result := filterPausePointCapturedVariablesByName(baseResponse(), []string{"velocity", "health"}) if result.CapturedVariableNamesNotFound != nil { diff --git a/cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go b/cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go index 1fe91211f5..84d3646d3a 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_status_expect_test.go @@ -171,6 +171,35 @@ func TestRunPausePointStatusOmitsExpectationFieldsWithoutExpectFlag(t *testing.T } } +// Verifies a marker that is armed but not yet hit reports its expectations as not found rather than +// omitting them, and that Status stays the field distinguishing "not hit yet" from "hit and wrong". +func TestRunPausePointStatusReportsExpectationsAsNotFoundBeforeAHit(t *testing.T) { + originalQuery := queryPausePointStatus + t.Cleanup(func() { + queryPausePointStatus = originalQuery + }) + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Success: true, Id: id, Status: pausePointStatusEnabled, IsEnabled: true}, nil + } + + code, output := runPausePointStatusForExpect(t, []string{"--id", "jump", "--expect", "speed=5"}) + + if code != 0 { + t.Fatalf("expected success, got %d: %s", code, output) + } + payload := decodePausePointStatusExpectPayload(t, output) + if payload.Status != pausePointStatusEnabled { + t.Fatalf("Status must still report the marker is only armed: %q", payload.Status) + } + if len(payload.Expectations) != 1 || payload.Expectations[0].Found || payload.Expectations[0].Passed { + t.Fatalf("an unhit marker captured nothing, so the expectation is not found: %#v", payload.Expectations) + } +} + // Verifies an invalid --expect value is rejected by pause-point-status the same way // await-pause-point rejects it, instead of being reported as an unknown option. func TestRunPausePointStatusRejectsInvalidExpectValue(t *testing.T) { From 964972b7fdb107bb95e2ec2435c1bacdd5101d8d Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 07:44:51 +0900 Subject: [PATCH 10/13] Agree the article with the command name in the owner message The owner sentence hardcoded "is an ... not a one", so a vowel-initial command in the second slot read as "not a await-pause-point one". One helper now decides the article for both slots from the command name. --- .../pause_point_unknown_option.go | 19 +++++++++++++++- .../pause_point_unknown_option_test.go | 22 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_unknown_option.go b/cli/project-runner/internal/projectrunner/pause_point_unknown_option.go index 4f88374f27..1ee7fc3c9a 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_unknown_option.go +++ b/cli/project-runner/internal/projectrunner/pause_point_unknown_option.go @@ -40,7 +40,10 @@ var pausePointCarriedOverEnableFlagNames = []string{ func pausePointUnknownOptionError(command string, name string) *clierrors.ArgumentError { message := fmt.Sprintf("Unknown option %q for %s.", "--"+name, command) if owner, ok := pausePointFlagOwnerCommand(name); ok && owner != command { - message = fmt.Sprintf("--%s is an %s option, not a %s one.", name, owner, command) + message = fmt.Sprintf("--%s is %s %s option, not %s %s one.", + name, + indefiniteArticleFor(owner), owner, + indefiniteArticleFor(command), command) if owner == pausePointEnableCommandName && isPausePointCarriedOverEnableFlag(name) { message += " The value passed to " + pausePointEnableCommandName + " is already applied to the response of this command, so it does not need to be passed again here." @@ -55,6 +58,20 @@ func pausePointUnknownOptionError(command string, name string) *clierrors.Argume } } +// indefiniteArticleFor picks the article for a command name interpolated into a message. Command +// names are lower-case ASCII identifiers, so the initial letter decides it — "an await-pause-point +// option" rather than "a await-pause-point option". Both slots of the owner sentence go through +// this, so neither reads as broken English for a vowel-initial command. +func indefiniteArticleFor(commandName string) string { + if commandName == "" { + return "a" + } + if strings.ContainsRune("aeiou", rune(commandName[0])) { + return "an" + } + return "a" +} + // pausePointFlagOwnerCommand reports which pause-point command accepts the flag, searching in a // fixed order so the answer never depends on map iteration. func pausePointFlagOwnerCommand(name string) (string, bool) { diff --git a/cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go b/cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go index 340871bc24..21289b5a5a 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_unknown_option_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "strings" "testing" + + "github.com/hatayama/unity-cli-loop/common/clicore" ) // Verifies a flag that belongs to enable-pause-point names its real owner and states that the value @@ -63,6 +65,26 @@ func TestParsePausePointStatusUnknownOptionNamesAwaitAsTheOwner(t *testing.T) { } } +// Verifies the article agrees with the command name in both slots of the owner sentence, so a +// vowel-initial command such as await-pause-point does not produce "a await-pause-point one". +func TestPausePointUnknownOptionArticleAgreesWithTheCommandName(t *testing.T) { + _, err := parseWaitForPausePointOptions( + []string{"--id", "jump", "--max-preview-elements", "5"}) + + if err == nil { + t.Fatal("expected error for an enable-pause-point flag passed to await-pause-point") + } + if !strings.Contains( + err.Error(), + "--max-preview-elements is an enable-pause-point option, not an await-pause-point one.") { + t.Errorf("article mismatch in the owner sentence: %s", err.Error()) + } + + if article := indefiniteArticleFor(clicore.PausePointStatusUserCommandName); article != "a" { + t.Errorf("a consonant-initial command takes \"a\", got %q", article) + } +} + // Verifies the owner reported for a flag several commands accept is fixed rather than dependent on // map iteration order, so the same misuse always produces the same message. func TestPausePointUnknownOptionOwnerIsDeterministic(t *testing.T) { From 1b8dec8a2d08f73bf3bf358f244f03ce95877c20 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 08:05:24 +0900 Subject: [PATCH 11/13] Document the new pause-point response fields in the skill SKILL.md still said --expect was not available on pause-point-status, which this branch makes false, and none of the new diagnostic fields were described. Covers --expect on pause-point-status (including what an unhit marker reports and that the verdict never moves the exit code), the top-level TriggerFailed and its warning, CapturedVariableNamesNotFound, and RejectedByActivePausePointId on a refused simulate call. Generated copies regenerated with skills install. --- .agents/skills/uloop-pause-point/SKILL.md | 8 ++++---- .claude/skills/uloop-pause-point/SKILL.md | 8 ++++---- .../src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.agents/skills/uloop-pause-point/SKILL.md b/.agents/skills/uloop-pause-point/SKILL.md index 68702eb471..31e64add12 100644 --- a/.agents/skills/uloop-pause-point/SKILL.md +++ b/.agents/skills/uloop-pause-point/SKILL.md @@ -18,7 +18,7 @@ uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seco Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is rejected. -Before writing a `--trigger` command that differs from the example, load the skill of the tool you are about to trigger. `--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so the input cannot land before arming and nothing needs to run in the background. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame), in which case the trigger is rejected because PlayMode is already paused and runs nothing — check `TriggerResult` before treating such a hit as input-driven. If the trigger command itself is rejected before it runs — its argument parsing fails (`INVALID_ARGUMENT`) or the command name is unknown (`UNKNOWN_COMMAND`) — the wait is abandoned immediately with a `PAUSE_POINT_TRIGGER_FAILED` error instead of waiting out `--timeout-seconds`: the marker stays armed, a PlayMode resumed by `--resume-play` is paused again, and `Error.NextActions` carries the recovery commands — fix the trigger value and re-run the same command. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` — see Fast-Progressing Games. +Before writing a `--trigger` command that differs from the example, load the skill of the tool you are about to trigger. `--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so the input cannot land before arming and nothing needs to run in the background. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame), in which case the trigger is rejected because PlayMode is already paused and runs nothing — the hit response then carries `TriggerFailed: true` at the top level and a `Warning` explaining that no input reached the game (the triggered command's own response stays in `TriggerResult`), so do not treat such a hit as input-driven. If the trigger command itself is rejected before it runs — its argument parsing fails (`INVALID_ARGUMENT`) or the command name is unknown (`UNKNOWN_COMMAND`) — the wait is abandoned immediately with a `PAUSE_POINT_TRIGGER_FAILED` error instead of waiting out `--timeout-seconds`: the marker stays armed, a PlayMode resumed by `--resume-play` is paused again, and `Error.NextActions` carries the recovery commands — fix the trigger value and re-run the same command. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` — see Fast-Progressing Games. When the game reaches the line on its own, omit `--trigger`. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event): run `enable-pause-point` without `--await` in the foreground (its response returning is the arm confirmation), then start `uloop await-pause-point --id ` in the background, then send the inputs. Do not approximate arm-waiting with a fixed sleep after a backgrounded enable. @@ -53,8 +53,8 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - The snapshot is taken **before** the resolved line executes, exactly like an IDE breakpoint on that line. To inspect a value after an assignment, place the pause point on the following line. - `Scope` is `Local`, `Parameter`, `InstanceField`, or `This`. The synthetic `this` entry identifies which instance or GameObject was hit via `UnityObjectPath` and `UnityObjectInstanceId`; `UnityEngine.Object` values carry the same handle fields for follow-up digs with `get-hierarchy`, `find-game-objects`, or `execute-dynamic-code`. - `--captured-variables names` on `await-pause-point`/`pause-point-status` drops every `Value` and keeps `Name`/`Scope`/`TypeName` — use it first on field-heavy classes, then fetch full values with a plain `pause-point-status` call. -- When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. -- Pass `--expect 'name=value'` (repeatable; on `await-pause-point` and `enable-pause-point --await`, not `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. +- When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. Requested names that matched nothing are listed in `CapturedVariableNamesNotFound`, so a partial match is visible without comparing the response against the request by hand. +- Pass `--expect 'name=value'` (repeatable; on `await-pause-point`, `enable-pause-point --await`, and `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. On `pause-point-status` a marker that has not been hit yet reports each expectation as not found, and the verdict never changes the exit code — a polling loop reads `AllExpectationsPassed`, not the exit status. - Collection values (arrays, `List`, dictionaries, plain objects) render as a JSON preview capped at 10 elements by default. When the elements you need sit past that cap (a 10x20 grid, a long list), re-enable with `--max-preview-elements ` (1–1000). The value set at enable time also caps the previews in every later `pause-point-status` response for that marker — status has no flag to change it. - While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the return is a `(bool Found, object Value)` tuple, and the holder clears on resume. (file:line marker hits only — id-only markers store no capture) These are **live objects in their frame-completed state, not snapshots** — use them only to dig further into objects that are still alive, never to reconstruct what a value was at the paused line. @@ -95,7 +95,7 @@ For "N frames after the input" (for example, three frames after a key press), ad A pause point hits only when control flow reaches the patched line (or the `Pause(id)` call). `simulate-keyboard` returning `PressEdgeObserved=true` means the input edge was observed, not that your target game logic has reached the pause line yet. -If a `simulate-*` command instead returns a failure whose message says PlayMode is paused, suspect a pause point hit rather than an unrelated failure: an active pause point can make PlayMode paused mid-simulation, and the `simulate-*` call surfaces that as a preflight failure. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. +If a `simulate-*` command instead returns a failure whose message says PlayMode is paused, suspect a pause point hit rather than an unrelated failure: an active pause point can make PlayMode paused mid-simulation, and the `simulate-*` call surfaces that as a preflight failure. The failure response names the responsible marker in `RejectedByActivePausePointId`. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## When To Use diff --git a/.claude/skills/uloop-pause-point/SKILL.md b/.claude/skills/uloop-pause-point/SKILL.md index 68702eb471..31e64add12 100644 --- a/.claude/skills/uloop-pause-point/SKILL.md +++ b/.claude/skills/uloop-pause-point/SKILL.md @@ -18,7 +18,7 @@ uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seco Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is rejected. -Before writing a `--trigger` command that differs from the example, load the skill of the tool you are about to trigger. `--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so the input cannot land before arming and nothing needs to run in the background. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame), in which case the trigger is rejected because PlayMode is already paused and runs nothing — check `TriggerResult` before treating such a hit as input-driven. If the trigger command itself is rejected before it runs — its argument parsing fails (`INVALID_ARGUMENT`) or the command name is unknown (`UNKNOWN_COMMAND`) — the wait is abandoned immediately with a `PAUSE_POINT_TRIGGER_FAILED` error instead of waiting out `--timeout-seconds`: the marker stays armed, a PlayMode resumed by `--resume-play` is paused again, and `Error.NextActions` carries the recovery commands — fix the trigger value and re-run the same command. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` — see Fast-Progressing Games. +Before writing a `--trigger` command that differs from the example, load the skill of the tool you are about to trigger. `--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so the input cannot land before arming and nothing needs to run in the background. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame), in which case the trigger is rejected because PlayMode is already paused and runs nothing — the hit response then carries `TriggerFailed: true` at the top level and a `Warning` explaining that no input reached the game (the triggered command's own response stays in `TriggerResult`), so do not treat such a hit as input-driven. If the trigger command itself is rejected before it runs — its argument parsing fails (`INVALID_ARGUMENT`) or the command name is unknown (`UNKNOWN_COMMAND`) — the wait is abandoned immediately with a `PAUSE_POINT_TRIGGER_FAILED` error instead of waiting out `--timeout-seconds`: the marker stays armed, a PlayMode resumed by `--resume-play` is paused again, and `Error.NextActions` carries the recovery commands — fix the trigger value and re-run the same command. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` — see Fast-Progressing Games. When the game reaches the line on its own, omit `--trigger`. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event): run `enable-pause-point` without `--await` in the foreground (its response returning is the arm confirmation), then start `uloop await-pause-point --id ` in the background, then send the inputs. Do not approximate arm-waiting with a fixed sleep after a backgrounded enable. @@ -53,8 +53,8 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - The snapshot is taken **before** the resolved line executes, exactly like an IDE breakpoint on that line. To inspect a value after an assignment, place the pause point on the following line. - `Scope` is `Local`, `Parameter`, `InstanceField`, or `This`. The synthetic `this` entry identifies which instance or GameObject was hit via `UnityObjectPath` and `UnityObjectInstanceId`; `UnityEngine.Object` values carry the same handle fields for follow-up digs with `get-hierarchy`, `find-game-objects`, or `execute-dynamic-code`. - `--captured-variables names` on `await-pause-point`/`pause-point-status` drops every `Value` and keeps `Name`/`Scope`/`TypeName` — use it first on field-heavy classes, then fetch full values with a plain `pause-point-status` call. -- When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. -- Pass `--expect 'name=value'` (repeatable; on `await-pause-point` and `enable-pause-point --await`, not `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. +- When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. Requested names that matched nothing are listed in `CapturedVariableNamesNotFound`, so a partial match is visible without comparing the response against the request by hand. +- Pass `--expect 'name=value'` (repeatable; on `await-pause-point`, `enable-pause-point --await`, and `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. On `pause-point-status` a marker that has not been hit yet reports each expectation as not found, and the verdict never changes the exit code — a polling loop reads `AllExpectationsPassed`, not the exit status. - Collection values (arrays, `List`, dictionaries, plain objects) render as a JSON preview capped at 10 elements by default. When the elements you need sit past that cap (a 10x20 grid, a long list), re-enable with `--max-preview-elements ` (1–1000). The value set at enable time also caps the previews in every later `pause-point-status` response for that marker — status has no flag to change it. - While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the return is a `(bool Found, object Value)` tuple, and the holder clears on resume. (file:line marker hits only — id-only markers store no capture) These are **live objects in their frame-completed state, not snapshots** — use them only to dig further into objects that are still alive, never to reconstruct what a value was at the paused line. @@ -95,7 +95,7 @@ For "N frames after the input" (for example, three frames after a key press), ad A pause point hits only when control flow reaches the patched line (or the `Pause(id)` call). `simulate-keyboard` returning `PressEdgeObserved=true` means the input edge was observed, not that your target game logic has reached the pause line yet. -If a `simulate-*` command instead returns a failure whose message says PlayMode is paused, suspect a pause point hit rather than an unrelated failure: an active pause point can make PlayMode paused mid-simulation, and the `simulate-*` call surfaces that as a preflight failure. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. +If a `simulate-*` command instead returns a failure whose message says PlayMode is paused, suspect a pause point hit rather than an unrelated failure: an active pause point can make PlayMode paused mid-simulation, and the `simulate-*` call surfaces that as a preflight failure. The failure response names the responsible marker in `RejectedByActivePausePointId`. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## When To Use diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md index 68702eb471..31e64add12 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md @@ -18,7 +18,7 @@ uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seco Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is rejected. -Before writing a `--trigger` command that differs from the example, load the skill of the tool you are about to trigger. `--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so the input cannot land before arming and nothing needs to run in the background. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame), in which case the trigger is rejected because PlayMode is already paused and runs nothing — check `TriggerResult` before treating such a hit as input-driven. If the trigger command itself is rejected before it runs — its argument parsing fails (`INVALID_ARGUMENT`) or the command name is unknown (`UNKNOWN_COMMAND`) — the wait is abandoned immediately with a `PAUSE_POINT_TRIGGER_FAILED` error instead of waiting out `--timeout-seconds`: the marker stays armed, a PlayMode resumed by `--resume-play` is paused again, and `Error.NextActions` carries the recovery commands — fix the trigger value and re-run the same command. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` — see Fast-Progressing Games. +Before writing a `--trigger` command that differs from the example, load the skill of the tool you are about to trigger. `--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so the input cannot land before arming and nothing needs to run in the background. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame), in which case the trigger is rejected because PlayMode is already paused and runs nothing — the hit response then carries `TriggerFailed: true` at the top level and a `Warning` explaining that no input reached the game (the triggered command's own response stays in `TriggerResult`), so do not treat such a hit as input-driven. If the trigger command itself is rejected before it runs — its argument parsing fails (`INVALID_ARGUMENT`) or the command name is unknown (`UNKNOWN_COMMAND`) — the wait is abandoned immediately with a `PAUSE_POINT_TRIGGER_FAILED` error instead of waiting out `--timeout-seconds`: the marker stays armed, a PlayMode resumed by `--resume-play` is paused again, and `Error.NextActions` carries the recovery commands — fix the trigger value and re-run the same command. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` — see Fast-Progressing Games. When the game reaches the line on its own, omit `--trigger`. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event): run `enable-pause-point` without `--await` in the foreground (its response returning is the arm confirmation), then start `uloop await-pause-point --id ` in the background, then send the inputs. Do not approximate arm-waiting with a fixed sleep after a backgrounded enable. @@ -53,8 +53,8 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - The snapshot is taken **before** the resolved line executes, exactly like an IDE breakpoint on that line. To inspect a value after an assignment, place the pause point on the following line. - `Scope` is `Local`, `Parameter`, `InstanceField`, or `This`. The synthetic `this` entry identifies which instance or GameObject was hit via `UnityObjectPath` and `UnityObjectInstanceId`; `UnityEngine.Object` values carry the same handle fields for follow-up digs with `get-hierarchy`, `find-game-objects`, or `execute-dynamic-code`. - `--captured-variables names` on `await-pause-point`/`pause-point-status` drops every `Value` and keeps `Name`/`Scope`/`TypeName` — use it first on field-heavy classes, then fetch full values with a plain `pause-point-status` call. -- When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. -- Pass `--expect 'name=value'` (repeatable; on `await-pause-point` and `enable-pause-point --await`, not `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. +- When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. Requested names that matched nothing are listed in `CapturedVariableNamesNotFound`, so a partial match is visible without comparing the response against the request by hand. +- Pass `--expect 'name=value'` (repeatable; on `await-pause-point`, `enable-pause-point --await`, and `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. On `pause-point-status` a marker that has not been hit yet reports each expectation as not found, and the verdict never changes the exit code — a polling loop reads `AllExpectationsPassed`, not the exit status. - Collection values (arrays, `List`, dictionaries, plain objects) render as a JSON preview capped at 10 elements by default. When the elements you need sit past that cap (a 10x20 grid, a long list), re-enable with `--max-preview-elements ` (1–1000). The value set at enable time also caps the previews in every later `pause-point-status` response for that marker — status has no flag to change it. - While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the return is a `(bool Found, object Value)` tuple, and the holder clears on resume. (file:line marker hits only — id-only markers store no capture) These are **live objects in their frame-completed state, not snapshots** — use them only to dig further into objects that are still alive, never to reconstruct what a value was at the paused line. @@ -95,7 +95,7 @@ For "N frames after the input" (for example, three frames after a key press), ad A pause point hits only when control flow reaches the patched line (or the `Pause(id)` call). `simulate-keyboard` returning `PressEdgeObserved=true` means the input edge was observed, not that your target game logic has reached the pause line yet. -If a `simulate-*` command instead returns a failure whose message says PlayMode is paused, suspect a pause point hit rather than an unrelated failure: an active pause point can make PlayMode paused mid-simulation, and the `simulate-*` call surfaces that as a preflight failure. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. +If a `simulate-*` command instead returns a failure whose message says PlayMode is paused, suspect a pause point hit rather than an unrelated failure: an active pause point can make PlayMode paused mid-simulation, and the `simulate-*` call surfaces that as a preflight failure. The failure response names the responsible marker in `RejectedByActivePausePointId`. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## When To Use From 4c8036a2d988da849aadf441e927cac769b06e89 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 08:05:24 +0900 Subject: [PATCH 12/13] Drop the ToolContracts using left unused by the preflight migration Both files stopped referencing any ToolContracts type when they moved to PlayModeToolPreflightResult; the test file's copy was already removed. --- .../Common/Preflight/PlayModeToolPreflightService.cs | 1 - .../SimulateMouseUi/MouseUiSimulationValidator.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs index 4300b40430..fe05f3a5b1 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/Preflight/PlayModeToolPreflightService.cs @@ -3,7 +3,6 @@ using UnityEngine; using io.github.hatayama.UnityCliLoop.Runtime; -using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs index 6c9035e143..239600f464 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationValidator.cs @@ -2,7 +2,6 @@ using UnityEngine.EventSystems; using io.github.hatayama.UnityCliLoop.Runtime; -using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { From 48bdaaf486ab3fb8990781a421c4589f8e4de63f Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 27 Jul 2026 08:05:24 +0900 Subject: [PATCH 13/13] Split the not-found filter tests and pin TriggerFailed's wire name The five subtests added for CapturedVariableNamesNotFound pushed TestFilterPausePointCapturedVariablesByName to cyclomatic complexity 24 against the repository maximum of 15, so they move to their own test function sharing a hoisted fixture. The refusal test now also asserts the raw stdout contains "TriggerFailed": true, since every existing assertion goes through a Go struct and would survive a renamed json tag. --- ...int_captured_variable_names_filter_test.go | 111 ++++++++++-------- .../pause_point_trigger_diagnosis_test.go | 5 + 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go index 383ebdfb1c..f1ea9cd846 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go @@ -2,28 +2,32 @@ package projectrunner import "testing" +// capturedVariableNamesFilterResponse is the fixture both --captured-variable-names test functions +// filter: three current variables, two of which also appear in one history frame. +func capturedVariableNamesFilterResponse() pausePointStatusResponse { + return pausePointStatusResponse{ + CapturedVariables: []pausePointCapturedVariable{ + {Name: "velocity", Scope: "Local", TypeName: "Vector3", Value: pausePointVariableValue("(1,0,0)")}, + {Name: "this", Scope: "This", TypeName: "PlayerController", Value: pausePointVariableValue("PlayerController")}, + {Name: "health", Scope: "Local", TypeName: "Int32", Value: pausePointVariableValue("100")}, + }, + CapturedVariableHistory: []pausePointCapturedHistoryFrame{ + { + HitSequence: 1, + CapturedVariables: []pausePointCapturedVariable{ + {Name: "velocity", Scope: "Local", TypeName: "Vector3", Value: pausePointVariableValue("(0,0,0)")}, + {Name: "health", Scope: "Local", TypeName: "Int32", Value: pausePointVariableValue("100")}, + }, + }, + }, + } +} + // TestFilterPausePointCapturedVariablesByName verifies the --captured-variable-names filter: // single-name selection, multi-name selection, a name with no match, and that it composes with // the --captured-variables mode (filter narrows first, then mode strips values). func TestFilterPausePointCapturedVariablesByName(t *testing.T) { - baseResponse := func() pausePointStatusResponse { - return pausePointStatusResponse{ - CapturedVariables: []pausePointCapturedVariable{ - {Name: "velocity", Scope: "Local", TypeName: "Vector3", Value: pausePointVariableValue("(1,0,0)")}, - {Name: "this", Scope: "This", TypeName: "PlayerController", Value: pausePointVariableValue("PlayerController")}, - {Name: "health", Scope: "Local", TypeName: "Int32", Value: pausePointVariableValue("100")}, - }, - CapturedVariableHistory: []pausePointCapturedHistoryFrame{ - { - HitSequence: 1, - CapturedVariables: []pausePointCapturedVariable{ - {Name: "velocity", Scope: "Local", TypeName: "Vector3", Value: pausePointVariableValue("(0,0,0)")}, - {Name: "health", Scope: "Local", TypeName: "Int32", Value: pausePointVariableValue("100")}, - }, - }, - }, - } - } + baseResponse := capturedVariableNamesFilterResponse t.Run("single name keeps only the matching variable", func(t *testing.T) { result := filterPausePointCapturedVariablesByName(baseResponse(), []string{"velocity"}) @@ -72,6 +76,45 @@ func TestFilterPausePointCapturedVariablesByName(t *testing.T) { } }) + t.Run("empty names list leaves the response unchanged", func(t *testing.T) { + original := baseResponse() + result := filterPausePointCapturedVariablesByName(original, nil) + if len(result.CapturedVariables) != len(original.CapturedVariables) { + t.Fatalf("expected response unchanged with no names filter: %#v", result.CapturedVariables) + } + }) +} + +// TestParsePausePointCapturedVariableNames verifies comma-splitting, whitespace trimming, and +// that empty entries are dropped. +func TestParsePausePointCapturedVariableNames(t *testing.T) { + cases := map[string][]string{ + "": nil, + "velocity": {"velocity"}, + "velocity,this": {"velocity", "this"}, + "velocity, this , health": {"velocity", "this", "health"}, + "velocity,,this": {"velocity", "this"}, + } + + for input, expected := range cases { + names := parsePausePointCapturedVariableNames(input) + if len(names) != len(expected) { + t.Fatalf("input %q: length mismatch: got %#v, want %#v", input, names, expected) + } + for index, name := range names { + if name != expected[index] { + t.Fatalf("input %q: name[%d] mismatch: got %q, want %q", input, index, name, expected[index]) + } + } + } +} + +// TestFilterPausePointCapturedVariablesByNameReportsNotFound verifies which requested names are +// reported as matching nothing: request order is preserved, a history-only match counts as found, a +// repeat is reported once, and the all-or-nothing flag stays consistent with the list. +func TestFilterPausePointCapturedVariablesByNameReportsNotFound(t *testing.T) { + baseResponse := capturedVariableNamesFilterResponse + t.Run("reports which requested names matched nothing, in the requested order", func(t *testing.T) { result := filterPausePointCapturedVariablesByName( baseResponse(), []string{"shield", "velocity", "armor"}) @@ -118,36 +161,4 @@ func TestFilterPausePointCapturedVariablesByName(t *testing.T) { t.Fatalf("expected no missing names: %#v", result.CapturedVariableNamesNotFound) } }) - - t.Run("empty names list leaves the response unchanged", func(t *testing.T) { - original := baseResponse() - result := filterPausePointCapturedVariablesByName(original, nil) - if len(result.CapturedVariables) != len(original.CapturedVariables) { - t.Fatalf("expected response unchanged with no names filter: %#v", result.CapturedVariables) - } - }) -} - -// TestParsePausePointCapturedVariableNames verifies comma-splitting, whitespace trimming, and -// that empty entries are dropped. -func TestParsePausePointCapturedVariableNames(t *testing.T) { - cases := map[string][]string{ - "": nil, - "velocity": {"velocity"}, - "velocity,this": {"velocity", "this"}, - "velocity, this , health": {"velocity", "this", "health"}, - "velocity,,this": {"velocity", "this"}, - } - - for input, expected := range cases { - names := parsePausePointCapturedVariableNames(input) - if len(names) != len(expected) { - t.Fatalf("input %q: length mismatch: got %#v, want %#v", input, names, expected) - } - for index, name := range names { - if name != expected[index] { - t.Fatalf("input %q: name[%d] mismatch: got %q, want %q", input, index, name, expected[index]) - } - } - } } diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go index 9af4f3edaf..178eb8a24a 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis_test.go @@ -140,6 +140,11 @@ func TestRunWaitForPausePointWarnsWhenTheTriggerWasRefusedByThisMarker(t *testin if code != 0 { t.Fatalf("expected the hit to stay a success, got %d: %s", code, output) } + // Asserted on the raw stdout, not only through the decoded struct: callers read this by its wire + // name, so a renamed json tag must fail here rather than pass a struct round trip. + if !strings.Contains(output, `"TriggerFailed": true`) { + t.Errorf("TriggerFailed must appear at the top level under that exact name: %s", output) + } result := decodePausePointWaitResult(t, output) if !strings.Contains(result.Warning, "refused") { t.Errorf("expected a refusal warning: %q", result.Warning)