§1+§2: polish, harden, invariant re-proofs + ROADMAP update

[michaelroy-amd]

Stack 10/10 · base: supergoal/phase-9-reroute-and-gate

• Edge/error hardening: seam errors recoverable (no panic), approval Esc/Cancel no focus-trap, daemon no-double-spawn, no-key chat actionable.
• Invariants re-proved by command (rocm-dash-core pure; rocm-dash-tui 0 rocm-core edges; rig sole-namer; rocm→rocmd acyclic).
• Splits the oversized app.rs into app/{slash,chat,summary}.rs; dedups the agent complete() tail. Updates repo ROADMAP.md (§2 done, §1 parity-reached + gated).

Reviewed: final cumulative rust + security + maintainability over phases 1-10, zero unresolved CRITICAL/HIGH. Full suite green (1708 tests, --test-threads=1).

[michaelroy-amd]

Launch-panic fix: provider key resolution moved off the runtime thread

Symptom: bare rocm / rocm chat panicked at launch with
Cannot start a runtime from within a runtime (tokio multi-thread block_on)
whenever ANTHROPIC_API_KEY was unset in the shell.

Root cause (from the real backtrace):

run_chat → Runtime::block_on → run_async → resolved_args        (dash multi-thread runtime worker)
  → provider_keys::resolve_provider_api_key   (env-first, keyring-fallback)
  → NativeProviderKeyStore::get_secret → zbus_secret_service_keyring_store::Store::new
  → secret_service::blocking::SecretService::connect
  → zbus::blocking::Connection::session → Runtime::block_on    ← PANIC (runtime within a runtime)

resolved_args was being called inside run_async. With no env key, the Anthropic
lookup falls through to the OS secret-service store, whose zbus::blocking shim
builds its own tokio runtime + block_on — illegal while the dash runtime is
already entered on that thread. (Introduced on this branch in 614aac6, which began
sourcing the Anthropic key into ResolvedArgs.)

Fix: hoist resolved_args() to the synchronous run/run_chat entry points,
before rt.block_on, and pass the prebuilt ResolvedArgs into run_async. The
keyring lookup now runs on the plain main thread where zbus::blocking is legal.
run_async drops initial_tab and takes args instead, making it type-level
impossible to re-resolve keys on a runtime thread.

Verification:

• Root cause proven deterministically (throwaway tests, since removed): the keyring
  call panics inside block_on and is panic-free outside it.
• PTY launch with ANTHROPIC_API_KEY unset no longer panics.
• Existing dash::tests pass; clean build.

Audited all other keyring call sites (providers.rs, main.rs config subcommands,
legacy tui.rs) — all run on synchronous threads. Runtime provider-switching reuses
the already-resolved key from ResolvedArgs, so it never re-hits the keyring.

[michaelroy-amd]

Chat endpoint detection: use the real launched port (managed-services registry)

Problem: the dash chat only auto-detected the well-known local ports
(vLLM 8000, Lemonade 13305). An engine launched via the tool on a
non-default port — e.g. vLLM on :11435 — was invisible to chat detection, so
bare rocm fell back to the no-key default instead of talking to the local
server we just started.

Fix (115f322): detection is now registry-first. It queries the
read-only services tool and uses the actual endpoint_url of a ready,
OpenAI-compatible managed service (vllm/lemonade/llama.cpp/sglang/
pytorch), preferring the most recently created. ready mirrors the bin's own
HTTP readiness check (what rocm services shows), so a selected endpoint is one
already verified to serve; a best-effort /v1/models fetch confirms liveness
and supplies the served model id.

• Wired into both the startup backend resolution and the live "press d
  to detect" path.
• Falls back to the well-known TCP probe (8000/13305) when no managed
  service exists.
• Never overrides an explicitly configured chat_url/env URL — config
  precedence preserved (the registry only fills the tier where the default port
  would otherwise be probed).

Behavior

• vLLM already running at launch → chat auto-binds to its real endpoint, any port.
• Launched mid-session → press d in chat to re-detect live.
• Nothing managed → unchanged (probe 8000/13305).

Tests/lint: new pure picker pick_managed_chat_endpoint with 7 unit tests
(headline non-default-port case, ready-only gating, newest-wins, engine
filtering, model_ref fallback, malformed/empty). Full dash-tui suite (457)
passes; cargo fmt --check and cargo clippy --workspace --all-targets -D warnings clean.

Verify on an MI3xx node

rocm services            # status: ready, endpoint: http://127.0.0.1:11435/v1
rocm                     # chat tab auto-binds to :11435

[rominf]

LGTM. Verified the 767 deletions are all verbatim moves of app/mod.rs into app/{slash,chat,summary}.rs with visibility narrowed to pub(super) — no behavior, error handling, or validators removed; approval-gating intact. The launch-panic fix is type-level enforced (hoists the keyring probe out of the async runtime), and the "invariant re-proofs" are real assertions (no-double-spawn guard, recoverable seam errors, honest provider revert) rather than comment-claims.

Two minor notes: the architectural-purity invariants are compiler-enforced but have no guard test (a cargo tree CI check would catch a future accidental dep); and resolved_args() now probes the keyring unconditionally including on --demo/--replay paths (harmless, wasted round-trip).

[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 now show Verified. Re-reviewed the current diff against supergoal/phase-9-reroute-and-gate after the re-sign.

Approving. The headline honest provider revert is present and correct — ProviderSwitch { previous, target } snapshots the pre-switch provider and reverts to previous (not hardcoded Local) on build failure, fixing the #36 desync; test-covered. The app.rs → app/{mod,slash,chat,summary}.rs split is a faithful file move (bodies byte-identical), and the pre-runtime keyring resolution correctly avoids the nested-runtime panic.

Non-blocking follow-up (unchanged): a CI check asserting the acyclic rocm → rocmd cargo-tree invariant would be nice but isn't in this diff and doesn't gate merge.

[Copilot]

Pull request overview

Polishes and hardens the unified dashboard chat/daemon flows, re-proving key invariants while refactoring the rocm-dash TUI app module into cohesive submodules and updating the project roadmap to reflect §1/§2 completion status.

Changes:

• Refactors rocm-dash-tui app logic by splitting chat backend construction, slash-command routing, and summary rendering into dedicated modules.
• Improves chat/provider robustness (managed-service endpoint detection, actionable “no backend” empty state, provider-switch revert correctness) and deduplicates agent completion tail logic.
• Hardens daemon/background-helper behavior (no double-spawn guard) and avoids tokio runtime nesting issues by resolving keyring-dependent args before starting the dashboard runtime.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ROADMAP.md	Updates roadmap narrative/status for §1 parity and §2 daemon wiring regression.
crates/rocm-dash-tui/src/app/summary.rs	Adds pure, concise rendering/parsing helpers for slash-tool outcomes and /plan results.
crates/rocm-dash-tui/src/app/slash.rs	Moves slash-command parsing/routing out of app/mod.rs into an I/O-free reducer helper.
crates/rocm-dash-tui/src/app/mod.rs	Wires new submodules, improves provider-switch edge data, adds actionable no-backend message, and updates chat detection to consider managed endpoints.
crates/rocm-dash-tui/src/app/chat.rs	Centralizes provider agent construction, adds managed-service endpoint picking/detection, and keeps config persistence logic cohesive.
crates/rocm-dash-tui/src/agent.rs	Deduplicates agent complete() tail (timeout/error/annotation), centralizes token budget, and adds seam-error recoverability tests.
apps/rocm/src/main.rs	Adds background-helper liveness guard helper + tests; prints a note when interactive rocm chat ignores --provider (rerouted to dash chat).
apps/rocm/src/dash.rs	Extracts dashboard runtime builder and resolves args (incl. keyring lookup) before entering tokio runtime to avoid nested-runtime panics.

---

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

[rominf]

Re-approving the current head after the cascade rebase onto the updated base — refreshing my prior approval onto this exact commit.

Re-reviewed: §1/§2 polish: app module split (chat/slash/summary), provider-switch revert correctness, daemon no-double-spawn guard, nested-runtime fix.

Copilot re-reviewed this same commit (COMMENTED, no blocking findings). No clear issues; LGTM.