Improve handling flow configurations - #4371
Merged
ThaminduDilshan merged 1 commit intoJul 27, 2026
Merged
Conversation
ThaminduDilshan
requested review from
darshanasbg and
rajithacharith
as code owners
July 27, 2026 08:21
ThaminduDilshan
added
Type/Improvement
breaking change
Contributor
📝 WalkthroughWalkthroughThis change moves flow defaults and expiry settings into a server-configured ChangesFlow defaults and resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant inboundClientService
participant flowMgtService
participant serverConfigService
participant OUService
Client->>inboundClientService: Resolve flow defaults
inboundClientService->>flowMgtService: ResolveEffectiveFlowID
flowMgtService->>OUService: GetOrganizationUnit
OUService-->>flowMgtService: Organization-unit flow default
flowMgtService->>serverConfigService: GetMergedConfig("flow")
serverConfigService-->>flowMgtService: Server flow default
flowMgtService-->>inboundClientService: Effective flow ID
inboundClientService-->>Client: Resolved flow defaults
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
This was
linked to
issues
Jul 27, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
backend/internal/ou/store_test.go (1)
726-771: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover non-empty onboarding flow IDs in persistence tests.
Both the create and update “design fields” cases leave
UserOnboardingFlowIDempty, so these tests would pass even if a real value were dropped or serialized incorrectly. Set a non-empty ID in each fixture and assert that the metadata JSON contains that exact value.Based on the supplied store implementation and test cases, the new persistence path is not currently exercised with a real onboarding flow ID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/ou/store_test.go` around lines 726 - 771, Update the “success with design fields” fixtures in backend/internal/ou/store_test.go lines 726-771 and 1406-1448 for both create and update cases: assign a non-empty UserOnboardingFlowID and assert that the expected metadata JSON contains that exact value, exercising persistence and serialization of onboarding flow IDs.backend/internal/flow/flowexec/service_test.go (1)
1757-1757: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer named constants over magic-number literals in expiry assertions.
mock.MatchedBy(func(exp int64) bool { return exp == int64(1800) })ands.Equal(int64(1800), ...)hardcode the fallback expiry instead of referencingdefaultAuthFlowExpiry/defaultRecoveryFlowExpiry. If the constants ever change, these assertions silently stop verifying the actual default and just re-encode a stale expectation.Also applies to: 2332-2332, 650-666
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/flow/flowexec/service_test.go` at line 1757, Replace the hardcoded 1800 expiry literals in the affected assertions, including the mock.MatchedBy callbacks and s.Equal checks, with the appropriate named constants defaultAuthFlowExpiry or defaultRecoveryFlowExpiry. Preserve the existing assertion behavior while ensuring each test references the corresponding default expiry constant.backend/internal/flow/flowexec/service.go (1)
923-923: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a shared logger component-name constant for FlowExecService.
service.gorepeats"FlowExecService"across multiple logger initializations. Define and use oneloggerComponentNameFlowExecServiceconstant in this package so future renames stay localized and consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/flow/flowexec/service.go` at line 923, Define a package-level loggerComponentNameFlowExecService constant in the flow execution service package, then replace every hard-coded "FlowExecService" component name used by logger initializations in service.go with that constant, including the shown With call.backend/internal/flow/flowexec/flowDefaultsProvider_mock_test.go (1)
1-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the orphaned
flowDefaultsProvidermock artifact.
flowDefaultsProviderhas no non-mock references, while the implementation calls privateflowExecServicemethods directly (resolveDefaultFlowHandlereturns(string, *common.ServiceError)inmgt/service.go, andflowexec/service.goprivateresolveDefaultFlowHandlereturns onlystring). Keep the mock if the public interface is intended, or drop the mock-generated types if it was discarded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/flow/flowexec/flowDefaultsProvider_mock_test.go` around lines 1 - 166, Remove the orphaned generated flowDefaultsProvider mock artifact, including newFlowDefaultsProviderMock, flowDefaultsProviderMock, and its expectation/helper methods, because the implementation uses private flowExecService methods directly. Do not alter the flowExecService implementations or their return contracts; retain the mock only if a live public interface reference is restored.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/internal/flow/flowexec/service.go`:
- Around line 919-942: Update getFlowSectionConfig to log an error when the
merged "flow" configuration fails the flowconfig.FlowSectionConfig type
assertion before returning the zero FlowSectionConfig. Include enough context to
identify the unexpected type, while preserving the existing fallback behavior.
In `@backend/internal/flow/mgt/service.go`:
- Around line 697-703: Log a warning when s.ouSvc.GetOrganizationUnit in the
OU-resolution block fails, including the OU identifier and error details, before
continuing to server-level default resolution. Preserve the existing successful
lookup and fallback behavior.
- Around line 738-782: Propagate the server-config read failure from
getFlowSectionConfig instead of logging a warning and returning an empty
configuration with nil error. Return the svcErr from GetMergedConfig so
resolveDefaultFlowHandle and ResolveEffectiveFlowID can surface infrastructure
failures while preserving the empty-config behavior when no service is
configured or the merged value is not a FlowSectionConfig.
In `@backend/internal/inboundclient/service.go`:
- Around line 618-622: After resolving the authentication flow in the
inbound-client acceptance path, explicitly reject an empty c.AuthFlowID before
assigning or accepting it. Update the logic around resolveFlowDefaults and
validateAuthFlowID so empty values produce the required validation error, while
valid resolved flow IDs continue through the existing assignment path.
In `@backend/internal/serverconfig/constants.go`:
- Around line 32-41: Update the documentation under docs/content/guides/ to
cover the new flow configuration represented by ConfigNameFlow, including
per-flow default handles, expiry settings, resolution precedence (override,
organization unit, then server default), and migration from prior hardcoded
defaults. For backend/internal/serverconfig/constants.go:32-41,
backend/cmd/server/bootstrap/02-server-configurations.yaml:1-13, and
backend/internal/inboundclient/service_test.go:1565-1633, add or link
documentation describing the flow section, bootstrap defaults and supported
keys, and inbound-client default resolution behavior respectively.
In `@backend/pkg/thunderidengine/engine.go`:
- Around line 172-178: The embedded-engine initialization in
`flowexec.Initialize` passes a nil server-config provider while default flow
handles are now resolved through it. Add an embedded-engine provider or option
supplying `DefaultAuthFlowHandle`, `DefaultSignOutFlowHandle`, and
`UserOnboardingFlowHandle`, or otherwise preserve an executable fallback path,
and pass it from `engine.go` so auth fallback and onboarding flow resolution
never use empty handles.
---
Nitpick comments:
In `@backend/internal/flow/flowexec/flowDefaultsProvider_mock_test.go`:
- Around line 1-166: Remove the orphaned generated flowDefaultsProvider mock
artifact, including newFlowDefaultsProviderMock, flowDefaultsProviderMock, and
its expectation/helper methods, because the implementation uses private
flowExecService methods directly. Do not alter the flowExecService
implementations or their return contracts; retain the mock only if a live public
interface reference is restored.
In `@backend/internal/flow/flowexec/service_test.go`:
- Line 1757: Replace the hardcoded 1800 expiry literals in the affected
assertions, including the mock.MatchedBy callbacks and s.Equal checks, with the
appropriate named constants defaultAuthFlowExpiry or defaultRecoveryFlowExpiry.
Preserve the existing assertion behavior while ensuring each test references the
corresponding default expiry constant.
In `@backend/internal/flow/flowexec/service.go`:
- Line 923: Define a package-level loggerComponentNameFlowExecService constant
in the flow execution service package, then replace every hard-coded
"FlowExecService" component name used by logger initializations in service.go
with that constant, including the shown With call.
In `@backend/internal/ou/store_test.go`:
- Around line 726-771: Update the “success with design fields” fixtures in
backend/internal/ou/store_test.go lines 726-771 and 1406-1448 for both create
and update cases: assign a non-empty UserOnboardingFlowID and assert that the
expected metadata JSON contains that exact value, exercising persistence and
serialization of onboarding flow IDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f8d6528c-6678-4f6e-9f93-5fa25201fbbe
⛔ Files ignored due to path filters (5)
backend/tests/mocks/flow/flowexecmock/flowDefaultsProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/serverConfigProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/FlowMgtServiceInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/ouProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/serverConfigProvider_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (35)
.gitignorebackend/cmd/server/bootstrap/02-server-configurations.yamlbackend/cmd/server/config/default.jsonbackend/cmd/server/servicemanager.gobackend/internal/flow/config/config.gobackend/internal/flow/config/config_test.gobackend/internal/flow/flowexec/constants.gobackend/internal/flow/flowexec/flowDefaultsProvider_mock_test.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/serverConfigProvider_mock_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/mgt/FlowMgtServiceInterface_mock_test.gobackend/internal/flow/mgt/init.gobackend/internal/flow/mgt/ouProvider_mock_test.gobackend/internal/flow/mgt/serverConfigProvider_mock_test.gobackend/internal/flow/mgt/server_config.gobackend/internal/flow/mgt/service.gobackend/internal/flow/mgt/service_test.gobackend/internal/inboundclient/ouProvider_mock_test.gobackend/internal/inboundclient/service.gobackend/internal/inboundclient/service_test.gobackend/internal/ou/error_constants.gobackend/internal/ou/handler.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/store.gobackend/internal/ou/store_test.gobackend/internal/serverconfig/constants.gobackend/internal/system/i18n/core/defaults.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/engine.gobackend/pkg/thunderidengine/providers/model.gobuild.ps1build.sh
💤 Files with no reviewable changes (1)
- backend/cmd/server/config/default.json
ThaminduDilshan
force-pushed
the
thamindu-flow-config
branch
from
July 27, 2026 08:56
d175790 to
24ea4b6
Compare
| flowExecService := newFlowExecService(flowProvider, flowStore, flowEngine, | ||
| actorProvider, observabilitySvc, transactioner, cryptoSvc, attestationVerifier, | ||
| graphBuilder, cfg) | ||
| graphBuilder, serverConfigSvc, cfg) |
Contributor
There was a problem hiding this comment.
Do we directly inject server config? or thought of having a proxy svc only to get default flows.
If we don't have any other data required from serverConfigs
FlowExec --> DefaultFlowProvider --> serverConfig
|-> GetDefaultFlow(FlowType string) (flowId string)
Member
Author
There was a problem hiding this comment.
Is this proxy pattern something we're following in other places when accessing server config svc? I have defined a serverConfigProvider interface to downscope it only to the required method, but this doesn't restrict it to flow configurations.
type serverConfigProvider interface {
GetMergedConfig(ctx context.Context, name string) (any, *tidcommon.ServiceError)
}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/internal/flow/flowexec/service_test.go (1)
650-666: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded expiry literals replace named default constants, inconsistent with sibling test.
TestGetFlowExpirySeconds(Line 650-666),TestInitiateAndExecute_ZeroExpiryUsesDefault(Line 1757), andTestGetFlowExpirySeconds_RecoveryFlow(Line 2332) now assert raw literals (1800, 3600, 86400) instead of the named default constants (e.g.,defaultRecoveryFlowExpiry), whileTestGetFlowExpirySeconds_SignOutFallsBackToDefault(Line 3283) still referencesdefaultSignOutFlowExpiry. Referencing constants keeps the tests tied to the actual source of truth and catches unintended default drift; literals can silently pass even if the underlying default constant changes.♻️ Suggested fix (illustrative, using existing named constants)
{ name: "Authentication flow", flowType: providers.FlowTypeAuthentication, - expected: 1800, + expected: defaultAuthFlowExpiry, }, { name: "Registration flow", flowType: providers.FlowTypeRegistration, - expected: 3600, + expected: defaultRegistrationFlowExpiry, }, { name: "User onboarding flow", flowType: providers.FlowTypeUserOnboarding, - expected: 86400, + expected: defaultUserOnboardingFlowExpiry, }, { name: "Unknown flow type (fallback)", flowType: providers.FlowType("UNKNOWN_FLOW"), - expected: 1800, + expected: defaultAuthFlowExpiry, },- mockStore.EXPECT().StoreFlowContext(mock.Anything, mock.Anything, - mock.MatchedBy(func(exp int64) bool { return exp == int64(1800) })). + mockStore.EXPECT().StoreFlowContext(mock.Anything, mock.Anything, + mock.MatchedBy(func(exp int64) bool { return exp == defaultAuthFlowExpiry })).- s.Equal(int64(1800), service.getFlowExpirySeconds(context.Background(), providers.FlowTypeRecovery)) + s.Equal(defaultRecoveryFlowExpiry, service.getFlowExpirySeconds(context.Background(), providers.FlowTypeRecovery))Also applies to: 1757-1757, 2332-2332
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/flow/flowexec/service_test.go` around lines 650 - 666, Replace the hardcoded expiry literals in TestGetFlowExpirySeconds, TestInitiateAndExecute_ZeroExpiryUsesDefault, and TestGetFlowExpirySeconds_RecoveryFlow with the corresponding named default constants used by the implementation, matching TestGetFlowExpirySeconds_SignOutFallsBackToDefault. Preserve each test’s existing expected flow behavior while ensuring assertions track the source-of-truth defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/internal/flow/flowexec/service_test.go`:
- Around line 650-666: Replace the hardcoded expiry literals in
TestGetFlowExpirySeconds, TestInitiateAndExecute_ZeroExpiryUsesDefault, and
TestGetFlowExpirySeconds_RecoveryFlow with the corresponding named default
constants used by the implementation, matching
TestGetFlowExpirySeconds_SignOutFallsBackToDefault. Preserve each test’s
existing expected flow behavior while ensuring assertions track the
source-of-truth defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d5c2fd2-cb4a-4d54-9114-f9d09601d2ea
⛔ Files ignored due to path filters (5)
backend/tests/mocks/flow/flowexecmock/flowDefaultsProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/serverConfigProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/FlowMgtServiceInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/ouProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/serverConfigProvider_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (36)
.gitignorebackend/cmd/server/bootstrap/02-server-configurations.yamlbackend/cmd/server/config/default.jsonbackend/cmd/server/servicemanager.gobackend/internal/flow/config/config.gobackend/internal/flow/config/config_test.gobackend/internal/flow/flowexec/constants.gobackend/internal/flow/flowexec/flowDefaultsProvider_mock_test.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/serverConfigProvider_mock_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/mgt/FlowMgtServiceInterface_mock_test.gobackend/internal/flow/mgt/init.gobackend/internal/flow/mgt/ouProvider_mock_test.gobackend/internal/flow/mgt/serverConfigProvider_mock_test.gobackend/internal/flow/mgt/server_config.gobackend/internal/flow/mgt/server_config_test.gobackend/internal/flow/mgt/service.gobackend/internal/flow/mgt/service_test.gobackend/internal/inboundclient/ouProvider_mock_test.gobackend/internal/inboundclient/service.gobackend/internal/inboundclient/service_test.gobackend/internal/ou/error_constants.gobackend/internal/ou/handler.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/store.gobackend/internal/ou/store_test.gobackend/internal/serverconfig/constants.gobackend/internal/system/i18n/core/defaults.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/engine.gobackend/pkg/thunderidengine/providers/model.gobuild.ps1build.sh
💤 Files with no reviewable changes (1)
- backend/cmd/server/config/default.json
🚧 Files skipped from review as they are similar to previous changes (30)
- backend/pkg/thunderidengine/providers/model.go
- backend/internal/flow/flowexec/constants.go
- backend/cmd/server/bootstrap/02-server-configurations.yaml
- backend/internal/system/i18n/core/defaults.go
- backend/internal/ou/model.go
- backend/pkg/thunderidengine/config/config.go
- backend/internal/ou/error_constants.go
- backend/internal/ou/store_test.go
- backend/internal/flow/mgt/init.go
- backend/internal/flow/flowexec/serverConfigProvider_mock_test.go
- backend/internal/flow/config/config.go
- backend/internal/serverconfig/constants.go
- backend/internal/ou/handler.go
- build.sh
- backend/internal/ou/service.go
- backend/pkg/thunderidengine/engine.go
- backend/cmd/server/servicemanager.go
- backend/internal/flow/mgt/server_config.go
- backend/internal/flow/config/config_test.go
- backend/internal/ou/store.go
- backend/internal/inboundclient/ouProvider_mock_test.go
- .gitignore
- backend/internal/flow/mgt/serverConfigProvider_mock_test.go
- backend/internal/flow/mgt/FlowMgtServiceInterface_mock_test.go
- backend/internal/flow/mgt/service.go
- backend/internal/flow/mgt/ouProvider_mock_test.go
- backend/internal/inboundclient/service_test.go
- backend/internal/flow/flowexec/service.go
- backend/internal/inboundclient/service.go
- backend/internal/flow/flowexec/init.go
thiva-k
reviewed
Jul 27, 2026
thiva-k
reviewed
Jul 27, 2026
| return err | ||
| } | ||
| c.RecoveryFlowID = recID | ||
| c.IsRecoveryFlowEnabled = c.RecoveryFlowID != "" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== Current relevant diff ==='
git diff --unified=40 HEAD^..HEAD -- backend/internal/inboundclient/service.go || true
echo '=== Service outline ==='
ast-grep outline backend/internal/inboundclient/service.go --match 'resolveFlowDefaults|reconcileReferencedFlows|walkReferencedFlows' --view expanded || true
echo '=== Resolution implementation ==='
rg -n -A90 -B12 'func \(s \*inboundClientService\) resolveFlowDefaults\b' backend/internal/inboundclient/service.go
echo '=== Related test locations and assertions ==='
rg -n -A12 -B6 'resolveFlowDefaults|IsRegistrationFlowEnabled|IsRecoveryFlowEnabled|ResolveEffectiveFlowID' backend/internal/inboundclient/service_test.goLength of output: 26341
@ThaminduDilshan, the fix is not fully present yet.
In resolveFlowDefaults, Lines 634-636 and Lines 643-645 only set the registration and recovery flags to false when their resolved IDs are empty. They do not set the flags to true when ResolveEffectiveFlowID supplies a non-empty ID.
This also conflicts with TestResolveFlowDefaults_ResolvesAllFlowTypes, which resolves both IDs from initially zero-valued flags and expects both flags to be true.
Please derive both flags directly from the resolved IDs:
c.IsRegistrationFlowEnabled = c.RegistrationFlowID != ""
c.IsRecoveryFlowEnabled = c.RecoveryFlowID != ""I could not verify this as resolved.
Member
Author
There was a problem hiding this comment.
Even if a flow id is derived, if the API call (or caller) set it to false, we should honor it.
@coderabbitai
Member
Author
There was a problem hiding this comment.
@coderabbitai Even if a flow id is derived, if the API call (or caller) set it to false, we should honor it.
Can you verify this with the implementation whether your previous comment is still valid?
Contributor
There was a problem hiding this comment.
@ThaminduDilshan, verified. My previous comment is not valid given this intended behavior.
The current implementation preserves an explicitly disabled registration or recovery flow when an ID is resolved, and only forces the flag to false when no effective flow ID exists. Changing the flags to c.<flag> = c.<flowID> != "" would wrongly re-enable flows that the caller explicitly disabled.
✏️ Learnings added
Learnt from: ThaminduDilshan
Repo: thunder-id/thunderid PR: 4371
File: backend/internal/inboundclient/service.go:0-0
Timestamp: 2026-07-27T11:18:58.869Z
Learning: In `backend/internal/inboundclient/service.go`, `resolveFlowDefaults` must preserve caller-provided false values for `IsRegistrationFlowEnabled` and `IsRecoveryFlowEnabled` even when `ResolveEffectiveFlowID` derives a non-empty flow ID. The flags are only forced to false when their respective effective flow IDs are empty.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
ThaminduDilshan
force-pushed
the
thamindu-flow-config
branch
from
July 27, 2026 10:46
24ea4b6 to
7e3fdf1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/internal/inboundclient/service_test.go`:
- Around line 1564-1590: Update TestResolveFlowDefaults_ResolvesAllFlowTypes to
initialize IsRegistrationFlowEnabled and IsRecoveryFlowEnabled as true on the
inbound client fixture before calling resolveFlowDefaults, matching the
function’s existing contract of preserving enabled flags rather than deriving
them from resolved IDs.
In `@backend/internal/inboundclient/service.go`:
- Around line 623-627: The resolveFlowDefaults flow must reject an empty
AuthFlowID after resolve returns successfully. Immediately after assigning
c.AuthFlowID, validate that it is non-empty and return an appropriate error when
no authentication flow was resolved; preserve the existing error handling for
resolve failures and non-empty IDs.
In `@docs/content/deployment/configuration.mdx`:
- Around line 910-913: The expirySeconds description in the configuration table
incorrectly requires a positive integer. Update the `<type>.expirySeconds` entry
to state that zero or an omitted value falls back to the built-in default, while
retaining that negative values are invalid.
- Around line 904-959: The new Flow Defaults prose contains three em dashes,
which violate the repository copy guidelines. Update the paragraphs around
“Resolution order,” the authentication fallback statement, and the
handle-validation note to replace each em dash with commas, periods, or
rephrased sentences, without changing the documented behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ece01e3f-9ab6-4015-8b68-5e67525fdd75
⛔ Files ignored due to path filters (5)
backend/tests/mocks/flow/flowexecmock/flowDefaultsProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/serverConfigProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/FlowMgtServiceInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/ouProvider_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowmgtmock/serverConfigProvider_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (39)
.gitignorebackend/cmd/server/bootstrap/02-server-configurations.yamlbackend/cmd/server/config/default.jsonbackend/cmd/server/servicemanager.gobackend/internal/flow/config/config.gobackend/internal/flow/config/config_test.gobackend/internal/flow/flowexec/constants.gobackend/internal/flow/flowexec/flowDefaultsProvider_mock_test.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/serverConfigProvider_mock_test.gobackend/internal/flow/flowexec/service.gobackend/internal/flow/flowexec/service_test.gobackend/internal/flow/mgt/FlowMgtServiceInterface_mock_test.gobackend/internal/flow/mgt/init.gobackend/internal/flow/mgt/ouProvider_mock_test.gobackend/internal/flow/mgt/serverConfigProvider_mock_test.gobackend/internal/flow/mgt/server_config.gobackend/internal/flow/mgt/server_config_test.gobackend/internal/flow/mgt/service.gobackend/internal/flow/mgt/service_test.gobackend/internal/inboundclient/ouProvider_mock_test.gobackend/internal/inboundclient/service.gobackend/internal/inboundclient/service_test.gobackend/internal/ou/error_constants.gobackend/internal/ou/handler.gobackend/internal/ou/model.gobackend/internal/ou/service.gobackend/internal/ou/store.gobackend/internal/ou/store_test.gobackend/internal/serverconfig/constants.gobackend/internal/system/i18n/core/defaults.gobackend/pkg/thunderidengine/config/config.gobackend/pkg/thunderidengine/engine.gobackend/pkg/thunderidengine/providers/model.gobuild.ps1build.shdocs/content/deployment/configuration.mdxtests/integration/resources/declarative_resources/agents/agent-declarative-1.yamltests/integration/resources/declarative_resources/agents/agent-declarative-confidential.yaml
💤 Files with no reviewable changes (1)
- backend/cmd/server/config/default.json
🚧 Files skipped from review as they are similar to previous changes (29)
- backend/cmd/server/bootstrap/02-server-configurations.yaml
- backend/internal/ou/error_constants.go
- backend/internal/flow/config/config_test.go
- backend/internal/system/i18n/core/defaults.go
- backend/pkg/thunderidengine/config/config.go
- backend/internal/ou/handler.go
- build.sh
- backend/internal/flow/mgt/ouProvider_mock_test.go
- backend/internal/serverconfig/constants.go
- backend/internal/flow/flowexec/serverConfigProvider_mock_test.go
- backend/internal/inboundclient/ouProvider_mock_test.go
- backend/internal/ou/store.go
- .gitignore
- backend/cmd/server/servicemanager.go
- backend/internal/ou/model.go
- backend/internal/ou/store_test.go
- backend/internal/flow/config/config.go
- backend/internal/flow/mgt/service.go
- backend/internal/flow/mgt/server_config_test.go
- backend/internal/flow/mgt/FlowMgtServiceInterface_mock_test.go
- backend/internal/flow/mgt/init.go
- backend/internal/flow/mgt/serverConfigProvider_mock_test.go
- backend/internal/flow/flowexec/init.go
- backend/pkg/thunderidengine/engine.go
- backend/internal/flow/mgt/service_test.go
- backend/internal/ou/service.go
- backend/internal/flow/flowexec/flowDefaultsProvider_mock_test.go
- backend/internal/flow/flowexec/service.go
- backend/internal/flow/flowexec/service_test.go
ThaminduDilshan
force-pushed
the
thamindu-flow-config
branch
from
July 27, 2026 11:10
7e3fdf1 to
6bb8c36
Compare
ThaminduDilshan
enabled auto-merge
July 27, 2026 11:11
rajithacharith
approved these changes
Jul 27, 2026
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This was referenced Jul 27, 2026
This was referenced Aug 7, 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
3 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
This pull request introduces a new, unified server configuration section for flow management, allowing per-flow-type default handles and expiry times to be centrally managed. The changes refactor how default flow handles and expiry values are resolved, making them configurable via server settings instead of hardcoded values. The flow execution service is updated to read these values from the new configuration source. Additionally, new mock types are added to support testing of these changes.
🔧 Summary of Breaking Changes
This PR improves the default flow configuration experience in the server. Default flow handle configurations are now removed from the deployment.yaml file and moved to the server config API.
💥 Impact
Deployments already using custom default flow configurations via deployment.yaml will no longer work.
🔄 Migration Guide
Configure default flow configurations in a
/config/resources/server_configs/flow.yamlas a read only configurations or invoke the server config API to create the default flow configurations.Declarative - create
config/resources/server_configs/flow.yaml, then restart:Runtime - send the new value to
PUT /server-config/flow; it is applied immediately and merged with any declarative file:Approach
Configuration and Flow Management Improvements:
flowsection to the server configuration, supporting per-flow-type default handles and expiry times viaFlowSectionConfigandFlowTypeConfigstructs (backend/cmd/server/bootstrap/02-server-configurations.yaml,backend/internal/flow/config/config.go) [1] [2].serverConfigProviderinterface for reading flow configuration, replacing hardcoded defaults (backend/cmd/server/servicemanager.go,backend/internal/flow/flowexec/init.go,backend/internal/flow/flowexec/service.go) [1] [2] [3] [4] [5] [6] [7] [8] [9].Code Simplification and Test Support:
getFlowExpirySecondsmethod in favor of reading expiry values from configuration, and updated all usages accordingly (backend/internal/flow/flowexec/service.go) [1] [2].backend/internal/flow/flowexec/flowDefaultsProvider_mock_test.go,backend/internal/flow/flowexec/serverConfigProvider_mock_test.go) [1] [2].Other Updates:
backend/internal/flow/flowexec/constants.go,backend/internal/flow/config/config_test.go) [1] [2] [3].Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit