feat(api): retrieve stored interpretation-run requests by extra-segment GET - #453
Conversation
…nt GET
Publish GET /v1/interpretation-runs/{idempotency_key}/request so operators
fetch the accepted create request without POST replay. scientific_authority
stays false. Naruon and LineageWeave are refused. NaruonLiveService stays
POST-only. Cancel extra-segment stays refused. ADR 0085.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Coverage evidence needs follow-up
Repository rules require 100% production line and branch coverage. The new parser, metric recursion, and service branches lack visible complete coverage evidence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if rest != "request" || encoded_id.is_empty() { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| let idempotency_key = decode_path_segment(encoded_id)?; | ||
| require_nonempty(&idempotency_key)?; | ||
| if idempotency_key.contains('/') || idempotency_key.contains('\0') { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| if idempotency_key.len() > INTERPRETATION_RUN_RETRIEVAL_ID_MAX_LEN { | ||
| return Err(OrchestratorLiveError::LimitExceeded); |
There was a problem hiding this comment.
🟡 Accepted keys become unretrievable
POST accepts idempotency keys containing / or exceeding 128 bytes, but interpretation_run_stored_request_path_id rejects them. Their stored requests can never be retrieved.
Prompt for agents
Align interpretation-run creation and stored-request retrieval identifier contracts. InterpretationRunRequest validation currently accepts idempotency keys that the new path parser cannot represent, including slash-containing and over-128-byte keys. Either reject these keys during POST validation before storing them, with compatibility consideration for existing callers, or define and implement a path encoding/length contract that can retrieve every accepted key. Add tests that POST each boundary case and then retrieve the stored request.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if payload.contains("tepp.scientific_acceptance.v1") { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); |
There was a problem hiding this comment.
🟡 Schema-token identifiers block retrieval
refuse_metrics_on_interpretation_run_stored_request_payload scans every string value for the schema token. Any valid identifier containing it makes its stored request unretrievable.
Prompt for agents
Replace the raw payload substring check with structural JSON validation that rejects the scientific-acceptance schema only in contract-bearing keys or fields. Ordinary opaque values such as idempotency_key, tenant_workspace_id, snapshot_id, and evidence_span_ids must remain retrievable when they happen to contain the same text. Add a POST-then-GET regression test with the token inside a valid string field.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if is_interpretation_run_stored_request_path(path) { | ||
| return self.get_interpretation_run_stored_request(path, &headers, body); | ||
| } |
There was a problem hiding this comment.
🟡 Oversized paths return wrong status
An oversized stored-request path makes is_interpretation_run_stored_request_path return false, so dispatch uses GET-by-id parsing. Clients receive 400 instead of 413.
Prompt for agents
Route syntactically recognizable stored-request resource paths to get_interpretation_run_stored_request even when identifier validation fails. The current is_ok predicate hides LimitExceeded and sends the request to GET-by-id parsing, which converts the response to InvalidWirePayload. Preserve rejection of unrelated extra segments such as /cancel, and add a service-level test asserting 413 for an over-limit /request path.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Folded into #454
Closed as
superseded_by_fold, not discarded. #454's head contains this PR as its direct ancestor and has been retargeted to this PR's former base, so the stored-request GET implementation/tests and this review history remain intact while queue WIP is reduced.Canonical landing vehicle: #454 (
feat(api): consolidate interpretation-run stored-request GET and CLI).Do not reopen unless the folded head demonstrably loses unique behavior or evidence.