Skip to content

feat: add --resume-play to pause-point await flow#1964

Merged
hatayama merged 5 commits into
feature/pause-point-round9from
feature/pause-point-resume-play
Jul 23, 2026
Merged

feat: add --resume-play to pause-point await flow#1964
hatayama merged 5 commits into
feature/pause-point-round9from
feature/pause-point-resume-play

Conversation

@hatayama

@hatayama hatayama commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Summary

  • enable-pause-point --await / await-pause-point can now take --resume-play, so a manually paused PlayMode is resumed after the marker is confirmed armed and before --trigger runs — in one CLI call.
  • This removes the race where scenario setup and pause-point arming were split across tool round-trips while the game kept advancing.

User Impact

  • Before: arming while paused and then sending input required a separate resume step. Input triggers refused with PlayMode is paused..., so agents either raced real time between setup and arm, or could not fold "resume then trigger" into one await.
  • After: enable-pause-point --await --resume-play --trigger "..." resumes when needed, fires the trigger, and returns ResumePlayResult alongside the existing TriggerResult.

Changes

  • Parse --resume-play (requires --await on enable, same family as --trigger).
  • After arm confirmation: Status → Play (only if paused) → optional --trigger.
  • Report ResumePlayResult (WasPaused / Resumed / Error); on resume failure, skip trigger with a fixed skip error and keep waiting.
  • Add scripts/regression-harness-resume-play-paused-arm.sh and document it in docs/regression-harness.md.

Verification

Red (paused arm without --resume-play)

Command (project path redacted):

dist/darwin-arm64/uloop enable-pause-point \
  --file Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs \
  --line 19 --timeout-seconds 60 --await \
  --trigger "simulate-keyboard --action Press --key Space --duration 5" \
  --project-path <PROJECT_ROOT>

Observed (excerpt):

{
  "Success": false,
  "Error": {
    "ErrorCode": "PAUSE_POINT_EXPIRED",
    "Details": {
      "HitCount": 0,
      "Status": "Expired",
      "TriggerResult": {
        "Command": "simulate-keyboard --action Press --key Space --duration 5",
        "Completed": true,
        "Response": {
          "Message": "PlayMode is paused. Resume PlayMode before simulating keyboard input.",
          "Success": false
        }
      }
    }
  }
}

Green (same flow with --resume-play)

dist/darwin-arm64/uloop enable-pause-point \
  --file Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs \
  --line 19 --timeout-seconds 60 --await --resume-play \
  --trigger "simulate-keyboard --action Press --key Space --duration 5" \
  --project-path <PROJECT_ROOT>

Observed (excerpt):

{
  "Success": true,
  "Status": "Hit",
  "HitCount": 1,
  "ResumePlayResult": { "WasPaused": true, "Resumed": true },
  "TriggerResult": {
    "Completed": true,
    "Response": { "InterruptedByPausePoint": true, "Success": true }
  }
}

Automated / local

  • cd cli/project-runner && go test ./... — pass
  • sh scripts/check-go-cli.sh — exit 0
  • sh scripts/build-go-cli.sh — exit 0
  • PATH=<PROJECT_ROOT>/dist/darwin-arm64:$PATH sh scripts/regression-harness-resume-play-paused-arm.sh --project-path <PROJECT_ROOT> — exit 0

Review in cubic

hatayama and others added 3 commits July 24, 2026 02:17
Capture the Red contract before implementation: require --await,
resume-before-trigger ordering, skip-on-resume-failure, and skip when
the marker is not armed. Confirmed with compile errors against current API.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resume PlayMode after arm confirmation and before --trigger so a
paused-arm workflow can fire input in one CLI call. ResumePlayResult
mirrors TriggerResult's omit-when-unused reporting, and a failed resume
skips trigger dispatch with a fixed skip error.

Co-authored-by: Cursor <cursoragent@cursor.com>
Lock in the enable-pause-point --await --resume-play --trigger path that
resumes a manually paused PlayMode before firing the input trigger, using
the existing KeyStateAfterPauseInterruption scene.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@hatayama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ffaab1d5-586b-4cb4-ae3c-2abe8decef5f

📥 Commits

Reviewing files that changed from the base of the PR and between 1c5bafb and e434fdd.

📒 Files selected for processing (3)
  • cli/project-runner/internal/projectrunner/pause_point_enable.go
  • cli/project-runner/internal/projectrunner/pause_point_resume_play.go
  • cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go
📝 Walkthrough

Walkthrough

The project runner adds --resume-play support to pause-point enable and await commands, resumes paused Unity PlayMode before triggering, reports resume outcomes in JSON, updates help and list output, and adds unit and regression-harness coverage.

Changes

Pause-point resume-play flow

Layer / File(s) Summary
CLI flags and argument plumbing
cli/project-runner/internal/projectrunner/list_output.go, native_command_help.go, pause_point_enable.go, pause_point_wait.go
Adds --resume-play to help and catalog output, parses and validates it with --await, and passes it into pause-point waiting.
Unity PlayMode resume execution
cli/project-runner/internal/projectrunner/pause_point_resume_play.go, pause_point_resume_play_test.go
Queries PlayMode status and sends Play when the editor is paused, returning structured pause, resume, and error results.
Wait orchestration and result propagation
cli/project-runner/internal/projectrunner/pause_point_wait*.go, pause_point_types.go, pause_point_*_test.go
Checks marker arming, resumes before trigger dispatch, skips triggering when resume fails, and includes ResumePlayResult in success and error payloads.
Regression scenario and validation
scripts/regression-harness-resume-play-paused-arm.sh, docs/regression-harness.md, *_test.go
Adds tests and a harness scenario validating resume and trigger results when PlayMode is already paused.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant EnablePausePoint
  participant WaitForPausePoint
  participant ControlPlayMode
  participant TriggerCommand
  EnablePausePoint->>WaitForPausePoint: await with resume-play and trigger
  WaitForPausePoint->>ControlPlayMode: Status
  ControlPlayMode-->>WaitForPausePoint: paused status
  WaitForPausePoint->>ControlPlayMode: Play
  ControlPlayMode-->>WaitForPausePoint: resume result
  WaitForPausePoint->>TriggerCommand: dispatch trigger
  TriggerCommand-->>WaitForPausePoint: trigger result
  WaitForPausePoint-->>EnablePausePoint: pause-point and resume results
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding --resume-play to the pause-point await flow.
Description check ✅ Passed The description is directly related to the changeset and matches the new resume-play behavior and regression harness additions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/pause-point-resume-play

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go (1)

281-327: 📐 Maintainability & Code Quality | 🔵 Trivial

Consider a combined not-armed test for --resume-play + --trigger.

This test only exercises resumePlay alone against a not-armed marker. The implementation sets resumeResult and skippedTriggerResult in independent if blocks (not else-if) when not armed, so a test asserting both are populated together when both flags are passed would close a small coverage gap.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go`
around lines 281 - 327, Extend TestWaitForPausePointSkipsResumeWhenNotArmed to
pass both resumePlay and trigger options for a not-armed marker. Assert
resumePlayModeForPausePoint is not called, resumeResult contains the skip error,
and triggerResult is populated with the expected skipped-trigger result.
cli/project-runner/internal/projectrunner/pause_point_wait_poll.go (1)

41-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting the armed-branch logic into a helper.

The nested if armed { if resumePlay {...} else if trigger {...} } else {...} block is doing a lot in one function. A small helper (e.g., resolveResumeAndTrigger(...)) returning (triggerHandle, skippedTriggerResult, resumeResult) would reduce nesting without changing behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli/project-runner/internal/projectrunner/pause_point_wait_poll.go` around
lines 41 - 79, The nested armed/not-armed handling in the pause-point wait flow
should be extracted from the current function into a focused helper such as
resolveResumeAndTrigger. Have it accept the existing context, connection, and
options needed for pause-point resolution, return triggerHandle,
skippedTriggerResult, and resumeResult, and preserve all current resume-play
failure, trigger dispatch, and unarmed-marker result behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/regression-harness-resume-play-paused-arm.sh`:
- Around line 18-21: Update the argument parsing around PROJECT_PATH to accept
only the expected --project-path value pair, reject missing values and any
unsupported argument list with an error and nonzero exit, and do so before
project selection or cleanup can proceed.

---

Nitpick comments:
In `@cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go`:
- Around line 281-327: Extend TestWaitForPausePointSkipsResumeWhenNotArmed to
pass both resumePlay and trigger options for a not-armed marker. Assert
resumePlayModeForPausePoint is not called, resumeResult contains the skip error,
and triggerResult is populated with the expected skipped-trigger result.

In `@cli/project-runner/internal/projectrunner/pause_point_wait_poll.go`:
- Around line 41-79: The nested armed/not-armed handling in the pause-point wait
flow should be extracted from the current function into a focused helper such as
resolveResumeAndTrigger. Have it accept the existing context, connection, and
options needed for pause-point resolution, return triggerHandle,
skippedTriggerResult, and resumeResult, and preserve all current resume-play
failure, trigger dispatch, and unarmed-marker result behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 835eb274-5559-4d5c-896b-2718161df761

📥 Commits

Reviewing files that changed from the base of the PR and between e1e3467 and a553fb9.

📒 Files selected for processing (15)
  • cli/project-runner/internal/projectrunner/list_output.go
  • cli/project-runner/internal/projectrunner/list_output_test.go
  • cli/project-runner/internal/projectrunner/native_command_help.go
  • cli/project-runner/internal/projectrunner/native_command_help_test.go
  • cli/project-runner/internal/projectrunner/pause_point_enable.go
  • cli/project-runner/internal/projectrunner/pause_point_enable_test.go
  • cli/project-runner/internal/projectrunner/pause_point_resume_play.go
  • cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go
  • cli/project-runner/internal/projectrunner/pause_point_trigger_test.go
  • cli/project-runner/internal/projectrunner/pause_point_types.go
  • cli/project-runner/internal/projectrunner/pause_point_wait.go
  • cli/project-runner/internal/projectrunner/pause_point_wait_poll.go
  • cli/project-runner/internal/projectrunner/pause_point_wait_test.go
  • docs/regression-harness.md
  • scripts/regression-harness-resume-play-paused-arm.sh

Comment thread scripts/regression-harness-resume-play-paused-arm.sh
Avoid silently falling back to cwd when argv is wrong, so the cleanup
trap cannot clear pause points or stop Play Mode in the wrong project.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add injectable sendControlPlayModeForPausePointResume tests for the
four Status/Play outcomes, accept --resume-play=true on enable, drop
the unused Warning field, and rename the misleading playRequested flag.

Co-authored-by: Cursor <cursoragent@cursor.com>
@hatayama
hatayama merged commit 4dcbcdb into feature/pause-point-round9 Jul 23, 2026
2 checks passed
@hatayama
hatayama deleted the feature/pause-point-resume-play branch July 23, 2026 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant