The Refactoring Sisyphus

Alex Streed merged PR #21252 last week — a +731/-121 diff across 27 files titled “Remove internal Runner usage from bundle execution and CLI paths.” The mission: stop instantiating a full Runner (with its deployment polling, scheduled run polling, and limit management circus) just to execute a single flow run. Replace it with a leaner FlowRunExecutorContext. A reasonable goal. The execution, however, was a twelve-commit odyssey where the first commit does the work and the next eleven apologize for it.

The commit log reads like a post-incident timeline. “Make execute_bundle public for use by integration libraries.” “Fix circular import in bundles/execute.py.” “Suppress Runner deprecation warnings globally in pytest config.” “Use PrefectDeprecationWarning and date-based deprecation messages.” “Fix bad renames in integration bundle test files.” “Address review feedback: fix observer memory leak and stub flow resolver.” “Remove websocket dedup from observer to allow cancellation retries.” “Let transient errors propagate from propose_submitting.” “Install SIGTERM reschedule handler before process start.” That’s not iterative development. That’s playing whack-a-mole on main with an audience.

But the crown jewel — the moment that elevates this PR from “messy” to “legendary” — is the find-and-replace incident. Somewhere in the renaming spree, test_execute_bundle_from_s3 became testexecute_bundle_from_s3, and mock_runner became mockexecute_bundle. Pytest discovers test methods by the test_ prefix. Without the underscore, these aren’t tests anymore — they’re just methods, sitting in the test file, doing nothing, bothering no one. CI passed with flying colors. Of course it did; it’s easy to pass tests you aren’t running. Reviewer chrisguidry spotted it: “Ooops these aren’t getting run.” Alex’s response — “Good eye! That’s whack…” — is the cadence of a man who just realized his green CI badge was a participation trophy.

Then there’s the cancellation deduplication saga, which deserves its own short film. Alex added websocket dedup to FlowRunCancellingObserver. The Codex bot flagged it as a memory leak. Alex scoped it to in-flight runs. The bot said that would prevent cancellation retries. Alex removed the dedup entirely. The bot then flagged the absence of dedup as allowing duplicate cancellations. Alex, now visibly done, pointed out that the bot was contradicting its own earlier review. He’s right, of course. But the net diff of the dedup feature across four commits is zero lines changed. Four commits to arrive exactly where you started is not refactoring — it’s a round trip.

To Alex’s credit — and I say this through gritted teeth — the architectural instinct here is correct. Spinning up a fourteen-service Runner to execute one flow run is like booking a cruise ship to cross a river. FlowRunExecutorContext is a genuinely cleaner abstraction that manages exactly the components a one-shot execution needs. Alex understands the Runner’s guts better than anyone, which is why he’s the only person who could perform this surgery. Unfortunately, it’s also why the surgery took twelve incisions when it should have taken one.

Final Verdict: The Refactoring Sisyphus — ships the boulder, watches it roll back, ships it again