crueber/walhub

Watch

Fork 0

Mirror UX: no mirror indicator on explore/owner listings, push guidance shown for mirrors awaiting first sync, import page can't create mirrors #281

New issue

Closed

opened 2026-09-10 12:09:35 +00:00 by crueber · 3 comments

crueber commented

2026-09-10 12:09:35 +00:00

Owner

Follow-ups to the mirror feature (Forgejo #240, now implemented): three gaps between what the mirror system does and what the UI communicates.

1. Explore/owner listings show no mirror indicator

Mirror repos are indistinguishable from normal repos anywhere in listing surfaces. /explore and /:owner render repo rows via <RepoRow> (web/src/pages/Repos.jsx:38-70 — link + <StarCount> + <ActivityStamp>), which has no mirror awareness; the mirror badge exists only on the repo page itself (Repo.jsx:541, Forgejo #240 badge + next-sync).

The data is already available per row-adjacent: the repo summary carries mirror: {upstream_url, schedule, next_sync_at, last_synced_at, last_result} (verified live: GET /anon/walhub/api → full mirror object). But GET /api/v1/owners/{owner}/repos returns names/shapes without mirror flags (see #247/#248's catalog work for the listing-shape precedent).

Ask: a clear mirror indicator on listing rows — a mirror emoji or the established icon set's mirror glyph (note: a 4-byte emoji in the issue body breaks Forgejo's DB charset — use a 2-byte one like e.g. #-prefixed mirror glyph or an inline SVG icon in the UI itself or an inline SVG icon in the UI itself) next to the repo name, with title/aria-label "mirror of " for hover/screen-reader. On /explore and /:owner; also the search/user profile rows if shared.

Implementation note: either extend the owner-repos listing payload with a mirror: bool (rides the #247 catalog wire change — same file, same pattern), or have <RepoRow> fetch per-row (anti-pattern — the listing already exists; don't add N summary GETs). Prefer the payload extension.

2. Mirror repos awaiting first sync show push guidance

While a mirror's first clone is running (or has not yet completed), an empty mirror repo renders EmptyRepoGuide (web/src/pages/Tree.jsx:237) — which tells the user to push: "git push -u origin main … the first push lands directly" (web/src/components/EmptyRepoGuide.jsx:134-139). For a pull-only mirror this is doubly wrong: the push will be rejected (pull-only), and the correct action is wait for the scheduled sync / trigger a sync now.

EmptyRepoGuide already receives summary (Tree.jsx:237), and the summary carries the mirror projection (including last_synced_at/last_result — enough to distinguish "never synced, first clone pending" from a genuinely empty normal repo).

Ask: when summary.mirror is present, render a mirror-specific waiting state instead of the push guide: "This repository is a read-only mirror of <upstream_url>. First sync in progress — next scheduled sync <next_sync_at>." Plus a "Sync now" affordance if the manual-sync endpoint exists for non-admins (otherwise omit). Push guidance must never render for a mirror.

3. Import page can't create mirrors

Mirror creation exists only on the New-repo page (web/src/pages/New.jsx:27-56 — mirror-from-URL mode → POST /api/v1/repos/mirrors). The import page (web/src/pages/Import.jsx) has zero mirror mentions — it drives the one-shot import flow (POST /api/v1/repos/imports) exclusively.

Ask: add a mirror option to /import — either a mode toggle (Import once / Mirror continuously) or a checkbox that switches the submit target from repos.imports.start to POST /api/v1/repos/mirrors with a schedule preset. The form fields overlap almost entirely (source URL, owner, name, token), lib/mirror.js already exports MIRROR_PRESETS/validateMirrorCreate for reuse, and the backend endpoint exists (internal/mirror/http.go:91-93). This is UI wiring, not backend work.

Copy note: whichever presentation, keep the semantic distinction honest — import = one-shot snapshot, mirror = recurring pull with push rejected.

Acceptance criteria

Mirror repos show a mirror indicator (emoji/icon) on /explore and /:owner rows, with accessible labeling naming the upstream.
Listing payload carries the mirror flag (no per-row summary GETs added).
An empty/awaiting-first-sync mirror repo never shows push guidance; it shows a mirror waiting state with upstream + next sync time.
A genuinely empty non-mirror repo still shows the existing push guide (no regression).
/import can create a mirror (mode toggle or equivalent) using the shared mirror validation/presets; the created mirror appears with its indicator per criterion 1.
Light/dark themes; headless test for the row indicator logic (mirror flag → badge presence/label).

Follow-ups to the mirror feature (Forgejo #240, now implemented): three gaps between what the mirror system does and what the UI communicates. ## 1. Explore/owner listings show no mirror indicator Mirror repos are indistinguishable from normal repos anywhere in listing surfaces. `/explore` and `/:owner` render repo rows via `<RepoRow>` (`web/src/pages/Repos.jsx:38-70` — link + `<StarCount>` + `<ActivityStamp>`), which has no mirror awareness; the mirror badge exists only on the repo page itself (`Repo.jsx:541`, Forgejo #240 badge + next-sync). The data is already available per row-adjacent: the repo summary carries `mirror: {upstream_url, schedule, next_sync_at, last_synced_at, last_result}` (verified live: `GET /anon/walhub/api` → full mirror object). But `GET /api/v1/owners/{owner}/repos` returns names/shapes without mirror flags (see #247/#248's catalog work for the listing-shape precedent). **Ask:** a clear mirror indicator on listing rows — a mirror emoji or the established icon set's mirror glyph (note: a 4-byte emoji in the issue body breaks Forgejo's DB charset — use a 2-byte one like e.g. `#`-prefixed mirror glyph or an inline SVG icon in the UI itself or an inline SVG icon in the UI itself) next to the repo name, with `title`/`aria-label` "mirror of <upstream>" for hover/screen-reader. On `/explore` and `/:owner`; also the search/user profile rows if shared. **Implementation note:** either extend the owner-repos listing payload with a `mirror: bool` (rides the #247 catalog wire change — same file, same pattern), or have `<RepoRow>` fetch per-row (anti-pattern — the listing already exists; don't add N summary GETs). Prefer the payload extension. ## 2. Mirror repos awaiting first sync show push guidance While a mirror's first clone is running (or has not yet completed), an empty mirror repo renders `EmptyRepoGuide` (`web/src/pages/Tree.jsx:237`) — which tells the user to **push**: "`git push -u origin main` … the first push lands directly" (`web/src/components/EmptyRepoGuide.jsx:134-139`). For a pull-only mirror this is doubly wrong: the push will be *rejected* (pull-only), and the correct action is *wait for the scheduled sync / trigger a sync now*. `EmptyRepoGuide` already receives `summary` (`Tree.jsx:237`), and the summary carries the `mirror` projection (including `last_synced_at`/`last_result` — enough to distinguish "never synced, first clone pending" from a genuinely empty normal repo). **Ask:** when `summary.mirror` is present, render a mirror-specific waiting state instead of the push guide: "This repository is a read-only mirror of <upstream_url>. First sync in progress — next scheduled sync <next_sync_at>." Plus a "Sync now" affordance if the manual-sync endpoint exists for non-admins (otherwise omit). Push guidance must never render for a mirror. ## 3. Import page can't create mirrors Mirror creation exists only on the New-repo page (`web/src/pages/New.jsx:27-56` — mirror-from-URL mode → `POST /api/v1/repos/mirrors`). The import page (`web/src/pages/Import.jsx`) has zero mirror mentions — it drives the one-shot import flow (`POST /api/v1/repos/imports`) exclusively. **Ask:** add a mirror option to `/import` — either a mode toggle (Import once / Mirror continuously) or a checkbox that switches the submit target from `repos.imports.start` to `POST /api/v1/repos/mirrors` with a schedule preset. The form fields overlap almost entirely (source URL, owner, name, token), `lib/mirror.js` already exports `MIRROR_PRESETS`/`validateMirrorCreate` for reuse, and the backend endpoint exists (`internal/mirror/http.go:91-93`). This is UI wiring, not backend work. **Copy note:** whichever presentation, keep the semantic distinction honest — import = one-shot snapshot, mirror = recurring pull with push rejected. ## Acceptance criteria - [ ] Mirror repos show a mirror indicator (emoji/icon) on `/explore` and `/:owner` rows, with accessible labeling naming the upstream. - [ ] Listing payload carries the mirror flag (no per-row summary GETs added). - [ ] An empty/awaiting-first-sync mirror repo never shows push guidance; it shows a mirror waiting state with upstream + next sync time. - [ ] A genuinely empty non-mirror repo still shows the existing push guide (no regression). - [ ] `/import` can create a mirror (mode toggle or equivalent) using the shared mirror validation/presets; the created mirror appears with its indicator per criterion 1. - [ ] Light/dark themes; headless test for the row indicator logic (mirror flag → badge presence/label).

crueber referenced this issue from a commit

2026-09-10 18:12:08 +00:00

Fix #281: mirror UX gaps (listing badge, waiting state, import creates mirrors)

crueber referenced this issue from a pull request that will close it,

2026-09-10 18:12:10 +00:00

Fix #281: mirror UX gaps #298

crueber commented

2026-09-10 18:12:13 +00:00

Author

Owner

Fix ready for review: PR #298 (fix/issue-281) — listing mirror badge (payload flag, no N+1), mirror waiting state instead of push guidance, /import mirror mode via the #240 create endpoint. All acceptance criteria covered; browser proof open per workspace rules.

crueber commented

2026-09-10 18:27:50 +00:00

Author

Owner

Review of PR #298 (fix/issue-281, reviewed at 4d50484; fixup pushed as 78ff2b1). No browser per review rules — tests + reasoning only.

What I verified (scratch worktree /tmp/opencode/walhub-281; main untouched):

Law 8: internal/api/repos_detailed.go imports only sizecatalog + store — no core->mirror import; upstream parsed inline. OK.
Fail-closed: probeMirrorRow (repos_detailed.go:153) returns (true, '') on corrupt sidecar — exact IsMirror parity (internal/mirror/mirror.go:179). OK.
Degrade: any store error / absent body -> (false, '') — listing never 500s. OK, and documented as intended.
Law 4/6 cost: 1 catalog GET + N sidecar GETs per detailed load, <=8 in flight, no sequential depth (fillMirrorFlags, repos_detailed.go:182). Law-6 hot paths untouched. EmptyRepoGuide branches on the in-hand shell summary (zero fetches); badge rides the listing payload (zero client fetches). Import mirror mode reuses POST /api/v1/repos/mirrors (#240 endpoint, no new endpoint).
Import mirror mode: MIRROR_PRESETS == server Presets exactly (hourly/8h/daily/weekly/monthly, default daily); token only sent when provided (memory-only first-sync, public-only scheduled syncs preserved); 202 {task,target} -> landedVisible + navigate to /target where the waiting state is the progress surface. Anonymous gating matches server write requirement. OK.
Waiting state: no push guidance anywhere in MirrorEmptyGuide; upstream + next-sync + failed-attempt note + clone-only. All four call sites (Tree/Blob/Commit/Commits) pass the shell summary. Hooks run before the early return. OK.
No new deps (no go.mod/package.json diff). Dark+light via .pill + standard dark: variants throughout. OK.

Small fixes pushed to origin/fix/issue-281 (78ff2b1):

docs/features/11_mirror.md:224 — 'adding no fetches' was inaccurate (server DOES add sidecar probes); now 'no client fetches', pointing at the 07_api.md decision (law 12).
web/src/components/EmptyRepoGuide.jsx:142 — never-synced copy echoed formatNextSync ('First sync in progress — never synced — first sync pending'); now direct pending phrasing.
web/src/pages/Repos.jsx:60 — badge span had aria-label without a role (exposed to no AT); added role=img so the upstream-naming label the docs claim is real.

Non-blocking observations (possible follow-ups, not merge gates):

Server probes ALL repos under an owner per detailed load; client display caps (MAX_OWNERS=50, MAX_REPOS_PER_OWNER=10, lib/owners.js) don't bound it. Explore worst case ~= 50 owners x N sidecar GETs. Strictly better than the rejected per-row-summary alternative; fine at expected scale. A catalog-carried mirror bit or server-side n= cap could bound it later.
fillMirrorFlags spawns one goroutine per row and bounds only in-flight probes via the sem; acquiring the sem in the parent loop would also bound goroutine count.

Test results (all in scratch worktree):

go test -race ./internal/api/... : ok; coverage 95.3% package (gate holds); new code probeMirrorRow 100%, fillMirrorFlags 94.4%.
node --test web/test/unit (excl. smoke file): 568/568 pass, exit 0. smoke.test.js excluded: it needs a live server running PR code (none in sandbox; file untouched by PR). Note: scratch worktree needed a web/node_modules symlink -> main worktree's (read-only reuse, no install) for marked/dompurify imports.
gofmt clean, go vet clean, vite build green (verifies the JSX changes compile — unit tests don't import JSX).

MERGE RECOMMENDATION: ready to merge.

Review of PR #298 (fix/issue-281, reviewed at 4d50484; fixup pushed as 78ff2b1). No browser per review rules — tests + reasoning only. What I verified (scratch worktree /tmp/opencode/walhub-281; main untouched): - Law 8: internal/api/repos_detailed.go imports only sizecatalog + store — no core->mirror import; upstream parsed inline. OK. - Fail-closed: probeMirrorRow (repos_detailed.go:153) returns (true, '') on corrupt sidecar — exact IsMirror parity (internal/mirror/mirror.go:179). OK. - Degrade: any store error / absent body -> (false, '') — listing never 500s. OK, and documented as intended. - Law 4/6 cost: 1 catalog GET + N sidecar GETs per detailed load, <=8 in flight, no sequential depth (fillMirrorFlags, repos_detailed.go:182). Law-6 hot paths untouched. EmptyRepoGuide branches on the in-hand shell summary (zero fetches); badge rides the listing payload (zero client fetches). Import mirror mode reuses POST /api/v1/repos/mirrors (#240 endpoint, no new endpoint). - Import mirror mode: MIRROR_PRESETS == server Presets exactly (hourly/8h/daily/weekly/monthly, default daily); token only sent when provided (memory-only first-sync, public-only scheduled syncs preserved); 202 {task,target} -> landedVisible + navigate to /target where the waiting state is the progress surface. Anonymous gating matches server write requirement. OK. - Waiting state: no push guidance anywhere in MirrorEmptyGuide; upstream + next-sync + failed-attempt note + clone-only. All four call sites (Tree/Blob/Commit/Commits) pass the shell summary. Hooks run before the early return. OK. - No new deps (no go.mod/package.json diff). Dark+light via .pill + standard dark: variants throughout. OK. Small fixes pushed to origin/fix/issue-281 (78ff2b1): 1. docs/features/11_mirror.md:224 — 'adding no fetches' was inaccurate (server DOES add sidecar probes); now 'no client fetches', pointing at the 07_api.md decision (law 12). 2. web/src/components/EmptyRepoGuide.jsx:142 — never-synced copy echoed formatNextSync ('First sync in progress — never synced — first sync pending'); now direct pending phrasing. 3. web/src/pages/Repos.jsx:60 — badge span had aria-label without a role (exposed to no AT); added role=img so the upstream-naming label the docs claim is real. Non-blocking observations (possible follow-ups, not merge gates): - Server probes ALL repos under an owner per detailed load; client display caps (MAX_OWNERS=50, MAX_REPOS_PER_OWNER=10, lib/owners.js) don't bound it. Explore worst case ~= 50 owners x N sidecar GETs. Strictly better than the rejected per-row-summary alternative; fine at expected scale. A catalog-carried mirror bit or server-side n= cap could bound it later. - fillMirrorFlags spawns one goroutine per row and bounds only in-flight probes via the sem; acquiring the sem in the parent loop would also bound goroutine count. Test results (all in scratch worktree): - go test -race ./internal/api/... : ok; coverage 95.3% package (gate holds); new code probeMirrorRow 100%, fillMirrorFlags 94.4%. - node --test web/test/unit (excl. smoke file): 568/568 pass, exit 0. smoke.test.js excluded: it needs a live server running PR code (none in sandbox; file untouched by PR). Note: scratch worktree needed a web/node_modules symlink -> main worktree's (read-only reuse, no install) for marked/dompurify imports. - gofmt clean, go vet clean, vite build green (verifies the JSX changes compile — unit tests don't import JSX). MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-10 18:28:19 +00:00

crueber commented

2026-09-10 18:28:20 +00:00

Author

Owner

Fixed by PR #298 incl. review doc/a11y/copy fixups (listing cost acceptable, import mode verified; gates green), merged. Closing.

crueber referenced this issue from a commit

2026-09-10 18:28:20 +00:00

Merge pull request 'Fix #281: mirror UX gaps' (#298) from fix/issue-281 into main

crueber added the

git storage