From abf54fad9645d5826d84befe659bbc4d5cfbdf5b Mon Sep 17 00:00:00 2001 From: Micah Smith Date: Mon, 12 Oct 2026 18:02:26 -0400 Subject: [PATCH] refactor(autoland): handle Ctrl+C once at the run boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Phase 5 of the autoland step-abstraction stack: cancellation. This one intentionally changes behavior, listed at the end. **What was wrong.** `commands._install_signal_handler` installed a SIGINT handler that did the whole interruption from inside the signal handler: it set `ctx.aborted` and the abort reason, saved the checkpoint, printed, rendered the status, disposed of the worktree, and called `sys.exit(130)`. A Python signal handler runs between two arbitrary bytecodes of the main thread, so all of that could start in the middle of a checkpoint write, a console print, or a mutation of the run state. Meanwhile ~10 wait loops in `waits.py` and `native_stack.py` polled `ctx.aborted` / `ctx.abort_requested()`, but nothing set it during a run (the handler exited instead of returning), so those checks never fired, and neither did the "aborted" results they returned. **The new interrupt path.** There is no custom handler. Ctrl+C raises `KeyboardInterrupt` (Python's default) from wherever the run is: a wait's sleep, a `gh` call, a confirm prompt's `input()`. The fresh, resume, and replan flows all execute the plan through one new function, `commands._land`, which catches it around `engine.execute_plan` and does what the handler did, in normal control flow: records `abort_reason = "User interrupted (Ctrl+C)"`, saves the checkpoint, prints "Interrupted! State saved. Resume with --resume." and the status, disposes of the worktree by the same keep/cleanup rules (kept unless `--always-cleanup`), and exits 130. The `finally` blocks that were already in the flows release the lock and the worktree as `SystemExit` unwinds. The checkpoint stays at the step that was interrupted, so `--resume` runs that step again. `checkpointer.save` writes a temporary file and renames it into place, so a second Ctrl+C during that save leaves the checkpoint written as the step started, which resumes the same way. `_land` still calls `signal.signal(SIGINT, signal.default_int_handler)` once, as landing starts. That restores Python's default and is not a custom handler. A process started with SIGINT ignored (a background job of a non-interactive shell, e.g. an agent or script running `stack-pr autoland &`) inherits `SIG_IGN`, and Python keeps it. The old handler overrode that as a side effect. Without the restore, a takeover's SIGINT would be ignored and the takeover would time out. The new subprocess test fails without it. Takeover (`_stop_running_autoland`, from `--replan` or a new run of the same branch) is unchanged: the other run gets SIGINT, raises `KeyboardInterrupt`, saves, exits 130, and releases the lock. **Dead checks removed.** `LandingContext.aborted` and `abort_requested()` are gone, along with every `if ctx.aborted` check in `wait_for_workflow`, `wait_for_approval`, `wait_for_checks`, `wait_for_mergeable`, `enqueue_and_wait` (both loops), and `_await_native_stack_merge`, and the results only they produced (`MergeableResult(error="aborted")`, the stack merge's `"aborted"` failure and its "Stack merge was aborted" reason). `wait_for_approval`, `wait_for_checks`, `wait_for_mergeable`, `enqueue_and_wait`, `_rewait_after_retry`, and `_await_native_stack_merge` only took `ctx` for that check, so they and the `Waits` protocol no longer take it (`wait_for_workflow` keeps it, since it uses `ctx.last_landed_sha` as its default target). The status display now keys "ABORTED: …" off `abort_reason`, the field `aborted` was derived from on load. `abort_reason` is still persisted and shown by `--status`. The state file format is unchanged (`STATE_VERSION` 1, same keys). `aborted` was never written to it, only derived on load. **No `CancelToken`.** It would have no consumer. Nothing in the process asks for cancellation except SIGINT, and SIGINT is fully handled by `KeyboardInterrupt` unwinding to `_land`. That also covers the upcoming `x ` step, which runs a script as a subprocess. Ctrl+C in the terminal goes to the whole foreground process group, so the child gets SIGINT directly. In the parent, `subprocess.run` lets `KeyboardInterrupt` interrupt its wait, gives the child a moment to exit, then kills it and re-raises. A takeover's SIGINT reaches only the autoland process, not the child, but the same `subprocess.run` cleanup kills the child before the exception reaches `_land`. So the `x` step needs no cancellation machinery, only `subprocess.run` (or anything else that kills its child on `BaseException`). A token is worth adding when something has to cancel a run without a signal. **Landed SHA lookup in `WorkflowStep`: kept.** Phase 3 had the workflow step get the landed SHA through `sc.landed_sha()`, which records it on the run before the wait starts, so an interrupted wait's checkpoint includes it. That's still needed. The checkpoint is now saved at the boundary, but still mid-step, from the same run state. If the step only reported the SHA in its `StepResult`, an interrupted wait would never return one, and the resumed wait would follow whatever the target branch head is by then. **User-visible behavior changes** - When Ctrl+C or a takeover stops a landing run, the messages, status, checkpoint, worktree handling, and exit code 130 are the same as before. The difference is that this now runs in normal control flow instead of inside a signal handler. - A Ctrl+C after the plan has finished (while the final status prints or the worktree is cleaned up) is no longer treated as an interruption of the run. It used to save a checkpoint marked "User interrupted" for a finished run (re-creating one a successful run had just deleted). Now it is a plain `KeyboardInterrupt`, and the lock and worktree are still released by the existing `finally` blocks. Ctrl+C before landing starts (discovery, the replan/overwrite prompts) behaves as before. - A PR closed while autoland waits for its approval now stops the run with "PR #N was closed while waiting for approval" instead of "PR #N approval wait was aborted". This is the only way that wait fails, in both the one-at-a-time and the stack-merge path. ## Test plan - [x] New `tests/test_autoland_interrupt.py`, run against injected fake waits and UI: - [x] Ctrl+C (`KeyboardInterrupt`) inside an approval wait exits 130, the checkpoint has `abort_reason` "User interrupted (Ctrl+C)" at the interrupted step with earlier progress kept, "Interrupted! State saved. Resume with --resume." is printed, the lock is free, and the `--branch` worktree is preserved, or removed with `--always-cleanup`. - [x] Ctrl+C at a `c` confirm prompt gives the same result, and the confirmation stays unconfirmed. - [x] `--resume` after an interrupt continues from the interrupted step: the PR landed before it is not landed again, and the run finishes and deletes its checkpoint. - [x] A real subprocess run blocked in a wait, started with SIGINT ignored like a background job, is stopped by `_stop_running_autoland`: it exits 130, prints the interrupt message, checkpoints "User interrupted (Ctrl+C)", and gives up the lock. The test fails without the `default_int_handler` restore. - [x] Wait-loop tests that used to stop a poll by setting `ctx.aborted` now raise from the sleep instead, and still check that an unapproved PR, an unknown ancestry, or a failed or unfinished workflow run is still being waited on. - [x] Golden scenarios pass without regenerating any golden file. - [ ] Manual: press Ctrl+C during a real autoland wait (approval, checks, or merge queue) and at a `c` confirm prompt. Check the message, the exit code (`echo $?` shows 130), and `--status` ("ABORTED: User interrupted (Ctrl+C)"), then `--resume` and check it continues from the interrupted step. - [ ] Manual: start an autoland, then run `--replan` for the same branch from another terminal and confirm the takeover stops the first run and continues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 (1M context) stack-info: PR: https://github.com/micahjsmith/stack-pr/pull/73, branch: micahjsmith/stack/7 --- src/stack_pr/autoland/commands.py | 69 +++---- src/stack_pr/autoland/display.py | 8 +- src/stack_pr/autoland/engine.py | 1 - src/stack_pr/autoland/model.py | 7 +- src/stack_pr/autoland/native_stack.py | 17 +- src/stack_pr/autoland/services.py | 33 ++-- src/stack_pr/autoland/state.py | 1 - src/stack_pr/autoland/steps/land.py | 16 +- src/stack_pr/autoland/waits.py | 48 ++--- tests/autoland_scenarios.py | 3 +- tests/test_autoland.py | 71 ++++--- tests/test_autoland_golden.py | 2 - tests/test_autoland_interrupt.py | 257 ++++++++++++++++++++++++++ tests/test_autoland_optimize.py | 8 +- tests/test_autoland_step_execution.py | 15 +- 15 files changed, 373 insertions(+), 183 deletions(-) create mode 100644 tests/test_autoland_interrupt.py diff --git a/src/stack_pr/autoland/commands.py b/src/stack_pr/autoland/commands.py index 9df5021..761d342 100644 --- a/src/stack_pr/autoland/commands.py +++ b/src/stack_pr/autoland/commands.py @@ -237,20 +237,38 @@ def _dispose_worktree( worktree.announce_preserved() -def _install_signal_handler( +def _land( ctx: LandingContext, + common: cli.CommonArgs, + opts: AutolandOptions, checkpointer: AutolandCheckpointer, worktree: Worktree | None, - opts: AutolandOptions, ) -> None: - # Installed as landing starts: from here on, the handler and _finish - # decide whether the worktree is kept. + """Execute the plan, report how the run ended, and exit if it didn't succeed. + + This is the one place a run is interrupted. Ctrl+C anywhere in the plan's + execution (a wait's sleep, a gh call, a confirm prompt) raises + KeyboardInterrupt out of it, and so does a takeover by another autoland of + the branch, which sends this process SIGINT (``_stop_running_autoland``). + The run is then checkpointed at the step it was in, so ``--resume`` picks + that step up again, and exits 130; the callers' ``finally`` blocks release + the lock on the way out. + """ + # From here on, the run's outcome decides whether the worktree is kept. if worktree is not None: worktree.landing = True - - def handler(_sig: int, _frame: object) -> None: - ctx.aborted = True + # A process started with SIGINT ignored (a background job of a + # non-interactive shell) keeps ignoring it; restore Python's default so + # that a takeover's SIGINT still stops this run. + signal.signal(signal.SIGINT, signal.default_int_handler) + try: + success = engine.execute_plan( + ctx, common, opts, checkpointer, services=Services.production() + ) + except KeyboardInterrupt: ctx.abort_reason = "User interrupted (Ctrl+C)" + # Atomic (a temporary file, then a rename): a second Ctrl+C here leaves + # the checkpoint taken as the step started, which resumes the same way. checkpointer.save(ctx) runtime.console.print( "\n[red bold]Interrupted! State saved. Resume with --resume.[/red bold]\n" @@ -258,8 +276,7 @@ def handler(_sig: int, _frame: object) -> None: print_status(ctx) _dispose_worktree(worktree, opts, succeeded=False) sys.exit(130) - - signal.signal(signal.SIGINT, handler) + _finish(ctx, checkpointer, worktree, opts, success=success) def _finish( @@ -422,16 +439,7 @@ def _run_fresh(common: cli.CommonArgs, opts: AutolandOptions) -> None: return runtime.console.print(f"[dim]State file: {checkpointer.path}[/dim]\n") - _install_signal_handler(ctx, checkpointer, worktree, opts) - _finish( - ctx, - checkpointer, - worktree, - opts, - success=engine.execute_plan( - ctx, common, opts, checkpointer, services=Services.production() - ), - ) + _land(ctx, common, opts, checkpointer, worktree) finally: if worktree is not None: worktree.remove_unless_landing() @@ -483,7 +491,6 @@ def _run_resume(common: cli.CommonArgs, opts: AutolandOptions) -> None: runtime.console.print("[dim]Refreshing PR state from GitHub...[/dim]") discovery.enrich_stack(ctx.stack) - ctx.aborted = False ctx.abort_reason = "" if ctx.current_step >= len(ctx.plan): @@ -497,16 +504,7 @@ def _run_resume(common: cli.CommonArgs, opts: AutolandOptions) -> None: print_status(ctx) engine.print_execution_preview(ctx, opts) runtime.console.print(f"[dim]State file: {sf_path}[/dim]\n") - _install_signal_handler(ctx, checkpointer, worktree, opts) - _finish( - ctx, - checkpointer, - worktree, - opts, - success=engine.execute_plan( - ctx, common, opts, checkpointer, services=Services.production() - ), - ) + _land(ctx, common, opts, checkpointer, worktree) finally: if worktree is not None: worktree.remove_unless_landing() @@ -721,16 +719,7 @@ def _replan(common: cli.CommonArgs, opts: AutolandOptions, state_path: Path) -> path=state_path, branch=branch, base=common.target, plan_file=plan_file ) runtime.console.print(f"[dim]State file: {checkpointer.path}[/dim]\n") - _install_signal_handler(ctx, checkpointer, worktree, opts) - _finish( - ctx, - checkpointer, - worktree, - opts, - success=engine.execute_plan( - ctx, common, opts, checkpointer, services=Services.production() - ), - ) + _land(ctx, common, opts, checkpointer, worktree) finally: if worktree is not None: worktree.remove_unless_landing() diff --git a/src/stack_pr/autoland/display.py b/src/stack_pr/autoland/display.py index ecff426..84a3e1d 100644 --- a/src/stack_pr/autoland/display.py +++ b/src/stack_pr/autoland/display.py @@ -74,8 +74,8 @@ def _pick_glyphs() -> _Glyphs: """The richest glyph set stdout can actually encode. Writing an un-encodable character raises UnicodeEncodeError, which would - take down the whole landing run — print_status is called from the SIGINT - handler and from _finish, so even a successful autoland would die on the + take down the whole landing run — print_status is called when a run is + interrupted and from _finish, so even a successful autoland would die on the way out, leaving its checkpoint and worktree behind. """ encoding = getattr(sys.stdout, "encoding", None) or "ascii" @@ -265,7 +265,7 @@ def render_status_rich(ctx: LandingContext) -> Group: # pragma: no cover # expand=False so the pad doesn't fill the rest of the line with # blanks; a wrapped detail still stays under the indent. parts.append(Padding(detail, (0, 0, 0, indent), expand=False)) - if ctx.aborted: + if ctx.abort_reason: parts += [Text(), Text(f"ABORTED: {ctx.abort_reason}", style="red bold")] return Group(*parts) @@ -281,7 +281,7 @@ def render_status_plain(ctx: LandingContext) -> str: lines.append("".join(text for text, _ in _header_segments(row, widths))) if row.detail: lines.append(f"{indent}{GLYPHS.detail} {row.detail}") - if ctx.aborted: + if ctx.abort_reason: lines += ["", f"ABORTED: {ctx.abort_reason}"] return "\n".join(lines) diff --git a/src/stack_pr/autoland/engine.py b/src/stack_pr/autoland/engine.py index cb5f56b..071abf8 100644 --- a/src/stack_pr/autoland/engine.py +++ b/src/stack_pr/autoland/engine.py @@ -117,6 +117,5 @@ def _abort( ctx: LandingContext, checkpointer: AutolandCheckpointer, reason: str ) -> bool: ctx.abort_reason = reason - ctx.aborted = True checkpointer.save(ctx) return False diff --git a/src/stack_pr/autoland/model.py b/src/stack_pr/autoland/model.py index 44add89..1d1ed18 100644 --- a/src/stack_pr/autoland/model.py +++ b/src/stack_pr/autoland/model.py @@ -68,15 +68,10 @@ class LandingContext: plan: list[PlanStep] = field(default_factory=list) current_step: int = 0 current_index: int = 0 # index into stack for the active land step - aborted: bool = False + # Why the run stopped, or "" while it hasn't. Saved with the checkpoint. abort_reason: str = "" last_landed_sha: str = "" # merge commit of the last landed PR - def abort_requested(self) -> bool: - # A method, not a plain attribute read, so that mypy does not carry - # narrowing of `aborted` across calls (or the SIGINT handler) that set it. - return self.aborted - def _deserialize_entry(data: dict) -> StackEntry: return StackEntry( diff --git a/src/stack_pr/autoland/native_stack.py b/src/stack_pr/autoland/native_stack.py index 8a02dd2..64f1ea3 100644 --- a/src/stack_pr/autoland/native_stack.py +++ b/src/stack_pr/autoland/native_stack.py @@ -83,7 +83,6 @@ def _await_native_stack_merge( uuid: str | None, *, opts: AutolandOptions, - ctx: LandingContext, ) -> str: """Wait for a requested stack merge to finish. @@ -94,8 +93,6 @@ def _await_native_stack_merge( top = entries[-1] awake_elapsed = 0.0 while True: - if ctx.aborted: - return "aborted" if awake_elapsed > opts.merge_timeout: return "timed out waiting for the stack to merge" @@ -184,14 +181,16 @@ def land_as_native_stack( failure = "" abort_reason = "" for entry in entries: - if not waits.wait_for_approval(entry, opts=opts, ctx=ctx): - abort_reason = f"PR #{entry.pr_number} approval wait was aborted" + if not waits.wait_for_approval(entry, opts=opts): + abort_reason = ( + f"PR #{entry.pr_number} was closed while waiting for approval" + ) break else: for entry in entries: if entry.state == PRState.MERGED: continue - if not waits.wait_for_checks(entry, opts=opts, ctx=ctx): + if not waits.wait_for_checks(entry, opts=opts): abort_reason = f"PR #{entry.pr_number} checks failed after retries" break @@ -202,7 +201,7 @@ def land_as_native_stack( if ( not abort_reason and bottom is not None - and not waits.wait_for_mergeable(bottom, opts=opts, ctx=ctx).ready + and not waits.wait_for_mergeable(bottom, opts=opts).ready ): abort_reason = f"PR #{bottom.pr_number} failed to merge" @@ -246,9 +245,7 @@ def land_as_native_stack( if "HTTP 409" not in str(e): failure = f"stack merge request failed: {e}" if not failure: - failure = _await_native_stack_merge(entries, uuid, opts=opts, ctx=ctx) - if failure == "aborted": - abort_reason = "Stack merge was aborted" + failure = _await_native_stack_merge(entries, uuid, opts=opts) landed = all(e.state == PRState.MERGED for e in entries) if not landed and dissolvable: diff --git a/src/stack_pr/autoland/services.py b/src/stack_pr/autoland/services.py index 12d3eaa..fbd48b3 100644 --- a/src/stack_pr/autoland/services.py +++ b/src/stack_pr/autoland/services.py @@ -43,21 +43,18 @@ def escape(self, text: str) -> str: class Waits(Protocol): """The long-running waits of landing a PR or checking a workflow. - Each returns whether it succeeded; False also when the run was aborted. - The waits record their progress on the entry or step they are given. + Each returns whether it succeeded, and records its progress on the entry + or step it is given. Ctrl+C while one is waiting raises KeyboardInterrupt + out of it, which the run handles at its boundary (``commands._land``). """ def wait_for_approval( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext + self, entry: StackEntry, *, opts: AutolandOptions ) -> bool: ... - def wait_for_checks( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: ... + def wait_for_checks(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: ... - def enqueue_and_wait( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: ... + def enqueue_and_wait(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: ... def wait_for_workflow( self, @@ -136,20 +133,14 @@ def escape(self, text: str) -> str: class _ModuleWaits: - def wait_for_approval( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: - return waits.wait_for_approval(entry, opts=opts, ctx=ctx) + def wait_for_approval(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: + return waits.wait_for_approval(entry, opts=opts) - def wait_for_checks( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: - return waits.wait_for_checks(entry, opts=opts, ctx=ctx) + def wait_for_checks(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: + return waits.wait_for_checks(entry, opts=opts) - def enqueue_and_wait( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: - return waits.enqueue_and_wait(entry, opts=opts, ctx=ctx) + def enqueue_and_wait(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: + return waits.enqueue_and_wait(entry, opts=opts) def wait_for_workflow( self, diff --git a/src/stack_pr/autoland/state.py b/src/stack_pr/autoland/state.py index f94d2af..833b321 100644 --- a/src/stack_pr/autoland/state.py +++ b/src/stack_pr/autoland/state.py @@ -72,7 +72,6 @@ def load(cls, path: Path) -> tuple[AutolandCheckpointer, LandingContext]: stack=[_deserialize_entry(e) for e in data["stack"]], plan=[_deserialize_step(s) for s in data["plan"]], current_step=data.get("current_step", 0), - aborted=bool(data.get("abort_reason")), abort_reason=data.get("abort_reason", ""), last_landed_sha=data.get("last_landed_sha", ""), ) diff --git a/src/stack_pr/autoland/steps/land.py b/src/stack_pr/autoland/steps/land.py index 13a3c4f..3b0738a 100644 --- a/src/stack_pr/autoland/steps/land.py +++ b/src/stack_pr/autoland/steps/land.py @@ -168,20 +168,18 @@ def execute(self, sc: StepContext) -> StepResult: ) ui.print(sc.banner(f"Landing PR #{entry.pr_number} — {entry.title}")) - waits, opts, run = sc.services.waits, sc.opts, sc.run + waits, opts = sc.services.waits, sc.opts # Each wait may find the PR merged by then (e.g. by hand), which ends # the landing early. - if not waits.wait_for_approval(entry, opts=opts, ctx=run): - return StepResult.abort(f"PR #{entry.pr_number} approval wait was aborted") - if not entry.is_merged() and not waits.wait_for_checks( - entry, opts=opts, ctx=run - ): + if not waits.wait_for_approval(entry, opts=opts): + return StepResult.abort( + f"PR #{entry.pr_number} was closed while waiting for approval" + ) + if not entry.is_merged() and not waits.wait_for_checks(entry, opts=opts): return StepResult.abort( f"PR #{entry.pr_number} checks failed after retries" ) - if not entry.is_merged() and not waits.enqueue_and_wait( - entry, opts=opts, ctx=run - ): + if not entry.is_merged() and not waits.enqueue_and_wait(entry, opts=opts): return StepResult.abort(f"PR #{entry.pr_number} failed to merge") landed_sha = repo.landed_sha(sc.common, entry.pr_number) diff --git a/src/stack_pr/autoland/waits.py b/src/stack_pr/autoland/waits.py index 79521a9..609fdf1 100644 --- a/src/stack_pr/autoland/waits.py +++ b/src/stack_pr/autoland/waits.py @@ -185,8 +185,6 @@ def wait_for_workflow( ancestry = _Ancestry(common) awake_elapsed = 0.0 while True: - if ctx.aborted: - return False if awake_elapsed > opts.workflow_timeout: step.state = "failed" step.error_message = ( @@ -274,10 +272,9 @@ def _refresh_review(entry: StackEntry) -> bool: return entry.is_approved -def wait_for_approval( - entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext -) -> bool: - """Wait until the PR has required approvals. Returns False if aborted.""" +def wait_for_approval(entry: StackEntry, *, opts: AutolandOptions) -> bool: + """Wait until the PR has required approvals (or merged). Returns False if + the PR was closed.""" # An unknown state (a failed read) is settled by the checks wait after this. pr_state = _poll_read(gh.github.pr_state, entry.pr_number) if pr_state == "MERGED": @@ -300,8 +297,6 @@ def wait_for_approval( ) while True: - if ctx.aborted: - return False runtime.console.print( f"[dim]PR #{entry.pr_number}: waiting for approval — " f"polling in {opts.poll_interval}s[/dim]" @@ -333,16 +328,11 @@ def wait_for_approval( ) -def wait_for_checks( - entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext -) -> bool: +def wait_for_checks(entry: StackEntry, *, opts: AutolandOptions) -> bool: """Wait for required checks to pass. Returns True on success.""" entry.state = PRState.WAITING_FOR_CHECKS while True: - if ctx.aborted: - return False - pr_state = _poll_read(gh.github.pr_state, entry.pr_number) if pr_state == "MERGED": entry.state = PRState.MERGED @@ -398,14 +388,9 @@ class MergeableResult: error: str = "" -def wait_for_mergeable( - entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext -) -> MergeableResult: +def wait_for_mergeable(entry: StackEntry, *, opts: AutolandOptions) -> MergeableResult: """Wait until GitHub reports the PR as mergeable.""" while True: - if ctx.aborted: - return MergeableResult(error="aborted") - data = _poll_read(gh.github.merge_state, entry.pr_number) if data is None: runtime.resilient_sleep(opts.poll_interval) @@ -460,25 +445,18 @@ def wait_for_mergeable( runtime.resilient_sleep(opts.poll_interval) -def _rewait_after_retry( - entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext -) -> bool: +def _rewait_after_retry(entry: StackEntry, *, opts: AutolandOptions) -> bool: """Re-verify approval and checks before a queue retry.""" - if not wait_for_approval(entry, opts=opts, ctx=ctx): + if not wait_for_approval(entry, opts=opts): return False entry.check_retries = 0 - return wait_for_checks(entry, opts=opts, ctx=ctx) + return wait_for_checks(entry, opts=opts) -def enqueue_and_wait( - entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext -) -> bool: +def enqueue_and_wait(entry: StackEntry, *, opts: AutolandOptions) -> bool: """Add the PR to the merge queue and wait for it to merge.""" while True: - if ctx.aborted: - return False - - mergeable_result = wait_for_mergeable(entry, opts=opts, ctx=ctx) + mergeable_result = wait_for_mergeable(entry, opts=opts) if not mergeable_result.ready: return False if mergeable_result.already_merged: @@ -507,15 +485,13 @@ def enqueue_and_wait( ) return False entry.queue_retries += 1 - if not _rewait_after_retry(entry, opts=opts, ctx=ctx): + if not _rewait_after_retry(entry, opts=opts): return False continue entry.error_message = "Waiting in merge queue..." awake_elapsed = 0.0 while True: - if ctx.abort_requested(): - return False if awake_elapsed > opts.merge_timeout: entry.state = PRState.FAILED entry.error_message = "Timed out waiting for merge queue" @@ -549,7 +525,7 @@ def enqueue_and_wait( ) return False entry.queue_retries += 1 - if not _rewait_after_retry(entry, opts=opts, ctx=ctx): + if not _rewait_after_retry(entry, opts=opts): return False break # re-enqueue in outer loop diff --git a/tests/autoland_scenarios.py b/tests/autoland_scenarios.py index 03c2c0a..266db2b 100644 --- a/tests/autoland_scenarios.py +++ b/tests/autoland_scenarios.py @@ -22,7 +22,7 @@ - stack-pr itself: ``cli.get_stack`` returns the scenario's stack, ``cli.command_submit`` (the resubmit after a merge) does nothing, and ``git.get_current_branch_name`` is pinned; -- the SIGINT handler isn't installed, and any real subprocess fails the test. +- any real subprocess fails the test. """ from __future__ import annotations @@ -435,7 +435,6 @@ def run_shell_command(cmd: Any, **kwargs: Any) -> subprocess.CompletedProcess: mocker.patch.object(cli, "get_stack", return_value=raw_stack) mocker.patch.object(cli, "command_submit") mocker.patch("stack_pr.git.get_current_branch_name", return_value=BRANCH) - mocker.patch.object(commands.signal, "signal") def _autoland(argv: list[str], config: configparser.ConfigParser) -> int | None: diff --git a/tests/test_autoland.py b/tests/test_autoland.py index 6620ece..15f3eda 100644 --- a/tests/test_autoland.py +++ b/tests/test_autoland.py @@ -354,11 +354,14 @@ def test_wait_for_workflow_falls_back_to_github_compare(mocker) -> None: # noqa contains.assert_called_once_with("mergesha", "queuesha") +class _StopPollingError(Exception): + """Raised from a wait's sleep to end the wait after one poll.""" + + def test_wait_for_workflow_keeps_waiting_when_ancestry_unknown(mocker) -> None: # noqa: ANN001 # Neither git nor GitHub can answer: warn and keep waiting rather than # treating "unknown" as "the deploy included our code". def _runs(*_a, **_k) -> list: # noqa: ANN002, ANN003 - ctx.aborted = True # stop after one poll return [{"headSha": "mystery", "status": "completed", "conclusion": "success"}] mocker.patch.object(gh.github, "workflow_runs", side_effect=_runs) @@ -366,12 +369,12 @@ def _runs(*_a, **_k) -> list: # noqa: ANN002, ANN003 mocker.patch.object(runtime, "run", return_value=unknown) # the fetch mocker.patch("stack_pr.git.run_shell_command", return_value=unknown) mocker.patch.object(gh.github, "contains", return_value=None) - mocker.patch.object(runtime, "resilient_sleep", return_value=0.0) + mocker.patch.object(runtime, "resilient_sleep", side_effect=_StopPollingError) step = WorkflowStep(workflow="deploy.yaml") ctx = LandingContext(last_landed_sha="mergesha") - assert not waits.wait_for_workflow( - step, opts=_opts(), common=common_args(), ctx=ctx - ) + # Still waiting when the first poll's sleep comes around. + with pytest.raises(_StopPollingError): + waits.wait_for_workflow(step, opts=_opts(), common=common_args(), ctx=ctx) assert step.state != "succeeded" @@ -427,24 +430,21 @@ def test_ancestry_caches_verdicts(mocker) -> None: # noqa: ANN001 def test_wait_for_workflow_ignores_failed_and_incomplete(mocker) -> None: # noqa: ANN001 # A failed run and a still-running run on our SHA must not satisfy the - # checkpoint; abort so the poll loop terminates for the test. - calls = {"n": 0} - - def _runs(*_a, **_k) -> list: # noqa: ANN002, ANN003 - calls["n"] += 1 - ctx.aborted = True # stop after one poll - return [ + # checkpoint: the wait is still going when its first poll's sleep comes. + mocker.patch.object( + gh.github, + "workflow_runs", + return_value=[ {"headSha": "mergesha", "status": "completed", "conclusion": "failure"}, {"headSha": "mergesha", "status": "in_progress", "conclusion": None}, - ] - - mocker.patch.object(gh.github, "workflow_runs", side_effect=_runs) - mocker.patch.object(runtime, "resilient_sleep", return_value=0.0) + ], + ) + mocker.patch.object(runtime, "resilient_sleep", side_effect=_StopPollingError) step = WorkflowStep(workflow="deploy.yaml") ctx = LandingContext(last_landed_sha="mergesha") - assert not waits.wait_for_workflow( - step, opts=_opts(), common=common_args(), ctx=ctx - ) + with pytest.raises(_StopPollingError): + waits.wait_for_workflow(step, opts=_opts(), common=common_args(), ctx=ctx) + assert step.state != "succeeded" # --- plan parsing -------------------------------------------------------- @@ -940,7 +940,6 @@ def test_render_status_plain_headline_counts_completed_steps() -> None: def test_render_status_plain_reports_an_abort() -> None: ctx = _partly_landed_ctx() - ctx.aborted = True ctx.abort_reason = "PR #103 checks failed" assert "ABORTED: PR #103 checks failed" in display.render_status_plain(ctx) @@ -997,7 +996,7 @@ def test_unicode_glyphs_are_used_on_a_utf8_stdout(mocker) -> None: # noqa: ANN0 def test_plain_status_is_encodable_on_a_legacy_code_page(mocker) -> None: # noqa: ANN001 # Printing an un-encodable glyph raises UnicodeEncodeError, and print_status - # runs from the SIGINT handler and from _finish — so even a successful run + # runs when a run is interrupted and from _finish — so even a successful run # would die on the way out, stranding its checkpoint and worktree. mocker.patch.object(display, "GLYPHS", _ASCII_GLYPHS) display.render_status_plain(_partly_landed_ctx()).encode("cp1252") @@ -1055,7 +1054,6 @@ def test_rich_status_truncates_titles_but_never_the_failure_reason() -> None: "integration-tests, docs, and a few more besides" ) ctx = LandingContext(stack=stack, plan=[LandStep(entry_index=0, pr_number=101)]) - ctx.aborted = True ctx.abort_reason = ( "Rebase failed after merging #101: Command failed: git rebase --onto " "origin/main abc123 def456 -- error: could not apply def456" @@ -1189,14 +1187,12 @@ def test_state_round_trip_keeps_plan_file(tmp_path) -> None: # noqa: ANN001 def test_state_round_trip_keeps_abort_reason(tmp_path) -> None: # noqa: ANN001 ctx = LandingContext(stack=_stack(1), plan=generate_default_plan(_stack(1))) - ctx.aborted = True ctx.abort_reason = "CI failed on #0" sf = tmp_path / "state.json" AutolandCheckpointer(path=sf, branch="feat", base="main").save(ctx) _, loaded = AutolandCheckpointer.load(sf) - assert loaded.aborted is True assert loaded.abort_reason == "CI failed on #0" @@ -1731,7 +1727,6 @@ def _patch_replan_io(mocker, answer: str): # noqa: ANN001, ANN202 "stack_pr.autoland.gh.github.pr_state", side_effect=lambda pr: "MERGED" if pr in (101, 102) else "OPEN", ) - mocker.patch("stack_pr.autoland.commands.signal.signal") return mocker.patch("stack_pr.autoland.engine.execute_plan", return_value=True) @@ -1797,7 +1792,6 @@ def _patch_resume_io(mocker, *, success: bool = True): # noqa: ANN001, ANN202 mocker.patch("stack_pr.git.get_current_branch_name", return_value="feat") mocker.patch("stack_pr.cli.deduce_base", side_effect=lambda c: c) mocker.patch("stack_pr.autoland.discovery.enrich_stack") - mocker.patch("stack_pr.autoland.commands.signal.signal") return mocker.patch("stack_pr.autoland.engine.execute_plan", return_value=success) @@ -1816,7 +1810,6 @@ def test_resume_continues_from_the_checkpointed_step(tmp_path, mocker) -> None: assert ctx.current_step == 1 assert [e.pr_number for e in ctx.stack] == [0, 1] # The earlier failure no longer stands: this run gets a fresh attempt. - assert not ctx.aborted assert ctx.abort_reason == "" # A run that finishes leaves no checkpoint behind. assert not sf.exists() @@ -1962,15 +1955,17 @@ def test_wait_for_approval_by_review_decision( ) -> None: mocker.patch.object(gh.github, "pr_state", return_value="OPEN") mocker.patch.object(gh.github, "review_decision", return_value=decision) - ctx = LandingContext() - # Abort the wait on its first poll, so a PR that isn't approved returns. mocker.patch( - "stack_pr.autoland.runtime.resilient_sleep", - side_effect=lambda _s: setattr(ctx, "aborted", True), + "stack_pr.autoland.runtime.resilient_sleep", side_effect=_StopPollingError ) entry = _pinned_stack([101])[0] - assert waits.wait_for_approval(entry, opts=_opts(), ctx=ctx) is approved + if approved: + assert waits.wait_for_approval(entry, opts=_opts()) + else: + # A PR that isn't approved is still waited on at the first poll's sleep. + with pytest.raises(_StopPollingError): + waits.wait_for_approval(entry, opts=_opts()) # --- merging a run of land steps as a GitHub stack ------------------------- @@ -2287,7 +2282,7 @@ def test_wait_for_checks_polls_again_after_a_failed_state_read( _gh_replies(mocker, view=[bad_reply, _OPEN], checks=[_PASSING]) entry = _pinned_stack([101])[0] - assert waits.wait_for_checks(entry, opts=_opts(), ctx=LandingContext()) + assert waits.wait_for_checks(entry, opts=_opts()) @pytest.mark.usefixtures("autoland_console") @@ -2295,7 +2290,7 @@ def test_wait_for_checks_polls_again_after_a_malformed_checks_read(mocker) -> No _gh_replies(mocker, view=[_OPEN], checks=['{"oops": 1}', "[1]", _PASSING]) entry = _pinned_stack([101])[0] - assert waits.wait_for_checks(entry, opts=_opts(), ctx=LandingContext()) + assert waits.wait_for_checks(entry, opts=_opts()) @pytest.mark.usefixtures("autoland_console") @@ -2312,7 +2307,7 @@ def test_wait_for_approval_polls_again_after_a_failed_state_read(mocker) -> None ) entry = _pinned_stack([101])[0] - assert waits.wait_for_approval(entry, opts=_opts(), ctx=LandingContext()) + assert waits.wait_for_approval(entry, opts=_opts()) assert entry.state == PRState.MERGED @@ -2322,7 +2317,7 @@ def test_wait_for_mergeable_polls_again_after_a_malformed_read(mocker) -> None: _gh_replies(mocker, view=["", RuntimeError("timed out"), clean]) entry = _pinned_stack([101])[0] - result = waits.wait_for_mergeable(entry, opts=_opts(), ctx=LandingContext()) + result = waits.wait_for_mergeable(entry, opts=_opts()) assert result.ready @@ -2345,7 +2340,7 @@ def test_merge_queue_wait_polls_again_after_a_failed_read(mocker) -> None: # no ) entry = _pinned_stack([101])[0] - assert waits.enqueue_and_wait(entry, opts=_opts(), ctx=LandingContext()) + assert waits.enqueue_and_wait(entry, opts=_opts()) assert entry.state == PRState.MERGED @@ -2355,7 +2350,7 @@ def test_merge_queue_wait_still_times_out_while_reads_fail(mocker) -> None: # n entry = _pinned_stack([101])[0] opts = _opts(poll_interval=60, merge_timeout=300) - assert not waits.enqueue_and_wait(entry, opts=opts, ctx=LandingContext()) + assert not waits.enqueue_and_wait(entry, opts=opts) assert entry.error_message == "Timed out waiting for merge queue" diff --git a/tests/test_autoland_golden.py b/tests/test_autoland_golden.py index fc9e787..caff3ee 100644 --- a/tests/test_autoland_golden.py +++ b/tests/test_autoland_golden.py @@ -64,7 +64,6 @@ def _resume_checkpoint( stack=stack, plan=plan, current_step=4, - aborted=True, abort_reason="Workflow deploy.yaml failed or timed out", last_landed_sha=sha("merge-102"), ) @@ -104,7 +103,6 @@ def _replan_checkpoint( plan=plan, current_step=7, current_index=2, - aborted=True, abort_reason="User interrupted (Ctrl+C)", last_landed_sha=sha("merge-102"), ) diff --git a/tests/test_autoland_interrupt.py b/tests/test_autoland_interrupt.py new file mode 100644 index 0000000..3f6d7d5 --- /dev/null +++ b/tests/test_autoland_interrupt.py @@ -0,0 +1,257 @@ +"""Interrupting a landing run: Ctrl+C, or a takeover's SIGINT. + +Either way the run sees KeyboardInterrupt wherever it happens to be (a wait, +a confirm prompt), and the run boundary checkpoints it, exits 130, and +releases the lock, so ``--resume`` carries on from the interrupted step. +""" + +from __future__ import annotations + +import subprocess +import sys +from dataclasses import replace +from pathlib import Path +from unittest.mock import Mock + +import pytest +from pytest_mock import MockerFixture + +from stack_pr.autoland.commands import _run_resume, _stop_running_autoland +from stack_pr.autoland.model import ( + ConfirmStep, + LandingContext, + LandStep, + PRState, + StackEntry, +) +from stack_pr.autoland.services import Services +from stack_pr.autoland.state import AutolandCheckpointer, AutolandLock +from stack_pr.autoland.steps import AutolandStep +from stack_pr.autoland.worktree import Worktree +from tests.helpers import common_args +from tests.test_autoland_step_execution import ( + OPTS, + FakeRepo, + FakeUI, + FakeWaits, + services, + stack, +) + +INTERRUPTED = "User interrupted (Ctrl+C)" +REPO_ROOT = Path(__file__).resolve().parents[1] + + +def _save(path: Path, plan: list[AutolandStep]) -> None: + ctx = LandingContext(stack=stack(101, 102), plan=plan) + AutolandCheckpointer(path=path, branch="feat", base="main").save(ctx) + + +def _land_both() -> list[AutolandStep]: + return [ + LandStep(entry_index=0, pr_number=101), + LandStep(entry_index=1, pr_number=102), + ] + + +def _waits(approved: list[int], *, interrupt_at: int | None = None) -> FakeWaits: + """Waits that record each approved PR, and see Ctrl+C while waiting for + *interrupt_at*'s approval.""" + + def approval(entry: StackEntry) -> bool: + if entry.pr_number == interrupt_at: + raise KeyboardInterrupt + approved.append(entry.pr_number) + return True + + return FakeWaits(approval=approval) + + +class InterruptedUI(FakeUI): + """Ctrl+C at the first prompt.""" + + def input(self, prompt: str) -> str: + self.lines.append(prompt) + raise KeyboardInterrupt + + +@pytest.fixture +def resume_io(mocker: MockerFixture) -> Mock: + """Stub the git side of a resume; return the Services.production mock.""" + mocker.patch("stack_pr.git.get_current_branch_name", return_value="feat") + mocker.patch("stack_pr.cli.deduce_base", side_effect=lambda c: c) + mocker.patch("stack_pr.autoland.discovery.enrich_stack") + return mocker.patch.object(Services, "production") + + +@pytest.fixture +def worktree_events(mocker: MockerFixture) -> list[str]: + """Replace the --branch worktree with one that records what happens to it.""" + events: list[str] = [] + + class FakeWorktree(Worktree): + def create(self) -> None: + self.path = Path("worktree") + + def remove(self) -> None: + events.append("removed") + + def announce_preserved(self) -> None: + events.append("preserved") + + mocker.patch("stack_pr.autoland.commands.Worktree", FakeWorktree) + return events + + +def _resume(sf: Path, **overrides: object) -> None: + _run_resume(common_args(), replace(OPTS, resume=True, state_file=sf, **overrides)) + + +def _printed(console: Mock) -> str: + return "\n".join(str(a) for c in console.print.call_args_list for a in c.args) + + +def _assert_interrupted(sf: Path, console: Mock, *, at_step: int) -> LandingContext: + _, saved = AutolandCheckpointer.load(sf) + assert saved.abort_reason == INTERRUPTED + assert saved.current_step == at_step + assert "Interrupted! State saved. Resume with --resume." in _printed(console) + # The lock is free for the --resume. + lock = AutolandLock.for_state(sf) + assert lock.acquire() + lock.release() + return saved + + +@pytest.mark.parametrize( + ("always_cleanup", "worktree"), + [(False, ["preserved"]), (True, ["removed"])], + ids=["keeps-worktree", "always-cleanup"], +) +def test_ctrl_c_during_a_wait_checkpoints_and_exits_130( + tmp_path: Path, + autoland_console: Mock, + resume_io: Mock, + *, + worktree_events: list[str], + always_cleanup: bool, + worktree: list[str], +) -> None: + sf = tmp_path / "state.json" + _save(sf, _land_both()) + resume_io.return_value = services( + waits=_waits([], interrupt_at=102), repo=FakeRepo() + ) + + with pytest.raises(SystemExit) as exc: + _resume(sf, branch="feat", always_cleanup=always_cleanup) + + assert exc.value.code == 130 + saved = _assert_interrupted(sf, autoland_console, at_step=1) + assert saved.stack[0].state == PRState.MERGED + # Kept after an interruption, like after any failure, unless asked not to. + assert worktree_events == worktree + + +def test_ctrl_c_at_a_confirm_prompt_checkpoints_and_exits_130( + tmp_path: Path, autoland_console: Mock, resume_io: Mock +) -> None: + sf = tmp_path / "state.json" + _save(sf, [LandStep(entry_index=0, pr_number=101), ConfirmStep()]) + resume_io.return_value = services(InterruptedUI(), repo=FakeRepo()) + + with pytest.raises(SystemExit) as exc: + _resume(sf) + + assert exc.value.code == 130 + saved = _assert_interrupted(sf, autoland_console, at_step=1) + confirm = saved.plan[1] + assert isinstance(confirm, ConfirmStep) + assert not confirm.confirmed + + +@pytest.mark.usefixtures("autoland_console") +def test_resume_after_ctrl_c_continues_from_the_interrupted_step( + tmp_path: Path, resume_io: Mock +) -> None: + sf = tmp_path / "state.json" + _save(sf, _land_both()) + approved: list[int] = [] + resume_io.return_value = services( + waits=_waits(approved, interrupt_at=102), repo=FakeRepo() + ) + with pytest.raises(SystemExit): + _resume(sf) + assert approved == [101] + + resume_io.return_value = services(waits=_waits(approved), repo=FakeRepo()) + _resume(sf) + + # #101 isn't landed again; #102 is landed from the start of its step. + assert approved == [101, 102] + assert not sf.exists() + + +# A run blocked in a wait in its own process, started with SIGINT ignored the +# way a non-interactive shell starts a background job. +_BLOCKED_RUN = """ +import signal, sys, time +from dataclasses import replace +from pathlib import Path +from unittest import mock + +signal.signal(signal.SIGINT, signal.SIG_IGN) + +from stack_pr.autoland import commands +from stack_pr.autoland.services import Services +from tests.helpers import common_args +from tests.test_autoland_step_execution import OPTS, FakeRepo, FakeWaits, services + +def block(entry): + print("waiting", flush=True) + time.sleep(60) + return True + +sf = Path({path!r}) +with mock.patch("stack_pr.git.get_current_branch_name", return_value="feat"), \\ + mock.patch("stack_pr.cli.deduce_base", side_effect=lambda c: c), \\ + mock.patch("stack_pr.autoland.discovery.enrich_stack"), \\ + mock.patch.object(Services, "production", + return_value=services(waits=FakeWaits(approval=block), + repo=FakeRepo())): + commands._run_resume(common_args(), replace(OPTS, resume=True, state_file=sf)) +""" + + +def test_takeover_stops_a_run_blocked_in_a_wait( + tmp_path: Path, autoland_console: Mock +) -> None: + sf = tmp_path / "state.json" + _save(sf, _land_both()) + run = subprocess.Popen( + [sys.executable, "-c", _BLOCKED_RUN.format(path=str(sf))], + cwd=REPO_ROOT, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + ) + try: + assert run.stdout is not None + for line in run.stdout: + if line.strip() == "waiting": + break + autoland_console.input.return_value = "y" + lock = AutolandLock.for_state(sf) + + assert _stop_running_autoland(lock) + lock.release() + + output, _ = run.communicate(timeout=10) + assert run.returncode == 130, output + assert "Interrupted! State saved. Resume with --resume." in output + _, saved = AutolandCheckpointer.load(sf) + assert saved.abort_reason == INTERRUPTED + assert saved.current_step == 0 + finally: + run.kill() + run.wait() diff --git a/tests/test_autoland_optimize.py b/tests/test_autoland_optimize.py index f4f9e91..47bca31 100644 --- a/tests/test_autoland_optimize.py +++ b/tests/test_autoland_optimize.py @@ -282,11 +282,15 @@ def test_a_stack_merge_that_falls_through_lands_the_rest_one_at_a_time( def test_a_stack_merge_abort_stops_before_the_step(mocker) -> None: # noqa: ANN001 run = LandingContext(stack=stack(101, 102), plan=land(0, 1)) _stack_merge( - mocker, [NativeStackMergeResult(abort_reason="Stack merge was aborted")] + mocker, + [NativeStackMergeResult(abort_reason="Rebase failed after merging #102: boom")], ) recorder = _Recorder(run) - assert _unit(run).execute(recorder.context()) == "Stack merge was aborted" + assert ( + _unit(run).execute(recorder.context()) + == "Rebase failed after merging #102: boom" + ) assert recorder.events == [("enter", 0)] diff --git a/tests/test_autoland_step_execution.py b/tests/test_autoland_step_execution.py index 405e18c..b8073bb 100644 --- a/tests/test_autoland_step_execution.py +++ b/tests/test_autoland_step_execution.py @@ -105,21 +105,15 @@ class FakeWaits: calls: list[str] = field(default_factory=list) workflow_targets: list[str] = field(default_factory=list) - def wait_for_approval( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: + def wait_for_approval(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: self.calls.append("approval") return self.approval(entry) - def wait_for_checks( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: + def wait_for_checks(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: self.calls.append("checks") return self.checks(entry) - def enqueue_and_wait( - self, entry: StackEntry, *, opts: AutolandOptions, ctx: LandingContext - ) -> bool: + def enqueue_and_wait(self, entry: StackEntry, *, opts: AutolandOptions) -> bool: self.calls.append("queue") return self.queue(entry) @@ -294,7 +288,7 @@ def test_already_landed_step_reports_its_sha_only_at_the_end_of_the_prefix( @pytest.mark.parametrize( ("waits", "reason"), [ - (FakeWaits(approval=fails), "PR #101 approval wait was aborted"), + (FakeWaits(approval=fails), "PR #101 was closed while waiting for approval"), (FakeWaits(checks=fails), "PR #101 checks failed after retries"), (FakeWaits(queue=fails), "PR #101 failed to merge"), ], @@ -454,7 +448,6 @@ def test_engine_records_the_reason_a_step_aborted(tmp_path: Path) -> None: ctx, common_args(), OPTS, checkpointer, services=services(FakeUI()) ) - assert ctx.aborted assert ctx.abort_reason.startswith("Confirm step received EOF") _, saved = AutolandCheckpointer.load(checkpointer.path) assert saved.abort_reason == ctx.abort_reason