feat(api): cancel interpretation runs via loopback POST - #440
Conversation
GAP-003A unique slice stacked on GET-by-id HTTP: loopback
POST /v1/interpretation-runs/{idempotency_key}/cancel drops one
accepted hypothetical identity from tepp-orchestrator-loopback.
Naruon and LineageWeave refused. ADR 0073.
|
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.
🔍 Required verification evidence is incomplete
Repository rules require complete tests and 100% production line and branch coverage. The submitted plan lists focused tests but no complete-suite or coverage result.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if encoded.is_empty() || encoded.contains('/') { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| let idempotency_key = decode_path_segment(encoded)?; | ||
| require_nonempty(&idempotency_key)?; | ||
| if idempotency_key.contains('/') || idempotency_key.contains('\0') { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| if idempotency_key.len() > INTERPRETATION_RUN_CANCEL_ID_MAX_LEN { | ||
| return Err(OrchestratorLiveError::LimitExceeded); |
There was a problem hiding this comment.
🟡 Accepted runs become uncancellable
Creation accepts slash-containing or over-128-byte keys, but interpretation_run_cancel_path_id rejects them. Those accepted runs cannot use the new cancellation endpoint.
Prompt for agents
Align interpretation-run idempotency-key validation across creation and cancellation. InterpretationRunRequest::validate currently accepts slash-containing and arbitrarily long keys, while interpretation_run_cancel_path_id and the cancel exchange reject slashes and keys over 128 bytes. Either constrain creation to the shared path-safe limit or define an encoding and length contract that lets every previously accepted key be cancelled. Add service-level tests that create and then cancel boundary and encoded keys.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if interpretation_run_cancel_path_id(path).is_ok() { | ||
| return self.cancel_interpretation_run(path, &headers, body); | ||
| } |
There was a problem hiding this comment.
🟡 Oversized cancellations report wrong status
is_ok() discards the cancel parser's limit error. Oversized cancellation keys return 400 instead of the contract's 413 response.
| if interpretation_run_cancel_path_id(path).is_ok() { | |
| return self.cancel_interpretation_run(path, &headers, body); | |
| } | |
| if path.starts_with(&format!("{INTERPRETATION_RUN_PATH}/")) && path.ends_with("/cancel") { | |
| return self.cancel_interpretation_run(path, &headers, body); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| let idempotency_key = interpretation_run_cancel_path_id(path)?; | ||
| if !body.is_empty() { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| refuse_metrics_on_interpretation_run_cancel_payload(body)?; | ||
| refuse_retrieval_get_headers(headers)?; | ||
| let (_, accepted) = self | ||
| .accepted_runs | ||
| .remove(&idempotency_key) | ||
| .ok_or(OrchestratorLiveError::InvalidWirePayload)?; |
There was a problem hiding this comment.
📝 Info: Validation precedes cancellation mutation
Body, header, consumer, credential, and path checks finish before accepted_runs.remove. Rejected requests leave the accepted run unchanged.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn refuse_metrics_on_interpretation_run_cancel_payload( | ||
| payload: &str, | ||
| ) -> Result<(), OrchestratorLiveError> { | ||
| if payload.trim().is_empty() { | ||
| return Ok(()); | ||
| } | ||
| let value: serde_json::Value = | ||
| serde_json::from_str(payload).map_err(|_| OrchestratorLiveError::InvalidWirePayload)?; | ||
| let Some(object) = value.as_object() else { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| }; | ||
| if object | ||
| .get("schema_version") | ||
| .and_then(serde_json::Value::as_str) | ||
| == Some("tepp.scientific_acceptance.v1") | ||
| { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| if FORBIDDEN_CANCEL_KEYS | ||
| .iter() | ||
| .any(|key| object.contains_key(*key)) | ||
| { | ||
| return Err(OrchestratorLiveError::InvalidWirePayload); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🔍 Public metric filter has narrow scope
refuse_metrics_on_interpretation_run_cancel_payload checks top-level keys only and accepts arbitrary objects. The live endpoint rejects them earlier, but external callers can misread this broader contract.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Closing as |
1 participant
Summary
GAP-003A unique slice stacked on #438: loopback
POST /v1/interpretation-runs/{idempotency_key}/cancelremoves one accepted hypothetical interpretation-run identity fromOrchestratorLiveService/tepp-orchestrator-loopback.contextual-orchestratoronly. Empty body. Identity travels in the path.idempotency-keyand collection pagination headers are refused.claim_status=hypothetical,scientific_authority=false,cancelled=true.tepp.scientific_acceptance.v1, RMSE, bias, coverage, SE-gate,evidence_span_ids,findings, andcausal_scorenever appear.NaruonLiveServicestays POST-only for analysis-run and export.Test plan
cargo test -p orchestrator_livecargo clippy -p orchestrator_live --all-targets -- -D warningscargo doc -p orchestrator_live --no-depspython3 scripts/validate_documentation.pypython3 scripts/check_docstrings.py