Skip to content

Resolve groups to repository selectors (GH-20) - #58

Merged
Ilyes512 merged 3 commits into
mainfrom
feat/GH-20-resolve-groups-to-selectors
Aug 9, 2026
Merged

Ilyes512 merged 3 commits into
mainfrom
feat/GH-20-resolve-groups-to-selectors

Conversation

@Ilyes512

@Ilyes512 Ilyes512 commented Aug 8, 2026 •

Copy link
Copy Markdown
Member

Closes #20.

Summary

  • internal/config/resolve.go turns the groups section into selectors, not repository lists — enumeration needs the network and lands with repos.go: repository enumeration, pagination and filtering #33, and staying offline is what makes composition, cycles, and glob precedence testable without an HTTP mock.
  • Exactly one source per group; zero or two is ErrAmbiguousGroupSource. include_groups flattens into a deduplicated union, and a cycle is reported as the chain to break (a -> b -> c -> a, ErrCyclicGroup).
  • include then exclude via fnmatch, over the repository name only. The user split rides on the selector as AuthenticatedUser, and visibility: private for anybody else warns instead of silently selecting nothing.
  • Resolution.Desired is the resolution rule, including the case the tool's safety rests on: no group resolves to R, so R wants nothing — which is not the same answer as an empty config.
  • Adds one dependency, github.com/danwakefield/fnmatch, for the glob matching the design's groups reference specifies.

Rebased on main, and what that turned up

This branch was rebased onto main after #60 (validate.go) and #59 (palette) merged. The docs conflicts were consolidations — both sections kept, main's Validation section alongside this branch's Resolution section — but the rebase also put two implementations of the same four rules into one package, and they had already drifted:

validate.go (as merged) resolve.go (this branch)
Cycle chain a → b → a a -> b -> a
Ambiguous source strings.Join(sources, " and ") describeSources(...)

Validate runs first, so the only message a user could ever see was the one neither docs page documented. validate.go:165 also carried a comment saying it walked the graph "so that resolve.go can expand a group without a visited set of its own" — which this branch's resolve.go had made untrue.

So the last commit gives the four group-graph rules one owner:

  • validateGroups is now _, err := c.Resolve(""); return err. Resolution has to establish all four before it can build a selector — a group with two sources has no one source to resolve, a cycle cannot be flattened — so a copy in validate.go was a second implementation of a rule that already existed. validateGroupSource, validateNoGroupCycle, repoRefPattern, and the newly-unused sortedKeys are gone; validate.go is ~100 lines shorter.
  • The login passed to Resolve is empty because none of the four rules depends on it, and the resolution is discarded — which also keeps the visibility: private warning a property of the run rather than something validation swallows.
  • One behaviour change worth a look: ParseRepoRef was weaker than the pattern it replaces — it accepted specsnl/label sync, which validate.go rejected and has a test for. Delegating naively would have silently dropped a real rule, so ParseRepoRef picked up the character check and is now the single judge of a reference, for a repos: entry and a --repo alike. Whitespace around either half is still trimmed (specsnl / labelsync parses); a space or colon inside one is now ErrInvalidRepoRef.
  • The inherited comment claiming a trailing .git is caught was wrong and is corrected: a dot is in GitHub's character set, so docs.example.com is an ordinary name and .git cannot be told apart from one. There is now a test recording that it is taken literally.
  • TestValidateReportsTheResolveMessage pins all four messages through Validate, because that is the call a user's failure comes from, and its doc comment says that changing it is a change to usage/_index.md and architecture/configuration.md too.

Scope

Every box on #20 is ticked. Judgement calls the issue did not settle, all documented in the code:

  • Glob matching is case-insensitive (FNM_CASEFOLD), because GitHub repository names are — owner/Repo and owner/repo are one repository.
  • A repos selector carries no filters at all. include/exclude/skip_*/visibility apply to org and user per the design's reference table; a filter that dropped a repository the config named outright would be a surprise rather than a safety net.
  • The group rules live in resolve.go rather than validate.go, per the section above. The rule table in architecture/configuration.md gained a "Checked in" column so the split is on the page rather than something to rediscover.

Docs

architecture/configuration.md — the Resolution section (selectors, one source and composition, filters, the user split, the resolution rule), plus the rule table's "Checked in" column and a section on why the group rules have one owner. usage/_index.md — the same rules from the config file's side, including the one that matters most: a repository no group selects is never touched.

Checks

task checkall is green, as is the CI sequence run locally: go mod tidy -diff, golangci-lint run (0 issues), go test -race -tags=integration ./..., and markdownlint (0 issues).

internal/config/resolve.go turns the groups section into selectors rather
than repository lists: enumeration needs the network and lands with #33,
and keeping this package offline is what makes composition, cycles, and
glob precedence testable with plain stdlib tests.

- Exactly one source per group; zero or two is ErrAmbiguousGroupSource
- include_groups flattened as a deduplicated union, cycles reported as the
  chain that has to be broken (ErrCyclicGroup)
- include then exclude, via fnmatch over the repository name only
- The user split — authenticated user or not — is carried on the selector,
  because internal/github cannot re-derive it, and asking for a private
  visibility of somebody else's repositories warns rather than silently
  selecting nothing
- Resolution.Desired implements the resolution rule, including the case
  the tool's safety rests on: no group resolves to R, so R wants nothing
Architecture: what a selector is and why it is not a repository list, the
one-source rule, diamond versus cycle, filter precedence, the user split,
and the resolution rule.

Usage: the same rules from the config file's side, including the one that
matters most — a repository no group selects is never touched.
The rebase onto main landed validate.go's group checks alongside
resolve.go's, and the two copies had already drifted: the cycle chain was
printed with " → " in one and " -> " in the other. Since Validate runs
first, users only ever saw the arrow the docs did not promise.

resolve.go now owns all four rules about the group graph — one source per
group, the groups include_groups names, no cycle, and owner/repo
references — because resolution cannot produce a selector without
establishing each of them anyway. validateGroups delegates to
Resolve(""), so there is one implementation and one message per rule.

ParseRepoRef becomes the single judge of a reference, so a repos: entry
and a --repo are held to the same rule. It gained the character check
validate.go had: whitespace around either half is still trimmed, a space
or colon inside one is now ErrInvalidRepoRef.

A new test drives the four messages through Validate and pins them by
name, since that is the call a user's failure comes from, and says that a
change to it is a change to both docs pages.
@Ilyes512
Ilyes512 force-pushed the feat/GH-20-resolve-groups-to-selectors branch from 6a39a14 to d71bc08 Compare August 9, 2026 16:46
@Ilyes512
Ilyes512 merged commit 8d67983 into main Aug 9, 2026
5 checks passed
@Ilyes512
Ilyes512 deleted the feat/GH-20-resolve-groups-to-selectors branch August 9, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

resolve.go: groups to selectors, include_groups, cycles, globs

1 participant