From 4295bff7747850ff104786c90c85acdedd6f675b Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 29 Jul 2026 11:42:46 +0900 Subject: [PATCH 1/3] fix: make await-pause-point wait for a new hit on continuous markers Already-hit continuous/trace markers stay Status=Hit, so a second await was returning the stale snapshot. Baseline LastHitSequence at wait start and require a later sequence; keep single-shot and empty Mode immediate. Co-authored-by: Cursor --- .../pause_point_await_new_hit_test.go | 211 ++++++++++++++++++ .../projectrunner/pause_point_enable.go | 4 +- .../projectrunner/pause_point_errors.go | 14 +- .../pause_point_resume_play_test.go | 12 +- .../projectrunner/pause_point_trigger_test.go | 6 +- .../projectrunner/pause_point_wait.go | 10 +- .../projectrunner/pause_point_wait_poll.go | 130 ++++++++--- .../pause_point_wait_poll_test.go | 12 +- .../projectrunner/pause_point_wait_test.go | 20 +- 9 files changed, 355 insertions(+), 64 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go diff --git a/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go b/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go new file mode 100644 index 0000000000..6750f5c039 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go @@ -0,0 +1,211 @@ +package projectrunner + +import ( + "bytes" + "context" + "strings" + "testing" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// Verifies await on an already-hit continuous marker ignores the baseline snapshot and returns +// only after LastHitSequence advances. +func TestWaitForPausePointWaitsForNewHitOnAlreadyHitContinuousMarker(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + sequence := 5 + if queryCount >= 2 { + sequence = 6 + } + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: sequence, + Mode: pausePointModeContinuous, + LastHitSequence: sequence, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + response, state, _, _, hasNewHitBaseline, err := waitForPausePoint( + context.Background(), + unityipc.Connection{}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + }, + ) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateHit { + t.Fatalf("state mismatch: %s", state) + } + if !hasNewHitBaseline { + t.Fatal("expected a new-hit baseline for an already-hit continuous marker") + } + if response.LastHitSequence != 6 { + t.Fatalf("expected the advanced hit sequence, got %#v", response) + } + if queryCount < 2 { + t.Fatalf("expected at least two status polls, got %d", queryCount) + } +} + +// Verifies await on an already-hit continuous marker times out with PAUSE_POINT_WAIT_TIMEOUT and +// the already-hit baseline hint when LastHitSequence never advances. +func TestWaitForPausePointTimesOutWaitingForNewHitOnContinuousMarker(t *testing.T) { + originalQuery := queryPausePointStatus + originalClear := clearPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + clearPausePointStatus = originalClear + pausePointStatusPoll = originalPoll + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 5, + Mode: pausePointModeContinuous, + LastHitSequence: 5, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + clearPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Id: id, Status: pausePointStatusCleared}, nil + } + + stderr := &bytes.Buffer{} + exitCode := runWaitForPausePoint( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: 40 * time.Millisecond, + }, + &bytes.Buffer{}, + stderr, + ) + if exitCode != 1 { + t.Fatalf("exit code mismatch: %d", exitCode) + } + stderrText := stderr.String() + if !strings.Contains(stderrText, clierrors.ErrorCodePausePointWaitTimeout) { + t.Fatalf("expected %s, got stderr: %s", clierrors.ErrorCodePausePointWaitTimeout, stderrText) + } + if !strings.Contains(stderrText, pausePointHintAlreadyHitWaitingForNew) { + t.Fatalf("expected already-hit baseline hint, got stderr: %s", stderrText) + } + if strings.Contains(stderrText, clierrors.ErrorCodePausePointExpired) { + t.Fatalf("timeout must not be reclassified as expired: %s", stderrText) + } +} + +// Verifies an already-hit single-shot marker still returns immediately (baseline not applied). +func TestWaitForPausePointReturnsImmediatelyForAlreadyHitSingleShotMarker(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Hour + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + Mode: "single-shot", + LastHitSequence: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + response, state, _, _, hasNewHitBaseline, err := waitForPausePoint( + context.Background(), + unityipc.Connection{}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + }, + ) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateHit { + t.Fatalf("state mismatch: %s", state) + } + if hasNewHitBaseline { + t.Fatal("single-shot must not establish a new-hit baseline") + } + if response.LastHitSequence != 1 { + t.Fatalf("response mismatch: %#v", response) + } + if queryCount != 1 { + t.Fatalf("expected a single status query, got %d", queryCount) + } +} + +// Verifies the timeout hint for an already-hit baseline is distinct from the generic paused hint. +func TestPausePointTimeoutHintForNewHitBaseline(t *testing.T) { + response := pausePointStatusResponse{ + Id: "jump", + Status: pausePointStatusHit, + Mode: pausePointModeContinuous, + LastHitSequence: 5, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + HitCount: 5, + } + cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + }, response, pausePointWaitStateTimeout, true) + + if cliErr.ErrorCode != clierrors.ErrorCodePausePointWaitTimeout { + t.Fatalf("error code mismatch: %s", cliErr.ErrorCode) + } + if cliErr.Details["Hint"] != pausePointHintAlreadyHitWaitingForNew { + t.Fatalf("hint mismatch: %#v", cliErr.Details) + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 50c78da18d..0516ab09ca 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -382,7 +382,7 @@ func runPausePointWaitAfterEnable( stderr io.Writer, ) int { spinner := clicore.NewToolSpinner(stderr, pausePointEnableCommandName) - response, state, triggerResult, resumeResult, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, resumeResult, hasNewHitBaseline, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -436,7 +436,7 @@ func runPausePointWaitAfterEnable( clearPausePointAfterWaitTimeout(ctx, connection, options.id) } - waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state) + waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) waitErr.Command = pausePointEnableCommandName if enableFields.Warning != "" { waitErr.Details["EnableWarning"] = enableFields.Warning diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index a1484486ca..0905647c0c 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -14,6 +14,7 @@ func pausePointWaitError( options waitForPausePointOptions, response pausePointStatusResponse, state pausePointWaitState, + hasNewHitBaseline bool, ) clierrors.CLIError { response = normalizePausePointStatusResponse(response) @@ -73,7 +74,7 @@ func pausePointWaitError( options, response, true) - hint := pausePointTimeoutHint(response) + hint := pausePointTimeoutHint(response, hasNewHitBaseline) if hint != "" { timeoutError.Details["Hint"] = hint } @@ -110,6 +111,12 @@ 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." + // Returned when await timed out while waiting for a new hit on an already-hit continuous/trace + // marker. Why not reuse pausePointHintEditorAlreadyPaused: that hint diagnoses a marker that + // never fired, whereas here the marker already hit and the wait needs Play resumed so a later + // sequence can occur. + pausePointHintAlreadyHitWaitingForNew = "The marker had already hit and Unity may still be paused by that hit; pass --resume-play or resume Play Mode so a new hit can occur." + // Shared by both pausePointTimeoutHint and pausePointExpiredHint: patterns where the method // body genuinely ran (or was invoked) yet the marker never fired — a physics/message callback // missing a pre-existing GameObject, a pre-bound delegate bypassing the patch, or control flow @@ -123,7 +130,10 @@ const ( // pausePointTimeoutHint maps the final probed status to a deterministic diagnosis, // because timeouts are where agents struggle to tell a missed code path from Editor state. -func pausePointTimeoutHint(response pausePointStatusResponse) string { +func pausePointTimeoutHint(response pausePointStatusResponse, hasNewHitBaseline bool) string { + if hasNewHitBaseline { + return pausePointHintAlreadyHitWaitingForNew + } if !response.EditorState.IsPlaying { return pausePointHintPlayModeNotRunning } diff --git a/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go b/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go index acc5bd08fd..6b3a8af175 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go @@ -132,7 +132,7 @@ func TestWaitForPausePointResumesPlayBeforeTriggerWhenPaused(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -203,7 +203,7 @@ func TestWaitForPausePointSkipsPlayWhenAlreadyUnpaused(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -276,7 +276,7 @@ func TestWaitForPausePointSkipsTriggerWhenResumePlayFails(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -328,7 +328,7 @@ func TestWaitForPausePointSkipsResumeWhenNotArmed(t *testing.T) { return pausePointResumePlayResult{WasPaused: true, Resumed: true} } - _, state, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "does-not-exist", timeoutSeconds: 1, timeout: time.Second, @@ -395,7 +395,7 @@ func TestWaitForPausePointSkipsResumeAndTriggerWhenNotArmed(t *testing.T) { return 0 } - _, state, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "does-not-exist", timeoutSeconds: 1, timeout: time.Second, @@ -469,7 +469,7 @@ func TestWaitForPausePointResumesWithoutTriggerWhenArmed(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go index 0a6e48257a..dbcedc62a6 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go @@ -271,7 +271,7 @@ func TestWaitForPausePointJoinsTriggerResult(t *testing.T) { return 0 } - _, _, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -307,7 +307,7 @@ func TestWaitForPausePointJoinsTriggerResult(t *testing.T) { return 0 } - _, _, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -362,7 +362,7 @@ func TestWaitForPausePointSkipsTriggerWhenNotArmed(t *testing.T) { return 0 } - _, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "does-not-exist", timeoutSeconds: 1, timeout: time.Second, diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index e8f29efc75..575f1ab073 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -26,6 +26,12 @@ const ( pausePointStatusNotEnabled = "NotEnabled" pausePointStatusExpired = "Expired" pausePointStatusCleared = "Cleared" + + // 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. + pausePointModeContinuous = "continuous" + pausePointModeTrace = "trace" ) var ( @@ -208,7 +214,7 @@ func runWaitForPausePoint( ) int { startedAt := time.Now() spinner := clicore.NewToolSpinner(stderr, clicore.PausePointAwaitCommandName) - response, state, triggerResult, resumeResult, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, resumeResult, hasNewHitBaseline, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -257,7 +263,7 @@ func runWaitForPausePoint( clearPausePointAfterWaitTimeout(ctx, connection, options.id) } - waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state) + waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) if triggerResult != nil { waitErr.Details["TriggerResult"] = triggerResult } 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 e9da76523e..006a978d84 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go @@ -38,14 +38,19 @@ const ( // on it. Once confirmed armed (or already hit), optional resume runs synchronously, then the // trigger races the status poll loop and is joined once the wait itself settles, so a slow trigger // cannot delay reporting a pause-point hit. +// +// hasNewHitBaseline is true when the wait started against an already-hit continuous/trace marker +// and must observe a later LastHitSequence before succeeding. Callers pass it into timeout error +// construction so the hint can explain why a still-Hit marker did not count as a wait success. func waitForPausePoint( ctx context.Context, connection unityipc.Connection, options waitForPausePointOptions, -) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, *pausePointResumePlayResult, error) { - triggerHandle, skippedTriggerResult, resumeResult := startPausePointWaitSideEffects(ctx, connection, options) +) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, *pausePointResumePlayResult, bool, error) { + triggerHandle, skippedTriggerResult, resumeResult, baselineSequence, hasBaseline, baselineDecided := startPausePointWaitSideEffects(ctx, connection, options) - response, state, polledTriggerResult, err := waitForPausePointStatus(ctx, connection, options, triggerHandle) + response, state, polledTriggerResult, hasBaseline, err := waitForPausePointStatus( + ctx, connection, options, triggerHandle, baselineSequence, hasBaseline, baselineDecided) if state == pausePointWaitStateTriggerFailed && resumeResult != nil && resumeResult.Resumed { repaused := repausePlayModeAfterAbandonedWait(ctx, connection, *resumeResult) @@ -53,32 +58,36 @@ func waitForPausePoint( } if skippedTriggerResult != nil { - return response, state, skippedTriggerResult, resumeResult, err + return response, state, skippedTriggerResult, resumeResult, hasBaseline, err } // The trigger goroutine's buffered channel yields exactly one value, so a result already // received by the poll loop must be reused here instead of joining again — a second receive // would block for the whole grace window and then report Completed:false over a real result. if polledTriggerResult != nil { - return response, state, polledTriggerResult, resumeResult, err + return response, state, polledTriggerResult, resumeResult, hasBaseline, err } if triggerHandle != nil { - return response, state, triggerHandle.join(), resumeResult, err + return response, state, triggerHandle.join(), resumeResult, hasBaseline, err } - return response, state, nil, resumeResult, err + return response, state, nil, resumeResult, hasBaseline, err } // startPausePointWaitSideEffects performs the pre-wait --resume-play / --trigger work, returning // either a live trigger handle or the fixed TriggerResult explaining why the trigger was skipped. +// When it queries status to confirm arming, that response is the wait-start snapshot for the +// new-hit baseline (decided before --resume-play), so a post-resume hit advances LastHitSequence +// past the baseline. baselineDecided is false only when no arming query ran (plain await). func startPausePointWaitSideEffects( ctx context.Context, connection unityipc.Connection, options waitForPausePointOptions, -) (*pausePointTriggerHandle, *pausePointTriggerResult, *pausePointResumePlayResult) { +) (*pausePointTriggerHandle, *pausePointTriggerResult, *pausePointResumePlayResult, int, bool, bool) { if options.triggerCommand == "" && !options.resumePlay { - return nil, nil, nil + return nil, nil, nil, -1, false, false } - if !pausePointIsArmed(ctx, connection, options.id) { + armResponse, armed := queryPausePointArmStatus(ctx, connection, options.id) + if !armed { var resumeResult *pausePointResumePlayResult var skippedTriggerResult *pausePointTriggerResult if options.resumePlay { @@ -99,42 +108,48 @@ func startPausePointWaitSideEffects( Error: "trigger was not dispatched: the marker could not be confirmed armed at wait start", } } - return nil, skippedTriggerResult, resumeResult + return nil, skippedTriggerResult, resumeResult, -1, false, true } + baselineSequence, hasBaseline := pausePointNewHitBaseline(armResponse) + var resumeResult *pausePointResumePlayResult if options.resumePlay { result := resumePlayModeForPausePoint(ctx, connection) resumeResult = &result if result.Error != "" { if options.triggerCommand == "" { - return nil, nil, resumeResult + return nil, nil, resumeResult, baselineSequence, hasBaseline, true } return nil, &pausePointTriggerResult{ Command: pausePointTriggerCommandString(options.triggerCommand, options.triggerArgs), Error: "trigger was not dispatched: --resume-play failed to resume play mode", - }, resumeResult + }, resumeResult, baselineSequence, hasBaseline, true } } if options.triggerCommand == "" { - return nil, nil, resumeResult + return nil, nil, resumeResult, baselineSequence, hasBaseline, true } handle := startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) - return handle, nil, resumeResult + return handle, nil, resumeResult, baselineSequence, hasBaseline, true } -// pausePointIsArmed reports whether the marker is enabled or already hit. A query failure is +// queryPausePointArmStatus reports whether the marker is enabled or already hit. A query failure is // treated as not armed: dispatching a --trigger command against a marker this CLI cannot even // confirm exists would inject the trigger's action into the game with no corresponding wait. -func pausePointIsArmed(ctx context.Context, connection unityipc.Connection, id string) bool { +func queryPausePointArmStatus( + ctx context.Context, + connection unityipc.Connection, + id string, +) (pausePointStatusResponse, bool) { response, err := queryPausePointStatus(ctx, connection, id) if err != nil { - return false + return pausePointStatusResponse{}, false } state := pausePointWaitStateForStatus(response.Status) - return state == "" || state == pausePointWaitStateHit + return response, state == "" || state == pausePointWaitStateHit } func waitForPausePointStatus( @@ -142,7 +157,10 @@ func waitForPausePointStatus( connection unityipc.Connection, options waitForPausePointOptions, triggerHandle *pausePointTriggerHandle, -) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, error) { + baselineSequence int, + hasBaseline bool, + baselineDecided bool, +) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, bool, error) { waitContext, cancel := context.WithTimeout(ctx, options.timeout) defer cancel() @@ -158,16 +176,23 @@ func waitForPausePointStatus( if err == nil { lastResponse = response hasResponse = true - state := pausePointWaitStateForStatus(response.Status) + // Why only once: a later Enabled→Hit transition is the await success itself + // (enable --await). Re-baselining on that first mid-wait Hit would demand a second + // sequence bump and never return. + if !baselineDecided { + baselineSequence, hasBaseline = pausePointNewHitBaseline(response) + baselineDecided = true + } + state := pausePointWaitStateForPolledStatus(response, baselineSequence, hasBaseline) if state != "" { - return response, state, triggerResult, nil + return response, state, triggerResult, hasBaseline, nil } } else { // Why abort: every poll dials again, so a connect the operating system refused // permanently keeps failing for the whole --timeout and the refusal is reported only // after that wait is spent. if clierrors.IsPermanentConnectError(err) { - return lastResponse, "", triggerResult, err + return lastResponse, "", triggerResult, hasBaseline, err } lastErr = err } @@ -175,25 +200,30 @@ func waitForPausePointStatus( select { case <-waitContext.Done(): if ctx.Err() != nil { - return lastResponse, "", triggerResult, ctx.Err() + return lastResponse, "", triggerResult, hasBaseline, ctx.Err() } - finalResponse, finalState, hasFinalResponse, finalErr := queryPausePointStatusAtTimeout(ctx, connection, options.id) + finalResponse, finalState, hasFinalResponse, finalErr := queryPausePointStatusAtTimeout( + ctx, connection, options.id, baselineSequence, hasBaseline) if hasFinalResponse { lastResponse = finalResponse hasResponse = true + if !baselineDecided { + baselineSequence, hasBaseline = pausePointNewHitBaseline(finalResponse) + finalState = pausePointWaitStateForPolledStatus(finalResponse, baselineSequence, hasBaseline) + } if finalState != "" { - return finalResponse, finalState, triggerResult, nil + return finalResponse, finalState, triggerResult, hasBaseline, nil } } else if lastErr == nil { lastErr = finalErr } if hasResponse { - return lastResponse, pausePointWaitStateTimeout, triggerResult, nil + return lastResponse, pausePointWaitStateTimeout, triggerResult, hasBaseline, nil } if lastErr != nil { - return lastResponse, "", triggerResult, fmt.Errorf("timed out waiting for pause point status: %w", lastErr) + return lastResponse, "", triggerResult, hasBaseline, fmt.Errorf("timed out waiting for pause point status: %w", lastErr) } - return lastResponse, pausePointWaitStateTimeout, triggerResult, nil + return lastResponse, pausePointWaitStateTimeout, triggerResult, hasBaseline, nil case result := <-triggerDone: // Nil the channel so this case can never fire twice: the handle's channel holds a single // buffered value, and the caller reuses the result received here instead of joining. @@ -201,8 +231,8 @@ func waitForPausePointStatus( triggerDone = nil if pausePointTriggerRejectedBeforeExecution(result) { abortResponse, abortState := abortPausePointWaitAfterTriggerRejection( - ctx, connection, options.id, lastResponse) - return abortResponse, abortState, triggerResult, nil + ctx, connection, options.id, lastResponse, baselineSequence, hasBaseline) + return abortResponse, abortState, triggerResult, hasBaseline, nil } case <-ticker.C: } @@ -217,8 +247,11 @@ func abortPausePointWaitAfterTriggerRejection( connection unityipc.Connection, id string, lastResponse pausePointStatusResponse, + baselineSequence int, + hasBaseline bool, ) (pausePointStatusResponse, pausePointWaitState) { - response, state, hasResponse, _ := queryPausePointStatusAtTimeout(ctx, connection, id) + response, state, hasResponse, _ := queryPausePointStatusAtTimeout( + ctx, connection, id, baselineSequence, hasBaseline) if !hasResponse { return lastResponse, pausePointWaitStateTriggerFailed } @@ -232,6 +265,8 @@ func queryPausePointStatusAtTimeout( ctx context.Context, connection unityipc.Connection, id string, + baselineSequence int, + hasBaseline bool, ) (pausePointStatusResponse, pausePointWaitState, bool, error) { finalContext, cancel := context.WithTimeout(ctx, pausePointFinalStatusProbeTimeout) defer cancel() @@ -241,7 +276,36 @@ func queryPausePointStatusAtTimeout( return pausePointStatusResponse{}, "", false, err } - return response, pausePointWaitStateForStatus(response.Status), true, nil + return response, pausePointWaitStateForPolledStatus(response, baselineSequence, hasBaseline), true, nil +} + +// pausePointNewHitBaseline records LastHitSequence when await starts against an already-hit +// continuous/trace marker. Mode is allowlisted (never `!= "single-shot"`): an empty Mode is the +// old-package skew case and must keep the historical immediate-Hit success path. +func pausePointNewHitBaseline(response pausePointStatusResponse) (int, bool) { + if response.Status != pausePointStatusHit { + return -1, false + } + if response.Mode != pausePointModeContinuous && response.Mode != pausePointModeTrace { + return -1, false + } + return response.LastHitSequence, true +} + +// pausePointWaitStateForPolledStatus maps a polled status to a terminal wait state, treating an +// already-hit continuous/trace marker as non-terminal until LastHitSequence advances past baseline. +func pausePointWaitStateForPolledStatus( + response pausePointStatusResponse, + baselineSequence int, + hasBaseline bool, +) pausePointWaitState { + if response.Status == pausePointStatusHit { + if !hasBaseline || response.LastHitSequence > baselineSequence { + return pausePointWaitStateHit + } + return "" + } + return pausePointWaitStateForStatus(response.Status) } func pausePointWaitStateForStatus(status string) pausePointWaitState { 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 6882040ca0..8c783bd6a2 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 @@ -76,7 +76,7 @@ func TestWaitForPausePointAbortsWhenTheConnectIsRefused(t *testing.T) { } startedAt := time.Now() - _, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -128,7 +128,7 @@ func TestWaitForPausePointAbortsWhenTriggerRejectsArguments(t *testing.T) { } startedAt := time.Now() - _, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -363,7 +363,7 @@ func TestWaitForPausePointDoesNotAbortWhenTriggerSucceeds(t *testing.T) { return 0 } - _, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 50 * time.Millisecond, @@ -430,7 +430,7 @@ func TestWaitForPausePointReportsHitRacingATriggerRejection(t *testing.T) { return 1 } - response, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -677,7 +677,7 @@ func TestWaitForPausePointDoesNotRepauseWhenResumeWasANoOp(t *testing.T) { return 1 } - _, state, _, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, _, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -742,7 +742,7 @@ func TestWaitForPausePointDoesNotRepauseWhenItDidNotResume(t *testing.T) { return 1 } - _, state, _, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, _, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, 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 275927d1e3..ea2e2f7e67 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -172,7 +172,7 @@ func TestWaitForPausePointReturnsHitAfterEnabledStatus(t *testing.T) { return response, nil } - response, state, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -284,7 +284,7 @@ func TestPausePointExpiredErrorReportsRecoveryFields(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.Details["Expired"] != true { t.Fatalf("expired detail mismatch: %#v", cliErr.Details) @@ -313,7 +313,7 @@ func TestPausePointExpiredErrorReportsMarkerTimeoutSeconds(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 5, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.Details["TimeoutSeconds"] != 30 { t.Fatalf("timeoutSeconds detail mismatch: %#v", cliErr.Details) @@ -332,7 +332,7 @@ func TestPausePointExpiredErrorDerivesExpiredFromStatus(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.Details["Expired"] != true { t.Fatalf("expired detail mismatch: %#v", cliErr.Details) @@ -426,7 +426,7 @@ func TestWaitForPausePointReturnsNotEnabledStateImmediately(t *testing.T) { }, nil } - response, state, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -962,7 +962,7 @@ func TestPausePointTimeoutErrorIncludesDiagnosisHint(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, testCase.response, pausePointWaitStateTimeout) + }, testCase.response, pausePointWaitStateTimeout, false) if cliErr.Details["Hint"] != testCase.wantHint { t.Fatalf("hint mismatch: %#v", cliErr.Details) @@ -1011,7 +1011,7 @@ func TestPausePointExpiredErrorIncludesDiagnosisHint(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, testCase.response, pausePointWaitStateExpired) + }, testCase.response, pausePointWaitStateExpired, false) if cliErr.Details["Hint"] != testCase.wantHint { t.Fatalf("hint mismatch: %#v", cliErr.Details) @@ -1031,7 +1031,7 @@ func TestPausePointHintIsOmittedOutsideDiagnosableStates(t *testing.T) { timeoutErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, hitResponse, pausePointWaitStateTimeout) + }, hitResponse, pausePointWaitStateTimeout, false) if _, exists := timeoutErr.Details["Hint"]; exists { t.Fatalf("hint should be omitted when no diagnosis applies: %#v", timeoutErr.Details) } @@ -1044,7 +1044,7 @@ func TestPausePointHintIsOmittedOutsideDiagnosableStates(t *testing.T) { clearedErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, clearedResponse, pausePointWaitStateCleared) + }, clearedResponse, pausePointWaitStateCleared, false) if _, exists := clearedErr.Details["Hint"]; exists { t.Fatalf("hint should be omitted for cleared markers: %#v", clearedErr.Details) } @@ -1064,7 +1064,7 @@ func TestPausePointExpiredErrorReportsNoRemainingTime(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.ErrorCode != clierrors.ErrorCodePausePointExpired { t.Fatalf("error code mismatch: %#v", cliErr) From f0852c9269ec802ee2d1dea4cd169ff6210c0dfe Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 29 Jul 2026 12:09:07 +0900 Subject: [PATCH 2/3] fix: preserve armed continuous markers on new-hit await timeout Skip timeout clear when waiting for a later sequence, leave baseline undecided after a failed arm query, and never baseline enable --await hits that race in before the first status poll. Co-authored-by: Cursor --- .../pause_point_await_new_hit_test.go | 133 +++++++++++++++++- .../projectrunner/pause_point_enable.go | 6 +- .../projectrunner/pause_point_wait.go | 10 +- .../projectrunner/pause_point_wait_poll.go | 36 +++-- 4 files changed, 172 insertions(+), 13 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go b/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go index 6750f5c039..ad3a2fc742 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go @@ -71,7 +71,8 @@ func TestWaitForPausePointWaitsForNewHitOnAlreadyHitContinuousMarker(t *testing. } // Verifies await on an already-hit continuous marker times out with PAUSE_POINT_WAIT_TIMEOUT and -// the already-hit baseline hint when LastHitSequence never advances. +// the already-hit baseline hint when LastHitSequence never advances, without clearing the still-armed +// marker (the hint tells the caller to await again). func TestWaitForPausePointTimesOutWaitingForNewHitOnContinuousMarker(t *testing.T) { originalQuery := queryPausePointStatus originalClear := clearPausePointStatus @@ -98,12 +99,15 @@ func TestWaitForPausePointTimesOutWaitingForNewHitOnContinuousMarker(t *testing. EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, }, nil } + clearCalls := 0 clearPausePointStatus = func( ctx context.Context, connection unityipc.Connection, id string, ) (pausePointStatusResponse, error) { - return pausePointStatusResponse{Id: id, Status: pausePointStatusCleared}, nil + clearCalls++ + t.Fatal("timeout while waiting for a new hit must not clear the still-armed continuous marker") + return pausePointStatusResponse{}, nil } stderr := &bytes.Buffer{} @@ -121,6 +125,9 @@ func TestWaitForPausePointTimesOutWaitingForNewHitOnContinuousMarker(t *testing. if exitCode != 1 { t.Fatalf("exit code mismatch: %d", exitCode) } + if clearCalls != 0 { + t.Fatalf("expected clear not to run, got %d calls", clearCalls) + } stderrText := stderr.String() if !strings.Contains(stderrText, clierrors.ErrorCodePausePointWaitTimeout) { t.Fatalf("expected %s, got stderr: %s", clierrors.ErrorCodePausePointWaitTimeout, stderrText) @@ -133,6 +140,128 @@ func TestWaitForPausePointTimesOutWaitingForNewHitOnContinuousMarker(t *testing. } } +// Verifies a transient arm-query failure does not decide "no baseline", so a later stale continuous +// Hit is not returned as an immediate wait success. +func TestWaitForPausePointBaseliningSurvivesTransientArmQueryFailure(t *testing.T) { + originalQuery := queryPausePointStatus + originalClear := clearPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + clearPausePointStatus = originalClear + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + if queryCount == 1 { + return pausePointStatusResponse{}, context.DeadlineExceeded + } + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 5, + Mode: pausePointModeContinuous, + LastHitSequence: 5, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + clearPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + t.Fatal("baseline timeout must not clear the still-armed continuous marker") + return pausePointStatusResponse{}, nil + } + + stderr := &bytes.Buffer{} + exitCode := runWaitForPausePoint( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: 40 * time.Millisecond, + resumePlay: true, + }, + &bytes.Buffer{}, + stderr, + ) + if exitCode != 1 { + t.Fatalf("exit code mismatch: %d", exitCode) + } + stderrText := stderr.String() + if !strings.Contains(stderrText, clierrors.ErrorCodePausePointWaitTimeout) { + t.Fatalf("expected %s after a stale continuous Hit, got stderr: %s", clierrors.ErrorCodePausePointWaitTimeout, stderrText) + } + if !strings.Contains(stderrText, pausePointHintAlreadyHitWaitingForNew) { + t.Fatalf("expected already-hit baseline hint, got stderr: %s", stderrText) + } +} + +// Verifies enable --await never baselining a Hit that raced in before the first status query. +func TestWaitForPausePointAcceptsImmediateHitWhenMarkerJustEnabled(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Hour + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + Mode: pausePointModeContinuous, + LastHitSequence: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + response, state, _, _, hasNewHitBaseline, err := waitForPausePoint( + context.Background(), + unityipc.Connection{}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + markerJustEnabled: true, + }, + ) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateHit { + t.Fatalf("state mismatch: %s", state) + } + if hasNewHitBaseline { + t.Fatal("enable --await must not establish a new-hit baseline") + } + if response.LastHitSequence != 1 { + t.Fatalf("expected the raced enable-time hit, got %#v", response) + } + if queryCount != 1 { + t.Fatalf("expected a single status query, got %d", queryCount) + } +} + // Verifies an already-hit single-shot marker still returns immediately (baseline not applied). func TestWaitForPausePointReturnsImmediatelyForAlreadyHitSingleShotMarker(t *testing.T) { originalQuery := queryPausePointStatus diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 0516ab09ca..4a71711550 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -341,6 +341,7 @@ func runEnablePausePointAndAwait( triggerArgs: triggerArgs, startPath: startPath, resumePlay: resumePlay, + markerJustEnabled: true, } return runPausePointWaitAfterEnable( @@ -432,9 +433,12 @@ func runPausePointWaitAfterEnable( return 0 } - if state == pausePointWaitStateTimeout { + if state == pausePointWaitStateTimeout && !hasNewHitBaseline { clearPausePointAfterWaitTimeout(ctx, connection, options.id) } + // Why skip clear when hasNewHitBaseline: the continuous/trace marker is still armed, and the + // timeout hint tells the caller to await again (with --resume-play). Clearing here would disarm + // it and discard the raw capture holder, making that recovery path impossible. waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) waitErr.Command = pausePointEnableCommandName diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index 575f1ab073..c21852d891 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -61,6 +61,11 @@ type waitForPausePointOptions struct { // (if paused) before dispatching --trigger so a paused-arm workflow can fire input triggers // in one CLI call. resumePlay bool + + // markerJustEnabled is set only by enable-pause-point --await. Why: enable finishes before the + // first status query, and a real hit can race into that window on continuous/trace markers. + // Baselining that hit would demand a later sequence that never comes (continuous pauses on hit). + markerJustEnabled bool } type pausePointStatusOptions struct { @@ -259,9 +264,12 @@ func runWaitForPausePoint( return 0 } - if state == pausePointWaitStateTimeout { + if state == pausePointWaitStateTimeout && !hasNewHitBaseline { clearPausePointAfterWaitTimeout(ctx, connection, options.id) } + // Why skip clear when hasNewHitBaseline: the continuous/trace marker is still armed, and the + // timeout hint tells the caller to await again (with --resume-play). Clearing here would disarm + // it and discard the raw capture holder, making that recovery path impossible. waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) if triggerResult != nil { 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 006a978d84..72cf2beac6 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go @@ -108,10 +108,13 @@ func startPausePointWaitSideEffects( Error: "trigger was not dispatched: the marker could not be confirmed armed at wait start", } } - return nil, skippedTriggerResult, resumeResult, -1, false, true + // Why baselineDecided=false: a transient arm-query failure is also reported as not armed. + // Leaving baseline undecided lets the first successful poll establish it, so a stale + // continuous Hit cannot slip through as an immediate wait success. + return nil, skippedTriggerResult, resumeResult, -1, false, false } - baselineSequence, hasBaseline := pausePointNewHitBaseline(armResponse) + baselineSequence, hasBaseline, baselineDecided := decidePausePointNewHitBaseline(armResponse, options.markerJustEnabled) var resumeResult *pausePointResumePlayResult if options.resumePlay { @@ -119,21 +122,21 @@ func startPausePointWaitSideEffects( resumeResult = &result if result.Error != "" { if options.triggerCommand == "" { - return nil, nil, resumeResult, baselineSequence, hasBaseline, true + return nil, nil, resumeResult, baselineSequence, hasBaseline, baselineDecided } return nil, &pausePointTriggerResult{ Command: pausePointTriggerCommandString(options.triggerCommand, options.triggerArgs), Error: "trigger was not dispatched: --resume-play failed to resume play mode", - }, resumeResult, baselineSequence, hasBaseline, true + }, resumeResult, baselineSequence, hasBaseline, baselineDecided } } if options.triggerCommand == "" { - return nil, nil, resumeResult, baselineSequence, hasBaseline, true + return nil, nil, resumeResult, baselineSequence, hasBaseline, baselineDecided } handle := startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) - return handle, nil, resumeResult, baselineSequence, hasBaseline, true + return handle, nil, resumeResult, baselineSequence, hasBaseline, baselineDecided } // queryPausePointArmStatus reports whether the marker is enabled or already hit. A query failure is @@ -180,8 +183,8 @@ func waitForPausePointStatus( // (enable --await). Re-baselining on that first mid-wait Hit would demand a second // sequence bump and never return. if !baselineDecided { - baselineSequence, hasBaseline = pausePointNewHitBaseline(response) - baselineDecided = true + baselineSequence, hasBaseline, baselineDecided = decidePausePointNewHitBaseline( + response, options.markerJustEnabled) } state := pausePointWaitStateForPolledStatus(response, baselineSequence, hasBaseline) if state != "" { @@ -208,7 +211,8 @@ func waitForPausePointStatus( lastResponse = finalResponse hasResponse = true if !baselineDecided { - baselineSequence, hasBaseline = pausePointNewHitBaseline(finalResponse) + baselineSequence, hasBaseline, _ = decidePausePointNewHitBaseline( + finalResponse, options.markerJustEnabled) finalState = pausePointWaitStateForPolledStatus(finalResponse, baselineSequence, hasBaseline) } if finalState != "" { @@ -279,6 +283,20 @@ func queryPausePointStatusAtTimeout( return response, pausePointWaitStateForPolledStatus(response, baselineSequence, hasBaseline), true, nil } +// decidePausePointNewHitBaseline returns (sequence, hasBaseline, decided) for the wait-start +// snapshot. markerJustEnabled forces no baseline: any Hit during enable --await is the success +// itself, including a race where the first status query already observes sequence 1. +func decidePausePointNewHitBaseline( + response pausePointStatusResponse, + markerJustEnabled bool, +) (int, bool, bool) { + if markerJustEnabled { + return -1, false, true + } + sequence, hasBaseline := pausePointNewHitBaseline(response) + return sequence, hasBaseline, true +} + // pausePointNewHitBaseline records LastHitSequence when await starts against an already-hit // continuous/trace marker. Mode is allowlisted (never `!= "single-shot"`): an empty Mode is the // old-package skew case and must keep the historical immediate-Hit success path. From 4710817f00e8bc2d8a5f1d634768048f88da4457 Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 29 Jul 2026 12:10:53 +0900 Subject: [PATCH 3/3] style: place skip-clear comments before the timeout ifs Co-authored-by: Cursor --- .../internal/projectrunner/pause_point_enable.go | 6 +++--- .../internal/projectrunner/pause_point_wait.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 4a71711550..317a400a2b 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -433,12 +433,12 @@ func runPausePointWaitAfterEnable( return 0 } - if state == pausePointWaitStateTimeout && !hasNewHitBaseline { - clearPausePointAfterWaitTimeout(ctx, connection, options.id) - } // Why skip clear when hasNewHitBaseline: the continuous/trace marker is still armed, and the // timeout hint tells the caller to await again (with --resume-play). Clearing here would disarm // it and discard the raw capture holder, making that recovery path impossible. + if state == pausePointWaitStateTimeout && !hasNewHitBaseline { + clearPausePointAfterWaitTimeout(ctx, connection, options.id) + } waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) waitErr.Command = pausePointEnableCommandName diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index c21852d891..44c657d6e4 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -264,12 +264,12 @@ func runWaitForPausePoint( return 0 } - if state == pausePointWaitStateTimeout && !hasNewHitBaseline { - clearPausePointAfterWaitTimeout(ctx, connection, options.id) - } // Why skip clear when hasNewHitBaseline: the continuous/trace marker is still armed, and the // timeout hint tells the caller to await again (with --resume-play). Clearing here would disarm // it and discard the raw capture holder, making that recovery path impossible. + if state == pausePointWaitStateTimeout && !hasNewHitBaseline { + clearPausePointAfterWaitTimeout(ctx, connection, options.id) + } waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) if triggerResult != nil {