Skip to content

fix(verification): bound the KiCad UI health check - #283

Closed
nordic-style wants to merge 2 commits into
mixelpixx:mainfrom
nordic-style:codex/fix/bound-kicad-ui-health-check
Closed

fix(verification): bound the KiCad UI health check#283
nordic-style wants to merge 2 commits into
mixelpixx:mainfrom
nordic-style:codex/fix/bound-kicad-ui-health-check

Conversation

@nordic-style

@nordic-style nordic-style commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

User-visible problem and scope

check_kicad_ui.timeout_seconds was advertised but ignored. A caller could request a short health-check deadline and still wait on the fixed process/IPC behavior, while the response implied the timeout was meaningful.

This PR applies one bounded deadline across the complete KiCad process check and IPC ping. It also recognizes standalone pcbnew/eeschema processes and treats responsive IPC as authoritative running evidence.

Refs #251

Root cause and design

The handler never read the schema field and performed its phases independently. It also checked only the top-level kicad process and skipped IPC entirely when that process was absent, so a standalone macOS pcbnew returned running: false while open_project was successfully using its socket.

The handler now validates an integer range of 1–300 seconds, wraps the complete health future in one Tokio timeout, checks kicad, pcbnew, and eeschema, and always probes IPC. A responsive endpoint is sufficient running evidence; process_detected remains separate diagnostic evidence. Timeout failures are typed and do not wait for the abandoned inner future.

Compatibility and migration

The existing default remains 5 seconds. Valid callers now get the documented bound; invalid types/ranges get a structured invalid_argument. The check remains read-only.

Validation

  • cargo test -p konnect-core health_ --locked
  • cargo test -p konnect-core standalone_editors_count_as_kicad_ui_processes --locked
  • cargo test -p konnect-core responsive_ipc_is_sufficient_running_evidence --locked
  • cargo clippy --workspace --locked --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • Combined-series CI commands: docs, Clippy, and format pass; 585/588 konnect-core tests pass, with only the same three installed-Device:R fixture-shadowing failures reproduced on unmodified main.
  • Live macOS test with standalone pcbnew: running: true, process_detected: true, and ipc_responsive: true in 39 ms.

Risk and rollback

Risk is limited to health-check timing and cancellation. No project state is touched. Tests use a deliberately pending future to prove the outer deadline returns promptly. Rollback is one commit.

Validate timeout_seconds and apply one Tokio deadline to process detection plus IPC ping, reporting timeout state and elapsed time explicitly.

Refs mixelpixx#251
Treat pcbnew and eeschema as GUI processes on every platform, and let a responsive IPC endpoint prove that KiCad is running even when process inspection misses its launcher. Report process detection separately for diagnostics.\n\nRefs mixelpixx#251
@nordic-style

Copy link
Copy Markdown
Contributor Author

Superseded by #285, which now contains this fix rebased onto current main together with the remaining #251 work and the catalogue-wide guard. Closing this satellite so the series has one coordinated review path.

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