Skip to content

fix(audit): clean Ctrl-C handling, all/quit prompts, exit-code semantics, survivor validation - #135

Merged
CybotTM merged 3 commits into
mainfrom
fix/reconcile-prompt-interrupt-handling
Jul 30, 2026
Merged

fix(audit): clean Ctrl-C handling, all/quit prompts, exit-code semantics, survivor validation#135
CybotTM merged 3 commits into
mainfrom
fix/reconcile-prompt-interrupt-handling

Conversation

@CybotTM

@CybotTM CybotTM commented Jul 30, 2026

Copy link
Copy Markdown
Member

Problems

Three issues from real make reconcile-all runs after #134:

  1. Ctrl-C hung the run. Prompts executed inside ThreadPool workers; Ctrl-C killed the main thread while workers kept prompting, then Python's atexit hung joining a worker blocked in input() — repeated Ctrl-C required.
  2. make reconcile-all ended in Error 1 even when everything worked as designed — removals that need sudo are refused on purpose and handed over as copy-paste commands, yet still counted as failures for the exit code.
  3. A kept "duplicate" can be silently broken by the removal. Real incident: reconcile kept a stray copy of byobu's launcher script in ~/.local/bin and removed the apt package that owned /usr/lib/byobu — the survivor sourced those files and broke.

Fixes (one commit each)

  • Main-thread prompting + a/q answers. bulk_reconcile now detects in parallel (workers, no stdin) and prompts/removes sequentially on the main thread. Ctrl-C acts as quit: current prompt declined, remaining tools marked aborted, clean summary, no traceback. Prompt is now [y/N/a=all/q=quit] with sticky all/quit semantics.
  • manual_required outcome. Failures that only need a manual sudo command classify as manual_required: shown with ⚠, counted in a new "Manual action required" summary line, included in the aggregated command block, and excluded from the non-zero exit. Genuine errors still exit 1.
  • Survivor validation. After a successful removal, the kept installation is probed with its version command; if it no longer runs, the result fails with guidance to reinstall the removed package or delete the broken survivor.

Test plan

  • 9 new tests, written first and watched fail: sticky a/q/Ctrl-C prompt semantics, prompts-run-in-MainThread assertion, manual_required classification (all-manual vs mixed), exit-0 CLI behavior with summary line, broken/working survivor probes.
  • One existing test updated for changed behavior (test_reconcile_aggressive_mode now patches the survivor probe).
  • Full suite: 789 passed, 1 skipped; smoke test OK; pre-commit (flake8, isort, black) green.

https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym

CybotTM added 3 commits July 30, 2026 12:47
…nswers

Ctrl-C during a `--reconcile --all --apply` prompt killed the main thread
while ThreadPool workers kept prompting, then Python's atexit hung joining
a worker blocked in input() — requiring repeated Ctrl-C.

Restructure bulk_reconcile into parallel detection (workers, no stdin) and
a main-thread resolution phase that prompts and removes sequentially.
Ctrl-C now acts as quit: the current prompt is declined, remaining tools
are marked aborted, and the run ends cleanly with a summary.

The prompt also accepts 'a' (confirm this and all remaining removals) and
'q' (decline this and all remaining): [y/N/a=all/q=quit].

Claude-Session: https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
…ands

`make reconcile-all` ended in `Error 1` even when the run worked exactly as
designed: removals that require sudo are refused on purpose and handed to
the user as copy-paste commands, so they are an expected outcome, not an
error.

Classify results whose failures are all of the "manual sudo needed" kind
as action 'manual_required': shown with ⚠ instead of ✗, counted in a new
"Manual action required" summary line, included in the aggregated command
block, and excluded from the non-zero exit (both --all and single-tool
apply paths). Genuine errors still exit 1.

Claude-Session: https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Reconcile assumed same-named executables are interchangeable duplicates.
That broke byobu: the kept ~/.local/bin/byobu was a stray copy of the
package's launcher script, which sources /usr/lib/byobu/include/common —
removing the apt package silently broke the survivor.

After a successful removal, probe the kept installation with its version
command; if it no longer runs, mark the result failed with guidance to
reinstall the removed package or delete the broken survivor.

Claude-Session: https://claude.ai/code/session_01MH3EaniXCnJdwqNvrMB4Ym
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Copilot AI review requested due to automatic review settings July 30, 2026 10:52
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@CybotTM
CybotTM merged commit ede9765 into main Jul 30, 2026
24 of 25 checks passed
@CybotTM
CybotTM deleted the fix/reconcile-prompt-interrupt-handling branch July 30, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants