-
Notifications
You must be signed in to change notification settings - Fork 10
fix(server): make feature-request reads idempotent via content-derived identity (ARN-240) #402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
rita-aga
wants to merge
3
commits into
main
Choose a base branch
from
claude/arn-240-feature-request-get
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
121573d
test(server): failing tests for feature-request read side effects (AR…
rita-aga 2d70ecc
fix(server): make feature-request reads idempotent via content-derive…
rita-aga 904e9af
docs(adr): record the tenant-scope and legacy_record_id consequences …
rita-aga File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
254 changes: 254 additions & 0 deletions
254
crates/temper-server/src/observe/evolution/operations/feature_requests_test.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,254 @@ | ||
| //! ARN-240: GET /observe/evolution/feature-requests must be idempotent. | ||
| //! | ||
| //! The handler generates feature requests from trajectory gaps on every read. | ||
| //! Each generated record minted a fresh UUID-suffixed id, so the store | ||
| //! "upsert" inserted a NEW row per GET, and a fresh `FR-{uuid}` system entity | ||
| //! was dispatched per generated record per GET — reads spawned unbounded | ||
| //! duplicates, and re-generation clobbered developer-owned fields. | ||
|
|
||
| use std::collections::BTreeMap; | ||
|
|
||
| use crate::registry::SpecRegistry; | ||
| use axum::extract::{Query, State}; | ||
| use axum::http::HeaderMap; | ||
| use temper_runtime::ActorSystem; | ||
|
|
||
| use crate::state::{ServerState, TrajectoryEntry, TrajectorySource}; | ||
| use crate::storage::StorageStack; | ||
|
|
||
| fn failing_platform_entry(n: u64) -> TrajectoryEntry { | ||
| TrajectoryEntry { | ||
| timestamp: format!("2026-07-13T00:00:{n:02}Z"), | ||
| tenant: "arn240".to_string(), | ||
| entity_type: "Invoice".to_string(), | ||
| entity_id: format!("inv-{n}"), | ||
| action: "GenerateInvoice".to_string(), | ||
| success: false, | ||
| from_status: None, | ||
| to_status: None, | ||
| error: Some("EntitySetNotFound: Invoice".to_string()), | ||
| agent_id: Some("agent-1".to_string()), | ||
| session_id: None, | ||
| authz_denied: None, | ||
| denied_resource: None, | ||
| denied_module: None, | ||
| source: Some(TrajectorySource::Platform), | ||
| spec_governed: Some(false), | ||
| agent_type: None, | ||
| intent: None, | ||
| request_body: None, | ||
| matched_policy_ids: None, | ||
| } | ||
| } | ||
|
|
||
| const FEATURE_REQUEST_IOA: &str = r#" | ||
| [automaton] | ||
| name = "FeatureRequest" | ||
| states = ["New", "Ready"] | ||
| initial = "New" | ||
|
|
||
| [[state]] | ||
| name = "category" | ||
| type = "string" | ||
| initial = "" | ||
|
|
||
| [[state]] | ||
| name = "description" | ||
| type = "string" | ||
| initial = "" | ||
|
|
||
| [[state]] | ||
| name = "frequency" | ||
| type = "string" | ||
| initial = "" | ||
|
|
||
| [[state]] | ||
| name = "developer_notes" | ||
| type = "string" | ||
| initial = "" | ||
|
|
||
| [[state]] | ||
| name = "legacy_record_id" | ||
| type = "string" | ||
| initial = "" | ||
|
|
||
| [[action]] | ||
| name = "CreateFeatureRequest" | ||
| kind = "input" | ||
| from = ["New"] | ||
| to = "Ready" | ||
| params = ["category", "description", "frequency", "developer_notes", "legacy_record_id"] | ||
| hint = "Record a platform gap surfaced by the insight generator." | ||
| "#; | ||
|
|
||
| const FEATURE_REQUEST_CSDL: &str = r#"<?xml version="1.0" encoding="utf-8"?> | ||
| <edmx:Edmx Version="4.0" xmlns:edmx="http://docs.oasis-open.org/odata/ns/edmx"> | ||
| <edmx:DataServices> | ||
| <Schema Namespace="Temper.System" xmlns="http://docs.oasis-open.org/odata/ns/edm"> | ||
| <EntityType Name="FeatureRequest"> | ||
| <Key><PropertyRef Name="Id"/></Key> | ||
| <Property Name="Id" Type="Edm.String" Nullable="false"/> | ||
| </EntityType> | ||
| <EntityContainer Name="Container"> | ||
| <EntitySet Name="FeatureRequests" EntityType="Temper.System.FeatureRequest"/> | ||
| </EntityContainer> | ||
| </Schema> | ||
| </edmx:DataServices> | ||
| </edmx:Edmx>"#; | ||
|
|
||
| fn registry_with_system_feature_request_spec() -> SpecRegistry { | ||
| let mut registry = SpecRegistry::new(); | ||
| let csdl = temper_spec::parse_csdl(FEATURE_REQUEST_CSDL).expect("csdl parses"); | ||
| registry.register_tenant( | ||
| "temper-system", | ||
| csdl, | ||
| FEATURE_REQUEST_CSDL.to_string(), | ||
| &[("FeatureRequest", FEATURE_REQUEST_IOA)], | ||
| ); | ||
| registry | ||
| } | ||
|
|
||
| async fn state_with_gap_trajectories() -> (ServerState, tempfile::TempDir) { | ||
| let dir = tempfile::tempdir().expect("tempdir"); | ||
| let db_url = format!("file:{}", dir.path().join("arn240.db").display()); | ||
| let turso = temper_store_turso::TursoEventStore::new(&db_url, None) | ||
| .await | ||
| .expect("turso store"); | ||
| let stack = StorageStack::from_turso(turso); | ||
|
|
||
| // Three failing Platform-source entries for the same (action, error | ||
| // pattern) — exactly the FEATURE_REQUEST_THRESHOLD gap group. | ||
| let sink = stack.trajectory.clone().expect("trajectory sink"); | ||
| for n in 0..3 { | ||
| sink.persist_trajectory_entry(&failing_platform_entry(n)) | ||
| .await | ||
| .expect("persist trajectory"); | ||
| } | ||
|
|
||
| let system = ActorSystem::new("arn240-test"); | ||
| let mut state = ServerState::from_registry(system, registry_with_system_feature_request_spec()); | ||
| state.set_storage_stack(stack); | ||
| (state, dir) | ||
| } | ||
|
|
||
| fn system_headers() -> HeaderMap { | ||
| let mut headers = HeaderMap::new(); | ||
| headers.insert("x-temper-principal-kind", "system".parse().expect("hdr")); | ||
| headers | ||
| } | ||
|
|
||
| async fn get_feature_requests(state: &ServerState) -> serde_json::Value { | ||
| let response = super::handle_feature_requests( | ||
| State(state.clone()), | ||
| system_headers(), | ||
| Query(BTreeMap::new()), | ||
| ) | ||
| .await | ||
| .expect("GET feature-requests"); | ||
| response.0 | ||
| } | ||
|
|
||
| /// A read must not create anything new on re-read: the same gap group must | ||
| /// map to the same feature request, however many times it is listed. | ||
| #[tokio::test] | ||
| async fn repeated_get_does_not_duplicate_feature_requests() { | ||
| let (state, _dir) = state_with_gap_trajectories().await; | ||
|
|
||
| let first = get_feature_requests(&state).await; | ||
| assert_eq!( | ||
| first["total"], 1, | ||
| "one gap group must yield one feature request, got: {first}" | ||
| ); | ||
|
|
||
| let second = get_feature_requests(&state).await; | ||
| assert_eq!( | ||
| second["total"], 1, | ||
| "a GET is a read — re-reading must not create a duplicate feature \ | ||
| request for the same gap group, got: {second}" | ||
| ); | ||
| assert_eq!( | ||
| second["feature_requests"][0]["id"], first["feature_requests"][0]["id"], | ||
| "the same gap group must keep the same identity across reads" | ||
| ); | ||
| } | ||
|
|
||
| /// Re-generation must not clobber developer-owned fields: a disposition set | ||
| /// via PATCH survives subsequent GETs while agents keep hitting the same gap. | ||
| #[tokio::test] | ||
| async fn get_preserves_developer_disposition_and_notes() { | ||
| let (state, _dir) = state_with_gap_trajectories().await; | ||
|
|
||
| let first = get_feature_requests(&state).await; | ||
| let id = first["feature_requests"][0]["id"] | ||
| .as_str() | ||
| .expect("feature request id") | ||
| .to_string(); | ||
|
|
||
| let store = state.platform_metadata_store().expect("platform store"); | ||
| store | ||
| .update_feature_request(&id, "WontFix", Some("duplicate of FR-1")) | ||
| .await | ||
| .expect("developer updates disposition"); | ||
|
|
||
| let after = get_feature_requests(&state).await; | ||
| assert_eq!(after["total"], 1, "still exactly one row, got: {after}"); | ||
| assert_eq!( | ||
| after["feature_requests"][0]["disposition"], "WontFix", | ||
| "a GET must not reset a developer's disposition, got: {after}" | ||
| ); | ||
| assert_eq!( | ||
| after["feature_requests"][0]["developer_notes"], "duplicate of FR-1", | ||
| "a GET must not wipe developer notes, got: {after}" | ||
| ); | ||
| } | ||
|
|
||
| /// The system entity behind a feature request is created ONCE — the entity | ||
| /// journal for the record's deterministic id holds exactly one creation | ||
| /// event however many times the listing runs. (Previously every GET | ||
| /// dispatched a fresh `FR-{uuid}` entity per generated record.) | ||
| #[tokio::test] | ||
| async fn repeated_get_creates_the_system_entity_exactly_once() { | ||
| let (state, _dir) = state_with_gap_trajectories().await; | ||
|
|
||
| let first = get_feature_requests(&state).await; | ||
| let id = first["feature_requests"][0]["id"] | ||
| .as_str() | ||
| .expect("feature request id") | ||
| .to_string(); | ||
|
|
||
| let journal = |from: u64| { | ||
| let events = state.storage_stack.as_ref().expect("stack").events.clone(); | ||
| let persistence_id = format!("temper-system:FeatureRequest:{id}"); | ||
| async move { | ||
| events | ||
| .read_events(&persistence_id, from) | ||
| .await | ||
| .expect("read entity journal") | ||
| } | ||
| }; | ||
|
|
||
| // The first GET must journal the entity UNDER THE RECORD'S ID (a fresh | ||
| // per-read id would leave this journal empty). A new entity journals a | ||
| // bootstrap Created event plus the action event, so assert non-empty | ||
| // rather than a count coupled to that implementation detail. | ||
| let after_first = journal(0).await; | ||
| assert!( | ||
| !after_first.is_empty(), | ||
| "the first GET must create the system entity under the record's id" | ||
| ); | ||
|
|
||
| let second = get_feature_requests(&state).await; | ||
| assert_eq!( | ||
| second["feature_requests"][0]["id"].as_str(), | ||
| Some(id.as_str()), | ||
| "identity must be stable before comparing journals" | ||
| ); | ||
|
|
||
| let after_second = journal(0).await; | ||
| assert_eq!( | ||
| after_second.len(), | ||
| after_first.len(), | ||
| "the second GET must not journal anything — the entity is created \ | ||
| exactly once, on the read that first discovered the gap" | ||
| ); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.