-
Notifications
You must be signed in to change notification settings - Fork 0
m7 interactive tui
Status: Planned, reviewed (revised after pre-implementation review). Tracked by issue #9 and sub-issues #58, #59, and #60.
Add an interactive checklist to the existing repair pipeline without weakening the
safety contracts established by M4 through M6. M7 is a presentation-layer change:
package enumeration, alias resolution, collision detection, link inspection, and the
final re-inspection performed by SymlinkService remain authoritative.
M7 is delivered as three stacked, reviewable pull requests in this order:
- #58 — virtual-terminal capability and terminal-session lifetime
- #59 — repair-candidate checklist and command-line integration
- #60 — progress and final result summary
The implementation pull requests are reviewed before merge. They must not be merged as part of the implementation session; merging and issue closure remain maintainer actions.
A pre-implementation review against the merged M6 code (src/cli/Console.*,
src/cli/Dispatch.cpp) found defects that would have made the checklist, resize
handling, or exit-code contract behave incorrectly if implemented as originally
written. This revision fixes those defects before implementation starts. The most
load-bearing correction: this plan's original "existing exit-code precedence is
preserved" statement was incorrect — the merged runFix() already orders
interrupted ahead of anyInsufficientPermission, which does not match the documented
"insufficient permission returns 2" priority. M7 corrects the CLI's own precedence; see
"Progress, summary, and exit codes" below.
- The TUI is enabled only by
fix --tui. -
--tuiwithscanortest-ruleis a configuration error and returns exit code 3. -
--tuiconflicts with--jsonand--yes; each combination returns exit code 3. These checks live next to the existing--json-with-fix-without---yesconflict check inparseArguments()(afterapplyPositionals()), so the--help/--versionshort-circuit ahead of it is untouched. -
--dry-runis allowed. Selected entries are re-inspected and reported, but no filesystem mutation occurs. -
--no-colorandNO_COLORdisable SGR color only. They do not disable the cursor and screen-control sequences required by the TUI. This is already howConsolebehaves today (tryEnableVirtualTerminal()runs regardless ofnoColor;--no-coloronly gatescolorEnabled()) — #58 exposes that existing capability rather than changing it. - The TUI requires interactive stdin and stdout plus successful
ENABLE_VIRTUAL_TERMINAL_PROCESSINGactivation on stdout. If any capability is unavailable, no TUI escape sequence is emitted; a warning is written to stderr andfixcontinues through the existing line-oriented CLI confirmation flow. - Every console mode changed for the TUI is owned by an RAII terminal session and
restored on normal return, cancellation, exceptions, and partial initialization
failure — restoration runs at most once. The session also restores on
CTRL_CLOSE_EVENT/CTRL_LOGOFF_EVENT/CTRL_SHUTDOWN_EVENT(not just C++ unwinding), since those are delivered even when a destructor would not otherwise run (window closed, logoff, shutdown); the handler always returnsFALSEafter restoring so default OS handling still proceeds. Ctrl+C is deliberately not handled by this console-control-handler path while the checklist has input focus — see the terminal ownership note below.
stdout's ENABLE_VIRTUAL_TERMINAL_PROCESSING mode continues to be owned exclusively by
Console (constructor enables it once, destructor restores it once), unchanged from
M6. The new terminal session introduced by #58 owns three things only: stdin's console
mode, the alternate-screen state, and cursor visibility. It must not call
GetConsoleMode/SetConsoleMode on stdout — doing so would capture "original mode" as
whatever Console already changed it to, corrupting restoration order. Console grows
three read-only accessors (vtEnabled(), stdinInteractive(), stdoutInteractive())
so the terminal session and dispatch layer can query capability without re-deriving or
re-mutating it.
- Only non-colliding
MissingandBrokenitems are selectable. -
Ok,Mismatch, alias collisions, and executables without a valid alias are never offered as selectable repairs. Existing warnings and exclusion behavior remain. - Every selectable item starts unchecked. A repair requires an explicit Space key selection followed by Enter.
- Up/Down moves the cursor, Space toggles the current item, and Enter confirms the current selection.
- Escape, Q, or Ctrl+C cancels before mutation and returns success with no selected
repairs. Ctrl+C here is read as a key event, not delivered through
SetConsoleCtrlHandler: the checklist's input mode clearsENABLE_PROCESSED_INPUT, so Ctrl+C arrives as an ordinaryKEY_EVENT_RECORD(wVirtualKeyCode == 'C'with a Ctrl bit set indwControlKeyState) read by the same blockingReadConsoleInputWloop that reads every other key — not as a separateCTRL_C_EVENTfired on its own thread while the main thread is blocked insideReadConsoleInputWwaiting for a line-mode read that will never come. (That thread split is exactly the failure mode the existing batch-loop Ctrl+C handler,g_ctrlCRequestedinsrc/cli/Dispatch.cpp, relies on — it is correct for the non-interactive repair loop, where the main thread is doing work between checks, but wrong for a loop blocked reading keys.)SetConsoleCtrlHandleris registered only after the checklist session has ended, around the repair-loop phase described below. - Enter with no selected items is a successful no-op.
- Long lists use a scrolling viewport. A console resize recomputes the viewport without
losing the cursor or selection state.
- Resize detection requires the stdin input mode to include
ENABLE_WINDOW_INPUT(without it, noWINDOW_BUFFER_SIZE_EVENTis ever generated) and to excludeENABLE_VIRTUAL_TERMINAL_INPUT(enabling VT input turns key/resize signals into an escape-sequence stream and suppressesWINDOW_BUFFER_SIZE_EVENTentirely). -
ENABLE_QUICK_EDIT_MODEmust be cleared so a mouse drag cannot select text and stall the renderer; clearing it only takes effect whenENABLE_EXTENDED_FLAGSis also passed in the sameSetConsoleModecall — omittingENABLE_EXTENDED_FLAGSsilently no-ops the quick-edit change. -
WINDOW_BUFFER_SIZE_EVENT.dwSizereports the screen buffer size, not the visible window size, and must not be used as the viewport height/width directly. The viewport is derived fromGetConsoleScreenBufferInfo().srWindow(Bottom - Top + 1rows,Right - Left + 1columns). - Full stdin input-mode contract: set
ENABLE_WINDOW_INPUT | ENABLE_EXTENDED_FLAGS; clearENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_PROCESSED_INPUT | ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_QUICK_EDIT_MODE | ENABLE_MOUSE_INPUT.
- Resize detection requires the stdin input mode to include
- Package-derived text is sanitized (
sanitizeForDisplay()) before it is combined with intentional TUI control sequences. - Confirmation never authorizes mutation by itself. Each selected item is passed to the
shared repair-batch executor (#60), which performs the same fresh pre-action
inspection used by the CLI via
repairLink().
Alternate-screen control (ESC[?1049h/l) only has effect once stdout VT processing
is enabled, so initialization is strictly ordered: enable stdout VT (already owned by
Console, queried via vtEnabled()) → enter alternate screen → hide cursor → apply the
stdin input-mode change above. Teardown reverses this exactly: restore stdin input
mode → show cursor → leave alternate screen. A failure partway through initialization
unwinds only the steps that already succeeded, in reverse order, before reporting the
capability as unavailable — it never leaves a partial state (e.g., alternate screen
entered but stdin mode unchanged).
After selection is confirmed, the checklist session restores the terminal (per the
teardown order above) before repair begins, and the shared executor writes persistent
progress lines in the form [current/total] alias: result. The final summary reports:
- selected, processed, and remaining;
- created and replaced;
- planned operations in dry-run mode;
-
declined — an item whose interactive confirmation the user refused. This is
distinct from a dry-run's
plannedoutcome: declining a real (non-dry-run) repair must never be reported as though--dry-runhad produced a plan for it, since that would misrepresent an unattempted repair as an inspected-and-planned one.declineddoes not affect the exit code (see below). The existing JSON schema (docs/adr-phase-5.mdADR-0022) is unchanged —declinedis a console/TUI summary category only, not a new JSON field, to preserve script compatibility; - skipped items, specifically a fresh
Okresult (SkippedOk) or a refused mismatch (RefusedMismatch); - failed items; and
- whether the batch was interrupted, and how many items remained unprocessed.
Ctrl+C during the repair-loop phase (after the checklist session has ended and the
batch is executing, using the existing SetConsoleCtrlHandler-based mechanism, not the
checklist's own key handling) finishes the current item and stops before the next item.
Remaining items are counted in the summary.
Exit-code precedence, corrected from this plan's original text and now different from
the pre-M7 CLI path (a deliberate CLI-wide fix, applied to both the TUI and
non-interactive fix paths, not just the new TUI code):
- Insufficient permission → exit code 2, taking precedence over an interruption or any other partial failure that also occurred in the same batch.
- Otherwise, an interruption or another partial failure → exit code 10. An interruption
with zero remaining items (Ctrl+C observed after the last item had already been
processed) is not reported as an interruption for exit-code purposes — there is
nothing left it could have prevented — so this branch requires
remaining > 0. A dry-run batch that was interrupted still returns 10: an incomplete dry-run report is a partial failure to report, even though it mutated nothing. - Otherwise → exit code 0, covering full success, pre-mutation cancellation, an empty selection, and a successful (complete, uninterrupted) dry-run.
- Expose stdout-console, stdin-console, and VT-enabled capabilities separately from
color enablement, as read-only accessors on the existing
Consoleclass (not a new parallel capability type) —vtEnabled(),stdinInteractive(),stdoutInteractive().Consolealready computes VT capability internally (tryEnableVirtualTerminal()) and discards it into a local; #58 keeps it instead. - Add an injectable terminal-operations seam (matching the existing
ConsoleOperations/SymlinkServiceOperationsseam pattern already used in this codebase) and an RAII terminal session for stdin input mode, alternate-screen state, cursor visibility, and resize/key events — scoped exactly as described in "Terminal ownership boundary" above. stdout VT stays exclusivelyConsole's responsibility. - Cover successful activation, redirection (stdin and stdout independently), VT
activation failure, partial initialization failure and its reverse-order unwind,
--no-color(confirming VT capability is unaffected — this is a verification of existing behavior, not a new code path), restoration exactly once, and restoration triggered by a registered close/logoff/shutdown handler.
- Add a pure, unit-testable selection state model (no Win32 dependency) and a thin Win32/VT renderer built on the #58 terminal session.
- Implement navigation, toggling, confirmation, cancellation, resize, and scrolling per the stdin input-mode contract above.
- Integrate selection with the existing inventory and repair pipeline
(
nonCollisionItemsfiltered toMissing/Broken). - Enforce the command/option conflicts documented above, alongside the existing
--json-with-fixconflict check already inArgParser.cpp. - Preserve the current line-oriented CLI path as the capability fallback, with zero TUI escape sequences emitted when falling back.
- Extract one shared repair-batch executor used by the CLI and TUI paths, returning a
structured summary (counts by disposition, including
declined) and owning the exit- code decision described above in one place, so the CLI's non-interactivefixpath and the TUI path cannot diverge on either. - Report per-item progress and the final summary without duplicating repair decisions.
- Cover all result categories including
declined, dry-run, permission failure, general failure, interruption with remaining items, and interruption with zero remaining items.
Each implementation pull request must:
- build
Debug|x64,Release|x64,Debug|ARM64, andRelease|ARM64at/W4 /WX; - run the Debug and Release x64 MSTest suites and report the actual results;
- describe ARM64 as cross-built, not run, unless tested on an ARM64 host;
- add focused unit tests for every new state transition and Win32 operations seam;
- manually verify interactive keys (including Ctrl+C read as a key, not a control- handler event), resize, dry-run, no-color, capability fallback, collision/mismatch exclusion, Ctrl+C during the repair loop, declined vs. planned reporting, and the final summary against scratch Packages/Links directories;
- add no third-party dependency;
- request Copilot review, incorporate actionable feedback, reply to each addressed comment, and resolve its review thread; and
- remain unmerged after becoming review-ready.
Each pull request should use Closes for its sub-issue so the issue closes when a
maintainer eventually merges it. Until then, #58, #59, #60, and parent #9 remain open.
-
docs/adr-phase-6.md: one ADR per sub-issue, not one shared ADR, to avoid three stacked pull requests editing the same ADR section — ADR-0026 (#58, terminal ownership and fallback), ADR-0027 (#59, selection/option contract), ADR-0028 (#60, shared executor and the corrected exit-code precedence). ADR-0026 is reserved for #58 only; M8 issue #113 (--tui/--verbose/--quiet) uses ADR-0029, not ADR-0026, to avoid the numbering collision this revision found. -
docs/TODO.md: check one M7 item in each corresponding sub-issue. -
docs/PLAN.md: mark the TUI Definition-of-Done item only in #60. -
docs/task.md: record implementation and verification evidence for every sub-issue.
Canonical documentation and code comments remain English. Localized *_ja.md files are
not implementation inputs and are not changed.
Implemented M7 shipped a checklist that listed only Missing and Broken
candidates, per ADR-0027 decision 4. Issue #179 showed why that is not enough: on a host
whose inventory was 20 Ok links and one Mismatch, fix --tui printed no checklist
at all and fell silently through to the non-interactive loop, hiding the one candidate
that needed attention.
The checklist now lists Mismatch as a non-selectable informational row. The full,
current contract — worth remembering, because "fix never repairs a Mismatch" is easy
to forget:
LinkStatus |
In the checklist? | Selectable? | What fix does to it |
|---|---|---|---|
Missing |
yes | yes — checking it is consent to create the link | creates the symlink |
Broken |
yes | yes — checking it is consent to replace the link | deletes and recreates the symlink |
Mismatch |
yes, as [-] ... [cannot repair]
|
no |
nothing — reports refused (mismatch)
|
Ok |
no | — | nothing — reports already Ok
|
An alias collision is excluded before the checklist and never reaches it (ADR-0021).
A Mismatch is still never repaired — the entry under Links\ is a regular file, a
non-symlink reparse point, or a symlink to a different existing file, and replacing it
would destroy whatever is actually there (ADR-0014, ADR-0016). No --force /
--replace-mismatch option exists, and that remains the decision. The row is shown so
the user can see what needs manual attention, not so they can act on it here.
Two further corrections in the same change: the "nothing to list" case now warns on
stderr (ADR-0027 decision 5 always specified this; only the terminal-capability branch
implemented it), and the grouped fix preview is suppressed only when the checklist
actually ran, rather than whenever --tui was merely requested.
See --tui reporting and path display
and ADR-0047 in docs/adr-phase-10.md.