[jussielo-amd]

Closes #769.

fix.rs, uninstall_plan.rs and therock.rs each hand-rolled the same [y/N] prompt sequence: print the prompt, flush stdout, read a line from stdin, normalize it, and match y/yes. This promotes that sequence into a new shared rocm_core::confirm module (is_affirmative_answer, print_prompt, read_answer) and has all three call sites delegate to it.

print_prompt (fallible write+flush) and read_answer (fallible read+parse) stay two separate calls rather than one bundled function, because the three callers disagree on whether a failed write should be fatal: fix.rs's confirm() attempts the read anyway (its original behavior), while uninstall_plan::confirm_uninstall and therock::prompt_yes_no treat it as fatal, each with their own original "failed to flush ..." wording. Both are generic over Write/BufRead, so the sequence is unit-tested with in-memory buffers and failing-I/O test doubles rather than only reachable through e2e coverage.

fix.rs's confirm() also keeps its own --yes/non-interactive short-circuit and refusal wording layered on top of the shared primitives, rather than having that promoted too: that wording ("refusing to apply") is specific to the fix recipe flow and is asserted verbatim by tests/e2e-cucumber/tests/e2e/diagnose_steps.rs. The other two callers (uninstall_plan::confirm_uninstall, therock::prompt_yes_no) already guard the non-interactive/--yes case at their own call sites before ever reaching the prompt, so they only need the raw primitives, not a second copy of that short-circuit.

This is behavior-preserving relative to the pre-#769 original, with one deliberate improvement: a prompt write failure now returns an Err instead of risking a panic (the old print!-based code could panic on a write failure for a long enough prompt; none of the real prompts here are that long, but the fallible path costs nothing and removes the possibility outright). No prompt text or printed message changed. No new Gherkin scenario is needed since there's no user-observable behavior change; the existing e2e coverage for these flows is untouched.

Also adds a bullet to AGENTS.md's "Shared UI components" section (under a new crates/rocm-core-scoped heading, since the module is shared by both apps/rocm and rocm-core itself, not CLI-only) and a module-map entry in docs/architecture.md, per the issue's own documentation ask, so a future fourth copy has something to find instead of re-deriving the pattern again.

Review history

Copilot's initial review caught two real issues in the first commit (d6408dd3): print! panicking instead of returning Err on a write failure, and bundling write+flush+read into one fallible step changing fix::confirm()'s behavior on a write failure compared to before this module existed. Both are fixed in 50e0f87d (see replies on the original threads) after two independent fresh-review passes, which also caught a test-coverage gap (the primitives were hard-wired to real stdin/stdout and untestable) and several doc-accuracy issues, all addressed in that same commit.

Checklist

• No ticket ID in tests/e2e-cucumber/expectations.toml references Deduplicate the [y/N] confirmation prompt (three independent copies) #769; nothing to narrow.
• New rocm-core module follows the documented library-crate pattern (pub mod confirm;, accessed via the qualified rocm_core::confirm:: path).
• No user-facing message wording changed; nothing new to pin against its code path.

Verification

• cargo build -p rocm-core -p rocm
• cargo clippy --workspace --all-targets -- -D warnings
• cargo test --workspace --all-targets --no-fail-fast (all green)
• cargo xtask check-crate-edges
• cargo xtask coverage (no floor regressions)
• cargo fmt --check
• Leak scan against the diff: clean

[Copilot]

🟡 Changes recommended

The shared confirmation helper can panic instead of returning output errors, and its flush-error handling changes caller behavior and misreports output failures as input failures.

2 open issues

• print! panics on a write failure rather than returning an io::Error, so the new public helper… · New
• Propagating the flush error here changes the error paths despite the PR's behavior-preserving… · New

What changed in this PR

This PR centralizes the CLI’s duplicated [y/N] prompt logic in rocm-core, addressing #769 while keeping caller-specific confirmation policies in their existing wrappers.

Changes:

• Adds and exports shared response-parsing and prompt-reading helpers, with parser unit tests.
• Updates fix, uninstall, and SDK-overwrite prompts to use the shared helper.
• Documents the helper in contributor guidance and the architecture module map.

File	Description
docs/​architecture.md	Adds the shared confirmation module to the module map.
crates/​rocm-core/​src/​lib.rs	Exposes the module and re-exports its helpers.
crates/​rocm-core/​src/​fix.rs	Delegates prompt handling and relocates parser tests.
crates/​rocm-core/​src/​confirm.rs	Implements shared confirmation helpers and parser tests.
apps/​rocm/​src/​uninstall_plan.rs	Delegates the uninstall prompt to the shared helper.
apps/​rocm/​src/​therock.rs	Delegates the SDK-overwrite prompt to the shared helper.
AGENTS.md	Lists the confirmation helper among shared CLI components.

_{Review effort: Balanced}

[jussielo-amd]

Fixed in 50e0f87: print_prompt now writes via a fallible write! instead of print!, so a write failure returns Err instead of panicking.

[jussielo-amd]

Fixed in 50e0f87: split into print_prompt (fallible write+flush) and read_answer (fallible read+parse) so each caller keeps its own original error policy instead of one bundled Result forcing the same decision everywhere. fix::confirm() now explicitly discards a print_prompt error and still attempts the read (restoring its pre-existing behavior); uninstall_plan::confirm_uninstall and therock::prompt_yes_no keep propagating a write failure with their original "failed to flush ..." wording.