feat(dsh): expose observed recall and capture status through /pc - #1600
Conversation
Teingi
left a comment
There was a problem hiding this comment.
Two status-reporting issues reproduced through the registered /pc command with real HTTP responses on this head. The existing 238 unit tests and all 4 pinned DSH/SQLite runtime tests passed; the runtime suite used the local deterministic model fixture.
| cancellationReason(signal) === 'deadline_exceeded' ? 'TimeoutError' : 'AbortError')) : error | ||
| const { state: _state, operation: _operation, ...failure } = operationFailure(OPERATIONS[stage], observedError) | ||
| record(stage, { ...failure, state: 'unavailable', | ||
| ...(writeDispatched ? { confirmation: 'unconfirmed' as const } : {}) }) |
There was a problem hiding this comment.
[P2] Distinguish definite rejection from an unconfirmed write
A complete HTTP 401 response to automatic capture produces both authentication_failed / http_status: 401 and confirmation: unconfirmed in /pc. Capture and flush pass writeDispatched=true for every failure, so this field also labels definite authentication rejection as a write that may have taken effect. Please reserve unconfirmed for genuinely uncertain outcomes and distinguish responses that establish rejection. I reproduced this with a real local HTTP server and the registered plugin command.
There was a problem hiding this comment.
Rechecked on b36d5cc82a538d1d9a6adcf557063f35e4bab654; this still reproduces through the registered /pc command with a real HTTP server. A complete 401 response reports authentication_failed and http_status: 401, but also confirmation: unconfirmed. Please distinguish definite rejection from an uncertain write outcome instead of setting this field for every dispatched write failure. A regression test should assert that a complete authentication rejection is not labeled unconfirmed, while an in-flight timeout remains unconfirmed.
There was a problem hiding this comment.
Thanks for the reproduction and recheck, @Teingi. You were right: the earlier test checked the authentication code but missed the contradictory confirmation field. Fixed in 1620793.
Observed 401/403 responses now report confirmation: rejected for both capture and flush. A capture rejection skips flush with capture_rejected. In-flight timeouts, incomplete 202 responses and HTTP 5xx still retain uncertain write outcomes; known pre-send failures do not imply a possible write. The documentation explicitly limits rejection to the failed request, without claiming that earlier capture or flush work was rolled back.
The new real-HTTP registered-command regressions assert the rejected confirmation and the timeout/202 controls. The pinned DSH host suite also checks complete and stalled capture/flush rejections with the installed bundle. Validation: 255 package tests, 9 HTTP integration tests, 5 pinned-host runtime tests and 38 related Python tests passed; all 18 CI checks are green on this head (Main).
Please take another look when convenient, and thank you for catching this gap.
| position = result.kind === 'json' ? sourcePosition(result.value) : undefined | ||
| captureStatus = result.status | ||
| } catch (error) { | ||
| observation?.fail('capture', error, true, input.signal) |
There was a problem hiding this comment.
[P2] Preserve observed HTTP rejection when reading the body fails
With a real HTTP server that sends 401 plus X-PowerContext-Request-ID: req-review-http, starts the JSON body, then stalls, /pc reports only request_timeout and confirmation: unconfirmed; both http_status and request_id disappear. This sends users toward latency troubleshooting despite an observed authentication rejection. The existing client drops the response metadata on body-read failure, so forwarding that error here cannot preserve it. Please carry the received status and validated request ID through the failure and retain the rejection alongside the body timeout. Reproduced with requestTimeoutMs: 100 and the overall automatic deadline still active.
There was a problem hiding this comment.
Rechecked on b36d5cc82a538d1d9a6adcf557063f35e4bab654; this still reproduces with the real HTTP transport. The server sends 401 and a valid request ID, then stalls the response body. With requestTimeoutMs: 100 and the overall automatic deadline still active, /pc reports only request_timeout; both http_status and request_id are missing. Please carry the received status and validated request ID through body-read failures, preserving the authentication rejection alongside the body timeout. The stalled-202 timeout control passes. All 238 existing unit tests and all 4 pinned DSH/SQLite runtime tests also passed in this recheck; the runtime suite used the local deterministic model fixture.
There was a problem hiding this comment.
Thanks for pinpointing the response-body boundary, @Teingi. Fixed in 1620793: the client now carries the received HTTP status and validated request ID through a typed response-body failure, including the API discovery path. Outer transport wrapping and cancellation normalization preserve that evidence.
A 401 followed by a body timeout now reports authentication_failed, http_status: 401, the request ID, confirmation: rejected, and failure_phase: response_body / response_body_error: request_timeout. Doctor, direct results and bounded diagnostics retain the relevant response evidence as well. A stalled 202 remains unconfirmed and does not start flush; an unread 404 body is not treated as proof of a missing route.
Regression coverage uses real HTTP responses through the registered /pc command, including body timeout, disconnection, size limits, unsafe request IDs and private cancellation reasons. The real pinned DSH host suite separately verifies stalled capture/flush 401 responses and the stalled-202 control. All 18 CI checks are green on this head, including DSH package and host validation and SQLite/OceanBase acceptance.
I appreciate the detailed reproduction; it exposed a transport case the previous mocks did not exercise.
Which issue or RFC does this PR close?
Closes #1597. Implements F of #1450 without closing the parent tracker. Based on master
545b1609; independent of D (#1522) and E.Rationale for this change
After a normal prompt,
/pcshowed Scope and Server origin, but could not explain whether automatic recall, capture or insertion ran, succeeded, skipped input or failed. A healthy Doctor report does not establish what happened in that conversation. The status also needs to distinguish a rejected request from an unknown write outcome, and retain an observed HTTP rejection when reading its body fails.What changes are included in this PR?
/pccommand. Include attempt/turn identity, timestamps, age, safe failure/skip reason and freshness.confirmation: rejected) from uncertain write outcomes (unconfirmed). Known pre-send failures do not imply a possible write. A later rejected flush request does not undo earlier processing; capture rejection skips flush withcapture_rejected.failure_phase: response_bodyandresponse_body_error. Keep the established logger outcome vocabulary and redact unsafe IDs/content. A stalled 202 remains unconfirmed; an unread 404 body cannot establish a missing route; HTTP 5xx does not prove rollback.Are there any user-facing changes?
/pckeeps its Scope/Server output and adds anautomaticobservation.prepare: emptycan coexist withcapture: accepted. Source acceptance and a completed flush do not prove Memory production;injection: appendedproves only insertion into pre-step messages, not model consumption.For a 401 followed by a stalled response body, the capture observation reports
authentication_failed, HTTP 401, the validated request ID,confirmation: rejected, andresponse_body_error: request_timeout. Credentials remain the primary recovery action. A stalled successful response remains unconfirmed and cannot start dependent flush work.Freshness describes local observation age/applicability, not Server Memory freshness. Status remains readable with explicitly stale history if the current read-only Scope check fails. The plugin retains no prompt, recalled content, credentials or raw exception in the observation.
HTTP contracts, Server persistence, automatic-path budgets, tool/Skill names and host approval rules are unchanged. Direct 403 failures now identify authorization failure explicitly; logger outcomes retain the existing plugin diagnostic contract.
How was this change tested?
pnpm --dir integrations/dsh/plugins/powercontext run test:all: 264 passed (255 package tests, including 17 new real HTTP regressions, plus 9 HTTP integration tests).pnpm --dir integrations/dsh/plugins/powercontext run test:e2e:runtime: 5 passed with the real pinned DSH host, packaged plugin, SQLite Server and deterministic local model fixture. Covers clean installation/recall, failure recovery, Scope isolation, complete/stalled capture and flush rejection, and the stalled-202 control. One local run failed at SDK initialization before F assertions; the complete rerun passed at the unchanged 30-second initialization deadline. The transient initialization cause was not established.uv run pytest tests/test_dsh_cli.py tests/e2e/test_dsh_http_chain.py tests/test_integration_manifest.py -q --tb=short: 38 passed.uv run ty check --python-platform linux src scripts tests integrationspassed. The local whole-workspacety-checkhook is excluded because it includes unrelated personal documents and existing Windows-specific errors; CI runs unmodified quality checks.tsc --noEmit --allowImportingTsExtensionsprobe still reports unchanged baselineconfig.ts(stripSlash) and existing test typing errors. The modified runtime sources and new HTTP regression file produced no diagnostics. This probe is not counted as passing validation.1620793873ba3d13b6b07d1aaed0096a4f1e9397: Main (Python 3.11–3.14, quality, DSH/Pi/OpenDAL packages, website and Windows links), E2E harness (SQLite/OceanBase), native personal service (Windows/macOS/Linux), license header, and CLA.AI usage statement
OpenAI Codex (GPT-6 Astra) assisted with source inspection, implementation, code review, documentation and tests. Runtime acceptance uses the real pinned DSH host and Server with a deterministic local model fixture; it does not claim live-model routing qualification.