From a4f87c39bb89ea2335cb15dfcdb4fc9dfe0cde64 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 21:30:24 +0900 Subject: [PATCH 1/4] Add --trigger flag to await-pause-point/enable-pause-point --await Waiting for a pause point and then triggering the action that hits it required two separate CLI calls with a manual race (start a background simulate-* command, then await). --trigger dispatches the given uloop subcommand in-process right after the marker is armed, and embeds its response (or dispatch error) as TriggerResult in the wait's own JSON output, collapsing that into a single call. - Quote-aware tokenizer parses the --trigger value into argv-style tokens. - Fail-fast validation rejects targeting await-pause-point/enable-pause-point (would deadlock) and --project-path inside the trigger string (implicit project inheritance only). - The trigger runs concurrently via a goroutine and is joined with a short grace window after the wait itself settles, reusing pausePointFinalStatusProbeTimeout for consistency with the existing final-status-probe pattern. - Verified end-to-end against the KeyStateAfterPauseInterruption regression harness: enable-pause-point + await-pause-point --trigger "simulate-keyboard ..." now hits in ~280ms instead of running the full press duration. --- .../projectrunner/native_command_help.go | 2 + .../projectrunner/pause_point_enable.go | 68 ++- .../projectrunner/pause_point_enable_test.go | 12 +- .../projectrunner/pause_point_trigger.go | 241 +++++++++ .../projectrunner/pause_point_trigger_test.go | 471 ++++++++++++++++++ .../projectrunner/pause_point_types.go | 4 + .../projectrunner/pause_point_wait.go | 42 +- .../projectrunner/pause_point_wait_test.go | 5 +- .../internal/projectrunner/runner_commands.go | 2 +- 9 files changed, 822 insertions(+), 25 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_trigger.go create mode 100644 cli/project-runner/internal/projectrunner/pause_point_trigger_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 6a6079f65..9a3797065 100644 --- a/cli/project-runner/internal/projectrunner/native_command_help.go +++ b/cli/project-runner/internal/projectrunner/native_command_help.go @@ -18,6 +18,7 @@ const ( PausePointCapturedVariablesFlagName = "captured-variables" PausePointCapturedVariableNamesFlagName = "captured-variable-names" PausePointExpectFlagName = "expect" + PausePointTriggerFlagName = "trigger" ) // runnerNativeCommandOptions lists the flags accepted by each runner-owned @@ -32,6 +33,7 @@ var runnerNativeCommandOptions = map[string][]string{ "--" + PausePointCapturedVariablesFlagName, "--" + PausePointCapturedVariableNamesFlagName, "--" + PausePointExpectFlagName, + "--" + PausePointTriggerFlagName, }, clicore.PausePointStatusUserCommandName: { "--" + PausePointIDFlagName, diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 9829c3a9a..20cd0ceb2 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -25,11 +25,11 @@ const pausePointEnableCommandName = "enable-pause-point" const pausePointEnableAwaitFlagName = "await" // extractPausePointEnableAwaitFlags pulls the CLI-only --await/--captured-variables/ -// --captured-variable-names/--expect flags out of enable-pause-point args before generic schema -// parsing, because none of them are part of the Unity-side EnablePausePointSchema. +// --captured-variable-names/--expect/--trigger flags out of enable-pause-point args before +// generic schema parsing, because none of them are part of the Unity-side EnablePausePointSchema. func extractPausePointEnableAwaitFlags( args []string, -) ([]string, bool, pausePointCapturedVariablesMode, []string, []pausePointExpectation, error) { +) ([]string, bool, pausePointCapturedVariablesMode, []string, []pausePointExpectation, string, []string, error) { remaining := make([]string, 0, len(args)) await := false mode := pausePointCapturedVariablesModeFull @@ -37,6 +37,9 @@ func extractPausePointEnableAwaitFlags( var capturedVariableNames []string namesSet := false var expectations []pausePointExpectation + var triggerCommand string + var triggerArgs []string + triggerSet := false for index := 0; index < len(args); index++ { arg := args[index] @@ -46,10 +49,32 @@ func extractPausePointEnableAwaitFlags( continue } + if isPausePointFlag(arg, PausePointTriggerFlagName) { + name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) + if err != nil { + return nil, false, mode, nil, nil, "", nil, err + } + if name != PausePointTriggerFlagName { + remaining = append(remaining, arg) + continue + } + parsedCommand, parsedArgs, parseErr := parsePausePointTriggerCommand(pausePointEnableCommandName, value) + if parseErr != nil { + return nil, false, mode, nil, nil, "", nil, parseErr + } + triggerCommand = parsedCommand + triggerArgs = parsedArgs + triggerSet = true + if consumedNext { + index++ + } + continue + } + if isPausePointFlag(arg, PausePointCapturedVariablesFlagName) { name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { - return nil, false, mode, nil, nil, err + return nil, false, mode, nil, nil, "", nil, err } if name != PausePointCapturedVariablesFlagName { remaining = append(remaining, arg) @@ -57,7 +82,7 @@ func extractPausePointEnableAwaitFlags( } parsedMode, err := parsePausePointCapturedVariablesMode(value) if err != nil { - return nil, false, mode, nil, nil, err + return nil, false, mode, nil, nil, "", nil, err } mode = parsedMode modeSet = true @@ -70,7 +95,7 @@ func extractPausePointEnableAwaitFlags( if isPausePointFlag(arg, PausePointCapturedVariableNamesFlagName) { name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { - return nil, false, mode, nil, nil, err + return nil, false, mode, nil, nil, "", nil, err } if name != PausePointCapturedVariableNamesFlagName { remaining = append(remaining, arg) @@ -87,7 +112,7 @@ func extractPausePointEnableAwaitFlags( if isPausePointFlag(arg, PausePointExpectFlagName) { name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { - return nil, false, mode, nil, nil, err + return nil, false, mode, nil, nil, "", nil, err } if name != PausePointExpectFlagName { remaining = append(remaining, arg) @@ -95,7 +120,7 @@ func extractPausePointEnableAwaitFlags( } expectation, parseErr := parsePausePointExpectFlagValue(value) if parseErr != nil { - return nil, false, mode, nil, nil, parseErr + return nil, false, mode, nil, nil, "", nil, parseErr } expectations = append(expectations, expectation) if consumedNext { @@ -107,16 +132,18 @@ func extractPausePointEnableAwaitFlags( remaining = append(remaining, arg) } - if !await && (modeSet || namesSet || len(expectations) > 0) { + if !await && (modeSet || namesSet || len(expectations) > 0 || triggerSet) { option := "--" + PausePointCapturedVariablesFlagName switch { + case triggerSet: + option = "--" + PausePointTriggerFlagName case len(expectations) > 0: option = "--" + PausePointExpectFlagName case namesSet: option = "--" + PausePointCapturedVariableNamesFlagName } - return nil, false, mode, nil, nil, &clierrors.ArgumentError{ - Message: "--captured-variables, --captured-variable-names, and --expect require --await", + return nil, false, mode, nil, nil, "", nil, &clierrors.ArgumentError{ + Message: "--captured-variables, --captured-variable-names, --expect, and --trigger require --await", Option: option, Command: pausePointEnableCommandName, NextActions: []string{ @@ -125,7 +152,7 @@ func extractPausePointEnableAwaitFlags( } } - return remaining, await, mode, capturedVariableNames, expectations, nil + return remaining, await, mode, capturedVariableNames, expectations, triggerCommand, triggerArgs, nil } func isPausePointFlag(arg string, flagName string) bool { @@ -140,7 +167,7 @@ func runEnablePausePointCommand( stdout io.Writer, stderr io.Writer, ) int { - remainingArgs, await, capturedVariablesMode, capturedVariableNames, expectations, err := extractPausePointEnableAwaitFlags(args) + remainingArgs, await, capturedVariablesMode, capturedVariableNames, expectations, triggerCommand, triggerArgs, err := extractPausePointEnableAwaitFlags(args) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ ProjectRoot: connection.ProjectRoot, @@ -192,7 +219,8 @@ func runEnablePausePointCommand( } return runEnablePausePointAndAwait( - ctx, connection, params, capturedVariablesMode, capturedVariableNames, expectations, stdout, stderr) + ctx, connection, params, capturedVariablesMode, capturedVariableNames, expectations, + triggerCommand, triggerArgs, startPath, stdout, stderr) } // runEnablePausePointAndAwait sends the same single enable-pause-point IPC request the @@ -206,6 +234,9 @@ func runEnablePausePointAndAwait( capturedVariablesMode pausePointCapturedVariablesMode, capturedVariableNames []string, expectations []pausePointExpectation, + triggerCommand string, + triggerArgs []string, + startPath string, stdout io.Writer, stderr io.Writer, ) int { @@ -254,6 +285,9 @@ func runEnablePausePointAndAwait( capturedVariablesMode: capturedVariablesMode, capturedVariableNames: capturedVariableNames, expectations: expectations, + triggerCommand: triggerCommand, + triggerArgs: triggerArgs, + startPath: startPath, } return runPausePointWaitAfterEnable(ctx, connection, waitOptions, enableResponse.Warning, stdout, stderr) @@ -272,7 +306,7 @@ func runPausePointWaitAfterEnable( stderr io.Writer, ) int { spinner := clicore.NewToolSpinner(stderr, pausePointEnableCommandName) - response, state, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -287,6 +321,7 @@ func runPausePointWaitAfterEnable( // narrow or strip values, same as the plain await-pause-point path. expectations := evaluatePausePointExpectations(response.CapturedVariables, options.expectations) + response.TriggerResult = triggerResult response = filterPausePointCapturedVariableHistory(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) @@ -341,6 +376,9 @@ func runPausePointWaitAfterEnable( if enableWarning != "" { waitErr.Details["EnableWarning"] = enableWarning } + if triggerResult != nil { + waitErr.Details["TriggerResult"] = triggerResult + } if state == pausePointWaitStateTimeout { logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) if logsErr == nil { diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable_test.go b/cli/project-runner/internal/projectrunner/pause_point_enable_test.go index fd31b27b8..e91003bbe 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable_test.go @@ -16,7 +16,7 @@ import ( // Verifies --await is extracted and the remaining args are left untouched for schema parsing. func TestExtractPausePointEnableAwaitFlagsExtractsAwait(t *testing.T) { - remaining, await, mode, names, expectations, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--await"}) + remaining, await, mode, names, expectations, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--await"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -39,7 +39,7 @@ func TestExtractPausePointEnableAwaitFlagsExtractsAwait(t *testing.T) { // Verifies --captured-variables/--captured-variable-names are extracted alongside --await. func TestExtractPausePointEnableAwaitFlagsExtractsCapturedVariableOptions(t *testing.T) { - remaining, await, mode, names, _, err := extractPausePointEnableAwaitFlags([]string{ + remaining, await, mode, names, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ "--id", "jump", "--await", "--captured-variables", "names", "--captured-variable-names", "a,b", }) if err != nil { @@ -61,7 +61,7 @@ func TestExtractPausePointEnableAwaitFlagsExtractsCapturedVariableOptions(t *tes // Verifies --expect is extracted (repeatably) alongside --await, and unrelated args are untouched. func TestExtractPausePointEnableAwaitFlagsExtractsExpect(t *testing.T) { - remaining, await, _, _, expectations, err := extractPausePointEnableAwaitFlags([]string{ + remaining, await, _, _, expectations, _, _, err := extractPausePointEnableAwaitFlags([]string{ "--id", "jump", "--await", "--expect", "Health=100", "--expect", "Name=Enemy", }) if err != nil { @@ -86,7 +86,7 @@ func TestExtractPausePointEnableAwaitFlagsExtractsExpect(t *testing.T) { // Verifies --captured-variables without --await is rejected, since it has no effect otherwise. func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForCapturedVariables(t *testing.T) { - _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--captured-variables", "names"}) + _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--captured-variables", "names"}) if err == nil { t.Fatalf("expected an error") } @@ -97,7 +97,7 @@ func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForCapturedVariables(t *t // Verifies --expect without --await is rejected, since it has no effect otherwise. func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForExpect(t *testing.T) { - _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--expect", "Health=100"}) + _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--expect", "Health=100"}) if err == nil { t.Fatalf("expected an error") } @@ -108,7 +108,7 @@ func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForExpect(t *testing.T) { // Verifies enable-pause-point without --await leaves File/Line/Id/Mode args untouched. func TestExtractPausePointEnableAwaitFlagsWithoutAwaitLeavesArgsUnchanged(t *testing.T) { - remaining, await, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--file", "Assets/Foo.cs", "--line", "10"}) + remaining, await, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--file", "Assets/Foo.cs", "--line", "10"}) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger.go b/cli/project-runner/internal/projectrunner/pause_point_trigger.go new file mode 100644 index 000000000..49bf1daec --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger.go @@ -0,0 +1,241 @@ +package projectrunner + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "strings" + "time" + + 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" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// pausePointTriggerResult carries the outcome of the --trigger command dispatched from inside +// await-pause-point/enable-pause-point --await, so a caller sees what the trigger did without a +// second CLI round trip. +type pausePointTriggerResult struct { + // Command echoes the --trigger string as given. + Command string `json:"Command"` + + // Completed is false when the pause-point wait settled (hit/timeout/cleared/etc.) before the + // trigger command's own goroutine returned within the short grace window this CLI waits for + // it. The triggered command keeps running inside Unity even after this CLI process exits — + // only this process's own wait stops. In practice simulate-* commands return immediately once + // paused (see PR-1), so this should only surface for a trigger unrelated to the pause itself. + Completed bool `json:"Completed"` + + // Response is the triggered command's raw, unmodified JSON response. + Response json.RawMessage `json:"Response,omitempty"` + + // Error is set only when the triggered command's own dispatch failed outright (unparseable + // output, non-zero exit with no JSON) — not when the triggered command completed with its own + // Success:false, which is passed through in Response untouched. + Error string `json:"Error,omitempty"` +} + +// parsePausePointTriggerCommand splits a --trigger value into a command name and its arguments, +// and rejects shapes that cannot behave sensibly when dispatched from inside this CLI process. +func parsePausePointTriggerCommand(command string, value string) (string, []string, error) { + tokens, err := tokenizePausePointTriggerValue(value) + if err != nil { + return "", nil, &clierrors.ArgumentError{ + Message: err.Error(), + Option: "--" + PausePointTriggerFlagName, + Command: command, + NextActions: []string{ + "Quote the trigger command consistently, e.g. --trigger \"simulate-keyboard --action Press --key Space --duration 5\".", + }, + } + } + if len(tokens) == 0 { + return "", nil, clierrors.MissingValueArgumentError("--" + PausePointTriggerFlagName) + } + + triggerCommand := tokens[0] + triggerArgs := tokens[1:] + + // A pause-point wait cannot make progress from inside another pause-point wait: there is no + // legitimate use case, only a wasted goroutine that outlives its parent's own timeout. + if triggerCommand == clicore.PausePointAwaitCommandName || triggerCommand == pausePointEnableCommandName { + return "", nil, &clierrors.ArgumentError{ + Message: fmt.Sprintf( + "--trigger cannot target %q: waiting for a pause point from inside another pause-point wait cannot make progress.", + triggerCommand), + Option: "--" + PausePointTriggerFlagName, + Command: command, + NextActions: []string{ + "Pass a command that performs an action (for example simulate-keyboard), not another pause-point wait.", + }, + } + } + + for _, arg := range triggerArgs { + if isPausePointFlag(arg, tooldocs.ProjectPathFlagName) { + return "", nil, &clierrors.ArgumentError{ + Message: "--trigger cannot include --project-path: the triggered command always runs " + + "against the same project as the parent command.", + Option: "--" + PausePointTriggerFlagName, + Command: command, + NextActions: []string{ + "Remove --project-path from the trigger command string.", + }, + } + } + } + + return triggerCommand, triggerArgs, nil +} + +// tokenizePausePointTriggerValue splits a --trigger value into argv-style tokens, honoring single +// and double quotes so an argument value (for example a key name) can contain a space. This is +// intentionally not a full shell parser (no escape sequences, no nested quoting) — the trigger +// strings this flag targets are plain `uloop` subcommand invocations, not shell pipelines. +func tokenizePausePointTriggerValue(raw string) ([]string, error) { + var tokens []string + var current strings.Builder + hasCurrent := false + inQuote := false + var quoteChar rune + + flush := func() { + if hasCurrent { + tokens = append(tokens, current.String()) + current.Reset() + hasCurrent = false + } + } + + for _, r := range raw { + switch { + case inQuote: + if r == quoteChar { + inQuote = false + continue + } + current.WriteRune(r) + case r == '\'' || r == '"': + inQuote = true + quoteChar = r + hasCurrent = true + case r == ' ' || r == '\t': + flush() + default: + current.WriteRune(r) + hasCurrent = true + } + } + if inQuote { + return nil, fmt.Errorf("unterminated %q quote in --trigger value", string(quoteChar)) + } + flush() + + return tokens, nil +} + +// dispatchPausePointTriggerCommand delegates to runResolvedProjectCommand by default. Injectable +// so tests can simulate a trigger command's dispatch outcome without a real Unity connection. +// runResolvedProjectCommand's own call graph loops back through waitForPausePoint into this very +// variable, so it is assigned in init() (a statement, not a variable initializer) rather than as +// this var's own initializer expression — Go's package-initialization dependency analysis reports +// a cycle for the latter even though the actual call only happens later, at runtime. +var dispatchPausePointTriggerCommand func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, +) int + +func init() { + dispatchPausePointTriggerCommand = runResolvedProjectCommand +} + +// pausePointTriggerHandle lets the pause-point wait loop start the trigger command concurrently +// (right after the marker is confirmed armed) and join it once the wait itself has settled. +type pausePointTriggerHandle struct { + command string + done chan *pausePointTriggerResult +} + +// startPausePointTrigger dispatches the trigger command in-process, through the same command +// dispatch entry point every ordinary CLI invocation uses (runResolvedProjectCommand) — not a +// subprocess. It races against the pause-point wait loop that started it. +func startPausePointTrigger( + ctx context.Context, + connection unityipc.Connection, + startPath string, + triggerCommand string, + triggerArgs []string, +) *pausePointTriggerHandle { + handle := &pausePointTriggerHandle{ + command: pausePointTriggerCommandString(triggerCommand, triggerArgs), + done: make(chan *pausePointTriggerResult, 1), + } + + go func() { + handle.done <- runPausePointTriggerSync(ctx, connection, startPath, triggerCommand, triggerArgs, handle.command) + }() + + return handle +} + +// join waits briefly for the trigger goroutine started by startPausePointTrigger, once the +// pause-point wait itself has already settled. The grace window mirrors +// pausePointFinalStatusProbeTimeout: a genuine hit interrupts simulate-* commands immediately +// (see PR-1), so a trigger that is still running after this window is treated as still in flight +// rather than blocking the whole await call further. +func (h *pausePointTriggerHandle) join() *pausePointTriggerResult { + if h == nil { + return nil + } + + select { + case result := <-h.done: + return result + case <-time.After(pausePointFinalStatusProbeTimeout): + return &pausePointTriggerResult{Command: h.command, Completed: false} + } +} + +func runPausePointTriggerSync( + ctx context.Context, + connection unityipc.Connection, + startPath string, + triggerCommand string, + triggerArgs []string, + commandString string, +) *pausePointTriggerResult { + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := dispatchPausePointTriggerCommand(ctx, connection, triggerCommand, triggerArgs, startPath, &stdout, &stderr) + + result := &pausePointTriggerResult{Command: commandString, Completed: true} + + trimmed := bytes.TrimSpace(stdout.Bytes()) + if json.Valid(trimmed) { + result.Response = json.RawMessage(trimmed) + return result + } + + result.Error = strings.TrimSpace(stderr.String()) + if result.Error == "" { + result.Error = fmt.Sprintf( + "trigger command %q exited with code %d and produced no parseable output", commandString, exitCode) + } + return result +} + +func pausePointTriggerCommandString(triggerCommand string, triggerArgs []string) string { + if len(triggerArgs) == 0 { + return triggerCommand + } + return triggerCommand + " " + strings.Join(triggerArgs, " ") +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go new file mode 100644 index 000000000..12e33cf87 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go @@ -0,0 +1,471 @@ +package projectrunner + +import ( + "bytes" + "context" + "encoding/json" + "io" + "strings" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// Verifies the quote-aware tokenizer splits a --trigger value into argv-style tokens, including +// values that contain a space when quoted, and rejects an unterminated quote. +func TestTokenizePausePointTriggerValue(t *testing.T) { + cases := []struct { + name string + raw string + want []string + wantErr bool + }{ + { + name: "plain unquoted tokens", + raw: "simulate-keyboard --action Press --key Space --duration 5", + want: []string{"simulate-keyboard", "--action", "Press", "--key", "Space", "--duration", "5"}, + }, + { + name: "double-quoted value with a space", + raw: `simulate-mouse-input --action Move --position "10 20"`, + want: []string{"simulate-mouse-input", "--action", "Move", "--position", "10 20"}, + }, + { + name: "single-quoted value with a space", + raw: `simulate-mouse-input --action Move --position '10 20'`, + want: []string{"simulate-mouse-input", "--action", "Move", "--position", "10 20"}, + }, + { + name: "unterminated quote", + raw: `simulate-keyboard --key "Space`, + wantErr: true, + }, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + got, err := tokenizePausePointTriggerValue(testCase.raw) + if testCase.wantErr { + if err == nil { + t.Fatalf("expected an error, got tokens: %#v", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != len(testCase.want) { + t.Fatalf("token count mismatch: got %#v, want %#v", got, testCase.want) + } + for index, token := range got { + if token != testCase.want[index] { + t.Fatalf("token[%d] mismatch: got %q, want %q", index, token, testCase.want[index]) + } + } + }) + } +} + +// Verifies --trigger parsing accepts a plain command and rejects the shapes that cannot behave +// sensibly when dispatched from inside a pause-point wait: another pause-point wait command, and +// an explicit --project-path override. +func TestParsePausePointTriggerCommand(t *testing.T) { + cases := []struct { + name string + value string + wantCommand string + wantArgs []string + wantErrText string + }{ + { + name: "plain trigger command", + value: "simulate-keyboard --action Press --key Space --duration 5", + wantCommand: "simulate-keyboard", + wantArgs: []string{"--action", "Press", "--key", "Space", "--duration", "5"}, + }, + { + name: "empty value is missing", + value: "", + wantErrText: "requires a value", + }, + { + name: "rejects await-pause-point as trigger target", + value: "await-pause-point --id other", + wantErrText: "cannot make progress", + }, + { + name: "rejects enable-pause-point as trigger target", + value: "enable-pause-point --id other", + wantErrText: "cannot make progress", + }, + { + name: "rejects an explicit --project-path", + value: "simulate-keyboard --action Press --project-path /tmp/OtherProject", + wantErrText: "cannot include --project-path", + }, + { + name: "rejects --project-path= form", + value: "simulate-keyboard --action Press --project-path=/tmp/OtherProject", + wantErrText: "cannot include --project-path", + }, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + command, args, err := parsePausePointTriggerCommand("await-pause-point", testCase.value) + if testCase.wantErrText != "" { + if err == nil { + t.Fatalf("expected an error containing %q", testCase.wantErrText) + } + if !strings.Contains(err.Error(), testCase.wantErrText) { + t.Fatalf("error mismatch: got %v, want contains %q", err, testCase.wantErrText) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if command != testCase.wantCommand { + t.Fatalf("command mismatch: got %q, want %q", command, testCase.wantCommand) + } + if len(args) != len(testCase.wantArgs) { + t.Fatalf("args mismatch: got %#v, want %#v", args, testCase.wantArgs) + } + for index, arg := range args { + if arg != testCase.wantArgs[index] { + t.Fatalf("args[%d] mismatch: got %q, want %q", index, arg, testCase.wantArgs[index]) + } + } + }) + } +} + +// Verifies runPausePointTriggerSync passes a successfully dispatched trigger command's raw JSON +// response through untouched, and reports a dispatch-level failure (unparseable output) as Error +// instead, falling back to a synthesized message when the dispatched command wrote nothing to +// stderr. +func TestRunPausePointTriggerSync(t *testing.T) { + originalDispatch := dispatchPausePointTriggerCommand + defer func() { dispatchPausePointTriggerCommand = originalDispatch }() + + t.Run("passes through a successful trigger response", func(t *testing.T) { + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + _, _ = stdout.Write([]byte(`{"Success":true,"InterruptedByPausePoint":true}`)) + return 0 + } + + result := runPausePointTriggerSync( + context.Background(), unityipc.Connection{}, "", "simulate-keyboard", []string{"--action", "Press"}, + "simulate-keyboard --action Press") + + if !result.Completed { + t.Fatalf("expected Completed=true, got %#v", result) + } + if result.Error != "" { + t.Fatalf("expected no error, got %#v", result) + } + var response map[string]any + if err := json.Unmarshal(result.Response, &response); err != nil { + t.Fatalf("Response is not valid JSON: %v (%#v)", err, result) + } + if response["InterruptedByPausePoint"] != true { + t.Fatalf("response passthrough mismatch: %#v", response) + } + }) + + t.Run("reports the dispatched command's stderr on dispatch failure", func(t *testing.T) { + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + _, _ = stderr.Write([]byte("unknown command: bogus-command")) + return 1 + } + + result := runPausePointTriggerSync( + context.Background(), unityipc.Connection{}, "", "bogus-command", nil, "bogus-command") + + if !result.Completed { + t.Fatalf("expected Completed=true (the dispatch itself returned), got %#v", result) + } + if result.Response != nil { + t.Fatalf("expected no Response on failure, got %#v", result) + } + if result.Error != "unknown command: bogus-command" { + t.Fatalf("error mismatch: %#v", result) + } + }) + + t.Run("synthesizes an error when the dispatched command wrote nothing at all", func(t *testing.T) { + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + return 1 + } + + result := runPausePointTriggerSync( + context.Background(), unityipc.Connection{}, "", "bogus-command", nil, "bogus-command") + + if result.Error == "" || !strings.Contains(result.Error, "exited with code 1") { + t.Fatalf("expected a synthesized error message, got %#v", result) + } + }) +} + +// Verifies waitForPausePoint embeds the trigger result once the trigger completes before the +// pause-point wait settles, and reports Completed=false (without blocking further) when the +// trigger is still running after the short join grace window. +func TestWaitForPausePointJoinsTriggerResult(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + t.Run("trigger completes before the wait settles", func(t *testing.T) { + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + _, _, triggerResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if triggerResult == nil || !triggerResult.Completed { + t.Fatalf("expected a completed trigger result, got %#v", triggerResult) + } + if triggerResult.Command != "simulate-keyboard --action Press" { + t.Fatalf("command mismatch: %#v", triggerResult) + } + }) + + t.Run("trigger still running past the join grace window reports Completed=false", func(t *testing.T) { + release := make(chan struct{}) + t.Cleanup(func() { close(release) }) + + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + <-release + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + _, _, triggerResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if triggerResult == nil || triggerResult.Completed { + t.Fatalf("expected an in-flight (not completed) trigger result, got %#v", triggerResult) + } + if triggerResult.Command != "simulate-keyboard --action Press" { + t.Fatalf("command mismatch: %#v", triggerResult) + } + }) +} + +// Verifies a --trigger result is embedded in the timeout error envelope's Details, so a caller +// can see what the trigger command did even when the pause-point itself was never hit. +func TestRunWaitForPausePointEmbedsTriggerResultOnTimeout(t *testing.T) { + originalQuery := queryPausePointStatus + originalClear := clearPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + clearPausePointStatus = originalClear + dispatchPausePointTriggerCommand = originalDispatch + pausePointStatusPoll = originalPoll + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusEnabled, + IsEnabled: true, + EditorState: pausePointEditorState{IsPlaying: true, CapturedAt: "Current"}, + }, nil + } + clearPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Id: id, Status: pausePointStatusCleared}, nil + } + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runWaitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: 5 * time.Millisecond, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + }, &stdout, &stderr) + + if code != 1 { + t.Fatalf("expected timeout failure, got %d with stdout %s", code, stdout.String()) + } + envelope := parsePausePointErrorEnvelope(t, stderr.Bytes()) + triggerResult, ok := envelope.Error.Details["TriggerResult"].(map[string]any) + if !ok { + t.Fatalf("TriggerResult detail missing or wrong shape: %#v", envelope.Error.Details) + } + if triggerResult["Command"] != "simulate-keyboard --action Press" { + t.Fatalf("TriggerResult command mismatch: %#v", triggerResult) + } + if triggerResult["Completed"] != true { + t.Fatalf("TriggerResult should report Completed=true: %#v", triggerResult) + } +} + +// Verifies await-pause-point parses --trigger into the command/args pair and rejects a value that +// targets another pause-point wait. +func TestParseWaitForPausePointOptionsParsesTriggerFlag(t *testing.T) { + options, err := parseWaitForPausePointOptions([]string{ + "--id", "jump", "--trigger", "simulate-keyboard --action Press --key Space --duration 5", + }) + if err != nil { + t.Fatalf("parse failed: %v", err) + } + if options.triggerCommand != "simulate-keyboard" { + t.Fatalf("trigger command mismatch: %#v", options) + } + wantArgs := []string{"--action", "Press", "--key", "Space", "--duration", "5"} + if len(options.triggerArgs) != len(wantArgs) { + t.Fatalf("trigger args mismatch: %#v", options) + } + for index, arg := range options.triggerArgs { + if arg != wantArgs[index] { + t.Fatalf("trigger args[%d] mismatch: got %q, want %q", index, arg, wantArgs[index]) + } + } + + if _, err := parseWaitForPausePointOptions([]string{ + "--id", "jump", "--trigger", "await-pause-point --id other", + }); err == nil { + t.Fatalf("expected an error for a trigger targeting another pause-point wait") + } +} + +// Verifies enable-pause-point extracts --trigger alongside --await, and rejects --trigger without +// --await since it would otherwise have no effect. +func TestExtractPausePointEnableAwaitFlagsExtractsTrigger(t *testing.T) { + remaining, await, _, _, _, triggerCommand, triggerArgs, err := extractPausePointEnableAwaitFlags([]string{ + "--id", "jump", "--await", "--trigger", "simulate-keyboard --action Press --key Space --duration 5", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !await { + t.Fatalf("expected await to be true") + } + if triggerCommand != "simulate-keyboard" { + t.Fatalf("trigger command mismatch: %#v", triggerCommand) + } + wantArgs := []string{"--action", "Press", "--key", "Space", "--duration", "5"} + if len(triggerArgs) != len(wantArgs) { + t.Fatalf("trigger args mismatch: %#v", triggerArgs) + } + for index, arg := range triggerArgs { + if arg != wantArgs[index] { + t.Fatalf("trigger args[%d] mismatch: got %q, want %q", index, arg, wantArgs[index]) + } + } + if len(remaining) != 2 || remaining[0] != "--id" || remaining[1] != "jump" { + t.Fatalf("remaining args mismatch: %#v", remaining) + } +} + +// Verifies --trigger without --await is rejected, since it has no effect otherwise. +func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForTrigger(t *testing.T) { + _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ + "--id", "jump", "--trigger", "simulate-keyboard --action Press", + }) + if err == nil { + t.Fatalf("expected an error") + } + if !strings.Contains(err.Error(), "require --await") { + t.Fatalf("error message mismatch: %v", err) + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index 8df2c5817..8c9a984dd 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -40,6 +40,10 @@ type pausePointStatusResponse struct { // captured variable (current or history), so an agent doesn't mistake an empty // CapturedVariables array for "nothing was captured at this hit". CapturedVariableNameFilterNoMatch bool `json:"CapturedVariableNameFilterNoMatch,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"` } type pausePointEditorState struct { diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index 834e8ca4f..c041e0c14 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -43,6 +43,14 @@ type waitForPausePointOptions struct { capturedVariablesMode pausePointCapturedVariablesMode capturedVariableNames []string expectations []pausePointExpectation + + // triggerCommand/triggerArgs come from --trigger. startPath is threaded through so the + // trigger, when dispatched via runResolvedProjectCommand, can satisfy that function's + // signature; --trigger forbids --project-path, so the nested-project-path branch it would + // otherwise feed is unreachable here. + triggerCommand string + triggerArgs []string + startPath string } type pausePointStatusOptions struct { @@ -105,6 +113,7 @@ func runWaitForPausePointCommand( ctx context.Context, connection unityipc.Connection, args []string, + startPath string, stdout io.Writer, stderr io.Writer, ) int { @@ -116,6 +125,7 @@ func runWaitForPausePointCommand( }) return 1 } + options.startPath = startPath extendPausePointExpiryBeforeWait(ctx, connection, options, stderr) @@ -192,7 +202,7 @@ func runWaitForPausePoint( ) int { startedAt := time.Now() spinner := clicore.NewToolSpinner(stderr, clicore.PausePointAwaitCommandName) - response, state, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -208,6 +218,7 @@ func runWaitForPausePoint( // also requested via --captured-variable-names or --captured-variables=names. expectations := evaluatePausePointExpectations(response.CapturedVariables, options.expectations) + response.TriggerResult = triggerResult response = filterPausePointCapturedVariableHistory(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) @@ -259,6 +270,9 @@ func runWaitForPausePoint( } waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state) + if triggerResult != nil { + waitErr.Details["TriggerResult"] = triggerResult + } if state == pausePointWaitStateTimeout { // Best-effort: the timeout diagnosis must not depend on a second Unity round trip succeeding. logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) @@ -320,6 +334,13 @@ func parseWaitForPausePointOptions(args []string) (waitForPausePointOptions, err return waitForPausePointOptions{}, parseErr } options.expectations = append(options.expectations, expectation) + case PausePointTriggerFlagName: + triggerCommand, triggerArgs, parseErr := parsePausePointTriggerCommand(clicore.PausePointAwaitCommandName, value) + if parseErr != nil { + return waitForPausePointOptions{}, parseErr + } + options.triggerCommand = triggerCommand + options.triggerArgs = triggerArgs default: return waitForPausePointOptions{}, pausePointUnknownOptionError(clicore.PausePointAwaitCommandName, name) } @@ -393,10 +414,29 @@ func parsePausePointTimeoutSeconds(value string) (int, error) { return timeoutSeconds, nil } +// waitForPausePoint starts the --trigger command (if any) right away, since arm is already +// confirmed by this point (extendPausePointExpiryBeforeWait for await-pause-point; a successful +// enable response for enable-pause-point --await), then races it against the status poll loop. +// The trigger is joined once the wait itself settles, not before, so a slow trigger cannot delay +// reporting a pause-point hit. func waitForPausePoint( ctx context.Context, connection unityipc.Connection, options waitForPausePointOptions, +) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, error) { + var triggerHandle *pausePointTriggerHandle + if options.triggerCommand != "" { + triggerHandle = startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) + } + + response, state, err := waitForPausePointStatus(ctx, connection, options) + return response, state, triggerHandle.join(), err +} + +func waitForPausePointStatus( + ctx context.Context, + connection unityipc.Connection, + options waitForPausePointOptions, ) (pausePointStatusResponse, pausePointWaitState, error) { waitContext, cancel := context.WithTimeout(ctx, options.timeout) defer cancel() 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 c69a4975b..a18a74c74 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -115,6 +115,7 @@ func TestRunWaitForPausePointCommandExtendsExpiryBeforePolling(t *testing.T) { context.Background(), unityipc.Connection{}, []string{"--id", "jump", "--timeout-seconds", "7"}, + "", &stdout, &stderr) @@ -170,7 +171,7 @@ func TestWaitForPausePointReturnsHitAfterEnabledStatus(t *testing.T) { return response, nil } - response, state, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -424,7 +425,7 @@ func TestWaitForPausePointReturnsNotEnabledStateImmediately(t *testing.T) { }, nil } - response, state, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, diff --git a/cli/project-runner/internal/projectrunner/runner_commands.go b/cli/project-runner/internal/projectrunner/runner_commands.go index 2954f8f40..e9f520435 100644 --- a/cli/project-runner/internal/projectrunner/runner_commands.go +++ b/cli/project-runner/internal/projectrunner/runner_commands.go @@ -33,7 +33,7 @@ func runResolvedProjectCommand( case "focus-window": return clicore.RunFocusWindow(ctx, connection.ProjectRoot, stdout, stderr) case clicore.PausePointAwaitCommandName: - return runWaitForPausePointCommand(ctx, connection, commandArgs, stdout, stderr) + return runWaitForPausePointCommand(ctx, connection, commandArgs, startPath, stdout, stderr) case clicore.PausePointStatusUserCommandName: return runPausePointStatusCommand(ctx, connection, commandArgs, stdout, stderr) case pausePointEnableCommandName: From c0766d55cb8ccc5529ab3106c75e86ebc46ab1c2 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 21:31:51 +0900 Subject: [PATCH 2/4] Add regression harness scenario for await-pause-point --trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The --trigger flag's own manual verification procedure needs to stay runnable beyond this one review round, so capture it as a permanent harness script rather than a throwaway command sequence — reusing the KeyStateAfterPauseInterruption scene scaffolding from the existing trap harness. Confirmed passing locally: enable-pause-point + await-pause-point --trigger hits in 0s instead of waiting out the 5s triggered Press. --- docs/regression-harness.md | 1 + .../regression-harness-pause-point-trigger.sh | 89 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100755 scripts/regression-harness-pause-point-trigger.sh diff --git a/docs/regression-harness.md b/docs/regression-harness.md index 14ad0f138..9c14b8d98 100644 --- a/docs/regression-harness.md +++ b/docs/regression-harness.md @@ -30,3 +30,4 @@ scenario covers. | Trap | Scene | Script | |------|-------|--------| | Key state divergence after a pause-point interruption | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` | `scripts/regression-harness-key-state-after-pause-interruption.sh` | +| `await-pause-point --trigger` hits within the marker timeout instead of waiting out the triggered command's full duration | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` (reused) | `scripts/regression-harness-pause-point-trigger.sh` | diff --git a/scripts/regression-harness-pause-point-trigger.sh b/scripts/regression-harness-pause-point-trigger.sh new file mode 100755 index 000000000..89076ce97 --- /dev/null +++ b/scripts/regression-harness-pause-point-trigger.sh @@ -0,0 +1,89 @@ +#!/bin/sh +set -e +# Regression harness for the await-pause-point/enable-pause-point --await +# --trigger flag: dispatching the action that hits the marker from inside the +# wait call itself must report a hit within the marker's timeout, not fall +# back to the full duration of the triggered command. Reuses the +# KeyStateAfterPauseInterruption scene scaffolding (see +# regression-harness-key-state-after-pause-interruption.sh) but replaces its +# background-Press-then-separate-await flow with a single enable + +# await --trigger call. +# +# Usage: sh scripts/regression-harness-pause-point-trigger.sh [--project-path ] +# +# Prerequisites: +# - Assets/RegressionHarness/KeyStateAfterPauseInterruption/KeyStateAfterPauseInterruption.unity +# must be open in a running Unity Editor +# - uloop CLI must be installed +# - jq must be installed + +PROJECT_PATH="" +if [ "$1" = "--project-path" ] && [ -n "$2" ]; then + PROJECT_PATH="$2" +fi + +MARKER_ID="Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs:19" +MARKER_FILE="Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs" +MARKER_LINE="19" +PRESS_DURATION="5" +RESULT_FILE="$(mktemp)" + +run_uloop() { + if [ -n "$PROJECT_PATH" ]; then + uloop "$@" --project-path "$PROJECT_PATH" + else + uloop "$@" + fi +} + +log() { + printf "\033[36m[pause-point-trigger]\033[0m %s\n" "$1" +} + +cleanup() { + rm -f "$RESULT_FILE" + run_uloop clear-pause-point --all > /dev/null 2>&1 || true + run_uloop control-play-mode --action Stop > /dev/null 2>&1 || true +} +trap cleanup EXIT + +log "Stopping any existing Play Mode session..." +run_uloop control-play-mode --action Stop > /dev/null + +log "Clearing any existing pause-point markers..." +run_uloop clear-pause-point --all > /dev/null + +log "Starting Play Mode..." +run_uloop control-play-mode --action Play > /dev/null + +log "Arming pause-point on $MARKER_FILE:$MARKER_LINE..." +run_uloop enable-pause-point --file "$MARKER_FILE" --line "$MARKER_LINE" --timeout-seconds 30 > /dev/null + +log "Awaiting the marker with --trigger driving Press for ${PRESS_DURATION}s in one call..." +START_SECONDS=$(date +%s) +run_uloop await-pause-point --id "$MARKER_ID" --timeout-seconds 30 \ + --trigger "simulate-keyboard --action Press --key Space --duration $PRESS_DURATION" \ + > "$RESULT_FILE" 2>&1 || true +ELAPSED_SECONDS=$(( $(date +%s) - START_SECONDS )) + +STATUS="$(jq -r '.Status' "$RESULT_FILE")" +SUCCESS="$(jq -r '.Success' "$RESULT_FILE")" +TRIGGER_INTERRUPTED="$(jq -r '.TriggerResult.Response.InterruptedByPausePoint' "$RESULT_FILE")" + +if [ "$SUCCESS" != "true" ] || [ "$STATUS" != "Hit" ] || [ "$TRIGGER_INTERRUPTED" != "true" ]; then + log "FAIL: expected Success=true Status=Hit TriggerResult.Response.InterruptedByPausePoint=true, got Success=$SUCCESS Status=$STATUS InterruptedByPausePoint=$TRIGGER_INTERRUPTED" + cat "$RESULT_FILE" + exit 1 +fi + +# Why: a --trigger that isn't actually racing the wait would only ever return +# after the triggered command's own full duration, so an early hit is what +# distinguishes real concurrent dispatch from a coincidence. +if [ "$ELAPSED_SECONDS" -ge "$PRESS_DURATION" ]; then + log "FAIL: await-pause-point took ${ELAPSED_SECONDS}s, not less than the triggered Press duration of ${PRESS_DURATION}s." + cat "$RESULT_FILE" + exit 1 +fi + +log "PASS: await-pause-point --trigger hit after ${ELAPSED_SECONDS}s (< ${PRESS_DURATION}s triggered Press duration)." +exit 0 From 94cb67c99f6a3966f9abe619b9f2ddbb2c2d1b54 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 21:41:08 +0900 Subject: [PATCH 3/4] Confirm the marker is armed before dispatching --trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The advisor's final review found that await-pause-point's extendPausePointExpiryBeforeWait step is best-effort and does not confirm the marker is actually enabled, so a --trigger command (e.g. a 5s simulate-keyboard Press) was being dispatched into the running game even when the wait was about to fail immediately with NotEnabled — a typo'd or expired --id would still inject the triggered action with no corresponding wait watching for it. waitForPausePoint now runs one status query up front and only starts the trigger once the marker is confirmed enabled or already hit; a query failure is also treated as not armed. Verified live: await-pause-point --id --trigger "simulate-keyboard ..." now returns NotEnabled in ~74ms with no TriggerResult in the response, instead of letting Press run. --- .../projectrunner/pause_point_trigger_test.go | 55 +++++++++++++++++++ .../projectrunner/pause_point_wait.go | 25 +++++++-- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go index 12e33cf87..3d22caff6 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go @@ -326,6 +326,61 @@ func TestWaitForPausePointJoinsTriggerResult(t *testing.T) { }) } +// Verifies --trigger is never dispatched when the initial status query finds the marker not +// armed (unknown --id, already expired, etc.): dispatching the trigger's action into the running +// game would be a side effect with no corresponding wait, since the wait itself is about to fail. +func TestWaitForPausePointSkipsTriggerWhenNotArmed(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Id: id, Status: pausePointStatusNotEnabled}, nil + } + + dispatchCalled := false + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + dispatchCalled = true + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + _, state, triggerResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "does-not-exist", + timeoutSeconds: 1, + timeout: time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateNotEnabled { + t.Fatalf("expected not_enabled state, got %q", state) + } + if dispatchCalled { + t.Fatal("expected dispatchPausePointTriggerCommand not to be called for a not-armed marker") + } + if triggerResult != nil { + t.Fatalf("expected a nil trigger result, got %#v", triggerResult) + } +} + // Verifies a --trigger result is embedded in the timeout error envelope's Details, so a caller // can see what the trigger command did even when the pause-point itself was never hit. func TestRunWaitForPausePointEmbedsTriggerResultOnTimeout(t *testing.T) { diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index c041e0c14..eb3286c75 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -414,18 +414,19 @@ func parsePausePointTimeoutSeconds(value string) (int, error) { return timeoutSeconds, nil } -// waitForPausePoint starts the --trigger command (if any) right away, since arm is already -// confirmed by this point (extendPausePointExpiryBeforeWait for await-pause-point; a successful -// enable response for enable-pause-point --await), then races it against the status poll loop. -// The trigger is joined once the wait itself settles, not before, so a slow trigger cannot delay -// reporting a pause-point hit. +// waitForPausePoint confirms the marker is actually armed with one status query before starting +// the --trigger command: extendPausePointExpiryBeforeWait (the await-pause-point entry point) is +// best-effort and does not confirm arm, so a typo'd or already-expired --id must not still get the +// trigger dispatched into the running game before the wait immediately fails on it. Once confirmed +// armed (or already hit), the trigger races the status poll loop and is joined once the wait itself +// settles, so a slow trigger cannot delay reporting a pause-point hit. func waitForPausePoint( ctx context.Context, connection unityipc.Connection, options waitForPausePointOptions, ) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, error) { var triggerHandle *pausePointTriggerHandle - if options.triggerCommand != "" { + if options.triggerCommand != "" && pausePointIsArmed(ctx, connection, options.id) { triggerHandle = startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) } @@ -433,6 +434,18 @@ func waitForPausePoint( return response, state, triggerHandle.join(), err } +// pausePointIsArmed reports whether the marker is enabled or already hit. A query failure is +// treated as not armed: dispatching a --trigger command against a marker this CLI cannot even +// confirm exists would inject the trigger's action into the game with no corresponding wait. +func pausePointIsArmed(ctx context.Context, connection unityipc.Connection, id string) bool { + response, err := queryPausePointStatus(ctx, connection, id) + if err != nil { + return false + } + state := pausePointWaitStateForStatus(response.Status) + return state == "" || state == pausePointWaitStateHit +} + func waitForPausePointStatus( ctx context.Context, connection unityipc.Connection, From 9a5fc9817f25948fc45d3c29db0d99e1021e9c60 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 21:45:00 +0900 Subject: [PATCH 4/4] Always populate TriggerResult when --trigger was given, even on skip A skipped trigger (marker not confirmed armed) previously left TriggerResult entirely omitted, indistinguishable from "the trigger ran but never reported back in time" (Completed=false with a Command). If the initial arm-check query hits a transient IPC error while the marker itself is actually armed, the trigger silently never fires, the marker never gets hit, the wait times out, and nothing in the response explains why. waitForPausePoint now always returns a TriggerResult when --trigger was given: on skip, Completed=false with an explanatory Error, so the contract is simply "passing --trigger always yields a TriggerResult." --- .../projectrunner/pause_point_trigger_test.go | 15 +++++++++++++-- .../projectrunner/pause_point_wait.go | 19 +++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go index 3d22caff6..3c8c3f125 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go @@ -329,6 +329,8 @@ func TestWaitForPausePointJoinsTriggerResult(t *testing.T) { // Verifies --trigger is never dispatched when the initial status query finds the marker not // armed (unknown --id, already expired, etc.): dispatching the trigger's action into the running // game would be a side effect with no corresponding wait, since the wait itself is about to fail. +// TriggerResult is still populated (Completed:false, explanatory Error) rather than omitted, so a +// caller can tell "the trigger never ran" apart from "the trigger ran but never reported back." func TestWaitForPausePointSkipsTriggerWhenNotArmed(t *testing.T) { originalQuery := queryPausePointStatus originalDispatch := dispatchPausePointTriggerCommand @@ -376,8 +378,17 @@ func TestWaitForPausePointSkipsTriggerWhenNotArmed(t *testing.T) { if dispatchCalled { t.Fatal("expected dispatchPausePointTriggerCommand not to be called for a not-armed marker") } - if triggerResult != nil { - t.Fatalf("expected a nil trigger result, got %#v", triggerResult) + if triggerResult == nil { + t.Fatal("expected a non-nil trigger result explaining the skip, got nil") + } + if triggerResult.Completed { + t.Fatalf("expected Completed=false for a skipped trigger, got %#v", triggerResult) + } + if triggerResult.Command != "simulate-keyboard --action Press" { + t.Fatalf("command mismatch: %#v", triggerResult) + } + if triggerResult.Error == "" { + t.Fatalf("expected a non-empty Error explaining why the trigger was skipped, got %#v", triggerResult) } } diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index eb3286c75..2b76fa232 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -426,11 +426,26 @@ func waitForPausePoint( options waitForPausePointOptions, ) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, error) { var triggerHandle *pausePointTriggerHandle - if options.triggerCommand != "" && pausePointIsArmed(ctx, connection, options.id) { - triggerHandle = startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) + var skippedTriggerResult *pausePointTriggerResult + if options.triggerCommand != "" { + if pausePointIsArmed(ctx, connection, options.id) { + triggerHandle = startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) + } else { + // --trigger always yields a TriggerResult when given, even when skipped: a silently + // omitted TriggerResult would be indistinguishable from "the trigger ran but this CLI + // exited before it reported anything," hiding the real reason (marker not confirmed + // armed) behind a plain timeout/not-enabled error with no clue why the trigger never fired. + skippedTriggerResult = &pausePointTriggerResult{ + Command: pausePointTriggerCommandString(options.triggerCommand, options.triggerArgs), + Error: "trigger was not dispatched: the marker could not be confirmed armed at wait start", + } + } } response, state, err := waitForPausePointStatus(ctx, connection, options) + if skippedTriggerResult != nil { + return response, state, skippedTriggerResult, err + } return response, state, triggerHandle.join(), err }