§1: in-chat approval modal + core mutating tools (install/engine/serve/services)

[michaelroy-amd]

Stack 4/10 · base: supergoal/phase-3-readonly-and-nav

The defining capability: mutating ROCm actions behind explicit human approval.

• Reuses ui/approval.rs; mutating tools surface ApprovalRequired via ClientMsg::ChatApprovalRequired → app-loop modal → on Approve execute_approved (runs as a captured subprocess, no TUI corruption) → exactly one follow-up turn.
• Wires install/engine/serve/services + safety validators reused unchanged (no public bind / loopback-only / no CPU device).

Reviewed: rust + security + maintainability — approval gate is unbypassable. All cargo gates green.

[rominf]

Approve with minor comments. Reviewed this as the security-critical PR and the gate holds: the approval seam is enforced centrally — the LLM tool loop can only return ApprovalRequired, execution happens solely via execute_approved from the event-loop Approve verdict, and both paths re-validate before running. No bypass, no TOCTOU on the validated args, argv-vector spawning (no shell injection). Excellent test coverage proving approval-is-required from both entrypoints.

Minors:

• launch_server arg-building pushes positional user values as bare argv; a value like --managed would be parsed as a flag by the child clap parser (arg-confusion, not injection — same gated binary). A -- separator before positionals would be good defense-in-depth (pre-existing, low risk).
• approval.rs defaults the modal cursor to Approve, so Enter on a freshly-opened destructive modal approves. The two-key flow mitigates it, but defaulting destructive modals to Deny is the safer convention.

[rominf]

Requesting changes on signing grounds — this is separate from the code review above, which still stands.

Every commit on this PR is unsigned: GitHub reports verification.verified = false (reason unsigned) for all of them. Signed commits are required before this can merge.

To fix, configure commit signing and re-sign the branch history, e.g. with SSH signing:

git config gpg.format ssh
git config user.signingkey <your-signing-key.pub>
git config commit.gpgsign true
git rebase --exec 'git commit --amend --no-edit -S' <base-branch>
git push --force-with-lease

Since this is a stacked PR, re-signing is easiest from the bottom of the stack upward — re-signing a base branch rewrites its SHAs and the dependent branches will need rebasing/re-pushing on top.

Once the commits show as Verified I'll clear this.

[rominf]

Signing gate cleared — all commits on this PR now show Verified (verification.verified = true, reason valid). Re-reviewed the current diff against this PR's own base after the re-sign; it's substantively unchanged from my prior review.

Approving on both grounds: code is sound and commits are signed.

[Copilot]

Pull request overview

Adds an explicit human-approval gate for mutating ROCm actions invoked from Dash chat/slash flows, wiring the approval modal through the app loop and providing a safe “execute approved” seam that runs via captured subprocess output (to avoid TUI corruption).

Changes:

• Introduces approval intent plumbing + execute_approved across the dash↔bin tool-exec seam, plus app-loop modal routing and result follow-up turn behavior.
• Adds slash-command routing for both read-only executor-backed commands and approval-gated mutating commands (install/engine/serve/services), including concise summarization of tool results.
• Registers read-only and mutating ROCm tools in the Rig agent clients, with mutating tools surfacing approval requests to the UI instead of executing in the tool loop.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
docs/dash-parity-map.md	Adds a Phase/coverage parity map for legacy slash commands and their Dash surfaces/tests.
crates/rocm-dash-tui/src/ui/tabs/instances.rs	Updates test helpers to include newly added AppState fields.
crates/rocm-dash-tui/src/ui/mod.rs	Renders the approval modal last so it overlays all other UI surfaces.
crates/rocm-dash-tui/src/tool_exec.rs	Extends the seam with an approval intent payload and an execute_approved method.
crates/rocm-dash-tui/src/client.rs	Adds client message variants for slash-tool replies and approval request/result messaging.
crates/rocm-dash-tui/src/app.rs	Implements slash-command handling, approval modal flow, executor-backed slash execution, and concise tool result summaries + tests.
crates/rocm-dash-tui/src/agent.rs	Registers read-only and mutating ROCm tools; mutating tools surface approvals to the app via a channel + tests.
apps/rocm/src/main.rs	Accepts doctor as an alias for examine and adds a guard test ensuring dash read-tool names are accepted by the bin.
apps/rocm/src/dash_seam.rs	Implements execute_approved in the bin seam and updates approval intent payload to carry (name, arguments) for replay.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.