feat(api): consolidate temporal-context retrieval GET and CLI - #452
feat(api): consolidate temporal-context retrieval GET and CLI#452seonghobae wants to merge 2 commits into
Conversation
Publish GET /v1/temporal-context/{idempotency_key} so operators fetch one
metric-free LineageWeave identity without POST replay. Event labels and
actor lists never appear. Collection GET stays closed. Naruon is refused.
NaruonLiveService stays POST-only. ADR 0083.
… CLI
Publish tepp-temporal-context-get get so operators mint LineageWeave
GET /v1/temporal-context/{idempotency_key} onto spawned tepp-loopback TCP.
Receipts stay metric-free temporal_association_only. Event labels and
actor lists never appear. Naruon is refused. NaruonLiveService stays
POST-only. ADR 0084.
|
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}
| refuse_scientific_acceptance(&response.body)?; | ||
| refuse_metrics_on_temporal_context_retrieval_payload(&response.body)?; | ||
| refuse_event_pii(&response.body)?; |
There was a problem hiding this comment.
🟡 Opaque keys become unretrievable
When an opaque key contains a forbidden field name, refuse_metrics_on_temporal_context_retrieval_payload rejects the valid response. The CLI cannot retrieve that accepted identity.
Prompt for agents
The CLI filters the serialized retrieval response as raw text before parsing it. Valid opaque idempotency-key values containing event_label, actor_references, timeline_events, or tepp.scientific_acceptance.v1 are therefore rejected, although validate_temporal_context_registry_identity and the POST registry accept them. Update the response validation in crates/tepp_api/src/temporal_context_retrieval_cli.rs and the shared filtering helper in crates/tepp_api/src/temporal_context_retrieval_http.rs to inspect JSON field names and structure rather than substrings inside string values. Preserve rejection of actual forbidden fields and scientific-acceptance payloads, and add round-trip tests using accepted keys containing each reserved term.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
changed the title
There was a problem hiding this comment.
🔍 Coverage gate needs current-head evidence
The new parsers add many untested error branches. Repository rules require 100% production line and branch coverage before merge.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } else { | ||
| self.accepted_temporal_contexts.insert(replay_key, item); | ||
| } |
There was a problem hiding this comment.
🟡 Failed submissions remain retrievable
A large valid POST can fail during response serialization after insert records its identity. A later GET then reports the failed submission as accepted.
Prompt for agents
In crates/tepp_api/src/analysis_run_live.rs, accept_temporal_context mutates accepted_temporal_contexts before build_temporal_context and response serialization have completed. A request below the 64 KiB request limit can produce a response above the 64 KiB TemporalContextResponse limit because response fields duplicate request identifiers. The POST then returns an error while GET still finds the recorded identity. Build and serialize the successful response first, then commit the registry mutation only after every fallible acceptance step succeeds. Add a regression test with a valid under-limit request whose expanded response exceeds its output limit, asserting that the failed POST does not create a retrievable key.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if let Some(stored) = self.accepted_temporal_contexts.get(&replay_key) { | ||
| if stored.knowledge_cutoff != item.knowledge_cutoff { | ||
| return Err(ApiError::InvalidWirePayload); | ||
| } |
There was a problem hiding this comment.
🟡 Changed retries reuse accepted keys
Reusing a key with the same cutoff but different events passes the knowledge_cutoff check. The conflicting retry is recomputed instead of rejected.
Prompt for agents
In crates/tepp_api/src/analysis_run_live.rs, accepted_temporal_contexts stores only TemporalContextRetrieved, so accept_temporal_context can compare only knowledge_cutoff on replay. Preserve the original TemporalContextRequest and successful response, as the analysis-run and project-history registries do. Return the original result for an identical retry and reject any request change under the same consumer/idempotency key. Add tests for identical replay and for changed events, subject_post_id, and other request fields with an unchanged cutoff.
Was this helpful? React with 👍 or 👎 to provide feedback.
| TEMPORAL_CONTEXT_RETRIEVAL_CONTRACT_VERSION, | ||
| )?; | ||
| validate_temporal_context_registry_identity(&self.idempotency_key)?; | ||
| require_nonempty(&self.knowledge_cutoff)?; |
There was a problem hiding this comment.
🟡 Malformed cutoffs pass retrieval validation
TemporalContextRetrieved accepts any nonempty knowledge_cutoff. Untrusted retrieval responses can therefore publish a malformed timestamp as the applied cutoff.
Prompt for agents
In crates/tepp_api/src/temporal_context_retrieval_http.rs, TemporalContextRetrieved::validate checks knowledge_cutoff only with require_nonempty. Validate it with the canonical KnowledgeCutoff RFC 3339 parser used by TemporalContextRequest so constructors, from_json, and CLI rendering reject malformed cutoff semantics. Add tests for malformed and canonical timestamps.
Was this helpful? React with 👍 or 👎 to provide feedback.
| next_request_serial: u64, | ||
| accepted_runs: HashMap<String, (AnalysisRunRequest, AnalysisRunAccepted)>, | ||
| accepted_project_histories: HashMap<String, (ProjectHistoryRequest, ProjectHistoryProjection)>, | ||
| accepted_temporal_contexts: HashMap<String, TemporalContextRetrieved>, |
There was a problem hiding this comment.
📝 Info: Registry lifetime matches declared scope
The registry disappears when tepp-loopback restarts. ADR 0083 explicitly leaves persistence to GAP-003B, so this is not a defect here.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if method == "GET" { | ||
| return self.get_temporal_context(path, &headers, body); |
There was a problem hiding this comment.
📝 Info: GET dispatch remains route-restricted
All GETs enter one handler, but temporal_context_retrieval_path_id admits only item paths. Collection, unrelated, query, and extra-segment routes remain closed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let exchange = lineageweave_temporal_context_retrieval_exchange( | ||
| &invocation.origin, | ||
| &invocation.idempotency_key, | ||
| )?; | ||
| loopback_http1_from_temporal_context_retrieval_exchange(&exchange, &invocation.host) |
There was a problem hiding this comment.
🔍 Origin flag has no runtime effect
--origin only helps construct a path later sent to --host. Any valid origin produces the same request, making the required flag misleading.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
exact-head COMMENT only (not APPROVE) on cb97aad9f87283df4d94abe8c6df61a0a476c893. Devin COMMENTED is not independent APPROVE.
Unique remains tepp-temporal-context-get + ADR 0084 (GET /v1/temporal-context/{idempotency_key} identity retrieval, not stored-request). validate_temporal_context_registry_identity plus metric-free FORBIDDEN_RETRIEVAL_KEYS; LineageWeave retrieval exchange; NaruonLiveService stays POST-only. Do not duplicate this retrieval CLI. Stored-request CLI is #464 (ADR 0092); do not fold unscoped by-client-key disclosure into this slice. Persistence remains GAP-003B (#287 stay out).
Still draft vs main. Do not un-draft. Zero exact-head APPROVEs. Do not merge without two independent current-head APPROVEs under ruleset 18156473. MERGEABLE/BLOCKED is not two APPROVEs.
1 participant
Consolidated landing vehicle
This PR folds predecessor #451 into one LineageWeave-facing Analysis Run / temporal-context retrieval application-adapter vehicle. The current head contains #451 as its direct ancestor, so retargeting to protected
mainpreserves the GET implementation/tests while eliminating one open micro-PR. #451 remains as immutable review/history evidence; sibling stored-request vehicle #464 also retains this ancestry.Preserved GET behavior from #451:
GET /v1/temporal-context/{idempotency_key}, hostile identity/header refusal, LineageWeave-only consumer boundary, metric-freeinference_status=temporal_association_onlyidentity, collection-path refusal, backward-compatible POST behavior, and focused retrieval tests.CLI behavior on this head: published
tepp-temporal-context-get get, empty-stdin admission, public-bind/localhost/http-origin/unpublished-consumer/credential/identity refusals, typed exchange and metric-free rendering.This is one Analysis Run application/adapter landing vehicle, not a bounded context. ADR 0083/0084 are implementation lineage pending #437 normalization. Merge only after fresh exact-head required workflows, resolved conversations, and qualifying independent approval under live ruleset 18156473. No predecessor-head evidence transfer or bypass.