Skip to content

Prioritize filtered repos during manual sync#405

Merged
mariusvniekerk merged 6 commits into
mainfrom
prioritize-filtered-repos-on-refresh
May 30, 2026
Merged

Prioritize filtered repos during manual sync#405
mariusvniekerk merged 6 commits into
mainfrom
prioritize-filtered-repos-on-refresh

Conversation

@mariusvniekerk
Copy link
Copy Markdown
Collaborator

@mariusvniekerk mariusvniekerk commented May 29, 2026

Summary

  • Pass the current repo filter through manual refresh so selected repos are synced first.
  • Reorder the ad-hoc sync queue locally without changing the tracked repo set or sync cadence.
  • Regenerate the OpenAPI/client artifacts and wire the UI sync store to send the priority list.
  • Add coverage for the sync store, sync ordering, and the HTTP refresh path.

Testing

  • go test ./internal/github/syncertest -run TestSyncerTriggerRunWithPrioritySyncsSelectedReposFirst -shuffle=on
  • go test ./internal/server/e2etest -run TestTriggerSyncE2EPrioritizesFilteredRepos -shuffle=on
  • go test ./internal/server -run 'TestOpenAPIDocumentsCustomStatusCodes|TestAPITriggerSyncBypassesNextSyncAfter' -shuffle=on
  • bun --cwd frontend vitest run ../packages/ui/src/stores/sync.svelte.test.ts
  • bun run typecheck (frontend)
  • Pre-commit hooks passed, including test-short

@mariusvniekerk @codex
fix: prioritize filtered repos during manual sync
c20dd64 
Manual refreshes used the configured repository order even when the user had narrowed the dashboard to a small set of repos. That made the visible repos wait behind unrelated sync work, especially on larger configurations with bounded parallelism.\n\nThe refresh endpoint now accepts the current repo filter as an ordered priority list and the syncer only reorders the local run snapshot, leaving the tracked repo set unchanged. Filter values that do not resolve to tracked repos are ignored so stale UI state cannot break a sync.\n\nValidation: go test ./internal/github/syncertest -run TestSyncerTriggerRunWithPrioritySyncsSelectedReposFirst -shuffle=on; go test ./internal/server/e2etest -run TestTriggerSyncE2EPrioritizesFilteredRepos -shuffle=on; go test ./internal/server -run 'TestOpenAPIDocumentsCustomStatusCodes|TestAPITriggerSyncBypassesNextSyncAfter' -shuffle=on; bun --cwd frontend vitest run ../packages/ui/src/stores/sync.svelte.test.ts; bun run typecheck (frontend)

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (c20dd64)

Clean: both reviews found no Medium, High, or Critical issues.

Synthesized from 2 reviews (agents: codex | types: default, security)

@mariusvniekerk @codex
docs: document manual sync priority filters
9a4fa63 
The roborev design pass flagged that priority_repo's accepted wire format was implicit. Documenting the accepted repeated/comma-separated values and host-qualified versus bare repo path matching in the API source keeps the OpenAPI-generated clients and schema self-describing.\n\nValidation: go test ./internal/server -run TestOpenAPIDocumentsCustomStatusCodes -shuffle=on; go test ./internal/server/e2etest -run TestTriggerSyncE2EPrioritizesFilteredRepos -shuffle=on

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (9a4fa63)

The PR has one medium-severity issue to address before merge.

Medium

  • internal/server/huma_routes.go:2893 - matchPriorityRepo treats any filter with three or more slash-separated segments as host/repo_path, so a bare nested repo path like group/subgroup/project cannot match a tracked GitLab repo even though the API docs say bare values match by repo path. Only use the host-qualified branch when the first segment looks like a host, such as containing . or :, or reuse the existing repo filter parsing semantics so bare nested repo paths compare against repoPathForPriority.

Synthesized from 2 reviews (agents: codex | types: default, security)

@mariusvniekerk @codex
fix: support nested repo paths in sync priorities
f01423d 
Roborev caught that documenting bare repo paths exposed an existing ambiguity: a nested repo path such as group/subgroup/project was interpreted as host/repo_path and could never match a tracked GitLab repo. Try the tracked repo_path match before host-qualified parsing, and only treat a value as host-qualified when the leading segment looks like a platform host.\n\nValidation: go test ./internal/server -run TestMatchPriorityRepoSupportsNestedBareRepoPaths -shuffle=on; go test ./internal/server/e2etest -run TestTriggerSyncE2EPrioritizesFilteredRepos -shuffle=on

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (f01423d)

Medium finding remains: host-qualified priority filters can miss valid single-label self-hosted hosts.

Medium

  • internal/server/huma_routes.go:2896
    Host-qualified priority filters only work when the first path segment “looks like” a host by containing ./: or being localhost. Config validation allows single-label self-hosted provider hosts such as gitea, so a valid filter like priority_repo=gitea/acme/widget is treated as a bare repo path and will not prioritize the tracked repo on host gitea.

    Fix: Match host-qualified filters by comparing the first segment against tracked repo hosts instead of using looksLikePlatformHost, and add coverage for a single-label self-hosted host.

Synthesized from 2 reviews (agents: codex | types: default, security)

@mariusvniekerk
fix: support self-hosted sync priority filters
025d950 
Manual sync priority filters can be host-qualified, but the previous matcher rejected single-label self-hosted hosts such as gitea before comparing against tracked repositories. Compare the host segment against tracked repo hosts so those filters prioritize the intended repository while preserving bare nested repo path matching.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (025d950)

Summary verdict: One medium test coverage gap remains; no security findings were reported.

Medium

  • internal/server/huma_routes.go:2898 - The new priority filter matching includes nested bare-path and host-qualified/self-hosted behavior, but full-stack e2e coverage only covers flat github.com repo filters. Add an API + SQLite e2e with a configured non-default host and/or nested repo_path, then POST /api/v1/sync?priority_repo=<host>/<nested-path> and assert that repo is dispatched before the rest.

Synthesized from 2 reviews (agents: codex | types: default, security)

@mariusvniekerk
test: cover hosted sync priority filters
6006f45 
The manual sync priority matcher now supports host-qualified filters for non-default provider hosts. Add an HTTP and SQLite e2e case that configures a GitHub Enterprise-style host and verifies the selected repository is dispatched first through the refresh endpoint.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (6006f45)

No Medium, High, or Critical findings were reported.

Security review found no issues.

Synthesized from 2 reviews (agents: codex | types: default, security)

@mariusvniekerk
test: cover single-label sync priority hosts
f7b6bcb 
The manual sync priority bug affected single-label self-hosted provider hosts. Adjust the new e2e coverage to use that host shape so the HTTP sync endpoint verifies the regression-prone filter format directly.
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (f7b6bcb)

No Medium, High, or Critical findings reported.

Both reviews are clean at the requested severity threshold.

Synthesized from 2 reviews (agents: codex | types: default, security)

@mariusvniekerk mariusvniekerk merged commit 8bb6b4f into main May 30, 2026
16 checks passed
@mariusvniekerk mariusvniekerk deleted the prioritize-filtered-repos-on-refresh branch May 30, 2026 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mariusvniekerk