feat: Add --trigger to await-pause-point for one-command verification#1920
Conversation
Waiting for a pause point and then triggering the action that hits it required two separate CLI calls with a manual race (start a background simulate-* command, then await). --trigger dispatches the given uloop subcommand in-process right after the marker is armed, and embeds its response (or dispatch error) as TriggerResult in the wait's own JSON output, collapsing that into a single call. - Quote-aware tokenizer parses the --trigger value into argv-style tokens. - Fail-fast validation rejects targeting await-pause-point/enable-pause-point (would deadlock) and --project-path inside the trigger string (implicit project inheritance only). - The trigger runs concurrently via a goroutine and is joined with a short grace window after the wait itself settles, reusing pausePointFinalStatusProbeTimeout for consistency with the existing final-status-probe pattern. - Verified end-to-end against the KeyStateAfterPauseInterruption regression harness: enable-pause-point + await-pause-point --trigger "simulate-keyboard ..." now hits in ~280ms instead of running the full press duration.
The --trigger flag's own manual verification procedure needs to stay runnable beyond this one review round, so capture it as a permanent harness script rather than a throwaway command sequence — reusing the KeyStateAfterPauseInterruption scene scaffolding from the existing trap harness. Confirmed passing locally: enable-pause-point + await-pause-point --trigger hits in 0s instead of waiting out the 5s triggered Press.
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
The advisor's final review found that await-pause-point's extendPausePointExpiryBeforeWait step is best-effort and does not confirm the marker is actually enabled, so a --trigger command (e.g. a 5s simulate-keyboard Press) was being dispatched into the running game even when the wait was about to fail immediately with NotEnabled — a typo'd or expired --id would still inject the triggered action with no corresponding wait watching for it. waitForPausePoint now runs one status query up front and only starts the trigger once the marker is confirmed enabled or already hit; a query failure is also treated as not armed. Verified live: await-pause-point --id <nonexistent> --trigger "simulate-keyboard ..." now returns NotEnabled in ~74ms with no TriggerResult in the response, instead of letting Press run.
A skipped trigger (marker not confirmed armed) previously left TriggerResult entirely omitted, indistinguishable from "the trigger ran but never reported back in time" (Completed=false with a Command). If the initial arm-check query hits a transient IPC error while the marker itself is actually armed, the trigger silently never fires, the marker never gets hit, the wait times out, and nothing in the response explains why. waitForPausePoint now always returns a TriggerResult when --trigger was given: on skip, Completed=false with an explanatory Error, so the contract is simply "passing --trigger always yields a TriggerResult."
8e19067
into
feature/round7-pause-point-improvements
Summary
await-pause-pointandenable-pause-point --awaitnow accept--trigger '<uloop command>', so waiting for a pause point and triggering the action that hits it is a single call instead of a manually raced pair of commands.User Impact
await-pause-point/enable-pause-point --awaitand hoping the timing lined up.--trigger, and the CLI dispatches it right after the marker is armed, then reports its result inline asTriggerResultin the same JSON response — no more manual backgrounding or timing races.Changes
--triggerparsing: quote-aware tokenization of the trigger command string, with fail-fast validation that rejects targetingawait-pause-point/enable-pause-point(would deadlock) and rejects--project-pathinside the trigger string (the triggered command always runs against the same project).TriggerResult(Command,Completed,Response,Error) is embedded in the wait's JSON output only when--triggerwas used — omitted entirely otherwise, so existing callers see no schema change.scripts/regression-harness-pause-point-trigger.sh) that reuses the existingKeyStateAfterPauseInterruptionscene to verify the trigger hits within the marker's timeout rather than waiting out the triggered command's full duration.--idreports its own failure without also injecting the trigger's action into the running game.Verification
sh scripts/check-go-cli.sh: 0 issues across all four Go modules, all tests pass.go test ./internal/projectrunner/... -run 'Trigger|PausePoint': all new and updated table-driven tests pass, including a case asserting the trigger is never dispatched when the marker isn't armed.enable-pause-point+await-pause-point --trigger "simulate-keyboard --action Press --key Space --duration 5"against theKeyStateAfterPauseInterruptionscene hits in ~280ms withTriggerResult.Response.InterruptedByPausePoint: true, instead of waiting out the full 5s duration.await-pause-point --id <nonexistent> --trigger "simulate-keyboard --action Press --key Space --duration 5"returnsNotEnabledin ~74ms with noTriggerResultin the response — the trigger never ran.