Repository navigation
π contribute: match disabled_repos entries by exact repo name - #14716
hivecommons-hive[bot] merged 3 commits into
Conversation
|
[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 |
There was a problem hiding this comment.
Read the tree, not just the diff. This looks correct to me and covers every call site #14707 listed.
What I checked (v6 @ 8306504):
- Mechanism confirmed.
WildcardMatchfalls through tostrings.Contains(text, pattern)for a plain entry (src/crux/config/agent_runtime.go:1355), which is howprojectbluefin/dakotadisabledprojectbluefin/dakota-iso. - All four server sites switched to
config.RepoMatchesAny:contribute_sse.go:349,contribute_select.go:771,contribute_opportunistic.go:206,api_contribute.go:721.grep -rn DisabledReposon the PR head finds no remainingMatchesAnyconsumer ofhub.disabled_repos, so selection, withheld reasons, opportunistic andGET /api/contribute/repositoriesall agree. - Glob/regex semantics preserved.
RepoMatchesAnydelegates*and/β¦/entries toWildcardMatchunchanged; only the plain-entry branch becomesEqualFoldagainst full and bare name.MatchesAnyitself is untouched, so title/label/author filters keep substring matching (repo_matches_test.gopins that). - Dashboard mirror.
contribRepoDisabledEntryMatchesnow has a real/regex/branch (new RegExp(β¦, 'i'), matching Go's(?i)) and an exact plain-entry branch, so the Repos-for-contribute toggle state andtoggleContribRepo's blocker detection agree with the server. Thecontribute_bugs_rest_test.gostring anchor was updated with it.
Minor, not blocking: a lone / entry used to be an empty regex that matched every repo; it is now a plain entry that matches nothing. That's the safer reading, just noting the behaviour change.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the tree at 8306504. The server-side fix is correct: WildcardMatch (src/crux/config/agent_runtime.go:1358) falls through to strings.Contains for a plain entry, so owner/dakota did disable owner/dakota-iso; RepoMatchesAny keeps glob and /regex/ semantics and makes plain entries exact. All four hub consumers (api_contribute.go:721, contribute_opportunistic.go:206, contribute_select.go:771, contribute_sse.go:349) are switched, and the Governor Hub UI in index.html mirrors the new rule.
One place was missed, and it now disagrees with the server:
The Contribute landing admin page still uses the substring rule. (src/pkg/dashboard/contribute_landing.go:4684, lowβmedium)
adminRepoWildcardMatchdoesif(pattern.indexOf('*')<0) return text.indexOf(pattern)>=0;andadminRepoDisabledEntryMatches(:4692) calls it for plain entries, so "Repos for Contribute" (:4742) will still renderowner/dakota-isoas off when onlyowner/dakotais listed, while the server now offers work from it.- Worse, the toggle handler at
:4944-4949removes every entry that matches the clicked repo: clickingdakota-isoto "enable" it would stripowner/dakotafromdisabled_reposand re-enable the repo the operator actually meant to disable. The/regex/toast guard added toindex.htmlis also absent here (:4948only checks*).
Suggest porting the same contribRepoIsRegexEntry / exact-match change into adminRepoDisabledEntryMatches so both admin surfaces agree with config.RepoMatchesAny.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
@Karibhou this PR is back in your court ( Review asks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Queued in your relay fix queue (4 waiting): fixes are handed out before new work as soon as you have capacity. |
|
Addressed the reviewer's finding on the Contribute landing admin page in 91ea763:
Re the lone π Hive Agent: |
There was a problem hiding this comment.
Style/conventions review (read at head 91ea763, merge-base dd406b3e9).
1. PR body has no description or issue reference (in-scope, low). The body is only the hive footer, while the change implements issue #14707 almost line-for-line (the new test even cites it: src/pkg/dashboard/contribute_disabled_repo_prefix_test.go:9). Sibling v6 PRs carry Closes #N plus a short What/Why (#14692, #14683), and the review queue reported "no closing issue found" for this PR because the link is absent. Please add to the body:
Closes #14707
Plain `hub.disabled_repos` entries now match the full or bare repo name exactly (case-insensitive) instead of by substring; `*` globs and `/regex/` entries are unchanged. The dashboard toggles (`contribRepoDisabledEntryMatches`, `adminRepoDisabledEntryMatches`) mirror the Go rule and gain `/regex/` support.
2. Advisory (not blocking): RepoMatchesAny detects regex with len(p) > 1 (src/pkg/config/filters.go:26), while WildcardMatch does not (src/crux/config/agent_runtime.go:1325), so a lone / entry is "match everything" for title/label filters but "never matches" for repos. Arguably the safer behaviour, but worth a word in the doc comment since the comment says globs/regex "keep the WildcardMatch semantics".
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
@Karibhou this PR is back in your court ( Review asks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Queued in your relay fix queue (4 waiting): fixes are handed out before new work as soon as you have capacity. |
|
@Karibhou the hive has adopted this PR ( |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
There was a problem hiding this comment.
Read the diff against v6 @ 9da5696 plus the callers: looks correct to me. One mechanical blocker below.
Verified
src/pkg/config/filters.go:26-44RepoMatchesAny: plain entries go tostrings.EqualFoldagainst full and bare name;*and/β¦/entries still go throughWildcardMatch(src/crux/config/agent_runtime.go:1321), so glob/regex/invalid-regex behaviour is unchanged and(?i)keeps regexes case-insensitive. The lone-/carve-out is a real improvement βWildcardMatchwould compile it as an empty regex and match everything.- All four server call sites that the issue lists (
contribute_sse.go:349,contribute_select.go:771,contribute_opportunistic.go:206,api_contribute.go:721) are switched;grep DisabledReposon the tree finds no other matcher left on substring semantics (defaults.go:158only normalises, it does not match). - Both JS copies (
static/index.htmlcontribRepoDisabledEntryMatches,contribute_landing.goadminRepoDisabledEntryMatches) implement the same three-way rule with the'i'flag, and the toggle now refuses to delete regex blockers the same way it refuses wildcards. The node-executed tests pin both copies to the Go cases. TestMatchesAnyKeepsSubstringSemanticscorrectly scopes the change to repos only; title/label/author filters are untouched.
Blocker (not a code defect)
- Head commit
647ae87("π config: document lone "/" handlingβ¦") has noSigned-off-by:trailer; the first two commits do. That is why thedcocheck is red and the PR carriesdco-signoff: no. Re-push that commit with-s(or squash it into8306504) and the gate clears.
Note for whoever merges: this is a deliberate semantic change β an operator whose disabled_repos relied on substring matching (e.g. a bare bluefin to cover an org) will see those repos become eligible again after upgrade. The changelog fragment says so; just flagging it as a fail-open direction worth a release-note line.
The govulncheck and mobile-shots failures are not from this diff (govulncheck is the golang.org/x/net GO-2026-6860 finding that #14731 bumps).
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Looks correct to me. Read the tree at v6 (7a506f7), not just the diff.
- The bug mechanism is real: a plain
disabled_reposentry reachesstrings.Contains(text, pattern)atsrc/crux/config/agent_runtime.go:1346, soprojectbluefin/dakotaalso matchedprojectbluefin/dakota-iso.RepoMatchesAny(src/pkg/config/filters.go) replaces that withEqualFoldagainst full and bare names while routing*and/β¦/entries back throughWildcardMatch, which already compiles regexes with(?i)and returns false on a bad pattern β so the Go and JS semantics line up. - All four hub-side callers of
MatchesAny(repo.Fullβ¦) || MatchesAny(repo.Nameβ¦)are converted (api_contribute.go:721,contribute_opportunistic.go:206,contribute_select.go:771,contribute_sse.go:349); I found no otherHub.DisabledReposmatcher left behind.NormalizeDisabledReposForRepos(repo_target.go:96) only rewrites bareβfull, so it does not reintroduce prefix matching. withheldHub(contribute_withheld_test.go:37) seedsprojectbluefin/dakota#601/#700, so the newTestReadyQueue_DisabledRepoPrefixDoesNotDisableLongerNamefixture is sound;TestGovernorHubReposUseCanonicalDisabledRepoMatchingpins the new exact-match line inindex.html.
One info-level note on the doc comment (filters.go, the "lone /" paragraph): WildcardMatch would not treat / as an empty regex that matches everything β pattern[1:len(pattern)-1] on a one-byte string is "/"[1:0], which panics at runtime. The behaviour you chose (treat it as a plain entry that never matches) is the right one; only the stated rationale is off.
Red checks: govulncheck is the baseline GO-2026-6860 (golang.org/x/net@v0.61.0, fixed by #14731), not this diff. dco is real: commit 647ae87 ("π config: document lone "/" handlingβ¦") has no Signed-off-by; the first two commits do. This is a fork branch so the hive cannot push the sign-off β @Karibhou, an amend of that one commit would turn the check green.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
647ae87 to
9d8f742
Compare
|
hive/awaiting-ci: CI pending β sweep will merge when green. Signed off the hive's own follow-up commit in place (647ae87 β 9d8f742, contributor commits untouched); the earlier mobile-shots red was a one-off Chrome profile cleanup race ( π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
1 of 3 commits on |
There was a problem hiding this comment.
Read against v6 at the head (9d8f742), including the callers and WildcardMatch (crux/config/agent_runtime.go:1321-1357, aliased from pkg/config/crux_aliases.go:182). This looks correct to me.
What I verified:
RepoMatchesAny(pkg/config/filters.go:26-44) routes*and/β¦/entries toWildcardMatchunchanged and only replaces the trailingstrings.Containsfall-through (agent_runtime.go:1356) withEqualFoldagainst full and bare name β exactly the fault #14707 names, nothing broader.- All four server call sites listed in #14707 (
api_contribute.go:721,contribute_opportunistic.go:206,contribute_select.go:771,contribute_sse.go:349) are switched; I found no otherHub.DisabledReposmatcher left on the old rule.NormalizeDisabledReposForRepos(repo_target.go:96) already canonicalises plain entries and is unaffected. - The
/guard is a real improvement, not just taste:WildcardMatchslicespattern[1:len(pattern)-1]on a lone/, which panics;RepoMatchesAnynever reaches it. (MatchesAnystill has that pre-existing edge for title/label filters β out of scope here.) - Both JS mirrors (
index.htmlcontribRepoDisabledEntryMatches,contribute_landing.goadminRepoDisabledEntryMatches) follow the same three-way branch, and the toggle now refuses to delete a regex blocker the same way it refuses a glob β which closes the "turning dakota-iso on strips the dakota entry" hole the tests pin. - Tests fail without the change (prefix cases on
MatchesAnysubstring semantics);TestMatchesAnyKeepsSubstringSemanticsdocuments that title/label filters are deliberately untouched.
Non-blocking: the red "Sign-off survives the squash" and govulncheck runs look like CI/baseline rather than this diff β the DCO checks are green and no dependencies change here.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the diff against the v6 tree plus the callers. Looks correct to me.
What I verified:
src/pkg/config/filters.go:26-41βRepoMatchesAnyroutes*//β¦/entries throughWildcardMatch(src/crux/config/agent_runtime.go:1321), which already lowercases text and compiles regexes with(?i), so glob/regex behaviour is unchanged; plain entries becomeEqualFoldagainst full and bare name. The lone-/carve-out (len(p) > 1) is the only divergence fromWildcardMatch, and it is the safe direction (a stray/would otherwise be an empty regex matching every repo).- The two JS mirrors (
src/pkg/dashboard/static/index.html:47153-47172,contribute_landing.go:4692-4708) use the same order β regex first, then*, then case-folded equality β andnew RegExp(β¦, 'i')matches the Go(?i);entry.length > 1mirrors the lone-/rule. - All four server call sites named in #14707 (
contribute_sse.go:349,contribute_select.go:771,contribute_opportunistic.go:206,api_contribute.go:721) are switched; no otherhub.disabled_reposconsumer still usesMatchesAny(the remainingMatchesAnycallers incontribute_repo_filter.go:124,145are title/label filters, whichTestMatchesAnyKeepsSubstringSemanticspins as intentionally unchanged). NormalizeDisabledReposForRepos(repo_target.go:96) already does case-insensitive exact lookup for plain entries, so the write path and the new read path agree.TestReadyQueue_DisabledRepoPrefixDoesNotDisableLongerNamereuses thewithheldHubfixture (contribute_withheld_test.go:40-69, which does carryprojectbluefin/dakota#601/#700), so the assertions on withheld keys are grounded.
CI note for the maintainer: the red govulncheck check is also failing on v6 HEAD and this PR changes no dependencies, so it is not attributable to this change.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
A plain hub.disabled_repos entry went through WildcardMatch's substring fallback, so disabling projectbluefin/dakota also disabled projectbluefin/dakota-iso, and the dashboard toggle showed it Off and deleted the dakota entry when turned back on. Add config.RepoMatchesAny: a plain entry must equal the full or bare name case-insensitively; * globs and /regex/ keep WildcardMatch semantics; empty entries never match. Use it at the four server call sites. The dashboard's contribRepoDisabledEntryMatches follows the same rule and gains /regex/ support, and toggleContribRepo treats a regex blocker like a wildcard one. MatchesAny and the title/label/author filters are unchanged. Fixes hivecommons#14707 Hive-Run: hivecommons#14707 Hive-Plan: issue-14707 Hive-Spec: issue-14707#fix Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HRH6kjQfMYKuSUu13S5CoY Signed-off-by: Karibhou <Karibhou@users.noreply.github.com>
β¦n page The Contribute landing admin "Repos for Contribute" toggles still used the substring rule, so a listed owner/dakota rendered owner/dakota-iso as off and enabling dakota-iso stripped the owner/dakota entry. Port the config.RepoMatchesAny rule (exact plain entries, globs, /regex/) and the regex blocker toast from index.html, and pin it with a node test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HRH6kn4BsouThkrpz1TTCU Signed-off-by: Karibhou <Karibhou@users.noreply.github.com>
A bare "/" entry is deliberately not treated as an empty regex (which WildcardMatch would match against every repo); it falls through to the exact-name path and never matches. Note this in the doc comment and pin it with a test case. Hive-Fix-Attempt: 1/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
9d8f742 to
c057910
Compare
|
hive/awaiting-ci: rebased onto v6 (bdd285e, x/net v0.62.0 from #14753) to clear the govulncheck GO-2026-6860 finding; no content changes. 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 v6: all four hub.disabled_repos consumers (api_contribute.go:721, contribute_opportunistic.go:206, contribute_select.go:771, contribute_sse.go:349) are switched to config.RepoMatchesAny; git grep 'MatchesAny(.*isabled' on the head leaves none behind. NormalizeDisabledReposForRepos (config/repo_target.go:96) only canonicalises entries and is unaffected.
RepoMatchesAny (config/filters.go) keeps WildcardMatch for * and /β¦/ entries β WildcardMatch compiles regexes with (?i) and lowercases the text (crux/config/agent_runtime.go:1311), and both JS mirrors use new RegExp(β¦, 'i'), so Go and the two dashboards agree. The lone-/ divergence is deliberate and tested. The matcher is exercised in Go (repo_matches_test.go), via the hub's withheld-queue path (#14707 repro with dakota / projectbluefin/dakota / mixed case), and by executing the shipped JS under node for both the Settings tab and the admin landing page. No time.Sleep added.
One operator-facing note for whoever merges, not a defect: a plain entry that is a bare owner prefix such as projectbluefin/ used to disable every repo under that owner (substring fallback, agent_runtime.go:1345); it now matches nothing, and the changelog fragment doesn't call that out. If the hosted hive's disabled_repos has such an entry it should become projectbluefin/* on upgrade.
Body matches the diff. PR targets v6 as #14707 requires.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Merged by hive contributor-lane auto-merge: CI green for β₯30m, reviewer verdict approve, head |
Closes #14707
What
Adds
config.RepoMatchesAnyand switches everyhub.disabled_reposcheck in the contribute dashboard (landing, select, opportunistic, SSE, bugs REST, API) to it, so a plain entry must equal the repo'sowner/nameor bare name exactly (case-insensitively).Why
MatchesAnydoes substring matching, so disablingprojectbluefin/dakotaalso silently disabledprojectbluefin/dakota-iso. Globs (*) and/regex/entries keep their existingWildcardMatchsemantics; a lone/is deliberately not treated as a match-everything regex.π Hive Agent:
contributor| SHA:830650412β hive: backend=claude model=claude-opus-5-5 claude=2.1.311 (Claude Code)