From 16a21bd4f65396289089ae0158734fa1fabcd4af Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 20:38:17 +0900 Subject: [PATCH 01/13] fix(pause-point): abort the wait when Unity rejects the trigger before it runs A --trigger command that Unity refuses in preflight reports the refusal on stdout as a normal Success:false response, so the stderr error-envelope check never saw it and the wait ran out the marker's whole lifetime before reporting PAUSE_POINT_EXPIRED with the real cause buried in Details.TriggerResult. Read RejectedBeforeExecution and Message off the triggered command's own response and abort on the same terms as a CLI-side rejection: the command performed no action, so the marker can never be hit by it. A rejection owned by the awaited marker itself still does not abort - that is the marker having been hit before the trigger ran. Quote the rejection's own reason in PAUSE_POINT_TRIGGER_FAILED instead of asserting argument parsing or an unknown command name, state a failed trigger at the front of an expired or timed-out message, and pick the recovery step that matches where the rejection came from. --- .../projectrunner/pause_point_errors.go | 57 ++++- .../projectrunner/pause_point_errors_test.go | 121 ++++++++- .../pause_point_trigger_diagnosis.go | 48 +++- .../projectrunner/pause_point_wait_poll.go | 3 +- .../pause_point_wait_poll_test.go | 240 ++++++++++++++++++ 5 files changed, 457 insertions(+), 12 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index c64a321d9f..f74f8cc115 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -51,12 +51,7 @@ func pausePointWaitError( case pausePointWaitStateTriggerFailed: waitErr = pausePointStateError( clierrors.ErrorCodePausePointTriggerFailed, - "The --trigger command was rejected before it ran (argument parsing or an unknown command "+ - "name), so the wait was abandoned instead of waiting out the remaining timeout. This "+ - "command did not clear the marker: see Details.TriggerResult for the rejection and "+ - "Details.RemainingMilliseconds for how long the marker stays armed. A zero "+ - "RemainingMilliseconds with an empty Details.Status means the final status re-read "+ - "failed — run pause-point-status to confirm the marker.", + pausePointTriggerFailedMessage(triggerResult), projectRoot, options, response, @@ -64,7 +59,7 @@ func pausePointWaitError( // change first. Reporting a permanent failure as retryable is what made the original // incident waste a full timeout window on it. false) - waitErr.NextActions = pausePointTriggerFailedNextActions(options.id) + waitErr.NextActions = pausePointTriggerFailedNextActions(options.id, triggerResult) case pausePointWaitStateCleared: waitErr = pausePointStateError( clierrors.ErrorCodePausePointCleared, @@ -91,10 +86,44 @@ func pausePointWaitError( } if pausePointTriggerFailed(triggerResult) { waitErr.Details["TriggerFailed"] = true + waitErr.Message = prefixPausePointMessageWithTriggerFailure(waitErr.Message, state, triggerResult) } return waitErr } +// pausePointTriggerFailedMessage states what the rejection was and what it left behind. Why the +// reason is quoted rather than asserted: the rejection can come from this CLI (argument parsing, an +// unknown command name) or from Unity refusing the command before it ran, and naming the wrong one +// is what sent agents looking for a typo in a trigger value that was correct. +func pausePointTriggerFailedMessage(triggerResult *pausePointTriggerResult) string { + return fmt.Sprintf( + "The trigger was rejected before it ran (%s); the marker stayed armed and was never hit. "+ + "The wait was abandoned instead of waiting out the remaining timeout. This command did not "+ + "clear the marker: see Details.TriggerResult for the rejection and "+ + "Details.RemainingMilliseconds for how long the marker stays armed. A zero "+ + "RemainingMilliseconds with an empty Details.Status means the final status re-read "+ + "failed — run pause-point-status to confirm the marker.", + pausePointTriggerRejectionReason(triggerResult)) +} + +// prefixPausePointMessageWithTriggerFailure states a failed trigger in the top-level message of an +// expired or timed-out wait. Why: Details.TriggerFailed and Details.Hint already carry it, but an +// agent that reads only Error.Message otherwise sees a missed code path and re-triggers the same +// failing command. The trigger-failed state already says this in its own message. +func prefixPausePointMessageWithTriggerFailure( + message string, + state pausePointWaitState, + triggerResult *pausePointTriggerResult, +) string { + if state == pausePointWaitStateTriggerFailed { + return message + } + return fmt.Sprintf( + "The --trigger command failed (%s). %s", + pausePointTriggerRejectionReason(triggerResult), + message) +} + // pausePointTriggerFailedNextActions replaces the generic enable/id-mismatch guidance, which does // not apply here: the marker was confirmed armed and only the --trigger value is wrong. // @@ -107,7 +136,7 @@ func pausePointWaitError( // Why the await form carries the real id: it is the one recovery command this function can spell // out completely, and naming a command without its arguments is exactly the failure this guidance // exists to prevent. -func pausePointTriggerFailedNextActions(id string) []string { +func pausePointTriggerFailedNextActions(id string, triggerResult *pausePointTriggerResult) []string { return []string{ "Fix the --trigger value in the command you just ran and run that command again. Re-running " + "`enable-pause-point --await` is safe and is the cleanest reset: it restarts the marker's " + @@ -115,10 +144,20 @@ func pausePointTriggerFailedNextActions(id string) []string { fmt.Sprintf( "The marker is still armed, so you can also wait on it directly: "+ "uloop await-pause-point --id %q --trigger \"\"", id), - "For an INVALID_ARGUMENT rejection, check the rejected value against the triggered command's own --help; for UNKNOWN_COMMAND, the first token must be a uloop subcommand name written without the leading 'uloop'.", + pausePointTriggerRejectionRecoveryAction(triggerResult), } } +// pausePointTriggerRejectionRecoveryAction picks the recovery step that matches where the rejection +// came from. A Unity-side refusal has nothing to do with argument syntax, so the argument/command-name +// advice would send the caller looking for a typo in a value that was already correct. +func pausePointTriggerRejectionRecoveryAction(triggerResult *pausePointTriggerResult) string { + if pausePointTriggerRejectedBeforeExecution(triggerResult) { + return "For an INVALID_ARGUMENT rejection, check the rejected value against the triggered command's own --help; for UNKNOWN_COMMAND, the first token must be a uloop subcommand name written without the leading 'uloop'." + } + return "Fix the precondition named in the trigger message (for example enter Play Mode with 'uloop control-play-mode --action Play'), then run the same enable-pause-point command again." +} + const ( pausePointHintPlayModeNotRunning = "PlayMode is not running. Start PlayMode (or trigger the marker code path in Edit Mode), then wait again." pausePointHintEditorAlreadyPaused = "Unity is already paused, so gameplay cannot reach the marker. Resume PlayMode before waiting again." diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go index d976815d6b..c2523ce15b 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go @@ -3,6 +3,7 @@ package projectrunner import ( "encoding/json" "reflect" + "strings" "testing" ) @@ -420,7 +421,10 @@ const wantPausePointTriggerFailedUnknownCommandNextAction = "For an INVALID_ARGU // from UNKNOWN_COMMAND's leading-uloop format mistake, so a prefixed value is not sent to // the triggered command's --help. func TestPausePointTriggerFailedNextActionsDiagnosesUnknownCommandPrefix(t *testing.T) { - got := pausePointTriggerFailedNextActions("jump") + got := pausePointTriggerFailedNextActions("jump", &pausePointTriggerResult{ + Completed: true, + Error: argumentErrorTriggerStderr, + }) want := []string{ "Fix the --trigger value in the command you just ran and run that command again. Re-running " + "`enable-pause-point --await` is safe and is the cleanest reset: it restarts the marker's " + @@ -644,3 +648,118 @@ func TestPausePointTimeoutError_TriggerRejected_WinsOverNewHitBaseline(t *testin t.Fatalf("TriggerFailed mismatch: %#v", cliErr.Details["TriggerFailed"]) } } + +// Verifies a CLI-side rejection keeps the argument/command-name recovery step, which is the only +// cause that shape of rejection can have. +func TestPausePointTriggerFailedNextActionsForCliRejection(t *testing.T) { + result := &pausePointTriggerResult{ + Completed: true, + Error: argumentErrorTriggerStderr, + } + + actions := pausePointTriggerFailedNextActions("jump", result) + + if len(actions) != 3 { + t.Fatalf("expected three recovery steps, got %#v", actions) + } + if !strings.Contains(actions[2], "INVALID_ARGUMENT") { + t.Fatalf("expected the argument-syntax recovery step, got %q", actions[2]) + } +} + +// Verifies a Unity-side pre-execution rejection replaces the argument-syntax recovery step with the +// precondition step: nothing about the trigger's arguments was wrong. +func TestPausePointTriggerFailedNextActionsForUnityRejection(t *testing.T) { + result := &pausePointTriggerResult{ + Completed: true, + Response: []byte(`{"Success":false,"RejectedBeforeExecution":true,` + + `"Message":"PlayMode is not active. Use control-play-mode tool to start PlayMode first."}`), + } + + actions := pausePointTriggerFailedNextActions("jump", result) + + if len(actions) != 3 { + t.Fatalf("expected three recovery steps, got %#v", actions) + } + if strings.Contains(actions[2], "INVALID_ARGUMENT") { + t.Fatalf("the argument-syntax recovery step must not be used for a Unity-side rejection, got %q", actions[2]) + } + if !strings.Contains(actions[2], "uloop control-play-mode --action Play") { + t.Fatalf("expected the precondition recovery step, got %q", actions[2]) + } +} + +// Verifies an expired wait whose trigger failed states that in the top-level message, so an agent +// reading only Error.Message does not treat it as a missed code path. +func TestPausePointWaitErrorPrefixesExpiredMessageWhenTriggerFailed(t *testing.T) { + result := &pausePointTriggerResult{ + Completed: true, + Response: []byte(`{"Success":false,` + + `"Message":"PlayMode is not active. Use control-play-mode tool to start PlayMode first."}`), + } + + waitErr := pausePointWaitError( + "/project", + waitForPausePointOptions{id: "jump", timeoutSeconds: 10}, + pausePointStatusResponse{Id: "jump", Status: pausePointStatusExpired}, + pausePointWaitStateExpired, + false, + false, + result) + + if !strings.HasPrefix(waitErr.Message, "The --trigger command failed (PlayMode is not active.") { + t.Fatalf("expected the trigger failure stated first: %q", waitErr.Message) + } + if !strings.Contains(waitErr.Message, "Pause point expired before it was hit.") { + t.Fatalf("expected the original expiry message preserved: %q", waitErr.Message) + } +} + +// Verifies a timed-out wait whose trigger failed carries the same top-level statement. +func TestPausePointWaitErrorPrefixesTimeoutMessageWhenTriggerFailed(t *testing.T) { + result := &pausePointTriggerResult{ + Completed: true, + Response: []byte(`{"Success":false,"Message":"PlayMode is paused."}`), + } + + waitErr := pausePointWaitError( + "/project", + waitForPausePointOptions{id: "jump", timeoutSeconds: 10}, + pausePointStatusResponse{Id: "jump", Status: pausePointStatusEnabled}, + pausePointWaitStateTimeout, + false, + false, + result) + + if !strings.HasPrefix(waitErr.Message, "The --trigger command failed (PlayMode is paused). ") { + t.Fatalf("expected the trigger failure stated first: %q", waitErr.Message) + } + if !strings.Contains(waitErr.Message, "was not hit within 10s") { + t.Fatalf("expected the original timeout message preserved: %q", waitErr.Message) + } +} + +// Verifies the trigger-failed state does not double-state the failure: its own message already +// leads with the rejection. +func TestPausePointWaitErrorDoesNotPrefixTriggerFailedMessage(t *testing.T) { + result := &pausePointTriggerResult{ + Completed: true, + Error: argumentErrorTriggerStderr, + } + + waitErr := pausePointWaitError( + "/project", + waitForPausePointOptions{id: "jump", timeoutSeconds: 10}, + pausePointStatusResponse{Id: "jump", Status: pausePointStatusEnabled}, + pausePointWaitStateTriggerFailed, + false, + false, + result) + + if strings.Contains(waitErr.Message, "The --trigger command failed") { + t.Fatalf("the trigger-failed message must not be prefixed again: %q", waitErr.Message) + } + if !strings.HasPrefix(waitErr.Message, "The trigger was rejected before it ran (argument parsing or an unknown command name)") { + t.Fatalf("expected the rejection reason quoted in the message: %q", waitErr.Message) + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go index 39861450bb..2070224604 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go @@ -1,12 +1,58 @@ package projectrunner -import "encoding/json" +import ( + "encoding/json" + "strings" +) // 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"` + + // RejectedBeforeExecution is set by a triggered command whose preflight refused it, so the + // command performed no action at all. Unity reports this on stdout as a normal response, which + // is why the stderr error-envelope check alone cannot see it. + RejectedBeforeExecution bool `json:"RejectedBeforeExecution"` + + // Message carries the triggered command's own reason for failing, so the wait can state it + // instead of asserting a cause it did not observe. + Message string `json:"Message"` +} + +// pausePointTriggerRejectedByUnityBeforeExecution reports whether Unity refused the triggered +// command before it executed anything. Like the stderr-envelope check, this proves the trigger +// performed no action, so waiting out the marker's remaining lifetime cannot change the outcome. +// +// Why the awaited marker is excluded: a rejection naming the marker being awaited is the marker +// having been hit before the trigger ran (pausePointTriggerRefusalWarning diagnoses that case), and +// the hit itself is the wait's success, not a reason to abort. +func pausePointTriggerRejectedByUnityBeforeExecution( + result *pausePointTriggerResult, + awaitedPausePointID string, +) bool { + response, ok := decodePausePointTriggerResponse(result) + if !ok || response.Success == nil || *response.Success { + return false + } + if !response.RejectedBeforeExecution { + return false + } + return response.RejectedByActivePausePointId != awaitedPausePointID +} + +// pausePointTriggerRejectionReason returns the triggered command's own reason for the rejection, +// shaped to read inside a parenthesis: a trailing sentence period is dropped so the enclosing +// sentence does not end in "..)." Why a fallback: a stderr-envelope rejection carries no Unity +// response to quote, and its cause is always one of the two shapes +// pausePointTriggerRejectedBeforeExecution matches. +func pausePointTriggerRejectionReason(result *pausePointTriggerResult) string { + response, ok := decodePausePointTriggerResponse(result) + if ok && response.Message != "" { + return strings.TrimSuffix(strings.TrimSpace(response.Message), ".") + } + return "argument parsing or an unknown command name" } // pausePointTriggerRefusalWarning warns about the case where the marker was hit before the trigger diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go index 7159c6ac16..2348d33b4e 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go @@ -303,7 +303,8 @@ func waitForPausePointStatus( // buffered value, and the caller reuses the result received here instead of joining. state.triggerResult = result triggerDone = nil - if pausePointTriggerRejectedBeforeExecution(result) { + if pausePointTriggerRejectedBeforeExecution(result) || + pausePointTriggerRejectedByUnityBeforeExecution(result, options.id) { abortResponse, abortState := abortPausePointWaitAfterTriggerRejection( ctx, connection, options.id, state.lastResponse, state.baselineSequence, state.hasBaseline) return abortResponse, abortState, state.triggerResult, state.hasBaseline, nil diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go b/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go index 5070fd3601..c72cb29a7c 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go @@ -896,3 +896,243 @@ func TestWaitForPausePointUnarmedErrorOmitsStatusSuffixWhenArmQueryFails(t *test t.Fatalf("ResumePlayResult.Error mismatch: got %#v, want %q", resumeResult, pausePointResumeNotArmedAtWaitStartError) } } + +// unityPreflightRejectionTriggerStdout is a triggered command's own response for a rejection that +// happened before the command did anything, reported on stdout as a normal Unity response rather +// than as a CLI error envelope on stderr. +const unityPreflightRejectionTriggerStdout = `{"Success":false,` + + `"Message":"PlayMode is not active. Use control-play-mode tool to start PlayMode first.",` + + `"Action":"Press","RejectedBeforeExecution":true}` + +// Verifies a Unity-side pre-execution rejection reported on stdout aborts the wait immediately +// instead of waiting out the marker's remaining lifetime: the trigger performed no action, so the +// marker can never be hit by it. +func TestWaitForPausePointAbortsWhenUnityRejectsTriggerBeforeExecution(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + pausePointStatusPoll = originalPoll + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointArmedStatusResponse(id), 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(unityPreflightRejectionTriggerStdout)) + return 1 + } + + startedAt := time.Now() + _, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 60, + timeout: 10 * time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press", "--key", "Space"}, + }) + elapsed := time.Since(startedAt) + + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateTriggerFailed { + t.Fatalf("expected trigger_failed state, got %q", state) + } + if elapsed >= 5*time.Second { + t.Fatalf("expected an early abort, waited %v of a 10s timeout", elapsed) + } + if triggerResult == nil { + t.Fatal("expected a TriggerResult reporting the rejection, got nil") + } +} + +// Verifies a rejection owned by the awaited marker itself does not abort: that is the marker +// having been hit before the trigger ran, which the refusal warning already diagnoses. +func TestWaitForPausePointDoesNotAbortWhenTriggerRejectionNamesTheAwaitedMarker(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + pausePointStatusPoll = originalPoll + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointArmedStatusResponse(id), 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":false,"Message":"PlayMode is paused.","RejectedBeforeExecution":true,` + + `"RejectedByActivePausePointId":"jump"}`)) + return 1 + } + + _, state, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 60, + timeout: 50 * time.Millisecond, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateTimeout { + t.Fatalf("a rejection owned by the awaited marker must let the wait settle on its own, got state %q", state) + } +} + +// Verifies a mid-flight failure (the command started and then failed) does not abort the wait: +// only a rejection that provably happened before execution proves the marker can never be hit. +func TestWaitForPausePointDoesNotAbortWhenTriggerFailsMidFlight(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + pausePointStatusPoll = originalPoll + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointArmedStatusResponse(id), 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":false,"Message":"Key 'Spacf' is not a known key name."}`)) + return 1 + } + + _, state, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 60, + timeout: 50 * time.Millisecond, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press", "--key", "Spacf"}, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateTimeout { + t.Fatalf("a mid-flight trigger failure must not abort the wait, got state %q", state) + } +} + +// Verifies the abort on a Unity-side pre-execution rejection keeps the marker armed and reports +// the rejection's own reason in the top-level error message, instead of the argument-parsing +// wording that only fits a CLI-side rejection. +func TestRunWaitForPausePointReportsUnityRejectionReasonInMessage(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 pausePointArmedStatusResponse(id), nil + } + clearCalled := false + clearPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + clearCalled = true + 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(unityPreflightRejectionTriggerStdout)) + return 1 + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runWaitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 60, + timeout: 10 * time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press", "--key", "Space"}, + }, &stdout, &stderr) + + if code != 1 { + t.Fatalf("expected failure exit code, got %d with stdout %s", code, stdout.String()) + } + if clearCalled { + t.Fatal("expected the marker to stay armed: clear must not be called when the trigger was rejected") + } + + envelope := parsePausePointErrorEnvelope(t, stderr.Bytes()) + if envelope.Error.ErrorCode != clierrors.ErrorCodePausePointTriggerFailed { + t.Fatalf("error code mismatch: %#v", envelope.Error) + } + if strings.Contains(envelope.Error.Message, "argument parsing or an unknown command name") { + t.Fatalf("the CLI-side rejection wording must not be asserted for a Unity-side rejection: %#v", envelope.Error) + } + if !strings.Contains(envelope.Error.Message, "PlayMode is not active") { + t.Fatalf("expected the rejection's own reason in the top-level message: %#v", envelope.Error) + } + if !strings.Contains(envelope.Error.Message, "the marker stayed armed and was never hit") { + t.Fatalf("expected the marker's state stated in the top-level message: %#v", envelope.Error) + } +} From 21cb0f18babc750b17e979356b5c811972cf6544 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 20:39:04 +0900 Subject: [PATCH 02/13] fix(pause-point): lead the non-firing diagnosis with the event never having happened Every reason the shared non-firing hint listed assumed the awaited event occurred and the marker still missed it, so an agent whose collision or input simply never happened read the cached-dispatch and pre-bound-delegate explanations as the diagnosis and went looking for a patching bug. State the simplest cause first: check the game state with execute-dynamic-code before suspecting dispatch. The hint is shared by the timeout and expired diagnoses, so both gain it. --- .../projectrunner/pause_point_errors.go | 4 +++- .../projectrunner/pause_point_errors_test.go | 17 +++++++++++++++-- .../projectrunner/pause_point_wait_test.go | 4 ++-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index f74f8cc115..9e6c9c6dd1 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -188,12 +188,14 @@ const ( pausePointExpiredResolvedFieldsGuidance = "The marker stayed armed at the resolved line shown in Details; that line was never executed within the window." // Shared by both pausePointTimeoutHint and pausePointExpiredHint: reasons a wait saw no hit — - // a physics/message callback missing a pre-existing GameObject, a pre-bound delegate + // the awaited event simply not having happened, a physics/message callback missing a + // pre-existing GameObject, a pre-bound delegate // bypassing the patch, control flow exiting on an earlier branch, or --line resolving // against a compiled map that no longer matches the editor after a hot reload. Kept as a // single constant so the two hints stay in sync instead of drifting copies of the same // diagnosis. pausePointNonFiringPatternsHint = "If the target line never hit despite the trigger firing, check the non-firing patterns: " + + "(0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; " + "(1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; " + "(2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; " + "(3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. " + diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go index c2523ce15b..abeeda7b16 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go @@ -237,7 +237,7 @@ func TestPausePointWaitHintsDifferentiateHitWhenSkips(t *testing.T) { Status: pausePointStatusEnabled, EditorState: pausePointEditorState{IsPlaying: true}, }, - wantHint: "Marker was enabled but never hit. Confirm the id matches UloopPausePoint.Pause(\"\") and that the code path was executed. In fast-progressing games the state may have already moved past the marker (for example back to Ready or GameOver), so re-trigger the code path and wait again. If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method. If PlayMode kept progressing on its own while you were arranging state (timers, gravity, spawners), the scenario may have already been consumed before this marker could fire; next time, run `control-play-mode --action Pause` before setup and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds. If the target line never hit despite the trigger firing, check the non-firing patterns: (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", + wantHint: "Marker was enabled but never hit. Confirm the id matches UloopPausePoint.Pause(\"\") and that the code path was executed. In fast-progressing games the state may have already moved past the marker (for example back to Ready or GameOver), so re-trigger the code path and wait again. If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method. If PlayMode kept progressing on its own while you were arranging state (timers, gravity, spawners), the scenario may have already been consumed before this marker could fire; next time, run `control-play-mode --action Pause` before setup and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds. If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", }, { name: "expired without skipped conditional hits", @@ -246,7 +246,7 @@ func TestPausePointWaitHintsDifferentiateHitWhenSkips(t *testing.T) { Status: pausePointStatusExpired, EditorState: pausePointEditorState{IsPlaying: true}, }, - wantHint: "The enable-pause-point --timeout-seconds window (measured from enable, not from this wait) ran out before the marker was hit. If the target line never hit despite the trigger firing, check the non-firing patterns: (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", + wantHint: "The enable-pause-point --timeout-seconds window (measured from enable, not from this wait) ran out before the marker was hit. If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", }, } @@ -763,3 +763,16 @@ func TestPausePointWaitErrorDoesNotPrefixTriggerFailedMessage(t *testing.T) { t.Fatalf("expected the rejection reason quoted in the message: %q", waitErr.Message) } } + +// Verifies the non-firing patterns hint leads with the possibility that the awaited event never +// happened, before any dispatch-bypass explanation. +func TestPausePointNonFiringPatternsHintLeadsWithEventNeverOccurred(t *testing.T) { + if !strings.Contains(pausePointNonFiringPatternsHint, "(0) the awaited game event never occurred") { + t.Fatalf("expected pattern (0) in the hint: %q", pausePointNonFiringPatternsHint) + } + zeroIndex := strings.Index(pausePointNonFiringPatternsHint, "(0)") + oneIndex := strings.Index(pausePointNonFiringPatternsHint, "(1)") + if zeroIndex < 0 || oneIndex < 0 || zeroIndex > oneIndex { + t.Fatalf("pattern (0) must come before pattern (1): %q", pausePointNonFiringPatternsHint) + } +} 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 268ac4a720..049c4643a5 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -1556,7 +1556,7 @@ func TestPausePointTimeoutErrorIncludesDiagnosisHint(t *testing.T) { "If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. " + "If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method. " + "If PlayMode kept progressing on its own while you were arranging state (timers, gravity, spawners), the scenario may have already been consumed before this marker could fire; next time, run `control-play-mode --action Pause` before setup and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds. " + - "If the target line never hit despite the trigger firing, check the non-firing patterns: (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", + "If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", }, } @@ -1605,7 +1605,7 @@ func TestPausePointExpiredErrorIncludesDiagnosisHint(t *testing.T) { HitCount: 0, }, wantHint: "The enable-pause-point --timeout-seconds window (measured from enable, not from this wait) ran out before the marker was hit. " + - "If the target line never hit despite the trigger firing, check the non-firing patterns: (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", + "If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", }, } From b474dd04ce1276f3d5e8534e07052ac71c1b34fb Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 20:44:07 +0900 Subject: [PATCH 03/13] fix(pause-point): report a pre-execution refusal as a structured response field The CLI could not tell "the trigger was refused before it did anything" from "the trigger ran and failed": both arrive as Success:false with only a message, and matching message text is exactly the brittleness RejectedByActivePausePointId was introduced to avoid. A refusal that no pause point owns - PlayMode simply not running, the common case for a --trigger - carried no structured signal at all. Add RejectedBeforeExecution to the four tool responses a --trigger can dispatch and set it only on the PlayMode preflight branch. Each tool now builds that response through a factory, so a mid-flight failure cannot claim the flag by copying the shape. --- .../references/troubleshooting.md | 2 +- .../references/output.md | 1 + .../references/troubleshooting.md | 2 +- .../references/output.md | 1 + ...usePointPreflightRejectionResponseTests.cs | 143 ++++++++++++++++++ ...intPreflightRejectionResponseTests.cs.meta | 11 ++ .../SimulateKeyboardResponseContractTests.cs | 4 +- .../Skill/references/troubleshooting.md | 2 +- .../ReplayInput/ReplayInputResponse.cs | 9 ++ .../ReplayInput/ReplayInputResponseFactory.cs | 34 +++++ .../ReplayInputResponseFactory.cs.meta | 11 ++ .../ReplayInput/ReplayInputUseCase.cs | 8 +- .../KeyboardInputSimulationResponseFactory.cs | 20 +++ .../SimulateKeyboardResponse.cs | 9 ++ .../SimulateKeyboardUseCase.cs | 8 +- .../Skill/references/output.md | 1 + .../MouseInputSimulationResponseFactory.cs | 20 +++ .../SimulateMouseInputResponse.cs | 9 ++ .../SimulateMouseInputUseCase.cs | 8 +- .../MouseUiSimulationResponseFactory.cs | 3 +- .../SimulateMouseUiResponse.cs | 9 ++ 21 files changed, 289 insertions(+), 26 deletions(-) create mode 100644 Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs create mode 100644 Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs create mode 100644 Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs.meta diff --git a/.agents/skills/uloop-pause-point/references/troubleshooting.md b/.agents/skills/uloop-pause-point/references/troubleshooting.md index ad8ed123b1..d41a321933 100644 --- a/.agents/skills/uloop-pause-point/references/troubleshooting.md +++ b/.agents/skills/uloop-pause-point/references/troubleshooting.md @@ -16,7 +16,7 @@ Expired responses include `MethodEntryCount`: `0` means the armed method was nev 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. 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. +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`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## Locating Where Control Flow Stops diff --git a/.agents/skills/uloop-simulate-keyboard/references/output.md b/.agents/skills/uloop-simulate-keyboard/references/output.md index 873b24d45e..79b2891b7d 100644 --- a/.agents/skills/uloop-simulate-keyboard/references/output.md +++ b/.agents/skills/uloop-simulate-keyboard/references/output.md @@ -15,6 +15,7 @@ Returns JSON with: - `DeferredLatchSyncScheduled` (boolean): Set `true` on successful `ReleaseAll` / `KeyUp` when a one-shot player-update latch sync was scheduled. Omitted when false. The sync runs on the next Dynamic/Fixed/Manual Input System update (not Editor); while PlayMode is paused it therefore waits until resume. Gameplay polling during that same input update may still see a stale press; polling from `Update` after that input update should see the key up - `InterruptedByPausePoint` / `PausePointId` / `PausePointHitCount` / `PausePointHits`: Pause-point interruption info (all nullable except the boolean). `PausePointHits` lists every marker hit during this input in hit order; `PausePointId` only names the latest one. See the Pause Point Inspection section in SKILL.md - `RejectedByActivePausePointId` (string, nullable): Set when an active pause point rejected this call before any input was injected — distinct from `PausePointId`, which reports a marker hit during the call. When set, the action never happened, so do not read `Success` alone +- `RejectedBeforeExecution` (boolean): `true` when the PlayMode preflight refused this call before any input was injected, whatever the reason (PlayMode not running, PlayMode paused, an active pause point). A mid-flight failure leaves it `false`. `await-pause-point --trigger` reads this to abort the wait immediately instead of waiting the marker out - `PressDeliveredToGame` (boolean, nullable): Set only when a `Press` / `KeyDown` was interrupted by a pause point. `true`: the Input System applied the press in a gameplay update before the pause, so the game may already have consumed it. Do not retry; re-check the affected state and `pause-point-status`. `false`: the queued edge was discarded before any gameplay update, the game never observed a press, and a retry after resume is safe. `null` for other actions and uninterrupted responses. Distinct from `PressEdgeObserved`, which says whether a gameplay update saw the press edge (`wasPressedThisFrame`) and can still be `false` after apply - `PressEdgeObserved` (boolean, nullable): For `Press` and `KeyDown`, whether the press edge (`wasPressedThisFrame`) was visible inside a gameplay input update. `false` means the CLI succeeded but gameplay polling most likely missed the edge — verify with a focused log instead of trusting `Success` alone. `null` for every other action (`KeyUp`, `ReleaseAll`) and for timed-out responses; pause-point interruptions still report the observed value. When a single-shot pause point is armed, do not blindly retry on `false`: the input may still have registered late, so check `pause-point-status` for a hit first — a blind retry can consume a re-enabled marker or double-fire the scenario - `PressHoldExtendedFrames` (integer, nullable): Extra observation frames the key stayed held beyond the normal duration window while waiting for `wasPressedThisFrame`; `null` when the release was not delayed diff --git a/.claude/skills/uloop-pause-point/references/troubleshooting.md b/.claude/skills/uloop-pause-point/references/troubleshooting.md index ad8ed123b1..d41a321933 100644 --- a/.claude/skills/uloop-pause-point/references/troubleshooting.md +++ b/.claude/skills/uloop-pause-point/references/troubleshooting.md @@ -16,7 +16,7 @@ Expired responses include `MethodEntryCount`: `0` means the armed method was nev 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. 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. +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`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## Locating Where Control Flow Stops diff --git a/.claude/skills/uloop-simulate-keyboard/references/output.md b/.claude/skills/uloop-simulate-keyboard/references/output.md index 873b24d45e..79b2891b7d 100644 --- a/.claude/skills/uloop-simulate-keyboard/references/output.md +++ b/.claude/skills/uloop-simulate-keyboard/references/output.md @@ -15,6 +15,7 @@ Returns JSON with: - `DeferredLatchSyncScheduled` (boolean): Set `true` on successful `ReleaseAll` / `KeyUp` when a one-shot player-update latch sync was scheduled. Omitted when false. The sync runs on the next Dynamic/Fixed/Manual Input System update (not Editor); while PlayMode is paused it therefore waits until resume. Gameplay polling during that same input update may still see a stale press; polling from `Update` after that input update should see the key up - `InterruptedByPausePoint` / `PausePointId` / `PausePointHitCount` / `PausePointHits`: Pause-point interruption info (all nullable except the boolean). `PausePointHits` lists every marker hit during this input in hit order; `PausePointId` only names the latest one. See the Pause Point Inspection section in SKILL.md - `RejectedByActivePausePointId` (string, nullable): Set when an active pause point rejected this call before any input was injected — distinct from `PausePointId`, which reports a marker hit during the call. When set, the action never happened, so do not read `Success` alone +- `RejectedBeforeExecution` (boolean): `true` when the PlayMode preflight refused this call before any input was injected, whatever the reason (PlayMode not running, PlayMode paused, an active pause point). A mid-flight failure leaves it `false`. `await-pause-point --trigger` reads this to abort the wait immediately instead of waiting the marker out - `PressDeliveredToGame` (boolean, nullable): Set only when a `Press` / `KeyDown` was interrupted by a pause point. `true`: the Input System applied the press in a gameplay update before the pause, so the game may already have consumed it. Do not retry; re-check the affected state and `pause-point-status`. `false`: the queued edge was discarded before any gameplay update, the game never observed a press, and a retry after resume is safe. `null` for other actions and uninterrupted responses. Distinct from `PressEdgeObserved`, which says whether a gameplay update saw the press edge (`wasPressedThisFrame`) and can still be `false` after apply - `PressEdgeObserved` (boolean, nullable): For `Press` and `KeyDown`, whether the press edge (`wasPressedThisFrame`) was visible inside a gameplay input update. `false` means the CLI succeeded but gameplay polling most likely missed the edge — verify with a focused log instead of trusting `Success` alone. `null` for every other action (`KeyUp`, `ReleaseAll`) and for timed-out responses; pause-point interruptions still report the observed value. When a single-shot pause point is armed, do not blindly retry on `false`: the input may still have registered late, so check `pause-point-status` for a hit first — a blind retry can consume a re-enabled marker or double-fire the scenario - `PressHoldExtendedFrames` (integer, nullable): Extra observation frames the key stayed held beyond the normal duration window while waiting for `wasPressedThisFrame`; `null` when the release was not delayed diff --git a/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs b/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs new file mode 100644 index 0000000000..e7b340554d --- /dev/null +++ b/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs @@ -0,0 +1,143 @@ +#nullable enable +using Newtonsoft.Json; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture for the structured "this command was refused before it did anything" flag every + /// tool a pause-point --trigger can dispatch reports. The CLI aborts a wait on that flag, so the + /// wiring from a rejected preflight to the wire field is what these tests pin. + /// + public class PausePointPreflightRejectionResponseTests + { + [Test] + public void SimulateKeyboardResponse_ByDefault_DoesNotClaimRejectionBeforeExecution() + { + // Verifies a failure that is not a preflight rejection leaves the flag false. + SimulateKeyboardResponse response = new() { Success = false, Message = "Key 'Spacf' is not a known key name." }; + + Assert.That(response.RejectedBeforeExecution, Is.False); + } + + [Test] + public void SimulateKeyboardResponse_WhenRejectedBeforeExecution_SerializesTheFlag() + { + // Verifies simulate-keyboard reports the flag under the exact name the CLI matches on. + string json = JsonConvert.SerializeObject( + new SimulateKeyboardResponse { Success = false, RejectedBeforeExecution = true }); + + Assert.That(json, Does.Contain("\"RejectedBeforeExecution\":true")); + } + + [Test] + public void SimulateMouseInputResponse_WhenRejectedBeforeExecution_SerializesTheFlag() + { + // Verifies simulate-mouse-input reports the flag under the exact name the CLI matches on. + string json = JsonConvert.SerializeObject( + new SimulateMouseInputResponse { Success = false, RejectedBeforeExecution = true }); + + Assert.That(json, Does.Contain("\"RejectedBeforeExecution\":true")); + } + + [Test] + public void SimulateMouseUiResponse_WhenRejectedBeforeExecution_SerializesTheFlag() + { + // Verifies simulate-mouse-ui reports the flag under the exact name the CLI matches on. + string json = JsonConvert.SerializeObject( + new SimulateMouseUiResponse { Success = false, RejectedBeforeExecution = true }); + + Assert.That(json, Does.Contain("\"RejectedBeforeExecution\":true")); + } + + [Test] + public void ReplayInputResponse_WhenRejectedBeforeExecution_SerializesTheFlag() + { + // Verifies replay-input reports the flag under the exact name the CLI matches on. + string json = JsonConvert.SerializeObject( + new ReplayInputResponse { Success = false, RejectedBeforeExecution = true }); + + Assert.That(json, Does.Contain("\"RejectedBeforeExecution\":true")); + } + + [Test] + public void MouseUiPreflightFailure_WhenPreflightRejected_ReportsRejectionBeforeExecution() + { + // Verifies simulate-mouse-ui's preflight rejection response sets the flag and names the marker. + SimulateMouseUiResponse response = MouseUiSimulationResponseFactory.CreatePreflightFailure( + CreateMouseUiCommand(), + PlayModeToolPreflightResult.FailureRejectedByPausePoint("PlayMode is paused.", "marker")); + + Assert.That(response.RejectedBeforeExecution, Is.True); + Assert.That(response.RejectedByActivePausePointId, Is.EqualTo("marker")); + } + + [Test] + public void MouseUiFailure_WhenNotAPreflightRejection_DoesNotClaimRejectionBeforeExecution() + { + // Verifies a mid-flight simulate-mouse-ui failure leaves the flag false. + SimulateMouseUiResponse response = MouseUiSimulationResponseFactory.CreateFailure( + CreateMouseUiCommand(), + "No EventSystem in the scene."); + + Assert.That(response.RejectedBeforeExecution, Is.False); + } + +#if ULOOP_HAS_INPUT_SYSTEM + [Test] + public void KeyboardPreflightFailure_WhenPreflightRejected_ReportsRejectionBeforeExecution() + { + // Verifies simulate-keyboard's preflight rejection response sets the flag and names the marker. + SimulateKeyboardResponse response = KeyboardInputSimulationResponseFactory.PreflightRejectedResult( + UnityCliLoopKeyboardAction.Press, + PlayModeToolPreflightResult.FailureRejectedByPausePoint("PlayMode is paused.", "marker")); + + Assert.That(response.RejectedBeforeExecution, Is.True); + Assert.That(response.RejectedByActivePausePointId, Is.EqualTo("marker")); + Assert.That(response.Success, Is.False); + Assert.That(response.Message, Is.EqualTo("PlayMode is paused.")); + } + + [Test] + public void MouseInputPreflightFailure_WhenPreflightRejected_ReportsRejectionBeforeExecution() + { + // Verifies simulate-mouse-input's preflight rejection response sets the flag and names the marker. + SimulateMouseInputResponse response = MouseInputSimulationResponseFactory.PreflightRejectedResult( + UnityCliLoopMouseInputAction.Click, + PlayModeToolPreflightResult.FailureRejectedByPausePoint("PlayMode is paused.", "marker")); + + Assert.That(response.RejectedBeforeExecution, Is.True); + Assert.That(response.RejectedByActivePausePointId, Is.EqualTo("marker")); + Assert.That(response.Success, Is.False); + } + + [Test] + public void ReplayInputPreflightFailure_WhenPreflightRejected_ReportsRejectionBeforeExecution() + { + // Verifies replay-input's preflight rejection response sets the flag and names the marker. + ReplayInputResponse response = ReplayInputResponseFactory.PreflightRejectedResult( + ReplayInputAction.Start, + PlayModeToolPreflightResult.FailureRejectedByPausePoint("PlayMode is paused.", "marker")); + + Assert.That(response.RejectedBeforeExecution, Is.True); + Assert.That(response.RejectedByActivePausePointId, Is.EqualTo("marker")); + Assert.That(response.Success, Is.False); + } +#endif + + private static MouseUiSimulationCommand CreateMouseUiCommand() + { + (MouseUiSimulationCommand? command, string? errorMessage) = + MouseUiSimulationCommand.TryFromSchema(new SimulateMouseUiSchema + { + Action = UnityCliLoopMouseUiAction.Click + }); + Assert.That(errorMessage, Is.Null); + Assert.That(command, Is.Not.Null); + return command!; + } + } +} diff --git a/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs.meta b/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs.meta new file mode 100644 index 0000000000..e591e80d22 --- /dev/null +++ b/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2cb9206db02014c74988811f45fe76d9 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SimulateKeyboardResponseContractTests.cs b/Assets/Tests/Editor/SimulateKeyboardResponseContractTests.cs index 3454a7de0f..4afe455495 100644 --- a/Assets/Tests/Editor/SimulateKeyboardResponseContractTests.cs +++ b/Assets/Tests/Editor/SimulateKeyboardResponseContractTests.cs @@ -13,11 +13,13 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor public sealed class SimulateKeyboardResponseContractTests { private const string OmittedOptionalFieldsJson = - "{\"Message\":\"\",\"Action\":\"\",\"InterruptedByPausePoint\":false,\"Success\":true}"; + "{\"Message\":\"\",\"Action\":\"\",\"InterruptedByPausePoint\":false," + + "\"RejectedBeforeExecution\":false,\"Success\":true}"; private const string PopulatedOptionalFieldsJson = "{\"Message\":\"ok\",\"Action\":\"Press\",\"Warning\":\"focus editor\",\"KeyName\":\"Space\"," + "\"InterruptedByPausePoint\":false,\"RejectedByActivePausePointId\":\"marker\"," + + "\"RejectedBeforeExecution\":false," + "\"PausePointId\":\"hit\",\"PausePointHitCount\":1," + "\"PausePointHits\":[{\"Id\":\"hit\",\"HitCount\":1}]," + "\"PressEdgeObserved\":true,\"PressDeliveredToGame\":true,\"PressHoldExtendedFrames\":2," diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md index ad8ed123b1..d41a321933 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md @@ -16,7 +16,7 @@ Expired responses include `MethodEntryCount`: `0` means the armed method was nev 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. 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. +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`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## Locating Where Control Flow Stops diff --git a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs index 773aec61e8..a8437d569b 100644 --- a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponse.cs @@ -24,5 +24,14 @@ public class ReplayInputResponse : UnityCliLoopToolResponse /// awaits. /// public string? RejectedByActivePausePointId { get; set; } + + /// + /// True when this command was refused before it did anything: the PlayMode preflight + /// rejected it. Why a separate flag from RejectedByActivePausePointId: a refusal is not + /// always owned by a pause point (PlayMode simply not running is the common case), and the + /// CLI's --trigger wait has to abort on "the trigger performed no action" without matching + /// message text. Mid-flight failures leave this false. + /// + public bool RejectedBeforeExecution { get; set; } } } diff --git a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs new file mode 100644 index 0000000000..cff44a0d44 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs @@ -0,0 +1,34 @@ +#if ULOOP_HAS_INPUT_SYSTEM +#nullable enable +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Creates wire-visible responses for replay-input outcomes that carry structured state beyond a + /// message. + /// + internal static class ReplayInputResponseFactory + { + /// + /// Creates the failure response for a rejected PlayMode preflight. Separate from the other + /// failure shapes so only a genuine pre-execution refusal can claim RejectedBeforeExecution: + /// the CLI aborts a pause-point wait on that flag. + /// + internal static ReplayInputResponse PreflightRejectedResult( + ReplayInputAction action, + PlayModeToolPreflightResult preflight) + { + Debug.Assert(!preflight.IsValid, "PreflightRejectedResult must only be called for a rejected preflight"); + return new ReplayInputResponse + { + Success = false, + Message = preflight.ErrorMessage, + Action = action.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId, + RejectedBeforeExecution = true + }; + } + } +} +#endif diff --git a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs.meta b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs.meta new file mode 100644 index 0000000000..1d5cf035bc --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputResponseFactory.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 50ede3b3fbad04500966f6782171e75b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs index d23f144e15..590ff750c9 100644 --- a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs @@ -97,13 +97,7 @@ private static ReplayInputResponse ExecuteStart(ReplayInputSchema request) PlayModeToolPreflightResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) { - return new ReplayInputResponse - { - Success = false, - Message = preflight.ErrorMessage, - Action = ReplayInputAction.Start.ToString(), - RejectedByActivePausePointId = preflight.RejectedByActivePausePointId - }; + return ReplayInputResponseFactory.PreflightRejectedResult(ReplayInputAction.Start, preflight); } if (InputReplayer.IsReplaying) diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs index 4d189d1013..b098749a96 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs @@ -13,6 +13,26 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal static class KeyboardInputSimulationResponseFactory { + /// + /// Creates the failure response for a rejected PlayMode preflight. Separate from the other + /// failure shapes so only a genuine pre-execution refusal can claim RejectedBeforeExecution: + /// the CLI aborts a pause-point wait on that flag. + /// + internal static SimulateKeyboardResponse PreflightRejectedResult( + UnityCliLoopKeyboardAction action, + PlayModeToolPreflightResult preflight) + { + Debug.Assert(!preflight.IsValid, "PreflightRejectedResult must only be called for a rejected preflight"); + return new SimulateKeyboardResponse + { + Success = false, + Message = preflight.ErrorMessage, + Action = action.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId, + RejectedBeforeExecution = true + }; + } + // pressEdgeObserved stays nullable because KeyUp has no press edge to report; // Press/KeyDown must pass their observation so pause-point interruptions (the // most common E2E path) do not silently drop the field. diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs index 1a2ed740d6..80236b3d41 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs @@ -30,6 +30,15 @@ public class SimulateKeyboardResponse : UnityCliLoopToolResponse [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public string? RejectedByActivePausePointId { get; set; } + /// + /// True when this command was refused before it did anything: the PlayMode preflight + /// rejected it. Why a separate flag from RejectedByActivePausePointId: a refusal is not + /// always owned by a pause point (PlayMode simply not running is the common case), and the + /// CLI's --trigger wait has to abort on "the trigger performed no action" without matching + /// message text. Mid-flight failures leave this false. + /// + public bool RejectedBeforeExecution { get; set; } + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public string? PausePointId { get; set; } [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index 88b18927d7..f72ab92c6b 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -59,13 +59,7 @@ public async Task ExecuteAsync( : PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) { - return new SimulateKeyboardResponse - { - Success = false, - Message = preflight.ErrorMessage, - Action = parameters.Action.ToString(), - RejectedByActivePausePointId = preflight.RejectedByActivePausePointId - }; + return KeyboardInputSimulationResponseFactory.PreflightRejectedResult(parameters.Action, preflight); } if (parameters.Action == UnityCliLoopKeyboardAction.ReleaseAll) diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/references/output.md b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/references/output.md index 873b24d45e..79b2891b7d 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/references/output.md +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/references/output.md @@ -15,6 +15,7 @@ Returns JSON with: - `DeferredLatchSyncScheduled` (boolean): Set `true` on successful `ReleaseAll` / `KeyUp` when a one-shot player-update latch sync was scheduled. Omitted when false. The sync runs on the next Dynamic/Fixed/Manual Input System update (not Editor); while PlayMode is paused it therefore waits until resume. Gameplay polling during that same input update may still see a stale press; polling from `Update` after that input update should see the key up - `InterruptedByPausePoint` / `PausePointId` / `PausePointHitCount` / `PausePointHits`: Pause-point interruption info (all nullable except the boolean). `PausePointHits` lists every marker hit during this input in hit order; `PausePointId` only names the latest one. See the Pause Point Inspection section in SKILL.md - `RejectedByActivePausePointId` (string, nullable): Set when an active pause point rejected this call before any input was injected — distinct from `PausePointId`, which reports a marker hit during the call. When set, the action never happened, so do not read `Success` alone +- `RejectedBeforeExecution` (boolean): `true` when the PlayMode preflight refused this call before any input was injected, whatever the reason (PlayMode not running, PlayMode paused, an active pause point). A mid-flight failure leaves it `false`. `await-pause-point --trigger` reads this to abort the wait immediately instead of waiting the marker out - `PressDeliveredToGame` (boolean, nullable): Set only when a `Press` / `KeyDown` was interrupted by a pause point. `true`: the Input System applied the press in a gameplay update before the pause, so the game may already have consumed it. Do not retry; re-check the affected state and `pause-point-status`. `false`: the queued edge was discarded before any gameplay update, the game never observed a press, and a retry after resume is safe. `null` for other actions and uninterrupted responses. Distinct from `PressEdgeObserved`, which says whether a gameplay update saw the press edge (`wasPressedThisFrame`) and can still be `false` after apply - `PressEdgeObserved` (boolean, nullable): For `Press` and `KeyDown`, whether the press edge (`wasPressedThisFrame`) was visible inside a gameplay input update. `false` means the CLI succeeded but gameplay polling most likely missed the edge — verify with a focused log instead of trusting `Success` alone. `null` for every other action (`KeyUp`, `ReleaseAll`) and for timed-out responses; pause-point interruptions still report the observed value. When a single-shot pause point is armed, do not blindly retry on `false`: the input may still have registered late, so check `pause-point-status` for a hit first — a blind retry can consume a re-enabled marker or double-fire the scenario - `PressHoldExtendedFrames` (integer, nullable): Extra observation frames the key stayed held beyond the normal duration window while waiting for `wasPressedThisFrame`; `null` when the release was not delayed diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs index fa93a53e0a..7a916ae32b 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs @@ -13,6 +13,26 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal static class MouseInputSimulationResponseFactory { + /// + /// Creates the failure response for a rejected PlayMode preflight. Separate from the other + /// failure shapes so only a genuine pre-execution refusal can claim RejectedBeforeExecution: + /// the CLI aborts a pause-point wait on that flag. + /// + internal static SimulateMouseInputResponse PreflightRejectedResult( + UnityCliLoopMouseInputAction action, + PlayModeToolPreflightResult preflight) + { + Debug.Assert(!preflight.IsValid, "PreflightRejectedResult must only be called for a rejected preflight"); + return new SimulateMouseInputResponse + { + Success = false, + Message = preflight.ErrorMessage, + Action = action.ToString(), + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId, + RejectedBeforeExecution = true + }; + } + // Echoes the full conversion so callers can verify the Y-flip math against a // screenshot instead of trusting a hidden Screen.height-based flip. internal static SimulateMouseInputResponse SuccessButtonResult( diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs index 0b8f26888d..6d0450b649 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputResponse.cs @@ -56,6 +56,15 @@ public class SimulateMouseInputResponse : UnityCliLoopToolResponse /// public string? RejectedByActivePausePointId { get; set; } + /// + /// True when this command was refused before it did anything: the PlayMode preflight + /// rejected it. Why a separate flag from RejectedByActivePausePointId: a refusal is not + /// always owned by a pause point (PlayMode simply not running is the common case), and the + /// CLI's --trigger wait has to abort on "the trigger performed no action" without matching + /// message text. Mid-flight failures leave this false. + /// + public bool RejectedBeforeExecution { 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 d7c15f750e..f311a308be 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs @@ -59,13 +59,7 @@ public async Task ExecuteAsync( PlayModeToolPreflightResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) { - return new SimulateMouseInputResponse - { - Success = false, - Message = preflight.ErrorMessage, - Action = parameters.Action.ToString(), - RejectedByActivePausePointId = preflight.RejectedByActivePausePointId - }; + return MouseInputSimulationResponseFactory.PreflightRejectedResult(parameters.Action, preflight); } if (!System.Enum.IsDefined(typeof(UnityCliLoopMouseInputAction), parameters.Action)) diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs index 356eead17d..32bf262603 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiSimulationResponseFactory.cs @@ -39,7 +39,8 @@ internal static SimulateMouseUiResponse CreatePreflightFailure( Success = false, Message = preflight.ErrorMessage, Action = parameters.Action.ToString(), - RejectedByActivePausePointId = preflight.RejectedByActivePausePointId + RejectedByActivePausePointId = preflight.RejectedByActivePausePointId, + RejectedBeforeExecution = true }; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs index 8da0c6270a..1c7354dc75 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/SimulateMouseUiResponse.cs @@ -27,6 +27,15 @@ public class SimulateMouseUiResponse : UnityCliLoopToolResponse /// public string? RejectedByActivePausePointId { get; set; } + /// + /// True when this command was refused before it did anything: the PlayMode preflight + /// rejected it. Why a separate flag from RejectedByActivePausePointId: a refusal is not + /// always owned by a pause point (PlayMode simply not running is the common case), and the + /// CLI's --trigger wait has to abort on "the trigger performed no action" without matching + /// message text. Mid-flight failures leave this false. + /// + public bool RejectedBeforeExecution { get; set; } + public string? PausePointId { get; set; } public int? PausePointHitCount { get; set; } public List? PausePointHits { get; set; } From d01c8b76613dcbcf60c1a9729b24ae76579165ab Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 20:46:31 +0900 Subject: [PATCH 04/13] fix(pause-point): lead expired guidance with the awaited event, not cached dispatch An expired physics-message marker reported cached message dispatch as the leading explanation even when MethodEntryCount was a measured 0. That reads as "the patch was bypassed" and sends agents into recreate-the-GameObject workarounds, when a measured 0 is much better evidence that the collision or input the marker waited for never happened at all. State the game-state check first for an instrumented marker whose entry count is 0, and keep cached dispatch as the follow-up for a body that provably ran. An uninstrumented marker's 0 is unmeasured, so it keeps the previous guidance. --- Assets/Tests/Editor/PausePointTests.cs | 48 +++++++++++++++++-- .../PausePoints/UloopPausePointEntry.cs | 14 +++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index a98c3a7470..fb7919d58b 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -405,11 +405,53 @@ public void GetStatus_WhenPhysicsDispatchMayBypassAndMethodNeverEntered_ReportsB UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus("jump"); - Assert.That(snapshot.Message, Does.Contain("may have bypassed the patch")); + Assert.That(snapshot.Message, Does.Contain("the armed method was never entered")); Assert.That(snapshot.Message, Does.Not.Contain("The armed method was never invoked.")); Assert.That(snapshot.RecommendedNextAction, Does.Contain("destroy and recreate")); } + /// + /// What: with the method never entered, expiry leads with the awaited event possibly not + /// having happened and keeps the cached-dispatch explanation behind that, so a missing + /// collision is not read as a patching bug. + /// + [Test] + public void GetStatus_WhenPhysicsDispatchMayBypassAndMethodNeverEntered_LeadsWithTheEventNotHavingHappened() + { + UloopPausePointRegistry.SetMethodEntryInstrumented("jump"); + UloopPausePointRegistry.Enable("jump", 1, patchDispatchMayBypass: true); + _nowUtc = _nowUtc.AddSeconds(2); + + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus("jump"); + + Assert.That(snapshot.Message, Does.Contain("execute-dynamic-code")); + Assert.That( + snapshot.Message.IndexOf("execute-dynamic-code", System.StringComparison.Ordinal), + Is.LessThan(snapshot.Message.IndexOf("cached message dispatch", System.StringComparison.Ordinal)), + "the game-state check must come before the cached-dispatch explanation"); + Assert.That(snapshot.RecommendedNextAction, Does.Contain("execute-dynamic-code")); + Assert.That( + snapshot.RecommendedNextAction.IndexOf("execute-dynamic-code", System.StringComparison.Ordinal), + Is.LessThan(snapshot.RecommendedNextAction.IndexOf("destroy and recreate", System.StringComparison.Ordinal)), + "the game-state check must come before the GameObject-recreation workaround"); + } + + /// + /// What: without method-entry instrumentation a zero entry count is unmeasured, not + /// evidence, so expiry keeps the plain cached-dispatch guidance. + /// + [Test] + public void GetStatus_WhenPhysicsDispatchMayBypassAndNotInstrumented_KeepsCachedDispatchGuidance() + { + UloopPausePointRegistry.Enable("jump", 1, patchDispatchMayBypass: true); + _nowUtc = _nowUtc.AddSeconds(2); + + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus("jump"); + + Assert.That(snapshot.RecommendedNextAction, Does.Contain("cached physics dispatch")); + Assert.That(snapshot.RecommendedNextAction, Does.Not.Contain("execute-dynamic-code")); + } + /// /// What: a source-location enable on a physics message method expires with the /// cached-dispatch wording rather than "was never invoked". @@ -430,7 +472,7 @@ public void GetStatus_WhenSourceLocationPhysicsMarkerExpiresWithoutHit_ReportsBy UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(response.Id); - Assert.That(snapshot.Message, Does.Contain("may have bypassed the patch")); + Assert.That(snapshot.Message, Does.Contain("cached message dispatch bypassing the patch")); Assert.That(snapshot.Message, Does.Not.Contain("The armed method was never invoked.")); Assert.That(snapshot.RecommendedNextAction, Does.Contain("destroy and recreate")); } @@ -463,7 +505,7 @@ public void GetStatus_WhenExpiredPhysicsMarkerIsReEnabledWithoutClear_ReportsByp UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(secondEnable.Id); - Assert.That(snapshot.Message, Does.Contain("may have bypassed the patch")); + Assert.That(snapshot.Message, Does.Contain("cached message dispatch bypassing the patch")); Assert.That(snapshot.Message, Does.Not.Contain("The armed method was never invoked.")); Assert.That(snapshot.RecommendedNextAction, Does.Contain("destroy and recreate")); } diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs index 3ab536c186..62b18d03b6 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs @@ -397,9 +397,13 @@ private string CreateExpiredMessage(int methodEntryCount, int hitWhenSkippedCoun return $"Pause point expired before it was hit. The armed method ran {methodEntryCount} time(s) but the armed line was never reached (branch not taken)."; } + // Reaching here means the marker is instrumented and methodEntryCount is 0: both the + // uninstrumented case and every positive entry count returned above. Cached dispatch is + // therefore only one of two explanations, and the far more common one is that the + // awaited event never happened - so that is stated first. if (PatchDispatchMayBypass) { - return "Pause point expired before it was hit. No entry through the armed patch was recorded. This marker sits in (or is called from) a Unity physics message method, and Unity's cached message dispatch may have bypassed the patch even though the method body ran; MethodEntryCount 0 does not prove the method was never invoked. Destroy and recreate the target GameObject after enabling, or embed UloopPausePoint.Pause(\"id\") in the method body and arm it with enable-pause-point --id."; + return "Pause point expired before it was hit and the armed method was never entered. The awaited game event (collision, input, trigger) may simply not have happened during the wait; check the game state with execute-dynamic-code first. Only if the body provably ran, suspect Unity's cached message dispatch bypassing the patch."; } return "Pause point expired before it was hit. The armed method was never invoked."; @@ -431,6 +435,14 @@ private string CreateExpiredRecommendedNextAction(int methodEntryCount, int hitW return "The armed method ran but the armed line was never reached, so a longer --timeout-seconds alone will not help. Check the condition that guards the armed line (the trigger may have fired while it was false), then re-enable the marker and trigger the code path again once the precondition holds; --mode continuous keeps the marker armed across repeated attempts. Clearing the expired marker first is not required."; } + // Why before the plain cached-dispatch branch: a measured entry count of 0 makes "the + // event never happened" the leading explanation, while an uninstrumented marker's 0 is + // unmeasured and carries no such evidence. + if (PatchDispatchMayBypass && HasMethodEntryInstrumentation && methodEntryCount == 0) + { + return "Check the game state with execute-dynamic-code to confirm the awaited event happened, then re-arm the marker and trigger it again. If you can show the method body ran while MethodEntryCount stayed 0, destroy and recreate the target GameObject after enabling, or switch to UloopPausePoint.Pause(\"id\") with --id."; + } + if (PatchDispatchMayBypass) { return "Confirm whether the method body actually ran (a log inside it, or a pause point on a plain method it calls). If it did, the patch was bypassed by cached physics dispatch: destroy and recreate the GameObject after enabling, or switch to UloopPausePoint.Pause(\"id\") with --id. Only raise --timeout-seconds if the body never ran."; From 44e12ab96ea57e26dbc7974274cdb1bf92126f64 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 20:48:50 +0900 Subject: [PATCH 05/13] fix(pause-point): offer the trigger-free await in the enable arming guidance A successful enable only ever named the --trigger form as the way to wait, so a marker driven by physics, a timer, or a multi-step action read as needing a trigger command it has no single command for. await-pause-point has never required --trigger. Name the blocking wait without a trigger between the status read and the one-call trigger form, and say the same in the quick-check reference. --- .../references/quick-check-template.md | 2 +- .../references/quick-check-template.md | 2 +- .../PausePointCompiledLineMapWarningTests.cs | 6 ++++- .../Editor/PausePointEnableGuidanceTests.cs | 26 +++++++++++++++++-- Assets/Tests/Editor/PausePointTests.cs | 2 +- .../Skill/references/quick-check-template.md | 2 +- .../PausePoint/SourcePausePointConstants.cs | 2 +- 7 files changed, 34 insertions(+), 8 deletions(-) diff --git a/.agents/skills/uloop-pause-point/references/quick-check-template.md b/.agents/skills/uloop-pause-point/references/quick-check-template.md index b13d75c278..0394f0f4bd 100644 --- a/.agents/skills/uloop-pause-point/references/quick-check-template.md +++ b/.agents/skills/uloop-pause-point/references/quick-check-template.md @@ -17,7 +17,7 @@ Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is reje 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. `execute-dynamic-code` is one such command: `--trigger "execute-dynamic-code --code-file "`. A short snippet can go inline because quotes keep whitespace as one argument (`--trigger "execute-dynamic-code --code 'return 1;'"`); the tokenizer does not handle escaped or nested quotes, so snippets that contain quotes or are otherwise complex should use `--code-file` — inline `--code` still works for simple snippets. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame). Commands that cannot run while PlayMode is paused are then rejected — a hit response 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; a timeout or expired wait that observed the same rejection also carries `Error.Details.TriggerFailed: true` and a Hint pointing at `Details.TriggerResult`. That safety valve does not apply to `execute-dynamic-code`, which still runs while paused: a pre-hit trigger of it will execute. 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 `Completed: false` — with the reason in `Error` when the trigger was skipped, or with an `Explanation` when the wait settled before the trigger reported its result). 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.md](fast-progressing-games.md). -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). `execute-dynamic-code` — whether `--code-file` or inline `--code` — is one command; do not split the wait just to run it. When you do need split steps: 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. +When the game reaches the line on its own, omit `--trigger`. To block until it hits, wait on the marker with no trigger command at all: `uloop await-pause-point --id --timeout-seconds ` (also offered in the enable response's `RecommendedNextAction`) — `--trigger` is optional there. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event). `execute-dynamic-code` — whether `--code-file` or inline `--code` — is one command; do not split the wait just to run it. When you do need split steps: 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. `--timeout-seconds` on enable starts the marker lifetime clock at enable time and is also the deadline `--await` waits against, so size it to cover both the trigger and the wait. Give any agent-shell timeout or yield window more room than `--timeout-seconds`, never the same value: the response prints only when the wait ends, so a wrapper that cuts off at the same boundary reports empty output even though the command completed and printed just past the cutoff. When that happens, do not re-run the enable blindly — read the outcome with `uloop pause-point-status --id `; an expired marker's record stays readable there. Some agent shells cap a foreground call's output window (often around 30 seconds) regardless of any timeout you configure and report the call as completed with empty output; when the wait must exceed that cap, use the split steps above (enable without --await, then await in the background) instead of one long foreground wait. diff --git a/.claude/skills/uloop-pause-point/references/quick-check-template.md b/.claude/skills/uloop-pause-point/references/quick-check-template.md index b13d75c278..0394f0f4bd 100644 --- a/.claude/skills/uloop-pause-point/references/quick-check-template.md +++ b/.claude/skills/uloop-pause-point/references/quick-check-template.md @@ -17,7 +17,7 @@ Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is reje 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. `execute-dynamic-code` is one such command: `--trigger "execute-dynamic-code --code-file "`. A short snippet can go inline because quotes keep whitespace as one argument (`--trigger "execute-dynamic-code --code 'return 1;'"`); the tokenizer does not handle escaped or nested quotes, so snippets that contain quotes or are otherwise complex should use `--code-file` — inline `--code` still works for simple snippets. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame). Commands that cannot run while PlayMode is paused are then rejected — a hit response 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; a timeout or expired wait that observed the same rejection also carries `Error.Details.TriggerFailed: true` and a Hint pointing at `Details.TriggerResult`. That safety valve does not apply to `execute-dynamic-code`, which still runs while paused: a pre-hit trigger of it will execute. 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 `Completed: false` — with the reason in `Error` when the trigger was skipped, or with an `Explanation` when the wait settled before the trigger reported its result). 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.md](fast-progressing-games.md). -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). `execute-dynamic-code` — whether `--code-file` or inline `--code` — is one command; do not split the wait just to run it. When you do need split steps: 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. +When the game reaches the line on its own, omit `--trigger`. To block until it hits, wait on the marker with no trigger command at all: `uloop await-pause-point --id --timeout-seconds ` (also offered in the enable response's `RecommendedNextAction`) — `--trigger` is optional there. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event). `execute-dynamic-code` — whether `--code-file` or inline `--code` — is one command; do not split the wait just to run it. When you do need split steps: 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. `--timeout-seconds` on enable starts the marker lifetime clock at enable time and is also the deadline `--await` waits against, so size it to cover both the trigger and the wait. Give any agent-shell timeout or yield window more room than `--timeout-seconds`, never the same value: the response prints only when the wait ends, so a wrapper that cuts off at the same boundary reports empty output even though the command completed and printed just past the cutoff. When that happens, do not re-run the enable blindly — read the outcome with `uloop pause-point-status --id `; an expired marker's record stays readable there. Some agent shells cap a foreground call's output window (often around 30 seconds) regardless of any timeout you configure and report the call as completed with empty output; when the wait must exceed that cap, use the split steps above (enable without --await, then await in the background) instead of one long foreground wait. diff --git a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs index 1b67a0af1e..475581d539 100644 --- a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs +++ b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs @@ -714,7 +714,11 @@ public void Enable_WhenCompiledLineMatchesEditedFile_UsesMatchedCompiledLineMapW + ResolveFailureFile + ":" + requestedLine - + "\". To arm, trigger, and collect in one call, add --await --resume-play --trigger \"\" next time."; + + "\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"" + + ResolveFailureFile + + ":" + + requestedLine + + "\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; Assert.That(response.RecommendedNextAction, Is.EqualTo(expectedArming)); AssertLineBasis(response, "LastCompiledSource"); } diff --git a/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs b/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs index 38a0370afa..1902b6183e 100644 --- a/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs +++ b/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs @@ -24,7 +24,7 @@ public sealed class PausePointEnableGuidanceTests private const int FixtureClosingBraceLine = 13; private const string ExpectedArmingNextActionForJump = - "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To arm, trigger, and collect in one call, add --await --resume-play --trigger \"\" next time."; + "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"jump\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; private const string ExpectedRearmDiscardWarningGeneration1 = "Generation 1 of this pause point had already hit; this re-arm discarded its CapturedVariables and CapturedVariableHistory. Read results with pause-point-status before re-arming when you need them."; @@ -83,6 +83,24 @@ public void ResolveSuccessEnableRecommendedNextAction_WhenExistingIsEmpty_Explai Assert.That(action, Does.Contain("without")); } + /// + /// What: arming guidance offers a blocking wait that needs no --trigger, so a marker driven + /// by physics or a multi-step action is not presented as requiring a trigger command. + /// + [Test] + public void ResolveSuccessEnableRecommendedNextAction_WhenExistingIsEmpty_OffersAwaitWithoutATrigger() + { + string action = PausePointEnableWarnings.ResolveSuccessEnableRecommendedNextAction( + string.Empty, + "jump"); + + Assert.That(action, Does.Contain("uloop await-pause-point --id \"jump\" --timeout-seconds")); + Assert.That( + action.IndexOf("await-pause-point", StringComparison.Ordinal), + Is.LessThan(action.IndexOf("--trigger", StringComparison.Ordinal)), + "the trigger-free wait must be offered before the trigger form"); + } + /// /// What: a non-empty RecommendedNextAction is kept verbatim and is not replaced by arming guidance. /// @@ -146,7 +164,11 @@ public void Enable_WhenFileLinePathSucceeds_SetsArmingRecommendedNextAction() + FixtureFilePath + ":" + FixtureStatementLine - + "\". To arm, trigger, and collect in one call, add --await --resume-play --trigger \"\" next time."; + + "\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"" + + FixtureFilePath + + ":" + + FixtureStatementLine + + "\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; Assert.That(response.RecommendedNextAction, Is.EqualTo(expected)); string json = JsonConvert.SerializeObject( response, diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index fb7919d58b..1d6a50c11a 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -1489,7 +1489,7 @@ public async Task Enable_WhenMarkerCreated_ReturnsStateManagementFields() Assert.That( response.RecommendedNextAction, Is.EqualTo( - "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To arm, trigger, and collect in one call, add --await --resume-play --trigger \"\" next time.")); + "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"jump\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\".")); } [Test] diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/quick-check-template.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/quick-check-template.md index b13d75c278..0394f0f4bd 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/quick-check-template.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/quick-check-template.md @@ -17,7 +17,7 @@ Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is reje 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. `execute-dynamic-code` is one such command: `--trigger "execute-dynamic-code --code-file "`. A short snippet can go inline because quotes keep whitespace as one argument (`--trigger "execute-dynamic-code --code 'return 1;'"`); the tokenizer does not handle escaped or nested quotes, so snippets that contain quotes or are otherwise complex should use `--code-file` — inline `--code` still works for simple snippets. One race does remain: the marker itself can hit before the trigger executes (for example on a line that runs every frame). Commands that cannot run while PlayMode is paused are then rejected — a hit response 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; a timeout or expired wait that observed the same rejection also carries `Error.Details.TriggerFailed: true` and a Hint pointing at `Details.TriggerResult`. That safety valve does not apply to `execute-dynamic-code`, which still runs while paused: a pre-hit trigger of it will execute. 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 `Completed: false` — with the reason in `Error` when the trigger was skipped, or with an `Explanation` when the wait settled before the trigger reported its result). 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.md](fast-progressing-games.md). -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). `execute-dynamic-code` — whether `--code-file` or inline `--code` — is one command; do not split the wait just to run it. When you do need split steps: 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. +When the game reaches the line on its own, omit `--trigger`. To block until it hits, wait on the marker with no trigger command at all: `uloop await-pause-point --id --timeout-seconds ` (also offered in the enable response's `RecommendedNextAction`) — `--trigger` is optional there. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event). `execute-dynamic-code` — whether `--code-file` or inline `--code` — is one command; do not split the wait just to run it. When you do need split steps: 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. `--timeout-seconds` on enable starts the marker lifetime clock at enable time and is also the deadline `--await` waits against, so size it to cover both the trigger and the wait. Give any agent-shell timeout or yield window more room than `--timeout-seconds`, never the same value: the response prints only when the wait ends, so a wrapper that cuts off at the same boundary reports empty output even though the command completed and printed just past the cutoff. When that happens, do not re-run the enable blindly — read the outcome with `uloop pause-point-status --id `; an expired marker's record stays readable there. Some agent shells cap a foreground call's output window (often around 30 seconds) regardless of any timeout you configure and report the call as completed with empty output; when the wait must exceed that cap, use the split steps above (enable without --await, then await in the background) instead of one long foreground wait. diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index c3a19f0709..79d52713db 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -430,7 +430,7 @@ internal static class SourcePausePointConstants // so agents arm a marker and then stall instead of running the path or using --await. // Format: marker id. public const string EnableSuccessArmingRecommendedNextActionFormat = - "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"{0}\". To arm, trigger, and collect in one call, add --await --resume-play --trigger \"\" next time."; + "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"{0}\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"{0}\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; // Why warn: Registry.Enable replaces the entry and drops CapturedVariables, // CapturedVariableHistory, and hit snapshots. The raw capture holder is kept on purpose. From b9afc6542b2c2e1c19b6adeb36b44d012e2248b6 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:06:14 +0900 Subject: [PATCH 06/13] fix(pause-point): stop blaming the trigger value for a Unity-side refusal The first recovery step told the caller to fix the --trigger value even when Unity had accepted the command as well-formed and refused it for Editor state, sending them to edit a command that was already correct. Branch it on the rejection's source the same way the third step already is: an envelope rejection keeps the trigger-value fix, a Unity-side pre-execution refusal says the trigger was valid and points at the state in its message. --- .../internal/projectrunner/pause_point_errors.go | 16 +++++++++++++--- .../projectrunner/pause_point_errors_test.go | 13 +++++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index 9e6c9c6dd1..76e7df4921 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -138,9 +138,7 @@ func prefixPausePointMessageWithTriggerFailure( // exists to prevent. func pausePointTriggerFailedNextActions(id string, triggerResult *pausePointTriggerResult) []string { return []string{ - "Fix the --trigger value in the command you just ran and run that command again. Re-running " + - "`enable-pause-point --await` is safe and is the cleanest reset: it restarts the marker's " + - "HitCount and --timeout-seconds countdown, and re-patching an already patched id is a no-op.", + pausePointTriggerRejectionFirstAction(triggerResult), fmt.Sprintf( "The marker is still armed, so you can also wait on it directly: "+ "uloop await-pause-point --id %q --trigger \"\"", id), @@ -148,6 +146,18 @@ func pausePointTriggerFailedNextActions(id string, triggerResult *pausePointTrig } } +// pausePointTriggerRejectionFirstAction states what has to change, which depends on where the +// rejection came from. Telling a caller to fix a --trigger value that Unity itself accepted as +// well-formed sends them editing a correct command instead of the Editor state that refused it. +func pausePointTriggerRejectionFirstAction(triggerResult *pausePointTriggerResult) string { + if pausePointTriggerRejectedBeforeExecution(triggerResult) { + return "Fix the --trigger value in the command you just ran and run that command again. Re-running " + + "`enable-pause-point --await` is safe and is the cleanest reset: it restarts the marker's " + + "HitCount and --timeout-seconds countdown, and re-patching an already patched id is a no-op." + } + return "The trigger command itself was valid; Unity refused to run it because of the state named in the trigger message." +} + // pausePointTriggerRejectionRecoveryAction picks the recovery step that matches where the rejection // came from. A Unity-side refusal has nothing to do with argument syntax, so the argument/command-name // advice would send the caller looking for a typo in a value that was already correct. diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go index abeeda7b16..aaef7cf639 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go @@ -649,8 +649,8 @@ func TestPausePointTimeoutError_TriggerRejected_WinsOverNewHitBaseline(t *testin } } -// Verifies a CLI-side rejection keeps the argument/command-name recovery step, which is the only -// cause that shape of rejection can have. +// Verifies a CLI-side rejection keeps both steps that name the --trigger value as what has to +// change: that is the only cause this shape of rejection can have. func TestPausePointTriggerFailedNextActionsForCliRejection(t *testing.T) { result := &pausePointTriggerResult{ Completed: true, @@ -662,6 +662,9 @@ func TestPausePointTriggerFailedNextActionsForCliRejection(t *testing.T) { if len(actions) != 3 { t.Fatalf("expected three recovery steps, got %#v", actions) } + if !strings.HasPrefix(actions[0], "Fix the --trigger value in the command you just ran") { + t.Fatalf("expected the trigger-value fix as the first step, got %q", actions[0]) + } if !strings.Contains(actions[2], "INVALID_ARGUMENT") { t.Fatalf("expected the argument-syntax recovery step, got %q", actions[2]) } @@ -681,6 +684,12 @@ func TestPausePointTriggerFailedNextActionsForUnityRejection(t *testing.T) { if len(actions) != 3 { t.Fatalf("expected three recovery steps, got %#v", actions) } + if strings.Contains(actions[0], "Fix the --trigger value") { + t.Fatalf("a Unity-side rejection must not blame the --trigger value, got %q", actions[0]) + } + if !strings.HasPrefix(actions[0], "The trigger command itself was valid;") { + t.Fatalf("expected the first step to clear the trigger value, got %q", actions[0]) + } if strings.Contains(actions[2], "INVALID_ARGUMENT") { t.Fatalf("the argument-syntax recovery step must not be used for a Unity-side rejection, got %q", actions[2]) } From a9c0b2a00a12aebe90273d565cd79a85ae5cf1b4 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:07:29 +0900 Subject: [PATCH 07/13] fix(pause-point): stop the non-firing hint from assuming a trigger fired The hint is shared by the timeout and expired diagnoses, and both reach it on a plain await-pause-point with no --trigger at all. Its opening clause claimed a trigger had fired, so the one case that most needs the patterns list was told they did not apply to it. --- .../internal/projectrunner/pause_point_errors.go | 2 +- .../internal/projectrunner/pause_point_errors_test.go | 4 ++-- .../internal/projectrunner/pause_point_wait_test.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index 76e7df4921..5e48b07331 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -204,7 +204,7 @@ const ( // against a compiled map that no longer matches the editor after a hot reload. Kept as a // single constant so the two hints stay in sync instead of drifting copies of the same // diagnosis. - pausePointNonFiringPatternsHint = "If the target line never hit despite the trigger firing, check the non-firing patterns: " + + pausePointNonFiringPatternsHint = "If the target line never hit, check the non-firing patterns: " + "(0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; " + "(1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; " + "(2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; " + diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go index aaef7cf639..c1d20f05e9 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go @@ -237,7 +237,7 @@ func TestPausePointWaitHintsDifferentiateHitWhenSkips(t *testing.T) { Status: pausePointStatusEnabled, EditorState: pausePointEditorState{IsPlaying: true}, }, - wantHint: "Marker was enabled but never hit. Confirm the id matches UloopPausePoint.Pause(\"\") and that the code path was executed. In fast-progressing games the state may have already moved past the marker (for example back to Ready or GameOver), so re-trigger the code path and wait again. If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method. If PlayMode kept progressing on its own while you were arranging state (timers, gravity, spawners), the scenario may have already been consumed before this marker could fire; next time, run `control-play-mode --action Pause` before setup and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds. If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", + wantHint: "Marker was enabled but never hit. Confirm the id matches UloopPausePoint.Pause(\"\") and that the code path was executed. In fast-progressing games the state may have already moved past the marker (for example back to Ready or GameOver), so re-trigger the code path and wait again. If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method. If PlayMode kept progressing on its own while you were arranging state (timers, gravity, spawners), the scenario may have already been consumed before this marker could fire; next time, run `control-play-mode --action Pause` before setup and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds. If the target line never hit, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", }, { name: "expired without skipped conditional hits", @@ -246,7 +246,7 @@ func TestPausePointWaitHintsDifferentiateHitWhenSkips(t *testing.T) { Status: pausePointStatusExpired, EditorState: pausePointEditorState{IsPlaying: true}, }, - wantHint: "The enable-pause-point --timeout-seconds window (measured from enable, not from this wait) ran out before the marker was hit. If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", + wantHint: "The enable-pause-point --timeout-seconds window (measured from enable, not from this wait) ran out before the marker was hit. If the target line never hit, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", }, } 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 049c4643a5..81dcb79212 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -1556,7 +1556,7 @@ func TestPausePointTimeoutErrorIncludesDiagnosisHint(t *testing.T) { "If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. " + "If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method. " + "If PlayMode kept progressing on its own while you were arranging state (timers, gravity, spawners), the scenario may have already been consumed before this marker could fire; next time, run `control-play-mode --action Pause` before setup and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds. " + - "If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", + "If the target line never hit, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file.", }, } @@ -1605,7 +1605,7 @@ func TestPausePointExpiredErrorIncludesDiagnosisHint(t *testing.T) { HitCount: 0, }, wantHint: "The enable-pause-point --timeout-seconds window (measured from enable, not from this wait) ran out before the marker was hit. " + - "If the target line never hit despite the trigger firing, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", + "If the target line never hit, check the non-firing patterns: (0) the awaited game event never occurred while the marker was armed — check the game state with execute-dynamic-code before suspecting dispatch; (1) the method is a physics/message callback or is called from one on a GameObject that existed before enable — recreate the GameObject or embed UloopPausePoint.Pause; (2) the method was already bound into a delegate/event before enable — the pre-bound invocation path bypasses the patch; (3) the method ran but exited on an earlier branch (for example a guard rejected the action because game state had already moved on) — arm a second marker on the early-return line to see which path ran. (4) the file has active hot-reload patches and the marker resolved against the last compiled source, so the armed line may sit in a different method than the editor shows — check ResolvedMethod, or run 'uloop compile' and re-enable. For patterns (1) and (2), hot-reloading a temporary log line into the method (`uloop hot-reload`) and re-triggering gives a one-way check: the log appearing proves the body ran even though the marker missed. The log staying absent proves nothing — the same cached dispatch can bypass the hot-reload patch too. Note: arming that temporary hot reload itself creates the pattern (4) condition for any later --line in the same file. Once the cause is addressed, re-enable the marker (raise --timeout-seconds only if the window itself was too short) and trigger the code path again.", }, } From 747d58808e146e5259ae5788518ae6d5f5df402a Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:14:08 +0900 Subject: [PATCH 08/13] fix(pause-point): quote the real dispatch failure instead of asserting an argument problem pausePointTriggerRejectionReason fell back to "argument parsing or an unknown command name" whenever the trigger response could not be decoded. That fallback also covered every dispatch failure written to TriggerResult.Error -- a dropped connection, an unreachable Editor, a timeout -- so an EXPIRED or TIMEOUT wait whose trigger died mid-flight was told about an argument problem that never happened, which is the class of invented cause this work removes. The reason now reads the dispatch error first: the error envelope's Message when the stderr text is one, the trimmed raw text otherwise. The fixed text is used only when the failure produced no text at all. Also refresh two doc comments that no longer described the code: the stdout-rejection note now points at pausePointTriggerRejectedByUnityBeforeExecution, and pausePointTriggerFailedNextActions no longer claims only the --trigger value can be wrong. --- .../projectrunner/pause_point_errors.go | 4 +- .../projectrunner/pause_point_errors_test.go | 55 ++++++++++++++++++- .../projectrunner/pause_point_trigger.go | 3 +- .../pause_point_trigger_diagnosis.go | 37 +++++++++++-- 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index 5e48b07331..46d617f8d4 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -125,7 +125,9 @@ func prefixPausePointMessageWithTriggerFailure( } // pausePointTriggerFailedNextActions replaces the generic enable/id-mismatch guidance, which does -// not apply here: the marker was confirmed armed and only the --trigger value is wrong. +// not apply here: the marker was confirmed armed and it is the trigger that never ran. What has to +// change depends on where the rejection came from — the --trigger value for a CLI-side rejection, +// the Editor state for a Unity-side refusal — so the first and third steps branch on it. // // Why re-running the same command comes first: this response answers the command the caller just // ran, so "fix the --trigger value in that command and run it again" asks them to change one value diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go index c1d20f05e9..a0f70df652 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go @@ -768,8 +768,8 @@ func TestPausePointWaitErrorDoesNotPrefixTriggerFailedMessage(t *testing.T) { if strings.Contains(waitErr.Message, "The --trigger command failed") { t.Fatalf("the trigger-failed message must not be prefixed again: %q", waitErr.Message) } - if !strings.HasPrefix(waitErr.Message, "The trigger was rejected before it ran (argument parsing or an unknown command name)") { - t.Fatalf("expected the rejection reason quoted in the message: %q", waitErr.Message) + if !strings.HasPrefix(waitErr.Message, `The trigger was rejected before it ran (Invalid value for --action: "Hold")`) { + t.Fatalf("expected the rejection's own reason quoted in the message: %q", waitErr.Message) } } @@ -785,3 +785,54 @@ func TestPausePointNonFiringPatternsHintLeadsWithEventNeverOccurred(t *testing.T t.Fatalf("pattern (0) must come before pattern (1): %q", pausePointNonFiringPatternsHint) } } + +// Verifies a trigger whose dispatch died mid-flight (a dropped connection, an unreachable Editor) +// is quoted by its own error text, not reported as an argument or command-name problem it never had. +func TestPausePointWaitErrorQuotesDispatchFailureInsteadOfAssertingArgumentParsing(t *testing.T) { + result := &pausePointTriggerResult{ + Completed: true, + Error: `{"Success":false,"Error":{"ErrorCode":"UNITY_NOT_REACHABLE",` + + `"Message":"Unity Editor is not reachable."}}`, + } + + waitErr := pausePointWaitError( + "/project", + waitForPausePointOptions{id: "jump", timeoutSeconds: 10}, + pausePointStatusResponse{Id: "jump", Status: pausePointStatusExpired}, + pausePointWaitStateExpired, + false, + false, + result) + + if strings.Contains(waitErr.Message, "argument parsing") { + t.Fatalf("a dispatch failure must not be reported as an argument problem: %q", waitErr.Message) + } + if !strings.HasPrefix(waitErr.Message, "The --trigger command failed (Unity Editor is not reachable). ") { + t.Fatalf("expected the dispatch failure's own message quoted: %q", waitErr.Message) + } +} + +// Verifies a dispatch failure that produced no error envelope still quotes the raw stderr text it +// did produce, rather than falling back to a cause it cannot know. +func TestPausePointWaitErrorQuotesRawDispatchFailureText(t *testing.T) { + result := &pausePointTriggerResult{ + Completed: true, + Error: "trigger command \"simulate-keyboard\" exited with code 2 and produced no parseable output.", + } + + waitErr := pausePointWaitError( + "/project", + waitForPausePointOptions{id: "jump", timeoutSeconds: 10}, + pausePointStatusResponse{Id: "jump", Status: pausePointStatusEnabled}, + pausePointWaitStateTimeout, + false, + false, + result) + + if strings.Contains(waitErr.Message, "argument parsing") { + t.Fatalf("a dispatch failure must not be reported as an argument problem: %q", waitErr.Message) + } + if !strings.Contains(waitErr.Message, "exited with code 2 and produced no parseable output") { + t.Fatalf("expected the raw dispatch failure text quoted: %q", waitErr.Message) + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger.go b/cli/project-runner/internal/projectrunner/pause_point_trigger.go index fa2115ea53..6582a142fd 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger.go @@ -57,7 +57,8 @@ type pausePointTriggerResult struct { // pre-execution rejection keeps the wait running. // // Only the dispatched command's stderr is inspected, because that is where every error envelope is -// written; a Unity-side rejection arriving on stdout has no error code to match on. +// written. A Unity-side rejection arrives on stdout as an ordinary response carrying no error code +// to match on; pausePointTriggerRejectedByUnityBeforeExecution recognises that shape instead. func pausePointTriggerRejectedBeforeExecution(result *pausePointTriggerResult) bool { if result == nil { return false diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go index 2070224604..a61f9c29ea 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_diagnosis.go @@ -42,19 +42,46 @@ func pausePointTriggerRejectedByUnityBeforeExecution( return response.RejectedByActivePausePointId != awaitedPausePointID } -// pausePointTriggerRejectionReason returns the triggered command's own reason for the rejection, +// pausePointTriggerRejectionReason returns the triggered command's own reason for the failure, // shaped to read inside a parenthesis: a trailing sentence period is dropped so the enclosing -// sentence does not end in "..)." Why a fallback: a stderr-envelope rejection carries no Unity -// response to quote, and its cause is always one of the two shapes -// pausePointTriggerRejectedBeforeExecution matches. +// sentence does not end in "..)." +// +// Why the dispatch error is consulted before any fixed text: Error carries whatever the dispatch +// wrote to stderr, which is every failure shape and not only the two permanent rejections — a +// dropped connection and an unreachable Editor land here too. Naming argument parsing for one of +// those would be exactly the invented cause this diagnosis exists to remove, so the fixed text is +// the last resort, used only when the failure produced no text at all. func pausePointTriggerRejectionReason(result *pausePointTriggerResult) string { response, ok := decodePausePointTriggerResponse(result) if ok && response.Message != "" { - return strings.TrimSuffix(strings.TrimSpace(response.Message), ".") + return trimPausePointReasonForParenthesis(response.Message) + } + if result != nil && strings.TrimSpace(result.Error) != "" { + return trimPausePointReasonForParenthesis(pausePointTriggerDispatchErrorReason(result.Error)) } return "argument parsing or an unknown command name" } +// pausePointTriggerDispatchErrorReason reads the human-readable reason out of a dispatch failure's +// stderr: the error envelope's Message when it is one, and the raw text otherwise, because a +// dispatch that never produced an envelope still wrote the only explanation there is. +func pausePointTriggerDispatchErrorReason(dispatchError string) string { + trimmed := strings.TrimSpace(dispatchError) + envelope := struct { + Error struct { + Message string `json:"Message"` + } `json:"Error"` + }{} + if err := json.Unmarshal([]byte(trimmed), &envelope); err == nil && envelope.Error.Message != "" { + return envelope.Error.Message + } + return trimmed +} + +func trimPausePointReasonForParenthesis(reason string) string { + return strings.TrimSuffix(strings.TrimSpace(reason), ".") +} + // 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 From cdb00a3191a87473360674e4ad5061f0bb1aa9cb Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:17:09 +0900 Subject: [PATCH 09/13] fix(pause-point): write the arming hint's one-call form as a runnable command The arming guidance's last sentence named "enable-pause-point --await --resume-play --trigger ..." without the binary, while the two sentences before it are complete `uloop ...` command lines. Prefix it the same way so the whole hint reads as commands. --- Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs | 2 +- Assets/Tests/Editor/PausePointEnableGuidanceTests.cs | 4 ++-- Assets/Tests/Editor/PausePointTests.cs | 2 +- .../FirstPartyTools/PausePoint/SourcePausePointConstants.cs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs index 475581d539..93634a4033 100644 --- a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs +++ b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs @@ -718,7 +718,7 @@ public void Enable_WhenCompiledLineMatchesEditedFile_UsesMatchedCompiledLineMapW + ResolveFailureFile + ":" + requestedLine - + "\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; + + "\" --timeout-seconds . To arm, trigger, and collect in one call: uloop enable-pause-point --await --resume-play --trigger \"\"."; Assert.That(response.RecommendedNextAction, Is.EqualTo(expectedArming)); AssertLineBasis(response, "LastCompiledSource"); } diff --git a/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs b/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs index 1902b6183e..65287eed75 100644 --- a/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs +++ b/Assets/Tests/Editor/PausePointEnableGuidanceTests.cs @@ -24,7 +24,7 @@ public sealed class PausePointEnableGuidanceTests private const int FixtureClosingBraceLine = 13; private const string ExpectedArmingNextActionForJump = - "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"jump\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; + "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"jump\" --timeout-seconds . To arm, trigger, and collect in one call: uloop enable-pause-point --await --resume-play --trigger \"\"."; private const string ExpectedRearmDiscardWarningGeneration1 = "Generation 1 of this pause point had already hit; this re-arm discarded its CapturedVariables and CapturedVariableHistory. Read results with pause-point-status before re-arming when you need them."; @@ -168,7 +168,7 @@ public void Enable_WhenFileLinePathSucceeds_SetsArmingRecommendedNextAction() + FixtureFilePath + ":" + FixtureStatementLine - + "\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; + + "\" --timeout-seconds . To arm, trigger, and collect in one call: uloop enable-pause-point --await --resume-play --trigger \"\"."; Assert.That(response.RecommendedNextAction, Is.EqualTo(expected)); string json = JsonConvert.SerializeObject( response, diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 1d6a50c11a..18567481df 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -1489,7 +1489,7 @@ public async Task Enable_WhenMarkerCreated_ReturnsStateManagementFields() Assert.That( response.RecommendedNextAction, Is.EqualTo( - "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"jump\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\".")); + "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"jump\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"jump\" --timeout-seconds . To arm, trigger, and collect in one call: uloop enable-pause-point --await --resume-play --trigger \"\".")); } [Test] diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 79d52713db..5715d701e4 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -430,7 +430,7 @@ internal static class SourcePausePointConstants // so agents arm a marker and then stall instead of running the path or using --await. // Format: marker id. public const string EnableSuccessArmingRecommendedNextActionFormat = - "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"{0}\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"{0}\" --timeout-seconds . To arm, trigger, and collect in one call: enable-pause-point --await --resume-play --trigger \"\"."; + "Run the code path so the marker can hit, then read the outcome with: uloop pause-point-status --id \"{0}\". To block until it hits without a trigger command (e.g. waiting for physics or a multi-step action): uloop await-pause-point --id \"{0}\" --timeout-seconds . To arm, trigger, and collect in one call: uloop enable-pause-point --await --resume-play --trigger \"\"."; // Why warn: Registry.Enable replaces the entry and drops CapturedVariables, // CapturedVariableHistory, and hit snapshots. The raw capture holder is kept on purpose. From e326e591ebcbbdd5bc4fe6ad7da996584ce47e19 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:17:19 +0900 Subject: [PATCH 10/13] docs(pause-point): state the one refusal that does not abort the trigger wait The troubleshooting note said a preflight-refused --trigger always aborts the wait, but the wait deliberately keeps running when RejectedByActivePausePointId names the marker being awaited: that refusal means the marker was hit before the trigger ran, which is the wait's success rather than a dead end. Name the exception and regenerate the .claude and .agents copies. --- .agents/skills/uloop-pause-point/references/troubleshooting.md | 2 +- .claude/skills/uloop-pause-point/references/troubleshooting.md | 2 +- .../PausePoint/Skill/references/troubleshooting.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.agents/skills/uloop-pause-point/references/troubleshooting.md b/.agents/skills/uloop-pause-point/references/troubleshooting.md index d41a321933..064a3ec7a1 100644 --- a/.agents/skills/uloop-pause-point/references/troubleshooting.md +++ b/.agents/skills/uloop-pause-point/references/troubleshooting.md @@ -16,7 +16,7 @@ Expired responses include `MethodEntryCount`: `0` means the armed method was nev 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. The failure response names the responsible marker in `RejectedByActivePausePointId`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out. 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`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out — unless `RejectedByActivePausePointId` names the very marker being awaited, which means that marker was hit before the trigger ran; that wait keeps running, because the hit is its success. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## Locating Where Control Flow Stops diff --git a/.claude/skills/uloop-pause-point/references/troubleshooting.md b/.claude/skills/uloop-pause-point/references/troubleshooting.md index d41a321933..064a3ec7a1 100644 --- a/.claude/skills/uloop-pause-point/references/troubleshooting.md +++ b/.claude/skills/uloop-pause-point/references/troubleshooting.md @@ -16,7 +16,7 @@ Expired responses include `MethodEntryCount`: `0` means the armed method was nev 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. The failure response names the responsible marker in `RejectedByActivePausePointId`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out. 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`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out — unless `RejectedByActivePausePointId` names the very marker being awaited, which means that marker was hit before the trigger ran; that wait keeps running, because the hit is its success. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## Locating Where Control Flow Stops diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md index d41a321933..064a3ec7a1 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md @@ -16,7 +16,7 @@ Expired responses include `MethodEntryCount`: `0` means the armed method was nev 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. The failure response names the responsible marker in `RejectedByActivePausePointId`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out. 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`, and sets `RejectedBeforeExecution: true` for any pre-execution refusal. A `--trigger` refused that way aborts the wait right away with `PAUSE_POINT_TRIGGER_FAILED` quoting the refusal, instead of waiting the marker out — unless `RejectedByActivePausePointId` names the very marker being awaited, which means that marker was hit before the trigger ran; that wait keeps running, because the hit is its success. Check `uloop pause-point-status --id ` first to confirm the hit before treating it as a bug in the simulated action itself. ## Locating Where Control Flow Stops From 81f19c543d2daf8eb962ca13c626b963574657f1 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:17:19 +0900 Subject: [PATCH 11/13] test(pause-point): drive the preflight rejection through each tool's own entry point The existing tests called the response factories directly, so reverting a use case to build its rejection response inline would have kept them green. Add tests that call SimulateMouseUiUseCase.ExecuteAsync, SimulateKeyboardUseCase.ExecuteAsync, SimulateMouseInputUseCase.ExecuteAsync and ReplayInputUseCase.ReplayInputAsync with PlayMode stopped -- the preflight rejection an EditMode run produces naturally -- and assert the returned response reports RejectedBeforeExecution. replay-input's ExecuteStart is private, so Start is driven through the public ReplayInputAsync. Each response is read from an already-completed task rather than awaited or blocked on, so a regression fails the test instead of hanging the EditMode suite. --- ...usePointPreflightRejectionResponseTests.cs | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs b/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs index e7b340554d..5a0d28138d 100644 --- a/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs +++ b/Assets/Tests/Editor/PausePointPreflightRejectionResponseTests.cs @@ -1,4 +1,6 @@ #nullable enable +using System.Threading; +using System.Threading.Tasks; using Newtonsoft.Json; using NUnit.Framework; @@ -128,6 +130,72 @@ public void ReplayInputPreflightFailure_WhenPreflightRejected_ReportsRejectionBe } #endif + [Test] + public void MouseUiUseCase_WhenPlayModeIsStopped_ReportsRejectionBeforeExecution() + { + // Verifies the flag survives the real simulate-mouse-ui entry point, not just the factory: + // an EditMode run has PlayMode stopped, which is exactly the preflight rejection the CLI aborts on. + SimulateMouseUiResponse response = RunToCompletion( + new SimulateMouseUiUseCase().ExecuteAsync( + new SimulateMouseUiSchema { Action = UnityCliLoopMouseUiAction.Click }, + CancellationToken.None)); + + Assert.That(response.Success, Is.False); + Assert.That(response.RejectedBeforeExecution, Is.True); + } + +#if ULOOP_HAS_INPUT_SYSTEM + [Test] + public void KeyboardUseCase_WhenPlayModeIsStopped_ReportsRejectionBeforeExecution() + { + // Verifies simulate-keyboard's own entry point wires the rejected preflight to the wire flag. + SimulateKeyboardResponse response = RunToCompletion( + new SimulateKeyboardUseCase().ExecuteAsync( + new SimulateKeyboardSchema { Action = UnityCliLoopKeyboardAction.Press, Key = "Space" }, + CancellationToken.None)); + + Assert.That(response.Success, Is.False); + Assert.That(response.RejectedBeforeExecution, Is.True); + } + + [Test] + public void MouseInputUseCase_WhenPlayModeIsStopped_ReportsRejectionBeforeExecution() + { + // Verifies simulate-mouse-input's own entry point wires the rejected preflight to the wire flag. + SimulateMouseInputResponse response = RunToCompletion( + new SimulateMouseInputUseCase().ExecuteAsync( + new SimulateMouseInputSchema { Action = UnityCliLoopMouseInputAction.Click }, + CancellationToken.None)); + + Assert.That(response.Success, Is.False); + Assert.That(response.RejectedBeforeExecution, Is.True); + } + + [Test] + public void ReplayInputUseCase_WhenPlayModeIsStopped_ReportsRejectionBeforeExecution() + { + // Verifies replay-input's own entry point wires the rejected preflight to the wire flag. + // ExecuteStart is private, so Start is driven through the public ReplayInputAsync. + ReplayInputResponse response = RunToCompletion( + new ReplayInputUseCase().ReplayInputAsync( + new ReplayInputSchema { Action = ReplayInputAction.Start }, + CancellationToken.None)); + + Assert.That(response.Success, Is.False); + Assert.That(response.RejectedBeforeExecution, Is.True); + } +#endif + + // Why not await or block: a preflight rejection is returned before the use case reaches any + // real await, so the task is already complete. Asserting that instead of blocking keeps a + // regression from hanging the EditMode suite. + private static TResponse RunToCompletion(Task execution) + { + Assert.That(execution.IsCompleted, Is.True, + "The use case must reject the preflight before awaiting anything."); + return execution.Result; + } + private static MouseUiSimulationCommand CreateMouseUiCommand() { (MouseUiSimulationCommand? command, string? errorMessage) = From 2cfcbebd2f0ea8546f0c0b4b99514e4697a9939d Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:29:09 +0900 Subject: [PATCH 12/13] fix(pause-point): stop asserting the method never ran when the patch may be bypassed The expired message for a marker whose patch may be bypassed opened with "the armed method was never entered", stating as fact the one thing a zero MethodEntryCount cannot prove in that branch: if the dispatch bypassed the patch, the method can have run without the patch recording anything. Report the measurement instead -- the armed patch recorded no method entry -- and leave the explanations to the rest of the message. --- Assets/Tests/Editor/PausePointTests.cs | 4 ++-- Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 18567481df..2afe045c8c 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -397,7 +397,7 @@ public void GetStatus_WhenInstrumentedMethodNeverRuns_ReportsNeverInvoked() /// reports a bypass warning instead of claiming the method was never invoked. /// [Test] - public void GetStatus_WhenPhysicsDispatchMayBypassAndMethodNeverEntered_ReportsBypassNotNeverInvoked() + public void GetStatus_WhenPhysicsDispatchMayBypassAndNoMethodEntryRecorded_ReportsBypassNotNeverInvoked() { UloopPausePointRegistry.SetMethodEntryInstrumented("jump"); UloopPausePointRegistry.Enable("jump", 1, patchDispatchMayBypass: true); @@ -405,7 +405,7 @@ public void GetStatus_WhenPhysicsDispatchMayBypassAndMethodNeverEntered_ReportsB UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus("jump"); - Assert.That(snapshot.Message, Does.Contain("the armed method was never entered")); + Assert.That(snapshot.Message, Does.Contain("the armed patch recorded no method entry")); Assert.That(snapshot.Message, Does.Not.Contain("The armed method was never invoked.")); Assert.That(snapshot.RecommendedNextAction, Does.Contain("destroy and recreate")); } diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs index 62b18d03b6..1ea79e50ed 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs @@ -403,7 +403,7 @@ private string CreateExpiredMessage(int methodEntryCount, int hitWhenSkippedCoun // awaited event never happened - so that is stated first. if (PatchDispatchMayBypass) { - return "Pause point expired before it was hit and the armed method was never entered. The awaited game event (collision, input, trigger) may simply not have happened during the wait; check the game state with execute-dynamic-code first. Only if the body provably ran, suspect Unity's cached message dispatch bypassing the patch."; + return "Pause point expired before it was hit and the armed patch recorded no method entry. The awaited game event (collision, input, trigger) may simply not have happened during the wait; check the game state with execute-dynamic-code first. Only if the body provably ran, suspect Unity's cached message dispatch bypassing the patch."; } return "Pause point expired before it was hit. The armed method was never invoked."; From 41f0dea5291b9a29181d0845ab75d48f4bdb2044 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 21:29:20 +0900 Subject: [PATCH 13/13] fix(pause-point): stop asking for a corrected trigger command Unity never faulted The second recovery step spelled the await fallback with --trigger "" for every trigger failure. After a Unity-side pre-execution refusal there is no correction to make: the command was well-formed and only the Editor state refused it, so the placeholder named an edit the caller cannot perform. Branch the step on the rejection source like the first and third already do, and reuse the same trigger command once the precondition holds. --- .../projectrunner/pause_point_errors.go | 21 +++++++++++++++---- .../projectrunner/pause_point_errors_test.go | 19 ++++++++++++++--- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index 46d617f8d4..9ddd22b252 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -127,7 +127,7 @@ func prefixPausePointMessageWithTriggerFailure( // pausePointTriggerFailedNextActions replaces the generic enable/id-mismatch guidance, which does // not apply here: the marker was confirmed armed and it is the trigger that never ran. What has to // change depends on where the rejection came from — the --trigger value for a CLI-side rejection, -// the Editor state for a Unity-side refusal — so the first and third steps branch on it. +// the Editor state for a Unity-side refusal — so all three steps branch on it. // // Why re-running the same command comes first: this response answers the command the caller just // ran, so "fix the --trigger value in that command and run it again" asks them to change one value @@ -141,9 +141,7 @@ func prefixPausePointMessageWithTriggerFailure( func pausePointTriggerFailedNextActions(id string, triggerResult *pausePointTriggerResult) []string { return []string{ pausePointTriggerRejectionFirstAction(triggerResult), - fmt.Sprintf( - "The marker is still armed, so you can also wait on it directly: "+ - "uloop await-pause-point --id %q --trigger \"\"", id), + pausePointTriggerRejectionAwaitAction(id, triggerResult), pausePointTriggerRejectionRecoveryAction(triggerResult), } } @@ -160,6 +158,21 @@ func pausePointTriggerRejectionFirstAction(triggerResult *pausePointTriggerResul return "The trigger command itself was valid; Unity refused to run it because of the state named in the trigger message." } +// pausePointTriggerRejectionAwaitAction spells out the await form the caller can fall back on, with +// the placeholder that matches where the rejection came from. Asking for a "corrected" trigger +// command after Unity refused a well-formed one would name a correction that does not exist: the +// command stays as it was and only the Editor state has to change. +func pausePointTriggerRejectionAwaitAction(id string, triggerResult *pausePointTriggerResult) string { + if pausePointTriggerRejectedBeforeExecution(triggerResult) { + return fmt.Sprintf( + "The marker is still armed, so you can also wait on it directly: "+ + "uloop await-pause-point --id %q --trigger \"\"", id) + } + return fmt.Sprintf( + "The marker is still armed, so once the precondition holds you can wait on it directly: "+ + "uloop await-pause-point --id %q --trigger \"\"", id) +} + // pausePointTriggerRejectionRecoveryAction picks the recovery step that matches where the rejection // came from. A Unity-side refusal has nothing to do with argument syntax, so the argument/command-name // advice would send the caller looking for a typo in a value that was already correct. diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go index a0f70df652..a02cf0709c 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors_test.go @@ -649,7 +649,7 @@ func TestPausePointTimeoutError_TriggerRejected_WinsOverNewHitBaseline(t *testin } } -// Verifies a CLI-side rejection keeps both steps that name the --trigger value as what has to +// Verifies a CLI-side rejection keeps every step that names the --trigger value as what has to // change: that is the only cause this shape of rejection can have. func TestPausePointTriggerFailedNextActionsForCliRejection(t *testing.T) { result := &pausePointTriggerResult{ @@ -665,13 +665,17 @@ func TestPausePointTriggerFailedNextActionsForCliRejection(t *testing.T) { if !strings.HasPrefix(actions[0], "Fix the --trigger value in the command you just ran") { t.Fatalf("expected the trigger-value fix as the first step, got %q", actions[0]) } + if !strings.Contains(actions[1], `--trigger ""`) { + t.Fatalf("expected the await step to ask for a corrected trigger command, got %q", actions[1]) + } if !strings.Contains(actions[2], "INVALID_ARGUMENT") { t.Fatalf("expected the argument-syntax recovery step, got %q", actions[2]) } } -// Verifies a Unity-side pre-execution rejection replaces the argument-syntax recovery step with the -// precondition step: nothing about the trigger's arguments was wrong. +// Verifies a Unity-side pre-execution rejection replaces every step that would blame the trigger's +// arguments -- the await form included -- with precondition wording: nothing about the trigger's +// arguments was wrong. func TestPausePointTriggerFailedNextActionsForUnityRejection(t *testing.T) { result := &pausePointTriggerResult{ Completed: true, @@ -690,6 +694,15 @@ func TestPausePointTriggerFailedNextActionsForUnityRejection(t *testing.T) { if !strings.HasPrefix(actions[0], "The trigger command itself was valid;") { t.Fatalf("expected the first step to clear the trigger value, got %q", actions[0]) } + if strings.Contains(actions[1], "corrected trigger command") { + t.Fatalf("a Unity-side rejection has no correction to make to the trigger command, got %q", actions[1]) + } + if !strings.Contains(actions[1], `--trigger ""`) { + t.Fatalf("expected the await step to reuse the same trigger command, got %q", actions[1]) + } + if !strings.Contains(actions[1], "once the precondition holds") { + t.Fatalf("expected the await step to name the precondition, got %q", actions[1]) + } if strings.Contains(actions[2], "INVALID_ARGUMENT") { t.Fatalf("the argument-syntax recovery step must not be used for a Unity-side rejection, got %q", actions[2]) }