Repository navigation
π fix(convergence): scope the mutation writer slot per target instead of per repo - #14671
Conversation
β¦f per repo Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
|
hive/awaiting-ci: CI pending β sweep will merge when green. π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Looks correct to me. Read against the v5 tree, not just the diff:
TargetWriterScopeparses the subject the boundary actually mints:claimSubjectatsrc/pkg/convergence/mutation/boundary.go:300buildsrepo#kind/target, and every numbered caller I surveyed insrc/pkg/github/**passesTarget: strconv.Itoa(number); branch/push/create callers pass a ref, which is non-decimal and falls back to the repo pool as intended. Noeffects.Kind*constant contains/, so stripping to the first/cannot mis-split.- Repo-wide safety is unchanged:
Overlaps(claim.go:128-130) still makes aClaimTypeRepoclaim conflict with every task claim in the repo viaErrClaimHeld, so narrowing only the capacity bucket does not let a repo claim run alongside an in-flight per-target mutation. main.go:2027is the only productionOpenLedgercaller, so the scope switch is complete;OpenLedgerkeeps its per-repo behaviour for existing tests/callers.settings_parity_allowlist.txtentry included, which is whytest (dashboard β¦)is green here (contrast #14677, same day, which missed it).
One suggestion, not a defect: convergence.mutation.max_writers_per_target is named in the changelog but documented nowhere an operator reads β src/hive.yaml.example has no convergence: block and src/docs/env-vars.md is the only place convergence.mode appears. A two-line example under a convergence: stanza would make the knob discoverable. Fine to do in a follow-up.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
hive/check-report (head π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
CI triage for
This PR touches only Go sources, a test, a changelog fragment, and the settings-parity allowlist β nothing in the Dockerfile or build-cache configuration β so there is no code fix to push. The build should pass on a re-run once the cache is warm/repopulated. π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Read ledger.go, boundary.go and claim.go at this head. Looks correct to me, and it is the per-target option #14645 asked for first.
What I checked, since TargetWriterScope depends on the subject shape rather than a structured field:
boundary.go:302-304builds the subject asrepo#<kind>/<target>; every kind insrc/crux/effects/effects.go:19-30is a plain snake_case token with no/, soledger.go:79-87strips exactly the kind and keys on the target. A numeric target (strconv.Itoa(number), the normal case) buckets asrepo#N; a branch-named target (automerge_contributor_adopt.go:677,:703βhive/adopt-7) is left withadopt-7after the first/, failsisDecimal, and falls back to the repo bucket as the doc comment promises.claim.go:116-129βOverlapsis untouched, so the repo-claim-vs-task-claim exclusion still holds across buckets; only the capacity count changed.OpenLedgerkeepsRepoWriterScope(ledger.go:155), so existing callers and the cross-process tests keep today's semantics; onlybootAdvisoryWithopts into the target scope.MutationMaxWritersPerTarget()returns 0 for nil/unset/non-positive andOpenLedgerScopedmaps 0 to the default of 1, so an absentconvergence.mutationblock behaves as the changelog states.
One remark, not a finding: the error-string branch at ledger.go:313-317 picks the "repo β¦ exhausted" wording by scope == c.Repo, which is also what a target-scoped ledger emits for branch pushes/creates β accurate, since those are counted per repo.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Why
#14645 found that in 4 hours on one hive, 110 of 111
shadow fence would have denied external mutationwarnings saidrepo hivecommons/hive writers exhausted (max 1). Most were label-only writes: 85label_mutation, 11issue_comment, and a handful of other kinds. The cause isDefaultMaxWritersPerRepo = 1together withactiveWritersLocked(repo). That gives each repository one writer slot shared by every issue, PR and mutation kind, so a single in-flight review or merge on any PR blocks every label write on every other item. These writes all still ran because the boundary is in shadow mode. Once it moves to enforce, they would be denied.Decision taken by hive
The writer slot is now scoped per target (
owner/repo#N) instead of per repository. Mutations with no issue or PR number (branch_push,issue_create,pull_request_create, digest/branch targets) fall back to one shared pool per repo. The default stays max 1 per scope, and the limit can be set with the new keyconvergence.mutation.max_writers_per_target. We picked per-target over per-(repo, kind) because it is the simplest change that fully separates unrelated items. Writes to the same item still go one at a time, which keeps label, review and merge ordering on one PR serialized.The change adds code and renames nothing:
mutation.OpenLedgerandDefaultMaxWritersPerRepobehave as before (per-repo scope).WriterScope,RepoWriterScope,TargetWriterScope,OpenLedgerScoped,DefaultMaxWritersPerTarget.cmd/hivenow opens the boundary ledger withOpenLedgerScoped(..., cfg.MutationMaxWritersPerTarget(), TargetWriterScope).repo X writers exhausted (max N), and target-scoped claims readtarget X#N writers exhausted (max N). The boundary.go comment now describes both.Tests (each fails without the change)
repo#numberis refused withtarget β¦ writers exhausted (max 1).max_writers_per_target: 2lets two writers onto one target and refuses the third.label_mutationon β¨ Multi-agent KubeStellar deployment: supervisor + 4 executorsΒ #5 and docs: update supervisor CLAUDE.md with EXECUTOR MODE and dispatcher protocolΒ #6 are admitted while areview_submiton β¨ Add macOS/launchd support, hybrid scanner pattern, and KubeStellar case studyΒ #3 is in flight, and a label on β¨ Add macOS/launchd support, hybrid scanner pattern, and KubeStellar case studyΒ #3 itself is still denied.All tests use injected clocks and contain no
time.Sleep.Closes #14645
β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@clubanderson