Skip to content

Commit e81a8b5

Browse files
Address review: narrow allowance override, add suggestion_id conversion tests
1. should_accept_via_suggestion_allowance now only overrides the RequestLimitReached disabled state. Previously a positive suggestion allowance overrode force_enabled for every disabled PromptAlertState (NoConnection, DelinquentDueToPaymentIssue, anonymous hard gate, overage/spend policy), letting those Free users click a chip while blocked for unrelated reasons. Added regression coverage for all of those states. 2. Added conversion tests asserting suggestion_id actually survives: PromptChip extraction through convert_input_to_user_input onto the wire (with and without an offer id), and proto history through convert_passive_suggestion_result_to_input back into a restored AIAgentInput (with and without an offer id).
1 parent f61f65b commit e81a8b5

4 files changed

Lines changed: 189 additions & 12 deletions

File tree

app/src/ai/agent/api/convert_conversation_tests.rs

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::ai::agent::api::convert_conversation::*;
88
use crate::ai::agent::conversation::{
99
AIAgentHarness, AIConversationId, ServerAIConversationMetadata,
1010
};
11-
use crate::ai::agent::{AIAgentInput, UserQueryMode};
11+
use crate::ai::agent::{AIAgentInput, PassiveSuggestionResultType, UserQueryMode};
1212
use crate::ai::ambient_agents::AmbientAgentTaskId;
1313
use crate::cloud_object::{Revision, ServerMetadata, ServerPermissions};
1414
use crate::persistence::model::ConversationUsageMetadata;
@@ -2242,3 +2242,79 @@ fn test_handoff_rehydration_system_query_is_hidden() {
22422242
"Agent output should still be rendered"
22432243
);
22442244
}
2245+
2246+
/// The `suggestion_id` is the entire anti-fraud capability behind the
2247+
/// lifetime prompt-suggestion allowance: it must survive a round trip
2248+
/// through persisted history back into a restored `AIAgentInput`.
2249+
#[test]
2250+
fn passive_suggestion_result_prompt_restores_offer_id_from_history() {
2251+
let passive_result = api::message::PassiveSuggestionResult {
2252+
result: Some(api::PassiveSuggestionResultType {
2253+
trigger: Some(
2254+
api::passive_suggestion_result_type::Trigger::AgentResponseCompleted(
2255+
api::passive_suggestion_result_type::AgentResponseCompleted {},
2256+
),
2257+
),
2258+
suggestion: Some(api::passive_suggestion_result_type::Suggestion::Prompt(
2259+
api::passive_suggestion_result_type::Prompt {
2260+
prompt: "do the thing".to_string(),
2261+
},
2262+
)),
2263+
suggestion_id: "offer-123".to_string(),
2264+
}),
2265+
context: None,
2266+
};
2267+
2268+
let input = convert_passive_suggestion_result_to_input(&passive_result)
2269+
.expect("should convert to an input");
2270+
2271+
match input {
2272+
AIAgentInput::PassiveSuggestionResult { suggestion, .. } => match suggestion {
2273+
PassiveSuggestionResultType::Prompt {
2274+
prompt,
2275+
suggestion_id,
2276+
} => {
2277+
assert_eq!(prompt, "do the thing");
2278+
assert_eq!(suggestion_id, Some("offer-123".to_string()));
2279+
}
2280+
other => panic!("Expected prompt suggestion, got {other:?}"),
2281+
},
2282+
other => panic!("Expected PassiveSuggestionResult input, got {other:?}"),
2283+
}
2284+
}
2285+
2286+
/// A chip with no live offer (legacy/static chip, or an old server) must not
2287+
/// invent an id: an empty wire `suggestion_id` restores to `None`, not
2288+
/// `Some("")`.
2289+
#[test]
2290+
fn passive_suggestion_result_prompt_with_absent_offer_id_restores_to_none() {
2291+
let passive_result = api::message::PassiveSuggestionResult {
2292+
result: Some(api::PassiveSuggestionResultType {
2293+
trigger: Some(
2294+
api::passive_suggestion_result_type::Trigger::AgentResponseCompleted(
2295+
api::passive_suggestion_result_type::AgentResponseCompleted {},
2296+
),
2297+
),
2298+
suggestion: Some(api::passive_suggestion_result_type::Suggestion::Prompt(
2299+
api::passive_suggestion_result_type::Prompt {
2300+
prompt: "static suggestion".to_string(),
2301+
},
2302+
)),
2303+
suggestion_id: String::new(),
2304+
}),
2305+
context: None,
2306+
};
2307+
2308+
let input = convert_passive_suggestion_result_to_input(&passive_result)
2309+
.expect("should convert to an input");
2310+
2311+
match input {
2312+
AIAgentInput::PassiveSuggestionResult { suggestion, .. } => match suggestion {
2313+
PassiveSuggestionResultType::Prompt { suggestion_id, .. } => {
2314+
assert_eq!(suggestion_id, None);
2315+
}
2316+
other => panic!("Expected prompt suggestion, got {other:?}"),
2317+
},
2318+
other => panic!("Expected PassiveSuggestionResult input, got {other:?}"),
2319+
}
2320+
}

app/src/ai/agent/api/convert_to_tests.rs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
use std::sync::Arc;
2+
13
use chrono::{DateTime, Utc};
24
use warp_core::command::ExitCode;
35
use warp_multi_agent_api as api;
46

57
use crate::ai::agent::task::TaskId;
68
use crate::ai::agent::{
7-
AIAgentActionResult, AIAgentActionResultType, AIAgentContext,
8-
TransferShellCommandControlToUserResult,
9+
AIAgentActionResult, AIAgentActionResultType, AIAgentContext, AIAgentInput,
10+
PassiveSuggestionResultType, TransferShellCommandControlToUserResult,
911
};
1012
use crate::terminal::model::block::BlockId;
1113

@@ -157,6 +159,57 @@ fn transfer_control_snapshot_result_converts_to_tool_call_result_input() {
157159
}
158160
}
159161

162+
#[test]
163+
fn passive_suggestion_result_prompt_carries_offer_id_onto_the_wire() {
164+
let input = AIAgentInput::PassiveSuggestionResult {
165+
trigger: None,
166+
suggestion: PassiveSuggestionResultType::Prompt {
167+
prompt: "do the thing".to_string(),
168+
suggestion_id: Some("offer-123".to_string()),
169+
},
170+
context: Arc::new([]),
171+
};
172+
173+
let converted = super::convert_input_to_user_input(input).expect("should convert");
174+
match converted {
175+
api::request::input::user_inputs::user_input::Input::PassiveSuggestionResult(input) => {
176+
let result = input.result.expect("result should be set");
177+
assert_eq!(result.suggestion_id, "offer-123");
178+
match result.suggestion {
179+
Some(api::passive_suggestion_result_type::Suggestion::Prompt(prompt)) => {
180+
assert_eq!(prompt.prompt, "do the thing");
181+
}
182+
other => panic!("Expected prompt suggestion, got {other:?}"),
183+
}
184+
}
185+
other => panic!("Expected passive-suggestion-result input, got {other:?}"),
186+
}
187+
}
188+
189+
#[test]
190+
fn passive_suggestion_result_prompt_without_offer_id_sends_empty_suggestion_id() {
191+
let input = AIAgentInput::PassiveSuggestionResult {
192+
trigger: None,
193+
suggestion: PassiveSuggestionResultType::Prompt {
194+
prompt: "do the thing".to_string(),
195+
suggestion_id: None,
196+
},
197+
context: Arc::new([]),
198+
};
199+
200+
let converted = super::convert_input_to_user_input(input).expect("should convert");
201+
match converted {
202+
api::request::input::user_inputs::user_input::Input::PassiveSuggestionResult(input) => {
203+
let result = input.result.expect("result should be set");
204+
assert_eq!(
205+
result.suggestion_id, "",
206+
"a chip with no live offer must not invent an id"
207+
);
208+
}
209+
other => panic!("Expected passive-suggestion-result input, got {other:?}"),
210+
}
211+
}
212+
160213
#[test]
161214
fn transfer_control_finished_result_converts_to_tool_call_result_input() {
162215
let block_id = BlockId::default();

app/src/terminal/view/inline_banner/prompt_suggestions.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -346,11 +346,15 @@ fn should_open_unavailable_modal(state: &PromptAlertState, app: &AppContext) ->
346346
}
347347

348348
/// Whether a click should accept the chip by drawing on the lifetime
349-
/// prompt-suggestion allowance instead of the interactive wallet. This is the
350-
/// gate PRODUCT.md requires: remaining > 0 must never open the interactive
351-
/// unavailable modal and must dispatch `ResolvePromptSuggestion` instead.
352-
fn should_accept_via_suggestion_allowance(app: &AppContext) -> bool {
353-
AIRequestUsageModel::as_ref(app).can_accept_prompt_suggestion()
349+
/// prompt-suggestion allowance instead of the interactive wallet. Only ever
350+
/// overrides the `RequestLimitReached` disabled state -- every other
351+
/// disabled state (offline, delinquent, anonymous hard gate, overage/spend
352+
/// policy) keeps its own disabled treatment and tooltip regardless of
353+
/// suggestion credits, per PRODUCT.md's "remaining > 0 and the other accept
354+
/// rules passing" gate.
355+
fn should_accept_via_suggestion_allowance(state: &PromptAlertState, app: &AppContext) -> bool {
356+
matches!(state, PromptAlertState::RequestLimitReached)
357+
&& AIRequestUsageModel::as_ref(app).can_accept_prompt_suggestion()
354358
}
355359

356360
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -427,7 +431,8 @@ impl View for PromptSuggestionsView {
427431

428432
let prompt_alert_state = self.prompt_alert.as_ref(app).state();
429433
let open_unavailable_modal = should_open_unavailable_modal(prompt_alert_state, app);
430-
let can_accept_via_suggestion_allowance = should_accept_via_suggestion_allowance(app);
434+
let can_accept_via_suggestion_allowance =
435+
should_accept_via_suggestion_allowance(prompt_alert_state, app);
431436
// Only relevant when the disablement reason is the shared
432437
// "out of requests" state; other disabled states (offline, payment
433438
// issues) keep their own, higher-priority tooltip.

app/src/terminal/view/inline_banner/prompt_suggestions_tests.rs

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,50 @@ fn should_open_unavailable_modal_is_false_when_suggestion_remaining_is_positive(
7373
"remaining > 0 must never open the interactive unavailable modal"
7474
);
7575
assert!(
76-
should_accept_via_suggestion_allowance(ctx),
76+
should_accept_via_suggestion_allowance(&PromptAlertState::RequestLimitReached, ctx),
7777
"remaining > 0 must allow the accept to go through"
7878
);
7979
});
8080
})
8181
}
8282

83+
/// PRODUCT.md gates the click on remaining > 0 *and* the other accept rules
84+
/// passing: a positive suggestion allowance must not override any disabled
85+
/// state besides `RequestLimitReached`. Free users who are offline or
86+
/// delinquent must stay blocked (with their own tooltip) even with credits.
87+
#[test]
88+
fn should_accept_via_suggestion_allowance_is_false_for_other_disabled_states_even_with_remaining() {
89+
App::test((), |mut app| async move {
90+
let _flag = FeatureFlag::OpenWarpNewSettingsModes.override_enabled(true);
91+
initialize_app_with_allowance(
92+
&mut app,
93+
Some(PromptSuggestionAllowance {
94+
limit: 300,
95+
used: 100,
96+
}),
97+
);
98+
99+
app.read(|ctx| {
100+
for state in [
101+
PromptAlertState::NoConnection,
102+
PromptAlertState::DelinquentDueToPaymentIssue,
103+
PromptAlertState::AnonymousUserRequestLimitHardGate,
104+
PromptAlertState::OveragesToggleableButNotEnabled,
105+
PromptAlertState::MonthlyOveragesSpendLimitReached,
106+
] {
107+
assert!(
108+
!should_accept_via_suggestion_allowance(&state, ctx),
109+
"a positive suggestion allowance must not override the {state:?} disabled state"
110+
);
111+
assert!(
112+
!should_open_unavailable_modal(&state, ctx),
113+
"the interactive unavailable modal is specific to RequestLimitReached, not {state:?}"
114+
);
115+
}
116+
});
117+
})
118+
}
119+
83120
#[test]
84121
fn should_open_unavailable_modal_is_false_when_suggestion_allowance_is_exhausted() {
85122
App::test((), |mut app| async move {
@@ -99,7 +136,10 @@ fn should_open_unavailable_modal_is_false_when_suggestion_allowance_is_exhausted
99136
&PromptAlertState::RequestLimitReached,
100137
ctx
101138
));
102-
assert!(!should_accept_via_suggestion_allowance(ctx));
139+
assert!(!should_accept_via_suggestion_allowance(
140+
&PromptAlertState::RequestLimitReached,
141+
ctx
142+
));
103143
});
104144
})
105145
}
@@ -118,7 +158,10 @@ fn should_open_unavailable_modal_is_true_when_no_suggestion_allowance_exists() {
118158
&PromptAlertState::RequestLimitReached,
119159
ctx
120160
));
121-
assert!(!should_accept_via_suggestion_allowance(ctx));
161+
assert!(!should_accept_via_suggestion_allowance(
162+
&PromptAlertState::RequestLimitReached,
163+
ctx
164+
));
122165
});
123166
})
124167
}

0 commit comments

Comments
 (0)