feat(app): harden cancellable LAPP turns

2026-07-28 08:44:53 -04:00
parent 4831db1763
commit 23672e857b
26 changed files with 2822 additions and 167 deletions
@@ -11,6 +11,7 @@ use thiserror::Error;
 mod adjudication;
 mod context;
 mod lapp_provider;
+mod lifecycle;

 pub use adjudication::{
    AdjudicatingTurnPlanProvider, AdjudicationCatalog, AdjudicationError, AdjudicationModel,
@@ -29,9 +30,10 @@ pub use context::{
    encode_compiled_scene_context,
 };
 pub use lapp_provider::{
-    ChatExecutor, LappAdjudicationModel, LappTurnPlanProvider, OpenLappChatExecutor,
-    TURN_PLAN_TOOL_NAME,
+    ChatExecutor, LappAdjudicationModel, LappNativeCallGate, LappNativeCallPermit,
+    LappTurnPlanProvider, OpenLappChatExecutor, TURN_PLAN_TOOL_NAME,
 };
+pub use lifecycle::{TurnControl, TurnInterruption};

 pub const LAPP_BASELINE_COMMIT: &str = "5ba3c659e1536ec4bee16340faca603940a5cb17";
 pub const MAX_WORLD_BOOK_ENTRIES: usize = 8;
@@ -53,7 +55,16 @@ pub enum ProviderError {
    #[error("LAPP chat client could not be configured")]
    Configuration { code: Option<openlapp::ErrorCode> },
    #[error("LAPP chat request failed")]
-    Upstream { code: Option<openlapp::ErrorCode> },
+    Upstream {
+        code: Option<openlapp::ErrorCode>,
+        status: Option<u16>,
+    },
+    #[error("turn was cancelled")]
+    Cancelled,
+    #[error("turn timed out")]
+    TimedOut,
+    #[error("another LAPP native request is still active")]
+    NativeCallBusy,
    #[error("model returned an invalid turn plan")]
    InvalidModelOutput { kind: InvalidModelOutputKind },
    #[error("turn context could not be encoded")]
@@ -83,6 +94,28 @@ pub trait TurnPlanProvider {
        request: &TurnRequest,
        state: &RuntimeState,
    ) -> Result<TurnPlan, ProviderError>;
+
+    /// Produce a plan while observing a one-shot turn control.
+    ///
+    /// The default keeps existing providers source-compatible and discards
+    /// their result if cancellation arrives while they run. Providers capable
+    /// of interrupting in-flight work should override this method.
+    fn plan_turn_with_control(
+        &mut self,
+        request: &TurnRequest,
+        state: &RuntimeState,
+        control: &TurnControl,
+    ) -> Result<TurnPlan, ProviderError> {
+        if let Some(interruption) = control.interruption() {
+            return Err(provider_interruption(interruption));
+        }
+        let result = self.plan_turn(request, state);
+        if let Some(interruption) = control.interruption() {
+            Err(provider_interruption(interruption))
+        } else {
+            result
+        }
+    }
 }

 impl<Provider: TurnPlanProvider + ?Sized> TurnPlanProvider for &mut Provider {
@@ -93,6 +126,15 @@ impl<Provider: TurnPlanProvider + ?Sized> TurnPlanProvider for &mut Provider {
    ) -> Result<TurnPlan, ProviderError> {
        (**self).plan_turn(request, state)
    }
+
+    fn plan_turn_with_control(
+        &mut self,
+        request: &TurnRequest,
+        state: &RuntimeState,
+        control: &TurnControl,
+    ) -> Result<TurnPlan, ProviderError> {
+        (**self).plan_turn_with_control(request, state, control)
+    }
 }

 /// Projects only already-committed state into the player-safe read model.
@@ -132,6 +174,26 @@ where

    /// Validate, plan, reduce, commit, then project one player turn.
    pub fn submit_turn(&mut self, request: &TurnRequest) -> Result<TurnResult, TurnFailure> {
+        self.submit_turn_with_control(request, &TurnControl::new())
+    }
+
+    /// Validate, plan, reduce, atomically claim commit, append, then project.
+    ///
+    /// No store mutation occurs when cancellation wins before the commit
+    /// boundary. Once the boundary is claimed, [`TurnControl::cancel`]
+    /// returns `false` and this method reports the definitive append result.
+    pub fn submit_turn_with_control(
+        &mut self,
+        request: &TurnRequest,
+        control: &TurnControl,
+    ) -> Result<TurnResult, TurnFailure> {
+        let _attempt = control.begin_attempt().map_err(|error| match error {
+            lifecycle::BeginAttemptError::Cancelled => cancelled_turn(),
+            lifecycle::BeginAttemptError::TimedOut => timed_out_turn(),
+            lifecycle::BeginAttemptError::AlreadyUsed => {
+                internal_failure("turn control was already used")
+            }
+        })?;
        validate_turn_request(request)?;

        let current = self
@@ -144,7 +206,7 @@ where

        let plan = self
            .provider
-            .plan_turn(request, &current)
+            .plan_turn_with_control(request, &current, control)
            .map_err(|error| map_provider_error(&error))?;
        validate_turn_plan(request, &plan)?;

@@ -165,6 +227,13 @@ where
            state_hash,
        };

+        control.begin_commit().map_err(|error| match error {
+            lifecycle::BeginCommitError::Cancelled => cancelled_turn(),
+            lifecycle::BeginCommitError::TimedOut => timed_out_turn(),
+            lifecycle::BeginCommitError::InvalidState => {
+                internal_failure("turn control boundary is invalid")
+            }
+        })?;
        self.store
            .append_node(&node, &committed)
            .map_err(|error| map_store_error(&error))?;
@@ -313,15 +382,6 @@ pub fn load_default_lapp_profile() -> Result<openlapp::Profile, ProviderError> {
    openlapp::load_default_profile().map_err(|error| ProviderError::Profile { code: error.code() })
 }

-#[must_use]
-pub fn provider_failure(message: impl Into<String>) -> TurnFailure {
-    TurnFailure {
-        code: TurnFailureCode::ProviderUnavailable,
-        message: message.into(),
-        retryable: true,
-    }
-}
-
 fn validate_turn_result(request: &TurnRequest, result: &TurnResult) -> Result<(), TurnFailure> {
    if result.committed_node_id.trim().is_empty() {
        return Err(invalid_model_output("committed node id is empty"));
@@ -376,14 +436,94 @@ fn map_provider_error(error: &ProviderError) -> TurnFailure {
        ProviderError::InvalidModelOutput { .. } => {
            invalid_model_output("model returned an invalid turn plan")
        }
-        ProviderError::FixtureExhausted
-        | ProviderError::Profile { .. }
-        | ProviderError::Configuration { .. }
-        | ProviderError::Upstream { .. }
-        | ProviderError::ContextEncoding => provider_unavailable(),
+        ProviderError::FixtureExhausted | ProviderError::NativeCallBusy => provider_unavailable(),
+        ProviderError::Profile { .. } => provider_configuration(),
+        ProviderError::Configuration { code } => {
+            if code.is_some_and(is_credential_error) {
+                provider_credentials()
+            } else {
+                provider_configuration()
+            }
+        }
+        ProviderError::Upstream { code, status } => classify_upstream_failure(*code, *status),
+        ProviderError::ContextEncoding => internal_failure("turn context could not be prepared"),
+        ProviderError::Cancelled => cancelled_turn(),
+        ProviderError::TimedOut => timed_out_turn(),
    }
 }

+fn classify_upstream_failure(
+    code: Option<openlapp::ErrorCode>,
+    status: Option<u16>,
+) -> TurnFailure {
+    match status {
+        Some(401 | 403) => return provider_credentials(),
+        Some(408) => return timed_out_turn(),
+        Some(425 | 500..=599) => return provider_unavailable(),
+        Some(429) => return provider_rate_limited(),
+        Some(400..=499) => return provider_rejected(),
+        _ => {}
+    }
+
+    match code {
+        Some(code) if is_credential_error(code) => provider_credentials(),
+        Some(openlapp::ErrorCode::WaitTimeout) => timed_out_turn(),
+        Some(openlapp::ErrorCode::InvalidResponse) => {
+            invalid_model_output("model provider returned an invalid response")
+        }
+        Some(
+            openlapp::ErrorCode::InvalidGenerationInput | openlapp::ErrorCode::GenerationJobInvalid,
+        ) => provider_rejected(),
+        Some(code) if is_configuration_error(code) => provider_configuration(),
+        _ => provider_unavailable(),
+    }
+}
+
+const fn is_credential_error(code: openlapp::ErrorCode) -> bool {
+    matches!(
+        code,
+        openlapp::ErrorCode::InvalidSecretReference
+            | openlapp::ErrorCode::UnsupportedSecretScheme
+            | openlapp::ErrorCode::EnvSecretMissing
+            | openlapp::ErrorCode::VaultBackendUnavailable
+            | openlapp::ErrorCode::VaultCredentialNotFound
+            | openlapp::ErrorCode::VaultCredentialExists
+            | openlapp::ErrorCode::VaultRecordInvalid
+            | openlapp::ErrorCode::VaultBindingMismatch
+            | openlapp::ErrorCode::VaultAccessDenied
+            | openlapp::ErrorCode::VaultOperationFailed
+            | openlapp::ErrorCode::CredentialUpdatePartialFailure
+    )
+}
+
+const fn is_configuration_error(code: openlapp::ErrorCode) -> bool {
+    matches!(
+        code,
+        openlapp::ErrorCode::InvalidJson
+            | openlapp::ErrorCode::DuplicateJsonKey
+            | openlapp::ErrorCode::UnsafeJsonInteger
+            | openlapp::ErrorCode::InvalidProfile
+            | openlapp::ErrorCode::ProviderNotFound
+            | openlapp::ErrorCode::ProviderDisabled
+            | openlapp::ErrorCode::ModelNotFound
+            | openlapp::ErrorCode::ModelDisabled
+            | openlapp::ErrorCode::ModelAmbiguous
+            | openlapp::ErrorCode::DefaultNotFound
+            | openlapp::ErrorCode::OperationNotSupported
+            | openlapp::ErrorCode::ProtocolNotSupported
+            | openlapp::ErrorCode::OptionNotSupported
+            | openlapp::ErrorCode::StreamingNotSupported
+            | openlapp::ErrorCode::ProfilePathInvalid
+            | openlapp::ErrorCode::ProfileReadUnstable
+            | openlapp::ErrorCode::ProfileLocked
+            | openlapp::ErrorCode::ProfileLockInvalid
+            | openlapp::ErrorCode::ProfileConflict
+            | openlapp::ErrorCode::ProfileWriteFailed
+            | openlapp::ErrorCode::ProfileUpdatePartialFailure
+            | openlapp::ErrorCode::DiscoveryNotConfigured
+    )
+}
+
 fn map_store_error(error: &StoreError) -> TurnFailure {
    match error {
        StoreError::StaleBranchHead { .. } => stale_node(),
@@ -427,7 +567,66 @@ fn invalid_model_output(message: impl Into<String>) -> TurnFailure {
 }

 fn provider_unavailable() -> TurnFailure {
-    provider_failure("turn provider is unavailable")
+    TurnFailure {
+        code: TurnFailureCode::ProviderUnavailable,
+        message: "turn provider is unavailable".into(),
+        retryable: true,
+    }
+}
+
+fn provider_configuration() -> TurnFailure {
+    TurnFailure {
+        code: TurnFailureCode::ProviderConfiguration,
+        message: "turn provider configuration is unavailable".into(),
+        retryable: false,
+    }
+}
+
+fn provider_credentials() -> TurnFailure {
+    TurnFailure {
+        code: TurnFailureCode::ProviderCredentials,
+        message: "turn provider credentials are unavailable".into(),
+        retryable: false,
+    }
+}
+
+fn provider_rate_limited() -> TurnFailure {
+    TurnFailure {
+        code: TurnFailureCode::ProviderRateLimited,
+        message: "turn provider is rate limited".into(),
+        retryable: true,
+    }
+}
+
+fn provider_rejected() -> TurnFailure {
+    TurnFailure {
+        code: TurnFailureCode::ProviderRejected,
+        message: "turn provider rejected the request".into(),
+        retryable: false,
+    }
+}
+
+fn cancelled_turn() -> TurnFailure {
+    TurnFailure {
+        code: TurnFailureCode::Cancelled,
+        message: "turn was cancelled".into(),
+        retryable: true,
+    }
+}
+
+fn timed_out_turn() -> TurnFailure {
+    TurnFailure {
+        code: TurnFailureCode::TimedOut,
+        message: "turn timed out".into(),
+        retryable: true,
+    }
+}
+
+const fn provider_interruption(interruption: TurnInterruption) -> ProviderError {
+    match interruption {
+        TurnInterruption::Cancelled => ProviderError::Cancelled,
+        TurnInterruption::TimedOut => ProviderError::TimedOut,
+    }
 }

 fn stale_node() -> TurnFailure {
@@ -461,7 +660,7 @@ mod tests {

    use super::{
        FakeProvider, MAX_WORLD_BOOK_ENTRIES, ProviderError, TurnProvider, execute_turn,
-        select_world_book_entries, validate_turn_request,
+        map_provider_error, select_world_book_entries, validate_turn_request,
    };

    fn request(intent: TurnIntent, input: &str) -> TurnRequest {
@@ -663,12 +862,14 @@ mod tests {
            ) -> Result<TurnResult, ProviderError> {
                Err(ProviderError::Upstream {
                    code: Some(openlapp::ErrorCode::HttpStatus),
+                    status: None,
                })
            }
        }

        let upstream = ProviderError::Upstream {
            code: Some(openlapp::ErrorCode::HttpStatus),
+            status: None,
        };
        assert_eq!(upstream.to_string(), "LAPP chat request failed");

@@ -681,6 +882,62 @@ mod tests {
        assert!(failure.retryable);
    }

+    #[test]
+    fn provider_failures_use_safe_actionable_categories() {
+        let credentials = map_provider_error(&ProviderError::Upstream {
+            code: Some(openlapp::ErrorCode::HttpStatus),
+            status: Some(401),
+        });
+        assert_eq!(credentials.code, TurnFailureCode::ProviderCredentials);
+        assert!(!credentials.retryable);
+
+        let rate_limited = map_provider_error(&ProviderError::Upstream {
+            code: Some(openlapp::ErrorCode::HttpStatus),
+            status: Some(429),
+        });
+        assert_eq!(rate_limited.code, TurnFailureCode::ProviderRateLimited);
+        assert!(rate_limited.retryable);
+
+        let request_timeout = map_provider_error(&ProviderError::Upstream {
+            code: Some(openlapp::ErrorCode::HttpStatus),
+            status: Some(408),
+        });
+        assert_eq!(request_timeout.code, TurnFailureCode::TimedOut);
+        assert!(request_timeout.retryable);
+
+        let rejected = map_provider_error(&ProviderError::Upstream {
+            code: Some(openlapp::ErrorCode::HttpStatus),
+            status: Some(400),
+        });
+        assert_eq!(rejected.code, TurnFailureCode::ProviderRejected);
+        assert!(!rejected.retryable);
+
+        let invalid_response = map_provider_error(&ProviderError::Upstream {
+            code: Some(openlapp::ErrorCode::InvalidResponse),
+            status: None,
+        });
+        assert_eq!(invalid_response.code, TurnFailureCode::InvalidModelOutput);
+        assert!(invalid_response.retryable);
+
+        let configuration = map_provider_error(&ProviderError::Profile {
+            code: openlapp::ErrorCode::ModelNotFound,
+        });
+        assert_eq!(configuration.code, TurnFailureCode::ProviderConfiguration);
+        assert!(!configuration.retryable);
+
+        for failure in [
+            credentials,
+            rate_limited,
+            request_timeout,
+            rejected,
+            invalid_response,
+            configuration,
+        ] {
+            assert!(!failure.message.contains("secret"));
+            assert!(!failure.message.contains("api_key"));
+        }
+    }
+
    #[test]
    fn world_book_selection_applies_flags_keywords_and_tags() {
        let entries = vec![
@@ -754,6 +1011,7 @@ mod tests {
 #[cfg(test)]
 mod persistent_turn_tests {
    use std::collections::{BTreeMap, VecDeque};
+    use std::time::Duration;

    use nana_domain::{
        ActionSuggestion, BeatKind, PlayerView, PresentationBeat, PresentationCharacter,
@@ -764,7 +1022,8 @@ mod persistent_turn_tests {
    use nana_store::{InMemoryStoryStore, StoryStore};

    use super::{
-        ProviderError, TurnEngine, TurnPlan, TurnPlanProvider, TurnProjector, hash_runtime_state,
+        ProviderError, TurnControl, TurnEngine, TurnPlan, TurnPlanProvider, TurnProjector,
+        hash_runtime_state,
    };

    struct RecordingPlanProvider {
@@ -794,6 +1053,24 @@ mod persistent_turn_tests {
        }
    }

+    struct CancellingPlanProvider {
+        response: TurnPlan,
+        control: TurnControl,
+        calls: usize,
+    }
+
+    impl TurnPlanProvider for CancellingPlanProvider {
+        fn plan_turn(
+            &mut self,
+            _request: &TurnRequest,
+            _state: &RuntimeState,
+        ) -> Result<TurnPlan, ProviderError> {
+            self.calls += 1;
+            assert!(self.control.cancel());
+            Ok(self.response.clone())
+        }
+    }
+
    struct RecordingProjector<'store> {
        store: &'store InMemoryStoryStore,
        calls: usize,
@@ -983,6 +1260,56 @@ mod persistent_turn_tests {
        );
    }

+    #[test]
+    fn cancellation_during_planning_discards_the_plan_without_moving_the_branch() {
+        let store = seeded_store();
+        let control = TurnControl::new();
+        let provider = CancellingPlanProvider {
+            response: plan("node_2", StateDelta { ops: Vec::new() }),
+            control: control.clone(),
+            calls: 0,
+        };
+        let mut engine = TurnEngine::new(&store, provider, projector(&store));
+
+        let failure = engine
+            .submit_turn_with_control(&request("node_1"), &control)
+            .expect_err("cancelled turn");
+
+        assert_eq!(failure.code, TurnFailureCode::Cancelled);
+        assert!(failure.retryable);
+        assert_eq!(engine.provider().calls, 1);
+        assert_eq!(engine.projector().calls, 0);
+        assert_eq!(
+            store.branch_head("story_1", "branch_main").expect("head"),
+            Some("node_1".into())
+        );
+        assert!(store.load_node("story_1", "node_2").is_err());
+    }
+
+    #[test]
+    fn expired_deadline_does_not_call_provider_or_move_the_branch() {
+        let store = seeded_store();
+        let control = TurnControl::with_timeout(Duration::ZERO);
+        let mut engine = TurnEngine::new(
+            &store,
+            RecordingPlanProvider::new(Ok(plan("node_2", StateDelta { ops: Vec::new() }))),
+            projector(&store),
+        );
+
+        let failure = engine
+            .submit_turn_with_control(&request("node_1"), &control)
+            .expect_err("timed out turn");
+
+        assert_eq!(failure.code, TurnFailureCode::TimedOut);
+        assert!(failure.retryable);
+        assert_eq!(engine.provider().calls, 0);
+        assert_eq!(engine.projector().calls, 0);
+        assert_eq!(
+            store.branch_head("story_1", "branch_main").expect("head"),
+            Some("node_1".into())
+        );
+    }
+
    #[test]
    fn reducer_failure_is_redacted_and_does_not_save() {
        let store = seeded_store();
@@ -1026,6 +1353,7 @@ mod persistent_turn_tests {
            &store,
            RecordingPlanProvider::new(Err(ProviderError::Upstream {
                code: Some(openlapp::ErrorCode::HttpStatus),
+                status: None,
            })),
            projector(&store),
        );