Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/added-workflowmodel-label-gc-commands.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **workflow models:** the stage engine now remembers which model labels it provisioned per repository and, on unbind or rebind to a model with a different label set, deletes the stale ones — only labels it created, outside the built-in stage family and worn by no open issue or PR; each deletion is audited `workflow_label_removed` and `workflow.label_gc: false` keeps everything. A bound model's `replies` are now offered by the Activity Monitor's human-gate pill and accepted as `/hive <verb>` on parked issues, applying the label the model maps to the reply's target state ([#17620](https://github.com/hivecommons/hive/issues/17620))
8 changes: 8 additions & 0 deletions src/cmd/hive/main_sweeps.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/hivecommons/hive/pkg/stageengine/prstage"
"github.com/hivecommons/hive/pkg/stagelabels"
"github.com/hivecommons/hive/pkg/tracing"
"github.com/hivecommons/hive/pkg/workflowmodel"
"github.com/hivecommons/hive/pkg/worksource"
"go.opentelemetry.io/otel/attribute"
)
Expand Down Expand Up @@ -1491,9 +1492,16 @@ func runIssueUnparkSweepIfDue(ctx context.Context, ghClient *github.Client, cfg
approveOpts.TrustedBotSkipReview = func() bool { return cfg.AutoMerge.TrustedBotSkipsReview() }
}
approveEngine := automerge.New(ghClient, approveOpts)
// A bound workflow model's replies are accepted as `/hive <verb>`
// (hivecommons/hive#17620); unbound repositories see no change.
var workflowFor func(owner, repo string) *workflowmodel.Profile
if cfg != nil {
workflowFor = stageEngineOptions(cfg, logger).WorkflowFor
}
result, err := ghClient.SweepIssueUnparkCommands(ctx, github.IssueUnparkSweepOptions{
PRApproveReply: approveEngine.PRApproveReply,
MaxActions: github.DefaultIssueUnparkSweepMaxActions,
WorkflowFor: workflowFor,
// cfg is nil only in tests; nil leaves every repo on the default path.
LabelDriven: func(repo string) bool { return cfg.RepoLabelDriven(repo) },
LabelApproval: labelApprovalPolicyResolver(cfg),
Expand Down
4 changes: 4 additions & 0 deletions src/cmd/hive/stage_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func stageEngineOptions(cfg *config.Config, logger *slog.Logger) *stageengine.Op
Metrics: stageengine.DefaultCounters(),
WorkflowFor: stageEngineWorkflowFor(cfg, wfstore.Open(wfstore.Root(cfg.Data.WorkflowsDir)), logger),
}
if cfg != nil {
stageEngine.opts.WorkflowProvisioned = wfstore.Open(wfstore.Root(cfg.Data.WorkflowsDir))
stageEngine.opts.WorkflowLabelGC = func() bool { return cfg.Workflow.LabelGCEnabled() }
}
})
return stageEngine.opts
}
Expand Down
79 changes: 79 additions & 0 deletions src/docs/design/workflow-models.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,3 +394,82 @@ fixture under `pkg/workflowmodel/ingest/testdata/techdocs` carries both.
until the label-driven engine half of phase 4 writes model labels onto items.
- Branch protection needs `administration:read`; without it the rulesets endpoint is used, which
omits legacy protection settings such as `enforce_admins`.

## Label GC and model commands (RFC phase 4, follow-up)

Two of the phase-4 backend gaps close here: stale model labels are now reclaimed, and a bound
model's `replies` reach the human gate as commands.

### Label garbage collection on unbind / rebind

`ensureStageLabels` already provisions a bound model's `LabelDefs` after the built-in family. It
now also remembers what it provisioned and reconciles on change
(`automerge_stage_labels_gc.go`):

- **Record.** After each provisioning pass the engine writes one JSON record per repository,
`workflows/provisioned/<owner>__<repo>.json` under `data.workflows_dir`
(`workflowmodel.ProvisionedLabels{repo, model name@version, hash, labels[]}`, through the
`workflowmodel.ProvisionedStore` interface the workflow store implements). The record is the
hive's memory of which labels *it* created or adopted by recolouring for a model; a label the
repository already had for its own reasons is never in it.
- **Diff.** When the profile hash differs from the record's — a rebind, a new version of the bound
model, or an unbind (lookup returns nil or hive-default) — the engine computes
`previous − current` model labels.
- **Delete, only when safe.** A stale label is deleted only if it is in the record, is not in the
built-in family (`stagelabels.LabelDefsFor` plus the retired spellings in `stagelabels.Renames`),
and no open issue or PR wears it (one `label:"x" is:open` search per label; a failed search keeps
the label and logs it). Anything kept stays in the record so the next pass retries it. Every
deletion is audited as `workflow_label_removed` with the repository, label and the model key it
belonged to.
- **Knob.** `workflow.label_gc` (`WorkflowConfig.LabelGC`, default `true`). `false` keeps every
label and logs what would have gone; the record is still maintained, so turning it back on
reclaims what accumulated. The knob is yaml-only (no Settings control; allowlisted in
`settings_parity_allowlist.txt`).
- **Zero change for unbound repositories.** With no `ProvisionedStore` wired, or for a repository
that was never bound, nothing is remembered and nothing is deleted; the pre-P4 path is unchanged
and its tests are byte-identical.

### Model replies as human-gate commands

A model's `replies[state]` lists the verbs a human may answer the gate with. They now flow through
three places, all for a repository bound to a non-default model only:

- **Lookup.** `Profile.RepliesForNumber(stage)` returns the verbs for the state at a stage number;
`Profile.ReplyForNumber(stage, verb)` resolves one to a `ReplyEffect{Verb, From, To, Label}` —
the transition the model declares with that verb as its `on` event (if any), the target state,
and the label `labels.entries` maps to that state (if any). `NormalizeReply` folds case, inner
whitespace and a leading slash so "Approve Analysis", "approve analysis" and "/approve analysis"
are one verb.
- **Comments.** The `/hive` parser (`parseUnparkCommandFor`) tries the model's verbs first —
multi-word, longest match wins, trailing punctuation trimmed, the rest of the line is the
decision text — then the built-ins. With no verbs it *is* the old parser, so `/hive approve` on
an unbound repository is exactly what it was. On a parked issue the sweep answers a model verb
with `applyIssueModelReply`: the parking labels are cleared as for `/hive approve`
(`approved-direction` added, `needs-human` lifted), the mapped label is applied when there is one
(audited `hive_label_applied` with `reply=<verb>`), and one reply comment is posted that begins
with the same `Decision:` sentence the decision history reads, then says what the model did
("moves the item from `a` to `b`; `label` applied" / "…the model maps no label to that state" /
"is recorded; the model declares no transition for it"). Answered replies are idempotency
watermarks like the built-ins. cmd/hive passes the stage engine's `WorkflowFor` to the un-park
sweep so comments see the same binding the labeller does.
- **Dashboard.** The tracker payload's `workflows.repos[<repo>].replies` maps stage number →
verbs (hive-default's block carries none). `stageTrackerHumanGateVerbs` appends them after the
built-in verbs, deduped by label, as `wf:<verb>` actions; choosing one prompts for an optional
note and POSTs `{"decision":"model-reply","reply":"<verb>","comment":…}` to
`/api/repos/{repo}/items/{n}/decision`. The handler resolves the bound profile
(`workflowProfileForRepo`), refuses unbound repositories and verbs the model does not offer at
the item's state with 409, PRs with 400, and otherwise runs the same `applyIssueModelReply`
path, audited `repo_item_decision_model_reply`.

### Known gaps (phase 4 follow-up)

- Model replies act on **parked issues** only. A verb declared for a PR state (TechDocs'
`draft-review: approve / request changes`) is not offered by the pill or honoured in comments; PRs
keep GitHub's native review signal. Un-parked issue states with replies are likewise not
answered — the reply needs a gate to answer.
- The pill offers model verbs only where it already shows a human gate; a model state with replies
but no `needs-human` / needs-decision label on the item shows nothing extra.
- Label GC trusts the provisioned record: labels created by an earlier hive build (before the
record existed) are adopted into the record on the next provisioning pass only while the model
still declares them; labels a model dropped *before* this change shipped are never reclaimed
automatically.
13 changes: 13 additions & 0 deletions src/docs/maintainer-commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,19 @@ assent, Hive posts a hint asking for `/hive approve` or `/hive decision <text>`
and leaves all labels unchanged. This avoids turning casual agreement into a
state-changing instruction.

## Model-defined replies

A repository bound to a [workflow model](design/workflow-models.md) may
declare its own human-gate verbs per state (`replies`). On a parked issue in
such a repository, `/hive <verb>` with one of those verbs — multi-word verbs
are fine, case does not matter — is accepted like `/hive decision`: the
parking labels are cleared, `approved-direction` is added, the label the model
maps to the verb's target state is applied when it declares one, and the reply
comment says what the model did with the verb. Any text after the verb is kept
as the decision. The Activity Monitor's human-gate pill offers the same verbs
after the built-in ones. Repositories without a binding, and pull requests,
behave exactly as described above; the model's verbs are not recognised there.

## Idempotency markers

Hive marks its standing parked-issue notice with
Expand Down
2 changes: 1 addition & 1 deletion src/docs/scale-envelope.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ ones to:
2. **Truncation must preserve the prioritization signal, not cut an arbitrary slice.**
This holds today and is easy to assume backwards:
- Issues are sorted by actionable rank tier, then **oldest first** within each tier before any cap is applied
(`RankActionableIssues`, `src/pkg/github/client.go:5247-5257`, descending `AgeMinutes` within each tier).
(`RankActionableIssues`, `src/pkg/github/client.go:5263-5273`, descending `AgeMinutes` within each tier).
- PRs are sorted by **review class then oldest-first within class**
(`SortPullRequestsForReview`, `src/pkg/github/review_priority.go:334`;
fixes → refactors/docs → tests).
Expand Down
13 changes: 13 additions & 0 deletions src/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,19 @@ type Config struct {
// WorkflowConfig is the `workflow:` block (hivecommons/hive#17620).
type WorkflowConfig struct {
Enabled bool `yaml:"enabled" json:"enabled"`
// LabelGC lets the stage engine delete a bound model's labels from a
// repository once the repository is unbound or rebound to a model that
// no longer declares them (RFC phase 4 follow-up). Only labels the hive
// itself provisioned, outside the built-in stage family and worn by no
// open issue or PR, are ever deleted; each deletion is audited as
// workflow_label_removed. nil means true; false leaves every label in
// place and logs what would have gone.
LabelGC *bool `yaml:"label_gc,omitempty" json:"label_gc,omitempty"`
}

// LabelGCEnabled is the effective workflow.label_gc (default true).
func (c WorkflowConfig) LabelGCEnabled() bool {
return c.LabelGC == nil || *c.LabelGC
}

// UnmarshalYAML accepts the operator-facing agents.github_api_hourly_cap
Expand Down
60 changes: 53 additions & 7 deletions src/pkg/dashboard/api_repo_item_decision.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,23 @@ const (
// superseding PR (hivecommons/hive#15136). It queues the same adoption
// the Adopt decision does; it is never implied by an Approve.
repoItemDecisionReauthor = "re-author"
// model-reply records one of the human-gate replies the repository's
// bound workflow model declares for the issue's current state
// (hivecommons/hive#17620, RFC phase 4): the verb rides in Reply and is
// recorded as `/hive <verb>`; the model's mapped label, if any, is
// applied. Refused on unbound repositories.
repoItemDecisionModelReply = "model-reply"

repoItemDecisionMaxBody = 16 << 10
repoItemDecisionMaxComment = 4000

auditRepoItemDecisionApprove = "repo_item_decision_approve"
auditRepoItemDecisionDirection = "repo_item_decision_direction"
auditRepoItemDecisionClose = "repo_item_decision_close"
auditRepoItemDecisionAdopt = "repo_item_decision_adopt"
auditRepoItemDecisionMerge = "repo_item_decision_merge"
auditRepoItemDecisionQueue = "repo_item_decision_queue_merge"
auditRepoItemDecisionApprove = "repo_item_decision_approve"
auditRepoItemDecisionDirection = "repo_item_decision_direction"
auditRepoItemDecisionModelReply = "repo_item_decision_model_reply"
auditRepoItemDecisionClose = "repo_item_decision_close"
auditRepoItemDecisionAdopt = "repo_item_decision_adopt"
auditRepoItemDecisionMerge = "repo_item_decision_merge"
auditRepoItemDecisionQueue = "repo_item_decision_queue_merge"

// repoItemDecisionAdoptMarker opens the attributed note an Adopt
// decision leaves on the PR.
Expand Down Expand Up @@ -125,6 +132,8 @@ type repoItemDecisionRequest struct {
Force bool `json:"force,omitempty"`
// To with decision "reroute" is the target clanker's login, or "any".
To string `json:"to,omitempty"`
// Reply with decision "model-reply" is the workflow model's verb.
Reply string `json:"reply,omitempty"`
}

// parseRepoItemSupersededBy resolves a "superseded by" reference the way an
Expand Down Expand Up @@ -266,6 +275,10 @@ func repoItemDecisionResultWith(repoFull string, number int, itemType, decision,
out.Outcome = "`/hive approve` recorded; parking labels lifted, `" + label + "` applied."
out.Next = "The hive un-parks the issue and carries on; nothing is merged."
}
case repoItemDecisionModelReply:
out.Action = rowActionDirected
out.Outcome = "Workflow reply recorded as `/hive " + strings.TrimSuffix(strings.TrimSpace(comment), ".") + "`; parking labels lifted, `" + label + "` applied."
out.Next = "The hive applies the label the model maps to this reply, if any, and carries on; nothing is merged."
case repoItemDecisionDirection:
out.Action = rowActionDirected
out.Outcome = "Direction recorded as `/hive decision`: " + strings.TrimSuffix(strings.TrimSpace(comment), ".") + "."
Expand Down Expand Up @@ -518,6 +531,39 @@ func (s *Server) handleRepoItemDecision(w http.ResponseWriter, r *http.Request)
s.audit.LogRecord(user, auditRepoItemDecisionClose, auditDetail("repo", repoFull, "number", numStr, "type", itemType, "reason", verb, "superseded_by", supersededBy), "", repoFull, number)
jsonResponse(w, repoItemDecisionResponse{OK: true, Repo: repoFull, Number: number, Decision: decision, Status: "closed", Label: label,
Result: repoItemDecisionResult(repoFull, number, itemType, decision, comment, supersededBy, label, nil)})
case repoItemDecisionModelReply:
verb := strings.TrimSpace(body.Reply)
if verb == "" {
jsonError(w, "a model reply needs the verb", http.StatusBadRequest)
return
}
if itemType == "pr" {
jsonError(w, "model replies are recorded on issues only", http.StatusBadRequest)
return
}
profile := s.workflowProfileForRepo(repoFull)
if profile == nil {
jsonError(w, "repository is not bound to a workflow model", http.StatusConflict)
return
}
event, err := gh.RecordOperatorIssueModelReply(ctx, repoFull, number, user, verb, comment, profile)
switch {
case errors.Is(err, ghpkg.ErrUnknownModelReply):
jsonError(w, err.Error(), http.StatusConflict)
return
case errors.Is(err, ghpkg.ErrIssueNotParked):
jsonError(w, "issue is not waiting on a decision", http.StatusConflict)
return
case errors.Is(err, ghpkg.ErrDecisionTargetIsPR):
jsonError(w, "this is a pull request; model replies are recorded on issues only", http.StatusBadRequest)
return
case err != nil:
jsonError(w, err.Error(), http.StatusBadGateway)
return
}
s.audit.LogRecord(user, auditRepoItemDecisionModelReply, auditDetail("repo", repoFull, "number", numStr, "workflow", profile.Name+"@"+profile.Version, "reply", verb, "decision", event.Decision), "", repoFull, number)
jsonResponse(w, repoItemDecisionResponse{OK: true, Repo: repoFull, Number: number, Decision: decision, Status: "unparked", Label: ghpkg.HumanAckLabel,
Result: repoItemDecisionResult(repoFull, number, itemType, decision, event.Decision, "", ghpkg.HumanAckLabel, nil)})
case repoItemDecisionReroute:
if itemType != "pr" {
jsonError(w, "reroute applies to pull requests", http.StatusBadRequest)
Expand Down Expand Up @@ -567,7 +613,7 @@ func (s *Server) handleRepoItemDecision(w http.ResponseWriter, r *http.Request)
}
s.handlePullRequestMergeDecision(ctx, w, repoFull, number, user, decision, comment)
default:
jsonError(w, "unknown decision; want approve, direction, close-superseded, close-wontfix, close-read, close-issue, adopt, reroute, re-author, merge-now or queue-merge", http.StatusBadRequest)
jsonError(w, "unknown decision; want approve, direction, model-reply, close-superseded, close-wontfix, close-read, close-issue, adopt, reroute, re-author, merge-now or queue-merge", http.StatusBadRequest)
}
}

Expand Down
1 change: 1 addition & 0 deletions src/pkg/dashboard/settings_parity_allowlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -617,3 +617,4 @@ data.workflows_dir # workflow-model store root; deployment knob, no Settings con
project.workflow_bindings[].name # set through the per-repo workflow select on Settings → Repos via PUT /api/workflows/bindings, not a leaf-path control (RFC #17620)
project.workflow_bindings[].repo # see project.workflow_bindings[].name
project.workflow_bindings[].version # see project.workflow_bindings[].name
workflow.label_gc # operator safety knob for model-label garbage collection on rebind; yaml-only, no Settings control planned (RFC #17620)
45 changes: 44 additions & 1 deletion src/pkg/dashboard/stage_tracker_workflows.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dashboard

import (
"sort"
"strconv"

"github.com/hivecommons/hive/pkg/config"
"github.com/hivecommons/hive/pkg/workflowmodel"
Expand All @@ -28,7 +29,13 @@ type StageTrackerWorkflow struct {
Version string `json:"version"`
Hash string `json:"hash,omitempty"`
Columns []StageTrackerWorkflowColumn `json:"columns,omitempty"`
Error string `json:"error,omitempty"`
// Replies maps a stage number (as a string key, the JSON object key
// the SPA indexes by `item.stage`) to the human-gate reply verbs the
// model declares for the state at that number (hivecommons/hive#17620,
// RFC phase 4). The gate pill offers them after the built-in verbs;
// each posts `/hive <verb>`. Absent when the model declares none.
Replies map[string][]string `json:"replies,omitempty"`
Error string `json:"error,omitempty"`
}

// StageTrackerWorkflowColumn is a column with the stage numbers it anchors,
Expand Down Expand Up @@ -59,9 +66,45 @@ func stageTrackerWorkflowBlock(p *workflowmodel.Profile) StageTrackerWorkflow {
sort.Ints(col.Numbers)
out.Columns = append(out.Columns, col)
}
// hive-default's replies are dashboard action ids, not comment verbs;
// its block stays exactly as before.
for n, id := range p.StateOfNumber {
if p.Name == hivedefault.Name {
break
}
if verbs := p.RepliesForNumber(n); len(verbs) > 0 && id != "" {
if out.Replies == nil {
out.Replies = map[string][]string{}
}
out.Replies[strconv.Itoa(n)] = append([]string{}, verbs...)
}
}
return out
}

// workflowProfileForRepo is the compiled model the repository is bound to
// — nil when unbound, bound to hive-default, or when the binding does not
// resolve — so the dashboard applies exactly the vocabulary the stage
// engine does (hivecommons/hive#17620).
func (s *Server) workflowProfileForRepo(repoFull string) *workflowmodel.Profile {
if s == nil || s.deps == nil || s.deps.Config == nil || !s.deps.Config.Workflow.Enabled {
return nil
}
lookup := s.workflowProfileLookup()
if lookup == nil {
return nil
}
b, ok := s.deps.Config.WorkflowBindingFor(repoFull)
if !ok {
return nil
}
p, err := lookup(b.Name, b.Version)
if err != nil || p == nil || p.Name == hivedefault.Name {
return nil
}
return p
}

// stageTrackerWorkflows builds the block from the config's bindings; nil when
// there are none.
func stageTrackerWorkflows(cfg *config.Config, lookup workflowProfileLookup, qualify func(string) string) *StageTrackerWorkflows {
Expand Down
Loading
Loading