Skip to content

fix(gui): keep the diagnostics report truthful across agent restarts#230

Merged
AprilNEA merged 1 commit into
masterfrom
fix/diagnostics-live-agent-state
Jun 13, 2026
Merged

fix(gui): keep the diagnostics report truthful across agent restarts#230
AprilNEA merged 1 commit into
masterfrom
fix/diagnostics-live-agent-state

Conversation

@AprilNEA

Copy link
Copy Markdown
Owner

Follow-up to #206. The Copy Diagnostics feature was written before the startup-states rework (#213/#215) landed, and the rebase kept its original snapshot plumbing. Two of those seams could make a report disagree with reality exactly when users are most likely to file one — right after an agent restart.

Changes

Read the agent status from the live AgentLink instead of a never-cleared copy. AppState.last_status was retained on every poll and never invalidated, while AgentLink already carries the full AgentStatus and flips to Unreachable on disconnect. A report copied while the agent was down would claim Agent: v… (connected) from the stale copy — misleading for exactly the "agent is unresponsive" class of bug report. The adapter now reads AppState::agent_status() (backed by AgentLink), and the duplicate field is gone.

Gate the inventory snapshot on InventoryHealth::Ready. The device-list merge already ignores pre-enumeration snapshots (an agent restart serves an empty list for the 1.5–5 s enumeration window), but the diagnostics snapshot was stored unconditionally. A report copied in that window lost its Receivers section and all per-device enrichment (codename, wpid, model-ids, transports — Connection degraded to unknown) while the UI still showed everything, because the visible list is protected by the gate. The snapshot now shares it, so report and UI follow the same staleness rule.

Surface enumeration health in the report. New Inventory: ready / scanning (first enumeration in progress) / ⚠️ unavailable (enumeration failed — see agent log) line in the App section, so a zero-device report is self-explaining.

Testing

  • Core rendering tests extended: the app() fixture pins Inventory: ready, the unreachable-agent test pins the placeholder, and a new test covers the scanning/unavailable wording (9 tests total).
  • cargo test -p openlogi-core, cargo clippy -p openlogi-gui -p openlogi-core --all-targets (zero warnings), and cargo fmt --check all pass locally; full-workspace clippy ran via the pre-push hook.

- Read the agent status from the live AgentLink instead of a never-
  cleared copy: a report copied while the agent is down now says "not
  connected" instead of replaying the last snapshot, and AppState stops
  retaining a second AgentStatus alongside AgentLink::Ready.
- Gate the diagnostics inventory snapshot on InventoryHealth::Ready —
  the same gate the device-list merge uses — so a report copied during
  the reconnect window keeps the receivers and transports the UI is
  still showing instead of an empty pre-enumeration list.
- Surface enumeration health as an "Inventory:" line in the App
  section so zero-device reports are self-explaining.
@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two staleness bugs in the Copy Diagnostics feature that surfaced after the agent-restart rework: agent status is now read from the live AgentLink (flips to Unreachable on disconnect) instead of a never-invalidated cached copy, and the inventory snapshot is gated on InventoryHealth::Ready so a report captured during the post-restart enumeration window can't show an empty device section while the UI is still showing devices.

  • AppState::last_status and store_agent_snapshot are removed; agent_status() (backed by AgentLink) is used directly, making the report's "Agent" line reflect the real connection state.
  • store_inventory_snapshot is now only called when InventoryHealth::Ready, mirroring the existing guard on refresh_inventories; both updates happen in the same update_global closure so device_list and last_inventory stay consistent.
  • A new InventoryState enum and inventory field on AppInfo surface "ready / scanning / unavailable" in the rendered report, giving context for a zero-device section.

Confidence Score: 5/5

Safe to merge — the changes are narrowly scoped to diagnostic snapshot collection and do not touch device control, IPC command dispatch, or any UI render path.

Both halves of the fix (live agent status read and gated inventory snapshot) follow the same consistency rule already used by refresh_inventories. The two updated state writes happen in the same update_global closure so device_list and last_inventory cannot diverge. Test coverage was extended to all four InventoryState / connectivity combinations. The removed last_status field had no other consumers, and agent_status() was already the correct source for all render-path reads.

No files require special attention.

Important Files Changed

Filename Overview
crates/openlogi-gui/src/state.rs Removes last_status: Option field and store_agent_snapshot / last_status methods; renames to store_inventory_snapshot which now only stores on Ready polls.
crates/openlogi-gui/src/diagnostics.rs Switches agent-status read from stale last_status to live agent_status(), adds inventory_state mapping helper, and populates the new AppInfo.inventory field.
crates/openlogi-gui/src/main.rs Splits store_agent_snapshot call into a ready-gated store_inventory_snapshot; both the inventory merge and snapshot store are now behind the same InventoryHealth::Ready guard.
crates/openlogi-core/src/diagnostics.rs Adds InventoryState enum, AppInfo.inventory field, rendering logic, and three new/updated tests (9 total) covering ready, scanning, unavailable, and unreachable-agent states.

Sequence Diagram

sequenceDiagram
    participant Poll as IPC Poll loop
    participant State as AppState
    participant Diag as diagnostics::collect()
    participant Report as DiagnosticsReport

    Note over Poll,State: Agent restart / reconnect window
    Poll->>State: GuiUpdate::Unreachable
    State->>State: set_agent_link(Unreachable)

    Poll->>State: "GuiUpdate::Ready { inventory: Scanning }"
    State->>State: "set_agent_link(Ready(status{Scanning}))"
    Note right of State: ready=false → skip refresh_inventories, skip store_inventory_snapshot

    Poll->>State: "GuiUpdate::Ready { inventory: Ready }"
    State->>State: refresh_inventories() → device_list updated
    State->>State: store_inventory_snapshot() → last_inventory updated

    Note over Diag,Report: User copies diagnostics (at any point)
    Diag->>State: agent_status() [live AgentLink]
    State-->>Diag: "None (Unreachable) OR AgentStatus{inventory}"
    Diag->>State: last_inventory() [gated snapshot]
    State-->>Diag: last completed enumeration
    Diag->>Report: "AppInfo{agent, inventory: None/Scanning/Ready/Unavailable}"
    Diag->>Report: receivers + devices from gated snapshot
Loading

Reviews (1): Last reviewed commit: "fix(gui): keep the diagnostics report tr..." | Re-trigger Greptile

@AprilNEA AprilNEA merged commit 85b0c36 into master Jun 13, 2026
8 checks passed
@AprilNEA AprilNEA deleted the fix/diagnostics-live-agent-state branch June 13, 2026 08:19
This was referenced Jun 12, 2026
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.

1 participant