Signal sign-out confirmation with the prompt action type - #4480
Conversation
Replace the RuntimeKeyLogoutPromptShown runtime-data guard on the session sign-out executor with the confirmation prompt's action type. When the End-User confirms, the prompt forwards its SIGN_OUT_CONFIRM action type to the executor on the re-run, so a confirmed request is told apart from the initial one without persisting a marker. Resolve that into an outcome by dispatching on the forwarded action type rather than comparing against the confirm type alone, so the confirmation prompt can grow further actions: each action type the prompt raises maps to one signOutOutcome, and an unrecognized type is treated as no consent and prompts again rather than falling through to termination. Mark the confirmation button of the Conditional Sign Out Flow template with that type, both on the prompt action the engine reads and on the element itself, so the flow builder's Action selector shows Sign out for it and the type survives editing the flow. Describe prompts[].action.type as an opaque string owned by the consuming executor instead of enumerating its values in the flow management and flow execution API specs and in the flow configuration guide. The field already accepted arbitrary strings, so the enumerations were never the contract, and they would need editing in three places for every action type added. Refs thunder-id#4299
📝 WalkthroughWalkthroughThis PR changes sign-out confirmation to use a forwarded action type instead of a persisted runtime marker. It removes ChangesSign-out confirmation action type
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Node as Sign-out Prompt Node
participant Executor as SessionSignOutExecutor
participant Session as SessionService
Node->>Executor: forward action data
Executor->>Executor: decide(ctx)
alt actionType == SIGN_OUT_CONFIRM
Executor->>Session: Terminate()
else action missing or unrecognized
Executor->>Node: prompt for confirmation
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
docs/content/guides/flows/advanced-configurations.mdxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/internal/flow/executor/session_signout_executor_test.go`:
- Around line 114-115: Update the test comment before the
session_signout_executor_test.go test case to correctly describe the
id_token_hint condition. Change "valid id_token_hint" to "no valid
id_token_hint" to accurately reflect the test setup and the executor behavior
defined in session_signout_executor.go where this path applies when no valid
id_token_hint is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fd3b697-3d88-4d46-ac10-8f5eb84a9ad7
📒 Files selected for processing (7)
api/flow-execution.yamlapi/flow-management.yamlbackend/internal/flow/common/constants.gobackend/internal/flow/executor/session_signout_executor.gobackend/internal/flow/executor/session_signout_executor_test.godocs/content/guides/flows/advanced-configurations.mdxfrontend/apps/console/src/features/flows/data/templates.json
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| // been shown in this flow run. | ||
| func (e *sessionSignOutExecutor) confirmationRequired(ctx *providers.NodeContext) bool { | ||
| // signOutOutcome is what the executor does with the current request. | ||
| type signOutOutcome int |
There was a problem hiding this comment.
Can there be a case where there are more than 2 signout outcomes? If not we could simply make decide() function to return a boolean instead of defining s dedicated type.
WDYT?
There was a problem hiding this comment.
There are 3 outcomes
- Prompt for confirmation
- Terminate the session (Sign out button in the prompt)
- Cancel the termination request (future planned cancel button in the prompt)
Hence keeping the current implementation.
Purpose
The session sign-out executor told a confirmed sign-out apart from the initial request by persisting a
logoutPromptShownmarker in RuntimeData. This addresses @ThaminduDilshan's review comment on #4299: the confirmation prompt already knows which button the End-User pressed, so it can forward that as the prompt action's type and the executor can read it straight off the flow definition. No marker has to be stored.This also restructures the executor so the confirmation prompt can grow further actions (a sign-out cancel button is planned), and stops the API documentation from enumerating action-type values.
Approach
Read the decision from the action type.
common.ActionTypeSignOutConfirm(SIGN_OUT_CONFIRM) marks the confirmation prompt's action edge. The prompt node already forwards a matched action's type onForwardedData[actionType](prompt_node.go:154), so on the re-run after the End-User confirms, the executor sees it and terminates.RuntimeKeyLogoutPromptShownis gone.Dispatch on the type rather than compare against one value. The previous guard was
actionType != SIGN_OUT_CONFIRM, which collapsed three distinct cases (no type, the confirm type, some other type) into one boolean and would have sent any future action type down the prompt-again path.decidenow returns a namedsignOutOutcomefrom a switch keyed on the action type:Adding the planned cancel action is then one constant, one
case, onesignOutOutcome, and its handling inExecute. Thedefaultbranch is deliberately fail closed: an unrecognized type never terminates a session, so adding a type to a flow before the executor handles it cannot sign a user out unintentionally.Mark the template's confirm button. The Conditional Sign Out Flow template carries the type on the prompt action (which the engine reads) and on the button element (which the flow builder's Action selector reads and re-emits on save). Without the element field, opening that template in the builder and saving it would silently drop the type and regress the flow into an endless confirmation loop.
Stop enumerating action types in the docs. The
prompts[].action.typedescription listedSUBMITandREJECTin three places, while the same description also said arbitrary strings are accepted. The enumerations were therefore never the contract, and every new action type would need edits in all three. They are replaced with a structural description: the value is opaque, and the executor that consumes it owns the vocabulary.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Bug Fixes
Documentation