morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #1780] [MERGED] fix(cli): await browser scenario cleanup on interrupts #4812

New issue

Closed

opened 2026-10-31 00:55:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:55:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1780
Author: @N0zoM1z0
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/11/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/browser-scenario-cli-interrupt-cleanup-20261011

📝 Commits (1)

b1329c1 fix(cli): await browser scenario cleanup on interrupts

📊 Changes

3 files changed (+258 additions, -49 deletions)

View changed files

📝 src/cliBrowserScenarioCommands.ts (+55 -49)
➕ tests/acceptance/browser/browserScenarioCancellationCli.test.ts (+84 -0)
➕ tests/fixtures/browserScenarioCancellation.mjs (+119 -0)

📄 Description

Summary

When interrupted during a browser scenario, the CLI now waits for provider cleanup and returns retained observations with exit code 130 for SIGINT or 143 for SIGTERM. Refs #1771; GitHub issue/PR searches found no open fix for this interrupt path.

Problem and expected behavior

Fresh main 914ba833 exits immediately on either OS signal while an action is active: no AbortSignal reaches the provider, cleanup callbacks are skipped, and stdout is empty. The command should translate an interrupt into cancellation, await its owned cleanup, and preserve completed and cancelled steps.

Change and scope

Wrap command logging with the existing withCommandCancellation and pass its signal to captureBrowserScenario. Keeping cancellation outside logging preserves conventional interrupt exit codes after error handling. Existing provider cleanup retries and ownership rules continue to apply.

Contract and boundary impact

Semantic owner and earliest changed stage: one-shot CLI signal boundary.
CLI and MCP/tool-catalog contract: CLI interrupt behavior corrected; schemas unchanged.
Provider, bridge, target-format, or platform compatibility: existing browser provider cancellation reused; POSIX signal regression skips Windows.
Evidence, artifact, provenance, or reconstruction contract: interrupted captures retain authenticated Evidence with incomplete capture accounting and cancelled steps.
Process execution, authorization, cleanup, or containment impact: owned provider cleanup is awaited before command completion.
Generated metadata, package, or installation impact: none.

Evidence and regression coverage

Added two real CLI subprocess tests sending SIGINT/SIGTERM after a controlled session starts its action. They assert normal exit 130/143, signal delivery, cleanup callbacks, authenticated stdout Evidence, completed start and cancelled action, and incomplete capture status.
Both tests fail against freshly compiled main 914ba833 with direct signal termination. An independent public-CLI probe also confirms skipped cleanup callbacks and empty output on that base.
After the change, the independent probe returns authenticated Evidence on both interrupts and confirms cleanup callbacks ran.
Scope: real OS signals and public compiled CLI, real Playwright provider and cleanup owner, controlled session. No Chromium process or browser profile is created. Windows interrupt behavior remains unverified.
Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Fresh single-threaded compile, 21 focused tests (new subprocess tests, existing CLI cleanup tests and provider tests), and independent public interrupt probe: passed.
Full single-threaded typecheck, lint/module boundaries, changed-file format check and architecture guards: passed.
Rebuilt the committed source and reran 21 tests plus both public interrupt probes: passed.
All steps supervised within 2 GiB process-family RSS and 90-second limits. Used explicit single-threaded checks rather than the default threaded pre-push hook.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: Linux CLI/OS signals; controlled browser transport as described above.
Package or release metadata impact: none.
Security, privacy, process, or containment review: explicit provider ownership and existing cleanup paths retained; test child processes owned by the shared registry.

Review checklist

Concrete problem; one focused Conventional Commit outcome.
Related issue linked; existing open fixes searched.
Regression fails on base and verifies observable behavior after the fix.
Contracts and generated metadata need no changes.
Representative CLI behavior and verification scope stated.
Final diff reviewed for unrelated changes and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1780 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/browser-scenario-cli-interrupt-cleanup-20261011` --- ### 📝 Commits (1) - [`b1329c1`](https://github.com/morluto/rea/commit/b1329c1fec1b2d8aa770e97e8b57476c1604387c) fix(cli): await browser scenario cleanup on interrupts ### 📊 Changes **3 files changed** (+258 additions, -49 deletions) <details> <summary>View changed files</summary> 📝 `src/cliBrowserScenarioCommands.ts` (+55 -49) ➕ `tests/acceptance/browser/browserScenarioCancellationCli.test.ts` (+84 -0) ➕ `tests/fixtures/browserScenarioCancellation.mjs` (+119 -0) </details> ### 📄 Description ## Summary When interrupted during a browser scenario, the CLI now waits for provider cleanup and returns retained observations with exit code 130 for SIGINT or 143 for SIGTERM. Refs #1771; GitHub issue/PR searches found no open fix for this interrupt path. ## Problem and expected behavior Fresh main `914ba833` exits immediately on either OS signal while an action is active: no AbortSignal reaches the provider, cleanup callbacks are skipped, and stdout is empty. The command should translate an interrupt into cancellation, await its owned cleanup, and preserve completed and cancelled steps. ## Change and scope Wrap command logging with the existing `withCommandCancellation` and pass its signal to `captureBrowserScenario`. Keeping cancellation outside logging preserves conventional interrupt exit codes after error handling. Existing provider cleanup retries and ownership rules continue to apply. ## Contract and boundary impact - Semantic owner and earliest changed stage: one-shot CLI signal boundary. - CLI and MCP/tool-catalog contract: CLI interrupt behavior corrected; schemas unchanged. - Provider, bridge, target-format, or platform compatibility: existing browser provider cancellation reused; POSIX signal regression skips Windows. - Evidence, artifact, provenance, or reconstruction contract: interrupted captures retain authenticated Evidence with incomplete capture accounting and cancelled steps. - Process execution, authorization, cleanup, or containment impact: owned provider cleanup is awaited before command completion. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Added two real CLI subprocess tests sending SIGINT/SIGTERM after a controlled session starts its action. They assert normal exit 130/143, signal delivery, cleanup callbacks, authenticated stdout Evidence, completed start and cancelled action, and incomplete capture status. - Both tests fail against freshly compiled main `914ba833` with direct signal termination. An independent public-CLI probe also confirms skipped cleanup callbacks and empty output on that base. - After the change, the independent probe returns authenticated Evidence on both interrupts and confirms cleanup callbacks ran. - Scope: real OS signals and public compiled CLI, real Playwright provider and cleanup owner, controlled session. No Chromium process or browser profile is created. Windows interrupt behavior remains unverified. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - Fresh single-threaded compile, 21 focused tests (new subprocess tests, existing CLI cleanup tests and provider tests), and independent public interrupt probe: passed. - Full single-threaded typecheck, lint/module boundaries, changed-file format check and architecture guards: passed. - Rebuilt the committed source and reran 21 tests plus both public interrupt probes: passed. - All steps supervised within 2 GiB process-family RSS and 90-second limits. Used explicit single-threaded checks rather than the default threaded pre-push hook. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Linux CLI/OS signals; controlled browser transport as described above. - Package or release metadata impact: none. - Security, privacy, process, or containment review: explicit provider ownership and existing cleanup paths retained; test child processes owned by the shared registry. ## Review checklist - [x] Concrete problem; one focused Conventional Commit outcome. - [x] Related issue linked; existing open fixes searched. - [x] Regression fails on base and verifies observable behavior after the fix. - [x] Contracts and generated metadata need no changes. - [x] Representative CLI behavior and verification scope stated. - [x] Final diff reviewed for unrelated changes and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:55:19 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#4812

No description provided.

Rows
Columns