zombor/pergamum
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages 2 Wiki Activity Actions

feat(settings): live-reconcile Running workflows section [shot:workflow-running-section] (bookshelf-17i2p) #1217

Closed
zombor wants to merge 1 commit from bd-bookshelf-17i2p into main
pull from: bd-bookshelf-17i2p
merge into: zombor:main
Conversation 4 Commits 1 Files changed 8 +876 -14
zombor commented 2026-07-24 00:22:12 +00:00
Owner
Copy link

Summary

  • Root cause (client-side, verified): the Running section <tbody> had no list controller — workflow-status updated the state badge in-place but nothing removed completed rows and nothing added new running rows (which only landed in History via workflow-list).
  • Fix: new workflow-running Stimulus controller on the Running section div polls GET /admin/wf-running every 5 s and reconciles the tbody: adds rows for newly-running instances, removes rows whose instance_id is no longer in the running feed, updates the count badge.
  • New endpoint GET /admin/wf-running returns {instances:[...], capped:bool} backed by the existing ListRunningInstances (bounded to 500 rows, cap+1 to detect overflow).
  • Removed data-controller="workflow-status" from Running rows — rows disappear via reconciler instead of lingering with a COMPLETED badge.
  • Poll interval is 5 s (vs 2 s for History) per the bead spec, configurable via data-workflow-running-interval-value.

Test plan

  • RunningInstancesHandler unit tests: 200 OK, JSON content-type, cap+1 passed to listRunning, capped=false default, overflow trims to 500 + capped=true, 500 on listRunning error
  • RegisterRoutes test: GET /admin/wf-running returns 200 (allowed) / 403 (denied)
  • Vitest workflow_running_controller.test.js (16 tests): connect starts timer, fetches URL, adds new running row, sets data-instance-id, no-duplicate on repeat poll, removes row leaving running set, keeps row still running, updates count badge, hides badge at 0, shows badge when >0, stops timer on disconnect, silent on network error, silent on non-OK HTTP, parent_id link, no parent_id "—", no execution_id, no count target
  • 100% JS coverage gate
  • All 4284 JS tests pass, all Go unit tests pass

Closes bead bookshelf-17i2p on merge.

## Summary - **Root cause (client-side, verified):** the Running section `<tbody>` had no list controller — `workflow-status` updated the state badge in-place but nothing removed completed rows and nothing added new running rows (which only landed in History via `workflow-list`). - **Fix:** new `workflow-running` Stimulus controller on the Running section div polls `GET /admin/wf-running` every 5 s and **reconciles** the tbody: adds rows for newly-running instances, removes rows whose `instance_id` is no longer in the running feed, updates the count badge. - **New endpoint** `GET /admin/wf-running` returns `{instances:[...], capped:bool}` backed by the existing `ListRunningInstances` (bounded to 500 rows, cap+1 to detect overflow). - Removed `data-controller="workflow-status"` from Running rows — rows disappear via reconciler instead of lingering with a COMPLETED badge. - Poll interval is 5 s (vs 2 s for History) per the bead spec, configurable via `data-workflow-running-interval-value`. ## Test plan - [x] `RunningInstancesHandler` unit tests: 200 OK, JSON content-type, cap+1 passed to listRunning, capped=false default, overflow trims to 500 + capped=true, 500 on listRunning error - [x] `RegisterRoutes` test: GET /admin/wf-running returns 200 (allowed) / 403 (denied) - [x] Vitest `workflow_running_controller.test.js` (16 tests): connect starts timer, fetches URL, adds new running row, sets data-instance-id, no-duplicate on repeat poll, removes row leaving running set, keeps row still running, updates count badge, hides badge at 0, shows badge when >0, stops timer on disconnect, silent on network error, silent on non-OK HTTP, parent_id link, no parent_id "—", no execution_id, no count target - [x] 100% JS coverage gate - [x] All 4284 JS tests pass, all Go unit tests pass Closes bead bookshelf-17i2p on merge.
workflows-running-section.png
64 KiB
workflows-running-section.png
feat(settings): live-reconcile Running workflows section — workflow-running Stimulus controller + /admin/wf-running endpoint (bookshelf-17i2p)
All checks were successful
/ JS Unit Tests (pull_request) Successful in 1m55s
Details
/ Test Race (pull_request) Successful in 4m1s
Details
/ E2E API (pull_request) Successful in 3m20s
Details
/ Coverage (pull_request) Successful in 4m36s
Details
/ Lint (pull_request) Successful in 6m18s
Details
/ Integration (pull_request) Successful in 6m8s
Details
/ E2E Browser (pull_request) Successful in 6m34s
Details
c9b6fe1d33
 
Root cause: Running section was a frozen page-load snapshot. Completed
workflows lingered (workflow-status flipped the badge to COMPLETED in place
but nothing removed the row); newly-started running workflows only landed in
the History section, never in Running.

Fix:
- Add GET /admin/wf-running endpoint (RunningInstancesHandler) backed by the
  existing ListRunningInstances; returns {instances:[...], capped:bool} with
  cap=500 and overflow detection.
- New Stimulus controller workflow_running_controller.js (data-controller=
  "workflow-running") attached to the Running section outer div; polls
  /admin/wf-running every 5s and RECONCILES the tbody: appends rows for new
  running instances, removes rows whose instance_id is no longer in the running
  set, updates the "Running N" count badge.
- Template: remove data-controller="workflow-status" from Running rows (rows
  disappear via reconciler instead of lingering with a COMPLETED badge); add
  data-workflow-running-target="body" on <tbody> and data-workflow-running-
  target="count" on the badge span; always render the badge span (display:none
  when empty, JS shows it when count > 0).
- Register GET /admin/wf-running in RegisterRoutes.
- Register workflow-running in app.js.
- Add <script> tag in settings_shell.html page_scripts block.

Tests: GoUnit RunningInstancesHandler (error, cap, overflow, capped=false
default, cap+1 passed to listRunning); register_routes_test covers new route
with allow/deny; Vitest workflow_running_controller covers connect, reconcile
add/remove/no-dup, count badge hide/show/update, disconnect, network errors,
parent_id, empty execution_id, no count target (16 tests). 100% JS coverage.

Closes bead bookshelf-17i2p on merge.
forgejo-actions commented 2026-07-24 00:28:21 +00:00
Copy link

Workflow Running section screenshot (workflows-running-section)

workflows-running-section

**Workflow Running section screenshot** (workflows-running-section) ![workflows-running-section](/attachments/5ac256ad-c5fa-4b3f-80d3-18b51febc78c)
zombor commented 2026-07-24 12:54:07 +00:00
Author
Owner
Copy link

Security Review — PR #1217 (/admin/wf-running running-instances poll endpoint)

Scope: new GET /admin/wf-running JSON endpoint + 5s reconciling poller.

AuthZ (primary focus): PASS. internal/wfengine/engine.go:1634 registers the route as
mux.Handle("GET /admin/wf-running", adminRequired(RunningInstancesHandler(listRunning, logger)))
— wrapped in the exact same adminRequired middleware as the sibling /admin/wf-instances
(line 1633) and every other wf-* route. register_routes_test.go asserts 403 on the deny
path. An unauthenticated / non-admin caller cannot enumerate running instances.

Data exposure: PASS. Reuses the existing WorkflowInstance struct
(diag_accessor.go:23 — instance_id, execution_id, parent_id, created_at, completed_at,
state, queue). No new fields; identical to what the server-rendered Running table already
exposed. No raw activity args, secrets, or per-user data in the payload.

DoS: PASS. Admin-gated and bounded — runningInstancesCap=500 with LIMIT ?
(diag_accessor.go:151-160); handler fetches cap+1 and trims. The 5s poll surface is
bounded to authenticated admins.

Injection: PASS. Endpoint accepts no request params (cap hardcoded to
runningInstancesCap+1, running_instances_handler.go:112); runningInstancesSQL is fully
parameterized (LIMIT ?). Client-side _buildRow escapes every interpolation via _esc()
(HTML) and encodeURIComponent (hrefs) — no XSS from instance/parent IDs.

Findings

[MINOR] templates/pages/settings_shell.html:964 — inline style="display:none" on the count badge
The settings page runs under the default CSP style-src 'self' (no unsafe-inline,
internal/middleware/security_headers.go:27), so the server-rendered
{{if not .WorkflowRunning}}style="display:none"{{end}} inline style attribute is blocked
by the browser — the badge shows "0" instead of being hidden on initial load. It self-heals
within 5s once the JS poller runs _updateCount() (which sets style.display via the DOM
property, allowed by CSP), so the impact is a brief cosmetic glitch, not a security issue.
Fix per the project's no-inline-style-CSP convention: render a hidden CSS class (e.g.
class="badge is-hidden") toggled by the controller instead of an inline style= attribute.

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## Security Review — PR #1217 (`/admin/wf-running` running-instances poll endpoint) Scope: new `GET /admin/wf-running` JSON endpoint + 5s reconciling poller. **AuthZ (primary focus): PASS.** `internal/wfengine/engine.go:1634` registers the route as `mux.Handle("GET /admin/wf-running", adminRequired(RunningInstancesHandler(listRunning, logger)))` — wrapped in the exact same `adminRequired` middleware as the sibling `/admin/wf-instances` (line 1633) and every other `wf-*` route. `register_routes_test.go` asserts 403 on the deny path. An unauthenticated / non-admin caller cannot enumerate running instances. **Data exposure: PASS.** Reuses the existing `WorkflowInstance` struct (`diag_accessor.go:23` — instance_id, execution_id, parent_id, created_at, completed_at, state, queue). No new fields; identical to what the server-rendered Running table already exposed. No raw activity args, secrets, or per-user data in the payload. **DoS: PASS.** Admin-gated and bounded — `runningInstancesCap`=500 with `LIMIT ?` (`diag_accessor.go:151-160`); handler fetches cap+1 and trims. The 5s poll surface is bounded to authenticated admins. **Injection: PASS.** Endpoint accepts no request params (cap hardcoded to `runningInstancesCap+1`, `running_instances_handler.go:112`); `runningInstancesSQL` is fully parameterized (`LIMIT ?`). Client-side `_buildRow` escapes every interpolation via `_esc()` (HTML) and `encodeURIComponent` (hrefs) — no XSS from instance/parent IDs. ### Findings [MINOR] templates/pages/settings_shell.html:964 — inline `style="display:none"` on the count badge The settings page runs under the default CSP `style-src 'self'` (no `unsafe-inline`, `internal/middleware/security_headers.go:27`), so the server-rendered `{{if not .WorkflowRunning}}style="display:none"{{end}}` inline style attribute is blocked by the browser — the badge shows "0" instead of being hidden on initial load. It self-heals within 5s once the JS poller runs `_updateCount()` (which sets `style.display` via the DOM property, allowed by CSP), so the impact is a brief cosmetic glitch, not a security issue. Fix per the project's no-inline-style-CSP convention: render a hidden CSS class (e.g. `class="badge is-hidden"`) toggled by the controller instead of an inline `style=` attribute. REVIEW VERDICT: 0 blocker, 0 major, 1 minor
zombor commented 2026-07-24 12:55:38 +00:00
Author
Owner
Copy link

UI Review — PR #1217 (bookshelf-17i2p)

Screenshot reviewed: comment 14920 attachment /attachments/5ac256ad-c5fa-4b3f-80d3-18b51febc78c (1328x800 PNG confirmed)

What I see

The Workflows page renders with a RUNNING section ("RUNNING" heading + "0" count badge) and a HISTORY section below. Both tables use .table workflows-table with identical column structure (Instance ID / Parent / State / Created / Completed). The HISTORY section shows one COMPLETED row with a teal .task-status badge. Typography, spacing, and table styling are visually consistent between sections. The workflow-running Stimulus controller correctly wires data-workflow-running-target="body" to the <tbody> and data-workflow-running-target="count" to the badge.

Findings

[MAJOR] templates/pages/settings_shell.html:1764 — inline style="display:none" on count badge violates CSP
  The template emits style="display:none" on the <span class="badge"> when there are
  zero running workflows. The production CSP enforces style-src 'self', which blocks
  inline style= attributes. The badge is NOT hidden and renders "0" visibly — confirmed
  in the screenshot: the "0" badge is visible when it should be invisible.
  The JS controller _updateCount() sets element.style.display directly (JS-set styles
  bypass style-src) so the JS side is fine; only the SSR initial state is wrong.

  Fix: use the hidden boolean attribute + a CSS override rule matching the existing
  pattern at static/css/main.css:1747 (.books-select-bar[hidden] { display:none }):
    Template: <span class="badge" data-workflow-running-target="count"
              {{if not .WorkflowRunning}}hidden{{end}}>{{len .WorkflowRunning}}</span>
    CSS: .badge[hidden] { display: none; }
  The JS _updateCount() can then toggle the hidden attribute (removeAttribute/setAttribute)
  instead of element.style.display.

Other checks (no issues)

  • RUNNING section uses .table workflows-table — same canonical class as HISTORY, no bespoke drift
  • Column structure matches HISTORY exactly (Instance ID / Parent / State / Created / Completed)
  • Server-rendered rows and JS-built rows both use .task-status task-status--running — canonical badge class
  • _buildRow() in workflow_running_controller.js produces the same HTML structure and canonical classes as the template rows
  • Modals elsewhere in the file continue to use .modal-overlay / .modal-dialog / .modal-header / .modal-body / .modal-footer — no regressions from this diff
  • No other style= attributes introduced in this diff
  • <p class="empty-state">No running workflows.</p> renders correctly
  • The <table> is always rendered (tbody always present for JS reconciliation) — correct
REVIEW VERDICT: 0 blocker, 1 major, 0 minor
## UI Review — PR #1217 (bookshelf-17i2p) **Screenshot reviewed:** comment 14920 attachment `/attachments/5ac256ad-c5fa-4b3f-80d3-18b51febc78c` (1328x800 PNG confirmed) ### What I see The Workflows page renders with a RUNNING section ("RUNNING" heading + "0" count badge) and a HISTORY section below. Both tables use `.table workflows-table` with identical column structure (Instance ID / Parent / State / Created / Completed). The HISTORY section shows one COMPLETED row with a teal `.task-status` badge. Typography, spacing, and table styling are visually consistent between sections. The `workflow-running` Stimulus controller correctly wires `data-workflow-running-target="body"` to the `<tbody>` and `data-workflow-running-target="count"` to the badge. --- ### Findings ``` [MAJOR] templates/pages/settings_shell.html:1764 — inline style="display:none" on count badge violates CSP The template emits style="display:none" on the <span class="badge"> when there are zero running workflows. The production CSP enforces style-src 'self', which blocks inline style= attributes. The badge is NOT hidden and renders "0" visibly — confirmed in the screenshot: the "0" badge is visible when it should be invisible. The JS controller _updateCount() sets element.style.display directly (JS-set styles bypass style-src) so the JS side is fine; only the SSR initial state is wrong. Fix: use the hidden boolean attribute + a CSS override rule matching the existing pattern at static/css/main.css:1747 (.books-select-bar[hidden] { display:none }): Template: <span class="badge" data-workflow-running-target="count" {{if not .WorkflowRunning}}hidden{{end}}>{{len .WorkflowRunning}}</span> CSS: .badge[hidden] { display: none; } The JS _updateCount() can then toggle the hidden attribute (removeAttribute/setAttribute) instead of element.style.display. ``` ### Other checks (no issues) - RUNNING section uses `.table workflows-table` — same canonical class as HISTORY, no bespoke drift - Column structure matches HISTORY exactly (Instance ID / Parent / State / Created / Completed) - Server-rendered rows and JS-built rows both use `.task-status task-status--running` — canonical badge class - `_buildRow()` in `workflow_running_controller.js` produces the same HTML structure and canonical classes as the template rows - Modals elsewhere in the file continue to use `.modal-overlay` / `.modal-dialog` / `.modal-header` / `.modal-body` / `.modal-footer` — no regressions from this diff - No other `style=` attributes introduced in this diff - `<p class="empty-state">No running workflows.</p>` renders correctly - The `<table>` is always rendered (tbody always present for JS reconciliation) — correct --- ``` REVIEW VERDICT: 0 blocker, 1 major, 0 minor ```
zombor commented 2026-07-24 12:59:28 +00:00
Author
Owner
Copy link

[MAJOR] templates/pages/settings_shell.html:1793 + static/js/controllers/workflow_running_controller.js — empty-state message visibility regression when workflows complete

The template now ALWAYS renders the element (removed {{if .WorkflowRunning}}), but the empty-state "No running workflows" message is still conditional on .WorkflowRunning being empty ONLY at page-load time.

Regression scenario:

  1. Page loads with running workflows → table rendered with rows
  2. JS controller polls every 5s and reconciles rows
  3. All workflows complete → controller removes all rows from tbody
  4. BUG: Empty table with headers is shown, but empty-state message is NOT shown (never server-rendered)
  5. User sees broken UX (empty table instead of friendly message)

Root cause: The controller's _reconcile() and _updateCount() manage row addition/removal and badge visibility, but do NOT manage empty-state message visibility or table visibility.

Test gap: vitest setup does not render empty-state element, so no test covers empty→rows→empty transitions. The test "hides the count badge when there are no running instances" removes a row but doesn't verify empty-state message visibility (which wasn't even in the test DOM).

Fix options:
(a) Always server-render empty-state message (remove {{if not .WorkflowRunning}} conditional) + controller shows/hides it based on row count
(b) Restore original template conditional structure + adjust controller to work with permanently-mounted tbody
(c) Add controller logic to hide table and show empty-state when row count becomes 0

Add test cases:

  • Poll returns new instance when starting empty → verify row is added
  • Poll returns empty when starting with rows → verify row is removed + empty-state visibility is correct (once implemented)

REVIEW VERDICT: 1 major, 0 minor

[MAJOR] templates/pages/settings_shell.html:1793 + static/js/controllers/workflow_running_controller.js — empty-state message visibility regression when workflows complete The template now ALWAYS renders the <table> element (removed {{if .WorkflowRunning}}), but the empty-state "No running workflows" message is still conditional on .WorkflowRunning being empty ONLY at page-load time. Regression scenario: 1. Page loads with running workflows → table rendered with rows 2. JS controller polls every 5s and reconciles rows 3. All workflows complete → controller removes all rows from tbody 4. BUG: Empty table with headers is shown, but empty-state message is NOT shown (never server-rendered) 5. User sees broken UX (empty table instead of friendly message) Root cause: The controller's _reconcile() and _updateCount() manage row addition/removal and badge visibility, but do NOT manage empty-state message visibility or table visibility. Test gap: vitest setup does not render empty-state element, so no test covers empty→rows→empty transitions. The test "hides the count badge when there are no running instances" removes a row but doesn't verify empty-state message visibility (which wasn't even in the test DOM). Fix options: (a) Always server-render empty-state message (remove {{if not .WorkflowRunning}} conditional) + controller shows/hides it based on row count (b) Restore original template conditional structure + adjust controller to work with permanently-mounted tbody (c) Add controller logic to hide table and show empty-state when row count becomes 0 Add test cases: - Poll returns new instance when starting empty → verify row is added - Poll returns empty when starting with rows → verify row is removed + empty-state visibility is correct (once implemented) REVIEW VERDICT: 1 major, 0 minor
zombor closed this pull request 2026-07-24 16:48:27 +00:00
All checks were successful
/ JS Unit Tests (pull_request) Successful in 1m55s
Details
/ Test Race (pull_request) Successful in 4m1s
Required
Details
/ E2E API (pull_request) Successful in 3m20s
Required
Details
/ Coverage (pull_request) Successful in 4m36s
Required
Details
/ Lint (pull_request) Successful in 6m18s
Required
Details
/ Integration (pull_request) Successful in 6m8s
Required
Details
/ E2E Browser (pull_request) Successful in 6m34s
Required
Details

Pull request closed

Please reopen this pull request to perform a merge.
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
2 participants
Notifications
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!1217
No description provided.