fix(ipc): report why KiCad did not answer a health check

[mixelpixx]

Current maintainer refresh

Rebased onto upstream/main 71d9e0cc12c6ad72c847f8eb8adbce17e51792ac after #579 and #591. New head: 8a50244 (full SHA recorded in the refresh comment). Both annotation and IPC-failure served-dispatch test modules are preserved. Range-diff shows only a changed insertion context; no production behavior was changed by the refresh. Required CI must rerun on this head. Evidence below referencing earlier heads is historical contributor evidence, not validation of this refreshed head.

Closes #532
Part of #531

Problem

check_kicad_ui and open_project reduced every IPC failure to one false. No configured address, nothing listening, a listener that refused this account, and a listener that is not KiCad all looked the same as KiCad being closed. #532's reporter had KiCad listening while Konnect ran as the Codex sandbox account, and nothing Konnect returned could tell them that. The same root cause sat under #300 in August.

send_command dials synchronously, so NNG already returns a distinct error for each case. ping() discarded it.

Measured first

These are Windows 11 named pipes dialed with Konnect's own socket options (Req0, 5 s send, 30 s receive):

Endpoint	NNG dial result	Time
no pipe bound	ConnectionRefused	0.1 ms
pipe whose descriptor leaves this account only Everyone's read access	PermissionDenied, and the listener never sees a client	0.1 ms
listener that accepts, then closes	Closed	0.3 ms
listener that accepts and never speaks	TimedOut	10.0 s

The nng source confirms the Unix side. posix_ipcdial.c maps ENOENT to ECONNREFUSED, posix_debug.c maps EACCES to EPERM, and ipc.c hard-codes a 10-second negotiation timeout. Unix sockets therefore land on the same variants. I can't run Unix here; CI's Linux and macOS test jobs pass the Unix access_denied and handshake_failed tests on this PR's head, and they are the evidence for that half.

Change

• konnect-ipc: the dial's nng::Error is classified into a typed UnreachableReason (not_configured, no_listener, access_denied, handshake_failed, transport_error), never matched from message text. TransportUnreachable stays the constructible unit struct it is on main; the reason travels in a private context layer directly above it whose Display is the message, so error chains render byte-for-byte as before and old-way construction keeps compiling (classifies as unreachable, reports transport_error). unreachable_reason(&anyhow::Error) reads that layer with anyhow's downcast_ref. The dial's error message explains the classified cause instead of listing every possible one.
• ping_outcome() returns PingOutcome::{Responsive, Unreachable { reason, message }, RequestFailed { message }}, so a request that did not complete and may have reached the endpoint (an explicit KiCad status such as AS_NOT_READY proves receipt; a receive timeout or malformed reply does not) stays apart from one that never arrived. ping() keeps its signature and its warning log.
• check_kicad_ui and open_project add ipc_failure: {kind, message}. It is null when no failure kind was established: the Ping succeeded with AS_OK, or check_kicad_ui's own deadline expired first (reported as timed_out: true). A request that did not complete is request_failed, not null, whether or not it reached KiCad; an explicit KiCad status in message proves receipt. access_denied proves an authorization refusal, not who owns the endpoint: the docs name a different account or a restrictive ACL as likely causes and same-user, out-of-sandbox operation as the known workaround. open_project's message follows the kind: access_denied, handshake_failed, and request_failed get their own headline (recorded verbatim in the migration entry), and the rest keep the existing one.
• Docs: API_MIGRATIONS.md entry (additive field; nothing removed or renamed; changed open_project.message wording and the reworded dial errors recorded; null defined). TROUBLESHOOTING.md gains a per-kind table and the out-of-sandbox HTTP setup from Windows: open_project reports IPC unreachable although KiCad api.sock pipe exists #300. tool-directory.md rows updated. The check_kicad_ui description names the kinds and the handshake timing limit below.

Limit, stated rather than hidden: a listener that accepts and never negotiates holds the dial for NNG's fixed 10 s. That is longer than check_kicad_ui's default 5 s budget, which reports timed_out first. The description, the troubleshooting entry, and the migration note all say to pass timeout_seconds above 10.

Not changed: IpcFailure::Unreachable(String) and every fallback decision. The reason is read only where a response reports it. attempt_ipc_write's gating is untouched.

Changed tool behavior

Per docs/RELIABILITY_CONTRACT.md (landed in #550 after the last review round), for check_kicad_ui and open_project:

Behavior	Contract and evidence for this change
Accepted inputs and declared defaults	Unchanged: check_kicad_ui.timeout_seconds integer 1–300, default 5; open_project.path optional. No argument added.
Invalid/unsupported inputs and structured errors	Unchanged: out-of-range or non-integer timeout_seconds returns invalid_argument naming the field (health_timeout_is_bounded_and_typed).
Target, data source and prerequisite state	The configured IPC address (ipc_address, KICAD_API_SOCKET, or discovery); the result is a live Ping with no file fallback. ipc_failure.message names the redacted endpoint that was dialled.
Observed changes and preserved unrelated objects	Not applicable: both tools are read-only health checks and mutate nothing.
Failure before/after mutation, including applied work	Not applicable (no mutation). What changed is failure classification: ipc_failure.kind is derived from the typed nng::Error at the dial, never from message text; a request that did not complete and may have reached the endpoint is request_failed (an explicit KiCad status proves receipt), kept apart from the unreachable kinds.
Recovery from partial/uncertain results without repeating applied work	ipc_failure: null means no kind was established (AS_OK, or timed_out: true from the check's own deadline). Each kind maps to one documented fix in TROUBLESHOOTING.md; handshake_failed needs timeout_seconds above 10 to be observed.

Tests

The listeners in crates/konnect-ipc/tests/unreachable_reasons.rs are real pipes and sockets. No KiCad is involved.

• All platforms: an address nothing listens on reports no_listener.
• Windows: the restricted-descriptor pipe reports access_denied, and the listener's accept times out, proving the refusal happens before connection. A listener that closes reports handshake_failed. A silent listener reports handshake_failed in under 20 s.
• Unix: a socket with mode 000 reports access_denied (skipped as root). A listener that closes reports handshake_failed.
• mock_server_test.rs: an AS_BAD_REQUEST answer is RequestFailed, not unreachable, and an AS_OK answer is Responsive.
• Unit: every nng::Error maps to the reason whose fix applies; every reason has a distinct name and explanation; an unconfigured client reports not_configured; the reason is read from the marker, and a look-alike message yields None.
• konnect-core: evidence shape; check_kicad_ui and open_project report no_listener end to end through their handlers; the open_project headline follows each kind.
• Served dispatch: ipc_failure_kind_and_message_survive_the_served_dispatch drives both tools through McpHandler::handle_message (tools/call), and ipc_failure_kind_and_message_reach_the_client_over_stdio drives the real binary over the stdio protocol with KICAD_API_SOCKET pointing at a socket nothing listens on; both assert ipc_failure.kind and message in the served result.

Neuters (each guard reverted alone, then the listed tests run):

Guard reverted	Caught by
PermissionDenied classified as transport_error	each_dial_error_maps_to_the_reason_whose_fix_applies, windows::a_pipe_that_refuses_this_account_reports_access_denied
ping_outcome drops the typed reason	an_address_nothing_listens_on_reports_no_listener, an_unconfigured_client_reports_not_configured, and all three Windows listener tests
check_kicad_ui returns ipc_failure: null	check_kicad_ui_names_why_ipc_did_not_answer
open_project ignores the kind for its headline	open_project_says_kicad_answered_when_it_answered_with_an_error
unreachable_reason ignores the detail layer (reports transport_error for everything)	six tests: the_reason_is_read_from_the_marker_never_from_the_text, an_unconfigured_client_reports_not_configured, an_address_nothing_listens_on_reports_no_listener, and the three Windows listener tests

The last row is why the second commit exists. On the first pass, that neuter left every test green: the only handler test reached no_listener, whose headline is the generic one. A mock answering AS_NOT_READY now drives the handler to request_failed. My first attempt at the third neuter didn't compile, so it proved nothing; it was redone with a replacement that builds. All four neuters were run with --no-fail-fast, so the real-pipe tests are shown catching them as well as the unit tests.

Head 104c5aa (one commit on e33b9a0): CI 10/10. Local gate on the same head: fmt, clippy -D warnings, --lib --tests 1842 passed / 0 failed, --doc, cargo xtask fix-doc-counts --check unchanged, reliability_contract 2 passed / 0 failed.

End to end, through the release binary over stdio

e2e532.py starts konnect.exe with only KICAD_API_SOCKET set, then calls check_kicad_ui and open_project against stand-in pipes bound by a separate process, and against pcbnew 10.0.5.

Endpoint	check_kicad_ui	ipc_failure.kind	open_project headline
no pipe	0.2 s	no_listener	"KiCad IPC is not reachable…"
pipe leaving this account only read access	0.2 s	access_denied	"The KiCad IPC endpoint refused this account, likely a different account or a restrictive ACL…"
pipe that accepts, then closes	0.2 s	handshake_failed	"…did not complete NNG's handshake, so it is probably not KiCad…"
silent pipe, timeout_seconds: 5	5.0 s, timed_out: true	null	—
silent pipe, timeout_seconds: 15	10.2 s	handshake_failed	—
pcbnew seconds after launch	0.3 s	no_listener (its server was not up yet)	"KiCad IPC is not reachable…"
pcbnew after 35 s	0.2 s, ipc_responsive: true	null	"KiCad IPC is available…"

An example message, verbatim: Cannot connect to KiCad IPC at ipc://C:\konnect-e2e532\readonly.sock: Permission denied. The operating system refused this account access to the endpoint. Likely causes: the endpoint was created by a different account, as when a sandboxed AI client (for example Codex on Windows) runs Konnect as a separate user, or a restrictive ACL. Known workaround: run Konnect as the same operating-system user as KiCad, outside the sandbox. Guide: https://github.com/mixelpixx/Konnect/blob/main/docs/TROUBLESHOOTING.md

I could not create a second Windows account, so the cross-account case itself was not run. The stand-in reproduces its access check: a descriptor that grants this account nothing beyond Everyone's read access.

Overlap

Earlier #523/#528/#530 overlap notes are obsolete after their integration into main. This refresh resolves the #579 overlap in mcp/handler.rs by preserving both independent test modules. All existing API migration entries are retained. No tools or generated counts changed.

🤖 Generated with Claude Code

[mixelpixx]

Head 249cc89, CI 10/10. The only change since e999730 is the Linux clippy fix: Duration and Instant were imported at the top of unreachable_reasons.rs but used only in its Windows module. The Unix access_denied and handshake_failed tests pass on ubuntu and macOS.

[neusse]

Two public-contract corrections are needed before this can enter the merge queue.

• Correct docs/API_MIGRATIONS.md: it currently says “No existing field changed value” and then documents that open_project.message now changes for access_denied, handshake_failed, and request_failed. Say instead that no existing field was removed or renamed, and explicitly record the changed message wording. Also describe ipc_failure: null as “no failure kind was established” rather than only “KiCad answered,” because the health-check timeout path also returns null.
• Account for the exported Rust API change: TransportUnreachable changes from a constructible unit struct to a field struct containing reason. Either preserve the old constructible shape with a separate typed marker/context, or document this source-compatibility break explicitly in the migration entry.

On exact head 249cc89a32267c07d9f7671e71bcaa76a37131d7, the typed classification, handler behavior, platform tests, ten required checks, Closes #532, and Part of #531 accounting otherwise look sound. The reporter's exact cross-account Codex run remains useful validation debt, not a merge wall: the restricted-descriptor pipe provides an adequate deterministic proxy. Please make the focused compatibility/documentation correction and rerun CI.

[neusse]

Merge-train placement: please make the two requested compatibility/documentation corrections, but hold the final reconstruction and CI run until #523 lands. The shared-file order is #530 → #528 → #523 → #535, so each branch is rebuilt once rather than after every predecessor. #535 remains the terminal PR for #532 and partial work for #531.

[mixelpixx]

Both corrections are made and committed locally. Per the train, they're held until #523 lands, then pushed once with the reconstruction.

• Compatibility: I kept the old shape. TransportUnreachable is a unit struct again, exactly as on main. The reason travels in a private context layer directly above the marker. Its Display is the message, so error chains render byte-for-byte as before. unreachable_reason finds that layer with anyhow's downcast_ref, which searches every context layer. A marker built the old way still classifies as unreachable and reports transport_error. New tests pin both behaviours: construction the old way, and the unchanged chain rendering. Removing the lookup fails six tests, including the real-pipe ones.
• Migration entry: it now says nothing was removed or renamed, and records verbatim the changed open_project.message for access_denied, handshake_failed, and request_failed, plus the reworded dial errors. ipc_failure: null is defined as "no failure kind was established": KiCad answered, or check_kicad_ui's own timeout expired first. The same definition is in TROUBLESHOOTING.md and the tool description.

Local gate on the corrected branch: 1797 passed / 0 failed, fmt, clippy -D warnings, doctests, counts unchanged. If you'd rather the break were documented than avoided, say so before #523 lands and I'll reconstruct that way instead.

[neusse]

#523 has landed as 503b7f94e743b2af917ce835bdcb74d0b92c71e1, and the full post-merge gate passes. #535 is now the next PR in the completed #530 -> #528 -> #523 -> #535 train.

Please reconstruct the corrected local implementation once onto current main, keeping only #535's unique commit(s), preserve all preceding migration/tool-directory entries, rerun all ten required checks, update the exact-head/current-base evidence, and reply with the new head SHA. The previously described compatibility-preserving TransportUnreachable shape and corrected migration wording are the accepted direction.

[mixelpixx]

Reconstructed onto current main (503b7f94e743b2af917ce835bdcb74d0b92c71e1, which is #523's merge), with both corrections you asked for. New exact head: d34bf442da3af9d56dbc68aaba23195d5b6656ef, one commit; the five review-round commits are squashed into it and its message carries the full story, including the compatibility decision.

Corrections, as held since 01:17Z:

• TransportUnreachable is a unit struct again, exactly as on main. The reason travels in a private context layer directly above the marker; its Display is the message, so error chains render byte-for-byte as before. unreachable_reason finds it with anyhow's downcast_ref, which searches every context layer. A marker built the old way still classifies as unreachable and reports transport_error; two new tests pin the old-way construction and the unchanged chain rendering.
• The migration entry now says nothing was removed or renamed, records the changed open_project.message wording verbatim for access_denied, handshake_failed, and request_failed, notes the reworded dial errors, and defines ipc_failure: null as "no failure kind was established" (KiCad answered, or check_kicad_ui's own timeout expired first). The same definition is in TROUBLESHOOTING.md and the tool description.

Reconstruction evidence. Per-file hunks are identical to the corrected pre-rebase tree (4a4b3f3, compared with index lines stripped and @@ headers normalised for the two files main also changed above our hunks); the migration entry's added lines are identical; it sits directly below #523's entry. The tree contains no conflict markers (checked with git grep).

All ten required checks green on d34bf44. Local gate on the same head: fmt, clippy -D warnings, --lib --tests 1813 passed / 0 failed, --doc, fix-doc-counts --check unchanged. Neuter results on the corrected code stand from the 01:17Z run (removing the detail lookup fails six tests, including the real-pipe ones). e2e532.py re-run against this head's release binary: no_listener, access_denied, handshake_failed (closing and silent listeners), and the 5 s timed_out/null case all report as in the PR body's table.

PR body updated to the new head. Ready for the exact-head check. Terminal: Closes #532, Part of #531.

[neusse]

One small public-contract wording correction remains on exact head d34bf442da3af9d56dbc68aaba23195d5b6656ef.

The implementation correctly returns ipc_failure.kind: "request_failed" when KiCad answers the Ping with an error status such as AS_NOT_READY. Several descriptions instead say ipc_failure is null when “KiCad answered,” which contradicts that behavior. Please replace that wording with “the Ping succeeded with AS_OK” (or equivalent), while retaining the other null case: check_kicad_ui's outer deadline expired before an outcome was established.

Please correct the wording consistently in:

• crates/konnect-core/src/tools/mod.rs (ipc_failure_evidence documentation)
• crates/konnect-core/src/tools/verification.rs (check_kicad_ui tool description)
• crates/konnect-ipc/src/client.rs (failure_kind and failure_message documentation)
• docs/API_MIGRATIONS.md
• docs/TROUBLESHOOTING.md
• the PR description

Everything else at this head passes review: current main is incorporated, all ten required checks are green, the public TransportUnreachable unit-struct shape is preserved, tests and platform coverage are sound, and Closes #532 / Part of #531 is correct. This is a focused wording fix; no runtime redesign is requested.

[mixelpixx]

All four corrected on the new exact head 104c5aa67aac570289d9253571aab762ebae9fd3 (one commit, reconstructed onto current main e33b9a0):

1. request_failed no longer claims receipt. Every public explanation now says the request did not complete and may have reached the endpoint, with an explicit KiCad status in message as the proof of receipt and a receive timeout or malformed reply as the cases that prove nothing: PingOutcome::RequestFailed doc, ipc_failure_evidence doc, the check_kicad_ui description, the open_project headline (now "The KiCad IPC request did not complete and may have reached the endpoint; a KiCad status in ipc_failure proves receipt, and KiCad may still be starting."), the migration entry, and the troubleshooting row, whose "what to do" now splits on whether a KiCad status is present. The runtime classification is unchanged, as you allowed.
2. access_denied proves a refusal, not ownership. The AccessDenied doc and explanation(), the open_project headline, the check_kicad_ui description, the migration entry and the troubleshooting section now describe a different account or a restrictive ACL as likely causes and same-user, out-of-sandbox operation as the known workaround.
3. Served-dispatch regressions. ipc_failure_kind_and_message_survive_the_served_dispatch drives check_kicad_ui and open_project through McpHandler::handle_message (tools/call) and asserts ipc_failure.kind/message in the served result; ipc_failure_kind_and_message_reach_the_client_over_stdio does the same through the real binary over the stdio protocol (crates/konnect/tests/protocol_stdio.rs) with KICAD_API_SOCKET pointing at a socket nothing listens on and the config search isolated. Both pass on this head.
4. Commit message now says the null case is "the Ping succeeded with AS_OK", and its request_failed/access_denied paragraphs match the docs.

All ten required checks green on 104c5aa67aac570289d9253571aab762ebae9fd3. Local gate on the same head: fmt, clippy -D warnings, --lib --tests 1842 passed / 0 failed, --doc, fix-doc-counts --check unchanged, reliability_contract 2 passed / 0 failed. e2e532.py through the release binary: every call accepted, no_listener, access_denied, handshake_failed (closing and silent listeners) and the 5 s timed_out/null case all report as in the body's table, with the new access_denied and request_failed wording in the served messages.

PR body updated (evidence, the new headline wording, the served-dispatch tests). Ready for the exact-head check. Terminal: Closes #532, Part of #531.

[neusse]

Please reconstruct this once onto current main and rerun the final exact-head evidence.

The substantive review is now complete on 104c5aa67aac570289d9253571aab762ebae9fd3: the truthful request_failed boundary, authorization wording, compatibility shape, served tools/call coverage, real stdio coverage, documentation, and Closes #532 / Part of #531 accounting all satisfy the prior requests.

The remaining blocker is branch freshness. This head's actual parent/merge-base is c9d93b366c332035566e56459cea8b3fdbee7cd5, while current main is 829403fe0caf97f690c10a62c732da10bac83ba9 after #570, #571, and #573. The PR body also names e33b9a0 as its base, which does not match the commit graph. A local merge-tree check finds no conflicts, so this should be mechanical:

• reconstruct the one unique fix(ipc): report why KiCad did not answer a health check #535 commit onto 829403f;
• preserve the current corrected content and all newer main entries;
• update the PR body's exact base/head statements;
• rerun the ten required checks and reply with the new full head SHA.

Once that refreshed exact head is green, this is next for final review in the merge train.

[mixelpixx]

Reconstructed once onto current main (829403f, #573's merge). New exact head: e407850e55f44ba9b32567012945424ac07adb27, still one commit; every per-file hunk is byte-identical to the reviewed 104c5aa (compared with index lines stripped and @@ headers normalised across all fourteen files), the migration entry is unchanged and in place, and the tree has no conflict markers.

All ten required checks green on e407850. Local gate on the same head: fmt, clippy -D warnings, --lib --tests 1848 passed / 0 failed, --doc, fix-doc-counts --check unchanged, reliability_contract green. e2e532.py through the release binary: no listener -> no_listener (0.2 s); read-only pipe -> access_denied (0.2 s); listener that closes -> handshake_failed (0.2 s); silent listener under the default 5 s budget -> timed_out: true with ipc_failure: null; silent listener under a 15 s budget -> handshake_failed at 10.2 s (NNG's 10 s negotiation limit). All through check_kicad_ui on the e407850 release build, under #551's dispatch-time schema enforcement.

Nothing changed but the base. Ready for the final exact-head check. Terminal: Closes #532, Part of #531.

[mixelpixx]

main moved again overnight (#584, #585), so I reconstructed once more before you got to the final check. New exact head: d9431df, one commit on 9ca98f1; every per-file hunk is byte-identical to the reviewed e407850/104c5aa (fourteen files, index lines stripped and @@ headers normalised), the migration entry is in place, no conflict markers.

All ten required checks green on d9431df. Local gate on the same head: fmt, clippy -D warnings, --lib --tests 1865 passed / 0 failed, --doc, fix-doc-counts --check unchanged, reliability_contract green. e2e532.py through the release binary built from this head: no listener -> no_listener (0.5 s); read-only pipe -> access_denied (0.4 s); listener that closes -> handshake_failed (0.4 s); silent listener under the default 5 s budget -> timed_out: true with ipc_failure: null; silent listener under a 15 s budget -> handshake_failed at 10.5 s (NNG's 10 s negotiation limit). check_kicad_ui and open_project agree on the kind in every case, under #551's schema enforcement and #546's closed argument sets.

Nothing changed but the base. Terminal: Closes #532, Part of #531.

[neusse]

The substantive review is complete on d9431df: independent spec and standards reviews found no material blockers, its parent is current main 9ca98f1, and all ten required checks pass. The stable patch matches the previously accepted e407850 content. Closes #532 / Part of #531 accounting is correct.

One queue update: #579 now has its narrow recovery correction and is awaiting George exact-head approval. A merge simulation of the two heads finds one overlap in crates/konnect-core/src/mcp/handler.rs, where both PRs append independent test modules. We will land #579 first; then reconstruct #535 once onto resulting main, preserving BOTH annotation-dispatch and IPC-failure-dispatch test modules. No production redesign is requested. Migration entries merge automatically in this simulation; still verify all existing entries are retained.

Please hold final reconstruction/CI until #579 lands, then update the stale body head/base and historical #523/#528/#530 overlap section, rerun required checks, and reply with the new full head SHA. That refreshed head gets the final merge approval request. The exact reporter cross-account observation remains disclosed validation debt; #531 peer-identity work is not claimed complete.

[neusse]

The dependency has landed: #579 merged into main at 99da7d7. #535 is next in the least-resistance order. Please refresh against this latest main once, preserving both the annotation served-dispatch tests and your IPC-failure dispatch tests in mcp/handler.rs. Update the stale base/head and dependency notes in the PR body, then rerun required CI on the refreshed head. Our review of d9431df found no material code blockers; this is integration and final-head validation, not a request for redesign. Keep Closes #532 and Part of #531 intact.

[neusse]

Maintainer refresh complete: new exact head 8a50244 on current main 71d9e0c. Resolved only the handler.rs append overlap, preserving both annotate_dispatch_tests and ipc_failure_dispatch_tests. Range-diff versus d9431df shows only changed insertion context; production hunks unchanged. All migration entries retained and historical overlap notes corrected. Formatting and strict workspace Clippy pass; workspace tests and doctests are running locally, and new required CI has been triggered. No merge armed pending final-head evidence and approval.

[neusse]

Local final-head validation is complete on 8a50244: cargo fmt --all -- --check; cargo clippy --workspace --locked --all-targets -- -D warnings; cargo test --workspace --locked --lib --tests; cargo test --workspace --locked --doc all exited 0. Windows no-listener, access-denied, closed-listener and silent-handshake integration tests passed. Environment-dependent ignored tests remain unrun; no fresh live-KiCad or cross-account result is claimed. GitHub reports MERGEABLE and closingIssuesReferences contains #532 only; #531 remains partial. Waiting for required hosted checks and final-head merge approval.

[neusse]

Re-reviewed exact head 8a50244. Previous requested changes are resolved: public error wording distinguishes uncertain receipt from proven response and authorization from ownership; TransportUnreachable compatibility is preserved; migration semantics and served-dispatch regression coverage are present. The refresh preserves both annotation and IPC-failure dispatch tests, with unchanged production hunks. Local fmt, strict Clippy, workspace tests, and doctests exited 0; all ten required hosted checks passed on this head. Exact reporter cross-account observation remains disclosed validation debt; #531 is partial. George approved this exact head for merge.