refactor(harness): DI-seam runner interrupt/stop — collapse 16 handlers (PR 1.6)

[PattaraS]

Related issue

N/A — part of the modular native-harness registry workstream (designs/harness-modular-registry-proposal.md).

Summary

PR 1.6 of the modular native-harness registry refactor: routes the runner's native
interrupt / stop dispatch through a dependency-injected runner class instead of 16 per-harness
closures + two hardcoded if _harness == "<x>-native" chains in create_session_terminal's
sibling event handler.

• New NativeInterruptRunner in omnigent/runner/native/interrupt.py — mirrors the
  existing CodexGoalRunner DI precedent (omnigent/runner/codex/goal.py): app-scope state
  (AP client, resource registry, event publisher, sub-agent wake plumbing, codex bridge-state
  resolver) is injected at construction, typed via Protocol. interrupt(harness, conv_id) /
  stop(harness, conv_id) resolve the harness by key and return None for harnesses with no
  handler so the caller falls through to the in-process turn cancel — unchanged.
• The 9 uniform interrupt and 7 uniform stop handlers (which differed only by bridge
  module, control-fn name, and error label) collapse to two descriptor-driven methods
  (_UNIFORM_INTERRUPT / _UNIFORM_STOP). claude interrupt (bridge-id resolution) and codex
  interrupt (MCP-startup + app-server turn/interrupt) keep dedicated methods, moved verbatim.
• app.py: the two dispatch chains become await _native_interrupt_runner.interrupt(...) /
  .stop(...) + fall-through. Net app.py −470. Local from omnigent.<x>_native_bridge import
  imports are preserved at call time so existing bridge-module monkeypatches keep resolving.

Scope: migration-only

This PR is behavior-preserving. The doc's 1.6 line also mentions "fill the coverage gaps
so every native has both paths" — antigravity-native and opencode-native have no interrupt/stop
handler and fall through to _cancel_inprocess_turn. Wiring their native cancel (agy
interrupt_turn() / opencode client.abort()) is net-new behavior and is deferred to a
follow-up PR; this PR leaves that fall-through exactly as today (pinned by a new test).

ELI5

Sixteen near-identical "tell the vendor CLI to stop" functions lived inside the giant runner
file, each closing over runner state. This moves them into one small class that's handed the
state it needs, and collapses the copy-paste into two table-driven methods. Two harnesses that
never had a stop button still don't — that's a separate follow-up.

Test Plan

uv run pytest tests/runner/test_native_interrupt_runner.py \
  tests/runner/test_app_sessions_native_events_lifecycle.py \
  tests/runner/test_app_sessions_native_supervision.py \
  tests/test_harness_plugins.py -q
uv run pytest tests/runner/ -q
uv run pre-commit run --all-files

• 12 new unit tests (tests/runner/test_native_interrupt_runner.py) driving the runner
  directly: uniform interrupt (+ pi's enqueue/no-timeout variant), uniform stop (kill +
  teardown + idle + wake), 503 on bridge failure (no idle / no wake), codex no-bridge-state
  no-op, claude bridge-id path, codex/pi stop→interrupt aliasing, and the no-handler → None
  contract for antigravity/opencode.
• Existing HTTP-path tests (test_app_sessions_native_events_lifecycle.py,
  test_app_sessions_native_supervision.py) POST to /events and patch bridge-module
  functions — they pass untouched (call-time imports preserve the patch point), so no
  monkeypatch repoints were needed.

Demo

N/A — internal runner refactor, no user-facing or UI change.

Type of change

• Bug fix
• Feature
• UI / frontend change
• Refactor / chore
• Docs
• Test / CI
• Breaking change

Test coverage

• Unit tests added / updated
• Integration tests added / updated
• E2E tests added / updated
• Manual verification completed
• Existing tests cover this change
• Not applicable

Coverage notes

Behavior-preserving refactor covered by the existing HTTP-path interrupt/stop tests plus 12 new
NativeInterruptRunner unit tests. Manual verification: tests/runner/ local run is green
except for 9 pre-existing codex-native failures (settings/model-options/interrupt) that
reproduce identically on clean main — they need the codex app-server to boot / model
catalog discovery, unavailable in this local env (CI has both). Confirmed the codex interrupt
tests fail with the same "no bridge state → skipped" mode on my branch as on main, i.e. the
refactor preserves that path.

Behavior-preservation notes for reviewers:

• codex/pi stop → interrupt aliasing preserved (they have no distinct stop).
• error codes / context strings kept verbatim ("<x>_native_interrupt_failed" /
  "_stop_failed") — asserted by tests; no display_name drift on the wire.
• pi-only warn-log on interrupt failure preserved via a log_on_error descriptor flag (the
  TUI harnesses returned 503 without logging; pi logged).
• antigravity/opencode still fall through to _cancel_inprocess_turn; pinned by
  test_no_handler_harnesses_return_none.

---

Workstream progress (designs/harness-modular-registry-proposal.md, ~14 PRs):
Phase 1 — 1.1 (#3239), 1.2 (#3244), 1.3 (#3314), 1.5a (#3495), 1.5b-i (#3500), 1.5b-ii
(#3501), 1.5c (#3543) landed; 1.6 is this PR. Remaining: 1.7 (seeding loop), 1.8
(enumerations); Phase 2 — 2.1–2.4. The runner interrupt/stop surface is now behind a DI seam;
the antigravity/opencode gap-fill is the one deferred follow-up. (Also flips the stale 1.5c
ledger row to landed.)

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[PattaraS]

/review

[omnigent-ci]

Polly AI Review

Summary of review

This is a clean, behavior-preserving migration. The 16 per-harness interrupt/stop closures and their two if _harness == "<x>-native" dispatch chains collapse onto a DI'd NativeInterruptRunner that mirrors the existing CodexGoalRunner precedent. I verified the special-cased claude/codex methods are moved verbatim, the uniform handlers faithfully reproduce the original error types (OSError for pi, RuntimeError for the TUI harnesses), the pi-only log_on_error warning, the stop teardown/idle/wake sequence, and the codex/pi stop→interrupt aliasing. No blocking issues, no security issues found.

Blocking issues

None.

Security vulnerabilities

None. No new external input, deserialization, path, or auth surface — the runner still resolves bridge dirs from server-provided session labels exactly as before, and 503 detail continues to flow through _client_safe_error_detail.

Non-blocking notes

• Dispatch now canonicalizes aliases. The old chains matched exact strings (if _harness == "claude-native"); the new code routes through native_coding_agent_for_harness(...), which runs canonicalize_harness first. A reversed/alias spelling (e.g. native-claude) that previously fell through to _cancel_inprocess_turn will now hit the native handler. In practice _session_harness_name returns the canonical form so this is almost certainly inert, but it is a slight broadening of the match surface worth being aware of — arguably an improvement.

• Comment/docstring miscount. The module comment and PR body say "seven uniform stop harnesses," but _UNIFORM_STOP has six entries (cursor, goose, kiro, kimi, hermes, qwen) — claude is special-cased and codex/pi alias to interrupt. The "seven" refers to total stop handlers (6 uniform + claude). Purely a doc nit; no behavior impact.

• Deferred coverage gap is well-scoped. antigravity-native / opencode-native intentionally return None and fall through to the in-process cancel, matching prior behavior, and this is pinned by test_no_handler_harnesses_return_none. The follow-up to wire interrupt_turn() / client.abort() is correctly out of scope here.

• Uniform handlers use importlib.import_module + getattr at call time rather than the from ... import the special-cased methods use. Both resolve module attributes at call time, so existing bridge-module monkeypatches still repoint correctly — confirmed against the untouched HTTP-path tests (which patch claude_native_bridge.kill_session, kiro_native_bridge.inject_interrupt, etc.). No action needed; just noting the two idioms coexist.

Overall assessment

A well-executed, low-risk refactor: it removes ~470 lines of copy-paste from the oversized app.py, follows the established DI-runner pattern, preserves the exact error codes / status transitions / wake plumbing of the originals, and backs the migration with 12 focused unit tests plus the unchanged HTTP-path suite. The intentional coverage deferral is documented and test-pinned. Approve once the trivial "seven"→"six" doc nit is (optionally) tidied.

---

_{Automated review by Polly · workflow run}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[PattaraS]

Thanks Polly — approve confirmed. On the notes:

• "seven"→"six" doc miscount — fixed in 48af007e: the module comments now read "seven uniform interrupt" / "six uniform stop" and spell out the totals (nine interrupt handlers = 7 uniform + claude + codex; seven stop = 6 uniform + claude, with codex/pi aliasing to interrupt). Doc-only.
• Alias canonicalization broadening — agreed it's inert in practice (_session_harness_name returns the canonical form) and arguably an improvement; leaving as-is.
• importlib vs from...import idioms / deferred antigravity+opencode gap — noted, no action; both are intentional and the deferral is test-pinned by test_no_handler_harnesses_return_none.

[github-actions]

🏷️ Doc impact: no-doc-update

Internal refactor extracting native-harness interrupt/stop closures into a NativeInterruptRunner class with added unit tests; behavior is unchanged and no user-facing surface, integration, or default was modified.

_{Auto-classified on merge. Set the label manually before merging to override. · run}