Skip to content

feat(api): consolidate interpretation-run stored-request and run-id lookup - #467

Closed
seonghobae wants to merge 3 commits into
feat/interpretation-run-retrieval-get-gap-003afrom
feat/interpretation-run-lookup-get-gap-003a
Closed

feat(api): consolidate interpretation-run stored-request and run-id lookup#467
seonghobae wants to merge 3 commits into
feat/interpretation-run-retrieval-get-gap-003afrom
feat/interpretation-run-lookup-get-gap-003a

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Superseded by fold into #468

Strict ancestry was verified before closure: #467 exact head 44b9be577ec4a3269a5320d7f1b0e0f5368aa9ff is the merge base of #468 and #468 exact head 287584d4d99d1cf44f26dbeb4d8f010fc167837a was exactly one commit ahead with zero commits behind. #468 was retargeted to this PR's base, so the complete #453#454 stored-request GET/CLI work and this run-id lookup GET remain in the surviving Analysis Run application/adapter vehicle together with the new lookup CLI. This PR is closed only to reduce duplicate WIP; its discussion and review history remain immutable evidence.

This lineage does not create an independent bounded context or per-route architecture authority. Current landing work continues on #468 under #435 queue-authority policy and #437 ADR-identity normalization.

…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.
…d CLI

Publish tepp-interpretation-run-request get so operators retrieve the stored
create request on spawned tepp-orchestrator-loopback TCP without POST replay.
scientific_authority stays false. Empty stdin is admitted. Naruon and
LineageWeave are refused. NaruonLiveService stays POST-only. Cancel extra-
segment stays refused. ADR 0086.
GET /v1/interpretation-runs/by-run-id/{interpretation_run_id} returns the
metric-free hypothetical identity on OrchestratorLiveService. Dual identity
of GET-by-id (idempotency_key). Zero and ambiguous matches fail closed.
claim_status remains hypothetical; scientific_authority remains false.
NaruonLiveService stays POST-only. Does not re-open cancel lineages.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Devin Review

Comment on lines +200 to +202
if is_interpretation_run_lookup_path(path) {
return self.lookup_interpretation_run_by_run_id(path, &headers, body);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Oversized lookups return wrong status

is_interpretation_run_lookup_path classifies an oversized lookup as another route before its parser can return LimitExceeded. The listener returns 400 instead of 413.

Prompt for agents
Route lookup-shaped paths independently of whether their IDs pass validation. In crates/orchestrator_live/src/service.rs, dispatch any path with the exact by-run-id resource prefix to lookup_interpretation_run_by_run_id, then let interpretation_run_lookup_path_id preserve LimitExceeded for oversized IDs. Keep near-prefix paths and unrelated GET-by-id paths on their existing routes, and add a live-service test asserting 413 for an oversized lookup ID.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +62 to +64
if idempotency_key == "by-run-id" {
return Err(OrchestratorLiveError::InvalidWirePayload);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Reserved key loses identity retrieval

POST accepts by-run-id, but interpretation_run_retrieval_path_id rejects that run's retrieval URL. The accepted run cannot be fetched by its client key.

Prompt for agents
Make reservation of the by-run-id key consistent across the interpretation-run API. Either preserve GET-by-id access for an already valid opaque idempotency key through unambiguous routing, or reject the reserved key during POST request validation and in contextual_orchestrator_interpretation_run_retrieval_exchange. Add coverage proving that every key accepted by POST remains retrievable, including the reserved-key boundary and percent-encoded form.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +56 to +89
pub fn interpretation_run_lookup_path_id(path: &str) -> Result<String, OrchestratorLiveError> {
let remainder = path
.strip_prefix(INTERPRETATION_RUN_PATH)
.ok_or(OrchestratorLiveError::InvalidWirePayload)?;
let encoded = remainder
.strip_prefix('/')
.ok_or(OrchestratorLiveError::InvalidWirePayload)?;
let encoded = encoded
.strip_prefix(INTERPRETATION_RUN_LOOKUP_PREFIX)
.ok_or(OrchestratorLiveError::InvalidWirePayload)?;
let encoded = encoded
.strip_prefix('/')
.ok_or(OrchestratorLiveError::InvalidWirePayload)?;
if encoded.is_empty() || encoded.contains('/') {
return Err(OrchestratorLiveError::InvalidWirePayload);
}
let interpretation_run_id = decode_path_segment(encoded)?;
require_nonempty(&interpretation_run_id)?;
if interpretation_run_id == INTERPRETATION_RUN_LOOKUP_PREFIX {
return Err(OrchestratorLiveError::InvalidWirePayload);
}
if interpretation_run_id.contains('/') || interpretation_run_id.contains('\0') {
return Err(OrchestratorLiveError::InvalidWirePayload);
}
if interpretation_run_id.len() > INTERPRETATION_RUN_LOOKUP_ID_MAX_LEN {
return Err(OrchestratorLiveError::LimitExceeded);
}
Ok(interpretation_run_id)
}

/// Whether `path` is the lookup-by-run-id resource.
#[must_use]
pub fn is_interpretation_run_lookup_path(path: &str) -> bool {
interpretation_run_lookup_path_id(path).is_ok()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Coverage evidence remains incomplete

The repository requires 100% production line and branch coverage. The test plan lists focused tests and Clippy, but no fresh coverage gate.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae
seonghobae marked this pull request as draft September 1, 2026 18:07
@seonghobae seonghobae changed the title feat(api): resolve interpretation-run identity by server-assigned run id feat(api): consolidate interpretation-run stored-request and run-id lookup Sep 1, 2026
@seonghobae
seonghobae changed the base branch from feat/interpretation-run-stored-request-cli-gap-003a to feat/interpretation-run-retrieval-get-gap-003a September 1, 2026 18:10
@seonghobae seonghobae closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant