Rename flowID in flow execution to executionId#2213
Merged
ThaminduDilshan merged 1 commit intoasgardeo:mainfrom Apr 10, 2026
Merged
Rename flowID in flow execution to executionId#2213ThaminduDilshan merged 1 commit intoasgardeo:mainfrom
ThaminduDilshan merged 1 commit intoasgardeo:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRenames the runtime flow identifier from flowId/FlowID to executionId/ExecutionID across API schemas, backend models, service/store interfaces, logger keys, observability keys, executor code, frontend samples, docs, mocks, and tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
senthalan
reviewed
Apr 6, 2026
4d84f3c to
c588d9d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2213 +/- ##
==========================================
- Coverage 89.53% 89.52% -0.01%
==========================================
Files 913 913
Lines 60588 60593 +5
==========================================
+ Hits 54245 54246 +1
- Misses 4705 4710 +5
+ Partials 1638 1637 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a6d7120 to
9f3d801
Compare
6 tasks
521b148 to
1c7e492
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/static/api/next/combined.yaml (1)
1791-1857:⚠️ Potential issue | 🟠 Major
/flow/executeexamples and schemas are out of sync (executionIdvsflowId)The examples now use
executionId, but the declared schemas in this same spec still require/property-nameflowId(e.g.,SubSequentFlowRequest,IncompleteFlowResponse,CompleteFlowResponse,ErrorFlowResponse). This creates an invalid contract for generated clients and validators.Please apply the rename in the source OpenAPI spec (
api/flow-execution.yaml) for bothrequiredlists and property names, then regeneratedocs/static/api/next/combined.yamlinstead of hand-editing this file.Based on learnings:
docs/static/api/next/combined.yamlis auto-generated and contract changes should be made in source OpenAPI files, not via direct edits. As per coding guidelines:/flow/executemust useexecutionIdconsistently across both schemas and examples for subsequent request and all 200 response variants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/static/api/next/combined.yaml` around lines 1791 - 1857, The examples for POST /flow/execute use executionId but the source schemas still define/require flowId (SubSequentFlowRequest, IncompleteFlowResponse, CompleteFlowResponse, ErrorFlowResponse), causing a contract mismatch; to fix, update the source OpenAPI file api/flow-execution.yaml to rename the flowId property to executionId (and update any required lists) for the mentioned schemas and any subsequent-request schemas, then regenerate docs/static/api/next/combined.yaml (do not hand-edit combined.yaml) so the examples and schemas are consistent for /flow/execute.backend/internal/oauth/oauth2/authz/service.go (1)
257-292:⚠️ Potential issue | 🟠 Major🔴 Documentation Required: This PR introduces a user-facing change (e.g., new/modified API endpoint, configuration option, authentication flow, or SDK behavior) but does not include corresponding documentation updates under
docs/. Please update the relevant documentation before merging.Line 291 changes the authorization flow/query contract to
executionId(replacingflowId). Please document this rename in the API/auth flow docs (at minimumdocs/content/apis.mdxand related API examples such asdocs/static/api/next/combined.yaml, including/flow/executerequest/response examples where the same identifier is exposed).As per coding guidelines, user-facing API/auth-flow changes in
**/*.gomust include matching updates underdocs/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/oauth/oauth2/authz/service.go` around lines 257 - 292, The change replaces the previously exposed flow identifier name (flowId) with executionID/executionId in the authorization flow and query contract (see usage of executionID, queryParams[oauth2const.ExecutionID], and any references to flow execution such as the /flow/execute endpoint); update documentation accordingly by renaming occurrences of flowId to executionId in API docs (at minimum docs/content/apis.mdx) and in API spec/examples (e.g., docs/static/api/next/combined.yaml and any /flow/execute request/response examples), ensuring examples, parameter descriptions, and SDK/docs snippets show the new identifier name and any required request/response shapes that expose it.backend/internal/flow/flowexec/handler.go (1)
53-80:⚠️ Potential issue | 🟠 Major🔴 Documentation Required for
/flow/executeidentifier renameThis handler now processes and returns
executionId(Line 53, Line 68, Line 80), which is a user-facing API contract change. I don’t see corresponding documentation updates in the provided changeset.🔴 Documentation Required: This PR introduces a user-facing change
(e.g., new/modified API endpoint, configuration option, authentication flow, or SDK behavior)
but does not include corresponding documentation updates underdocs/.
Please update the relevant documentation before merging.Please document the
/flow/executerequest/response field rename (flowId→executionId) in:
docs/content/apis.mdx(API contract/reference),- relevant flow-execution guide content under
docs/content/guides/,- and ensure generated docs artifacts (e.g.,
docs/static/api/next/combined.yaml) are synced.As per coding guidelines
**/*.go: “If ANY user-facing API/auth/config/SDK change is detected and docs updates are missing under docs/, flag this as a major issue.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/handler.go` around lines 53 - 80, The code changes rename the public API field from flowId to executionId (see FlowResponse.ExecutionID, the handler in handler.go and the call to h.flowExecService.Execute that now uses executionID), so update user-facing documentation to match: modify the /flow/execute request/response examples and schema in docs/content/apis.mdx, update any flow-execution guide pages under docs/content/guides/ to use executionId everywhere (including sample payloads and SDK examples), and regenerate/sync the generated docs artifact (e.g., docs/static/api/next/combined.yaml) so the published API spec reflects the new executionId field.backend/internal/flow/flowexec/engine.go (1)
935-942:⚠️ Potential issue | 🟠 MajorUse one trace identifier consistently for all flow lifecycle events.
publishFlowStartedEvent/publishFlowFailedEventstill usectx.TraceID, while other flow/node events now usectx.ExecutionIDas the event trace ID. This splits a single flow execution across different trace IDs and weakens correlation.🔧 Proposed fix
func publishFlowStartedEvent(ctx *EngineContext, obsSvc observability.ObservabilityServiceInterface) { @@ evt := event.NewEvent( - ctx.TraceID, // Use TraceID from context + ctx.ExecutionID, // Use ExecutionID as TraceID string(event.EventTypeFlowStarted), event.ComponentFlowEngine, ). @@ func publishFlowFailedEvent(ctx *EngineContext, svcErr *serviceerror.ServiceError, flowStartTime int64, flowEndTime int64, obsSvc observability.ObservabilityServiceInterface) { @@ evt := event.NewEvent( - ctx.TraceID, // Use TraceID from context + ctx.ExecutionID, // Use ExecutionID as TraceID string(event.EventTypeFlowFailed), event.ComponentFlowEngine, ).Also applies to: 996-1003
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/engine.go` around lines 935 - 942, publishFlowStartedEvent and publishFlowFailedEvent create events using ctx.TraceID while other flow/node events use ctx.ExecutionID, splitting a single flow execution across different trace IDs; update the event construction in publishFlowStartedEvent and publishFlowFailedEvent (the event.NewEvent(...) calls) to use ctx.ExecutionID as the event trace identifier instead of ctx.TraceID so all lifecycle events share the same trace ID, and verify the same change is applied to the similar block around the other occurrence referenced (the lines around the second event.NewEvent usage).api/flow-execution.yaml (2)
216-219:⚠️ Potential issue | 🟡 MinorUse
COMPLETEin the complete-response schema example.
CompleteFlowResponse.flowStatusstill advertisesPROMPT_ONLY, while the response example above usesCOMPLETE. That leaves generated docs with conflicting contract examples for the same schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/flow-execution.yaml` around lines 216 - 219, The schema for CompleteFlowResponse currently sets flowStatus example to "PROMPT_ONLY" which conflicts with the earlier response example that uses "COMPLETE"; update the flowStatus example value to "COMPLETE" (or otherwise align both examples) in the flowStatus property of the CompleteFlowResponse schema so the documented contract is consistent—look for the flowStatus property definition in the flow execution schema and change its example string to "COMPLETE".
40-48:⚠️ Potential issue | 🟠 MajorDocument the field the backend actually deserializes.
SubSequentFlowRequeststill usesactionId, but the backend request model isFlowRequest.Actionwithjson:"action"inbackend/internal/flow/flowexec/model.go, and the integration helper intests/integration/testutils/oauth2_utils.goposts"action". Clients generated from this spec will send the wrong key and lose the selected action on subsequent/flow/executecalls. Rename the schema/example field toaction, or make the backend accept both names.Also applies to: 164-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/flow-execution.yaml` around lines 40 - 48, The OpenAPI example uses "actionId" but the backend deserializes FlowRequest.Action (json:"action") causing mismatches; update the API spec examples and any SubSequentFlowRequest schema in api/flow-execution.yaml to use the key "action" (e.g., replace actionId with action in the subSequentRequestExample and the other occurrence noted around lines 164-167) so generated clients send "action" like tests/integration/testutils/oauth2_utils.go and match backend/internal/flow/flowexec/model.go, or alternatively add support for both keys in the backend deserialization if you prefer backward compatibility.
🧹 Nitpick comments (11)
tests/integration/oauth/authz/authz_test.go (1)
841-842: Update assertion text to match the renamed field.Line 842 still says “Expected flow ID” while Line 841 validates
ExecutionID. Please align the message to avoid debugging confusion.Suggested patch
- if flowStep.ExecutionID == "" { - ts.T().Fatalf("Expected flow ID, got empty string") + if flowStep.ExecutionID == "" { + ts.T().Fatalf("Expected execution ID, got empty string") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/oauth/authz/authz_test.go` around lines 841 - 842, Update the test assertion message to reference the renamed field ExecutionID: change the ts.T().Fatalf message that currently reads "Expected flow ID, got empty string" to something like "Expected execution ID, got empty string" (or "Expected ExecutionID, got empty string") so it matches the checked symbol flowStep.ExecutionID and reduces confusion when ts.T().Fatalf is triggered.tests/integration/flow/registration/github_registration_test.go (1)
482-482: UseexecutionIDnaming in test messages/locals for consistency.The assertions and locals still use
Flow ID/flowIDwhile the field is nowExecutionID. Aligning names will reduce confusion in failures and maintenance.♻️ Proposed naming cleanup
- ts.Require().NotEmpty(flowStep.ExecutionID, "Flow ID should not be empty") + ts.Require().NotEmpty(flowStep.ExecutionID, "Execution ID should not be empty") - flowID := flowStep.ExecutionID + executionID := flowStep.ExecutionID - completeFlowStep, err := common.CompleteFlow(flowID, inputs, "") + completeFlowStep, err := common.CompleteFlow(executionID, inputs, "")Also applies to: 518-518, 520-520, 585-585, 603-603
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/registration/github_registration_test.go` at line 482, Assertions and local variables still use "Flow ID"/flowID while the struct field is ExecutionID; update all test message strings and any local variable names to use "executionID" (and the camelCase executionID variable) for consistency. Locate occurrences around the ts.Require().NotEmpty(flowStep.ExecutionID, ...) and related assertions (e.g., lines referenced where flowStep.ExecutionID is checked) and replace message text "Flow ID" with "executionID" and rename any local flowID variables to executionID so test failures and locals match the flowStep.ExecutionID field.tests/integration/flow/authentication/multi_action_input_binding_test.go (1)
409-427: Use lowerCamelCase for the local variable at Line 409
ExecutionIDis a local variable; in Go it should beexecutionIDfor idiomatic unexported naming.♻️ Suggested rename
- ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - flowStep, err = common.CompleteFlow(ExecutionID, nil, "action_google") + flowStep, err = common.CompleteFlow(executionID, nil, "action_google") ... - flowStep, err = common.CompleteFlow(ExecutionID, inputs, "") + flowStep, err = common.CompleteFlow(executionID, inputs, "")Based on learnings: “In asgardeo/thunder Go code, follow Go naming conventions: exported identifiers use CamelCase … and unexported identifiers use lowerCamelCase.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/multi_action_input_binding_test.go` around lines 409 - 427, The local variable ExecutionID is using exported-style CamelCase; rename it to executionID throughout this test to follow Go unexported naming conventions—update the declaration (ExecutionID := flowStep.ExecutionID) and all uses in this snippet and subsequent calls such as common.CompleteFlow(executionID, ...), ensuring references to ExecutionID, ExecutionID variable passed into CompleteFlow, and any assertions or helper calls use executionID consistently.tests/integration/flow/authentication/sensitive_input_cleanup_test.go (1)
255-255: Update stale assertion message terminologyAt Line 255, the assertion still says
"Flow ID should not be empty"while the field is nowExecutionID. Consider updating the message to avoid confusion during failures.✏️ Suggested text fix
- ts.Require().NotEmpty(flowStep.ExecutionID, "Flow ID should not be empty") + ts.Require().NotEmpty(flowStep.ExecutionID, "Execution ID should not be empty")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/sensitive_input_cleanup_test.go` at line 255, Update the stale assertion message for the NotEmpty check on flowStep.ExecutionID: change the message string from "Flow ID should not be empty" to something referencing ExecutionID (e.g., "ExecutionID should not be empty") so test failures clearly point to the ExecutionID field; locate the assertion call ts.Require().NotEmpty(flowStep.ExecutionID, ...) in the sensitive_input_cleanup_test.go test and replace the message accordingly.tests/integration/flow/authentication/authz_test.go (1)
355-355: Use “Execution ID” in assertion failure messages.The checks are correct, but the messages still refer to “Flow ID”.
Also applies to: 399-399, 435-435
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/authz_test.go` at line 355, Update the test assertion messages to refer to "Execution ID" instead of "Flow ID": change the failure message passed to ts.Require().NotEmpty for the flowStep.ExecutionID checks (calls to ts.Require().NotEmpty(flowStep.ExecutionID, ...)) so they read "Execution ID should not be empty" at each occurrence (the current occurrences around the tests at the three noted spots).tests/integration/flow/authentication/basic_auth_test.go (1)
315-315: Update stale assertion messages to “Execution ID”.These assertions now validate
ExecutionID, but the failure text still says “Flow ID”, which is confusing in test failures.Also applies to: 412-412, 430-430, 479-479, 516-516, 574-574
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/basic_auth_test.go` at line 315, Update the stale assertion messages that read "Flow ID should not be empty" to "Execution ID should not be empty" wherever ts.Require().NotEmpty is checking flowStep.ExecutionID (e.g., the calls using ts.Require().NotEmpty(flowStep.ExecutionID, ...)); change the message string in each occurrence (lines around the current one plus the other listed occurrences) so failures clearly reference "Execution ID".tests/integration/flow/authentication/google_auth_test.go (2)
335-335: Update stale assertion text to “Execution ID”.These assertions validate
ExecutionID, but the messages still mention “Flow ID”.Also applies to: 386-386
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/google_auth_test.go` at line 335, Update the assertion messages that reference the wrong label: change the descriptive text in the NotEmpty assertions that check flowStep.ExecutionID (e.g., ts.Require().NotEmpty(flowStep.ExecutionID, "...")) to read "Execution ID should not be empty" (and similarly update the other occurrence around the second location) so the message matches the actual field being validated.
388-388: Use lowerCamelCase for localexecutionIDvariables.Lines 388, 433, 451, and 486 declare local variables as
ExecutionID, which violates Go naming conventions. Local variables should use lowerCamelCase.♻️ Proposed fix
- ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID - completeFlowStep, err := common.CompleteFlow(ExecutionID, inputs, "") + completeFlowStep, err := common.CompleteFlow(executionID, inputs, "")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/google_auth_test.go` at line 388, The local variable name `ExecutionID` violates Go's lowerCamelCase convention; rename the local variable to `executionID` wherever it's declared from `flowStep.ExecutionID` (the occurrences currently named `ExecutionID` in google_auth_test.go) and update all subsequent uses in the same scope (assertions, function calls, etc.) to the new name `executionID` to keep consistency with Go naming conventions.tests/integration/flow/authentication/conditional_exec_auth_test.go (2)
383-394: Consider lowercase for local variable naming.The changes correctly use
flowStep.ExecutionID, but the local variableExecutionIDuses PascalCase. Go convention is lowerCamelCase for local variables.💅 Optional: Use lowerCamelCase for local variable
- ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID redirectURLStr := flowStep.Data.RedirectURL // Step 2: Simulate user authorization at Google authCode, err := testutils.SimulateFederatedOAuthFlow(redirectURLStr) ts.Require().NoError(err, "Failed to simulate Google authorization") // Step 3: Complete the flow with the authorization code inputs := map[string]string{ "code": authCode, } - flowStep, err = common.CompleteFlow(ExecutionID, inputs, "") + flowStep, err = common.CompleteFlow(executionID, inputs, "")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/conditional_exec_auth_test.go` around lines 383 - 394, The local variable ExecutionID should be renamed to follow Go's lowerCamelCase convention (e.g., executionID); update its declaration that currently assigns flowStep.ExecutionID and all subsequent uses (notably the call to common.CompleteFlow(ExecutionID, ...)) to use executionID instead so the code follows Go naming conventions.
428-452: Same naming convention applies to the second test.💅 Optional: Use lowerCamelCase for local variable
- ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID redirectURLStr := flowStep.Data.RedirectURL ... - flowStep, err = common.CompleteFlow(ExecutionID, inputs, "") + flowStep, err = common.CompleteFlow(executionID, inputs, "") ... - flowStep, err = common.CompleteFlow(ExecutionID, ouInputs, "") + flowStep, err = common.CompleteFlow(executionID, ouInputs, "")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/conditional_exec_auth_test.go` around lines 428 - 452, Rename the local variable ExecutionID to lowerCamelCase executionID throughout this test (and apply the same change in the second test mentioned) to match the project's naming convention; update all references where executionID is passed to common.CompleteFlow and any assertions that reference ExecutionID so the identifier is consistent (e.g., the assignments from flowStep.ExecutionID, the calls to common.CompleteFlow(ExecutionID, ...) and any subsequent uses).backend/internal/flow/flowexec/service.go (1)
338-341: Rename the guard errors toexecution ID.These branches validate
executionID, but the returned errors still sayflow ID cannot be empty. That makes the rename incomplete in logs and internal error chains.Also applies to: 364-366, 384-386
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/service.go` around lines 338 - 341, The guard error messages still say "flow ID cannot be empty" even though the parameter is executionID; update all such validation error strings to "execution ID cannot be empty" (e.g., in removeContext on s *flowExecService) and any other functions in the same file that validate executionID (the other two guard branches flagged in the review). Find the fmt.Errorf calls that return "flow ID cannot be empty" and change the message text to "execution ID cannot be empty" so logs and error chains consistently reference execution ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/flow/core/model.go`:
- Line 33: The API identifier was renamed from flowId/FlowID to
executionId/ExecutionID and the docs under docs/ need corresponding updates:
search for occurrences of flowId and FlowID and update examples and schema in
docs/static/api/next/combined.yaml (ensure the /flow/execute request/response
examples and schema use executionId) and update docs/content/apis.mdx (or the
specific API reference page) to describe the new executionId field and replace
example payloads accordingly; make sure both request and response examples, any
parameter descriptions, and example JSON bodies reflect ExecutionID/executionId
and preserve the same semantics.
In `@backend/internal/flow/executor/constants.go`:
- Line 103: Update the TODO comment that mentions executionID to fix the typo
and grammar: change "TODO: Check whether executionID need to be added her." to
"TODO: Check whether executionID needs to be added here." — locate the comment
near the executionID mention in constants.go and replace the sentence
accordingly.
- Around line 103-104: Add the execution instance identifier to the
nonUserAttributes filter: update the nonUserAttributes slice to include
"executionId" (camelCase, lowercase 'e') alongside existing entries like
"flowID" so execution identifiers are excluded from user attributes during
provisioning, and update the TODO comment to state that executionId must be
added rather than merely checked; modify the declaration of nonUserAttributes in
constants.go accordingly (refer to the nonUserAttributes variable).
In `@backend/internal/flow/flowexec/error_constants.go`:
- Line 51: Update the stale comment above the ErrorInvalidExecutionID constant
so it references "execution ID" instead of "flow ID"; locate the declaration for
ErrorInvalidExecutionID in this file and change the comment to something like
"ErrorInvalidExecutionID defines the error response for invalid execution ID
errors" so the comment matches the renamed constant and its message.
In `@backend/internal/flow/flowexec/service.go`:
- Around line 80-82: The public request/response field was renamed from flowId
to executionId (see Execute in flowexec/service.go and the /flow/execute
contract), but docs weren't updated; update docs/content/apis.mdx and the
relevant guide under docs/content/guides/ to replace flowId with executionId in
the /flow/execute API spec and examples, add a short migration note advising
clients to rename the request/response field and how to map old flowId to new
executionId, and ensure any sample payloads, OpenAPI snippets, and SDK usage
examples reflect executionId consistently.
In `@backend/internal/oauth/oauth2/authz/handler_test.go`:
- Around line 200-202: The tests and code rename the OAuth authorize/login
correlation query parameter from flowId to executionId (see
oauth2const.ExecutionID used in handler_test.go and the /oauth2/authorize
handling), but the PR lacks user-facing docs updates; update
docs/content/apis.mdx (OAuth authorize behavior) and the relevant guides under
docs/content/guides/ to state that the query param is now executionId (replacing
flowId), include example authorize/login URLs, migration notes for clients, and
adjust any samples or SDK snippets to use oauth2?executionId=<value> so docs
match the code change.
In `@tests/integration/flow/authentication/github_auth_test.go`:
- Line 323: The assertion messages still refer to "Flow ID" while the field is
now ExecutionID; update the NotEmpty assertion message(s) that check
flowStep.ExecutionID (both occurrences including the one around line 323 and the
one around line 371) to say "ExecutionID should not be empty" (or similar) so
the message matches the renamed identifier used in the test (search for
ts.Require().NotEmpty(flowStep.ExecutionID, ...) and replace the second argument
accordingly).
In `@tests/integration/flow/common/utils.go`:
- Around line 143-147: The flow execution API changed: subsequent /flow/execute
requests/responses use executionId instead of flowId; update the docs and
OpenAPI examples to reflect this change. Find the client/test usage in
CompleteFlow where executionId is set and ensure all API reference narrative,
example request/response payloads (incomplete/complete/display-only/error flows)
and any example subsequent requests show executionId consistently; also update
the API changelog entry to mention the change. Ensure example schemas and sample
payloads in the OpenAPI/combined examples are updated so generated docs and SDKs
reflect executionId in both requests and responses.
In `@tests/integration/flow/registration/github_registration_test.go`:
- Line 520: Tests show the public continuation identifier for /flow/execute
changed from flowId to executionId (see flowID := flowStep.ExecutionID in
github_registration_test.go); update the user-facing docs and API spec to
reflect this rename: modify the API guide (apis.mdx) to describe the executionId
field and its semantics, update the combined.yaml API schema and any example
request/response snippets for /flow/execute to use executionId instead of
flowId, and adjust API reference examples that mention flowId so they use
executionId. Ensure examples, request/response schemas, and any SDK snippets are
consistent with the new field name.
In
`@tests/integration/flow/registration/google_registration_with_role_group_test.go`:
- Line 440: The assertion message uses an outdated term "Flow ID" for
flowStep.ExecutionID; update the assertion call (ts.Require().NotEmpty
referencing flowStep.ExecutionID) to use a correct, clear message such as
"Execution ID should not be empty" so the failure text matches the actual field
being tested.
---
Outside diff comments:
In `@api/flow-execution.yaml`:
- Around line 216-219: The schema for CompleteFlowResponse currently sets
flowStatus example to "PROMPT_ONLY" which conflicts with the earlier response
example that uses "COMPLETE"; update the flowStatus example value to "COMPLETE"
(or otherwise align both examples) in the flowStatus property of the
CompleteFlowResponse schema so the documented contract is consistent—look for
the flowStatus property definition in the flow execution schema and change its
example string to "COMPLETE".
- Around line 40-48: The OpenAPI example uses "actionId" but the backend
deserializes FlowRequest.Action (json:"action") causing mismatches; update the
API spec examples and any SubSequentFlowRequest schema in
api/flow-execution.yaml to use the key "action" (e.g., replace actionId with
action in the subSequentRequestExample and the other occurrence noted around
lines 164-167) so generated clients send "action" like
tests/integration/testutils/oauth2_utils.go and match
backend/internal/flow/flowexec/model.go, or alternatively add support for both
keys in the backend deserialization if you prefer backward compatibility.
In `@backend/internal/flow/flowexec/engine.go`:
- Around line 935-942: publishFlowStartedEvent and publishFlowFailedEvent create
events using ctx.TraceID while other flow/node events use ctx.ExecutionID,
splitting a single flow execution across different trace IDs; update the event
construction in publishFlowStartedEvent and publishFlowFailedEvent (the
event.NewEvent(...) calls) to use ctx.ExecutionID as the event trace identifier
instead of ctx.TraceID so all lifecycle events share the same trace ID, and
verify the same change is applied to the similar block around the other
occurrence referenced (the lines around the second event.NewEvent usage).
In `@backend/internal/flow/flowexec/handler.go`:
- Around line 53-80: The code changes rename the public API field from flowId to
executionId (see FlowResponse.ExecutionID, the handler in handler.go and the
call to h.flowExecService.Execute that now uses executionID), so update
user-facing documentation to match: modify the /flow/execute request/response
examples and schema in docs/content/apis.mdx, update any flow-execution guide
pages under docs/content/guides/ to use executionId everywhere (including sample
payloads and SDK examples), and regenerate/sync the generated docs artifact
(e.g., docs/static/api/next/combined.yaml) so the published API spec reflects
the new executionId field.
In `@backend/internal/oauth/oauth2/authz/service.go`:
- Around line 257-292: The change replaces the previously exposed flow
identifier name (flowId) with executionID/executionId in the authorization flow
and query contract (see usage of executionID,
queryParams[oauth2const.ExecutionID], and any references to flow execution such
as the /flow/execute endpoint); update documentation accordingly by renaming
occurrences of flowId to executionId in API docs (at minimum
docs/content/apis.mdx) and in API spec/examples (e.g.,
docs/static/api/next/combined.yaml and any /flow/execute request/response
examples), ensuring examples, parameter descriptions, and SDK/docs snippets show
the new identifier name and any required request/response shapes that expose it.
In `@docs/static/api/next/combined.yaml`:
- Around line 1791-1857: The examples for POST /flow/execute use executionId but
the source schemas still define/require flowId (SubSequentFlowRequest,
IncompleteFlowResponse, CompleteFlowResponse, ErrorFlowResponse), causing a
contract mismatch; to fix, update the source OpenAPI file
api/flow-execution.yaml to rename the flowId property to executionId (and update
any required lists) for the mentioned schemas and any subsequent-request
schemas, then regenerate docs/static/api/next/combined.yaml (do not hand-edit
combined.yaml) so the examples and schemas are consistent for /flow/execute.
---
Nitpick comments:
In `@backend/internal/flow/flowexec/service.go`:
- Around line 338-341: The guard error messages still say "flow ID cannot be
empty" even though the parameter is executionID; update all such validation
error strings to "execution ID cannot be empty" (e.g., in removeContext on s
*flowExecService) and any other functions in the same file that validate
executionID (the other two guard branches flagged in the review). Find the
fmt.Errorf calls that return "flow ID cannot be empty" and change the message
text to "execution ID cannot be empty" so logs and error chains consistently
reference execution ID.
In `@tests/integration/flow/authentication/authz_test.go`:
- Line 355: Update the test assertion messages to refer to "Execution ID"
instead of "Flow ID": change the failure message passed to ts.Require().NotEmpty
for the flowStep.ExecutionID checks (calls to
ts.Require().NotEmpty(flowStep.ExecutionID, ...)) so they read "Execution ID
should not be empty" at each occurrence (the current occurrences around the
tests at the three noted spots).
In `@tests/integration/flow/authentication/basic_auth_test.go`:
- Line 315: Update the stale assertion messages that read "Flow ID should not be
empty" to "Execution ID should not be empty" wherever ts.Require().NotEmpty is
checking flowStep.ExecutionID (e.g., the calls using
ts.Require().NotEmpty(flowStep.ExecutionID, ...)); change the message string in
each occurrence (lines around the current one plus the other listed occurrences)
so failures clearly reference "Execution ID".
In `@tests/integration/flow/authentication/conditional_exec_auth_test.go`:
- Around line 383-394: The local variable ExecutionID should be renamed to
follow Go's lowerCamelCase convention (e.g., executionID); update its
declaration that currently assigns flowStep.ExecutionID and all subsequent uses
(notably the call to common.CompleteFlow(ExecutionID, ...)) to use executionID
instead so the code follows Go naming conventions.
- Around line 428-452: Rename the local variable ExecutionID to lowerCamelCase
executionID throughout this test (and apply the same change in the second test
mentioned) to match the project's naming convention; update all references where
executionID is passed to common.CompleteFlow and any assertions that reference
ExecutionID so the identifier is consistent (e.g., the assignments from
flowStep.ExecutionID, the calls to common.CompleteFlow(ExecutionID, ...) and any
subsequent uses).
In `@tests/integration/flow/authentication/google_auth_test.go`:
- Line 335: Update the assertion messages that reference the wrong label: change
the descriptive text in the NotEmpty assertions that check flowStep.ExecutionID
(e.g., ts.Require().NotEmpty(flowStep.ExecutionID, "...")) to read "Execution ID
should not be empty" (and similarly update the other occurrence around the
second location) so the message matches the actual field being validated.
- Line 388: The local variable name `ExecutionID` violates Go's lowerCamelCase
convention; rename the local variable to `executionID` wherever it's declared
from `flowStep.ExecutionID` (the occurrences currently named `ExecutionID` in
google_auth_test.go) and update all subsequent uses in the same scope
(assertions, function calls, etc.) to the new name `executionID` to keep
consistency with Go naming conventions.
In `@tests/integration/flow/authentication/multi_action_input_binding_test.go`:
- Around line 409-427: The local variable ExecutionID is using exported-style
CamelCase; rename it to executionID throughout this test to follow Go unexported
naming conventions—update the declaration (ExecutionID := flowStep.ExecutionID)
and all uses in this snippet and subsequent calls such as
common.CompleteFlow(executionID, ...), ensuring references to ExecutionID,
ExecutionID variable passed into CompleteFlow, and any assertions or helper
calls use executionID consistently.
In `@tests/integration/flow/authentication/sensitive_input_cleanup_test.go`:
- Line 255: Update the stale assertion message for the NotEmpty check on
flowStep.ExecutionID: change the message string from "Flow ID should not be
empty" to something referencing ExecutionID (e.g., "ExecutionID should not be
empty") so test failures clearly point to the ExecutionID field; locate the
assertion call ts.Require().NotEmpty(flowStep.ExecutionID, ...) in the
sensitive_input_cleanup_test.go test and replace the message accordingly.
In `@tests/integration/flow/registration/github_registration_test.go`:
- Line 482: Assertions and local variables still use "Flow ID"/flowID while the
struct field is ExecutionID; update all test message strings and any local
variable names to use "executionID" (and the camelCase executionID variable) for
consistency. Locate occurrences around the
ts.Require().NotEmpty(flowStep.ExecutionID, ...) and related assertions (e.g.,
lines referenced where flowStep.ExecutionID is checked) and replace message text
"Flow ID" with "executionID" and rename any local flowID variables to
executionID so test failures and locals match the flowStep.ExecutionID field.
In `@tests/integration/oauth/authz/authz_test.go`:
- Around line 841-842: Update the test assertion message to reference the
renamed field ExecutionID: change the ts.T().Fatalf message that currently reads
"Expected flow ID, got empty string" to something like "Expected execution ID,
got empty string" (or "Expected ExecutionID, got empty string") so it matches
the checked symbol flowStep.ExecutionID and reduces confusion when ts.T().Fatalf
is triggered.
🪄 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
Run ID: 09ad6e5d-1538-4b1e-9bd0-18533027d247
⛔ Files ignored due to path filters (2)
backend/tests/mocks/flow/flowexecmock/FlowExecServiceInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/flowStoreInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (98)
api/flow-execution.yamlbackend/internal/flow/core/executor.gobackend/internal/flow/core/executor_test.gobackend/internal/flow/core/model.gobackend/internal/flow/core/node_test.gobackend/internal/flow/core/prompt_node.gobackend/internal/flow/core/prompt_node_test.gobackend/internal/flow/core/representation_node_test.gobackend/internal/flow/core/task_execution_node.gobackend/internal/flow/core/task_execution_node_test.gobackend/internal/flow/executor/attribute_collector.gobackend/internal/flow/executor/attribute_collector_test.gobackend/internal/flow/executor/attribute_uniqueness_validator.gobackend/internal/flow/executor/attribute_uniqueness_validator_test.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/auth_assert_executor_test.gobackend/internal/flow/executor/authz_executor.gobackend/internal/flow/executor/authz_executor_test.gobackend/internal/flow/executor/basic_auth_executor.gobackend/internal/flow/executor/basic_auth_executor_test.gobackend/internal/flow/executor/consent_executor.gobackend/internal/flow/executor/consent_executor_test.gobackend/internal/flow/executor/constants.gobackend/internal/flow/executor/credential_setter.gobackend/internal/flow/executor/credential_setter_test.gobackend/internal/flow/executor/email_executor.gobackend/internal/flow/executor/email_executor_test.gobackend/internal/flow/executor/federated_auth_resolver_executor.gobackend/internal/flow/executor/federated_auth_resolver_executor_test.gobackend/internal/flow/executor/http_request_executor.gobackend/internal/flow/executor/http_request_executor_test.gobackend/internal/flow/executor/identifying_executor.gobackend/internal/flow/executor/identifying_executor_test.gobackend/internal/flow/executor/invite_executor.gobackend/internal/flow/executor/invite_executor_test.gobackend/internal/flow/executor/oauth_executor.gobackend/internal/flow/executor/oauth_executor_test.gobackend/internal/flow/executor/oidc_auth_executor.gobackend/internal/flow/executor/oidc_auth_executor_test.gobackend/internal/flow/executor/ou_executor.gobackend/internal/flow/executor/ou_executor_test.gobackend/internal/flow/executor/ou_resolver_executor.gobackend/internal/flow/executor/ou_resolver_executor_test.gobackend/internal/flow/executor/passkey_executor.gobackend/internal/flow/executor/passkey_executor_test.gobackend/internal/flow/executor/permission_validator.gobackend/internal/flow/executor/permission_validator_test.gobackend/internal/flow/executor/provisioning_executor.gobackend/internal/flow/executor/provisioning_executor_test.gobackend/internal/flow/executor/sms_auth_executor.gobackend/internal/flow/executor/sms_auth_executor_test.gobackend/internal/flow/executor/sms_executor.gobackend/internal/flow/executor/sms_executor_test.gobackend/internal/flow/executor/user_type_resolver.gobackend/internal/flow/executor/user_type_resolver_test.gobackend/internal/flow/flowexec/FlowExecServiceInterface_mock_test.gobackend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/engine_event_enabled_test.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/flow/flowexec/flowStoreInterface_mock_test.gobackend/internal/flow/flowexec/handler.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/model_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/flowexec/store.gobackend/internal/flow/flowexec/store_test.gobackend/internal/oauth/oauth2/authz/handler_test.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/system/log/constants.gobackend/internal/system/observability/event/datakeys.godocs/static/api/next/combined.yamltests/integration/flow/authentication/assurance_test.gotests/integration/flow/authentication/attribute_collect_test.gotests/integration/flow/authentication/authz_test.gotests/integration/flow/authentication/basic_auth_test.gotests/integration/flow/authentication/conditional_exec_auth_test.gotests/integration/flow/authentication/github_auth_test.gotests/integration/flow/authentication/google_auth_test.gotests/integration/flow/authentication/multi_action_input_binding_test.gotests/integration/flow/authentication/prompt_actions_test.gotests/integration/flow/authentication/sensitive_input_cleanup_test.gotests/integration/flow/authentication/sms_auth_test.gotests/integration/flow/authentication/verbose_meta_test.gotests/integration/flow/common/model.gotests/integration/flow/common/utils.gotests/integration/flow/registration/basic_registration_test.gotests/integration/flow/registration/github_registration_test.gotests/integration/flow/registration/google_registration_test.gotests/integration/flow/registration/google_registration_with_role_group_test.gotests/integration/flow/registration/http_request_executor_runtime_data_test.gotests/integration/flow/registration/ou_registration_test.gotests/integration/flow/registration/sms_registration_test.gotests/integration/oauth/authz/authz_test.gotests/integration/testutils/models.gotests/integration/testutils/oauth2_utils.go
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
samples/api/postman/authentication/authentication_demo.json (1)
1984-1988: Consider renaming the collection variable too.The API field is now
executionId, but this sample still persists it underexec_flow_id. Renaming the variable and its later consumers would make the collection match the new contract end-to-end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/api/postman/authentication/authentication_demo.json` around lines 1984 - 1988, The sample persists the API's executionId under the old collection variable name "exec_flow_id"; update the pm.collectionVariables.set call to use a matching name (e.g., "executionId") and update all consumers that read this variable (pm.collectionVariables.get or any references) so the collection variable key and its usage align with the new API field; search for occurrences of "exec_flow_id" and replace them with the new variable name while keeping the value set from jsonResponse.executionId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/api/postman/authentication/authentication_demo.json`:
- Around line 58-64: The script extracts executionId via
url.searchParams.get("executionId") but doesn't fail if it's missing, so add a
guard after retrieving executionId: if executionId is null/empty, immediately
fail the pre-request/test using Postman's failure helpers (e.g.,
pm.test/pm.expect or pm.fail) and do not call pm.environment.set for
executionId; keep setting authId as before but ensure you log and fail fast when
executionId is absent so subsequent requests don't proceed with a null value.
- Line 2858: The request body in the Postman sample (the "raw" field for the
/flow/execute sample containing "executionId" and "inputs" with
"username"/"password") includes a JavaScript-style comment // "mobileNumber":
... which makes the JSON invalid; remove the inline comment or convert it to a
valid JSON element (either delete the mobileNumber line or add it as a real
key/value) so the "raw" string is valid JSON before sending; update the raw
payload string accordingly in the authentication_demo.json sample.
---
Nitpick comments:
In `@samples/api/postman/authentication/authentication_demo.json`:
- Around line 1984-1988: The sample persists the API's executionId under the old
collection variable name "exec_flow_id"; update the pm.collectionVariables.set
call to use a matching name (e.g., "executionId") and update all consumers that
read this variable (pm.collectionVariables.get or any references) so the
collection variable key and its usage align with the new API field; search for
occurrences of "exec_flow_id" and replace them with the new variable name while
keeping the value set from jsonResponse.executionId.
🪄 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
Run ID: ed5000ae-02d4-4053-8ef0-b3af610344e1
📒 Files selected for processing (8)
ARCHITECTURE.mdREADME.mddocs/content/guides/key-concepts/authentication/passwordless/passkeys.mdxdocs/static/api/next/combined.yamlsamples/api/postman/authentication/README.mdsamples/api/postman/authentication/authentication_demo.jsonsamples/api/postman/authentication/environment.jsontests/e2e/tests/sample-app-authentication/README-MFA.md
✅ Files skipped from review due to trivial changes (4)
- samples/api/postman/authentication/environment.json
- samples/api/postman/authentication/README.md
- README.md
- ARCHITECTURE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/static/api/next/combined.yaml
f290ab4 to
a47c727
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/internal/flow/flowexec/service.go (1)
43-45:⚠️ Potential issue | 🟠 MajorDocumentation still needs to land with this breaking API rename.
Already raised on an earlier revision, but this change still renames the public
/flow/executerequest/response identifier without the matching contract/guide/sample updates. Please update the source OpenAPI spec and the user-facing guides/samples that carry this field (api/flow-execution.yaml,docs/content/guides/key-concepts/authentication/passwordless/passkeys.mdx, andsamples/api/postman/authentication/README.md) before merge.As per coding guidelines, "🔴 Documentation Required: This PR introduces a user-facing change (e.g., new/modified API endpoint, configuration option, authentication flow, or SDK behavior) but does not include corresponding documentation updates under
docs/." Based on learnings,docs/static/api/next/combined.yamlis auto-generated from the source OpenAPI files and should not be hand-edited.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/service.go` around lines 43 - 45, The public API field/identifier renamed by the Flow service (see Execute/InitiateFlow change) must be propagated to the source OpenAPI and user docs: update api/flow-execution.yaml to use the new request/response identifier names, then regenerate the combined spec (docs/static/api/next/combined.yaml must NOT be edited directly); update the user-facing references in docs/content/guides/key-concepts/authentication/passwordless/passkeys.mdx and the samples/api/postman/authentication/README.md to reflect the new field names and any example payloads, and run the OpenAPI/spec generation step to verify the combined.yaml and samples align with the new API contract before merging.
🧹 Nitpick comments (3)
samples/apps/react-vanilla-sample/src/pages/LoginPage.tsx (1)
346-348: Potential stale closure: usedata.executionIdfor consistency.This line uses the
executionIdstate variable, butprocessAuthResponsedoesn't includeexecutionIdin its dependency array (line 377). SincesetExecutionId(data.executionId || '')is called just above at line 266, the state variable won't have the updated value yet due to React's async state updates.In
init()(line 453) andinitPromptSignupDecision()(line 549), the pattern correctly usesdata.executionIddirectly from the response. Consider using the same pattern here for consistency and correctness.♻️ Suggested fix
- submitAuthDecision(executionId, singleAction.ref) + submitAuthDecision(data.executionId || '', singleAction.ref)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/apps/react-vanilla-sample/src/pages/LoginPage.tsx` around lines 346 - 348, The call to submitAuthDecision and subsequent processAuthResponse uses the stale executionId state; instead, read the executionId from the server response and pass that through: in the promise handler for submitAuthDecision (the then block receiving result), extract result.data.executionId and pass it to processAuthResponse (and to any further calls that currently use the executionId state). Update references around submitAuthDecision(...) and processAuthResponse(...) so they use result.data.executionId (and keep singleAction.ref as before) to avoid relying on the async React state update.samples/apps/react-vanilla-sample/src/services/authService.ts (2)
370-375: JSDoc description is inconsistent with the renamed parameter.The parameter is renamed to
executionIdbut the description still says "The flow ID". Update for consistency with the rename.📝 Suggested fix
/** * Submits the user's selected authentication option when multiple options are available. * - * `@param` {string} executionId - The flow ID received from the initiateNativeAuth response. + * `@param` {string} executionId - The execution ID received from the initiateNativeAuth response. * `@param` {string} actionId - The ID of the selected authentication action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/apps/react-vanilla-sample/src/services/authService.ts` around lines 370 - 375, Update the JSDoc for submitAuthDecision so the parameter description matches the renamed parameter: replace the phrase "The flow ID received from the initiateNativeAuth response." with a description that references executionId (e.g., "The execution ID received from the initiateNativeAuth response.") and ensure the other param docs (actionId, inputs) remain accurate for the submitAuthDecision function.
410-416: JSDoc description is inconsistent with the renamed parameter.Same issue as above - the description still refers to "flow ID".
📝 Suggested fix
/** * Submits the native authentication form data to the server. * - * `@param` {string} executionId - The flow ID received from the initiateNativeAuth response. + * `@param` {string} executionId - The execution ID received from the initiateNativeAuth response. * `@param` {object} payload - The payload containing the form data or other required information.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/apps/react-vanilla-sample/src/services/authService.ts` around lines 410 - 416, The JSDoc for submitNativeAuth still refers to "flow ID" while the parameter has been renamed to executionId; update the function comment to describe executionId consistently (e.g., "executionId - The execution ID returned from initiateNativeAuth") and make corresponding wording changes in the param description and `@returns` section if needed so the docs match the submitNativeAuth(executionId: string, ...) signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/flow/flowexec/engine.go`:
- Around line 823-829: Several events are using ctx.TraceID while node events
and publishFlowCompletedEvent use ctx.ExecutionID, causing trace split; update
publishFlowStartedEvent and publishFlowFailedEvent (and the other occurrences
noted around the blocks at 894-900, 935-942, 996-1003) so they set the event
TraceID to ctx.ExecutionID instead of ctx.TraceID. Locate the event construction
calls in publishFlowStartedEvent and publishFlowFailedEvent that pass
ctx.TraceID (and any similar builder calls) and replace them with
ctx.ExecutionID, keeping all other WithStatus/WithData calls unchanged so the
whole flow lifecycle shares the same trace id.
In `@backend/internal/oauth/oauth2/constants/constants.go`:
- Line 89: The change renames the public flow execution identifier to
ExecutionID ("executionId"), which is a user-facing auth/API behavior change;
update documentation to reflect this rename and how clients must pass/propagate
it: add/modify docs/content/apis.mdx to document the /flow/execute API using
"executionId" in requests/responses, update
docs/content/guides/key-concepts/authentication/passwordless/passkeys.mdx to
show carrying "executionId" across flow steps with examples and required fields,
and update observability guidance in
docs/content/community/contributing/contributing-code/backend-development/observability.md
to note the new flow correlation key name ("executionId") and any impacts on
logs/metrics/tracing. Ensure references use the constant name ExecutionID when
describing the code-level identifier and include any migration notes for
clients.
In `@tests/integration/oauth/authz/authz_test.go`:
- Around line 841-842: The test assertion message is stale: it checks
flowStep.ExecutionID but the failure text says "flow ID"; update the Fatalf
message in the test (the block checking flowStep.ExecutionID) to reference
"ExecutionID" (e.g., "Expected ExecutionID, got empty string") so failure output
matches the actual field being asserted; locate the assertion that reads
ts.T().Fatalf("Expected flow ID, got empty string") and change the message to
mention ExecutionID.
---
Duplicate comments:
In `@backend/internal/flow/flowexec/service.go`:
- Around line 43-45: The public API field/identifier renamed by the Flow service
(see Execute/InitiateFlow change) must be propagated to the source OpenAPI and
user docs: update api/flow-execution.yaml to use the new request/response
identifier names, then regenerate the combined spec
(docs/static/api/next/combined.yaml must NOT be edited directly); update the
user-facing references in
docs/content/guides/key-concepts/authentication/passwordless/passkeys.mdx and
the samples/api/postman/authentication/README.md to reflect the new field names
and any example payloads, and run the OpenAPI/spec generation step to verify the
combined.yaml and samples align with the new API contract before merging.
---
Nitpick comments:
In `@samples/apps/react-vanilla-sample/src/pages/LoginPage.tsx`:
- Around line 346-348: The call to submitAuthDecision and subsequent
processAuthResponse uses the stale executionId state; instead, read the
executionId from the server response and pass that through: in the promise
handler for submitAuthDecision (the then block receiving result), extract
result.data.executionId and pass it to processAuthResponse (and to any further
calls that currently use the executionId state). Update references around
submitAuthDecision(...) and processAuthResponse(...) so they use
result.data.executionId (and keep singleAction.ref as before) to avoid relying
on the async React state update.
In `@samples/apps/react-vanilla-sample/src/services/authService.ts`:
- Around line 370-375: Update the JSDoc for submitAuthDecision so the parameter
description matches the renamed parameter: replace the phrase "The flow ID
received from the initiateNativeAuth response." with a description that
references executionId (e.g., "The execution ID received from the
initiateNativeAuth response.") and ensure the other param docs (actionId,
inputs) remain accurate for the submitAuthDecision function.
- Around line 410-416: The JSDoc for submitNativeAuth still refers to "flow ID"
while the parameter has been renamed to executionId; update the function comment
to describe executionId consistently (e.g., "executionId - The execution ID
returned from initiateNativeAuth") and make corresponding wording changes in the
param description and `@returns` section if needed so the docs match the
submitNativeAuth(executionId: string, ...) signature.
🪄 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
Run ID: d1bbe764-c776-4559-b069-2fb0f19a7b20
⛔ Files ignored due to path filters (13)
backend/tests/mocks/flow/flowexecmock/FlowExecServiceInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/flowStoreInterface_mock.gois excluded by!**/*_mock.godocs/__backup__/README.mdis excluded by!docs/__backup__/**docs/__backup__/authentication/server-orchestrated-flow/authentication.mdis excluded by!docs/__backup__/**docs/__backup__/authorization/flow-based-authorization.mdis excluded by!docs/__backup__/**docs/__backup__/flows/flow-execution.mdis excluded by!docs/__backup__/**docs/__backup__/observability/events.mdis excluded by!docs/__backup__/**docs/__backup__/registration/server-orchestrated-flow/registration.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/features/public-clients.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/authorization-code.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/client-credentials.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/token-exchange.mdis excluded by!docs/__backup__/**frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (112)
ARCHITECTURE.mdREADME.mdapi/flow-execution.yamlbackend/internal/flow/core/executor.gobackend/internal/flow/core/executor_test.gobackend/internal/flow/core/model.gobackend/internal/flow/core/node_test.gobackend/internal/flow/core/prompt_node.gobackend/internal/flow/core/prompt_node_test.gobackend/internal/flow/core/representation_node_test.gobackend/internal/flow/core/task_execution_node.gobackend/internal/flow/core/task_execution_node_test.gobackend/internal/flow/executor/attribute_collector.gobackend/internal/flow/executor/attribute_collector_test.gobackend/internal/flow/executor/attribute_uniqueness_validator.gobackend/internal/flow/executor/attribute_uniqueness_validator_test.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/auth_assert_executor_test.gobackend/internal/flow/executor/authz_executor.gobackend/internal/flow/executor/authz_executor_test.gobackend/internal/flow/executor/basic_auth_executor.gobackend/internal/flow/executor/basic_auth_executor_test.gobackend/internal/flow/executor/consent_executor.gobackend/internal/flow/executor/consent_executor_test.gobackend/internal/flow/executor/credential_setter.gobackend/internal/flow/executor/credential_setter_test.gobackend/internal/flow/executor/email_executor.gobackend/internal/flow/executor/email_executor_test.gobackend/internal/flow/executor/federated_auth_resolver_executor.gobackend/internal/flow/executor/federated_auth_resolver_executor_test.gobackend/internal/flow/executor/http_request_executor.gobackend/internal/flow/executor/http_request_executor_test.gobackend/internal/flow/executor/identifying_executor.gobackend/internal/flow/executor/identifying_executor_test.gobackend/internal/flow/executor/invite_executor.gobackend/internal/flow/executor/invite_executor_test.gobackend/internal/flow/executor/oauth_executor.gobackend/internal/flow/executor/oauth_executor_test.gobackend/internal/flow/executor/oidc_auth_executor.gobackend/internal/flow/executor/oidc_auth_executor_test.gobackend/internal/flow/executor/ou_executor.gobackend/internal/flow/executor/ou_executor_test.gobackend/internal/flow/executor/ou_resolver_executor.gobackend/internal/flow/executor/ou_resolver_executor_test.gobackend/internal/flow/executor/passkey_executor.gobackend/internal/flow/executor/passkey_executor_test.gobackend/internal/flow/executor/permission_validator.gobackend/internal/flow/executor/permission_validator_test.gobackend/internal/flow/executor/provisioning_executor.gobackend/internal/flow/executor/provisioning_executor_test.gobackend/internal/flow/executor/sms_auth_executor.gobackend/internal/flow/executor/sms_auth_executor_test.gobackend/internal/flow/executor/sms_executor.gobackend/internal/flow/executor/sms_executor_test.gobackend/internal/flow/executor/user_type_resolver.gobackend/internal/flow/executor/user_type_resolver_test.gobackend/internal/flow/flowexec/FlowExecServiceInterface_mock_test.gobackend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/engine_event_enabled_test.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/flow/flowexec/flowStoreInterface_mock_test.gobackend/internal/flow/flowexec/handler.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/model_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/flowexec/store.gobackend/internal/flow/flowexec/store_test.gobackend/internal/oauth/oauth2/authz/handler_test.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/system/log/constants.gobackend/internal/system/observability/event/datakeys.godocs/content/community/contributing/contributing-code/backend-development/observability.mddocs/content/guides/key-concepts/authentication/passwordless/passkeys.mdxdocs/static/api/next/combined.yamlfrontend/apps/thunder-gate/src/pages/AcceptInvitePage.tsxfrontend/pnpm-workspace.yamlsamples/api/postman/authentication/README.mdsamples/api/postman/authentication/authentication_demo.jsonsamples/api/postman/authentication/environment.jsonsamples/apps/react-vanilla-sample/src/pages/LoginPage.tsxsamples/apps/react-vanilla-sample/src/services/authService.tstests/e2e/tests/sample-app-authentication/README-MFA.mdtests/e2e/utils/thunder-setup/mfa-setup.tstests/integration/flow/authentication/assurance_test.gotests/integration/flow/authentication/attribute_collect_test.gotests/integration/flow/authentication/authz_test.gotests/integration/flow/authentication/basic_auth_test.gotests/integration/flow/authentication/conditional_exec_auth_test.gotests/integration/flow/authentication/github_auth_test.gotests/integration/flow/authentication/google_auth_test.gotests/integration/flow/authentication/multi_action_input_binding_test.gotests/integration/flow/authentication/prompt_actions_test.gotests/integration/flow/authentication/sensitive_input_cleanup_test.gotests/integration/flow/authentication/sms_auth_test.gotests/integration/flow/authentication/verbose_meta_test.gotests/integration/flow/common/model.gotests/integration/flow/common/utils.gotests/integration/flow/registration/basic_registration_test.gotests/integration/flow/registration/github_registration_test.gotests/integration/flow/registration/google_registration_test.gotests/integration/flow/registration/google_registration_with_role_group_test.gotests/integration/flow/registration/http_request_executor_runtime_data_test.gotests/integration/flow/registration/ou_registration_test.gotests/integration/flow/registration/sms_registration_test.gotests/integration/oauth/authz/authz_test.gotests/integration/oauth/token/refresh_token_test.gotests/integration/oauth/userinfo/userinfo_test.gotests/integration/testutils/models.gotests/integration/testutils/oauth2_utils.go
✅ Files skipped from review due to trivial changes (42)
- backend/internal/flow/core/prompt_node.go
- backend/internal/flow/executor/consent_executor.go
- backend/internal/flow/core/node_test.go
- backend/internal/flow/executor/ou_resolver_executor.go
- backend/internal/flow/executor/oidc_auth_executor.go
- backend/internal/flow/core/task_execution_node.go
- backend/internal/flow/executor/federated_auth_resolver_executor.go
- backend/internal/flow/executor/credential_setter.go
- backend/internal/flow/executor/sms_executor.go
- docs/content/community/contributing/contributing-code/backend-development/observability.md
- backend/internal/flow/executor/identifying_executor.go
- frontend/pnpm-workspace.yaml
- backend/internal/flow/executor/basic_auth_executor.go
- backend/internal/flow/executor/attribute_uniqueness_validator_test.go
- frontend/apps/thunder-gate/src/pages/AcceptInvitePage.tsx
- backend/internal/flow/executor/ou_executor_test.go
- backend/internal/flow/executor/email_executor.go
- README.md
- backend/internal/flow/executor/provisioning_executor.go
- backend/internal/flow/executor/sms_executor_test.go
- backend/internal/flow/executor/user_type_resolver.go
- samples/api/postman/authentication/environment.json
- backend/internal/flow/executor/basic_auth_executor_test.go
- backend/internal/flow/core/task_execution_node_test.go
- samples/api/postman/authentication/README.md
- backend/internal/flow/flowexec/engine_event_enabled_test.go
- tests/e2e/tests/sample-app-authentication/README-MFA.md
- docs/content/guides/key-concepts/authentication/passwordless/passkeys.mdx
- backend/internal/oauth/oauth2/authz/handler_test.go
- tests/integration/flow/authentication/conditional_exec_auth_test.go
- tests/integration/flow/authentication/prompt_actions_test.go
- tests/integration/flow/registration/google_registration_test.go
- backend/internal/flow/executor/http_request_executor_test.go
- tests/integration/flow/authentication/multi_action_input_binding_test.go
- backend/internal/flow/executor/oidc_auth_executor_test.go
- tests/integration/flow/authentication/basic_auth_test.go
- backend/internal/flow/executor/auth_assert_executor_test.go
- backend/internal/oauth/oauth2/authz/service_test.go
- backend/internal/flow/executor/user_type_resolver_test.go
- backend/internal/flow/executor/provisioning_executor_test.go
- docs/static/api/next/combined.yaml
- backend/internal/flow/flowexec/flowStoreInterface_mock_test.go
🚧 Files skipped from review as they are similar to previous changes (44)
- backend/internal/flow/executor/authz_executor.go
- ARCHITECTURE.md
- backend/internal/flow/core/model.go
- backend/internal/flow/executor/permission_validator.go
- backend/internal/flow/core/executor_test.go
- backend/internal/flow/executor/passkey_executor.go
- backend/internal/flow/executor/attribute_uniqueness_validator.go
- backend/internal/flow/executor/invite_executor_test.go
- backend/internal/flow/executor/http_request_executor.go
- backend/internal/flow/executor/permission_validator_test.go
- backend/internal/flow/executor/identifying_executor_test.go
- backend/internal/oauth/oauth2/authz/service.go
- backend/internal/flow/executor/attribute_collector_test.go
- backend/internal/system/log/constants.go
- backend/internal/flow/executor/email_executor_test.go
- backend/internal/flow/executor/ou_resolver_executor_test.go
- backend/internal/flow/executor/sms_auth_executor_test.go
- backend/internal/flow/executor/auth_assert_executor.go
- backend/internal/flow/executor/invite_executor.go
- tests/integration/flow/registration/google_registration_with_role_group_test.go
- backend/internal/flow/executor/sms_auth_executor.go
- backend/internal/flow/flowexec/error_constants.go
- backend/internal/system/observability/event/datakeys.go
- tests/integration/flow/authentication/github_auth_test.go
- tests/integration/flow/registration/http_request_executor_runtime_data_test.go
- tests/integration/flow/common/model.go
- backend/internal/flow/flowexec/model_test.go
- tests/integration/flow/registration/ou_registration_test.go
- tests/integration/flow/registration/github_registration_test.go
- backend/internal/flow/flowexec/store_test.go
- tests/integration/flow/registration/sms_registration_test.go
- tests/integration/testutils/models.go
- backend/internal/flow/executor/consent_executor_test.go
- tests/integration/flow/authentication/attribute_collect_test.go
- api/flow-execution.yaml
- backend/internal/flow/core/prompt_node_test.go
- tests/integration/testutils/oauth2_utils.go
- backend/internal/flow/flowexec/model.go
- backend/internal/flow/flowexec/store.go
- samples/api/postman/authentication/authentication_demo.json
- backend/internal/flow/executor/oauth_executor.go
- tests/integration/flow/common/utils.go
- backend/internal/flow/flowexec/FlowExecServiceInterface_mock_test.go
- backend/internal/flow/executor/passkey_executor_test.go
a47c727 to
70de5b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/integration/oauth/authz/authz_test.go (1)
841-842:⚠️ Potential issue | 🟡 MinorAssertion message is stale for
ExecutionIDcheck.Line 841 validates
flowStep.ExecutionID, but Line 842 still says “flow ID”, which can mislead debugging output.Suggested patch
if flowStep.ExecutionID == "" { - ts.T().Fatalf("Expected flow ID, got empty string") + ts.T().Fatalf("Expected execution ID, got empty string") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/oauth/authz/authz_test.go` around lines 841 - 842, Update the assertion message for the ExecutionID check in the test (the if block that inspects flowStep.ExecutionID in authz_test.go) so it accurately references "ExecutionID" instead of the stale "flow ID" wording; modify the Fatalf call message to say something like "Expected ExecutionID, got empty string" to improve clarity when the test fails.
🧹 Nitpick comments (5)
backend/internal/flow/executor/passkey_executor_test.go (1)
132-144: LGTM! Change is consistent with the production code rename.The helper function correctly sets
ExecutionIDinstead of the oldFlowIDfield, aligning with the broader rename incore.NodeContextand the production code inpasskey_executor.gothat usesctx.ExecutionIDfor logging.Optional nit: The constant
testPasskeyFlowID(line 44) could be renamed totestPasskeyExecutionIDfor naming consistency with the new field name it populates.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/executor/passkey_executor_test.go` around lines 132 - 144, Rename the test constant testPasskeyFlowID to testPasskeyExecutionID and update all references to it (e.g., in createPasskeyNodeContext) so the constant name matches the core.NodeContext.ExecutionID field; ensure tests and any uses in passkey_executor_test.go and other test files are updated to refer to testPasskeyExecutionID and still populate ExecutionID in core.NodeContext, keeping runtime behavior unchanged while restoring naming consistency with passkey_executor.go which logs ctx.ExecutionID.backend/internal/flow/executor/ou_resolver_executor_test.go (1)
83-84: Optional: reduce repeated execution-id literals in test setup.The suite repeats
"test-flow"/"flow-123"in many contexts. Extracting constants (or a small context builder helper) would reduce rename churn in future refactors.♻️ Suggested cleanup
+const testExecutionID = "test-flow" +const promptExecutionID = "flow-123" ... - ExecutionID: "test-flow", + ExecutionID: testExecutionID, ... - ExecutionID: "flow-123", + ExecutionID: promptExecutionID,Also applies to: 110-111, 133-133, 152-153, 170-170, 189-189, 213-214, 229-230, 247-247, 270-270, 298-298, 326-326, 359-359, 391-391, 415-415, 443-443, 473-473, 497-497, 522-522, 547-547, 575-575
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/executor/ou_resolver_executor_test.go` around lines 83 - 84, Extract repeated execution-id literals into shared test constants and optionally a small builder: define consts like testExecutionID = "test-flow" and testFlowID = "flow-123" at top of backend/internal/flow/executor/ou_resolver_executor_test.go (or in a test helper), then replace inline literals used in ExecutionID fields and any places using "flow-123" with those constants; alternatively provide a small helper (e.g., newTestExecutionContext(httpCtx, testExecutionID)) to construct the Context/Execution objects used across tests to remove duplication. Ensure references to ExecutionID and httpCtx in existing test cases are updated to use the new constants/helper so renames are centralized.backend/internal/flow/flowexec/model_test.go (1)
603-626: Pre-existing inconsistency:AppIDset to"test-flow-id"instead of"test-app-id".Line 604 (unchanged in this PR) sets
AppID: "test-flow-id", which appears inconsistent with other tests that use"test-app-id"for theAppIDfield. This doesn't cause test failures sinceAppIDisn't asserted in this test, but it reduces test clarity.The
ExecutionIDrename at lines 617-618 and assertion at line 626 are correct.Would you like me to open an issue to track fixing the
AppIDvalue for consistency across tests?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/model_test.go` around lines 603 - 626, The test creates a flowContextContent with AppID set to "test-flow-id" which is inconsistent with other tests; update the AppID value to "test-app-id" in the flowContextContent initialization in model_test.go so the constructed FlowContextDB.Context JSON uses the expected app identifier; this change is local to the test that calls FlowContextDB.ToEngineContext and verifies resultCtx.ExecutionID, so adjust the AppID field there (flowContextContent -> AppID) to "test-app-id" for consistency.tests/integration/flow/authentication/github_auth_test.go (1)
373-373: PreferexecutionID(lowerCamelCase) for local vars.Line 373, Line 418, and Line 436 use
ExecutionIDas a local variable. Please rename toexecutionIDto follow Go naming style for unexported identifiers.♻️ Suggested patch
- ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - completeFlowStep, err := common.CompleteFlow(ExecutionID, inputs, "") + completeFlowStep, err := common.CompleteFlow(executionID, inputs, "") ... - ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - _, err = common.CompleteFlow(ExecutionID, inputs, "") + _, err = common.CompleteFlow(executionID, inputs, "") ... - ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - flowStep, err = common.CompleteFlow(ExecutionID, inputs, "") + flowStep, err = common.CompleteFlow(executionID, inputs, "")Based on learnings: In asgardeo/thunder Go code, unexported identifiers should use lowerCamelCase, while exported identifiers use CamelCase.
Also applies to: 389-389, 418-418, 425-425, 436-436, 443-443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/github_auth_test.go` at line 373, Rename the local variable(s) currently named ExecutionID to executionID to follow Go's lowerCamelCase convention for unexported identifiers; specifically update the assignments like "ExecutionID := flowStep.ExecutionID" (and the other occurrences where a local variable is assigned from flowStep.ExecutionID) to "executionID := flowStep.ExecutionID" and adjust any subsequent references in the same test functions so they use executionID instead of ExecutionID.tests/integration/flow/authentication/conditional_exec_auth_test.go (1)
383-383: Use lowerCamelCase for localexecutionIDvariables.Line 383 and Line 428 introduce
ExecutionIDas a local variable. For unexported/local identifiers, use lowerCamelCase (executionID) for Go style consistency.♻️ Suggested patch
- ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - flowStep, err = common.CompleteFlow(ExecutionID, inputs, "") + flowStep, err = common.CompleteFlow(executionID, inputs, "") ... - ExecutionID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - flowStep, err = common.CompleteFlow(ExecutionID, inputs, "") + flowStep, err = common.CompleteFlow(executionID, inputs, "") ... - flowStep, err = common.CompleteFlow(ExecutionID, ouInputs, "") + flowStep, err = common.CompleteFlow(executionID, ouInputs, "")Based on learnings: In asgardeo/thunder Go code, unexported identifiers should use lowerCamelCase, while exported identifiers use CamelCase.
Also applies to: 394-394, 428-428, 441-441, 452-452
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/conditional_exec_auth_test.go` at line 383, Local variables named "ExecutionID" are using exported-style CamelCase; rename them to lowerCamelCase "executionID" wherever declared and used (e.g., the local assignment from flowStep.ExecutionID and subsequent uses around the affected blocks) to follow Go style for unexported identifiers—update the declarations and all references in the same functions/blocks (including the other occurrences noted at lines 394, 428, 441, 452) so the code compiles and behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/flow/flowexec/handler.go`:
- Around line 53-68: Update the public API docs to reflect the request/response
field rename from "flowId"/"flow_id" to "executionId"/"execution_id" for the
/flow/execute endpoint: update docs/content/apis.mdx (the /flow/execute
contract) and any guides under docs/content/guides/ that mention flow execution,
include example request and response JSON showing the new ExecutionID field
(matching the server-side FlowResponse usage of ExecutionID), add a clear
migration note for existing clients describing the rename and how to map old
fields to the new executionId, and ensure any SDK or sample snippets in docs are
updated to use executionId consistently.
In `@samples/apps/react-vanilla-sample/src/pages/LoginPage.tsx`:
- Around line 266-267: In processAuthResponse, avoid using the state variable
executionId for immediate chained single-action submission because
setExecutionId is async and the closure may hold a stale value; instead capture
data.executionId into a local const (e.g., const responseExecutionId =
data.executionId || '') before calling setExecutionId(responseExecutionId), and
then pass responseExecutionId to submitAuthDecision(responseExecutionId,
singleAction.ref) so the immediate auto-step uses the fresh response value
rather than the possibly stale executionId from state.
---
Duplicate comments:
In `@tests/integration/oauth/authz/authz_test.go`:
- Around line 841-842: Update the assertion message for the ExecutionID check in
the test (the if block that inspects flowStep.ExecutionID in authz_test.go) so
it accurately references "ExecutionID" instead of the stale "flow ID" wording;
modify the Fatalf call message to say something like "Expected ExecutionID, got
empty string" to improve clarity when the test fails.
---
Nitpick comments:
In `@backend/internal/flow/executor/ou_resolver_executor_test.go`:
- Around line 83-84: Extract repeated execution-id literals into shared test
constants and optionally a small builder: define consts like testExecutionID =
"test-flow" and testFlowID = "flow-123" at top of
backend/internal/flow/executor/ou_resolver_executor_test.go (or in a test
helper), then replace inline literals used in ExecutionID fields and any places
using "flow-123" with those constants; alternatively provide a small helper
(e.g., newTestExecutionContext(httpCtx, testExecutionID)) to construct the
Context/Execution objects used across tests to remove duplication. Ensure
references to ExecutionID and httpCtx in existing test cases are updated to use
the new constants/helper so renames are centralized.
In `@backend/internal/flow/executor/passkey_executor_test.go`:
- Around line 132-144: Rename the test constant testPasskeyFlowID to
testPasskeyExecutionID and update all references to it (e.g., in
createPasskeyNodeContext) so the constant name matches the
core.NodeContext.ExecutionID field; ensure tests and any uses in
passkey_executor_test.go and other test files are updated to refer to
testPasskeyExecutionID and still populate ExecutionID in core.NodeContext,
keeping runtime behavior unchanged while restoring naming consistency with
passkey_executor.go which logs ctx.ExecutionID.
In `@backend/internal/flow/flowexec/model_test.go`:
- Around line 603-626: The test creates a flowContextContent with AppID set to
"test-flow-id" which is inconsistent with other tests; update the AppID value to
"test-app-id" in the flowContextContent initialization in model_test.go so the
constructed FlowContextDB.Context JSON uses the expected app identifier; this
change is local to the test that calls FlowContextDB.ToEngineContext and
verifies resultCtx.ExecutionID, so adjust the AppID field there
(flowContextContent -> AppID) to "test-app-id" for consistency.
In `@tests/integration/flow/authentication/conditional_exec_auth_test.go`:
- Line 383: Local variables named "ExecutionID" are using exported-style
CamelCase; rename them to lowerCamelCase "executionID" wherever declared and
used (e.g., the local assignment from flowStep.ExecutionID and subsequent uses
around the affected blocks) to follow Go style for unexported identifiers—update
the declarations and all references in the same functions/blocks (including the
other occurrences noted at lines 394, 428, 441, 452) so the code compiles and
behavior remains identical.
In `@tests/integration/flow/authentication/github_auth_test.go`:
- Line 373: Rename the local variable(s) currently named ExecutionID to
executionID to follow Go's lowerCamelCase convention for unexported identifiers;
specifically update the assignments like "ExecutionID := flowStep.ExecutionID"
(and the other occurrences where a local variable is assigned from
flowStep.ExecutionID) to "executionID := flowStep.ExecutionID" and adjust any
subsequent references in the same test functions so they use executionID instead
of ExecutionID.
🪄 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
Run ID: 68314775-8dac-4d2e-8376-41ee7dad318a
⛔ Files ignored due to path filters (13)
backend/tests/mocks/flow/flowexecmock/FlowExecServiceInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/flowStoreInterface_mock.gois excluded by!**/*_mock.godocs/__backup__/README.mdis excluded by!docs/__backup__/**docs/__backup__/authentication/server-orchestrated-flow/authentication.mdis excluded by!docs/__backup__/**docs/__backup__/authorization/flow-based-authorization.mdis excluded by!docs/__backup__/**docs/__backup__/flows/flow-execution.mdis excluded by!docs/__backup__/**docs/__backup__/observability/events.mdis excluded by!docs/__backup__/**docs/__backup__/registration/server-orchestrated-flow/registration.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/features/public-clients.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/authorization-code.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/client-credentials.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/token-exchange.mdis excluded by!docs/__backup__/**frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (112)
ARCHITECTURE.mdREADME.mdapi/flow-execution.yamlbackend/internal/flow/core/executor.gobackend/internal/flow/core/executor_test.gobackend/internal/flow/core/model.gobackend/internal/flow/core/node_test.gobackend/internal/flow/core/prompt_node.gobackend/internal/flow/core/prompt_node_test.gobackend/internal/flow/core/representation_node_test.gobackend/internal/flow/core/task_execution_node.gobackend/internal/flow/core/task_execution_node_test.gobackend/internal/flow/executor/attribute_collector.gobackend/internal/flow/executor/attribute_collector_test.gobackend/internal/flow/executor/attribute_uniqueness_validator.gobackend/internal/flow/executor/attribute_uniqueness_validator_test.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/auth_assert_executor_test.gobackend/internal/flow/executor/authz_executor.gobackend/internal/flow/executor/authz_executor_test.gobackend/internal/flow/executor/basic_auth_executor.gobackend/internal/flow/executor/basic_auth_executor_test.gobackend/internal/flow/executor/consent_executor.gobackend/internal/flow/executor/consent_executor_test.gobackend/internal/flow/executor/credential_setter.gobackend/internal/flow/executor/credential_setter_test.gobackend/internal/flow/executor/email_executor.gobackend/internal/flow/executor/email_executor_test.gobackend/internal/flow/executor/federated_auth_resolver_executor.gobackend/internal/flow/executor/federated_auth_resolver_executor_test.gobackend/internal/flow/executor/http_request_executor.gobackend/internal/flow/executor/http_request_executor_test.gobackend/internal/flow/executor/identifying_executor.gobackend/internal/flow/executor/identifying_executor_test.gobackend/internal/flow/executor/invite_executor.gobackend/internal/flow/executor/invite_executor_test.gobackend/internal/flow/executor/oauth_executor.gobackend/internal/flow/executor/oauth_executor_test.gobackend/internal/flow/executor/oidc_auth_executor.gobackend/internal/flow/executor/oidc_auth_executor_test.gobackend/internal/flow/executor/ou_executor.gobackend/internal/flow/executor/ou_executor_test.gobackend/internal/flow/executor/ou_resolver_executor.gobackend/internal/flow/executor/ou_resolver_executor_test.gobackend/internal/flow/executor/passkey_executor.gobackend/internal/flow/executor/passkey_executor_test.gobackend/internal/flow/executor/permission_validator.gobackend/internal/flow/executor/permission_validator_test.gobackend/internal/flow/executor/provisioning_executor.gobackend/internal/flow/executor/provisioning_executor_test.gobackend/internal/flow/executor/sms_auth_executor.gobackend/internal/flow/executor/sms_auth_executor_test.gobackend/internal/flow/executor/sms_executor.gobackend/internal/flow/executor/sms_executor_test.gobackend/internal/flow/executor/user_type_resolver.gobackend/internal/flow/executor/user_type_resolver_test.gobackend/internal/flow/flowexec/FlowExecServiceInterface_mock_test.gobackend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/engine_event_enabled_test.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/flow/flowexec/flowStoreInterface_mock_test.gobackend/internal/flow/flowexec/handler.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/model_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/flowexec/store.gobackend/internal/flow/flowexec/store_test.gobackend/internal/oauth/oauth2/authz/handler_test.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/system/log/constants.gobackend/internal/system/observability/event/datakeys.godocs/content/community/contributing/contributing-code/backend-development/observability.mddocs/content/guides/key-concepts/authentication/passwordless/passkeys.mdxdocs/static/api/next/combined.yamlfrontend/apps/thunder-gate/src/pages/AcceptInvitePage.tsxfrontend/pnpm-workspace.yamlsamples/api/postman/authentication/README.mdsamples/api/postman/authentication/authentication_demo.jsonsamples/api/postman/authentication/environment.jsonsamples/apps/react-vanilla-sample/src/pages/LoginPage.tsxsamples/apps/react-vanilla-sample/src/services/authService.tstests/e2e/tests/sample-app-authentication/README-MFA.mdtests/e2e/utils/thunder-setup/mfa-setup.tstests/integration/flow/authentication/assurance_test.gotests/integration/flow/authentication/attribute_collect_test.gotests/integration/flow/authentication/authz_test.gotests/integration/flow/authentication/basic_auth_test.gotests/integration/flow/authentication/conditional_exec_auth_test.gotests/integration/flow/authentication/github_auth_test.gotests/integration/flow/authentication/google_auth_test.gotests/integration/flow/authentication/multi_action_input_binding_test.gotests/integration/flow/authentication/prompt_actions_test.gotests/integration/flow/authentication/sensitive_input_cleanup_test.gotests/integration/flow/authentication/sms_auth_test.gotests/integration/flow/authentication/verbose_meta_test.gotests/integration/flow/common/model.gotests/integration/flow/common/utils.gotests/integration/flow/registration/basic_registration_test.gotests/integration/flow/registration/github_registration_test.gotests/integration/flow/registration/google_registration_test.gotests/integration/flow/registration/google_registration_with_role_group_test.gotests/integration/flow/registration/http_request_executor_runtime_data_test.gotests/integration/flow/registration/ou_registration_test.gotests/integration/flow/registration/sms_registration_test.gotests/integration/oauth/authz/authz_test.gotests/integration/oauth/token/refresh_token_test.gotests/integration/oauth/userinfo/userinfo_test.gotests/integration/testutils/models.gotests/integration/testutils/oauth2_utils.go
✅ Files skipped from review due to trivial changes (42)
- backend/internal/flow/core/task_execution_node.go
- ARCHITECTURE.md
- backend/internal/flow/executor/credential_setter.go
- backend/internal/flow/executor/oidc_auth_executor.go
- frontend/apps/thunder-gate/src/pages/AcceptInvitePage.tsx
- samples/api/postman/authentication/environment.json
- frontend/pnpm-workspace.yaml
- backend/internal/flow/core/executor.go
- backend/internal/flow/core/prompt_node.go
- backend/internal/flow/executor/attribute_collector.go
- backend/internal/flow/executor/attribute_uniqueness_validator.go
- backend/internal/flow/executor/attribute_uniqueness_validator_test.go
- backend/internal/flow/executor/auth_assert_executor.go
- backend/internal/flow/executor/consent_executor.go
- backend/internal/flow/executor/federated_auth_resolver_executor_test.go
- backend/internal/flow/executor/http_request_executor.go
- backend/internal/flow/executor/identifying_executor.go
- backend/internal/flow/executor/ou_executor_test.go
- backend/internal/flow/executor/provisioning_executor.go
- tests/e2e/utils/thunder-setup/mfa-setup.ts
- backend/internal/flow/executor/authz_executor_test.go
- backend/internal/flow/executor/basic_auth_executor_test.go
- backend/internal/flow/executor/email_executor_test.go
- backend/internal/flow/executor/oauth_executor.go
- backend/internal/flow/flowexec/engine_event_enabled_test.go
- docs/content/guides/key-concepts/authentication/passwordless/passkeys.mdx
- tests/integration/flow/authentication/multi_action_input_binding_test.go
- tests/e2e/tests/sample-app-authentication/README-MFA.md
- backend/internal/flow/executor/http_request_executor_test.go
- backend/internal/flow/executor/identifying_executor_test.go
- tests/integration/flow/authentication/sms_auth_test.go
- backend/internal/flow/core/prompt_node_test.go
- backend/internal/flow/executor/provisioning_executor_test.go
- backend/internal/flow/executor/auth_assert_executor_test.go
- docs/content/community/contributing/contributing-code/backend-development/observability.md
- tests/integration/flow/common/model.go
- tests/integration/flow/authentication/verbose_meta_test.go
- backend/internal/flow/executor/authz_executor.go
- tests/integration/oauth/userinfo/userinfo_test.go
- tests/integration/flow/registration/google_registration_test.go
- backend/internal/flow/executor/user_type_resolver_test.go
- samples/api/postman/authentication/authentication_demo.json
🚧 Files skipped from review as they are similar to previous changes (52)
- backend/internal/flow/core/node_test.go
- README.md
- backend/internal/flow/core/executor_test.go
- backend/internal/flow/executor/email_executor.go
- backend/internal/flow/executor/federated_auth_resolver_executor.go
- backend/internal/flow/executor/ou_resolver_executor.go
- backend/internal/flow/executor/permission_validator.go
- backend/internal/flow/executor/ou_executor.go
- backend/internal/flow/core/model.go
- backend/internal/flow/executor/basic_auth_executor.go
- backend/internal/flow/executor/invite_executor_test.go
- backend/internal/flow/executor/passkey_executor.go
- backend/internal/flow/executor/permission_validator_test.go
- backend/internal/flow/executor/sms_auth_executor.go
- backend/internal/flow/executor/sms_executor.go
- backend/internal/flow/flowexec/engine.go
- backend/internal/system/log/constants.go
- backend/internal/oauth/oauth2/authz/service.go
- samples/api/postman/authentication/README.md
- backend/internal/flow/executor/sms_executor_test.go
- tests/integration/flow/authentication/google_auth_test.go
- tests/integration/flow/authentication/sensitive_input_cleanup_test.go
- backend/internal/flow/executor/consent_executor_test.go
- backend/internal/flow/executor/invite_executor.go
- backend/internal/oauth/oauth2/constants/constants.go
- backend/internal/flow/executor/user_type_resolver.go
- backend/internal/oauth/oauth2/authz/service_test.go
- backend/internal/flow/core/task_execution_node_test.go
- backend/internal/flow/flowexec/store_test.go
- tests/integration/flow/authentication/assurance_test.go
- tests/integration/flow/authentication/attribute_collect_test.go
- backend/internal/system/observability/event/datakeys.go
- tests/integration/flow/authentication/authz_test.go
- tests/integration/flow/authentication/prompt_actions_test.go
- tests/integration/flow/authentication/basic_auth_test.go
- api/flow-execution.yaml
- backend/internal/flow/executor/oauth_executor_test.go
- backend/internal/flow/executor/attribute_collector_test.go
- backend/internal/flow/flowexec/model.go
- tests/integration/flow/registration/ou_registration_test.go
- tests/integration/flow/registration/sms_registration_test.go
- tests/integration/flow/registration/github_registration_test.go
- samples/apps/react-vanilla-sample/src/services/authService.ts
- tests/integration/testutils/oauth2_utils.go
- backend/internal/flow/executor/oidc_auth_executor_test.go
- backend/internal/flow/executor/sms_auth_executor_test.go
- backend/internal/flow/flowexec/FlowExecServiceInterface_mock_test.go
- tests/integration/flow/common/utils.go
- backend/internal/flow/flowexec/store.go
- backend/internal/flow/flowexec/service.go
- docs/static/api/next/combined.yaml
- backend/internal/oauth/oauth2/authz/handler_test.go
70de5b8 to
7c34718
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/e2e/tests/sample-app-authentication/README-MFA.md (1)
136-136:⚠️ Potential issue | 🟡 MinorFix heading capitalization to satisfy Vale title-case lint.
The headings at Line 136 and Line 515 violate
WSO2-IAM.TitleCaseTitles.💡 Suggested fix
-### Create the notification sender +### Create the Notification Sender -### Create a user with mobile No +### Create a User with Mobile NumberAlso applies to: 515-515
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/tests/sample-app-authentication/README-MFA.md` at line 136, Change the heading "Create the notification sender" (and the other identical heading at the second occurrence) to Title Case to satisfy the WSO2-IAM.TitleCaseTitles lint rule; replace the lowercase words so the headings read "Create the Notification Sender" in both places (update the heading text in the README-MFA.md where the markdown heading text "Create the notification sender" appears).tests/integration/flow/registration/google_registration_with_role_group_test.go (1)
440-452:⚠️ Potential issue | 🟠 MajorPublic flow-execution identifier rename needs docs coverage.
🔴 Documentation Required: This PR introduces a user-facing change (e.g., new/modified API endpoint, configuration option, authentication flow, or SDK behavior) but does not include corresponding documentation updates under
docs/. Please update the relevant documentation before merging.Specifically, the
/flow/executecontract shift toexecutionId(validated at Line 440 and used at Line 452) should be documented indocs/content/apis.mdxand a flow-execution guide underdocs/content/guides/with migration examples fromflowIdtoexecutionId.As per coding guidelines "If ANY of the above are detected and the PR does NOT include corresponding updates under
docs/... flag this as a major issue ..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/registration/google_registration_with_role_group_test.go` around lines 440 - 452, The tests reveal a public contract change: the /flow/execute parameter was renamed from flowId to executionId (referenced in the test via flowStep.ExecutionID and the call to common.CompleteFlow), so add documentation updates: modify docs/content/apis.mdx to describe the new /flow/execute request/response schema using executionId, and create or update a guide under docs/content/guides/ showing the migration steps (example requests and responses) converting old flowId usages to executionId; ensure the docs include the validation behavior and an example mirroring the test flow that builds redirectURLStr and completes the flow with inputs {"code": "..."} so consumers can migrate safely.
♻️ Duplicate comments (3)
backend/internal/oauth/oauth2/authz/handler_test.go (1)
200-202:⚠️ Potential issue | 🟠 MajorDocumentation update is still required for the authorize/login query param rename.
Line 202 confirms the OAuth authorize/login correlation parameter moved to
executionId(fromflowId), which is a user-facing auth flow contract change.
🔴 Documentation Required: This PR introduces a user-facing change
(e.g., new/modified API endpoint, configuration option, authentication flow, or SDK behavior)
but does not include corresponding documentation updates underdocs/.
Please update the relevant documentation before merging.Please document this rename in
docs/content/apis.mdx(OAuth authorize behavior) and the related authorization/login guide underdocs/content/guides/, including migration notes and updated URL examples.As per coding guidelines: "If ANY of the above are detected and the PR does NOT include corresponding updates under
docs/, flag this as a major issue."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/oauth/oauth2/authz/handler_test.go` around lines 200 - 202, The test shows the OAuth correlation query param was renamed from flowId to executionId (see oauth2const.ExecutionID and references to flowId), but docs were not updated; update docs/content/apis.mdx (OAuth authorize behavior) and the relevant guide(s) under docs/content/guides/ to document the rename, include migration notes, updated example authorize/login URLs showing executionId instead of flowId, and mention the change in any SDK or config examples that reference flowId so callers know to use executionId.samples/api/postman/authentication/authentication_demo.json (2)
58-64:⚠️ Potential issue | 🟡 MinorFail fast if redirect is missing
executionIdbefore persisting variables.If
executionIdis absent, the collection proceeds and fails later in/flow/execute, which makes diagnosis harder.Suggested guard
" const executionId = url.searchParams.get(\"executionId\");", +" pm.test(\"Redirect includes executionId\", function() {", +" pm.expect(executionId).to.be.a(\"string\").and.not.empty;", +" });", " ", " pm.environment.set(\"authId\", authId);", -" pm.environment.set(\"executionId\", executionId);", +" if (executionId) {", +" pm.environment.set(\"executionId\", executionId);", +" }"," const executionId = url.searchParams.get(\"executionId\");", +" pm.test(\"Redirect includes executionId\", function() {", +" pm.expect(executionId).to.be.a(\"string\").and.not.empty;", +" });", " ", " pm.collectionVariables.set(\"auth_std_auth_id\", authId);", -" pm.collectionVariables.set(\"auth_std_flow_id\", executionId);", +" if (executionId) {", +" pm.collectionVariables.set(\"auth_std_flow_id\", executionId);", +" }",Also applies to: 3401-3407
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/api/postman/authentication/authentication_demo.json` around lines 58 - 64, Add a guard to fail fast when the redirect URL is missing executionId: check the result of url.searchParams.get("executionId") (the executionId variable) before calling pm.environment.set; if it's null/empty, throw or log a clear error and stop execution (e.g., by calling pm.test/pm.expect or equivalent) so you do not persist authId/executionId and downstream /flow/execute failures are avoided.
2858-2858:⚠️ Potential issue | 🟠 MajorRemove inline comment from raw body; this payload is invalid JSON.
Line 2858 includes
// "mobileNumber"..., which breaks JSON parsing for the sample request body.Suggested fix
-"raw": "{\n \"executionId\": \"{{exec_flow_id}}\",\n \"inputs\": {\n \"username\": \"{{demoLoginUser1Username}}\",\n \"password\": \"{{demoLoginUser1Password}}\"\n // \"mobileNumber\": \"{{demoLoginUser1Mobile}}\"\n\n },\n \"action\": \"action_001\"\n}\n", +"raw": "{\n \"executionId\": \"{{exec_flow_id}}\",\n \"inputs\": {\n \"username\": \"{{demoLoginUser1Username}}\",\n \"password\": \"{{demoLoginUser1Password}}\"\n },\n \"action\": \"action_001\"\n}\n",Run this read-only verification to confirm the target raw body parses as valid JSON:
#!/bin/bash set -euo pipefail python - <<'PY' import json from pathlib import Path path = Path("samples/api/postman/authentication/authentication_demo.json") collection = json.loads(path.read_text()) def walk(items): for item in items: yield item for child in item.get("item", []): yield from walk([child]) target_name = "02.02 - Complete Authentication (Username & Password)" for item in walk(collection["item"]): if item.get("name") == target_name: raw = item["request"]["body"]["raw"] try: json.loads(raw) print("VALID") except json.JSONDecodeError as exc: print(f"INVALID: {exc}") break else: print("Target request not found") PYExpected result:
VALIDafter the fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/api/postman/authentication/authentication_demo.json` at line 2858, The raw request body for the Postman request named "02.02 - Complete Authentication (Username & Password)" contains an inline JavaScript-style comment (// "mobileNumber"...) which makes the JSON invalid; remove that commented line (or replace it with a valid JSON entry like removing the field entirely or setting "mobileNumber": null) inside the request object's "raw" string in samples/api/postman/authentication/authentication_demo.json so the raw string parses as valid JSON.
🧹 Nitpick comments (3)
samples/apps/react-vanilla-sample/src/pages/LoginPage.tsx (1)
104-117: Consider a legacy session key fallback during rollout.At Line 117, state restores only from
executionId. If a user started auth before deployment (storedflowId) and returns from IdP after deployment, Line 760 can submit with an empty id. For smoother rollout, keep a temporary fallback to the legacy key.Proposed compatibility patch
const START_INIT_KEY = 'startInit'; const EXECUTION_ID_KEY = 'executionId'; +const LEGACY_FLOW_ID_KEY = 'flowId'; -const [executionId, setExecutionId] = useState<string>(sessionStorage.getItem(EXECUTION_ID_KEY) || ''); +const [executionId, setExecutionId] = useState<string>( + sessionStorage.getItem(EXECUTION_ID_KEY) || + sessionStorage.getItem(LEGACY_FLOW_ID_KEY) || + '' +);Also applies to: 256-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/apps/react-vanilla-sample/src/pages/LoginPage.tsx` around lines 104 - 117, The executionId state is only initialized from EXECUTION_ID_KEY ('executionId') and can be empty if a user began auth with the legacy key 'flowId'; update the initialization and restore logic for executionId (the useState for executionId and any places that read/setExecutionId/sessionStorage) to fall back to sessionStorage.getItem('flowId') when sessionStorage.getItem(EXECUTION_ID_KEY) is falsy, and when writing the id persist it to both keys (EXECUTION_ID_KEY and 'flowId') so older in-flight sessions remain compatible during rollout.tests/integration/flow/registration/google_registration_test.go (1)
506-507: Consider renaming localflowIDvariables toexecutionIDfor clarity.These locals now store execution identifiers; renaming would avoid semantic drift in test code.
✏️ Suggested cleanup
- flowID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - completeFlowStep, err := common.CompleteFlow(flowID, inputs, "") + completeFlowStep, err := common.CompleteFlow(executionID, inputs, "") ... - flowID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - _, err = common.CompleteFlow(flowID, inputs, "") + _, err = common.CompleteFlow(executionID, inputs, "") ... - flowID := flowStep.ExecutionID + executionID := flowStep.ExecutionID ... - flowStep, err = common.CompleteFlow(flowID, inputs, "") + flowStep, err = common.CompleteFlow(executionID, inputs, "")Also applies to: 571-572, 589-590
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/registration/google_registration_test.go` around lines 506 - 507, Rename the local variables named flowID to executionID where they store execution identifiers (e.g., replace occurrences like "flowID := flowStep.ExecutionID" and any subsequent uses in this test file and the other mentioned spots) so the variable name matches the value source (flowStep.ExecutionID) and avoids semantic drift; update all references in tests (including the other occurrences around lines with flowStep.Data.RedirectURL) to use executionID consistently.tests/integration/flow/authentication/github_auth_test.go (1)
373-374: Optional: use a consistent local name for execution identifier variables.Using one lowercase local naming pattern (e.g.,
executionID) improves readability across this file.Also applies to: 418-419, 436-437
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/flow/authentication/github_auth_test.go` around lines 373 - 374, Rename the local variable ExecutionID to use a consistent lowercase local naming (e.g., executionID) throughout the test file; locate the assignments like "ExecutionID := flowStep.ExecutionID" (and the similar instances around the other spots noted) and update the local variable name and all its subsequent usages to executionID to match the lowercase pattern and improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/flow/executor/invite_executor_test.go`:
- Around line 91-92: Tests assert the invite URL now uses the query parameter
rename (flowId → executionId) via the invite link runtime data
(common.RuntimeKeyInviteLink); add documentation updates reflecting this
user-facing change by updating the API reference and invite/onboarding guide to
document the new executionId parameter, provide migration guidance for clients
(how to map old flowId values to executionId or handle both temporarily), and
update any example invite URLs and SDK/snippet samples that generate or parse
the invite link so they use executionId consistently; ensure the docs mention
the migration impact and timeline.
---
Outside diff comments:
In `@tests/e2e/tests/sample-app-authentication/README-MFA.md`:
- Line 136: Change the heading "Create the notification sender" (and the other
identical heading at the second occurrence) to Title Case to satisfy the
WSO2-IAM.TitleCaseTitles lint rule; replace the lowercase words so the headings
read "Create the Notification Sender" in both places (update the heading text in
the README-MFA.md where the markdown heading text "Create the notification
sender" appears).
In
`@tests/integration/flow/registration/google_registration_with_role_group_test.go`:
- Around line 440-452: The tests reveal a public contract change: the
/flow/execute parameter was renamed from flowId to executionId (referenced in
the test via flowStep.ExecutionID and the call to common.CompleteFlow), so add
documentation updates: modify docs/content/apis.mdx to describe the new
/flow/execute request/response schema using executionId, and create or update a
guide under docs/content/guides/ showing the migration steps (example requests
and responses) converting old flowId usages to executionId; ensure the docs
include the validation behavior and an example mirroring the test flow that
builds redirectURLStr and completes the flow with inputs {"code": "..."} so
consumers can migrate safely.
---
Duplicate comments:
In `@backend/internal/oauth/oauth2/authz/handler_test.go`:
- Around line 200-202: The test shows the OAuth correlation query param was
renamed from flowId to executionId (see oauth2const.ExecutionID and references
to flowId), but docs were not updated; update docs/content/apis.mdx (OAuth
authorize behavior) and the relevant guide(s) under docs/content/guides/ to
document the rename, include migration notes, updated example authorize/login
URLs showing executionId instead of flowId, and mention the change in any SDK or
config examples that reference flowId so callers know to use executionId.
In `@samples/api/postman/authentication/authentication_demo.json`:
- Around line 58-64: Add a guard to fail fast when the redirect URL is missing
executionId: check the result of url.searchParams.get("executionId") (the
executionId variable) before calling pm.environment.set; if it's null/empty,
throw or log a clear error and stop execution (e.g., by calling
pm.test/pm.expect or equivalent) so you do not persist authId/executionId and
downstream /flow/execute failures are avoided.
- Line 2858: The raw request body for the Postman request named "02.02 -
Complete Authentication (Username & Password)" contains an inline
JavaScript-style comment (// "mobileNumber"...) which makes the JSON invalid;
remove that commented line (or replace it with a valid JSON entry like removing
the field entirely or setting "mobileNumber": null) inside the request object's
"raw" string in samples/api/postman/authentication/authentication_demo.json so
the raw string parses as valid JSON.
---
Nitpick comments:
In `@samples/apps/react-vanilla-sample/src/pages/LoginPage.tsx`:
- Around line 104-117: The executionId state is only initialized from
EXECUTION_ID_KEY ('executionId') and can be empty if a user began auth with the
legacy key 'flowId'; update the initialization and restore logic for executionId
(the useState for executionId and any places that
read/setExecutionId/sessionStorage) to fall back to
sessionStorage.getItem('flowId') when sessionStorage.getItem(EXECUTION_ID_KEY)
is falsy, and when writing the id persist it to both keys (EXECUTION_ID_KEY and
'flowId') so older in-flight sessions remain compatible during rollout.
In `@tests/integration/flow/authentication/github_auth_test.go`:
- Around line 373-374: Rename the local variable ExecutionID to use a consistent
lowercase local naming (e.g., executionID) throughout the test file; locate the
assignments like "ExecutionID := flowStep.ExecutionID" (and the similar
instances around the other spots noted) and update the local variable name and
all its subsequent usages to executionID to match the lowercase pattern and
improve readability.
In `@tests/integration/flow/registration/google_registration_test.go`:
- Around line 506-507: Rename the local variables named flowID to executionID
where they store execution identifiers (e.g., replace occurrences like "flowID
:= flowStep.ExecutionID" and any subsequent uses in this test file and the other
mentioned spots) so the variable name matches the value source
(flowStep.ExecutionID) and avoids semantic drift; update all references in tests
(including the other occurrences around lines with flowStep.Data.RedirectURL) to
use executionID consistently.
🪄 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
Run ID: 032ab77d-facc-4d52-8e1a-ec432c7a4d70
⛔ Files ignored due to path filters (13)
backend/tests/mocks/flow/flowexecmock/FlowExecServiceInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/flowStoreInterface_mock.gois excluded by!**/*_mock.godocs/__backup__/README.mdis excluded by!docs/__backup__/**docs/__backup__/authentication/server-orchestrated-flow/authentication.mdis excluded by!docs/__backup__/**docs/__backup__/authorization/flow-based-authorization.mdis excluded by!docs/__backup__/**docs/__backup__/flows/flow-execution.mdis excluded by!docs/__backup__/**docs/__backup__/observability/events.mdis excluded by!docs/__backup__/**docs/__backup__/registration/server-orchestrated-flow/registration.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/features/public-clients.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/authorization-code.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/client-credentials.mdis excluded by!docs/__backup__/**docs/__backup__/standards-based/oauth2-oidc/grant-types/token-exchange.mdis excluded by!docs/__backup__/**frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (112)
ARCHITECTURE.mdREADME.mdapi/flow-execution.yamlbackend/internal/flow/core/executor.gobackend/internal/flow/core/executor_test.gobackend/internal/flow/core/model.gobackend/internal/flow/core/node_test.gobackend/internal/flow/core/prompt_node.gobackend/internal/flow/core/prompt_node_test.gobackend/internal/flow/core/representation_node_test.gobackend/internal/flow/core/task_execution_node.gobackend/internal/flow/core/task_execution_node_test.gobackend/internal/flow/executor/attribute_collector.gobackend/internal/flow/executor/attribute_collector_test.gobackend/internal/flow/executor/attribute_uniqueness_validator.gobackend/internal/flow/executor/attribute_uniqueness_validator_test.gobackend/internal/flow/executor/auth_assert_executor.gobackend/internal/flow/executor/auth_assert_executor_test.gobackend/internal/flow/executor/authz_executor.gobackend/internal/flow/executor/authz_executor_test.gobackend/internal/flow/executor/basic_auth_executor.gobackend/internal/flow/executor/basic_auth_executor_test.gobackend/internal/flow/executor/consent_executor.gobackend/internal/flow/executor/consent_executor_test.gobackend/internal/flow/executor/credential_setter.gobackend/internal/flow/executor/credential_setter_test.gobackend/internal/flow/executor/email_executor.gobackend/internal/flow/executor/email_executor_test.gobackend/internal/flow/executor/federated_auth_resolver_executor.gobackend/internal/flow/executor/federated_auth_resolver_executor_test.gobackend/internal/flow/executor/http_request_executor.gobackend/internal/flow/executor/http_request_executor_test.gobackend/internal/flow/executor/identifying_executor.gobackend/internal/flow/executor/identifying_executor_test.gobackend/internal/flow/executor/invite_executor.gobackend/internal/flow/executor/invite_executor_test.gobackend/internal/flow/executor/oauth_executor.gobackend/internal/flow/executor/oauth_executor_test.gobackend/internal/flow/executor/oidc_auth_executor.gobackend/internal/flow/executor/oidc_auth_executor_test.gobackend/internal/flow/executor/ou_executor.gobackend/internal/flow/executor/ou_executor_test.gobackend/internal/flow/executor/ou_resolver_executor.gobackend/internal/flow/executor/ou_resolver_executor_test.gobackend/internal/flow/executor/passkey_executor.gobackend/internal/flow/executor/passkey_executor_test.gobackend/internal/flow/executor/permission_validator.gobackend/internal/flow/executor/permission_validator_test.gobackend/internal/flow/executor/provisioning_executor.gobackend/internal/flow/executor/provisioning_executor_test.gobackend/internal/flow/executor/sms_auth_executor.gobackend/internal/flow/executor/sms_auth_executor_test.gobackend/internal/flow/executor/sms_executor.gobackend/internal/flow/executor/sms_executor_test.gobackend/internal/flow/executor/user_type_resolver.gobackend/internal/flow/executor/user_type_resolver_test.gobackend/internal/flow/flowexec/FlowExecServiceInterface_mock_test.gobackend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/engine_event_enabled_test.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/flow/flowexec/flowStoreInterface_mock_test.gobackend/internal/flow/flowexec/handler.gobackend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/model_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/flowexec/store.gobackend/internal/flow/flowexec/store_test.gobackend/internal/oauth/oauth2/authz/handler_test.gobackend/internal/oauth/oauth2/authz/service.gobackend/internal/oauth/oauth2/authz/service_test.gobackend/internal/oauth/oauth2/constants/constants.gobackend/internal/system/log/constants.gobackend/internal/system/observability/event/datakeys.godocs/content/community/contributing/contributing-code/backend-development/observability.mddocs/content/guides/key-concepts/authentication/passwordless/passkeys.mdxdocs/static/api/next/combined.yamlfrontend/apps/thunder-gate/src/pages/AcceptInvitePage.tsxfrontend/pnpm-workspace.yamlsamples/api/postman/authentication/README.mdsamples/api/postman/authentication/authentication_demo.jsonsamples/api/postman/authentication/environment.jsonsamples/apps/react-vanilla-sample/src/pages/LoginPage.tsxsamples/apps/react-vanilla-sample/src/services/authService.tstests/e2e/tests/sample-app-authentication/README-MFA.mdtests/e2e/utils/thunder-setup/mfa-setup.tstests/integration/flow/authentication/assurance_test.gotests/integration/flow/authentication/attribute_collect_test.gotests/integration/flow/authentication/authz_test.gotests/integration/flow/authentication/basic_auth_test.gotests/integration/flow/authentication/conditional_exec_auth_test.gotests/integration/flow/authentication/github_auth_test.gotests/integration/flow/authentication/google_auth_test.gotests/integration/flow/authentication/multi_action_input_binding_test.gotests/integration/flow/authentication/prompt_actions_test.gotests/integration/flow/authentication/sensitive_input_cleanup_test.gotests/integration/flow/authentication/sms_auth_test.gotests/integration/flow/authentication/verbose_meta_test.gotests/integration/flow/common/model.gotests/integration/flow/common/utils.gotests/integration/flow/registration/basic_registration_test.gotests/integration/flow/registration/github_registration_test.gotests/integration/flow/registration/google_registration_test.gotests/integration/flow/registration/google_registration_with_role_group_test.gotests/integration/flow/registration/http_request_executor_runtime_data_test.gotests/integration/flow/registration/ou_registration_test.gotests/integration/flow/registration/sms_registration_test.gotests/integration/oauth/authz/authz_test.gotests/integration/oauth/token/refresh_token_test.gotests/integration/oauth/userinfo/userinfo_test.gotests/integration/testutils/models.gotests/integration/testutils/oauth2_utils.go
✅ Files skipped from review due to trivial changes (37)
- backend/internal/flow/executor/federated_auth_resolver_executor.go
- backend/internal/flow/executor/ou_executor_test.go
- docs/content/community/contributing/contributing-code/backend-development/observability.md
- samples/api/postman/authentication/environment.json
- backend/internal/flow/executor/attribute_uniqueness_validator.go
- backend/internal/flow/core/task_execution_node.go
- backend/internal/flow/executor/authz_executor_test.go
- frontend/pnpm-workspace.yaml
- backend/internal/flow/core/prompt_node.go
- backend/internal/flow/executor/auth_assert_executor.go
- backend/internal/flow/executor/permission_validator_test.go
- backend/internal/flow/core/executor.go
- README.md
- backend/internal/flow/executor/sms_executor.go
- backend/internal/flow/executor/ou_executor.go
- backend/internal/flow/executor/basic_auth_executor.go
- backend/internal/flow/executor/http_request_executor.go
- backend/internal/flow/executor/credential_setter_test.go
- backend/internal/flow/executor/identifying_executor.go
- backend/internal/oauth/oauth2/authz/service.go
- backend/internal/flow/executor/ou_resolver_executor_test.go
- frontend/apps/thunder-gate/src/pages/AcceptInvitePage.tsx
- tests/e2e/utils/thunder-setup/mfa-setup.ts
- docs/content/guides/key-concepts/authentication/passwordless/passkeys.mdx
- tests/integration/flow/registration/github_registration_test.go
- backend/internal/flow/executor/sms_executor_test.go
- backend/internal/flow/executor/federated_auth_resolver_executor_test.go
- backend/internal/flow/executor/provisioning_executor_test.go
- backend/internal/flow/executor/identifying_executor_test.go
- backend/internal/flow/executor/email_executor_test.go
- backend/internal/flow/core/executor_test.go
- backend/internal/flow/core/task_execution_node_test.go
- backend/internal/flow/executor/http_request_executor_test.go
- backend/internal/flow/executor/user_type_resolver_test.go
- tests/integration/flow/authentication/conditional_exec_auth_test.go
- ARCHITECTURE.md
- backend/internal/flow/flowexec/service.go
🚧 Files skipped from review as they are similar to previous changes (49)
- backend/internal/flow/executor/ou_resolver_executor.go
- backend/internal/flow/core/node_test.go
- backend/internal/flow/executor/provisioning_executor.go
- backend/internal/flow/core/model.go
- backend/internal/flow/executor/permission_validator.go
- backend/internal/flow/executor/attribute_collector_test.go
- backend/internal/flow/core/representation_node_test.go
- backend/internal/flow/executor/consent_executor.go
- samples/api/postman/authentication/README.md
- tests/integration/flow/authentication/authz_test.go
- tests/integration/testutils/models.go
- tests/integration/flow/authentication/sensitive_input_cleanup_test.go
- tests/integration/oauth/authz/authz_test.go
- tests/integration/flow/registration/sms_registration_test.go
- backend/internal/flow/executor/consent_executor_test.go
- backend/internal/system/observability/event/datakeys.go
- backend/internal/oauth/oauth2/constants/constants.go
- backend/internal/system/log/constants.go
- tests/integration/flow/registration/ou_registration_test.go
- backend/internal/flow/flowexec/engine.go
- backend/internal/flow/executor/sms_auth_executor.go
- tests/integration/flow/common/model.go
- tests/integration/flow/registration/http_request_executor_runtime_data_test.go
- tests/integration/flow/authentication/google_auth_test.go
- backend/internal/flow/executor/oidc_auth_executor_test.go
- tests/integration/oauth/userinfo/userinfo_test.go
- backend/internal/flow/executor/basic_auth_executor_test.go
- backend/internal/flow/flowexec/error_constants.go
- backend/internal/oauth/oauth2/authz/service_test.go
- backend/internal/flow/executor/invite_executor.go
- backend/internal/flow/flowexec/engine_event_enabled_test.go
- samples/apps/react-vanilla-sample/src/services/authService.ts
- backend/internal/flow/flowexec/model_test.go
- backend/internal/flow/flowexec/store_test.go
- backend/internal/flow/executor/sms_auth_executor_test.go
- backend/internal/flow/executor/email_executor.go
- tests/integration/flow/authentication/multi_action_input_binding_test.go
- tests/integration/flow/authentication/assurance_test.go
- backend/internal/flow/flowexec/model.go
- api/flow-execution.yaml
- backend/internal/flow/flowexec/store.go
- backend/internal/flow/executor/oauth_executor_test.go
- backend/internal/flow/executor/user_type_resolver.go
- tests/integration/flow/authentication/basic_auth_test.go
- tests/integration/flow/registration/basic_registration_test.go
- backend/internal/flow/executor/authz_executor.go
- backend/internal/flow/flowexec/flowStoreInterface_mock_test.go
- backend/internal/flow/executor/auth_assert_executor_test.go
- backend/internal/flow/flowexec/FlowExecServiceInterface_mock_test.go
58b1e60 to
9b305c1
Compare
ThaminduDilshan
previously approved these changes
Apr 10, 2026
53eeefb to
96ccc03
Compare
96ccc03 to
e77dbe1
Compare
ThaminduDilshan
approved these changes
Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
The flow execution response structure is confusing due to the use of two different flow IDs: one for the flow definition and one for the runtime flow execution instance.
This improvement rename
flowIdtoexecutionIdin flow execution context.🔧 Summary of Breaking Changes
The
flowIdin the flow execution context has been renamed toexecutionId. This change is consistently applied across API request and response parameters inflow/executeendpoint, replacing all occurrences offlowIdwithexecutionId. To maintain alignment, the method parameters in bothFlowExecServiceInterfaceandflowStoreInterfacehave also been updated accordingly.The following models are impacted by this change:
NodeContextEngineContextFlowContextDB💥 Impact
Any call/process goes through
flow/executeendpoint or flow execution engine or executors expect the new valueexecutionIdto be present.🔄 Migration Guide
N/A
Approach
N/A
Related Issues
Related PRs
Checklist
executionIdas misspelled is ignored.breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Bug Fixes
Documentation