Skip to content

✨ Workflow models: simulate an issue/PR against a model - #17688

Merged
clubanderson merged 6 commits into
v5from
workflowmodel/v5-p3c-simulate
Nov 11, 2026
Merged

clubanderson merged 6 commits into
v5from
workflowmodel/v5-p3c-simulate

Conversation

@clubanderson

Copy link
Copy Markdown
Member

RFC phase 3c — Simulate of the workflow-models RFC (#17620), on v5, follows #17681 (merged).

"Simulate: paste or pick a real issue/PR and see which state the model puts it in and why, using the same Facts the engine would use. This is the operator's confidence check before binding."

What

API — POST /api/workflows/{name}/{version}/simulate (read-only, any logged-in role)

Request:

{ "kind": "issue"|"pr", "facts": { ...SimulateFacts... }, "repo": "owner/repo", "number": 123 }
  • facts is a lowerCamel JSON mirror of stageengine.Facts (labels, open, merged, draft, origin, author{login,kind,selfAuthored,role}, park{holds,needsHuman,…}, issue{bucket,…}, pr{checks,review,dco,conflicting,changesRequested,humanApprover,lane,…}). Enumerations use the engine's name helpers (author.kind hive|clanker|human|unknown, pr.checks green|red|pending|unknown, pr.review approved|changes_requested|pending|unavailable|requires_human, pr.dco yes|no|unknown); new exported stageengine.ParseAuthorKind/ParseChecks/ParseReview/ParseDCO are their inverses, so a response's facts can be pasted back as a request.
  • repo+number without facts → best-effort fill through the hive's GitHub client (new github.Client.WorkflowItemSnapshot: issue, PR, mergeable state, head check summary). kind may be omitted (taken from the item; mismatch → 400). No client wired → 422, never invented.

Response:

{ "decision": { "state", "column", "stage", "label", "reason", "abstain", "covered", "terminal" },
  "trace": [ { "id", "state", "kind", "evaluated", "matched", "skipped", "builtin", "reason",
               "predicates": [ { "fact", "op", "value", "holds", "known", "actual" } ] } ],
  "facts": { …normalized echo… }, "filled": [...], "unknown": [...], "notes": [...] }
  • New stageengine.Explain(profile, kind, facts) (Decision, []RuleTrace) shares holds() with the rules evaluator; Derive is untouched. Rules mode: every rule in order, other-kind rules skipped, evaluated until the first match (predicate verdicts + actual value), the rest evaluated:false; uncovered → abstain:true, covered:false, reason "workflow: not covered…". Builtin mode: single {builtin:true, state, reason} entry.
  • Built-in PR derivation needs lane facts; when none are given stageengine.SimulatedLane synthesizes one from the other facts and says so in notes (pass pr.lane to override).

UI — Simulate panel on the model detail page

Collapsed Simulate <details> in Settings → Workflows → detail: owner/repo#123 picker + Fetch facts, issue/PR radio, facts form generated from the declarative WF_SIM_FIELDS table, Run. Result: decision chip (state · column · stage/NNN · reason, or Not covered), the trace as an ordered list with the matched rule highlighted and each predicate as fact op value → ✓/✗/?, fetched/unknown lists, and the resulting state's chip highlighted in the diagram (.wf-dg-state.simulated). Tokens-only CSS, no raw px/colours, no downloads, no toLocaleString.

Guards

  • dashboard/openapi.json path + WorkflowSimulateRequest/Facts/Result schemas (route-parity test).
  • Go: TestWorkflowSimulateHiveDefaultPR, TestWorkflowSimulateRulesTrace, TestWorkflowSimulateErrors, TestWorkflowSimulateFetchesItem (dashboard); TestExplainRulesTrace, TestExplainBuiltinTrace (stageengine).
  • Node: TestWorkflowsSimulateWiring, TestWorkflowsSimulateNode (form kind-awareness + round-trip, matched-rule highlight, predicate verdicts, Not covered, diagram marking).
  • pkg/dashboard, pkg/dashboard/webstatic, pkg/stageengine, pkg/workflowmodel/... green locally (apart from the known macOS-only TestConfigExportReportsPersistedPath / TestKnowledgeDrainLegacyMemory); style/download/timezone/import ratchets unchanged; webstatic/gen -check clean.

Docs

  • docs/design/workflow-models.md → new "Simulate (RFC phase 3c)" section (API shape, trace semantics, best-effort fill caveats).
  • changelog.d/added-workflowmodel-simulate.md.

Refs #17620

@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 clubanderson 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

@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

github-actions Bot commented Nov 11, 2026 •

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.

Failing checks:

  • coverage

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 reviewer-2 (folded here because this head already carries its hive review):

Read the diff against v5 and the deriver it wraps. The logic looks right; what blocks this is the coverage gate, and the red is caused by this diff rather than flake.

Medium — coverage gate fails on two packages this PR adds code to. Job 130620704436: github: 87.9% (floor 88%), stageengine: 86.9% (floor 90%). Mapping that to the diff:

  • src/pkg/stageengine/simulate.go:17-58 SimulatedLane (twelve-arm switch), :63-73 SimulateRef, and workflow.go:128-185 ParseAuthorKind/ParseChecks/ParseReview/ParseDCO have no test in the stageengine package — workflow_explain_test.go only exercises Explain on issue items in rules mode (:20, :56, :74), so the builtin-mode trace branch (workflow.go:380-386) and the Skipped arm for wrong-kind rules (workflow.go:401) are also unexercised in-package. The dashboard tests drive these through the HTTP handler, but the per-package gate does not credit that.
  • src/pkg/github/workflow_item.go:35-73 Client.WorkflowItemSnapshot has no test in pkg/github; the only reference is the mock in api_workflows_simulate_test.go:179. A httptest server returning an issue, then a PR with mergeable_state: dirty and one with unknown, would cover the ConflictKnown and check-summary branches.

Table-driven tests for SimulatedLane (one case per arm) and the four Parse* round-trips should clear stageengine on their own; WorkflowItemSnapshot is the 0.1% in github.

What I verified as correct:

  • Explain (workflow.go:372-419) takes its Decision from DeriveFromModel(p, kind).Derive(f) and only builds the trace alongside, so the "identical to the engine" claim is structural, not a reimplementation. The trace's match condition (kind filter, then every predicate Holds) is the same conjunction rules() uses via holdsAll (workflow.go:102-108).
  • handleWorkflowSimulate (api_workflows_simulate.go:166-182): without facts, missing repo/number → 400, no deps.WorkflowItemSnapshot → 422, fetch error → 502, kind mismatch → 400 — matches the body's table. The fetch runs under workflowSimulateTimeout.
  • SimulatedLane is only injected when the caller supplied no lanes (api_workflows_simulate.go:211-213), so hand-set pr.lanes are honoured.
  • WorkflowItemSnapshot reuses storePRDetail (client.go:1608), which only refreshes the shared PR cache with fresher data — a harmless side effect of a read-only route.

Not a duplicate of #17685 (phase 4 backend); the two share only route registration in api_workflows.go, openapi.json and the design doc, so expect a trivial rebase for whichever lands second. Not approving — recommendation for a maintainer only.

@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.

Decision needed: @clubanderson should decide whether the simulate route's repo+number fill (which drives the hive's App token at any caller-chosen owner/repo) should be owner-gated like ingest or restricted to the hive's configured repositories, before it ships open to every logged-in role.

Read the Go against the tree at 392d1de (api_workflows_simulate.go, github/workflow_item.go, deps.go, cmd/hive/main.go, and api_workflows_ingest.go / api_agent_control.go for the existing gating). The engine side (SimulateRef, Explain, the Facts mirror and lane simulation) looks sound; the one thing I would not settle alone is a permission boundary.

Medium — any-role route drives the hive's GitHub token at an arbitrary repository.
api_workflows_simulate.go:171-199: handleWorkflowSimulate has no requireOwnerRole, and when facts is absent it passes the caller's repo and number straight to s.deps.WorkflowItemSnapshot, which is wired in cmd/hive/main.go:1094 to b.ghClient.WorkflowItemSnapshot. workflow_item.go:36-73 then does Issues.Get, PullRequests.Get and SummarizeCommitChecks on that owner/repo with the hive's credential. The only validation is strings.Contains(req.Repo, "/") (:179); nothing checks the repo against the hive's configured set. Mechanism: a dashboard user with a non-owner role can read labels, author, state, head SHA, base ref, mergeability and check summary of any issue/PR in any repository the hive's App installation can see — including private repos the hive is not configured to work — and the 502 body at :192 echoes the forge error verbatim. The existing precedent for "caller-chosen repo via the hive's forge client" is POST /api/workflows/ingest, which is requireOwnerRole-gated (api_workflows_ingest.go:37) for the same read. The body states "read-only, any logged-in role" as intended, so this is a deliberate choice — but it widens a trust boundary and should be an explicit owner decision, not an incidental one. Two cheap options: gate only the fetch path (hand-set facts can stay any-role), or refuse repos outside the configured list.

Also verified, no action needed:

  • Facts-present path never touches the forge (:172), and kind is validated by SimulateRef (stageengine/simulate.go).
  • storePRDetail (workflow_item.go:58) populates the shared PR-detail cache keyed by repo/number, so a simulated foreign repo cannot shadow a configured one.

Observed: the coverage check is red on this head; I did not dig into the log.

Sequencing: overlaps api_workflows.go, dashboard/openapi.json and docs/design/workflow-models.md with #17685 (phase 4). Different phases, not duplicates; whichever lands second needs a rebase.

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

@github-actions

Copy link
Copy Markdown
Contributor

Mutation testing

⏱️ The 900s mutation budget ran out, so 2 package(s) were not fully mutated and the results below do not cover them:

  • pkg/github: skipped (not started)
  • pkg/stageengine: skipped (not started)

❌ gremlins did not produce a report for 1 package(s), so no verdict can be derived from this run (see the job log):

  • pkg/dashboard: gremlins exited 137

Verdict: error (#17349).

@kubestellar-prow kubestellar-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 11, 2026
Add POST /api/workflows/{name}/{version}/simulate. It loads a stored
model, compiles it, converts a JSON Facts mirror into stageengine.Facts
and runs the same deriver the engine uses, returning the decision
(state, column, stage, reason) and a per-rule trace from the new
stageengine.Explain. repo+number without facts best-effort fills from
the forge and reports which facts were filled vs unknown.

Refs #17620

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Settings → Workflows → model detail grows a Simulate section: pick a
real owner/repo#N to fetch its facts, or describe one with a form
generated from WF_SIM_FIELDS, then Run to see the decision chip
(state · column · stage · reason), the ordered rule trace with the
matched rule highlighted and each predicate's verdict, and the
resulting state marked in the diagram.

Refs #17620

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

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

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Cover Explain (builtin issue/PR, every rules predicate op, unknown facts,
skipped rules), the Parse*/name helpers, SimulatedLane/SimulateRef, and
github WorkflowItemSnapshot plus small untested helpers (ChecksForRequired,
required-check resolver, ReporterTrust hold summary/provenance,
IssueTitleState, PRConflictCandidates, ReviewDeltaDiff, CI activity and
retry helpers) so the coverage gate clears its floors.

Refs #17620

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@clubanderson
clubanderson force-pushed the workflowmodel/v5-p3c-simulate branch from 3b9f898 to 90e077e Compare November 11, 2026 16:24
@kubestellar-prow kubestellar-prow Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 11, 2026
@clubanderson
clubanderson merged commit b7bc5d0 into v5 Nov 11, 2026
83 of 84 checks passed
@kubestellar-prow
kubestellar-prow Bot deleted the workflowmodel/v5-p3c-simulate branch November 11, 2026 16:39
@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback

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