From fab55b5f9ae2605465e23885698fe4f7810d1a66 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 20 Aug 2026 15:21:16 +0900 Subject: [PATCH 1/2] feat: explain frame-boundary pausing on hit responses and surface the expired next action Agents treat live reads after a Hit as at-line evidence, but Unity pauses at the next frame boundary. StatusNote on every Hit (not just trace) states that, and Expired errors now surface RecommendedNextAction at the front of NextActions instead of only under Details. Co-authored-by: Cursor --- .agents/skills/uloop-pause-point/SKILL.md | 2 +- .claude/skills/uloop-pause-point/SKILL.md | 2 +- .../CliOnlyTools~/PausePoint/Skill/SKILL.md | 2 +- .../projectrunner/pause_point_enable.go | 2 +- .../projectrunner/pause_point_enable_test.go | 79 +++++++- .../projectrunner/pause_point_errors.go | 22 ++- .../projectrunner/pause_point_types.go | 7 +- .../projectrunner/pause_point_wait.go | 20 ++- .../projectrunner/pause_point_wait_test.go | 169 ++++++++++++++++-- 9 files changed, 268 insertions(+), 37 deletions(-) diff --git a/.agents/skills/uloop-pause-point/SKILL.md b/.agents/skills/uloop-pause-point/SKILL.md index d9f50d4164..c59f9bdd7e 100644 --- a/.agents/skills/uloop-pause-point/SKILL.md +++ b/.agents/skills/uloop-pause-point/SKILL.md @@ -100,7 +100,7 @@ Choose the capture mode when enabling a pause point: - `trace` remains armed and records each hit without pausing Unity. - In every mode, `CapturedVariables` holds the latest hit and `CapturedVariableHistory` holds only strictly older frames, so with a single hit the history is empty (for `single-shot` it always is). When the latest-hit frame is excluded, `CapturedVariableHistoryNote` explains that the latest hit's variables are in `CapturedVariables`. - Prefer tracing a line that executes conditionally: a line that runs every frame fills the capped history within a fraction of a second and drops everything recorded before it. -- In trace mode, Status "Hit" does not mean Play Mode paused; the response carries a StatusNote saying the marker fired while the game kept running. +- On every Hit, the response carries a StatusNote. In trace mode it says Play Mode was not paused (the marker fired while the game kept running). In single-shot and continuous it says Unity pauses at the next frame boundary, so live reads after the hit reflect post-frame state; use CapturedVariables for at-line values. - An Expired response carries a RecommendedNextAction: re-enable the pause point with a longer --timeout-seconds (default 30) and trigger the code path again; clearing the expired marker first is not required. - For an already-hit `continuous` or `trace` marker, `await-pause-point` waits for a **new** hit after the wait starts (`LastHitSequence` advancing). It does not return the stale hit that is already present. Read the current hit with `pause-point-status` instead. If await times out while waiting for that new hit, the error stays `PAUSE_POINT_WAIT_TIMEOUT` and `Details.Hint` tells you to pass `--resume-play` (or resume Play Mode) so another hit can occur. A freshly enabled marker (including `enable-pause-point --await`) has no prior hit, so the first hit satisfies the wait as before. diff --git a/.claude/skills/uloop-pause-point/SKILL.md b/.claude/skills/uloop-pause-point/SKILL.md index d9f50d4164..c59f9bdd7e 100644 --- a/.claude/skills/uloop-pause-point/SKILL.md +++ b/.claude/skills/uloop-pause-point/SKILL.md @@ -100,7 +100,7 @@ Choose the capture mode when enabling a pause point: - `trace` remains armed and records each hit without pausing Unity. - In every mode, `CapturedVariables` holds the latest hit and `CapturedVariableHistory` holds only strictly older frames, so with a single hit the history is empty (for `single-shot` it always is). When the latest-hit frame is excluded, `CapturedVariableHistoryNote` explains that the latest hit's variables are in `CapturedVariables`. - Prefer tracing a line that executes conditionally: a line that runs every frame fills the capped history within a fraction of a second and drops everything recorded before it. -- In trace mode, Status "Hit" does not mean Play Mode paused; the response carries a StatusNote saying the marker fired while the game kept running. +- On every Hit, the response carries a StatusNote. In trace mode it says Play Mode was not paused (the marker fired while the game kept running). In single-shot and continuous it says Unity pauses at the next frame boundary, so live reads after the hit reflect post-frame state; use CapturedVariables for at-line values. - An Expired response carries a RecommendedNextAction: re-enable the pause point with a longer --timeout-seconds (default 30) and trigger the code path again; clearing the expired marker first is not required. - For an already-hit `continuous` or `trace` marker, `await-pause-point` waits for a **new** hit after the wait starts (`LastHitSequence` advancing). It does not return the stale hit that is already present. Read the current hit with `pause-point-status` instead. If await times out while waiting for that new hit, the error stays `PAUSE_POINT_WAIT_TIMEOUT` and `Details.Hint` tells you to pass `--resume-play` (or resume Play Mode) so another hit can occur. A freshly enabled marker (including `enable-pause-point --await`) has no prior hit, so the first hit satisfies the wait as before. diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md index d9f50d4164..c59f9bdd7e 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md @@ -100,7 +100,7 @@ Choose the capture mode when enabling a pause point: - `trace` remains armed and records each hit without pausing Unity. - In every mode, `CapturedVariables` holds the latest hit and `CapturedVariableHistory` holds only strictly older frames, so with a single hit the history is empty (for `single-shot` it always is). When the latest-hit frame is excluded, `CapturedVariableHistoryNote` explains that the latest hit's variables are in `CapturedVariables`. - Prefer tracing a line that executes conditionally: a line that runs every frame fills the capped history within a fraction of a second and drops everything recorded before it. -- In trace mode, Status "Hit" does not mean Play Mode paused; the response carries a StatusNote saying the marker fired while the game kept running. +- On every Hit, the response carries a StatusNote. In trace mode it says Play Mode was not paused (the marker fired while the game kept running). In single-shot and continuous it says Unity pauses at the next frame boundary, so live reads after the hit reflect post-frame state; use CapturedVariables for at-line values. - An Expired response carries a RecommendedNextAction: re-enable the pause point with a longer --timeout-seconds (default 30) and trigger the code path again; clearing the expired marker first is not required. - For an already-hit `continuous` or `trace` marker, `await-pause-point` waits for a **new** hit after the wait starts (`LastHitSequence` advancing). It does not return the stale hit that is already present. Read the current hit with `pause-point-status` instead. If await times out while waiting for that new hit, the error stays `PAUSE_POINT_WAIT_TIMEOUT` and `Details.Hint` tells you to pass `--resume-play` (or resume Play Mode) so another hit can occur. A freshly enabled marker (including `enable-pause-point --await`) has no prior hit, so the first hit satisfies the wait as before. diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 25e943f614..1a99ea4e6a 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -411,7 +411,7 @@ func runPausePointWaitAfterEnable( response.ResolvedMethod = enableFields.ResolvedMethod response.SnapshotTiming = enableFields.SnapshotTiming response = filterPausePointCapturedVariableHistory(response) - response = applyPausePointTraceStatusNote(response) + response = applyPausePointHitStatusNote(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable_test.go b/cli/project-runner/internal/projectrunner/pause_point_enable_test.go index bafd7d19cf..15ef949557 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable_test.go @@ -215,7 +215,7 @@ func TestRunEnablePausePointCommandAwaitsAfterSuccessfulEnable(t *testing.T) { } // Verifies enable-pause-point --await stdout includes StatusNote on a trace-mode Hit. -// Removing applyPausePointTraceStatusNote from the enable-await hit path makes this test Red. +// Removing applyPausePointHitStatusNote from the enable-await hit path makes this test Red. func TestRunEnablePausePointCommandIncludesStatusNoteOnTraceHit(t *testing.T) { originalQuery := queryPausePointStatus originalPoll := pausePointStatusPoll @@ -289,6 +289,83 @@ func TestRunEnablePausePointCommandIncludesStatusNoteOnTraceHit(t *testing.T) { assertStdoutHasPausePointTraceStatusNote(t, stdout.Bytes()) } +// Verifies enable-pause-point --await stdout includes the frame-boundary StatusNote +// on a non-trace Hit. Removing applyPausePointHitStatusNote from the enable-await +// hit path makes this test Red. +func TestRunEnablePausePointCommandIncludesStatusNoteOnSingleShotHit(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + originalFetch := fetchMatchingLogs + pausePointStatusPoll = time.Millisecond + t.Cleanup(func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + fetchMatchingLogs = originalFetch + }) + + statusResponses := []pausePointStatusResponse{ + {Id: "jump", Status: pausePointStatusEnabled, IsEnabled: true}, + { + Id: "jump", + Status: pausePointStatusHit, + Mode: "single-shot", + IsEnabled: true, + IsHit: true, + HitCount: 1, + }, + } + statusCallCount := 0 + queryPausePointStatus = func(ctx context.Context, connection unityipc.Connection, id string) (pausePointStatusResponse, error) { + response := statusResponses[statusCallCount] + statusCallCount++ + return response, nil + } + fetchMatchingLogs = func( + ctx context.Context, + connection unityipc.Connection, + searchText string, + maxCount int, + ) (pausePointMatchingLogsResult, error) { + return pausePointMatchingLogsResult{SearchText: searchText, Logs: []pausePointMatchingLog{}}, nil + } + + listener := newLoopbackIpcListener(t) + enableRequests := make(chan map[string]any, 1) + serverErr := make(chan error, 1) + go serveSingleIPCResponse( + listener, + pausePointEnableCommandName, + enableRequests, + serverErr, + `{"Success":true,"Id":"jump","Status":"Enabled","IsEnabled":true,"TimeoutSeconds":30}`, + ) + + connection := unityipc.Connection{ + Endpoint: unityipc.Endpoint{ + Network: listener.Addr().Network(), + Address: listener.Addr().String(), + }, + ProjectRoot: t.TempDir(), + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runEnablePausePointCommand( + context.Background(), + connection, + []string{"--id", "jump", "--await"}, + t.TempDir(), + &stdout, + &stderr) + + if code != 0 { + t.Fatalf("expected success, got %d with stderr %s", code, stderr.String()) + } + + assertStdoutHasPausePointStatusNote(t, stdout.Bytes(), + "Unity pauses at the next frame boundary; the rest of the hit frame already ran. Read at-line values from CapturedVariables; live reads via execute-dynamic-code reflect post-frame state.") +} + // Verifies file:line enable --await copies ResolvedLine / ResolvedLineText / ResolvedMethod / // SnapshotTiming from the enable response into the await hit payload. func TestRunEnablePausePointCommandAwaitPropagatesFileLineResolvedFields(t *testing.T) { diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index a90beb2b87..7301763a53 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -221,16 +221,24 @@ func pausePointStateError( SafeToRetry: retryable, ProjectRoot: projectRoot, Command: clicore.PausePointAwaitCommandName, - NextActions: []string{ - "Run `uloop enable-pause-point --id ` before waiting.", - "Confirm the code path calls `UloopPausePoint.Pause(\"\")` with the same id.", - "Check `Details.Status`, `Details.EditorState`, `Details.ElapsedSinceEnabledMilliseconds`, and `Details.RemainingMilliseconds` to distinguish a missed code path from an already-paused Editor.", - "If the marker is inside a custom asmdef, add a reference to `UnityCLILoop.PausePoints.Runtime`.", - }, - Details: pausePointStateErrorDetails(options, response), + NextActions: pausePointStateNextActions(response), + Details: pausePointStateErrorDetails(options, response), } } +func pausePointStateNextActions(response pausePointStatusResponse) []string { + nextActions := []string{ + "Run `uloop enable-pause-point --id ` before waiting.", + "Confirm the code path calls `UloopPausePoint.Pause(\"\")` with the same id.", + "Check `Details.Status`, `Details.EditorState`, `Details.ElapsedSinceEnabledMilliseconds`, and `Details.RemainingMilliseconds` to distinguish a missed code path from an already-paused Editor.", + "If the marker is inside a custom asmdef, add a reference to `UnityCLILoop.PausePoints.Runtime`.", + } + if response.RecommendedNextAction == "" { + return nextActions + } + return append([]string{response.RecommendedNextAction}, nextActions...) +} + func pausePointStateErrorDetails( options waitForPausePointOptions, response pausePointStatusResponse, diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index c80ac9b06a..10b7c9fc2b 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -73,9 +73,10 @@ type pausePointStatusResponse struct { // carries that hit. omitempty keeps the field off 0-hit and unfiltered responses. CapturedVariableHistoryNote string `json:"CapturedVariableHistoryNote,omitempty"` - // StatusNote is set by the CLI, not Unity, when Mode is trace and Status is Hit. - // omitempty keeps the field off every other mode and status so the shared status - // contract fixture stays unchanged. + // StatusNote is set by the CLI, not Unity, when Status is Hit. Trace mode explains + // that Play Mode was not paused; other modes explain the frame-boundary pause. + // omitempty keeps the field off non-Hit statuses so the shared status contract + // fixture stays unchanged. StatusNote string `json:"StatusNote,omitempty"` // TriggerResult is set by the CLI, not Unity, only when --trigger was passed. It is omitted diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index f6bbef0312..bc08b1a1c7 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -35,6 +35,10 @@ const ( // pausePointTraceStatusNote explains that a trace-mode Hit did not pause Play Mode. pausePointTraceStatusNote = "Trace mode does not pause Play Mode; Status 'Hit' records that the marker fired while the game kept running." + // Why: a non-trace Hit pauses at the next frame boundary, so live reads after the + // pause are already post-frame; agents otherwise treat them as at-line evidence. + pausePointHitFrameBoundaryStatusNote = "Unity pauses at the next frame boundary; the rest of the hit frame already ran. Read at-line values from CapturedVariables; live reads via execute-dynamic-code reflect post-frame state." + // Mode strings mirror UloopPausePointCaptureMode on the Unity side. Await uses an allowlist // (continuous/trace) for the new-hit baseline — never `Mode != "single-shot"` — so an empty // Mode from an older package keeps the historical immediate-Hit success path. @@ -128,11 +132,17 @@ func filterPausePointCapturedVariableHistory(response pausePointStatusResponse) return response } -// applyPausePointTraceStatusNote records that a trace-mode Hit did not pause Play Mode. -func applyPausePointTraceStatusNote(response pausePointStatusResponse) pausePointStatusResponse { - if response.Mode == pausePointModeTrace && response.Status == pausePointStatusHit { +// applyPausePointHitStatusNote records mode-specific Hit guidance: trace did not +// pause Play Mode; other modes paused at the next frame boundary. +func applyPausePointHitStatusNote(response pausePointStatusResponse) pausePointStatusResponse { + if response.Status != pausePointStatusHit { + return response + } + if response.Mode == pausePointModeTrace { response.StatusNote = pausePointTraceStatusNote + return response } + response.StatusNote = pausePointHitFrameBoundaryStatusNote return response } @@ -204,7 +214,7 @@ func runPausePointStatusCommand( } response = normalizePausePointStatusResponse(response) response = filterPausePointCapturedVariableHistory(response) - response = applyPausePointTraceStatusNote(response) + response = applyPausePointHitStatusNote(response) // Evaluated against the raw CapturedVariables, before the filters below can narrow or strip // values, for the same reason as on the await path (runWaitForPausePoint): otherwise an --expect // target not also requested via --captured-variable-names, or whose value names mode stripped, @@ -260,7 +270,7 @@ func runWaitForPausePoint( response.TriggerResult = triggerResult response.ResumePlayResult = resumeResult response = filterPausePointCapturedVariableHistory(response) - response = applyPausePointTraceStatusNote(response) + response = applyPausePointHitStatusNote(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) // Best-effort: a hit must stay a success even if Unity is busy while paused. 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 a0c4ac3536..4be6775e7f 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -444,6 +444,58 @@ func TestPausePointExpiredErrorReportsRecoveryFields(t *testing.T) { } } +// Verifies an Expired error prepends Unity's RecommendedNextAction onto NextActions +// so the recovery hint is visible without opening Details. +func TestPausePointExpiredErrorPrependsRecommendedNextAction(t *testing.T) { + const expiredNextAction = "Re-enable the marker with a longer --timeout-seconds and trigger the code path again; clearing the expired marker first is not required." + response := pausePointStatusResponse{ + Id: "jump", + Status: pausePointStatusExpired, + Expired: true, + EditorState: pausePointEditorState{IsPlaying: true, CapturedAt: "Current"}, + Message: "Pause point expired before it was hit.", + RecommendedNextAction: expiredNextAction, + } + + cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + }, response, pausePointWaitStateExpired, false, false) + + if len(cliErr.NextActions) == 0 { + t.Fatal("NextActions must not be empty") + } + if cliErr.NextActions[0] != expiredNextAction { + t.Fatalf("NextActions[0] mismatch: got %#v, want %#v", + cliErr.NextActions[0], expiredNextAction) + } +} + +// Verifies an empty RecommendedNextAction does not prepend a blank NextActions entry. +func TestPausePointExpiredErrorOmitsEmptyRecommendedNextAction(t *testing.T) { + response := pausePointStatusResponse{ + Id: "jump", + Status: pausePointStatusExpired, + Expired: true, + EditorState: pausePointEditorState{IsPlaying: true, CapturedAt: "Current"}, + Message: "Pause point expired before it was hit.", + } + + cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + }, response, pausePointWaitStateExpired, false, false) + + wantFirst := "Run `uloop enable-pause-point --id ` before waiting." + if len(cliErr.NextActions) == 0 { + t.Fatal("NextActions must not be empty") + } + if cliErr.NextActions[0] != wantFirst { + t.Fatalf("NextActions[0] mismatch: got %#v, want %#v", + cliErr.NextActions[0], wantFirst) + } +} + // Verifies recovery details use the marker lifetime instead of the wait deadline. func TestPausePointExpiredErrorReportsMarkerTimeoutSeconds(t *testing.T) { response := pausePointStatusResponse{ @@ -1125,9 +1177,11 @@ func TestPausePointStatusResponseOmitsEmptyCapturedVariableHistoryNote(t *testin } } -// Verifies StatusNote is set only for a trace-mode Hit: other modes and statuses stay empty +// Verifies StatusNote is set for every Hit: trace keeps the no-pause wording, +// and non-trace modes get the frame-boundary wording. Non-Hit statuses stay empty // so omitempty keeps the historical JSON shape. -func TestApplyPausePointTraceStatusNote(t *testing.T) { +func TestApplyPausePointHitStatusNote(t *testing.T) { + const frameBoundaryNote = "Unity pauses at the next frame boundary; the rest of the hit frame already ran. Read at-line values from CapturedVariables; live reads via execute-dynamic-code reflect post-frame state." cases := []struct { name string mode string @@ -1141,10 +1195,22 @@ func TestApplyPausePointTraceStatusNote(t *testing.T) { wantNote: pausePointTraceStatusNote, }, { - name: "continuous hit omits note", + name: "continuous hit sets frame-boundary note", mode: pausePointModeContinuous, status: pausePointStatusHit, - wantNote: "", + wantNote: frameBoundaryNote, + }, + { + name: "single-shot hit sets frame-boundary note", + mode: "single-shot", + status: pausePointStatusHit, + wantNote: frameBoundaryNote, + }, + { + name: "empty mode hit sets frame-boundary note", + mode: "", + status: pausePointStatusHit, + wantNote: frameBoundaryNote, }, { name: "trace enabled omits note", @@ -1162,7 +1228,7 @@ func TestApplyPausePointTraceStatusNote(t *testing.T) { for _, testCase := range cases { t.Run(testCase.name, func(t *testing.T) { - response := applyPausePointTraceStatusNote(pausePointStatusResponse{ + response := applyPausePointHitStatusNote(pausePointStatusResponse{ Mode: testCase.mode, Status: testCase.status, }) @@ -1202,8 +1268,8 @@ func TestPausePointStatusResponseIncludesStatusNote(t *testing.T) { } } -// Verifies an empty StatusNote is omitted from JSON so non-trace and non-Hit -// responses keep the historical shape. +// Verifies an empty StatusNote is omitted from JSON so non-Hit responses keep +// the historical shape. func TestPausePointStatusResponseOmitsEmptyStatusNote(t *testing.T) { marshaled, err := json.Marshal(pausePointStatusResponse{ Mode: pausePointModeTrace, @@ -1643,7 +1709,7 @@ func TestRunPausePointStatusOmitsCapturedVariableHistoryNoteOnZeroHit(t *testing } // Verifies pause-point-status stdout includes StatusNote when Unity reports a -// trace-mode Hit. Removing applyPausePointTraceStatusNote from the status +// trace-mode Hit. Removing applyPausePointHitStatusNote from the status // command path makes this test Red. func TestRunPausePointStatusIncludesStatusNoteOnTraceHit(t *testing.T) { originalQuery := queryPausePointStatus @@ -1682,8 +1748,10 @@ func TestRunPausePointStatusIncludesStatusNoteOnTraceHit(t *testing.T) { assertStdoutHasPausePointTraceStatusNote(t, stdout.Bytes()) } -// Verifies pause-point-status stdout omits StatusNote on a non-trace Hit. -func TestRunPausePointStatusOmitsStatusNoteOnContinuousHit(t *testing.T) { +// Verifies pause-point-status stdout includes the frame-boundary StatusNote on a +// non-trace Hit. Removing applyPausePointHitStatusNote from the status command +// path makes this test Red. +func TestRunPausePointStatusIncludesStatusNoteOnSingleShotHit(t *testing.T) { originalQuery := queryPausePointStatus defer func() { queryPausePointStatus = originalQuery @@ -1697,7 +1765,7 @@ func TestRunPausePointStatusOmitsStatusNoteOnContinuousHit(t *testing.T) { return pausePointStatusResponse{ Id: id, Status: pausePointStatusHit, - Mode: pausePointModeContinuous, + Mode: "single-shot", IsEnabled: true, IsHit: true, HitCount: 1, @@ -1717,13 +1785,12 @@ func TestRunPausePointStatusOmitsStatusNoteOnContinuousHit(t *testing.T) { t.Fatalf("expected success, got %d with stderr %s", code, stderr.String()) } - if strings.Contains(stdout.String(), "StatusNote") { - t.Fatalf("continuous Hit status JSON must omit StatusNote: %s", stdout.String()) - } + assertStdoutHasPausePointStatusNote(t, stdout.Bytes(), + "Unity pauses at the next frame boundary; the rest of the hit frame already ran. Read at-line values from CapturedVariables; live reads via execute-dynamic-code reflect post-frame state.") } // Verifies await-pause-point stdout includes StatusNote on a trace-mode Hit. -// Removing applyPausePointTraceStatusNote from the wait hit path makes this test Red. +// Removing applyPausePointHitStatusNote from the wait hit path makes this test Red. func TestRunWaitForPausePointCommandIncludesStatusNoteOnTraceHit(t *testing.T) { originalExtend := extendPausePointExpiry originalQuery := queryPausePointStatus @@ -1783,8 +1850,76 @@ func TestRunWaitForPausePointCommandIncludesStatusNoteOnTraceHit(t *testing.T) { assertStdoutHasPausePointTraceStatusNote(t, stdout.Bytes()) } +// Verifies await-pause-point stdout includes the frame-boundary StatusNote on a +// non-trace Hit. Removing applyPausePointHitStatusNote from the wait hit path +// makes this test Red. +func TestRunWaitForPausePointCommandIncludesStatusNoteOnSingleShotHit(t *testing.T) { + originalExtend := extendPausePointExpiry + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + t.Cleanup(func() { + extendPausePointExpiry = originalExtend + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }) + + extendPausePointExpiry = func( + ctx context.Context, + connection unityipc.Connection, + id string, + minimumRemainingSeconds int, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Id: id, Status: pausePointStatusEnabled}, nil + } + + statusResponses := []pausePointStatusResponse{ + {Id: "jump", Status: pausePointStatusEnabled, IsEnabled: true}, + { + Id: "jump", + Status: pausePointStatusHit, + Mode: "single-shot", + IsEnabled: true, + IsHit: true, + HitCount: 1, + }, + } + statusCallCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + response := statusResponses[statusCallCount] + statusCallCount++ + return response, nil + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runWaitForPausePointCommand( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + []string{"--id", "jump", "--timeout-seconds", "1"}, + "", + &stdout, + &stderr) + + if code != 0 { + t.Fatalf("expected success, got %d with stderr %s", code, stderr.String()) + } + + assertStdoutHasPausePointStatusNote(t, stdout.Bytes(), + "Unity pauses at the next frame boundary; the rest of the hit frame already ran. Read at-line values from CapturedVariables; live reads via execute-dynamic-code reflect post-frame state.") +} + func assertStdoutHasPausePointTraceStatusNote(t *testing.T, stdout []byte) { t.Helper() + assertStdoutHasPausePointStatusNote(t, stdout, pausePointTraceStatusNote) +} + +func assertStdoutHasPausePointStatusNote(t *testing.T, stdout []byte, wantNote string) { + t.Helper() var decoded map[string]json.RawMessage if err := json.Unmarshal(stdout, &decoded); err != nil { @@ -1800,9 +1935,9 @@ func assertStdoutHasPausePointTraceStatusNote(t *testing.T, stdout []byte) { if err := json.Unmarshal(rawNote, ¬e); err != nil { t.Fatalf("unmarshal note failed: %v", err) } - if note != pausePointTraceStatusNote { + if note != wantNote { t.Fatalf("StatusNote mismatch: got %#v, want %#v", - note, pausePointTraceStatusNote) + note, wantNote) } } From 1c04be019ce022d6ca4f29f4c3b09be54bc4c766 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 20 Aug 2026 15:30:13 +0900 Subject: [PATCH 2/2] test: pin StatusNote and Expired NextActions to independent literals Production constants as expected values cannot detect wording drift, and checking only the first NextActions entry would accept dropping the four recovery steps. Co-authored-by: Cursor --- .../projectrunner/pause_point_wait_test.go | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) 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 4be6775e7f..9366eb1c21 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -462,12 +462,16 @@ func TestPausePointExpiredErrorPrependsRecommendedNextAction(t *testing.T) { timeoutSeconds: 1, }, response, pausePointWaitStateExpired, false, false) - if len(cliErr.NextActions) == 0 { - t.Fatal("NextActions must not be empty") + wantNextActions := []string{ + expiredNextAction, + "Run `uloop enable-pause-point --id ` before waiting.", + "Confirm the code path calls `UloopPausePoint.Pause(\"\")` with the same id.", + "Check `Details.Status`, `Details.EditorState`, `Details.ElapsedSinceEnabledMilliseconds`, and `Details.RemainingMilliseconds` to distinguish a missed code path from an already-paused Editor.", + "If the marker is inside a custom asmdef, add a reference to `UnityCLILoop.PausePoints.Runtime`.", } - if cliErr.NextActions[0] != expiredNextAction { - t.Fatalf("NextActions[0] mismatch: got %#v, want %#v", - cliErr.NextActions[0], expiredNextAction) + if !reflect.DeepEqual(cliErr.NextActions, wantNextActions) { + t.Fatalf("NextActions mismatch:\n got: %#v\nwant: %#v", + cliErr.NextActions, wantNextActions) } } @@ -486,13 +490,15 @@ func TestPausePointExpiredErrorOmitsEmptyRecommendedNextAction(t *testing.T) { timeoutSeconds: 1, }, response, pausePointWaitStateExpired, false, false) - wantFirst := "Run `uloop enable-pause-point --id ` before waiting." - if len(cliErr.NextActions) == 0 { - t.Fatal("NextActions must not be empty") + wantNextActions := []string{ + "Run `uloop enable-pause-point --id ` before waiting.", + "Confirm the code path calls `UloopPausePoint.Pause(\"\")` with the same id.", + "Check `Details.Status`, `Details.EditorState`, `Details.ElapsedSinceEnabledMilliseconds`, and `Details.RemainingMilliseconds` to distinguish a missed code path from an already-paused Editor.", + "If the marker is inside a custom asmdef, add a reference to `UnityCLILoop.PausePoints.Runtime`.", } - if cliErr.NextActions[0] != wantFirst { - t.Fatalf("NextActions[0] mismatch: got %#v, want %#v", - cliErr.NextActions[0], wantFirst) + if !reflect.DeepEqual(cliErr.NextActions, wantNextActions) { + t.Fatalf("NextActions mismatch:\n got: %#v\nwant: %#v", + cliErr.NextActions, wantNextActions) } } @@ -1182,6 +1188,7 @@ func TestPausePointStatusResponseOmitsEmptyCapturedVariableHistoryNote(t *testin // so omitempty keeps the historical JSON shape. func TestApplyPausePointHitStatusNote(t *testing.T) { const frameBoundaryNote = "Unity pauses at the next frame boundary; the rest of the hit frame already ran. Read at-line values from CapturedVariables; live reads via execute-dynamic-code reflect post-frame state." + const traceNote = "Trace mode does not pause Play Mode; Status 'Hit' records that the marker fired while the game kept running." cases := []struct { name string mode string @@ -1192,7 +1199,7 @@ func TestApplyPausePointHitStatusNote(t *testing.T) { name: "trace hit sets note", mode: pausePointModeTrace, status: pausePointStatusHit, - wantNote: pausePointTraceStatusNote, + wantNote: traceNote, }, { name: "continuous hit sets frame-boundary note", @@ -1915,7 +1922,8 @@ func TestRunWaitForPausePointCommandIncludesStatusNoteOnSingleShotHit(t *testing func assertStdoutHasPausePointTraceStatusNote(t *testing.T, stdout []byte) { t.Helper() - assertStdoutHasPausePointStatusNote(t, stdout, pausePointTraceStatusNote) + assertStdoutHasPausePointStatusNote(t, stdout, + "Trace mode does not pause Play Mode; Status 'Hit' records that the marker fired while the game kept running.") } func assertStdoutHasPausePointStatusNote(t *testing.T, stdout []byte, wantNote string) {