Skip to content

✨ Workflow models: Settings → Workflows tab (library, diagram, ingest) - #17681

Merged
hivecommons-hive[bot] merged 3 commits into
v5from
workflowmodel/v5-p3b-workflows-tab
Nov 11, 2026
Merged

hivecommons-hive[bot] merged 3 commits into
v5from
workflowmodel/v5-p3b-workflows-tab

Conversation

@clubanderson

Copy link
Copy Markdown
Member

RFC phase 3b of the workflow-models RFC (#17620), on v5 (follows #17679).

What

  • Settings → Workflows tab (GOVERNOR_CONFIG_TABS, between Repos and Budget) with:
    • Library — every model this hive knows (built-in hive-default, local, ingested): provenance badge, compile badge (compiles / compiles · N warnings / does not compile), source, states/columns, bound repositories; View · Duplicate · Export (in-app JSON viewer) · Delete (withheld while bound).
    • Detail — facts, SVG diagram of the compiled columns/states/transitions (inferred dashed, initial/terminal marked), diagnostics, citations, raw JSON.
    • Ingest — pick a configured repo or type owner/repo, Preview / Ingest-and-save via POST /api/workflows/ingest.
  • Activity Monitor model badge deep-links to the library entry (openWorkflowLibraryEntry).
  • Binding stays on Settings → Repos; this tab never binds. Editing is phase 4.

Guards

  • workflows_library_ui_test.go: wiring asserts + node tests of the pure renderers (renderWorkflowLibraryRows, workflowDiagramSvg, wfBoundRepos, wfIngestDefaultName, wfProvenanceOf).
  • CSS tokens only (style ratchet green); in-app export viewer (no forced downloads); shared timezone date helper.
  • index.html regenerated (gen -check clean); api-reference citations clean.

Docs

  • docs/design/workflow-models.md → "Workflows tab (RFC phase 3b)"; changelog fragment.

Settings gains a Workflows tab listing every stored workflow model with
provenance/compile badges and bound-repository counts, a read-only detail
view that draws the compiled columns and states as an SVG diagram, and an
Ingest form over POST /api/workflows/ingest. The Activity Monitor model
badge deep-links into the library entry. Binding stays on Settings → Repos.

Refs #17620

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Nov 11, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kellyaa for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions

Copy link
Copy Markdown
Contributor

Changelog: this PR changes code but carries no changelog entry
(no changelog.d/ fragment and no CHANGELOG.md edit).

If it is user-visible — a feature, a fix an operator would notice, a
security change, a migration, a deprecation, or anything breaking —
please add a fragment file changelog.d/<category>-<pr-or-slug>.md
containing exactly your entry (see changelog.d/README.md; the
category is one of added/changed/deprecated/fixed/security). Please
do not append to CHANGELOG.md's ## Unreleased directly — that
shared section is the merge-conflict generator fragments replaced
(#5675). If it is a refactor, a test-only change, or dependency
churn, no entry is needed and you can ignore this.

This is a reminder, not a gate; it never blocks a merge.

@kubestellar-prow kubestellar-prow Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

@hivecommons-hive

hivecommons-hive Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor.

@clubanderson this PR is held at stage/071-needs-author and the hive will not merge it yet.

Why:

  • the review swarm requested changes

Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path.

Review findings from ci-maintainer (folded here because this head already carries its hive review):

Intent alignment (read at head b1a70829): the diff delivers the Library / Detail / Ingest tab the body describes, stays read-only (never binds), and adds no unrelated scope. Three places where the UI reads fields the backend does not emit mean two of the promised displays render empty, though — the node tests pass because they feed hand-built objects with the wrong shape.

1. Diagnostics text is never shown (badge tooltips read "W1: undefined") — medium

wfCompiledBadge and renderWorkflowDiagnostics read d.message / d.path (config-workflows-library.js:46,48,179), but the server's Diagnostic only has code, severity, subject, detail (src/pkg/workflowmodel/compile.go:12-17). So the Library compile-badge tooltip is code: undefined per line and the Detail/Ingest "Diagnostics" list is codes only. The test fixtures in workflows_library_ui_test.go use { code, message }, so they do not catch it.

        `<li class="wf-diag wf-diag-${esc(d.severity || 'warning')}"><span class="wf-diag-code mono">${esc(d.code || '')}</span> ${esc(d.detail || d.message || '')}${d.subject ? ` <span class="text-muted mono">${esc(d.subject)}</span>` : ''}</li>`

and at lines 46/48 use d.code + ': ' + (d.detail || d.message || '').

2. Ingest result always says "0 pipelines" and never lists them — medium

The PR body says the result "shows … pipelines found". renderWorkflowIngest does Array.isArray(r.pipelines) ? r.pipelines : [] (config-workflows-library.js:243), but the handler returns "pipelines": len(m.Spec.Pipelines) — an integer (src/pkg/dashboard/api_workflows_ingest.go:89-92). The count is therefore always 0 and the <ul class="wf-pipelines"> never renders. The full list is already in the response under model.spec.pipelines (model.go:113, items have id, name, system, path):

        const pipelines = Array.isArray(r.pipelines) ? r.pipelines : ((r.model && r.model.spec && Array.isArray(r.model.spec.pipelines)) ? r.model.spec.pipelines : []);

3. Citation text dropped — low

c.note at config-workflows-library.js:217; Citation is { path, line, text } (lifecycle.go:60-62). Use c.text.

4. Tab can stick on "Loading the workflow library…" — low

renderGovWorkflows calls loadWorkflowLibrary(cb) (:278), but that helper returns without ever invoking cb when a load is already in flight (config-workflows.js:16). If the user opens Repos (which starts the load) and switches to Workflows before it finishes, the Repos callback only rerenders when tab === 'Repos', so Workflows never rerenders until the next click. Smallest fix: in loadWorkflowLibrary, when loading is true, queue onDone (e.g. push to a _workflowState.waiters array drained in finally) rather than dropping it.

Review findings from adjudicator (folded here because this head already carries its hive review):

Correctness review (head b1a7082). The UI reads several fields under names the Go API does not emit, so the diagnostics and ingest surfaces described in the PR body render empty/zero against the real server. The node tests pass because their fixtures use the UI's names, not the API's.

1. Diagnostics text never shows — d.message / d.path vs API detail / subject (high)

workflowmodel.Diagnostic is {code, severity, subject, detail} (src/pkg/workflowmodel/compile.go:12-17). The UI reads d.message and d.path:

  • config-workflows-library.js:46,48 — compile-badge tooltip renders "state-no-exit: undefined".
  • config-workflows-library.js:179 — the Diagnostics list shows the code with an empty message and never the subject.
        `<li class="wf-diag wf-diag-${esc(d.severity || 'warning')}"><span class="wf-diag-code mono">${esc(d.code || '')}</span> ${esc(d.detail || d.message || '')}${d.subject ? ` <span class="text-muted mono">${esc(d.subject)}</span>` : ''}</li>`

and on lines 46/48 use d.code + ': ' + (d.detail || d.message || ''). Please also switch the fixtures in workflows_library_ui_test.go ({ severity: 'warning', code: 'W1', message: 'inferred' }) to detail so the test guards the real contract.

2. Ingest result always says "0 pipelines" and never lists them (medium)

handleWorkflowIngest returns "pipelines": len(m.Spec.Pipelines) — an integer (src/pkg/dashboard/api_workflows_ingest.go:87). config-workflows-library.js:243 does Array.isArray(r.pipelines) ? r.pipelines : [], so the summary on :246 is always 0 pipelines and the <ul class="wf-pipelines"> on :250 never renders. The PR body promises "pipelines found".

        const pipelines = Array.isArray(r.pipelines) ? r.pipelines : ((r.model && r.model.spec && Array.isArray(r.model.spec.pipelines)) ? r.model.spec.pipelines : []);

(Spec.Pipelines is in the returned model, src/pkg/workflowmodel/model.go:113, with id/name/system/path — matching what :250 already reads.)

3. Duplicate silently overwrites an existing <name>1 (medium)

wfLibDuplicate posts the copy as version: '1' (config-workflows-library.js:353-354) with no existence check. Store.Save is an upsert (store/store.go:264-272) and handleWorkflowSave does not check bindings (api_workflows.go:115-139), so duplicating the same model twice with the default name-copy — or typing the name of a model that repos are already bound to — replaces that model's file and index entry without a prompt, bypassing the "Delete is withheld while bound" guard this tab advertises. Suggested guard before the fetch on :345:

       if (!/^[a-z0-9][a-z0-9._-]*$/.test(target)) { showToast('Name must be lower-case letters, digits, dots, dashes or underscores.', 'error'); return; }
+      if (_workflowState.workflows.some(w => w.name === target && String(w.version) === '1')) { showToast(target + '1 already exists; choose another name.', 'error'); return; }

4. Citation notes never shown (low)

Citation is {path, line, text} (src/pkg/workflowmodel/lifecycle.go:59-63); config-workflows-library.js:217 reads c.note. Use c.text.

Not verified: the ingest placeholder wfIngestDefaultName(repo) (cncf/techdocs → cncf-techdocs) vs. the server's default base.Metadata.Name (ingest/lifecycle.go:32) — I did not trace the slug; if they differ the "Save as" placeholder misleads.

Review findings from reviewer-2 (folded here because this head already carries its hive review):

Read this against the v5 tree (dispatcher, config-workflows.js, api_workflows.go, api_workflows_ingest.go), not just the diff. Looks correct to me; one low-severity race worth a follow-up.

What I verified

  • Every API the tab calls exists with the method the JS uses: api_workflows.go:20-28 registers GET/POST /api/workflows, POST /api/workflows/ingest, GET/PUT/DELETE /api/workflows/{name}/{version}.
  • Response shapes match what the renderers read: handleWorkflowGet returns {entry, model, profile} (api_workflows.go:87-91); handleWorkflowIngest returns model/compiled/diagnostics/ref/files/pipelines and entry on save (api_workflows_ingest.go:88-94, :122) — exactly the keys renderWorkflowIngest consumes.
  • Ingest is owner-gated server-side (api_workflows_ingest.go:37 requireOwnerRole), so the client-side dashboardRoleAtLeast checks are UX, not the trust boundary.
  • All dispatcher attributes used (data-arg-types="j", data-change-action, data-input-action, data-keydown-action, data-prevent, data-stop) have existing precedents in acmm.js, acmm-lifecycle.js, config-agent.js.
  • Every interpolation of server data in the new HTML/SVG builders goes through esc(...); the unescaped ${id}/${k} in wfLibNav and the facts <dt> are constants. Path d attributes are numeric only.
  • renderGovWorkflows is wired in config-tabs.js:255 following the sibling renderGov* cases; manifest ordering places the new file after config-workflows.js, which defines _workflowState/loadWorkflowLibrary it depends on.

Low: tab can stick on "Loading…" if a library fetch is already in flight

config-workflows.js:16 — loadWorkflowLibrary(onDone) returns immediately when _workflowState.loading is true, and in that branch onDone is never invoked. renderGovWorkflows (new file, the !_workflowState.loaded branch) relies on its callback to re-render once the load completes. Sequence: operator opens Settings → Repos (config-workflows.js:50 starts the fetch with a Repos-only callback), switches to Workflows before it returns → the Workflows callback is dropped, the Repos callback sees tab !== 'Repos' and does nothing, and the Workflows tab shows "Loading the workflow library…" until the user clicks something. Window is one fetch round-trip, so impact is a stale panel, not data loss. Cheapest fix is in the existing helper: queue onDone callbacks while loading (or have the Workflows render re-check loaded on a short timer). Not a blocker for this PR.

Refs #17620

Signed-off-by: Andrew Anderson <andy@clubanderson.com>

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor but real issue; otherwise this reads correct to me. Checked every helper the new file calls against the v5 tree (renderConfigTab, switchConfigTab, openConfigDialog(type, agent, initialTabOverride), hivePrompt, saveErrorMessage, dashboardRoleAtLeast, _inceptionAuthHeaders, formatDashboardDateTime) — all exist with the signatures used. Every interpolation of server-supplied text in the library table, detail view, SVG diagram and ingest result goes through esc(); the only bare ${…} are numeric SVG coordinates and the static nav tab ids. Model name/version are encodeURIComponent-ed in every URL.

Workflows tab can stick on "Loading the workflow library…" — config-workflows-library.js, renderGovWorkflows():

if (!_workflowState.loaded) {
  loadWorkflowLibrary(() => { if (_configState.tab === 'Workflows') renderConfigTab('Workflows'); });
  return wfLibNav(...) + '…Loading the workflow library…';
}

loadWorkflowLibrary (base, src/pkg/dashboard/static-src/js/config-workflows.js:15-17) begins if (_workflowState.loading) return; — it returns without calling onDone when a load is already in flight. The Repos tab kicks off the same load (config-workflows.js:50) with a callback that only re-renders if _configState.tab === 'Repos'. So: open Settings → Repos, click the adjacent Workflows tab before GET /api/workflows returns → the Workflows callback is dropped, the Repos callback sees the wrong tab and does nothing, and the placeholder never refreshes until the user clicks another nav. wfLibReload() has the same shape (loaded = false; await loadWorkflowLibrary() returns immediately if a load is pending, then re-renders a tab that re-enters the same early return). Severity low — the window is one fetch — but it is the first place in the UI where two tabs race on this loader. Simplest fix is in the loader: queue onDone callbacks while loading is true and drain them in finally, which also fixes the Repos path for free.

Not a defect, just noting: renderGovWorkflows() consuming _wfLib.pending calls wfLibOpen(), which synchronously calls wfLibRerender() → renderConfigTab('Workflows') → renderGovWorkflows() again while the outer call is still building its string. Both renders end up producing the same "Loading key…" body so it is harmless today, but it is a re-entrant render.

Body vs. diff: the body says index.html was regenerated and the citation guard is clean — both are in the diff and the corresponding checks are green on this head. No gap found.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive
hivecommons-hive Bot merged commit 69d507f into v5 Nov 11, 2026
81 of 83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant