Skip to content

feat(settings): configurable close-confirmation policy for tab and quit#142

Merged
bastien-gallay merged 1 commit into
mainfrom
feat/close-confirm-policy
Jul 5, 2026
Merged

feat(settings): configurable close-confirmation policy for tab and quit#142
bastien-gallay merged 1 commit into
mainfrom
feat/close-confirm-policy

Conversation

@bastien-gallay

@bastien-gallay bastien-gallay commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

What

Make close confirmation configurable, per action, via settings.json:

"close": { "tab": "confirmWhenActive", "app": "confirmWhenActive" }

tab (tab close) and app (app quit) each take one of:

  • alwaysConfirm — always prompt
  • confirmWhenActive — prompt only while a session is running a foreground
    process (a working shell or any live Claude); an idle shell closes/quits
    silently
  • noConfirmation — close immediately

Both default to confirmWhenActive. Either field may be omitted.

Relationship to #140 / #79 / #80

A config layer over #140 (which landed the running-process predicate in core and
closed #79):

How

  • ConfirmClose enum (camelCase serde) + CloseSettings { tab, app } in
    settings.rs, default confirmWhenActive for both.
  • Pure decision seam ConfirmClose::confirms(active) -> bool; the caller
    supplies the predicate.
  • App::any_running_process added in core next to tab_has_running_process
    (so close and quit never re-derive "is something running?" in two places).
  • request_close/request_quit consult the policy; feat(shell): confirm tab close only when a process is running #140's
    "pending-confirmation owns the interaction" guard is preserved.
  • Quit prompt counts open sessions honestly ("N open session(s) will be
    force-stopped — any running work is lost") rather than always naming Claude;
    quit_prompt(0) degrades to a plain "Quit TermHerd?".

Known gap (tracked)

The predicate reads a plain shell running a non-Claude foreground program
(vim, a long make) as idle, so it can be closed/quit silently. Better
foreground-process detection is filed as #143; users who want the safe
behaviour today can set "close": { "app": "alwaysConfirm" }.

Tests

  • coreany_running_process spans tabs (idle → false, one Busy anywhere →
    true).
  • settings.rs — defaults, camelCase round-trip, confirms truth table,
    partial-block fill.
  • shell.rsnoConfirmation closes without arming; confirmWhenActive
    prompts while running then skips once exited; an idle-but-live session quits
    silently under the default
    (the Confirm app quit when a process is running #80 headline); alwaysConfirm prompts with
    nothing running; noConfirmation quits past a running session.

Full workspace green; fmt + clippy -D warnings clean.

Closes #80. Built on #79/#140. Known gap: #143.

@bastien-gallay

Copy link
Copy Markdown
Collaborator Author

Applied the code-review findings (all five) in e88a9bf:

  1. Quit vs. non-Claude foreground programs — left the running-process gate as-is (intended Confirm app quit when a process is running #80 behaviour) and filed Better foreground-process detection for close/quit confirmation (vim/less/make read as idle) #143 to track proper foreground-process detection; noted the interim alwaysConfirm workaround.
  2. Prompt wording — the quit modal no longer claims "N active session(s) … Claude will be killed" when the count includes idle shells / no Claude; it now reads "N open session(s) will be force-stopped — any running work is lost."
  3. Predicate home — moved the app-wide check into core as App::any_running_process, next to tab_has_running_process, so close and quit share one source of "is something running?".
  4. Test gap — added a test for the Confirm app quit when a process is running #80 headline (an idle-but-live session quits silently under the default policy), plus a core test for any_running_process.
  5. Issue refs in new comments — dropped the #NN references from the newly-added doc comments.

Add a `close` block to settings.json letting the user choose, per action,
whether closing prompts first:

  "close": { "tab": "confirmWhenActive", "app": "confirmWhenActive" }

Each of `tab` (tab close) and `app` (app quit) takes one of alwaysConfirm /
confirmWhenActive / noConfirmation. One pure decision seam,
ConfirmClose::confirms(active), backs both paths.

confirmWhenActive reuses the running-process predicate from #140: a tab keys
off App::tab_has_running_process, a quit off the new App::any_running_process
(both over LiveSession::has_running_process, single-sourced in core), so an
idle shell closes/quits silently while a working shell or a live Claude
confirms.

Both actions default to confirmWhenActive, which preserves #140's shipped
tab-close behaviour and extends the same predicate to the quit path — so an
all-idle app now quits without a prompt. alwaysConfirm / noConfirmation are the
new opt-in overrides. The quit prompt now counts open sessions honestly ("N
open session(s) will be force-stopped — any running work is lost") instead of
claiming Claude is always the victim.

Known gap: the predicate reads a plain shell running a non-Claude foreground
program (vim, a long make) as idle, so it can be closed/quit silently —
better foreground-process detection is tracked in #143.

Closes #80.
@bastien-gallay bastien-gallay force-pushed the feat/close-confirm-policy branch from e88a9bf to 52b6e24 Compare July 5, 2026 14:12
@bastien-gallay bastien-gallay merged commit f1e1ba3 into main Jul 5, 2026
16 checks passed
@bastien-gallay bastien-gallay deleted the feat/close-confirm-policy branch July 5, 2026 14:16
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.

Confirm app quit when a process is running Confirm tab close only when a process is running in its shell

1 participant