Skip to content

feat: hide repositories from the UI via a per-repo gear menu - #862

Open
wesm wants to merge 13 commits into
mainfrom
feat/repository-ui-visibility
Open

feat: hide repositories from the UI via a per-repo gear menu#862
wesm wants to merge 13 commits into
mainfrom
feat/repository-ui-visibility

Conversation

@wesm

@wesm wesm commented Aug 8, 2026

Copy link
Copy Markdown
Member

Repositories can now be hidden from the UI without removing them from configuration.

  • Each exact repository row in Settings gains a gear menu holding the existing local clone path editor and a "Hide from UI" / "Show in UI" action.
  • Hidden repositories drop out of the repository list, repository summaries, the repo typeahead, global repo filters, and the mobile activity selector. Direct links, item feeds, and the Settings page itself still show them, and syncing continues unchanged.
  • The preference is stored against the provider-verified repository identity, so it survives renames and is not inherited by a different repository that later reuses the same owner/name route.
  • Hiding applies to exact repositories only: glob rows have no gear menu and the API rejects glob targets; repositories not yet verified by their provider return a conflict until a sync completes.

repo-gear-menu.png

🤖 Generated with Claude Code

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (84b723f)

Changes need fixes: three medium-severity issues remain.

Medium

  • internal/server/helpers.go:129 — Renamed repositories retain only one configured-path provenance. If a visible current-path entry precedes a hidden old-path entry, deduplication discards the hidden provenance, making “hidden wins” depend on configuration order. Retain all configured paths per stable repository identity and test both orderings.

  • packages/ui/src/views/mobileActivityRepoOptions.ts:26 — An existing hidden repository selection is not cleared when mobile options exclude it. The global normalizer preserves unknown provider-qualified values, leaving activity filtered to a repository absent from the selector. Strictly validate the mobile selection against interactive options and clear unavailable selections.

  • internal/server/e2etest/settings_test.go:767 — Coverage verifies persistence only; no full-stack browser test confirms that the Settings toggle updates repository selectors and workspace creation. Add Playwright coverage using the real API and seeded database to hide and restore a repository and clear an active selection.


Reviewers: 2 done | Synthesis: codex, 6s | Total: 5m57s

@wesm

wesm commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

settings-globs-pr.png

@wesm

wesm commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Repository visibility action in the repository gear menu:

repository-visibility-gear-menu.png

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (67f58aa)

Changes need one medium-severity fix before approval.

Medium

  • packages/ui/src/views/mobileActivityRepoOptions.ts:26 — Hidden repositories are excluded from the mobile selector, but App.svelte::getNormalizedGlobalRepo still validates persisted selections against all configured repositories. A hidden repository can therefore remain selected on mobile, continue filtering activity, pull requests, and issues, and appear as a dropdown value without a matching option.

    Suggested fix: Normalize mobile selections against the visible repository set, or clear selectedRepo when it is absent from repoOptions. Add coverage for loading mobile views with a hidden repository persisted as the global selection.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 5m58s

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (6c90556)

Overall verdict: One medium-severity correctness issue requires attention; no material security vulnerabilities were found.

Medium

  • frontend/src/App.svelte:632 — Valid repository selections can be incorrectly cleared. Global selection validation treats visible exact configuration rows as the complete interactive catalog. Repositories discovered through visible globs, or repositories whose provider route changed after configuration, exist only in /repos; valid selections are therefore removed and cleared from local storage after settings load.
    • Fix: Validate against the merged /repos catalog plus visible exact entries, and wait until that catalog has loaded before pruning. Add coverage for persisted and newly selected glob-backed or renamed repositories.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 10m16s

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (422d7a8)

The PR has two medium-severity issues involving renamed repository visibility and missing full-stack coverage.

Medium

  • Stale configured paths can override hidden-entry visibility
    Location: packages/ui/src/utils/repo-visibility.ts:50, frontend/src/App.svelte:663, frontend/src/lib/components/RepoTypeahead.svelte:149
    Effective visibility is inferred only from configured owner/name values. If an exact repository is renamed by its provider into a path matched by a hidden glob, the server removes the current identity from /repos, but the frontend re-adds the stale configured path as a valid selection. This violates the hidden-entry-wins rule.
    Fix: Base effective visibility on resolved catalog identities and configuration provenance, or treat the filtered catalog as authoritative while adding only demonstrably unresolved exact entries. Add coverage for a renamed exact repository overlapping a hidden glob.

  • Visibility settings lack full-stack browser coverage
    Location: frontend/src/lib/components/settings/RepoSettings.svelte:269
    Existing server e2e coverage verifies only PUT persistence, while mocked and component tests do not establish that the gear action updates the real catalog, clears selectors, and survives reload.
    Fix: Add a Playwright full-stack test that hides and shows an exact repository and a glob through the gear menu, verifies /repos plus selector/workspace visibility, and confirms persistence after reload.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 10m4s

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (554d1ec)

The repository visibility controls have two medium-severity consistency issues involving renamed repositories and overlapping hidden globs.

Medium

  • Incorrect unresolved-repository detectionpackages/ui/src/utils/repo-visibility.ts:74, internal/server/settings_handlers.go:137

    matched_repo_count === 0 is treated as proof that an exact configuration row is unresolved, but matchedRepoCount ignores ConfiguredRepoPath. After a provider rename, this can restore a stale configured route and potentially resurrect a repository hidden by a glob.

    Fix: Make exact-match counting provenance-aware (for example, using repoMatchesConfig) or expose explicit configuration-to-catalog resolution state. Add coverage through the real settings and catalog APIs for a provider rename combined with an overlapping hidden glob.

  • Mobile options use stale configuration identitiespackages/ui/src/views/mobileActivityRepoOptions.ts:25

    Mobile repository options are derived from configuration identities rather than the authoritative filtered /repos catalog. Following a rename, a hidden glob can remove the current route from /repos while mobile still shows the stale configured route.

    Fix: Build mobile options from the filtered /repos catalog, applying any intentional unresolved-exact fallback consistently with the desktop selector. Add phone-profile coverage for renamed repositories hidden by overlapping configuration.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 9m5s

@wesm
wesm force-pushed the feat/repository-ui-visibility branch from 554d1ec to efafb71 Compare August 8, 2026 17:07
@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (efafb71)

Repository visibility has two medium-severity correctness issues involving renamed repositories and stale configured paths.

Medium

  • Renamed repositories can have stale paths incorrectly restoredpackages/ui/src/utils/repo-visibility.ts:74
    matched_repo_count === 0 is treated as proof that an exact row is unresolved, but matchedRepoCount in internal/server/settings_handlers.go:137 ignores ConfiguredRepoPath provenance. A provider-renamed repository can therefore report zero matches, restoring its stale configured path and potentially displaying a repository hidden under its current route. Make exact-repository counts use provenance-aware repoMatchesConfig (or expose an explicit resolution field), and test against a real post-rename settings response.

  • Mobile activity can show renamed repositories hidden by a globpackages/ui/src/views/mobileActivityRepoOptions.ts:26
    Mobile activity reconstructs visibility from configuration rows instead of the authoritative repository catalog. Since the client-side matcher lacks rename provenance, a renamed repository hidden by a glob matching its current route can reappear under its stale configured path. Build mobile options from the filtered /repos catalog, add only genuinely unresolved exact configuration rows, and cover a renamed repository hidden by an overlapping glob.


Reviewers: 2 done | Synthesis: codex, 20s | Total: 6m54s

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (539ef9c)

Code review found two medium-severity correctness issues; no security vulnerabilities were identified.

Medium

  • frontend/src/App.svelte:659 — Catalog normalization clears a repository pinned through ui.repo when hideRepoSelector is enabled and the repository is hidden from the interactive catalog. The fixed-scope view then incorrectly falls back to “All repos.” Preserve the host-pinned repository in this mode by bypassing interactive-catalog normalization.

  • frontend/src/lib/components/settings/RepoSettings.svelte:168 — Concurrent visibility changes replace the entire repository settings array with the last response received, allowing an older response to overwrite a newer row’s confirmed state. Serialize these mutations or reconcile responses by mutation generation, applying only the field and row owned by each request.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 5m52s

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (f431631)

The PR needs changes to prevent repository selections from disappearing or being cleared unexpectedly.

Medium

  • packages/ui/src/utils/repo-visibility.ts:74matched_repo_count > 0 does not guarantee the authoritative /repos catalog contains the repository. Direct exact-repository additions and fresh-startup resolution can populate the syncer before a database row is persisted, causing the configured fallback to be suppressed and the repository to disappear from selectors and workspace creation until reload.

    • Fix: Persist resolved repository identities before publishing settings/tracked state, treat identityless fallback refs as unmatched, and add a full-stack regression test for selection immediately after an exact repository is added.
  • frontend/src/lib/components/RepoTypeahead.svelte:95 — A structured /repos error ends loading without recording that catalog loading failed. The validation effect then treats an empty catalog as authoritative and calls onchange(undefined), permanently clearing the persisted repository filter after a transient API failure.

    • Fix: Track successful catalog loading separately, prune selections only after a successful response, and preserve the prior catalog and selection when loading fails.

Reviewers: 2 done | Synthesis: codex, 14s | Total: 8m1s

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (3def5af)

Changes need revision: two medium-severity consistency issues can leave repository settings persisted or displayed incorrectly.

Medium

  • internal/server/settings_handlers.go:812 — The config file is saved before persistResolvedRepos runs. If database persistence fails, only the in-memory append is rolled back, so the API returns an error while the repository remains on disk and reappears after reload. Persist the resolved identity before saving the modified config, or durably restore the prior config on failure. Add a failure-path test covering both memory and the saved file.

  • frontend/src/lib/components/settings/RepoSettings.svelte:159 — Only visibility requests use the mutation queue, while worktree edits, refreshes, removals, and additions can concurrently apply complete settings.repos snapshots. Out-of-order responses may overwrite newer state, resurrect removed rows, or show stale visibility/path values. Use one shared queue or generation-aware reconciliation for all repository-setting mutations, or disable competing mutations while a request is pending.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 11m56s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (2428c26)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 16m56s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (795dd57)

One medium-severity issue could cause hidden-state leakage across repository identities.

Medium

  • Stable repository identity violationinternal/server/helpers.go:129

    A hidden exact entry can match both its renamed repository through ConfiguredRepoPath and an unrelated repository that later reuses the old route. If a visible glob tracks both, the route successor is incorrectly removed from selectors.

    Fix: When an exact entry has a provenance match, apply its visibility only to that repository. Fall back to route matching only when no provenance-bound repository exists, and add route-reuse coverage.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 15m51s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (1dc53b4)

Repository visibility controls have two medium-severity persistence and identity issues that can surface after renames or restarts.

Medium

  • internal/server/settings_handlers.go:812 — Pre-response persistence uses DB.UpsertRepo, which does not update the route of an existing provider ID. Re-adding a previously cataloged repository after a rename reports a match, but /repos retains the old route, causing selectors and workspace creation to omit it until a later sync.

    • Fix: Persist provider-resolved results through timestamp-fenced ReconcileRepositoryObservation, and test adding a known provider ID under its renamed route.
  • internal/server/helpers.go:125 — Stable visibility binding relies on ConfiguredRepoPath, which exists only in the in-memory tracked reference. After a reload or restart, the exact path is rebound to its current occupant; following a rename and route reuse, the successor may become hidden while the originally hidden repository reappears through an overlapping glob.

    • Fix: Persist the exact config entry’s stable provider-ID association and restore it during resolution. Add coverage for rename plus route reuse across reload and restart.

Reviewers: 2 done | Synthesis: codex, 14s | Total: 11m55s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (f6ae3ea)

High-severity stable-binding flaw remains, with two medium-severity consistency issues.

High

  • internal/github/configured_repo_bindings.go:64 — When a bound route resolves to a different provider ID, the fallback remains syncable at that conflicting route. A later sync can replace it with the route successor while preserving ConfiguredRepoPath, transferring visibility and other exact-entry settings to the wrong repository.
    • Fix: Fail closed on identity mismatch. Resolve the repository by stable ID/current route, or leave it untracked until revalidated. Add full-stack coverage for rename-plus-reuse before the catalog observes the rename.

Medium

  • internal/github/configured_repo_bindings.go:121 — The accepted result from ReconcileRepositoryObservation is ignored. A delayed provider response rejected as stale can still update the exact-path binding, after which callers publish stale route and metadata.

    • Fix: Treat accepted == false as non-authoritative. Prevent binding or publishing that ref, and return either an authoritative catalog ref or a retryable error.
  • internal/server/config_reload.go:423 — Bindings are pruned before SetReposWithContext succeeds. If repository lifecycle application fails, the old in-memory configuration remains active while its bindings may already be deleted, losing stable identity across the next reload or restart.

    • Fix: Commit binding replacement only after a successful reload, or snapshot and restore bindings on every subsequent failure.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 18m55s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (2e8681b)

Code review found four medium-severity correctness and resilience issues; no security vulnerabilities were identified.

Medium

  • Non-atomic repository binding persistenceinternal/server/settings_handlers.go:799, internal/server/repo_import_handlers.go:582
    Repository bindings are persisted before the config file is saved, but save failures restore only s.cfg.Repos. Bulk persistence can also retain earlier bindings when a later item fails, leaving stale bindings that subsequent reloads may treat as authoritative.
    Fix: Make binding updates atomic across the entire mutation and restore the previous binding set whenever binding persistence or config publication fails.

  • Binding mismatches incorrectly treated as safely unresolvedpackages/ui/src/utils/repo-visibility.ts:73
    The frontend interprets matched_repo_count === 0 as proof that an exact entry is safely unresolved. Stable-binding identity mismatches also produce zero matches, causing the stale configured route to be reintroduced after the authoritative catalog excluded it.
    Fix: Expose an explicit resolution or binding-mismatch status and allow fallbacks only for genuinely unbound or offline entries.

  • Hidden repository filtering can fall back to an unsafe route matchinternal/server/helpers.go:125
    A hidden exact entry is matched by stable provenance only while its bound repository remains tracked. If identity verification removes that reference, filtering falls back to the configured route and may hide a different repository occupying that route through an overlapping glob.
    Fix: Resolve hidden exact entries using their persisted binding ID and never fall back to route matching when a stable binding exists.

  • Transient catalog failures permanently remove repository optionsfrontend/src/lib/components/RepoTypeahead.svelte:96
    An initial catalog failure leaves fetchedRepos empty while matched configured repositories are excluded as fallbacks, causing normal desktop and mobile options to disappear for the component’s lifetime.
    Fix: Preserve a usable option snapshot during catalog failures and retry requests with bounded backoff.


Reviewers: 2 done | Synthesis: codex, 16s | Total: 12m26s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (a5a99e9)

The PR needs changes for three medium-severity correctness and resilience issues.

Medium

  • internal/server/settings_handlers.go:1200 — Removing an exact repository does not remove its stable-identity binding. Re-adding the same route from config can resolve against the deleted entry’s old identity, causing incorrect rejection or tracking. Prune the binding within the deletion transaction, and restore both configuration and bindings if persistence fails.

  • internal/github/configured_repo_bindings.go:145 — Provider resolutions without a stable external ID are persisted as route-only rows and reported as successful. This allows callers to publish configuration without a verified identity, undermining route-reuse protection. Reject resolved repositories that lack a stable external ID before saving configuration or updating the tracked set.

  • frontend/src/App.svelte:513 — The app-level catalog request used to validate persisted selections has no retry behavior. On mobile, where the independently retrying typeahead is absent, a transient failure can leave a hidden or removed repository selected indefinitely. Add bounded retries or share the typeahead’s retried catalog state.


Reviewers: 2 done | Synthesis: codex, 14s | Total: 18m55s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (dc3b83f)

Verdict: Changes need follow-up for one high-severity test gap and one medium-severity mobile fallback bug.

High

  • internal/github/configured_repo_bindings.go:34 — The stable-identity route-reuse fix lacks an end-to-end test covering binding persistence, provider rename and route reuse, daemon restart/reload, and resulting tracked/API repository identity. Add a real HTTP API + SQLite test that verifies the original repository remains tracked and the successor does not inherit its configuration provenance.

Medium

  • packages/ui/src/views/mobileActivityRepoOptions.ts:47 — If the initial /repos request exhausts its retries, the mobile selector receives an empty catalog and hides normal configured repositories with matched_repo_count > 0, unlike the desktop fallback. Pass catalog load status into the option builder and, when loading fails, include visible, non-mismatched exact configuration rows.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 18m44s

@mariusvniekerk mariusvniekerk self-assigned this Aug 9, 2026
@mariusvniekerk
mariusvniekerk force-pushed the feat/repository-ui-visibility branch from e7dd346 to 7592e9b Compare August 9, 2026 17:26
@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (7592e9b)

The changes need fixes for two medium-severity consistency and crash-safety issues.

Medium

  • frontend/src/lib/components/settings/RepoSettings.svelte:102 — Repository mutations bypass SettingsWorkflow transport-uncertainty reconciliation. If a committed request loses its response, the UI reports failure and remains stale; removal also misreports a failed post-DELETE refresh as a deletion failure. Route these commands, including visibility updates, through SettingsWorkflow, or preserve its ordered reconciliation semantics and distinguish refresh failures after acknowledgement.

  • internal/server/settings_handlers.go:1211 — Deletion prunes the stable-identity binding before atomically saving the updated TOML. A process termination between those operations leaves the old configuration without its binding, potentially allowing the path to bind to a route successor after restart. Persist the configuration removal first, then prune the binding, with rollback handling pruning failures while retaining crash-safe ordering.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 16m50s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (a87bcd3)

Changes need fixes: five medium-severity correctness and concurrency issues remain.

Medium

  • Bulk imports can lose stable repository identityinternal/server/repo_import_handlers.go:510
    Bulk imports save the provider-resolved route in Config, while Ref.ConfiguredRepoPath retains the requested route. After a repository rename or redirect, the binding may be stored under a path absent from the saved config and later pruned. Set Ref.ConfiguredRepoPath from the finalized config path before persisting the binding, and add coverage for renamed-route bulk imports.

  • Concurrent glob-refresh rollback can erase valid bindingsinternal/server/settings_handlers.go:960, internal/github/configured_repo_bindings.go:112
    Glob refreshes are not serialized with config reloads, and failed persistence restores the entire binding snapshot. A concurrent refresh rollback can therefore erase exact bindings created by a successful reload. Hold configReloadMu across refresh verification, persistence, and tracked-set replacement, or transactionally isolate binding snapshot, mutation, and rollback.

  • Shared bulk-import timestamps can incorrectly reject later lookups as staleinternal/server/repo_import_handlers.go:650
    A single observation timestamp is captured before all sequential lookups. Later repositories may be rejected as stale if a sync observation occurs before their lookup, even when their provider response is newer. Capture and persist an observation timestamp immediately before each repository lookup.

  • Catalog failures can expose stale renamed repository routesfrontend/src/lib/components/RepoTypeahead.svelte:171, frontend/src/lib/views/mobileActivityRepoOptions.ts:48
    During catalog failure, every non-mismatched exact config path is treated as safe. A bound repository renamed into a hidden glob can reappear under its stale configured route, potentially selecting a route successor. Require authoritative resolved identity before exposing matched exact paths, or include the resolved route and effective visibility in the settings response.

  • Transient catalog states remove valid mobile glob optionsfrontend/src/lib/views/mobileActivityRepoOptions.ts:51
    The mobile selector ignores the retained catalog whenever a refresh starts or fails, so all glob-backed options disappear despite the last successful catalog being preserved. Track whether a catalog has loaded successfully and keep using the retained catalog during refreshes and transient failures.


Reviewers: 2 done | Synthesis: codex, 16s | Total: 18m39s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (707c6f6)

The PR has one high-severity repository identity flaw and one medium-severity test coverage regression.

High

  • internal/github/configured_repo_bindings.go:34 — Stable bindings are consulted only during startup and config reload. Normal sync can resolve a tracked mutable route to a different repository identity, while recovery from an identityless startup fallback never creates the binding. This may sync and expose the wrong repository while the daemon remains running.
    • Fix: Make regular identity reconciliation binding-aware: persist a binding when an exact fallback first resolves, resolve subsequent syncs through the bound identity’s current route, and reject cross-identity observations before publishing them. Add full-stack coverage for route reuse during a running daemon.

Medium

  • frontend/tests/e2e-full/settings-globs.spec.ts:181 — The change removes full-stack regression coverage for lost bulk-import responses, failed promotion rollback, and concurrent fleet/activity settings writes. Unit workflow tests do not cover the same HTTP, persistence, and UI integration boundaries.
    • Fix: Restore these end-to-end scenarios, moving them into dedicated specs if suite organization or runtime is the concern.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m41s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (057b69e)

Changes need revision: three medium-severity repository identity and filtering issues remain.

Medium

  • frontend/src/App.svelte:777normalizeRepoFilterSelection preserves unmatched provider-qualified values. On mobile, where RepoTypeahead is absent, a persisted hidden repository remains in local storage and continues filtering activity even though the dropdown displays “All repos.”

    • Fix: Once the catalog is authoritative, filter normalized selections against valid provider-qualified identities while retaining the outage and host-pinned guards.
  • internal/server/settings_handlers.go:1238 — Deletion reconciles tracked repositories using routes and the single ConfiguredRepoPath, ignoring remaining stable bindings. If two exact paths resolve to the same repository and the provenance-bearing path is deleted, a renamed repository can be dropped from tracking despite another exact binding remaining, or lose protection against future route reuse.

    • Fix: Transfer provenance to a remaining exact binding targeting the same provider ID, or rebuild the tracked set from all remaining bindings before publishing it.
  • internal/github/sync.go:6197, internal/server/settings_handlers.go:1218 — Binding creation is not fenced to the current configured-repository generation. A sync started before deletion can recreate a pruned binding for an entry that no longer exists, causing a later hot reload or re-add to follow stale identity.

    • Fix: Add a configuration-generation fence shared by sync reconciliation and pruning, rejecting binding writes when the originating tracked entry is no longer current.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 22m11s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (5fc12f4)

One medium-severity concurrency issue should be addressed before merge.

Medium

  • internal/github/configured_repo_bindings.go:124 — Failure rollback restores the entire binding-table snapshot. A concurrent sync may create a valid binding after that snapshot, only for ReplaceConfiguredRepoBindings to delete it during rollback, undermining stable repository identity across restarts or route reuse. Serialize the full snapshot/mutate/rollback sequence with all binding writers, or roll back only keys changed by this operation using compare-and-swap semantics.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 20m13s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (338247d)

High-risk concurrency issues remain in repository binding synchronization.

High

  • internal/github/sync.go:6249ensureCurrentConfiguredRepoBinding acquires reposMu before the configured-binding mutex, while reload/add/delete paths use the reverse order. Concurrent repository sync and configuration changes can deadlock the daemon. Establish a single global lock order—such as acquiring the binding lease before reposMu—while keeping the generation check and binding write atomic.

Medium

  • internal/server/settings_handlers.go:955 — The asynchronous sync inherits a binding-lease context through context.WithoutCancel(ctx). After the handler releases the mutex, the retained lease marker can cause later binding operations to skip locking, enabling concurrent writes or rollback restoration to lose bindings. Capture a lease-free background context before acquiring the lock, or release the lease before starting the sync.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 13m40s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (cd83ce3)

Verdict: Three medium-severity issues require attention before merge.

Medium

  • Stale repositories remain usable after visibility changes
    Locations: frontend/src/App.svelte:813, frontend/src/lib/components/RepoTypeahead.svelte:100
    If the /repos refresh exhausts its retries, the previous catalog and selection remain active. A newly hidden repository can therefore stay selected or appear in mobile options indefinitely. Version catalogs against visibility configuration changes, invalidate or filter stale entries, and clear known-hidden selections while retaining bounded refresh attempts.

  • Concurrent sync can spuriously reject valid repository resolution
    Location: internal/github/configured_repo_bindings.go:178
    Provider observations are timestamped before the network lookup and rejected if a concurrent regular sync records a newer observation—even when both resolve to the same stable repository identity. Retry resolution or rebuild the reference from the current catalog entry when identities match, without publishing stale route metadata.

  • Provider I/O holds the global repository-binding lock indefinitely
    Location: internal/server/config_reload.go:404
    Configuration reload holds configuredRepoBindingsMu while performing potentially unbounded provider requests. A stalled or malicious forge can consequently block settings, repository listing, and exact-repository syncs across unrelated providers. Perform provider I/O outside the lock, then briefly reacquire it to verify the binding/config generation and persist atomically; also use a bounded operation context.


Reviewers: 2 done | Synthesis: codex, 14s | Total: 20m29s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (809bee9)

Repository visibility changes have three medium-severity correctness and availability issues involving route reuse and binding-lock scope.

Medium

  • internal/server/settings_handlers.go:307removeConfigRepos treats a mutable route match as sufficient even when an exact entry has a stable binding. If the original repository is renamed, its old route is reused, and an overlapping glob is deleted, the route successor remains tracked despite no remaining config entry targeting its identity.

    • Fix: Make stable binding matches authoritative for exact entries; use route matching only when no binding exists. Add deletion coverage for an exact-bound repository plus a glob and route successor.
  • frontend/src/lib/utils/repo-visibility.ts:125 — Client-side filtering applies hidden exact entries by owner/name rather than stable identity. After a hidden repository is renamed and a successor reuses its route, the server returns the successor as visible, but desktop/mobile selectors remove it and may clear its persisted selection.

    • Fix: Trust a successfully loaded server catalog for exact-entry visibility, or expose enough binding identity to safely match exact entries. Add browser e2e coverage for route reuse.
  • internal/server/config_reload.go:429 — The configured-binding lease remains held while SetReposWithContext performs archive lifecycle preparation. For an identityless fallback, this triggers another provider lookup without the 20-second resolution deadline, potentially blocking settings/catalog binding readers and mutations indefinitely during an outage.

    • Fix: Complete archive/provider reconciliation before acquiring the binding lease, or separate preparation from the short binding/publication transaction while retaining a bounded provider context.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 17m48s

@mariusvniekerk
mariusvniekerk force-pushed the feat/repository-ui-visibility branch from 809bee9 to e707f9f Compare August 10, 2026 14:13
@wesm

wesm commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

clanking

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (e707f9f)

The change needs fixes for stable repository visibility and reload lock contention.

High

  • frontend/src/lib/utils/repo-visibility.ts:58 — Exact hidden entries are matched using mutable owner/name routes. If a hidden repository is renamed and its former route is reused, the server marks the successor visible, but filterInteractiveRepoCatalog removes it and removeKnownHiddenRepoSelections clears its selection, preventing it from appearing in selectors and workspace creation.
    • Fix: Treat successful /repos responses as authoritative. Avoid route-based filtering for stable-bound exact entries, preserve selections found in the authoritative catalog, and add browser coverage for rename plus route reuse.

Medium

  • internal/server/config_reload.go:429 — The configured-repository binding lease remains held while SetReposWithContext performs archive preparation. For an identityless fallback, EnsureConfigured can retry GetRepository without the earlier 20-second resolution timeout, blocking binding readers and writers—including settings and /repos requests—during a provider outage.
    • Fix: Complete archive preparation and provider I/O before acquiring the binding lease. Hold the lease only while persisting bindings, publishing the tracked set, pruning, and rolling back.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 16m30s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (ad3f0b0)

Overall verdict: Two medium-severity issues require fixes; no critical, high-severity, or security findings were identified.

Medium

  • Rejected config reload can leave archive state incorrectly pausedinternal/server/config_reload.go:433

    Archive preparation durably pauses repositories absent from the candidate config. If binding locking, snapshotting, persistence, or pruning later fails, the reload is rejected and previous repositories are restored only in memory, leaving their archive state paused as configuration_removed.

    Recommended fix: Reconcile the archive lifecycle back to the previous configuration on every post-preparation failure, or make archive preparation part of a compensating transaction. Add a regression test confirming that a rejected reload preserves the prior archive state.

  • Repository menu actions can become inaccessible in clipped settings viewsfrontend/src/lib/components/settings/RepoConfigMenu.svelte:102

    The menu is always positioned below its trigger using local absolute positioning. For lower repository rows inside the clipped, scrolling settings surface, the menu can extend beyond the visible area and hide visibility actions.

    Recommended fix: Use the shared floating-overlay positioning primitives with collision handling, fixed positioning or a portal, and treat both the trigger and panel as dismissal owners.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 14m44s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (5bad833)

High-severity correctness issues remain in repository reload and import flows, with two medium-severity identity reconciliation defects.

High

  • internal/server/config_reload.go:430 — Reload refs are copied and archive-prepared before persistence canonicalizes stale same-identity observations. A delayed lookup can republish an obsolete route, while archive preparation may timestamp it as newer, potentially displacing a route successor and stranding the bound repository.
    • Fix: Canonicalize provider observations using their original timestamps before archive preparation and publication, rebuild the resolved set from corrected batches, and add a full-stack regression test racing reload resolution against a newer rename observation.

Medium

  • internal/github/repo_config_resolver.go:427ExpandedRepoSet.Add transfers exact-entry provenance when replacing a route fallback without checking for conflicting stable IDs. If an overlapping glob returns a route successor before the renamed original, the successor can inherit the original’s binding and never sync correctly.

    • Fix: Transfer provenance only when stable identities match, preserve conflicting route entries until identity-based reconciliation, and test a transient exact failure with the successor listed before the renamed repository.
  • internal/server/repo_import_handlers.go:613 — Bulk import persists each ref through a temporary one-element slice, discarding authoritative route corrections from PersistProviderResolvedRepos. It then publishes stale addRefs, so an imported renamed repository can disappear from /repos until another successful sync.

    • Fix: Persist slices backed by addRefs, or explicitly return and publish the corrected refs.

Reviewers: 2 done | Synthesis: codex, 14s | Total: 28m5s

@wesm

wesm commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

This one spiralled out of control and is going to require a redo

wesm and others added 5 commits August 10, 2026 14:48
TestNotificationSyncReconcilesReusedRouteE2E pinned its seeded activity
to 2026-08-03; the activity endpoint's default window is 7d, so the
assertion silently became date-dependent and began failing on 2026-08-10
on every branch. Seed a now-relative instant like the neighboring tests
so the fixture stays inside the default window on any calendar day.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hiding a repository from interactive catalogs is an operator presentation
preference, so it lives in SQLite keyed by the internal catalog row id
rather than in operator config. Config entries are mutable routes: keying
visibility by them forced a parallel route-to-identity binding reconciler
(the abandoned PR #862 approach) that raced catalog reconciliation.
Keyed by the stable row, renames keep the preference through ordinary
catalog reconciliation, route reuse cannot inherit it because reuse
creates a distinct row, and deleting the row cascades the preference
away.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PUT /repo/{provider}/{owner}/{name}/ui-visibility (plus the host-prefixed
form) toggles the hidden-from-UI preference for an exact configured
repository. The entry must resolve to a provider-verified catalog row —
the preference attaches to stable identity, so an unverified repository
conflicts until sync verifies it, and glob entries are rejected because
visibility belongs to exact repositories.

Filtering happens once, server-side, in /repos and /repos/summaries so
every selector and picker consistently omits hidden repositories without
client-side matching. Item feeds, provider-aware direct routes, and the
settings surface stay unfiltered; settings responses carry an explicit
hidden_from_ui flag per configured entry so the management surface can
offer the restore action.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each exact repository row in Settings gets a compact gear menu holding
the existing local-clone editor entry and a one-click Hide from UI /
Show in UI action, replacing the previous standalone clone icon without
adding permanent row chrome. The action saves through the ordered
settings workflow, disables at the menu item while pending, gates at the
items in embedded mode so the menu stays inspectable, and reports save
failures through the flash banner while keeping server-confirmed state.

Glob rows get no menu: visibility belongs to exact repositories.
Selector consumers gate on the server's hidden_from_ui flag instead of
reimplementing matching — the typeahead and mobile repo options skip
flagged entries, and global-filter normalization drops a hidden repo
from the current selection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hiding is a Settings-row gear-menu action scoped to exact repositories;
sync and direct links are unaffected, so the note lives with the
repository configuration guidance rather than the workflow tours.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@wesm
wesm force-pushed the feat/repository-ui-visibility branch from 5bad833 to 9430403 Compare August 10, 2026 20:33
@wesm wesm changed the title feat: hide configured repositories from interactive UI feat: hide repositories from the UI via a per-repo gear menu Aug 10, 2026
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (9430403)

Medium-severity issues remain in repository visibility identity handling and mobile filter normalization.

Medium

  • internal/server/settings_handlers.go:151, internal/server/settings_handlers.go:1150 — Visibility state is correlated and mutated through mutable route keys. If an inactive hidden repository shares its former route with a replacement, the replacement can appear hidden or receive the visibility mutation despite having a different provider ID. Correlate repositories and resolve mutations by (provider, host, platform_repo_id) when a verified ID exists, falling back to routes only when no stable ID is available. Add a route-reuse regression test.

  • frontend/src/lib/utils/repo-filter-values.ts:139 — Removing hidden repositories from normalization identities does not clear a selected hidden repository because normalizeRepoFilterValue preserves unknown provider-qualified values. On mobile, the hidden repository can remain selected and continue filtering requests after disappearing from the picker. Explicitly remove selections matching hidden exact repositories before general normalization while preserving unmatched selections needed for glob-resolved repositories, and test the selected-hidden case.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 5m17s

wesm and others added 3 commits August 10, 2026 17:49
The standalone per-row local clone button was replaced by the repository
gear menu, so the real-backend settings specs now open the editor via
the menu and verify the saved path by reopening the editor instead of
asserting the removed button's tooltip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hidden-state correlation on the settings surface and visibility
mutation resolution both keyed on route strings. A repository displaced
by route reuse keeps its old display route, so a replacement repository
at that route inherited the displaced row's hidden flag, and a mutation
resolved by route could land on whichever repository currently occupies
it. Correlation now uses the stable (provider, host, provider id)
identity key, and mutations resolve through the tracked ref's provider
id, falling back to the route only when no verified identity exists.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
General selection normalization preserves unknown provider-qualified
values because glob-resolved repositories have no configured entry of
their own. A selected repository that was then hidden matched no
interactive identity but survived normalization the same way, so it
kept filtering activity after disappearing from every picker. Selection
normalization now drops selections naming a hidden exact repository
before the general pass, while unmatched glob-resolved selections stay.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (5fe1e6c)

One medium-severity issue needs addressing; no security vulnerabilities were identified.

Medium

  • Hidden renamed repositories can remain selectedfrontend/src/lib/utils/repo-filter-values.ts:164

    Hidden selections are matched against the configured repo_path, but a renamed repository’s active filter value uses its current provider route. Because unknown qualified values are preserved, hiding a renamed repository leaves the global filter active, so feeds remain scoped to the hidden repository.

    Suggested fix: Include the repository’s current catalog route or stable identity in settings data and remove selections using that resolved identity. Add coverage for cases where the configured path differs from the current route.


Reviewers: 2 done | Synthesis: codex, 17s | Total: 7m50s

Selection cleanup matched hidden repositories by their configured
repo_path, but selections created from catalog rows use the
provider-verified current route, which diverges after a provider-side
rename — hiding a renamed repository left the global filter scoped to
it. Settings entries now expose the tracked repository's current route
as tracked_repo_path, and selection normalization clears hidden
selections against both the configured and current routes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (217402a)

The change is generally clean, but one medium-severity test coverage gap remains.

Medium

  • internal/server/huma_routes.go:687 — Hidden-repository filtering for /repos/summary, which drives the repository overview, lacks end-to-end coverage; current visibility tests only verify /repos.
    • Fix: Extend the API/SQLite e2e test to verify that hiding removes the repository from /api/v1/repos/summary and showing restores it.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 6m13s

The summaries endpoint drives the repository overview but its hidden
filtering was only covered indirectly; the wire-level e2e test now
verifies hiding removes the repository from /api/v1/repos/summary and
showing restores it. Verified the assertion catches a regression by
disabling the summaries filter and watching the test fail.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (40b11db)

Medium-severity issues remain in repository visibility handling and test coverage.

Medium

  • internal/server/settings_handlers.go:1213GetRepositoryByProviderID can return an inactive, displaced catalog row. If the tracked-repository snapshot lags database reconciliation during route reuse, a visibility request may update the old repository while the active replacement remains visible. Resolve visibility against a consistent tracked/catalog snapshot, or reject inactive entries and require retry after synchronization. Add coverage for route replacement while the syncer still references the prior provider ID.

  • internal/server/huma_routes.go:667, frontend/src/lib/components/terminal/NewWorkspaceDialog.svelte:115 — Hidden repositories disappear from /repos, but the workspace dialog silently falls back to the last-used or first repository when an explicit seed is unavailable. Opening “New workspace” from an existing workspace or direct route for a hidden repository may therefore target a different repository. Leave the selection empty and require user choice, or resolve the seed through a direct unfiltered path without exposing it in the normal picker.

  • frontend/src/lib/components/settings/RepoSettings.svelte:203 — The new hide/show workflow lacks browser end-to-end coverage using the real API and SQLite database. Add Playwright coverage that hides a repository through Settings, verifies its removal from interactive catalogs and active filter state, reloads, and then restores it.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m43s

wesm and others added 3 commits August 10, 2026 20:57
A tracked-repository snapshot that lags catalog reconciliation still
names the prior provider id after route reuse. Resolving that id
returned the displaced, inactive row, so a hide request silently
updated the old repository while the active replacement stayed visible.
Visibility mutations now require the resolved row to be
lifecycle-active and answer conflict-retry-after-sync otherwise, the
same as unverified entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The new-workspace dialog fell back to the last-used or first repository
when its explicit seed was missing from the loaded list — for example a
repository hidden from the UI — so creating a workspace from that
context silently targeted a different repository. An unresolvable seed
now leaves the selection empty and submission disabled until the user
picks a repository; the last-used and first-repo defaults still apply
when no seed was given.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The settings gear-menu hide/show cycle now has browser coverage against
the real API and SQLite database: hiding drops the repository from the
repos overview and releases a persisted global filter scoped to it,
the preference survives a reload, and Show in UI restores the card.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (58688bd)

Medium-severity repository visibility issues remain; no exploitable security weakness was identified.

Medium

  • internal/server/settings_handlers.go:1196 — Repository resolution and the visibility write occur in separate critical sections. Reconciliation can displace the repository after lifecycle validation but before SetRepoHiddenFromUI, attaching the preference to an inactive row while its replacement remains visible. Resolve, validate, and write visibility atomically while holding the repository-reconciliation read lock, and add a deterministic concurrent-reconciliation test.

  • internal/server/settings_handlers.go:1293 — Removing a hidden exact configuration leaves its SQLite preference behind. If a glob still matches the repository, it remains filtered from catalogs, while Settings exposes only the non-editable glob row—leaving no direct “Show in UI” path. Clear the preference when the last exact configuration for that stable repository is removed, or retain a management surface for hidden glob matches; add end-to-end coverage for the exact-plus-glob removal workflow.


Reviewers: 2 done | Synthesis: codex, 14s | Total: 12m40s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants