Add integration coverage for the flow engine and SSO sessions - #4862
Open
indeewari wants to merge 1 commit into
Open
Add integration coverage for the flow engine and SSO sessions#4862indeewari wants to merge 1 commit into
indeewari wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds integration tests for administration deletion, flow execution errors and lifecycle behavior, flow usage reporting, nested flow call-depth limits, registration-flow inference, and SSO session timeouts. ChangesFlow integration coverage
SSO session timeout coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
indeewari
added
the
trigger-pr-builder
![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: 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 `@tests/integration/flow/execution/administration_flow_test.go`:
- Around line 202-208: Update the negative deletion assertions in the
status-check branches to retain the existing rejection lower bound while
requiring statuses below http.StatusInternalServerError; apply this to both the
status == http.StatusOK branch and the corresponding assertion around the
adjacent unknown-subject case.
In `@tests/integration/flow/execution/flow_execution_error_test.go`:
- Around line 33-35: Add coverage for the errCodeAdminPermissionNeeded branch in
the flow execution error tests by introducing a signed-in non-administrator
fixture, executing an administration flow through the existing flow-by-ID test
path, and asserting the expected FES-1019 response. Keep the existing
client-credentials FES-1017 test unchanged and follow the suite’s established
fixture and assertion patterns.
In `@tests/integration/flow/mgt/flow_usages_test.go`:
- Around line 82-86: Update both FlowUsagesResponse test cases in
tests/integration/flow/mgt/flow_usages_test.go: lines 82-86 must assert Summary
is non-nil and empty for an unreferenced flow, while lines 122-133 must assert
Summary is non-nil and that Summary["application"] reports the bound
application.
In `@tests/integration/oauth/sso/session_timeout_test.go`:
- Around line 70-77: Update the ts.T().Cleanup callback to use ts.T().Errorf
instead of ts.T().Logf for failures from testutils.RestartServer and
testutils.ObtainAdminAccessToken, while preserving the existing error context so
incomplete session cleanup fails the test.
🪄 Autofix
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: bddd5c13-bfa7-4416-8451-4c09d1979ba2
📒 Files selected for processing (5)
tests/integration/flow/execution/administration_flow_test.gotests/integration/flow/execution/flow_execution_error_test.gotests/integration/flow/execution/flow_lifecycle_test.gotests/integration/flow/mgt/flow_usages_test.gotests/integration/oauth/sso/session_timeout_test.go
Comment on lines
+202
to
+208
| if status == http.StatusOK { | ||
| ts.NotEqual("COMPLETE", step.FlowStatus, | ||
| "Deleting an unknown subject must not report success: %s", string(body)) | ||
| return | ||
| } | ||
| ts.GreaterOrEqual(status, http.StatusBadRequest, | ||
| "Deleting an unknown subject should be reported as an error: %s", string(body)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject server failures in the negative tests.
Both branches accept HTTP 500 and higher as valid rejection behavior. A server failure can then satisfy the tests.
Keep the existing lower bound. Add an upper bound below http.StatusInternalServerError for both branches.
Proposed test change
ts.GreaterOrEqual(status, http.StatusBadRequest,
"Deleting an unknown subject should be reported as an error: %s", string(body))
+ts.Less(status, http.StatusInternalServerError,
+ "Deleting an unknown subject must not produce a server error: %s", string(body))Also applies to: 215-221
🤖 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 `@tests/integration/flow/execution/administration_flow_test.go` around lines
202 - 208, Update the negative deletion assertions in the status-check branches
to retain the existing rejection lower bound while requiring statuses below
http.StatusInternalServerError; apply this to both the status == http.StatusOK
branch and the corresponding assertion around the adjacent unknown-subject case.
Comment on lines
+33
to
+35
| // errCodeAdminPermissionNeeded (FES-1019) is not asserted yet: it needs a signed-in user whose | ||
| // permissions omit the system scope. See TestExecuteByFlowID_ClientCredentialsTokenRejected. | ||
| ) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Cover the administrator permission-denial branch.
This suite leaves FES-1019 untested. The client-credentials case only verifies missing user authentication with FES-1017. It does not verify authorization for an authenticated user without the system scope.
Add a signed-in non-administrator fixture. Execute an administration flow by ID. Assert the expected FES-1019 response. The PR reports relevant flow coverage below the required 80% target.
As per coding guidelines, “Write tests for new features and bug fixes, targeting at least 80% coverage.”
Also applies to: 250-254
🤖 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 `@tests/integration/flow/execution/flow_execution_error_test.go` around lines
33 - 35, Add coverage for the errCodeAdminPermissionNeeded branch in the flow
execution error tests by introducing a signed-in non-administrator fixture,
executing an administration flow through the existing flow-by-ID test path, and
asserting the expected FES-1019 response. Keep the existing client-credentials
FES-1017 test unchanged and follow the suite’s established fixture and assertion
patterns.
Source: Coding guidelines
Comment on lines
+82
to
+86
| suite.Equal(0, response.Count) | ||
| suite.Empty(response.Usages) | ||
| if suite.NotNil(response.TotalResults, "an unreferenced flow should report a known total") { | ||
| suite.Equal(0, *response.TotalResults) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert Summary in both usage-response cases.
FlowUsagesResponse defines nil Summary as unavailable dependency data. The current tests allow an endpoint that omits summary to pass.
tests/integration/flow/mgt/flow_usages_test.go#L82-L86: assert thatSummaryis non-nil and empty for an unreferenced flow.tests/integration/flow/mgt/flow_usages_test.go#L122-L133: assert thatSummary["application"]reports the bound application.
📍 Affects 1 file
tests/integration/flow/mgt/flow_usages_test.go#L82-L86(this comment)tests/integration/flow/mgt/flow_usages_test.go#L122-L133
🤖 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 `@tests/integration/flow/mgt/flow_usages_test.go` around lines 82 - 86, Update
both FlowUsagesResponse test cases in
tests/integration/flow/mgt/flow_usages_test.go: lines 82-86 must assert Summary
is non-nil and empty for an unreferenced flow, while lines 122-133 must assert
Summary is non-nil and that Summary["application"] reports the bound
application.
Comment on lines
+70
to
+77
| ts.T().Cleanup(func() { | ||
| ts.putSessionConfig(original) | ||
| if err := testutils.RestartServer(); err != nil { | ||
| ts.T().Logf("cleanup: server did not restart cleanly after session config restore: %v", err) | ||
| } | ||
| if err := testutils.ObtainAdminAccessToken(); err != nil { | ||
| ts.T().Logf("cleanup: failed to re-obtain admin token after restore: %v", err) | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail the test when session cleanup fails.
Lines 72-77 only log failures. If RestartServer fails, a running server can retain the short test timeouts. If ObtainAdminAccessToken fails, later tests can use invalid admin state. Mark these failures with Errorf so the test run cannot pass with incomplete cleanup.
Proposed fix
if err := testutils.RestartServer(); err != nil {
- ts.T().Logf("cleanup: server did not restart cleanly after session config restore: %v", err)
+ ts.T().Errorf("cleanup: server did not restart cleanly after session config restore: %v", err)
}
if err := testutils.ObtainAdminAccessToken(); err != nil {
- ts.T().Logf("cleanup: failed to re-obtain admin token after restore: %v", err)
+ ts.T().Errorf("cleanup: failed to re-obtain admin token after restore: %v", err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ts.T().Cleanup(func() { | |
| ts.putSessionConfig(original) | |
| if err := testutils.RestartServer(); err != nil { | |
| ts.T().Logf("cleanup: server did not restart cleanly after session config restore: %v", err) | |
| } | |
| if err := testutils.ObtainAdminAccessToken(); err != nil { | |
| ts.T().Logf("cleanup: failed to re-obtain admin token after restore: %v", err) | |
| } | |
| ts.T().Cleanup(func() { | |
| ts.putSessionConfig(original) | |
| if err := testutils.RestartServer(); err != nil { | |
| ts.T().Errorf("cleanup: server did not restart cleanly after session config restore: %v", err) | |
| } | |
| if err := testutils.ObtainAdminAccessToken(); err != nil { | |
| ts.T().Errorf("cleanup: failed to re-obtain admin token after restore: %v", err) | |
| } |
🤖 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 `@tests/integration/oauth/sso/session_timeout_test.go` around lines 70 - 77,
Update the ts.T().Cleanup callback to use ts.T().Errorf instead of ts.T().Logf
for failures from testutils.RestartServer and testutils.ObtainAdminAccessToken,
while preserving the existing error context so incomplete session cleanup fails
the test.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Cover the flow execution error branches, the flow usages endpoint, execution resume and context expiry, the shipped user deletion administration flow, the SSO session idle and absolute timeouts, registration flow inference, and the nested call depth limit. The administration flow test is the first integration coverage of the criteria based revocation path: one execution drives permission validation, pre-delete validation, criteria revocation, session termination and record deletion. Registration flow inference and the SSO session timeouts are both read at startup, so those tests patch the deployment configuration and restart the server, restoring and restarting again on cleanup.
indeewari
force-pushed
the
test/flow-integration-coverage
branch
from
August 12, 2026 03:49
1a6c44e to
e9dda2b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/integration/flow/execution/call_depth_test.go`:
- Around line 146-164: Strengthen TestExecute_ExceedingCallDepthRejected to
require errCodeMaxCallDepth (FES-1013) in both rejection paths: validate the
returned error contains the maximum call-depth code instead of only HTTP 400,
and require step.Error to be non-nil with that code before accepting the
step-based failure. Preserve the existing not-complete assertion.
In `@tests/integration/flow/mgt/flow_inference_test.go`:
- Around line 58-73: Update TearDownSuite in
tests/integration/flow/mgt/flow_inference_test.go (lines 58-73) to report
failures from PatchDeploymentConfig, RestartServer, and ObtainAdminAccessToken
through the test failure mechanism while continuing all remaining cleanup steps;
retain cleanup logging as appropriate. Also update the teardown in
tests/integration/flow/execution/call_depth_test.go (lines 80-97) so
application, flow, and organization-unit deletion failures fail the suite
without stopping subsequent cleanup.
🪄 Autofix
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: c029ecd7-27a7-4211-96ff-1e422bf5cb44
📒 Files selected for processing (2)
tests/integration/flow/execution/call_depth_test.gotests/integration/flow/mgt/flow_inference_test.go
Comment on lines
+146
to
+164
| func (ts *CallDepthTestSuite) TestExecute_ExceedingCallDepthRejected() { | ||
| step, err := common.InitiateAuthenticationFlow(ts.appID, false, nil, "") | ||
|
|
||
| // The engine may refuse the request outright or surface the failure on the step, depending on how | ||
| // far the chain unwinds before the limit trips. Either is acceptable; recursing without a limit is | ||
| // not. | ||
| if err != nil { | ||
| ts.Contains(err.Error(), fmt.Sprintf("%d", http.StatusBadRequest), | ||
| "a call chain past the depth limit should be rejected as a client error: %v", err) | ||
| return | ||
| } | ||
|
|
||
| ts.Require().NotNil(step, "expected a flow step for a rejected call chain") | ||
| ts.NotEqual("COMPLETE", step.FlowStatus, | ||
| "a call chain past the depth limit must not complete") | ||
| if step.Error != nil { | ||
| ts.Equal(errCodeMaxCallDepth, step.Error.Code, | ||
| "the failure should name the call depth limit") | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert FES-1013 in both error paths.
Lines 152-155 accept any HTTP 400 response. Lines 158-164 also accept a non-complete step when step.Error is nil. An unrelated validation or setup failure can pass this test. Require the maximum-depth error code in both paths.
backend/internal/flow/flowexec/error_constants.go:183-196 defines FES-1013 for this condition.
Proposed assertion change
if err != nil {
- ts.Contains(err.Error(), fmt.Sprintf("%d", http.StatusBadRequest),
- "a call chain past the depth limit should be rejected as a client error: %v", err)
+ ts.Contains(err.Error(), errCodeMaxCallDepth,
+ "a call chain past the depth limit should report the call-depth error: %v", err)
return
}
ts.Require().NotNil(step, "expected a flow step for a rejected call chain")
ts.NotEqual("COMPLETE", step.FlowStatus,
"a call chain past the depth limit must not complete")
-if step.Error != nil {
- ts.Equal(errCodeMaxCallDepth, step.Error.Code,
- "the failure should name the call depth limit")
-}
+ts.Require().NotNil(step.Error, "expected the call-depth error on the flow step")
+ts.Equal(errCodeMaxCallDepth, step.Error.Code,
+ "the failure should name the call depth limit")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (ts *CallDepthTestSuite) TestExecute_ExceedingCallDepthRejected() { | |
| step, err := common.InitiateAuthenticationFlow(ts.appID, false, nil, "") | |
| // The engine may refuse the request outright or surface the failure on the step, depending on how | |
| // far the chain unwinds before the limit trips. Either is acceptable; recursing without a limit is | |
| // not. | |
| if err != nil { | |
| ts.Contains(err.Error(), fmt.Sprintf("%d", http.StatusBadRequest), | |
| "a call chain past the depth limit should be rejected as a client error: %v", err) | |
| return | |
| } | |
| ts.Require().NotNil(step, "expected a flow step for a rejected call chain") | |
| ts.NotEqual("COMPLETE", step.FlowStatus, | |
| "a call chain past the depth limit must not complete") | |
| if step.Error != nil { | |
| ts.Equal(errCodeMaxCallDepth, step.Error.Code, | |
| "the failure should name the call depth limit") | |
| } | |
| func (ts *CallDepthTestSuite) TestExecute_ExceedingCallDepthRejected() { | |
| step, err := common.InitiateAuthenticationFlow(ts.appID, false, nil, "") | |
| // The engine may refuse the request outright or surface the failure on the step, depending on how | |
| // far the chain unwinds before the limit trips. Either is acceptable; recursing without a limit is | |
| // not. | |
| if err != nil { | |
| ts.Contains(err.Error(), errCodeMaxCallDepth, | |
| "a call chain past the depth limit should report the call-depth error: %v", err) | |
| return | |
| } | |
| ts.Require().NotNil(step, "expected a flow step for a rejected call chain") | |
| ts.NotEqual("COMPLETE", step.FlowStatus, | |
| "a call chain past the depth limit must not complete") | |
| ts.Require().NotNil(step.Error, "expected the call-depth error on the flow step") | |
| ts.Equal(errCodeMaxCallDepth, step.Error.Code, | |
| "the failure should name the call depth limit") |
🤖 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 `@tests/integration/flow/execution/call_depth_test.go` around lines 146 - 164,
Strengthen TestExecute_ExceedingCallDepthRejected to require errCodeMaxCallDepth
(FES-1013) in both rejection paths: validate the returned error contains the
maximum call-depth code instead of only HTTP 400, and require step.Error to be
non-nil with that code before accepting the step-based failure. Preserve the
existing not-complete assertion.
Comment on lines
+58
to
+73
| func (suite *FlowInferenceTestSuite) TearDownSuite() { | ||
| for _, flowID := range suite.createdFlowIDs { | ||
| if err := testutils.DeleteFlow(flowID); err != nil { | ||
| suite.T().Logf("teardown: failed to delete flow %s: %v", flowID, err) | ||
| } | ||
| } | ||
|
|
||
| if err := testutils.PatchDeploymentConfig(inferenceDisablePatch); err != nil { | ||
| suite.T().Logf("teardown: failed to restore inference config: %v", err) | ||
| } | ||
| if err := testutils.RestartServer(); err != nil { | ||
| suite.T().Logf("teardown: server did not restart cleanly after config restore: %v", err) | ||
| } | ||
| if err := testutils.ObtainAdminAccessToken(); err != nil { | ||
| suite.T().Logf("teardown: failed to re-obtain admin token after restore: %v", err) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail the suite when teardown cannot restore test state.
Both teardowns only log cleanup failures. A passing suite can therefore leave server configuration enabled, an unusable admin session, or resources with fixed handles. Later integration tests can run against contaminated state. Report each cleanup failure through the test failure mechanism, but continue the remaining cleanup steps.
tests/integration/flow/mgt/flow_inference_test.go#L58-L73: Fail the suite when flow configuration restoration, server restart, or admin-token recovery fails.tests/integration/flow/execution/call_depth_test.go#L80-L97: Fail the suite when application, flow, or organization-unit deletion fails.
📍 Affects 2 files
tests/integration/flow/mgt/flow_inference_test.go#L58-L73(this comment)tests/integration/flow/execution/call_depth_test.go#L80-L97
🤖 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 `@tests/integration/flow/mgt/flow_inference_test.go` around lines 58 - 73,
Update TearDownSuite in tests/integration/flow/mgt/flow_inference_test.go (lines
58-73) to report failures from PatchDeploymentConfig, RestartServer, and
ObtainAdminAccessToken through the test failure mechanism while continuing all
remaining cleanup steps; retain cleanup logging as appropriate. Also update the
teardown in tests/integration/flow/execution/call_depth_test.go (lines 80-97) so
application, flow, and organization-unit deletion failures fail the suite
without stopping subsequent cleanup.
senthalan
added
skip-changelog
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
2 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
Integration coverage for the flow engine and SSO sessions sat well below the 85% bar, with the gaps concentrated in error, resume, timeout and administration paths that unit tests cover only in isolation.
This adds 28 integration tests across seven areas. Measured statement coverage from the instrumented build (
target/coverage_integration.out):flow/sessionflow/mgtflow/flowexecflow/executorflow/core337 statements newly covered.
Approach
Flow execution error branches (
tests/integration/flow/execution/flow_execution_error_test.go)Six flow-execution error branches had no integration assertion at all. The three administration gates matter most, since
/flow/executeis a public path and these checks are the only thing between any caller and administration flow execution:Nested call depth (
tests/integration/flow/execution/call_depth_test.go)A chain of flows each calling the next, one longer than the engine allows, must be refused rather than recursing. The limit is what stops a mutually recursive set of flows, which the designer does not prevent an operator authoring, from exhausting the stack.
Registration flow inference (
tests/integration/flow/mgt/flow_inference_test.go)Creating an authentication flow with
flow.auto_infer_registrationenabled derives a registration flow from it, renamed and carrying the provisioning step that turns collected credentials into a user. The flag is off by default, so these tests patch the deployment configuration and restart, restoring on teardown. Also covers the flow-type executor requirements: a registration flow without a provisioning executor is rejected rather than stored.Flow usages (
tests/integration/flow/mgt/flow_usages_test.go)GET /flows/{flowId}/usageshad no test. Covers an unreferenced flow reporting a known-empty result, an application binding appearing as a usage with the fields the Console renders, and the not-found case.Execution lifecycle (
tests/integration/flow/execution/flow_lifecycle_test.go)Resume of an existing execution, refusal to resume without the required input, and context expiry. Expiry is driven by writing
authFlow.expirySecondsand needs no restart, because the flow section is read from merged server config on every execution. The original writable layer is restored on cleanup.Administration flow (
tests/integration/flow/execution/administration_flow_test.go)One execution of the shipped
default-user-deletion-flowdrives the whole chain: permission validation, pre-delete validation publishing the trusted revocation plan, criteria revocation, session termination, and record deletion. Plus the unknown-subject and missing-subject cases. This is the first integration coverage of the criteria-based revocation path.SSO session timeouts (
tests/integration/oauth/sso/session_timeout_test.go)Session timeouts are read once when the session service is constructed, so these tests write the session configuration and restart the server, restoring and restarting again on cleanup. The restore is registered before any change, so a mid-test failure cannot leave the run with second-scale session lifetimes.
The absolute-timeout test sets idle equal to absolute and uses the session mid-window. That slides the idle deadline past the absolute one, leaving the absolute cap as the only thing that can end the session, which is what distinguishes the two deadlines.
Notes for reviewers
Behaviours worth knowing, each of which cost a test run to discover:
AUTHENTICATIONflow must contain anAuthAssertExecutor(FLM-1023), so even a fixture flow that is never completed needs one./flow/executeas a public endpoint and skip token injection. Any test of the administration entry point has to set the bearer header itself on a raw client. This is documented in the helper.ERRORrather than re-presenting the prompt. That is now pinned by its own test.REGISTRATIONflow must carry both aUserTypeResolverand aProvisioningExecutor; the full table isrequiredExecutorsByFlowTypein the validator, alongside acompanionExecutorsmap that pairs executors which must appear together.PatchDeploymentConfigmerges at the top level only. Patching one key inside a nested block replaces the whole block, silently dropping its siblings. Doing that toflowdroppedmax_version_historyand broke two unrelated version-history tests in the same package, which no scoped test run could reveal. Both patches here restate the block exactly astests/integration/resources/deployment.yamlsets it.Known gaps
FES-1019(administration permission required) is not covered. A client credentials token is rejected as unauthenticated before permissions are consulted, because it establishes no user subject. Reaching that branch needs a signed-in non-administrator user. Recorded in the test file.flow/interceptorandflow/graphbuilderare unchanged, and deliberately so.CaptchaValidationProvideris an engine SDK extension point with no implementation or configuration in the server, so the captcha interceptor cannot execute in this deployment. The graph builder's error branches require a nil or node-less flow, or a structural build failure, both of which create-time validation rejects first. Both files already have unit tests, which is the right vehicle for defensive paths and SDK extension points.flow/executor, wherepasskey(2.4%),consent(3.0%) andfederated_auth_resolver(4.6%) hold over 500 uncovered statements needing WebAuthn and federated-IdP ceremony fixtures. That is separate work.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
🤖 Generated with Claude Code
Summary by CodeRabbit