feat: add host-visible integration diagnostics - #1360
Conversation
| return "version_mismatch" | ||
| if error.status == 503: | ||
| return "server_unavailable" | ||
| if error.status in {404, 409, 422}: |
There was a problem hiding this comment.
[P1] Suppressing 404/409/422 by status alone makes automatic integration failures silent. I reproduced 422 /v1/context/prepare returning no context and no diagnostic; capture and flush have the same issue for their real endpoints. The new capture test uses /v1/memory/entries/get even though _capture_prompt calls /v1/sources/content, so it misses this path. Please classify by operation, actual endpoint, and structured error code, and keep automatic prepare/capture/flush failures visible.
There was a problem hiding this comment.
The Codex and Claude paths are fixed, but this remains open for DSH, Pi, OpenClaw, and Hermes. Their classifiers still return undefined or None for 404/409/422 before considering the automatic operation and real endpoint. On the current head I reproduced silent 422 context_prepare, 422 capture, and 409 flush failures in DSH and OpenClaw, and the same three classifications returning None in Hermes; Pi has the same implementation. Please carry the operation/path/error-code classification and real-endpoint regression tests across those adapters.
| else: | ||
| import fcntl | ||
|
|
||
| fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX) |
There was a problem hiding this comment.
[P2] This lock is unbounded on the hook critical path (LOCK_EX / LK_LOCK). With another process holding the file lock, I reproduced should_emit() blocking for 6.5 seconds, so a diagnostic cooldown can stall an otherwise fail-open host invocation. Please use non-blocking locking or a very small bounded retry, fall back to emit or skip on contention, and add a subprocess contention regression test.
| assert errors.getvalue() == "" | ||
|
|
||
|
|
||
| def test_host_diagnostic_is_throttled_across_hook_invocations( |
There was a problem hiding this comment.
[P2] This is still a direct Python main() call, not a host-level fixture. It proves the JSON shape but not that Codex actually renders systemMessage; the Claude test has the same gap. RFC 1299 requires an acceptance test or recorded host fixture showing that the selected channel is user-visible. Please exercise the installed hook through each real host, or check in a recorded host fixture from that boundary.
There was a problem hiding this comment.
fixed and tested in codex
| except HTTPError as error: | ||
| raise _HttpStatusError(error.code) from error | ||
| try: | ||
| error_body = error.read(_MAX_RESPONSE_BYTES + 1) |
There was a problem hiding this comment.
[P2] Keep error-body parsing under the shared wall-clock deadline. This direct error.read(...) bypasses _read_response(..., deadline=request_deadline), so a peer that trickles one byte within each socket timeout can exceed the hook budget indefinitely. On the current head, a 422 response sending one byte every 20 ms took 0.714 s despite a 0.1 s deadline; Claude has the same branch. Please use a deadline- and size-bounded error-body reader and add a slow-4xx regression test.
2 participants
Which issue or RFC does this PR close?
This PR implements the first step of #1298. It does not close #1298.
This PR is downstream of #1343 and should be reviewed after #1343 is merged.
Rationale for this change
When PowerContext is unavailable or returns an incompatible response, integrations currently provide limited or inconsistent feedback. Users need a bounded, fail-open diagnostic that explains the failure and points to
powercontext doctorwithout exposing request details or disrupting the host.What changes are included in this PR?
authentication_failedversion_mismatchserver_unavailableinvalid_responsestderrpowercontext.codex.recallstderrpowercontext.claude_code.recallpowercontext.dshpowercontext.openclawpowercontext.pipowercontext.hermespowercontext doctorrecovery hint for unavailable servers.Are there any user-facing changes?
Yes.
Users now receive a bounded diagnostic when PowerContext is unavailable or returns a classified failure. Normal host execution continues, and no URL, token, response body, or user content is included in the diagnostic.
There are no breaking API or persistence-format changes.
How was this change tested?
uv run pytest tests/integrations/test_hermes_provider.py -q— 36 passeduv run ruff check integrations/hermes/plugins/powercontext/commands.py tests/integrations/test_hermes_provider.py— passednode --checkpowercontext doctorrecovery metadata.AI usage statement
Implemented and reviewed with OpenAI Codex