From f474cb5bb91c0b0a136decb732976fcdd1aec3b6 Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Thu, 22 Oct 2026 17:01:09 -0700 Subject: [PATCH 1/5] feat(rules): warn before saving an agent rule that RuleBot cannot do --- src/CONST/index.ts | 3 + src/languages/de.ts | 3 + src/languages/el.ts | 3 + src/languages/en.ts | 3 + src/languages/es.ts | 3 + src/languages/fr.ts | 3 + src/languages/it.ts | 3 + src/languages/ja.ts | 3 + src/languages/nl.ts | 3 + src/languages/pl.ts | 3 + src/languages/pt-BR.ts | 3 + src/languages/zh-hans.ts | 3 + src/libs/actions/Policy/Rules.ts | 43 +- .../rules/AgentRules/AddAgentRulePage.tsx | 13 +- .../rules/AgentRules/AddAgentRuleWriteTab.tsx | 38 +- .../rules/AgentRules/EditAgentRulePage.tsx | 34 +- .../AgentRules/useAgentRulePromptCheck.ts | 125 ++++++ .../workspace/rules/DescribeRuleForm.tsx | 38 +- .../workspace/rules/ImpossibleRuleNotice.tsx | 63 +++ tests/ui/AddAgentRulePageTest.tsx | 46 +- tests/ui/AgentRulePromptCheckTest.tsx | 407 ++++++++++++++++++ 21 files changed, 799 insertions(+), 44 deletions(-) create mode 100644 src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts create mode 100644 src/pages/workspace/rules/ImpossibleRuleNotice.tsx create mode 100644 tests/ui/AgentRulePromptCheckTest.tsx diff --git a/src/CONST/index.ts b/src/CONST/index.ts index 7a54e5e399d7..a561fded1b60 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -619,6 +619,9 @@ const CONST = { GENERATED_RULE: { PROMPT_MAX_LENGTH: 6000, + + // The slowest answers in the eval take about 13 seconds. After this time, the agent rule saves without an answer. + AGENT_RULE_CHECK_TIMEOUT_MS: 20000, STATE: { RULE: 'rule', UNSUPPORTED: 'unsupported', diff --git a/src/languages/de.ts b/src/languages/de.ts index b46fc8738572..c68d5bff34f6 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -8821,6 +8821,9 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc describeRuleForConcierge: 'Beschreiben Sie Ihre Regel und Concierge erstellt sie', gotIt: 'Verstanden', createRule: 'Regel erstellen', + createAnyway: 'Trotzdem erstellen', + saveAnyway: 'Trotzdem speichern', + checkingRule: 'Ihre Regel wird geprüft...', fallbackBanner: 'Dies ist keine Standardregel, daher haben wir sie als Agentenregel erstellt. Agentenregeln greifen, nachdem sich eine Ausgabe oder ein Bericht geändert hat, zum Beispiel indem sie die Ausgabe zurückhalten oder den Bericht ablehnen.', fallbackBannerWithExplanation: (explanation: string) => `Dies ist keine Standardregel, daher haben wir sie als Agentenregel formuliert. ${explanation}`, diff --git a/src/languages/el.ts b/src/languages/el.ts index 1eb71ef7ca54..a4d4e93e48a5 100644 --- a/src/languages/el.ts +++ b/src/languages/el.ts @@ -9290,6 +9290,9 @@ ${reportName}`, describeRuleHeadline: 'Περιγράψτε τον κανόνα σας', describeRuleForConcierge: 'Περιγράψτε τον κανόνα σας και το Concierge θα τον δημιουργήσει', createRule: 'Δημιουργία κανόνα', + createAnyway: 'Δημιουργία ούτως ή άλλως', + saveAnyway: 'Αποθήκευση ούτως ή άλλως', + checkingRule: 'Γίνεται έλεγχος του κανόνα σας...', fallbackBanner: 'Αυτός δεν είναι ένας τυπικός κανόνας, οπότε τον διαμορφώσαμε ως κανόνα πράκτορα. Οι κανόνες πράκτορα εφαρμόζονται αφού αλλάξει μια δαπάνη ή μια αναφορά, για παράδειγμα κρατώντας τη δαπάνη ή απορρίπτοντας την αναφορά.', fallbackBannerWithExplanation: (explanation: string) => `Αυτός δεν είναι ένας τυπικός κανόνας, οπότε τον συντάξαμε ως κανόνα εκπροσώπου. ${explanation}`, diff --git a/src/languages/en.ts b/src/languages/en.ts index ae40aaf45841..3ae4428ea0ec 100644 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -9582,6 +9582,9 @@ const translations = { describeRuleForConcierge: 'Describe your rule and Concierge will build it', disclaimer: 'AI agents can make mistakes.', createRule: 'Create rule', + createAnyway: 'Create anyway', + saveAnyway: 'Save anyway', + checkingRule: 'Checking your rule...', fallbackBanner: "This isn't a standard rule, so we drafted it as an agent rule. Agent rules act after an expense or report changes, for example by holding the expense or rejecting the report.", fallbackBannerWithExplanation: (explanation: string) => `This isn't a standard rule, so we drafted it as an agent rule. ${explanation}`, diff --git a/src/languages/es.ts b/src/languages/es.ts index 206a8d0429ad..4b2c74a8678d 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -8829,6 +8829,9 @@ ${reportName}`, describeRuleForConcierge: 'Describe tu regla y Concierge la creará', gotIt: 'Entendido', createRule: 'Crear regla', + createAnyway: 'Crear de todos modos', + saveAnyway: 'Guardar de todos modos', + checkingRule: 'Comprobando tu regla...', fallbackBanner: 'Esta no es una regla estándar, así que la redactamos como una regla de agente. Las reglas de agente actúan después de que un gasto o informe cambia, por ejemplo reteniendo el gasto o rechazando el informe.', fallbackBannerWithExplanation: (explanation: string) => `Esta no es una regla estándar, así que la redactamos como una regla de agente. ${explanation}`, diff --git a/src/languages/fr.ts b/src/languages/fr.ts index 13810352929c..ca9efc6e131d 100644 --- a/src/languages/fr.ts +++ b/src/languages/fr.ts @@ -8844,6 +8844,9 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e describeRuleForConcierge: 'Décrivez votre règle et Concierge la créera', gotIt: 'Compris', createRule: 'Créer une règle', + createAnyway: 'Créer quand même', + saveAnyway: 'Enregistrer quand même', + checkingRule: 'Vérification de votre règle...', fallbackBanner: 'Ce n’est pas une règle standard, nous l’avons donc rédigée comme une règle d’agent. Les règles d’agent agissent après la modification d’une dépense ou d’une note de frais, par exemple en bloquant la dépense ou en rejetant la note de frais.', fallbackBannerWithExplanation: (explanation: string) => `Ce n’est pas une règle standard, nous l’avons donc rédigée comme une règle d’agent. ${explanation}`, diff --git a/src/languages/it.ts b/src/languages/it.ts index 7fa79f709757..4cf55ea79774 100644 --- a/src/languages/it.ts +++ b/src/languages/it.ts @@ -8775,6 +8775,9 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`, describeRuleForConcierge: 'Descrivi la tua regola e Concierge la creerà', gotIt: 'Capito', createRule: 'Crea regola', + createAnyway: 'Crea comunque', + saveAnyway: 'Salva comunque', + checkingRule: 'Verifica della tua regola in corso...', fallbackBanner: 'Questa non è una regola standard, quindi l’abbiamo creata come regola agente. Le regole agente agiscono dopo che una spesa o un report vengono modificati, ad esempio bloccando la spesa o rifiutando il report.', fallbackBannerWithExplanation: (explanation: string) => `Questa non è una regola standard, quindi l’abbiamo formulata come regola dell’agente. ${explanation}`, diff --git a/src/languages/ja.ts b/src/languages/ja.ts index e3c76e4b3392..87b560c1900b 100644 --- a/src/languages/ja.ts +++ b/src/languages/ja.ts @@ -8661,6 +8661,9 @@ ${reportName}`, describeRuleForConcierge: 'ルールの内容を入力すると、Concierge が自動作成します', gotIt: '了解しました', createRule: 'ルールを作成', + createAnyway: 'このまま作成', + saveAnyway: 'このまま保存', + checkingRule: 'ルールを確認しています...', fallbackBanner: 'これは標準ルールではないため、エージェントルールとして作成しました。エージェントルールは、経費やレポートに変更があった後に動作し、例えば経費を保留にしたり、レポートを却下したりします。', fallbackBannerWithExplanation: (explanation: string) => `これは標準ルールではないため、エージェントルールとして作成しました。${explanation}`, diff --git a/src/languages/nl.ts b/src/languages/nl.ts index 0de492600d27..f6f92bc4ac71 100644 --- a/src/languages/nl.ts +++ b/src/languages/nl.ts @@ -8749,6 +8749,9 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`, describeRuleForConcierge: 'Beschrijf je regel en Concierge bouwt hem voor je', gotIt: 'Begrepen', createRule: 'Regel maken', + createAnyway: 'Toch maken', + saveAnyway: 'Toch opslaan', + checkingRule: 'Je regel wordt gecontroleerd...', fallbackBanner: 'Dit is geen standaardregel, dus hebben we hem opgesteld als een agentregel. Agentregels treden in werking nadat een uitgave of rapport verandert, bijvoorbeeld door de uitgave vast te houden of het rapport af te wijzen.', fallbackBannerWithExplanation: (explanation: string) => `Dit is geen standaardregel, dus hebben we het opgesteld als een agentregel. ${explanation}`, diff --git a/src/languages/pl.ts b/src/languages/pl.ts index afc2ef292e3e..1e9357e72789 100644 --- a/src/languages/pl.ts +++ b/src/languages/pl.ts @@ -8776,6 +8776,9 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`, describeRuleForConcierge: 'Opisz swoją regułę, a Concierge ją utworzy', gotIt: 'Jasne', createRule: 'Utwórz regułę', + createAnyway: 'Utwórz mimo to', + saveAnyway: 'Zapisz mimo to', + checkingRule: 'Sprawdzanie reguły...', fallbackBanner: 'To nie jest standardowa reguła, więc przygotowaliśmy ją jako regułę agenta. Reguły agenta działają po zmianie wydatku lub raportu, na przykład wstrzymując wydatek albo odrzucając raport.', fallbackBannerWithExplanation: (explanation: string) => `To nie jest standardowa reguła, więc przygotowaliśmy ją jako regułę agenta. ${explanation}`, diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts index bf61af514119..eb7874d7c103 100644 --- a/src/languages/pt-BR.ts +++ b/src/languages/pt-BR.ts @@ -8753,6 +8753,9 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`, describeRuleForConcierge: 'Descreva sua regra e o Concierge vai criá-la', gotIt: 'Entendi', createRule: 'Criar regra', + createAnyway: 'Criar mesmo assim', + saveAnyway: 'Salvar mesmo assim', + checkingRule: 'Verificando sua regra...', fallbackBanner: 'Esta não é uma regra padrão, então a criamos como uma regra de agente. As regras de agente entram em ação depois que uma despesa ou um relatório é alterado, por exemplo, ao reter a despesa ou rejeitar o relatório.', fallbackBannerWithExplanation: (explanation: string) => `Esta não é uma regra padrão, então nós a elaboramos como uma regra de agente. ${explanation}`, diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts index 238d15ce72cd..50b347c7b15e 100644 --- a/src/languages/zh-hans.ts +++ b/src/languages/zh-hans.ts @@ -8450,6 +8450,9 @@ ${reportName}`, describeRuleForConcierge: '描述你的规则，Concierge 会帮你创建', gotIt: '知道了', createRule: '创建规则', + createAnyway: '仍然创建', + saveAnyway: '仍然保存', + checkingRule: '正在检查你的规则...', fallbackBanner: '这不是一条标准规则，所以我们将其制定为代理规则。代理规则会在报销或报表发生变更后生效，例如暂扣该报销或驳回该报表。', fallbackBannerWithExplanation: (explanation: string) => `这不是一条标准规则，因此我们将其起草为代理规则。${explanation}`, }, diff --git a/src/libs/actions/Policy/Rules.ts b/src/libs/actions/Policy/Rules.ts index c899012c7a64..a19491a9d5f9 100644 --- a/src/libs/actions/Policy/Rules.ts +++ b/src/libs/actions/Policy/Rules.ts @@ -369,6 +369,45 @@ function generateRule(policyID: string, prompt: string, ruleType?: GeneratedRule return generationID; } +type AgentRuleFormID = typeof ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM | typeof ONYXKEYS.FORMS.EDIT_AGENT_RULE_FORM; + +/** + * Asks Concierge whether RuleBot can carry out an agent rule before the rule saves. The agent rule form shows its loading + * state until the answer arrives under `ONYXKEYS.GENERATED_RULE`. A failed request gets a `failed` answer, so the rule + * still saves. + * @returns the generationID the answer carries + */ +function checkAgentRulePrompt(formID: AgentRuleFormID, policyID: string, prompt: string): string { + const generationID = rand64(); + type CheckAgentRuleKey = AgentRuleFormID | typeof ONYXKEYS.GENERATED_RULE; + + const optimisticData: Array> = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: formID, + value: {isLoading: true}, + }, + ]; + const failureData: Array> = [ + { + onyxMethod: Onyx.METHOD.SET, + key: ONYXKEYS.GENERATED_RULE, + value: {generationID, state: CONST.GENERATED_RULE.STATE.FAILED}, + }, + ]; + + const parameters: GenerateRuleParams = {policyID, generationID, prompt}; + + API.write(WRITE_COMMANDS.GENERATE_RULE, parameters, {optimisticData, failureData}); + + return generationID; +} + +/** Stops the loading state of an agent rule form after Concierge answers the check */ +function stopAgentRuleCheckLoading(formID: AgentRuleFormID) { + Onyx.merge(formID, {isLoading: false}); +} + /** Shows on the prompt form why a description did not become a rule */ function setNewRulePromptError(state: GeneratedRuleState) { setNewRulePromptErrorMessage(PROMPT_ERROR_BY_STATE[state] ?? 'common.genericErrorMessage'); @@ -677,9 +716,11 @@ export { clearGeneratedRule, clearNewRulePrompt, stopNewRulePromptLoading, + checkAgentRulePrompt, + stopAgentRuleCheckLoading, updatePolicyAgentRule, deletePolicyAgentRule, clearMerchantRuleErrors, clearPolicyAgentRuleErrors, }; -export type {ImportedMerchantRule}; +export type {AgentRuleFormID, ImportedMerchantRule}; diff --git a/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx b/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx index e146e2c0f1eb..9710271fb991 100644 --- a/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx +++ b/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx @@ -82,12 +82,18 @@ function AddAgentRulePage({ const isDraftFromFallback = !!generationID && !didSelectSuggestion; const fallbackExplanation = generatedRule?.generationID === generationID ? generatedRule?.explanation : undefined; + // A suggestion or a draft of the describe flow saves without a check while the admin leaves its text unchanged + const [checkedPrompt, setCheckedPrompt] = useState(); + // The draft can load after the first render. Open Write one time only, because the admin must still be able to go // to Suggestions. const [didOpenTabForDraft, setDidOpenTabForDraft] = useState(false); if (!didOpenTabForDraft && hasDraftPrompt) { setDidOpenTabForDraft(true); setActiveTab(CONST.TAB.AGENT_RULE.WRITE); + if (generationID) { + setCheckedPrompt(draftValues?.[INPUT_IDS.PROMPT]); + } } // Reset the active tab to Suggestions when the workspace changes. @@ -131,6 +137,7 @@ function AddAgentRulePage({ return; } setDraftValues(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM, {[INPUT_IDS.PROMPT]: suggestion.prompt}); + setCheckedPrompt(suggestion.prompt); setDidSelectSuggestion(true); setActiveTab(CONST.TAB.AGENT_RULE.WRITE); }; @@ -267,7 +274,11 @@ function AddAgentRulePage({ textStyles={[styles.breakWord, styles.textSupporting]} /> )} - + )} diff --git a/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx b/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx index afb99b1578a8..c65d22220e2c 100644 --- a/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx +++ b/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx @@ -8,8 +8,11 @@ import Text from '@components/Text'; import TextInput from '@components/TextInput'; import useLocalize from '@hooks/useLocalize'; +import useOnyx from '@hooks/useOnyx'; import useThemeStyles from '@hooks/useThemeStyles'; +import ImpossibleRuleNotice from '@pages/workspace/rules/ImpossibleRuleNotice'; + import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import INPUT_IDS from '@src/types/form/AddAgentRuleForm'; @@ -20,18 +23,31 @@ import React, {useRef} from 'react'; import {View} from 'react-native'; import useAgentPromptInputStyles from './useAgentPromptInputStyles'; +import useAgentRulePromptCheck from './useAgentRulePromptCheck'; type AddAgentRuleFormID = typeof ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM; type AddAgentRuleWriteTabProps = { + policyID: string; + + /** A rule with this text saves without a check, because Concierge already wrote it */ + checkedPrompt?: string; + /** Called with the form values when the user saves the rule */ onSave: (values: FormOnyxValues) => void; }; -function AddAgentRuleWriteTab({onSave}: AddAgentRuleWriteTabProps) { +function AddAgentRuleWriteTab({policyID, checkedPrompt, onSave}: AddAgentRuleWriteTabProps) { const {translate} = useLocalize(); const styles = useThemeStyles(); const formRef = useRef(null); + const [isChecking = false] = useOnyx(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM, {selector: (form) => !!form?.isLoading}); + const {refusal, checkThenSavePrompt, checkSuggestedPrompt, dismissRefusal} = useAgentRulePromptCheck({ + policyID, + formID: ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM, + checkedPrompt, + savePrompt: (prompt) => onSave({[INPUT_IDS.PROMPT]: prompt}), + }); const describeRuleLabel = translate('workspace.rules.agentRules.describeRuleForConcierge'); const submitFormOnModEnter = (event: TextInputKeyPressEvent | KeyboardEvent) => { @@ -55,13 +71,18 @@ function AddAgentRuleWriteTab({onSave}: AddAgentRuleWriteTabProps) { const inputWrapperStyles = useAgentPromptInputStyles(); + const trySuggestedPrompt = (suggestedPrompt: string) => { + formRef.current?.resetForm({[INPUT_IDS.PROMPT]: suggestedPrompt}); + checkSuggestedPrompt(suggestedPrompt); + }; + return ( checkThenSavePrompt(values[INPUT_IDS.PROMPT])} + submitButtonText={translate(refusal ? 'workspace.rules.agentRules.createAnyway' : 'workspace.rules.agentRules.createRule')} style={[styles.flex1, styles.ph5]} submitFlexEnabled={false} shouldUseScrollView={false} @@ -82,6 +103,7 @@ function AddAgentRuleWriteTab({onSave}: AddAgentRuleWriteTabProps) { type="markdown" excludedMarkdownStyles={['mentionReport']} onKeyPress={submitFormOnModEnter} + onValueChange={dismissRefusal} multiline shouldSaveDraft shouldLabelStayOnSingleLine @@ -90,7 +112,15 @@ function AddAgentRuleWriteTab({onSave}: AddAgentRuleWriteTabProps) { inputStyle={[styles.flex1, styles.textAlignVerticalTop]} /> - {translate('workspace.rules.agentRules.disclaimer')} + {!!refusal && ( + + )} + + {translate(isChecking ? 'workspace.rules.agentRules.checkingRule' : 'workspace.rules.agentRules.disclaimer')} + ); diff --git a/src/pages/workspace/rules/AgentRules/EditAgentRulePage.tsx b/src/pages/workspace/rules/AgentRules/EditAgentRulePage.tsx index 55480fd16710..d9f6532a5dec 100644 --- a/src/pages/workspace/rules/AgentRules/EditAgentRulePage.tsx +++ b/src/pages/workspace/rules/AgentRules/EditAgentRulePage.tsx @@ -9,6 +9,7 @@ import Text from '@components/Text'; import TextInput from '@components/TextInput'; import useLocalize from '@hooks/useLocalize'; +import useOnyx from '@hooks/useOnyx'; import usePolicy from '@hooks/usePolicy'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -18,6 +19,7 @@ import type {SettingsNavigatorParamList} from '@libs/Navigation/types'; import NotFoundPage from '@pages/ErrorPage/NotFoundPage'; import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper'; +import ImpossibleRuleNotice from '@pages/workspace/rules/ImpossibleRuleNotice'; import useRuleDeleteHeaderProps from '@pages/workspace/rules/useRuleDeleteHeaderProps'; import {deletePolicyAgentRule, updatePolicyAgentRule} from '@userActions/Policy/Rules'; @@ -33,6 +35,7 @@ import React, {useRef} from 'react'; import {View} from 'react-native'; import useAgentPromptInputStyles from './useAgentPromptInputStyles'; +import useAgentRulePromptCheck from './useAgentRulePromptCheck'; import useShouldUseExpandedRevampFormLayout from './useShouldUseExpandedRevampFormLayout'; type EditAgentRulePageProps = PlatformStackScreenProps; @@ -70,8 +73,7 @@ function EditAgentRulePage({ return errors; }; - const saveRule = (values: FormOnyxValues): void => { - const newPrompt = values[INPUT_IDS.PROMPT]; + const saveRule = (newPrompt: string): void => { const previousPrompt = agentRule?.prompt ?? ''; if (newPrompt !== previousPrompt) { updatePolicyAgentRule(policyID, ruleID, newPrompt, previousPrompt, agentRule?.title); @@ -79,6 +81,19 @@ function EditAgentRulePage({ Navigation.goBack(); }; + const [isChecking = false] = useOnyx(ONYXKEYS.FORMS.EDIT_AGENT_RULE_FORM, {selector: (form) => !!form?.isLoading}); + const {refusal, checkThenSavePrompt, checkSuggestedPrompt, dismissRefusal} = useAgentRulePromptCheck({ + policyID, + formID: ONYXKEYS.FORMS.EDIT_AGENT_RULE_FORM, + checkedPrompt: agentRule?.prompt, + savePrompt: saveRule, + }); + + const trySuggestedPrompt = (suggestedPrompt: string) => { + formRef.current?.resetForm({[INPUT_IDS.PROMPT]: suggestedPrompt}); + checkSuggestedPrompt(suggestedPrompt); + }; + const inputWrapperStyles = useAgentPromptInputStyles(); const {deleteIconButtonProps} = useRuleDeleteHeaderProps({ @@ -120,8 +135,8 @@ function EditAgentRulePage({ ref={formRef} formID={ONYXKEYS.FORMS.EDIT_AGENT_RULE_FORM} validate={validate} - onSubmit={saveRule} - submitButtonText={translate('common.save')} + onSubmit={(values) => checkThenSavePrompt(values[INPUT_IDS.PROMPT])} + submitButtonText={translate(refusal ? 'workspace.rules.agentRules.saveAnyway' : 'common.save')} style={[styles.flex1, styles.ph5]} shouldUseScrollView={false} submitFlexEnabled={false} @@ -142,6 +157,7 @@ function EditAgentRulePage({ type="markdown" excludedMarkdownStyles={['mentionReport']} onKeyPress={submitFormOnModEnter} + onValueChange={dismissRefusal} defaultValue={agentRule.prompt} multiline shouldLabelStayOnSingleLine @@ -150,7 +166,15 @@ function EditAgentRulePage({ inputStyle={[styles.flex1, styles.textAlignVerticalTop]} /> - {translate('workspace.rules.agentRules.disclaimer')} + {!!refusal && ( + + )} + + {translate(isChecking ? 'workspace.rules.agentRules.checkingRule' : 'workspace.rules.agentRules.disclaimer')} + diff --git a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts new file mode 100644 index 000000000000..582598e3763b --- /dev/null +++ b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts @@ -0,0 +1,125 @@ +/** + * Checks with Concierge whether RuleBot can carry out an agent rule that the admin wrote, before the rule saves. An + * `impossible` answer stops the save and returns the refusal. A save after the refusal, or any other answer, saves the rule. + */ +import useNetwork from '@hooks/useNetwork'; +import useOnyx from '@hooks/useOnyx'; +import usePermissions from '@hooks/usePermissions'; + +import Log from '@libs/Log'; + +import {checkAgentRulePrompt, stopAgentRuleCheckLoading} from '@userActions/Policy/Rules'; +import type {AgentRuleFormID} from '@userActions/Policy/Rules'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import {useEffect, useRef, useState} from 'react'; + +type UseAgentRulePromptCheckParams = { + policyID: string; + + /** The form that shows the loading state during the check */ + formID: AgentRuleFormID; + + /** A rule with this text saves without a check, because Concierge already wrote it or the admin saved it before */ + checkedPrompt?: string; + + /** Saves the rule with this text */ + savePrompt: (prompt: string) => void; +}; + +type PendingCheck = { + generationID: string; + prompt: string; +}; + +function useAgentRulePromptCheck({policyID, formID, checkedPrompt, savePrompt}: UseAgentRulePromptCheckParams) { + const {isOffline} = useNetwork(); + const {isBetaEnabled} = usePermissions(); + const [generatedRule] = useOnyx(ONYXKEYS.GENERATED_RULE); + const pendingCheckRef = useRef(undefined); + const timeoutRef = useRef>(undefined); + const [checkGenerationID, setCheckGenerationID] = useState(); + const generatedRuleID = generatedRule?.generationID; + const generatedRuleState = generatedRule?.state; + const generatedRuleReason = generatedRule?.reason; + const refusal = generatedRuleState === CONST.GENERATED_RULE.STATE.IMPOSSIBLE && !!generatedRuleID && generatedRuleID === checkGenerationID ? generatedRule : undefined; + + const finishCheck = () => { + pendingCheckRef.current = undefined; + clearTimeout(timeoutRef.current); + stopAgentRuleCheckLoading(formID); + }; + + useEffect(() => { + const pendingCheck = pendingCheckRef.current; + if (!pendingCheck || !generatedRuleState || generatedRuleID !== pendingCheck.generationID) { + return; + } + finishCheck(); + if (generatedRuleState === CONST.GENERATED_RULE.STATE.IMPOSSIBLE) { + Log.info('[GenerateRule] Warned about an agent rule before it saved', false, {policyID, generationID: generatedRuleID, reason: generatedRuleReason}); + return; + } + savePrompt(pendingCheck.prompt); + }, [finishCheck, generatedRuleID, generatedRuleReason, generatedRuleState, policyID, savePrompt]); + + // The loading state stays in Onyx. Stop it when the admin leaves during a check, so that the form does not open loading. + useEffect( + () => () => { + clearTimeout(timeoutRef.current); + if (!pendingCheckRef.current) { + return; + } + stopAgentRuleCheckLoading(formID); + }, + [formID], + ); + + const startCheck = (prompt: string) => { + const generationID = checkAgentRulePrompt(formID, policyID, prompt); + pendingCheckRef.current = {generationID, prompt}; + setCheckGenerationID(generationID); + + // The rule must still save when the answer never arrives + clearTimeout(timeoutRef.current); + timeoutRef.current = setTimeout(() => { + if (pendingCheckRef.current?.generationID !== generationID) { + return; + } + Log.info('[GenerateRule] Saved an agent rule without an answer to its check', false, {policyID, generationID}); + finishCheck(); + savePrompt(prompt); + }, CONST.GENERATED_RULE.AGENT_RULE_CHECK_TIMEOUT_MS); + }; + + /** Saves the rule, after a check when the admin wrote or changed the text */ + const checkThenSavePrompt = (prompt: string) => { + if (refusal) { + Log.info('[GenerateRule] Saved an agent rule after a warning', false, {policyID, generationID: refusal.generationID, reason: refusal.reason}); + setCheckGenerationID(undefined); + savePrompt(prompt); + return; + } + const isUnchanged = !!checkedPrompt && prompt.trim() === checkedPrompt.trim(); + if (isUnchanged || isOffline || !isBetaEnabled(CONST.BETAS.DESCRIBE_RULE)) { + savePrompt(prompt); + return; + } + startCheck(prompt); + }; + + /** Checks the suggested rule of the refusal, which the form now shows in place of the admin's text */ + const checkSuggestedPrompt = (suggestedPrompt: string) => { + Log.info('[GenerateRule] Tried the suggested description', false, {policyID, generationID: refusal?.generationID, reason: refusal?.reason}); + startCheck(suggestedPrompt); + }; + + /** Hides the refusal when the admin changes the text, so that the next save checks the new text */ + const dismissRefusal = () => setCheckGenerationID(undefined); + + return {refusal, checkThenSavePrompt, checkSuggestedPrompt, dismissRefusal}; +} + +export default useAgentRulePromptCheck; diff --git a/src/pages/workspace/rules/DescribeRuleForm.tsx b/src/pages/workspace/rules/DescribeRuleForm.tsx index 3c1eafa7ef57..ebae7a0723cf 100644 --- a/src/pages/workspace/rules/DescribeRuleForm.tsx +++ b/src/pages/workspace/rules/DescribeRuleForm.tsx @@ -1,12 +1,10 @@ /** * Form where an admin describes a rule in plain English and Concierge builds the rule. */ -import Button from '@components/Button'; import FormProvider from '@components/Form/FormProvider'; import InputWrapper from '@components/Form/InputWrapper'; import type {FormInputErrors, FormOnyxValues, FormRef} from '@components/Form/types'; import type {AnimatedTextInputRef} from '@components/RNTextInput'; -import Text from '@components/Text'; import TextInput from '@components/TextInput'; import useLocalize from '@hooks/useLocalize'; @@ -27,11 +25,10 @@ import { } from '@userActions/Policy/Rules'; import CONST from '@src/CONST'; -import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import INPUT_IDS from '@src/types/form/NewRulePromptForm'; import type {PolicyCategories, PolicyCategory} from '@src/types/onyx'; -import type {GeneratedRuleImpossibleReason, GeneratedRuleState, GeneratedRuleType, GeneratedRuleValues} from '@src/types/onyx/GeneratedRule'; +import type {GeneratedRuleState, GeneratedRuleType, GeneratedRuleValues} from '@src/types/onyx/GeneratedRule'; import type {ReactNode} from 'react'; import type {StyleProp, TextInputKeyPressEvent, ViewStyle} from 'react-native'; @@ -41,6 +38,8 @@ import {useFocusEffect} from '@react-navigation/native'; import React, {useEffect, useRef, useState} from 'react'; import {View} from 'react-native'; +import ImpossibleRuleNotice from './ImpossibleRuleNotice'; + type NewRulePromptFormID = typeof ONYXKEYS.FORMS.NEW_RULE_PROMPT_FORM; type DescribeRuleFormProps = { @@ -100,15 +99,6 @@ type PendingGeneration = { const GENERATED_RULE_TYPES = new Set(Object.values(CONST.GENERATED_RULE.RULE_TYPE)); -// A reason that this app version does not know gets the general title -const IMPOSSIBLE_TITLE_BY_REASON: Partial> = { - [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.NO_TRIGGER]: 'workspace.rules.newRule.impossible.noTrigger', - [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.NO_CAPABILITY]: 'workspace.rules.newRule.impossible.noCapability', - [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.MISSING_DATA]: 'workspace.rules.newRule.impossible.missingData', - [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.RETROACTIVE]: 'workspace.rules.newRule.impossible.retroactive', - [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.HISTORY_LIMIT]: 'workspace.rules.newRule.impossible.historyLimit', -} satisfies Record; - /** Returns whether a description with this answer can become an agent rule */ function isAgentRuleFallbackState(state: GeneratedRuleState): boolean { return state === CONST.GENERATED_RULE.STATE.UNSUPPORTED || state === CONST.GENERATED_RULE.STATE.MULTIPLE_RULES || state === CONST.GENERATED_RULE.STATE.AGENT_RULE; @@ -162,7 +152,6 @@ function DescribeRuleForm({ const generatedRuleValues = generatedRule?.rule; const generatedAgentRulePrompt = generatedRule?.agentRulePrompt; const refusal = generatedRuleState === CONST.GENERATED_RULE.STATE.IMPOSSIBLE && !!generatedRuleID && generatedRuleID === submittedGenerationID ? generatedRule : undefined; - const refusalSuggestedPrompt = refusal?.suggestedPrompt; const inputLabel = translate('workspace.rules.newRule.describeRuleInputLabel'); const finish = () => { @@ -335,23 +324,10 @@ function DescribeRuleForm({ /> {!!refusal && ( - - {translate(IMPOSSIBLE_TITLE_BY_REASON[refusal.reason ?? ''] ?? 'workspace.rules.newRule.impossible.noCapability')} - {!!refusal.explanation && {refusal.explanation}} - {!!refusalSuggestedPrompt && ( - <> - {refusalSuggestedPrompt} - - - )} - + )} ); diff --git a/src/pages/workspace/rules/ImpossibleRuleNotice.tsx b/src/pages/workspace/rules/ImpossibleRuleNotice.tsx new file mode 100644 index 000000000000..e11448aa786a --- /dev/null +++ b/src/pages/workspace/rules/ImpossibleRuleNotice.tsx @@ -0,0 +1,63 @@ +/** + * Tells the admin why Concierge refused a rule, and offers the closest rule that RuleBot can carry out. + */ +import Button from '@components/Button'; +import Text from '@components/Text'; + +import useLocalize from '@hooks/useLocalize'; +import useThemeStyles from '@hooks/useThemeStyles'; + +import CONST from '@src/CONST'; +import type {TranslationPaths} from '@src/languages/types'; +import type {GeneratedRule} from '@src/types/onyx'; +import type {GeneratedRuleImpossibleReason} from '@src/types/onyx/GeneratedRule'; + +import React from 'react'; +import {View} from 'react-native'; + +type ImpossibleRuleNoticeProps = { + /** The `impossible` answer of Concierge */ + refusal: GeneratedRule; + + /** Called with the suggested rule when the admin taps Try this instead */ + onTrySuggestion: (suggestedPrompt: string) => void; +}; + +// A reason that this app version does not know gets the general title +const IMPOSSIBLE_TITLE_BY_REASON: Partial> = { + [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.NO_TRIGGER]: 'workspace.rules.newRule.impossible.noTrigger', + [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.NO_CAPABILITY]: 'workspace.rules.newRule.impossible.noCapability', + [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.MISSING_DATA]: 'workspace.rules.newRule.impossible.missingData', + [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.RETROACTIVE]: 'workspace.rules.newRule.impossible.retroactive', + [CONST.GENERATED_RULE.IMPOSSIBLE_REASON.HISTORY_LIMIT]: 'workspace.rules.newRule.impossible.historyLimit', +} satisfies Record; + +function ImpossibleRuleNotice({refusal, onTrySuggestion}: ImpossibleRuleNoticeProps) { + const {translate} = useLocalize(); + const styles = useThemeStyles(); + const suggestedPrompt = refusal.suggestedPrompt; + + return ( + + {translate(IMPOSSIBLE_TITLE_BY_REASON[refusal.reason ?? ''] ?? 'workspace.rules.newRule.impossible.noCapability')} + {!!refusal.explanation && {refusal.explanation}} + {!!suggestedPrompt && ( + <> + {suggestedPrompt} + + + )} + + ); +} + +ImpossibleRuleNotice.displayName = 'ImpossibleRuleNotice'; + +export default ImpossibleRuleNotice; diff --git a/tests/ui/AddAgentRulePageTest.tsx b/tests/ui/AddAgentRulePageTest.tsx index f5da3577c6fa..d3c436a5d262 100644 --- a/tests/ui/AddAgentRulePageTest.tsx +++ b/tests/ui/AddAgentRulePageTest.tsx @@ -114,8 +114,13 @@ jest.mock('@pages/workspace/rules/AgentRules/AddAgentRuleWriteTab', () => { Pressable: React.ComponentType<{accessibilityLabel?: string; onPress?: () => void; children?: React.ReactNode}>; Text: React.ComponentType<{children?: React.ReactNode}>; }>('react-native'); - return ({onSave}: {onSave: (values: {prompt: string}) => void}) => - ReactModule.createElement(Pressable, {accessibilityLabel: 'WriteTab', onPress: () => onSave({prompt: 'Hold gift card expenses'})}, ReactModule.createElement(Text, null, 'WriteTab')); + return ({onSave, checkedPrompt}: {onSave: (values: {prompt: string}) => void; checkedPrompt?: string}) => + ReactModule.createElement( + Pressable, + {accessibilityLabel: 'WriteTab', onPress: () => onSave({prompt: 'Hold gift card expenses'})}, + ReactModule.createElement(Text, null, 'WriteTab'), + ReactModule.createElement(Text, null, `CheckedPrompt: ${checkedPrompt ?? 'none'}`), + ); }); type AddAgentRulePageProps = PlatformStackScreenProps; @@ -202,6 +207,43 @@ describe('AddAgentRulePage', () => { expect(screen.getByText('WriteTab')).toBeOnTheScreen(); }); + it('lets the draft of the describe flow save without another check', async () => { + // Given a draft that Concierge wrote in the describe flow + await setAgentRuleDraft(PROMPT); + + // When the page renders with the generationID of that description + renderAddAgentRulePage(GENERATION_ID); + await waitForBatchedUpdatesWithAct(); + + // Then the Write tab treats the draft as checked, because Concierge already answered that RuleBot can do it + expect(screen.getByText(`CheckedPrompt: ${PROMPT}`)).toBeOnTheScreen(); + }); + + it('checks a draft that has no generationID', async () => { + // Given a draft that the admin typed on an earlier visit + await setAgentRuleDraft(PROMPT); + + // When the page renders without a generationID + renderAddAgentRulePage(); + await waitForBatchedUpdatesWithAct(); + + // Then the Write tab checks the draft before it saves, because Concierge never saw it + expect(screen.getByText('CheckedPrompt: none')).toBeOnTheScreen(); + }); + + it('lets a picked suggestion save without a check', async () => { + // Given the page opens on the suggestions + renderAddAgentRulePage(); + await waitForBatchedUpdatesWithAct(); + + // When the admin picks a suggestion + fireEvent.press(screen.getByLabelText('PickSuggestion')); + await waitForBatchedUpdatesWithAct(); + + // Then the Write tab treats the suggestion as checked, because Concierge wrote it + expect(screen.getByText('CheckedPrompt: Hold any expense over $75 without a receipt')).toBeOnTheScreen(); + }); + it('shows the fallback banner when the page opens with a generationID', async () => { // Given a draft that came from a description that was not a standard rule await setAgentRuleDraft(PROMPT); diff --git a/tests/ui/AgentRulePromptCheckTest.tsx b/tests/ui/AgentRulePromptCheckTest.tsx new file mode 100644 index 000000000000..7060c9ffa251 --- /dev/null +++ b/tests/ui/AgentRulePromptCheckTest.tsx @@ -0,0 +1,407 @@ +import {act, fireEvent, render, screen} from '@testing-library/react-native'; + +import usePolicy from '@hooks/usePolicy'; + +import * as API from '@libs/API'; +import type {GenerateRuleParams} from '@libs/API/parameters'; +import {WRITE_COMMANDS} from '@libs/API/types'; +import Navigation from '@libs/Navigation/Navigation'; +import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; +import type {SettingsNavigatorParamList} from '@libs/Navigation/types'; + +import AddAgentRuleWriteTab from '@pages/workspace/rules/AgentRules/AddAgentRuleWriteTab'; +import EditAgentRulePage from '@pages/workspace/rules/AgentRules/EditAgentRulePage'; + +import {updatePolicyAgentRule} from '@userActions/Policy/Rules'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import SCREENS from '@src/SCREENS'; +import type {GeneratedRule} from '@src/types/onyx'; + +import React from 'react'; +import Onyx from 'react-native-onyx'; + +import createRandomPolicy from '../utils/collections/policies'; +import getOnyxValue from '../utils/getOnyxValue'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +let mockPrompt = ''; +let mockIsBetaEnabled = true; +let mockIsOffline = false; + +jest.mock('@hooks/useLocalize', () => + jest.fn(() => ({ + translate: (key: string) => key, + })), +); +jest.mock('@hooks/useThemeStyles', () => + jest.fn( + () => + new Proxy( + {}, + { + get: () => ({}), + }, + ), + ), +); +jest.mock('@hooks/usePermissions', () => jest.fn(() => ({isBetaEnabled: () => mockIsBetaEnabled}))); +jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: mockIsOffline}))); +jest.mock('@hooks/usePolicy', () => jest.fn()); +jest.mock('@pages/workspace/rules/AgentRules/useAgentPromptInputStyles', () => jest.fn(() => ({}))); +jest.mock('@pages/workspace/rules/useRuleDeleteHeaderProps', () => jest.fn(() => ({deleteIconButtonProps: undefined}))); +jest.mock( + '@pages/workspace/AccessOrNotFoundWrapper', + () => + ({children}: {children: React.ReactNode}) => + children, +); +jest.mock( + '@components/ScreenWrapper', + () => + ({children}: {children: React.ReactNode}) => + children, +); +jest.mock( + '@components/CollapsibleHeaderOnKeyboard', + () => + ({children}: {children: React.ReactNode}) => + children, +); +jest.mock('@components/Header/composed/HeaderWithBackButtonAndTitle', () => () => null); +jest.mock('@libs/Navigation/Navigation', () => ({goBack: jest.fn()})); +jest.mock('@userActions/Policy/Rules', () => ({ + ...jest.requireActual>('@userActions/Policy/Rules'), + updatePolicyAgentRule: jest.fn(), + deletePolicyAgentRule: jest.fn(), +})); +jest.mock('@components/Text', () => { + const ReactModule = jest.requireActual('react'); + const {Text} = jest.requireActual<{Text: React.ComponentType<{children?: React.ReactNode}>}>('react-native'); + return ({children}: {children: React.ReactNode}) => ReactModule.createElement(Text, null, children); +}); +jest.mock('@components/Button', () => { + const ReactModule = jest.requireActual('react'); + const {Pressable, Text} = jest.requireActual<{ + Pressable: React.ComponentType<{onPress?: () => void; children?: React.ReactNode}>; + Text: React.ComponentType<{children?: React.ReactNode}>; + }>('react-native'); + const MockButton = ({onPress, children}: {onPress: () => void; children: React.ReactNode}) => ReactModule.createElement(Pressable, {onPress}, children); + MockButton.Text = ({children}: {children: React.ReactNode}) => ReactModule.createElement(Text, null, children); + return MockButton; +}); +jest.mock('@components/Form/InputWrapper', () => () => null); +jest.mock('@components/Form/FormProvider', () => { + const ReactModule = jest.requireActual('react'); + const {Pressable, Text} = jest.requireActual<{ + Pressable: React.ComponentType<{accessibilityLabel?: string; onPress?: () => void; children?: React.ReactNode}>; + Text: React.ComponentType<{children?: React.ReactNode}>; + }>('react-native'); + return ({children, onSubmit, submitButtonText}: {children: React.ReactNode; onSubmit: (values: {prompt: string}) => void; submitButtonText: string}) => + ReactModule.createElement( + ReactModule.Fragment, + null, + children, + ReactModule.createElement( + Pressable, + {accessibilityLabel: submitButtonText, onPress: () => onSubmit({prompt: mockPrompt})}, + ReactModule.createElement(Text, null, submitButtonText), + ), + ); +}); + +const POLICY_ID = 'policy1'; +const RULE_ID = 'rule1'; +const TYPED_PROMPT = "Text the approver's phone when a report is submitted"; +const SUGGESTED_PROMPT = 'When a report is submitted, send the approver a direct message'; +const CREATE_RULE = 'workspace.rules.agentRules.createRule'; +const CREATE_ANYWAY = 'workspace.rules.agentRules.createAnyway'; + +type WriteSpy = jest.SpiedFunction; +type EditAgentRulePageProps = PlatformStackScreenProps; + +function isGenerateRuleParams(parameters: unknown): parameters is GenerateRuleParams { + return typeof parameters === 'object' && parameters !== null && 'generationID' in parameters && 'prompt' in parameters; +} + +/** Returns the parameters of every GenerateRule request, oldest first */ +function getGenerateRuleRequests(writeSpy: WriteSpy): GenerateRuleParams[] { + return writeSpy.mock.calls + .filter(([command]) => command === WRITE_COMMANDS.GENERATE_RULE) + .map(([, parameters]) => parameters) + .filter(isGenerateRuleParams); +} + +function getLastGenerationID(writeSpy: WriteSpy): string { + const generationID = getGenerateRuleRequests(writeSpy).at(-1)?.generationID; + if (!generationID) { + throw new Error('The form did not request GenerateRule'); + } + return generationID; +} + +async function renderWriteTab(onSave: jest.Mock, checkedPrompt?: string) { + render( + , + ); + await waitForBatchedUpdatesWithAct(); +} + +async function submit(prompt: string, buttonLabel = CREATE_RULE) { + mockPrompt = prompt; + fireEvent.press(screen.getByLabelText(buttonLabel)); + await waitForBatchedUpdatesWithAct(); +} + +async function answerCheck(answer: GeneratedRule) { + await act(async () => { + await Onyx.set(ONYXKEYS.GENERATED_RULE, answer); + }); + await waitForBatchedUpdatesWithAct(); +} + +function buildImpossibleAnswer(generationID: string): GeneratedRule { + return { + generationID, + state: CONST.GENERATED_RULE.STATE.IMPOSSIBLE, + reason: CONST.GENERATED_RULE.IMPOSSIBLE_REASON.NO_CAPABILITY, + explanation: "Agents can't send text messages.", + suggestedPrompt: SUGGESTED_PROMPT, + }; +} + +describe('Agent rule prompt check', () => { + let writeSpy: WriteSpy; + + beforeEach(() => { + jest.clearAllMocks(); + mockPrompt = ''; + mockIsBetaEnabled = true; + mockIsOffline = false; + writeSpy = jest.spyOn(API, 'write').mockImplementation(() => Promise.resolve()); + }); + + afterEach(async () => { + jest.useRealTimers(); + writeSpy.mockRestore(); + await act(async () => { + await Onyx.clear(); + }); + }); + + it('saves a typed rule after Concierge answers that RuleBot can do it', async () => { + // Given an admin who typed an agent rule + const onSave = jest.fn(); + await renderWriteTab(onSave); + await submit(TYPED_PROMPT); + + // Then the rule waits for the check, which asks for any rule type because the admin did not pick one + expect(onSave).not.toHaveBeenCalled(); + const [request] = getGenerateRuleRequests(writeSpy); + expect(request).toMatchObject({policyID: POLICY_ID, prompt: TYPED_PROMPT}); + expect(request?.ruleType).toBeUndefined(); + + // When Concierge answers with an agent rule + await answerCheck({generationID: getLastGenerationID(writeSpy), state: CONST.GENERATED_RULE.STATE.AGENT_RULE}); + + // Then the rule saves with the text the admin typed, because only an impossible answer stops the save + expect(onSave).toHaveBeenCalledWith({prompt: TYPED_PROMPT}); + }); + + it('warns about an impossible rule and saves it when the admin taps Create anyway', async () => { + // Given an admin who typed a rule that RuleBot cannot carry out + const onSave = jest.fn(); + await renderWriteTab(onSave); + await submit(TYPED_PROMPT); + + // When Concierge answers that the rule is impossible + await answerCheck(buildImpossibleAnswer(getLastGenerationID(writeSpy))); + + // Then the rule does not save, and the form shows why with the suggested rule + expect(onSave).not.toHaveBeenCalled(); + expect(screen.getByText('workspace.rules.newRule.impossible.noCapability')).toBeTruthy(); + expect(screen.getByText("Agents can't send text messages.")).toBeTruthy(); + expect(screen.getByText(SUGGESTED_PROMPT)).toBeTruthy(); + + // When the admin taps Create anyway + await submit(TYPED_PROMPT, CREATE_ANYWAY); + + // Then the rule saves without another check, because the check only warns + expect(onSave).toHaveBeenCalledWith({prompt: TYPED_PROMPT}); + expect(getGenerateRuleRequests(writeSpy)).toHaveLength(1); + }); + + it('checks the suggested rule when the admin taps Try this instead', async () => { + // Given Concierge refused the typed rule + const onSave = jest.fn(); + await renderWriteTab(onSave); + await submit(TYPED_PROMPT); + await answerCheck(buildImpossibleAnswer(getLastGenerationID(writeSpy))); + + // When the admin taps Try this instead, and Concierge answers that RuleBot can do the suggestion + fireEvent.press(screen.getByText('workspace.rules.newRule.impossible.tryThisInstead')); + await waitForBatchedUpdatesWithAct(); + await answerCheck({generationID: getLastGenerationID(writeSpy), state: CONST.GENERATED_RULE.STATE.AGENT_RULE}); + + // Then the suggested rule is checked and saved in place of the typed rule + expect(getGenerateRuleRequests(writeSpy).at(-1)?.prompt).toBe(SUGGESTED_PROMPT); + expect(onSave).toHaveBeenCalledWith({prompt: SUGGESTED_PROMPT}); + }); + + it('saves an unchanged suggestion without a check', async () => { + // Given an admin who picked a suggested rule and did not change it + const onSave = jest.fn(); + await renderWriteTab(onSave, SUGGESTED_PROMPT); + + // When the admin creates the rule + await submit(SUGGESTED_PROMPT); + + // Then the rule saves at once, because Concierge already wrote the suggestion + expect(onSave).toHaveBeenCalledWith({prompt: SUGGESTED_PROMPT}); + expect(getGenerateRuleRequests(writeSpy)).toHaveLength(0); + }); + + it.each([ + {description: 'the describe rule beta is off', isBetaEnabled: false, isOffline: false}, + {description: 'the admin is offline', isBetaEnabled: true, isOffline: true}, + ])('saves a typed rule without a check when $description', async ({isBetaEnabled, isOffline}: {isBetaEnabled: boolean; isOffline: boolean}) => { + // Given an admin who typed an agent rule + mockIsBetaEnabled = isBetaEnabled; + mockIsOffline = isOffline; + const onSave = jest.fn(); + await renderWriteTab(onSave); + + // When the admin creates the rule + await submit(TYPED_PROMPT); + + // Then the rule saves at once, because the check cannot run or is not on for this account + expect(onSave).toHaveBeenCalledWith({prompt: TYPED_PROMPT}); + expect(getGenerateRuleRequests(writeSpy)).toHaveLength(0); + }); + + it('saves the rule when the check fails', async () => { + // Given an admin who typed an agent rule + const onSave = jest.fn(); + await renderWriteTab(onSave); + await submit(TYPED_PROMPT); + + // When the check fails + await answerCheck({generationID: getLastGenerationID(writeSpy), state: CONST.GENERATED_RULE.STATE.FAILED}); + + // Then the rule saves, because a failed check must not block the admin + expect(onSave).toHaveBeenCalledWith({prompt: TYPED_PROMPT}); + }); + + it('saves the rule when the answer does not arrive in time', async () => { + // Given an admin who typed an agent rule, and Concierge does not answer. The batched updates helper runs every + // pending timer when timers are fake, so this test flushes the microtasks itself. + jest.useFakeTimers({doNotFake: ['nextTick', 'setImmediate']}); + const flushMicrotasks = () => + act(async () => { + await Promise.resolve(); + }); + const onSave = jest.fn(); + render( + , + ); + await flushMicrotasks(); + mockPrompt = TYPED_PROMPT; + fireEvent.press(screen.getByLabelText(CREATE_RULE)); + await flushMicrotasks(); + + // When the time limit has not passed yet + act(() => { + jest.advanceTimersByTime(CONST.GENERATED_RULE.AGENT_RULE_CHECK_TIMEOUT_MS - 1); + }); + + // Then the rule still waits for the answer + expect(onSave).not.toHaveBeenCalled(); + + // When the time limit passes + act(() => { + jest.advanceTimersByTime(1); + }); + + // Then the rule saves, because a slow check must not block the admin + expect(onSave).toHaveBeenCalledWith({prompt: TYPED_PROMPT}); + }); + + it('stops the loading state when the admin leaves during a check', async () => { + // Given an admin who typed an agent rule, and the check has not answered + const onSave = jest.fn(); + render( + , + ); + await waitForBatchedUpdatesWithAct(); + mockPrompt = TYPED_PROMPT; + fireEvent.press(screen.getByLabelText(CREATE_RULE)); + await act(async () => { + await Promise.resolve(); + }); + + // When the admin leaves the page + screen.unmount(); + await waitForBatchedUpdatesWithAct(); + + // Then the form is not loading, because the next visit must not open with a loading button + expect((await getOnyxValue(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM))?.isLoading).toBe(false); + expect(onSave).not.toHaveBeenCalled(); + }); + + describe('EditAgentRulePage', () => { + const SAVED_PROMPT = 'Hold any expense over $75 without a receipt'; + + async function renderEditPage() { + jest.mocked(usePolicy).mockReturnValue({ + ...createRandomPolicy(0, CONST.POLICY.TYPE.CORPORATE), + id: POLICY_ID, + rules: {agentRules: {[RULE_ID]: {ruleID: RULE_ID, prompt: SAVED_PROMPT, title: 'Receipts', created: '2026-10-01 00:00:00'}}}, + }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- test-only screen props. The page reads only route.params. + const props = { + route: {key: 'Rules_Agent_Edit', name: SCREENS.WORKSPACE.RULES_AGENT_EDIT, params: {policyID: POLICY_ID, ruleID: RULE_ID}}, + navigation: {}, + } as unknown as EditAgentRulePageProps; + render(); + await waitForBatchedUpdatesWithAct(); + } + + it('saves an unchanged rule without a check', async () => { + // Given an admin who opened a saved agent rule + await renderEditPage(); + + // When the admin saves it without changing the text + await submit(SAVED_PROMPT, 'common.save'); + + // Then the page closes without a check or an update, because nothing changed + expect(getGenerateRuleRequests(writeSpy)).toHaveLength(0); + expect(updatePolicyAgentRule).not.toHaveBeenCalled(); + expect(Navigation.goBack).toHaveBeenCalled(); + }); + + it('warns about an impossible edit and saves it when the admin taps Save anyway', async () => { + // Given an admin who changed a saved agent rule to one that RuleBot cannot carry out + await renderEditPage(); + await submit(TYPED_PROMPT, 'common.save'); + + // When Concierge answers that the rule is impossible + await answerCheck(buildImpossibleAnswer(getLastGenerationID(writeSpy))); + + // Then the rule does not update until the admin taps Save anyway + expect(updatePolicyAgentRule).not.toHaveBeenCalled(); + await submit(TYPED_PROMPT, 'workspace.rules.agentRules.saveAnyway'); + expect(updatePolicyAgentRule).toHaveBeenCalledWith(POLICY_ID, RULE_ID, TYPED_PROMPT, SAVED_PROMPT, 'Receipts'); + }); + }); +}); From eada7520e1e2e77ed97ff5c0aeb2e508053d948e Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Sun, 25 Oct 2026 19:35:03 -0700 Subject: [PATCH 2/5] refactor(rules): run the agent rule check without the describeRule beta --- .../rules/AgentRules/useAgentRulePromptCheck.ts | 4 +--- tests/ui/AgentRulePromptCheckTest.tsx | 15 ++++----------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts index 582598e3763b..67bee6cc2bcf 100644 --- a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts +++ b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts @@ -4,7 +4,6 @@ */ import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; -import usePermissions from '@hooks/usePermissions'; import Log from '@libs/Log'; @@ -36,7 +35,6 @@ type PendingCheck = { function useAgentRulePromptCheck({policyID, formID, checkedPrompt, savePrompt}: UseAgentRulePromptCheckParams) { const {isOffline} = useNetwork(); - const {isBetaEnabled} = usePermissions(); const [generatedRule] = useOnyx(ONYXKEYS.GENERATED_RULE); const pendingCheckRef = useRef(undefined); const timeoutRef = useRef>(undefined); @@ -103,7 +101,7 @@ function useAgentRulePromptCheck({policyID, formID, checkedPrompt, savePrompt}: return; } const isUnchanged = !!checkedPrompt && prompt.trim() === checkedPrompt.trim(); - if (isUnchanged || isOffline || !isBetaEnabled(CONST.BETAS.DESCRIBE_RULE)) { + if (isUnchanged || isOffline) { savePrompt(prompt); return; } diff --git a/tests/ui/AgentRulePromptCheckTest.tsx b/tests/ui/AgentRulePromptCheckTest.tsx index 7060c9ffa251..b66d17d061a7 100644 --- a/tests/ui/AgentRulePromptCheckTest.tsx +++ b/tests/ui/AgentRulePromptCheckTest.tsx @@ -27,7 +27,6 @@ import getOnyxValue from '../utils/getOnyxValue'; import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; let mockPrompt = ''; -let mockIsBetaEnabled = true; let mockIsOffline = false; jest.mock('@hooks/useLocalize', () => @@ -46,7 +45,6 @@ jest.mock('@hooks/useThemeStyles', () => ), ), ); -jest.mock('@hooks/usePermissions', () => jest.fn(() => ({isBetaEnabled: () => mockIsBetaEnabled}))); jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: mockIsOffline}))); jest.mock('@hooks/usePolicy', () => jest.fn()); jest.mock('@pages/workspace/rules/AgentRules/useAgentPromptInputStyles', () => jest.fn(() => ({}))); @@ -181,7 +179,6 @@ describe('Agent rule prompt check', () => { beforeEach(() => { jest.clearAllMocks(); mockPrompt = ''; - mockIsBetaEnabled = true; mockIsOffline = false; writeSpy = jest.spyOn(API, 'write').mockImplementation(() => Promise.resolve()); }); @@ -266,20 +263,16 @@ describe('Agent rule prompt check', () => { expect(getGenerateRuleRequests(writeSpy)).toHaveLength(0); }); - it.each([ - {description: 'the describe rule beta is off', isBetaEnabled: false, isOffline: false}, - {description: 'the admin is offline', isBetaEnabled: true, isOffline: true}, - ])('saves a typed rule without a check when $description', async ({isBetaEnabled, isOffline}: {isBetaEnabled: boolean; isOffline: boolean}) => { - // Given an admin who typed an agent rule - mockIsBetaEnabled = isBetaEnabled; - mockIsOffline = isOffline; + it('saves a typed rule without a check when the admin is offline', async () => { + // Given an admin who typed an agent rule while offline + mockIsOffline = true; const onSave = jest.fn(); await renderWriteTab(onSave); // When the admin creates the rule await submit(TYPED_PROMPT); - // Then the rule saves at once, because the check cannot run or is not on for this account + // Then the rule saves at once, because the check cannot run offline expect(onSave).toHaveBeenCalledWith({prompt: TYPED_PROMPT}); expect(getGenerateRuleRequests(writeSpy)).toHaveLength(0); }); From ad0ad017b38dbd1b2b51926b34c0d01e1b5c4ad2 Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Tue, 27 Oct 2026 23:47:42 -0700 Subject: [PATCH 3/5] fix(rules): stop an agent rule check when the admin edits the text --- .../rules/AgentRules/AddAgentRuleWriteTab.tsx | 12 ++----- .../rules/AgentRules/EditAgentRulePage.tsx | 12 ++----- .../AgentRules/useAgentRulePromptCheck.ts | 35 +++++++++++++++---- tests/ui/AgentRulePromptCheckTest.tsx | 25 ++++++++++++- 4 files changed, 59 insertions(+), 25 deletions(-) diff --git a/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx b/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx index c65d22220e2c..fabe0a2e2c50 100644 --- a/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx +++ b/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx @@ -8,7 +8,6 @@ import Text from '@components/Text'; import TextInput from '@components/TextInput'; import useLocalize from '@hooks/useLocalize'; -import useOnyx from '@hooks/useOnyx'; import useThemeStyles from '@hooks/useThemeStyles'; import ImpossibleRuleNotice from '@pages/workspace/rules/ImpossibleRuleNotice'; @@ -41,10 +40,10 @@ function AddAgentRuleWriteTab({policyID, checkedPrompt, onSave}: AddAgentRuleWri const {translate} = useLocalize(); const styles = useThemeStyles(); const formRef = useRef(null); - const [isChecking = false] = useOnyx(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM, {selector: (form) => !!form?.isLoading}); - const {refusal, checkThenSavePrompt, checkSuggestedPrompt, dismissRefusal} = useAgentRulePromptCheck({ + const {refusal, isChecking, checkThenSavePrompt, trySuggestedPrompt, resetCheck} = useAgentRulePromptCheck({ policyID, formID: ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM, + formRef, checkedPrompt, savePrompt: (prompt) => onSave({[INPUT_IDS.PROMPT]: prompt}), }); @@ -71,11 +70,6 @@ function AddAgentRuleWriteTab({policyID, checkedPrompt, onSave}: AddAgentRuleWri const inputWrapperStyles = useAgentPromptInputStyles(); - const trySuggestedPrompt = (suggestedPrompt: string) => { - formRef.current?.resetForm({[INPUT_IDS.PROMPT]: suggestedPrompt}); - checkSuggestedPrompt(suggestedPrompt); - }; - return ( !!form?.isLoading}); - const {refusal, checkThenSavePrompt, checkSuggestedPrompt, dismissRefusal} = useAgentRulePromptCheck({ + const {refusal, isChecking, checkThenSavePrompt, trySuggestedPrompt, resetCheck} = useAgentRulePromptCheck({ policyID, formID: ONYXKEYS.FORMS.EDIT_AGENT_RULE_FORM, + formRef, checkedPrompt: agentRule?.prompt, savePrompt: saveRule, }); - const trySuggestedPrompt = (suggestedPrompt: string) => { - formRef.current?.resetForm({[INPUT_IDS.PROMPT]: suggestedPrompt}); - checkSuggestedPrompt(suggestedPrompt); - }; - const inputWrapperStyles = useAgentPromptInputStyles(); const {deleteIconButtonProps} = useRuleDeleteHeaderProps({ @@ -157,7 +151,7 @@ function EditAgentRulePage({ type="markdown" excludedMarkdownStyles={['mentionReport']} onKeyPress={submitFormOnModEnter} - onValueChange={dismissRefusal} + onValueChange={resetCheck} defaultValue={agentRule.prompt} multiline shouldLabelStayOnSingleLine diff --git a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts index 67bee6cc2bcf..766c65d778a2 100644 --- a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts +++ b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts @@ -2,6 +2,8 @@ * Checks with Concierge whether RuleBot can carry out an agent rule that the admin wrote, before the rule saves. An * `impossible` answer stops the save and returns the refusal. A save after the refusal, or any other answer, saves the rule. */ +import type {FormRef} from '@components/Form/types'; + import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; @@ -12,15 +14,24 @@ import type {AgentRuleFormID} from '@userActions/Policy/Rules'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import ADD_AGENT_RULE_INPUT_IDS from '@src/types/form/AddAgentRuleForm'; +import type EDIT_AGENT_RULE_INPUT_IDS from '@src/types/form/EditAgentRuleForm'; + +import type {RefObject} from 'react'; import {useEffect, useRef, useState} from 'react'; +const PROMPT_INPUT_ID = ADD_AGENT_RULE_INPUT_IDS.PROMPT satisfies typeof EDIT_AGENT_RULE_INPUT_IDS.PROMPT; + type UseAgentRulePromptCheckParams = { policyID: string; /** The form that shows the loading state during the check */ formID: AgentRuleFormID; + /** The form that shows the suggested rule when the admin tries it */ + formRef: RefObject; + /** A rule with this text saves without a check, because Concierge already wrote it or the admin saved it before */ checkedPrompt?: string; @@ -33,9 +44,10 @@ type PendingCheck = { prompt: string; }; -function useAgentRulePromptCheck({policyID, formID, checkedPrompt, savePrompt}: UseAgentRulePromptCheckParams) { +function useAgentRulePromptCheck({policyID, formID, formRef, checkedPrompt, savePrompt}: UseAgentRulePromptCheckParams) { const {isOffline} = useNetwork(); const [generatedRule] = useOnyx(ONYXKEYS.GENERATED_RULE); + const [isChecking = false] = useOnyx(formID, {selector: (form) => !!form?.isLoading}); const pendingCheckRef = useRef(undefined); const timeoutRef = useRef>(undefined); const [checkGenerationID, setCheckGenerationID] = useState(); @@ -108,16 +120,27 @@ function useAgentRulePromptCheck({policyID, formID, checkedPrompt, savePrompt}: startCheck(prompt); }; - /** Checks the suggested rule of the refusal, which the form now shows in place of the admin's text */ - const checkSuggestedPrompt = (suggestedPrompt: string) => { + /** Shows the suggested rule of the refusal in place of the admin's text, and checks it */ + const trySuggestedPrompt = (suggestedPrompt: string) => { Log.info('[GenerateRule] Tried the suggested description', false, {policyID, generationID: refusal?.generationID, reason: refusal?.reason}); + formRef.current?.resetForm({[PROMPT_INPUT_ID]: suggestedPrompt}); startCheck(suggestedPrompt); }; - /** Hides the refusal when the admin changes the text, so that the next save checks the new text */ - const dismissRefusal = () => setCheckGenerationID(undefined); + /** + * Stops a running check and hides the refusal when the admin changes the text. A running check must not save the text + * from before the change, so the admin saves again to check the new text. + */ + const resetCheck = () => { + setCheckGenerationID(undefined); + if (!pendingCheckRef.current) { + return; + } + Log.info('[GenerateRule] Stopped an agent rule check after the text changed', false, {policyID, generationID: pendingCheckRef.current.generationID}); + finishCheck(); + }; - return {refusal, checkThenSavePrompt, checkSuggestedPrompt, dismissRefusal}; + return {refusal, isChecking, checkThenSavePrompt, trySuggestedPrompt, resetCheck}; } export default useAgentRulePromptCheck; diff --git a/tests/ui/AgentRulePromptCheckTest.tsx b/tests/ui/AgentRulePromptCheckTest.tsx index b66d17d061a7..89d5f5024e0d 100644 --- a/tests/ui/AgentRulePromptCheckTest.tsx +++ b/tests/ui/AgentRulePromptCheckTest.tsx @@ -27,6 +27,7 @@ import getOnyxValue from '../utils/getOnyxValue'; import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; let mockPrompt = ''; +let mockOnPromptChange: ((value: string) => void) | undefined; let mockIsOffline = false; jest.mock('@hooks/useLocalize', () => @@ -89,7 +90,10 @@ jest.mock('@components/Button', () => { MockButton.Text = ({children}: {children: React.ReactNode}) => ReactModule.createElement(Text, null, children); return MockButton; }); -jest.mock('@components/Form/InputWrapper', () => () => null); +jest.mock('@components/Form/InputWrapper', () => ({onValueChange}: {onValueChange?: (value: string) => void}) => { + mockOnPromptChange = onValueChange; + return null; +}); jest.mock('@components/Form/FormProvider', () => { const ReactModule = jest.requireActual('react'); const {Pressable, Text} = jest.requireActual<{ @@ -277,6 +281,25 @@ describe('Agent rule prompt check', () => { expect(getGenerateRuleRequests(writeSpy)).toHaveLength(0); }); + it('stops the check and keeps the page open when the admin edits the text during the check', async () => { + // Given an admin who typed an agent rule, and the check has not answered + const onSave = jest.fn(); + await renderWriteTab(onSave); + await submit(TYPED_PROMPT); + const generationID = getLastGenerationID(writeSpy); + + // When the admin changes the text, and the answer for the old text arrives after that + act(() => { + mockOnPromptChange?.(`${TYPED_PROMPT} and the submitter`); + }); + await waitForBatchedUpdatesWithAct(); + await answerCheck({generationID, state: CONST.GENERATED_RULE.STATE.AGENT_RULE}); + + // Then nothing saves and the form stops loading, because a save now would lose the admin's edit + expect(onSave).not.toHaveBeenCalled(); + expect((await getOnyxValue(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM))?.isLoading).toBe(false); + }); + it('saves the rule when the check fails', async () => { // Given an admin who typed an agent rule const onSave = jest.fn(); From 920828c932702b0ef5848af788c66a88d183a33b Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Fri, 30 Oct 2026 13:53:42 -0700 Subject: [PATCH 4/5] fix(rules): keep the agent rule check across Write tab switches --- .../rules/AgentRules/AddAgentRulePage.tsx | 47 +++++++++------- .../AgentRules/useAgentRulePromptCheck.ts | 2 + tests/ui/AddAgentRulePageTest.tsx | 56 ++++++++++++++++--- tests/ui/AgentRulePromptCheckTest.tsx | 22 +++++++- 4 files changed, 95 insertions(+), 32 deletions(-) diff --git a/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx b/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx index bfac875fe3cc..b050061f92a6 100644 --- a/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx +++ b/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx @@ -79,6 +79,10 @@ function AddAgentRulePage({ const hasDraftPrompt = !!draftValues?.[INPUT_IDS.PROMPT]?.trim(); const [selectedSuggestion, setSelectedSuggestion] = useState(); + + // A new suggestion replaces the text of the Write tab. Mount a new Write tab, so that a check of the old text stops + // and cannot save it. + const [suggestionPickCount, setSuggestionPickCount] = useState(0); const isDraftFromFallback = !!generationID && !selectedSuggestion; const fallbackExplanation = generatedRule?.generationID === generationID ? generatedRule?.explanation : undefined; @@ -139,6 +143,7 @@ function AddAgentRulePage({ } setDraftValues(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM, {[INPUT_IDS.PROMPT]: suggestion.prompt}); setSelectedSuggestion(suggestion); + setSuggestionPickCount((count) => count + 1); setActiveTab(CONST.TAB.AGENT_RULE.WRITE); }; @@ -261,28 +266,28 @@ function AddAgentRulePage({ onSelectSuggestion={selectSuggestion} /> - {activeTab === CONST.TAB.AGENT_RULE.WRITE && ( - <> - {isDraftFromFallback && ( - - )} - + {isDraftFromFallback && ( + - - )} + )} + + diff --git a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts index 766c65d778a2..8b91ffce0ede 100644 --- a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts +++ b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts @@ -124,6 +124,8 @@ function useAgentRulePromptCheck({policyID, formID, formRef, checkedPrompt, save const trySuggestedPrompt = (suggestedPrompt: string) => { Log.info('[GenerateRule] Tried the suggested description', false, {policyID, generationID: refusal?.generationID, reason: refusal?.reason}); formRef.current?.resetForm({[PROMPT_INPUT_ID]: suggestedPrompt}); + + // Start the check after the reset, so that a text change from the reset cannot stop it startCheck(suggestedPrompt); }; diff --git a/tests/ui/AddAgentRulePageTest.tsx b/tests/ui/AddAgentRulePageTest.tsx index 74dea134be36..0c92385fd174 100644 --- a/tests/ui/AddAgentRulePageTest.tsx +++ b/tests/ui/AddAgentRulePageTest.tsx @@ -33,13 +33,14 @@ jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false}))); jest.mock('@hooks/usePolicy', () => jest.fn()); jest.mock('@hooks/useConfirmModal', () => jest.fn(() => ({showConfirmModal: jest.fn(() => Promise.resolve()), closeModal: jest.fn()}))); jest.mock('@hooks/useDiscardChangesConfirmation', () => jest.fn(() => ({suppressDiscardPrompt: jest.fn()}))); +// `dNone` hides the inactive tab, and a query does not find an element that the admin cannot see jest.mock('@hooks/useThemeStyles', () => jest.fn( () => new Proxy( {}, { - get: () => ({}), + get: (_target, styleName) => (styleName === 'dNone' ? {display: 'none'} : {}), }, ), ), @@ -104,6 +105,7 @@ const mockSuggestionPrompt = 'Hold any expense over $75 without a receipt'; // The prompt that the mocked Write tab saves. A test sets it to the suggestion's prompt to save the suggestion unchanged. let mockSavedPrompt = 'Hold gift card expenses'; +let mockWriteTabMountCount = 0; jest.mock('@pages/workspace/rules/AgentRules/AddAgentRuleSuggestionsTab', () => { const ReactModule = jest.requireActual('react'); @@ -120,13 +122,17 @@ jest.mock('@pages/workspace/rules/AgentRules/AddAgentRuleWriteTab', () => { Pressable: React.ComponentType<{accessibilityLabel?: string; onPress?: () => void; children?: React.ReactNode}>; Text: React.ComponentType<{children?: React.ReactNode}>; }>('react-native'); - return ({onSave, checkedPrompt}: {onSave: (values: {prompt: string}) => void; checkedPrompt?: string}) => - ReactModule.createElement( + return ({onSave, checkedPrompt}: {onSave: (values: {prompt: string}) => void; checkedPrompt?: string}) => { + ReactModule.useEffect(() => { + mockWriteTabMountCount += 1; + }, []); + return ReactModule.createElement( Pressable, {accessibilityLabel: 'WriteTab', onPress: () => onSave({prompt: mockSavedPrompt})}, ReactModule.createElement(Text, null, 'WriteTab'), ReactModule.createElement(Text, null, `CheckedPrompt: ${checkedPrompt ?? 'none'}`), ); + }; }); type AddAgentRulePageProps = PlatformStackScreenProps; @@ -164,6 +170,7 @@ describe('AddAgentRulePage', () => { beforeEach(() => { jest.clearAllMocks(); mockSavedPrompt = PROMPT; + mockWriteTabMountCount = 0; mockedUsePolicy.mockReturnValue({ ...createRandomPolicy(0, CONST.POLICY.TYPE.CORPORATE), id: POLICY_ID, @@ -197,7 +204,7 @@ describe('AddAgentRulePage', () => { await waitForBatchedUpdatesWithAct(); // Then the admin sees the drafted prompt and does not have to look for it - expect(screen.getByText('WriteTab')).toBeOnTheScreen(); + expect(screen.getByText('WriteTab')).toBeVisible(); }); it('opens on the Write tab when the draft prompt arrives after the first render', async () => { @@ -211,7 +218,7 @@ describe('AddAgentRulePage', () => { await waitForBatchedUpdatesWithAct(); // Then the page still switches to Write, because the first render must not decide the tab for good - expect(screen.getByText('WriteTab')).toBeOnTheScreen(); + expect(screen.getByText('WriteTab')).toBeVisible(); }); it('lets the draft of the describe flow save without another check', async () => { @@ -298,7 +305,7 @@ describe('AddAgentRulePage', () => { await waitForBatchedUpdatesWithAct(); // Then the Write tab opens without the banner, because no description was turned into this rule - expect(screen.getByText('WriteTab')).toBeOnTheScreen(); + expect(screen.getByText('WriteTab')).toBeVisible(); expect(screen.queryByText(FALLBACK_BANNER)).toBeNull(); }); @@ -313,7 +320,7 @@ describe('AddAgentRulePage', () => { // Then the page closes and the admin returns to the previous screen, because they did not come from the Suggestions tab expect(Navigation.goBack).toHaveBeenCalledTimes(1); - expect(screen.getByText('WriteTab')).toBeOnTheScreen(); + expect(screen.getByText('WriteTab')).toBeVisible(); }); it('switches from the Write tab to Suggestions on Back without a generationID', async () => { @@ -330,6 +337,37 @@ describe('AddAgentRulePage', () => { expect(screen.queryByText('WriteTab')).toBeNull(); }); + it('keeps the Write tab mounted while the admin looks at Suggestions', async () => { + // Given the Write tab is open for a typed draft + await setAgentRuleDraft(PROMPT); + renderAddAgentRulePage(); + await waitForBatchedUpdatesWithAct(); + + // When the admin goes to Suggestions and comes back without picking one + fireEvent.press(screen.getByLabelText('workspace.rules.agentRules.suggestionsTab')); + fireEvent.press(screen.getByLabelText('workspace.rules.agentRules.writeTab')); + await waitForBatchedUpdatesWithAct(); + + // Then the same Write tab shows, because a remount would stop a running check and drop its warning and a tried suggestion + expect(screen.getByText('WriteTab')).toBeVisible(); + expect(mockWriteTabMountCount).toBe(1); + }); + + it('mounts a new Write tab when the admin picks a suggestion', async () => { + // Given the Write tab is open for a typed draft + await setAgentRuleDraft(PROMPT); + renderAddAgentRulePage(); + await waitForBatchedUpdatesWithAct(); + + // When the admin picks a suggestion + fireEvent.press(screen.getByLabelText('workspace.rules.agentRules.suggestionsTab')); + fireEvent.press(screen.getByLabelText('PickSuggestion')); + await waitForBatchedUpdatesWithAct(); + + // Then a new Write tab shows the suggestion, because a check of the typed text must not save it in place of the suggestion + expect(mockWriteTabMountCount).toBe(2); + }); + it('drops the fallback banner and log after the admin picks a suggestion', async () => { // Given the describe flow opened the page with a generationID and the fallback banner const logSpy = jest.spyOn(Log, 'info'); @@ -342,7 +380,7 @@ describe('AddAgentRulePage', () => { fireEvent.press(screen.getByLabelText('workspace.rules.agentRules.suggestionsTab')); fireEvent.press(screen.getByLabelText('PickSuggestion')); await waitForBatchedUpdatesWithAct(); - expect(screen.getByText('WriteTab')).toBeOnTheScreen(); + expect(screen.getByText('WriteTab')).toBeVisible(); expect(screen.queryByText(FALLBACK_BANNER)).toBeNull(); fireEvent.press(screen.getByLabelText('WriteTab')); @@ -360,7 +398,7 @@ describe('AddAgentRulePage', () => { fireEvent.press(screen.getByLabelText('workspace.rules.agentRules.suggestionsTab')); fireEvent.press(screen.getByLabelText('PickSuggestion')); await waitForBatchedUpdatesWithAct(); - expect(screen.getByText('WriteTab')).toBeOnTheScreen(); + expect(screen.getByText('WriteTab')).toBeVisible(); // When the admin presses Back fireEvent.press(screen.getByLabelText('Back')); diff --git a/tests/ui/AgentRulePromptCheckTest.tsx b/tests/ui/AgentRulePromptCheckTest.tsx index 89d5f5024e0d..55a979985745 100644 --- a/tests/ui/AgentRulePromptCheckTest.tsx +++ b/tests/ui/AgentRulePromptCheckTest.tsx @@ -100,8 +100,25 @@ jest.mock('@components/Form/FormProvider', () => { Pressable: React.ComponentType<{accessibilityLabel?: string; onPress?: () => void; children?: React.ReactNode}>; Text: React.ComponentType<{children?: React.ReactNode}>; }>('react-native'); - return ({children, onSubmit, submitButtonText}: {children: React.ReactNode; onSubmit: (values: {prompt: string}) => void; submitButtonText: string}) => - ReactModule.createElement( + // `resetForm` also reports a text change, so that a test fails when a reset can stop the check that follows it + return ({ + children, + onSubmit, + submitButtonText, + ref, + }: { + children: React.ReactNode; + onSubmit: (values: {prompt: string}) => void; + submitButtonText: string; + ref?: React.Ref<{resetForm: (values: {prompt: string}) => void}>; + }) => { + ReactModule.useImperativeHandle(ref, () => ({ + resetForm: (values: {prompt: string}) => { + mockPrompt = values.prompt; + mockOnPromptChange?.(values.prompt); + }, + })); + return ReactModule.createElement( ReactModule.Fragment, null, children, @@ -111,6 +128,7 @@ jest.mock('@components/Form/FormProvider', () => { ReactModule.createElement(Text, null, submitButtonText), ), ); + }; }); const POLICY_ID = 'policy1'; From 4cf95580e5d910def8dad7c4a9ebb7839ba70912 Mon Sep 17 00:00:00 2001 From: Yuwen Memon Date: Mon, 2 Nov 2026 12:57:32 -0800 Subject: [PATCH 5/5] fix(rules): clear a stale check after reload and block Enter on hidden tab --- .../rules/AgentRules/AddAgentRulePage.tsx | 1 + .../rules/AgentRules/AddAgentRuleWriteTab.tsx | 6 +++++- .../AgentRules/useAgentRulePromptCheck.ts | 13 +++++++------ tests/ui/AddAgentRulePageTest.tsx | 18 +++++++++++++++++- tests/ui/AgentRulePromptCheckTest.tsx | 17 +++++++++++++++++ 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx b/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx index b050061f92a6..8f23d475f0b4 100644 --- a/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx +++ b/src/pages/workspace/rules/AgentRules/AddAgentRulePage.tsx @@ -285,6 +285,7 @@ function AddAgentRulePage({ key={suggestionPickCount} policyID={policyID} checkedPrompt={checkedPrompt} + isTabActive={activeTab === CONST.TAB.AGENT_RULE.WRITE} onSave={saveRule} /> diff --git a/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx b/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx index 08b237eef623..bd40d08055db 100644 --- a/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx +++ b/src/pages/workspace/rules/AgentRules/AddAgentRuleWriteTab.tsx @@ -33,11 +33,14 @@ type AddAgentRuleWriteTabProps = { /** A rule with this text saves without a check, because Concierge already wrote it */ checkedPrompt?: string; + /** The tab stays mounted while hidden, and Enter must not submit a hidden form */ + isTabActive: boolean; + /** Called with the form values when the user saves the rule */ onSave: (values: FormOnyxValues) => void; }; -function AddAgentRuleWriteTab({policyID, checkedPrompt, onSave}: AddAgentRuleWriteTabProps) { +function AddAgentRuleWriteTab({policyID, checkedPrompt, isTabActive, onSave}: AddAgentRuleWriteTabProps) { const {translate} = useLocalize(); const styles = useThemeStyles(); const formRef = useRef(null); @@ -83,6 +86,7 @@ function AddAgentRuleWriteTab({policyID, checkedPrompt, onSave}: AddAgentRuleWri submitFlexEnabled={false} shouldUseScrollView={false} enabledWhenOffline + disablePressOnEnter={!isTabActive} shouldHideFixErrorsAlert isSubmitButtonNeutral={suggestionButtonRow.isSubmitButtonNeutral} submitButtonAndFooterContainerStyles={suggestionButtonRow.containerStyles} diff --git a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts index e5222030888a..9b4e3b3c1be3 100644 --- a/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts +++ b/src/pages/workspace/rules/AgentRules/useAgentRulePromptCheck.ts @@ -75,17 +75,18 @@ function useAgentRulePromptCheck({policyID, formID, formRef, checkedPrompt, save savePrompt(pendingCheck.prompt); }, [finishCheck, generatedRuleID, generatedRuleReason, generatedRuleState, policyID, savePrompt]); - // The loading state stays in Onyx. Stop it when the admin leaves during a check, so that the form does not open loading. - useEffect( - () => () => { + // The loading state stays in Onyx, and a reload during a check does not run the cleanup. No check runs when the form + // mounts, therefore stop the loading state then too. Also stop it when the admin leaves during a check. + useEffect(() => { + stopAgentRuleCheckLoading(formID); + return () => { clearTimeout(timeoutRef.current); if (!pendingCheckRef.current) { return; } stopAgentRuleCheckLoading(formID); - }, - [formID], - ); + }; + }, [formID]); const startCheck = (prompt: string) => { const generationID = checkAgentRulePrompt(formID, policyID, prompt); diff --git a/tests/ui/AddAgentRulePageTest.tsx b/tests/ui/AddAgentRulePageTest.tsx index 0c92385fd174..50f9a73a5645 100644 --- a/tests/ui/AddAgentRulePageTest.tsx +++ b/tests/ui/AddAgentRulePageTest.tsx @@ -122,7 +122,7 @@ jest.mock('@pages/workspace/rules/AgentRules/AddAgentRuleWriteTab', () => { Pressable: React.ComponentType<{accessibilityLabel?: string; onPress?: () => void; children?: React.ReactNode}>; Text: React.ComponentType<{children?: React.ReactNode}>; }>('react-native'); - return ({onSave, checkedPrompt}: {onSave: (values: {prompt: string}) => void; checkedPrompt?: string}) => { + return ({onSave, checkedPrompt, isTabActive}: {onSave: (values: {prompt: string}) => void; checkedPrompt?: string; isTabActive: boolean}) => { ReactModule.useEffect(() => { mockWriteTabMountCount += 1; }, []); @@ -131,6 +131,7 @@ jest.mock('@pages/workspace/rules/AgentRules/AddAgentRuleWriteTab', () => { {accessibilityLabel: 'WriteTab', onPress: () => onSave({prompt: mockSavedPrompt})}, ReactModule.createElement(Text, null, 'WriteTab'), ReactModule.createElement(Text, null, `CheckedPrompt: ${checkedPrompt ?? 'none'}`), + ReactModule.createElement(Text, null, `IsTabActive: ${String(isTabActive)}`), ); }; }); @@ -353,6 +354,21 @@ describe('AddAgentRulePage', () => { expect(mockWriteTabMountCount).toBe(1); }); + it('stops Enter from submitting the Write tab while the admin looks at Suggestions', async () => { + // Given the Write tab is open for a typed draft + await setAgentRuleDraft(PROMPT); + renderAddAgentRulePage(); + await waitForBatchedUpdatesWithAct(); + expect(screen.getByText('IsTabActive: true')).toBeTruthy(); + + // When the admin goes to Suggestions + fireEvent.press(screen.getByLabelText('workspace.rules.agentRules.suggestionsTab')); + await waitForBatchedUpdatesWithAct(); + + // Then the hidden Write tab is inactive, because Enter must not save a rule that the admin cannot see + expect(screen.getByText('IsTabActive: false', {includeHiddenElements: true})).toBeTruthy(); + }); + it('mounts a new Write tab when the admin picks a suggestion', async () => { // Given the Write tab is open for a typed draft await setAgentRuleDraft(PROMPT); diff --git a/tests/ui/AgentRulePromptCheckTest.tsx b/tests/ui/AgentRulePromptCheckTest.tsx index 39ed8480a6ad..bba66003c71c 100644 --- a/tests/ui/AgentRulePromptCheckTest.tsx +++ b/tests/ui/AgentRulePromptCheckTest.tsx @@ -170,6 +170,7 @@ async function renderWriteTab(onSave: jest.Mock, checkedPrompt?: string) { , ); @@ -347,6 +348,7 @@ describe('Agent rule prompt check', () => { render( , ); @@ -378,6 +380,7 @@ describe('Agent rule prompt check', () => { render( , ); @@ -397,6 +400,20 @@ describe('Agent rule prompt check', () => { expect(onSave).not.toHaveBeenCalled(); }); + it('stops a loading state that a reload left during a check', async () => { + // Given a reload during a check, which leaves the loading state in Onyx without the cleanup + await act(async () => { + await Onyx.merge(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM, {isLoading: true}); + }); + + // When the form mounts again + await renderWriteTab(jest.fn()); + + // Then the form is not loading, because no check runs after the reload + expect((await getOnyxValue(ONYXKEYS.FORMS.ADD_AGENT_RULE_FORM))?.isLoading).toBe(false); + expect(screen.getByText('workspace.rules.agentRules.disclaimer')).toBeTruthy(); + }); + describe('EditAgentRulePage', () => { const SAVED_PROMPT = 'Hold any expense over $75 without a receipt';