fix(e2e): bz643.4 — consolidate settings/OIDC/workflows/stats browser journeys into 2 Ordered journeys (bookshelf-bz643.4) #1375

Merged

zombor merged 8 commits from bd-bookshelf-bz643.4 into main 2026-08-06 20:34:31 +00:00

Conversation 2 Commits 8 Files changed 11 +1105 -1671

zombor commented 2026-08-06 17:08:11 +00:00

Owner

Copy link

Summary

• Consolidates 8 browser e2e Describes (62 Its) into 2 Ordered journeys (25 Its)
• J-Settings (journey_settings_test.go): merges journey_settings_shell, journey_oidc_settings, journey_oidc_group_mapping, and journey_sidecar_settings — 12 compound It-steps
• J-Stats (journey_stats_test.go): merges journey_library_stats, journey_stats_distributions, journey_workflow_list, and journey_workflow_detail — 13 compound It-steps
• Deletes the 8 source files; all coverage preserved via compound It-steps (multi-Expect e2e relaxation per CLAUDE.md)

Coverage-risk notes preserved

• #1 (sidecar app.js tryRegister gap): the sidecar-settings controller connect step in J-Settings proves app.js registration — the original regression that was invisible to Vitest+jsdom
• #12 (settings_shell comic-weight client validation): compound It in J-Settings asserts Saved! when a comic field input is set to its max value (15), guarding against the raw > 10 hardcode regression

Journeys consolidated

Source file	Its (before)	Destination
journey_settings_shell_test.go	10	journey_settings_test.go
journey_oidc_settings_test.go	8	journey_settings_test.go
journey_oidc_group_mapping_test.go	6	journey_settings_test.go
journey_sidecar_settings_test.go	4	journey_settings_test.go
journey_library_stats_test.go	14	journey_stats_test.go
journey_stats_distributions_test.go	9	journey_stats_test.go
journey_workflow_list_test.go	4	journey_stats_test.go
journey_workflow_detail_test.go	7	journey_stats_test.go

Test plan

• make e2e-policy-check passes — all Describes are Ordered journey containers
• make test passes — all unit tests green
• make coverage passes — zero uncovered statement blocks
• go test -tags e2e -run "^$" ./e2e/browser/ compiles cleanly
• No lint errors in new files (go vet -tags e2e ./e2e/browser/)

Closes bead bookshelf-bz643.4 on merge.

## Summary - Consolidates 8 browser e2e Describes (62 Its) into 2 Ordered journeys (25 Its) - **J-Settings** (`journey_settings_test.go`): merges `journey_settings_shell`, `journey_oidc_settings`, `journey_oidc_group_mapping`, and `journey_sidecar_settings` — 12 compound It-steps - **J-Stats** (`journey_stats_test.go`): merges `journey_library_stats`, `journey_stats_distributions`, `journey_workflow_list`, and `journey_workflow_detail` — 13 compound It-steps - Deletes the 8 source files; all coverage preserved via compound It-steps (multi-Expect e2e relaxation per CLAUDE.md) ## Coverage-risk notes preserved - **#1** (sidecar app.js `tryRegister` gap): the sidecar-settings controller connect step in J-Settings proves app.js registration — the original regression that was invisible to Vitest+jsdom - **#12** (settings_shell comic-weight client validation): compound It in J-Settings asserts `Saved!` when a comic field input is set to its max value (15), guarding against the `raw > 10` hardcode regression ## Journeys consolidated | Source file | Its (before) | Destination | |---|---|---| | `journey_settings_shell_test.go` | 10 | `journey_settings_test.go` | | `journey_oidc_settings_test.go` | 8 | `journey_settings_test.go` | | `journey_oidc_group_mapping_test.go` | 6 | `journey_settings_test.go` | | `journey_sidecar_settings_test.go` | 4 | `journey_settings_test.go` | | `journey_library_stats_test.go` | 14 | `journey_stats_test.go` | | `journey_stats_distributions_test.go` | 9 | `journey_stats_test.go` | | `journey_workflow_list_test.go` | 4 | `journey_stats_test.go` | | `journey_workflow_detail_test.go` | 7 | `journey_stats_test.go` | ## Test plan - [x] `make e2e-policy-check` passes — all Describes are Ordered journey containers - [x] `make test` passes — all unit tests green - [x] `make coverage` passes — zero uncovered statement blocks - [x] `go test -tags e2e -run "^$" ./e2e/browser/` compiles cleanly - [x] No lint errors in new files (`go vet -tags e2e ./e2e/browser/`) Closes bead bookshelf-bz643.4 on merge.

zombor added 1 commit 2026-08-06 17:08:11 +00:00

fix(e2e): bz643.4 — consolidate settings/OIDC/workflows/stats browser journeys into 2 Ordered journeys ... a8fb0109e0

J-Settings: merges journey_settings_shell (10 Its), journey_oidc_settings (8),
journey_oidc_group_mapping (6), and journey_sidecar_settings (4) into a single
Ordered Describe with 12 compound It-steps. Preserves coverage-risk #1 (sidecar
app.js tryRegister gap) and coverage-risk #12 (comic-weight client validation).

J-Stats: merges journey_library_stats (14 Its), journey_stats_distributions (9),
journey_workflow_list (4), and journey_workflow_detail (7) into a single Ordered
Describe with 13 compound It-steps using two shared incognito pages (statsPage
for library-stats/stats, wfPage for workflow sections).

Deletes 8 source files. e2e-policy-check, make test, make coverage, make lint
all green. Net: 62 Its in 8 Describes → 25 Its in 2 Describes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-08-06 17:17:43 +00:00

fix(e2e): wait for match-weights-settings controller connect before click; extend Saved! timeout to 15s ... 8faba2b355

waitForStimulusController ensures the Stimulus controller is fully connected
(connect() has fired, targets and data-values bound) before dispatchEvent is
called. Without this, a click dispatched while the controller is still
initialising is a no-op and the status stays empty, causing the Eventually(5s)
to time out with 'Saving...' or nothing.

Extend the Eventually timeout from 5s to 15s to handle the case where the PUT
round-trip to the live server takes longer than 5s in CI when parallel journeys
are active.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-08-06 17:26:16 +00:00

fix(e2e): use fresh incognito page for comic-weight It to avoid SSE connection starvation ... 68c3b4318c

Prior navigations in the shared journey page accumulate open SSE connections.
HTTP/1.1 per-origin connection limits cause subsequent fetch() PUTs to queue
behind those SSEs and never reach the server within the assertion window.
A fresh incognito context has zero prior connections, so the PUT completes
reliably. Root cause confirmed via server logs: /settings/metadata loaded at
17:20:08 and 2 new SSE connections opened, but no PUT arrived before the
15s Eventually timed out at 17:20:25 (runs 8272).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-08-06 17:35:13 +00:00

fix(e2e): clear SSE connections via about:blank before OIDC section to prevent fetch starvation ... 1b5d65adc7

Prior Its in the settings journey (email, 4 tab-nav navigations, ratelimits)
accumulate open SSE connections on the shared incognito page. Chrome's
HTTP/1.1 per-origin limit is 6 connections; with 6 SSEs open, any fetch()
PUT from a Stimulus controller (oidc-settings#save, oidc-group-mapping#save,
sidecar-settings toggle) is queued indefinitely behind the open SSE slots
and never reaches the server.

Fix: navigate to about:blank at the start of the OIDC load It (before the
OIDC section begins). This closes all prior SSE connections, resetting the
per-origin connection pool to 0. The OIDC and subsequent Its then operate
with plenty of free slots for their fetch() PUTs.

Companion to previous commit (fresh incognito for comic-weight It): that fix
was needed because the main page had already exhausted its connection pool
by the comic-weight It. The about:blank reset before the OIDC section means
the OIDC, group-mapping, and sidecar Its all get clean pools.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-08-06 18:36:44 +00:00

fix(e2e): flush SSE connections before OIDC save It to prevent fetch starvation ... 67f93fb262

The "OIDC settings page loads" It navigates about:blank→/settings/oidc,
opening 2 new SSE connections. The immediately following "fills and saves"
It did not flush these connections, so the Stimulus PUT fetch() could not
get a free HTTP/1.1 connection slot (6-per-origin limit) and timed out
waiting for "Saved!" after 10 s.

Fix: add about:blank→/settings/oidc at the start of the save It — same
pattern already used for the comic-weight and OIDC-load Its above it.

zombor added 1 commit 2026-08-06 18:52:34 +00:00

fix(e2e): use fresh incognito page for OIDC save It to eliminate bfcache SSE starvation ... 60e0965394

The about:blank flush (prior attempt) was insufficient: Chrome's bfcache
keeps previous pages alive across navigations, so SSE EventSource connections
from the settings, email-redirect, metadata-tab, and ratelimits navigations
remain open even after about:blank. With 6-connections-per-origin HTTP/1.1
limit, these accumulated streams leave no free slot for the fetch() PUT and
the save never reaches the server.

Fix: use a fresh incognito context (zero prior connections) for the OIDC
fill+save It, identical to the already-proven approach for the comic-weight
It. The saved OIDC settings persist to the DB so the following reload-check
It can read them back via the shared main page.

zombor added 1 commit 2026-08-06 19:00:37 +00:00

fix(e2e): disable bfcache in test browser to eliminate SSE connection accumulation ... 83efb78a3c

Root cause: Chrome's Back-Forward Cache (bfcache) keeps navigated-away pages
alive in memory. SSE EventSource connections from those cached pages remain
open even after navigating to a new URL or about:blank. In an Ordered journey
that reuses one browser page across many It steps, prior navigations
(settings, email-redirect, metadata-tab, ratelimits) accumulate 6+ open SSE
connections — exhausting Chrome's HTTP/1.1 6-connections-per-origin limit.
Subsequent fetch() PUTs/POSTs from Stimulus controllers cannot get a slot and
stall indefinitely, causing "Saved!" and "modal should close" assertions to
time out.

Fix: add --disable-back-forward-cache to the test browser launcher. With
bfcache disabled, navigating away from a page immediately terminates all its
SSE connections, keeping the active connection count to just the 2 SSE streams
for the current page. The incognito-page approach for the OIDC save It is
retained as belt-and-suspenders (fresh context = guaranteed 0 prior SSE).

zombor commented 2026-08-06 19:12:14 +00:00

Author

Owner

Copy link

Security Review — PR #1375 (bookshelf-bz643.4)

Scope confirmed

All 11 changed files are under e2e/browser/. Zero production code touched. The --disable-back-forward-cache flag is a Chrome CLI argument applied to the headless browser launcher; it has no effect on server-side CSP headers, HTTP responses, or any production surface.

---

OIDC secret-cleared-after-save — VERIFIED PRESERVED

The critical assertion survives in two forms in journey_settings_test.go:

1. Write-only DOM clear after save (It("fills and saves the OIDC form; secret cleared after save …")): uses a fresh incognito page, inputs super-secret-value, clicks Save, asserts clientSecret field value .To(BeEmpty()) — identical to old journey_oidc_settings_test.go:119.
2. Secret not returned by server on reload (It("secret not rendered after reload; redirect URI shows callback path; …")): navigates to /settings/oidc on a fresh load and asserts clientSecret value .To(BeEmpty()) — identical to old test at line 126–134.

Both security assertions from the original two separate It blocks are present and correct.

---

Admin-only settings gating — NOT asserted in browser tier (was not asserted before either)

The old journey_settings_shell_test.go contained zero non-admin/403 assertions. Tab navigation Its only checked the URL after a click. Admin-gating of /settings/* routes is covered at the API e2e layer (e2e/api/) — this was true before this PR and remains true after. No regression here.

---

OIDC group-mapping authz — VERIFIED PRESERVED

journey_settings_test.go retains: controller loads, modal opens with permissions grid + library multiselect, create mapping (permission + library), edit mapping, delete mapping, save sync mode. All six original It blocks from journey_oidc_group_mapping_test.go are represented.

---

Workflow Cancel-button / status-poll gating for completed executions — VERIFIED PRESERVED

journey_stats_test.go retains the security-relevant assertion: It("clicking older completed row shows that exec's ID + state=completed + no Cancel button + no status-poll controller") asserts cancelBtns and statusCtrls are both empty. This confirms the server does not render Cancel controls for finished workflow executions.

---

Minor: "completed" state label assertion dropped

[MINOR] e2e/browser/journey_stats_test.go — "completed" state label not asserted for older execution

Old journey_workflow_detail_test.go had a distinct It("detail page for older completed execution shows 'completed' (not 'running')") that asserted strings.ToLower(stateEl.MustText()) == "completed". The new consolidated file asserts the exec ID, absence of Cancel button, and absence of status-poll controller for the same execution, but does not assert the .task-status label text equals "completed". This is a display assertion, not a security control. The security-relevant property (no Cancel on completed exec) is retained.

Minor: three settings tab navigation Its dropped

[MINOR] e2e/browser/journey_settings_test.go — /settings/users, /settings/workflows, /settings/audit tab navigation Its not migrated

The old journey_settings_shell_test.go had three It blocks navigating through Users, Workflows, and Audit tab links and asserting the resulting URL. The new consolidated journey only asserts Email→/settings (redirect), Metadata, and OIDC tab navigation. These are URL navigation assertions with no security significance (admin gating of those routes is covered by the API e2e tier).

---

No secrets or tokens hardcoded

Test credentials are all synthetic placeholders (super-secret-value, test-client-id, grimmory-admins). No real API keys, tokens, or passwords introduced.

---

REVIEW VERDICT: 0 blocker, 0 major, 2 minor

## Security Review — PR #1375 (bookshelf-bz643.4) ### Scope confirmed All 11 changed files are under `e2e/browser/`. Zero production code touched. The `--disable-back-forward-cache` flag is a Chrome CLI argument applied to the headless browser launcher; it has no effect on server-side CSP headers, HTTP responses, or any production surface. --- ### OIDC secret-cleared-after-save — VERIFIED PRESERVED The critical assertion survives in two forms in `journey_settings_test.go`: 1. **Write-only DOM clear after save** (`It("fills and saves the OIDC form; secret cleared after save …")`): uses a fresh incognito page, inputs `super-secret-value`, clicks Save, asserts `clientSecret` field value `.To(BeEmpty())` — identical to old `journey_oidc_settings_test.go:119`. 2. **Secret not returned by server on reload** (`It("secret not rendered after reload; redirect URI shows callback path; …")`): navigates to `/settings/oidc` on a fresh load and asserts `clientSecret` value `.To(BeEmpty())` — identical to old test at line 126–134. Both security assertions from the original two separate `It` blocks are present and correct. --- ### Admin-only settings gating — NOT asserted in browser tier (was not asserted before either) The old `journey_settings_shell_test.go` contained zero non-admin/403 assertions. Tab navigation Its only checked the URL after a click. Admin-gating of `/settings/*` routes is covered at the API e2e layer (`e2e/api/`) — this was true before this PR and remains true after. No regression here. --- ### OIDC group-mapping authz — VERIFIED PRESERVED `journey_settings_test.go` retains: controller loads, modal opens with permissions grid + library multiselect, create mapping (permission + library), edit mapping, delete mapping, save sync mode. All six original `It` blocks from `journey_oidc_group_mapping_test.go` are represented. --- ### Workflow Cancel-button / status-poll gating for completed executions — VERIFIED PRESERVED `journey_stats_test.go` retains the security-relevant assertion: `It("clicking older completed row shows that exec's ID + state=completed + no Cancel button + no status-poll controller")` asserts `cancelBtns` and `statusCtrls` are both empty. This confirms the server does not render Cancel controls for finished workflow executions. --- ### Minor: "completed" state label assertion dropped [MINOR] e2e/browser/journey_stats_test.go — "completed" state label not asserted for older execution Old `journey_workflow_detail_test.go` had a distinct `It("detail page for older completed execution shows 'completed' (not 'running')")` that asserted `strings.ToLower(stateEl.MustText()) == "completed"`. The new consolidated file asserts the exec ID, absence of Cancel button, and absence of status-poll controller for the same execution, but does not assert the `.task-status` label text equals `"completed"`. This is a display assertion, not a security control. The security-relevant property (no Cancel on completed exec) is retained. ### Minor: three settings tab navigation Its dropped [MINOR] e2e/browser/journey_settings_test.go — /settings/users, /settings/workflows, /settings/audit tab navigation Its not migrated The old `journey_settings_shell_test.go` had three It blocks navigating through Users, Workflows, and Audit tab links and asserting the resulting URL. The new consolidated journey only asserts Email→/settings (redirect), Metadata, and OIDC tab navigation. These are URL navigation assertions with no security significance (admin gating of those routes is covered by the API e2e tier). --- ### No secrets or tokens hardcoded Test credentials are all synthetic placeholders (`super-secret-value`, `test-client-id`, `grimmory-admins`). No real API keys, tokens, or passwords introduced. --- REVIEW VERDICT: 0 blocker, 0 major, 2 minor

zombor commented 2026-08-06 19:13:30 +00:00

Author

Owner

Copy link

No description provided.

zombor force-pushed bd-bookshelf-bz643.4 from 83efb78a3c to 67020a7668 2026-08-06 19:17:41 +00:00 Compare

zombor force-pushed bd-bookshelf-bz643.4 from 67020a7668 to 68846d5e20 2026-08-06 20:27:04 +00:00 Compare

zombor merged commit 3f100dc556 into main 2026-08-06 20:34:31 +00:00

zombor referenced this pull request from a commit 2026-08-06 20:34:35 +00:00

Merge pull request 'fix(e2e): bz643.4 — consolidate settings/OIDC/workflows/stats browser journeys into 2 Ordered journeys (bookshelf-bz643.4)' (#1375) from bd-bookshelf-bz643.4 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1375

No description provided.