crueber/walhub

Watch

Fork 0

Per-repo General-settings toggles: disable Issues/Pulls/Releases tabs, forks, watching, starring #522

New issue

Closed

opened 2026-09-14 15:47:13 +00:00 by crueber · 3 comments

crueber commented

2026-09-14 15:47:13 +00:00

Owner

Per-repo General-settings toggles: disable Issues/Pulls/Releases tabs, forks, watching, starring

What's requested

Six per-repo toggles on the repo General settings tab (like Forgejo's repository feature checkboxes), each backed end-to-end — settings model, summary projection, tab gating, and API-level guards:

Disable Issues tab — hides the Issues tab (and issue creation surfaces).
Disable Pull requests tab — hides the Pulls tab (and PR composer/fork-head flows that target this repo as base).
Disable Releases tab — hides the Releases tab.
Disable forks — the Fork affordance is gone and POST /api/v1/repos/{owner}/{repo}/forks refuses at the API level.
Prevent watchers — no new watches; existing watchers keep working (notification fan-out unaffected).
Prevent starring — no new stars; existing stars are retained and still counted.

Evidence / current shape (static read of `d5c706f`)

Settings model: per-repo settings are the WAL-published TOML doc (internal/api/settings.go, GET AuthRead / PUT AuthAdmin, <=16 KiB). RepoSettings in internal/config/settings.go:29 carries Description + pointer sections; ParseRepoSettings rejects unknown keys, Merge overrides host config, and Description is the precedent for display-metadata fields that ride the same doc but never merge. A new [repo]-style section (e.g. features booleans, default all-on) follows the Description pattern: validated, persisted, revisioned, merge-ignored, extraction tolerant on read paths (cf. DescriptionOf).
Summary projection: summaryBody (internal/api/summary.go) already carries feature flags with the #319 badge discipline (always present, never null): OpenIssues/OpenPulls, HasChecks (#505), ForkParent/Forks (#424). The six toggles project onto this shape (e.g. features: {issues: true, pulls: true, releases: true, forks: true, watch: true, star: true} or six booleans) so a single shared summary fetch gates everything client-side.
Tab gating precedent: showChecksTab/checksHidden in web/src/pages/Repo.jsx (~661, ~851) already hides the Checks tab from the same shared summary — the new toggles are a second instance of the identical mechanism (filter the TABS list at ~167 by summary flags, keep Code highlighted for paths of hidden tabs, deep links to disabled surfaces must land on a sane page, not a dead 404 route).
API write surfaces to guard:
- star: PUT /{o}/{r}/api/star (internal/social/http.go star, ~247)
- watch: PUT /{o}/{r}/api/watch (internal/notify/http.go watch, ~437; SetWatch in internal/notify)
- fork: POST /api/v1/repos/{owner}/{repo}/forks (internal/pulls/http.go route ~278, fork ~757)
Pills: the header pills were unified in #447 (counts left of label); the star/watch/fork pills need a disabled/hidden affordance consistent with that idiom.

Architecture notes

Tab-disable flags vs. write-guards are different failure modes: a disabled tab must still serve reads (existing issues/pulls/releases remain browsable via direct URL unless the tab is hidden — decide whether "disable tab" is display-only like Forgejo's, which it is: content stays reachable, the affordance and creation paths go away; say which in the implementation).
Un-star and un-watch must ALWAYS work, even when the toggle prevents new ones (matches the existing rule that unstar always works, internal/social/http.go ~246 comment).
Guards belong at the service/API write boundary (reject with 403/409 + a clear reason), not only in the UI — the UI pill is an affordance, the API check is the rule. Admin/owner bypass is a planner decision; if admins keep a UI path, the API still rejects non-admins.
Existing stars/watchers are never deleted by toggling; counts stay. Re-enabling restores affordances with counts intact.
Cache: the summary is ETag'd on the head SHA and SWR-classed — a settings-only change (no new commits) must still invalidate/revalidate the flag fields or clients serve stale tab state. Follow the #513/mutability cache-class ruling: the toggles are mutable state, so the flag projection needs the correct cache class/ETag coverage. Client-side, save must invalidate repo:{full} and the summary entry like the General tab's description save already does.
The three-twin route rule and the settings PUT admin gate already exist; no new route shapes are needed beyond what the summary already exposes.

Acceptance criteria

[repo] features-style TOML section (exact shape: planner's call) accepted by ParseRepoSettings; unknown keys still rejected; defaults = all features enabled (zero migration for existing repos).
General settings tab renders six toggles with the existing General-tab save path (CAS revision, error reseed, settings:* invalidations); non-admin saves surface the 403 like visibility does.
Summary projects the flags; flag fields covered by the response's ETag/cache contract so a settings-only change reflects without a new commit.
Issues / Pulls / Releases tabs hidden when toggled off, in the showChecksTab pattern; direct URLs to disabled surfaces do not 404 into a broken route; Code stays highlighted for their paths.
POST …/forks on a forks-disabled repo fails at the API level with a clear status + reason; the Fork pill/CTA is hidden or disabled with a tooltip.
PUT …/api/star and PUT …/api/watch on prevented repos fail at the API level with a clear status + reason; DELETE (un-star/un-watch) always succeeds; existing counts unaffected.
Star/Watch/Fork pills follow the #447 idiom (counts left of label) with a disabled affordance that explains why on hover/focus.
Re-enabling a feature restores tab, pill, and write affordances with counts intact.
Tests: settings parse/merge, summary projection, each API guard (star/watch/fork), and the tab-gating logic unit-covered.

Awaiting implementation via the normal pipeline. Scope note: one settings model, six booleans, one summary projection — do not split into six tickets.

# Per-repo General-settings toggles: disable Issues/Pulls/Releases tabs, forks, watching, starring ## What's requested Six per-repo toggles on the repo **General settings tab** (like Forgejo's repository feature checkboxes), each backed end-to-end — settings model, summary projection, tab gating, and API-level guards: 1. **Disable Issues tab** — hides the Issues tab (and issue creation surfaces). 2. **Disable Pull requests tab** — hides the Pulls tab (and PR composer/fork-head flows that target this repo as base). 3. **Disable Releases tab** — hides the Releases tab. 4. **Disable forks** — the Fork affordance is gone and `POST /api/v1/repos/{owner}/{repo}/forks` refuses at the API level. 5. **Prevent watchers** — no new watches; existing watchers keep working (notification fan-out unaffected). 6. **Prevent starring** — no new stars; existing stars are retained and still counted. ## Evidence / current shape (static read of d5c706f) - **Settings model**: per-repo settings are the WAL-published TOML doc (`internal/api/settings.go`, GET AuthRead / PUT AuthAdmin, <=16 KiB). `RepoSettings` in `internal/config/settings.go:29` carries `Description` + pointer sections; `ParseRepoSettings` rejects unknown keys, `Merge` overrides host config, and `Description` is the precedent for display-metadata fields that ride the same doc but never merge. A new `[repo]`-style section (e.g. `features` booleans, default all-on) follows the Description pattern: validated, persisted, revisioned, merge-ignored, extraction tolerant on read paths (cf. `DescriptionOf`). - **Summary projection**: `summaryBody` (`internal/api/summary.go`) already carries feature flags with the #319 badge discipline (always present, never null): `OpenIssues`/`OpenPulls`, `HasChecks` (#505), `ForkParent`/`Forks` (#424). The six toggles project onto this shape (e.g. `features: {issues: true, pulls: true, releases: true, forks: true, watch: true, star: true}` or six booleans) so a single shared summary fetch gates everything client-side. - **Tab gating precedent**: `showChecksTab`/`checksHidden` in `web/src/pages/Repo.jsx` (~661, ~851) already hides the Checks tab from the same shared summary — the new toggles are a second instance of the identical mechanism (filter the `TABS` list at ~167 by summary flags, keep Code highlighted for paths of hidden tabs, deep links to disabled surfaces must land on a sane page, not a dead 404 route). - **API write surfaces to guard**: - star: `PUT /{o}/{r}/api/star` (`internal/social/http.go` `star`, ~247) - watch: `PUT /{o}/{r}/api/watch` (`internal/notify/http.go` `watch`, ~437; `SetWatch` in internal/notify) - fork: `POST /api/v1/repos/{owner}/{repo}/forks` (`internal/pulls/http.go` route ~278, `fork` ~757) - **Pills**: the header pills were unified in #447 (counts left of label); the star/watch/fork pills need a disabled/hidden affordance consistent with that idiom. ## Architecture notes - Tab-disable flags vs. write-guards are different failure modes: a disabled tab must still serve reads (existing issues/pulls/releases remain browsable via direct URL unless the tab is hidden — decide whether "disable tab" is display-only like Forgejo's, which it is: content stays reachable, the affordance and creation paths go away; say which in the implementation). - Un-star and un-watch must ALWAYS work, even when the toggle prevents new ones (matches the existing rule that unstar always works, `internal/social/http.go` ~246 comment). - Guards belong at the service/API write boundary (reject with 403/409 + a clear reason), not only in the UI — the UI pill is an affordance, the API check is the rule. Admin/owner bypass is a planner decision; if admins keep a UI path, the API still rejects non-admins. - Existing stars/watchers are never deleted by toggling; counts stay. Re-enabling restores affordances with counts intact. - Cache: the summary is ETag'd on the head SHA and SWR-classed — a settings-only change (no new commits) must still invalidate/revalidate the flag fields or clients serve stale tab state. Follow the #513/mutability cache-class ruling: the toggles are mutable state, so the flag projection needs the correct cache class/ETag coverage. Client-side, save must invalidate `repo:{full}` and the summary entry like the General tab's description save already does. - The three-twin route rule and the settings PUT admin gate already exist; no new route shapes are needed beyond what the summary already exposes. ## Acceptance criteria - [ ] `[repo] features`-style TOML section (exact shape: planner's call) accepted by `ParseRepoSettings`; unknown keys still rejected; defaults = all features enabled (zero migration for existing repos). - [ ] General settings tab renders six toggles with the existing General-tab save path (CAS revision, error reseed, `settings:*` invalidations); non-admin saves surface the 403 like visibility does. - [ ] Summary projects the flags; flag fields covered by the response's ETag/cache contract so a settings-only change reflects without a new commit. - [ ] Issues / Pulls / Releases tabs hidden when toggled off, in the `showChecksTab` pattern; direct URLs to disabled surfaces do not 404 into a broken route; Code stays highlighted for their paths. - [ ] `POST …/forks` on a forks-disabled repo fails at the API level with a clear status + reason; the Fork pill/CTA is hidden or disabled with a tooltip. - [ ] `PUT …/api/star` and `PUT …/api/watch` on prevented repos fail at the API level with a clear status + reason; DELETE (un-star/un-watch) always succeeds; existing counts unaffected. - [ ] Star/Watch/Fork pills follow the #447 idiom (counts left of label) with a disabled affordance that explains why on hover/focus. - [ ] Re-enabling a feature restores tab, pill, and write affordances with counts intact. - [ ] Tests: settings parse/merge, summary projection, each API guard (star/watch/fork), and the tab-gating logic unit-covered. *Awaiting implementation via the normal pipeline. Scope note: one settings model, six booleans, one summary projection — do not split into six tickets.*

crueber added the

labels

2026-09-14 15:47:17 +00:00

crueber added this to the v1 milestone

2026-09-14 15:47:17 +00:00

crueber removed the

bug

label

2026-09-14 15:47:30 +00:00

crueber referenced this issue from a commit

2026-09-14 19:18:06 +00:00

Fix #522: per-repo feature toggles (issues/pulls/releases tabs, forks, watch, star)

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

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

Fix #522: per-repo feature toggles #532

crueber commented

2026-09-14 19:18:14 +00:00

Author

Owner

Implemented end-to-end in PR #532 (#532), branch fix/issue-522 — one settings model, six booleans, no split. All 9 acceptance criteria covered (settings section, General-tab toggles, summary projection + ETag, tab gating, fork/star/watch guards, pill affordances, re-enable, tests). Verification: -race green, coverage gate holds, node 1211/1212 (sole fail pre-existing live smoke), vite+esbuild green, e2e green, live HTTP probe green. Not merging — review requested.

Implemented end-to-end in PR #532 (https://git.packden.us/crueber/walhub/pulls/532), branch fix/issue-522 — one settings model, six booleans, no split. All 9 acceptance criteria covered (settings section, General-tab toggles, summary projection + ETag, tab gating, fork/star/watch guards, pill affordances, re-enable, tests). Verification: -race green, coverage gate holds, node 1211/1212 (sole fail pre-existing live smoke), vite+esbuild green, e2e green, live HTTP probe green. Not merging — review requested.

crueber commented

2026-09-14 19:41:35 +00:00

Author

Owner

REVIEW PR #532 (fix/issue-522, d592b73) — adversarial pass, verified in scratch worktree (removed afterward). No browser (tests + reasoning; no browser-facing layout risk beyond gated Shows).

(1) SETTINGS MODEL — PASS. internal/config/settings.go:29-56: [features] six *bool (nil=unset→enabled), zero-migration default via Resolve()/AllFeatures(). ParseRepoSettings rejects unknown keys incl. inside [features] (Undecoded head check); wrong types fail toml decode→400. Merge ignores Features (no host counterpart; effective/describe untouched by construction). Tests: TestParseRepoSettingsFeatures{,/Rejects}, TestRepoSettingsFeaturesMergeIgnored, TestFeaturesOf, TestResolvedFeaturesETagBits.

(2) SUMMARY PROJECTION — PASS. internal/api/summary.go:60,174-177,263-272: features always present (non-omitempty, #319 discipline), folded from manifest-inline TOML at +0 probes (same snapshot repoDescription uses; law 4/6 clean, off budgeted paths). ~t ETag suffix UNCONDITIONAL with fixed-order bits — the #513 lesson applied. Proven by TestSummaryFeaturesRevalidate: pre-#522 ETag (~k0, no ~t) revalidates to 200 not 304; settings-only flip busts (→~t011111); current ETag 304s; re-enable restores original ETag. TestWalSummaryFeatures covers the walView fold incl. fail-open.

(3) TAB GATING — PASS. web/src/lib/tabs.js: FEATURE_TABS={issues,pulls,releases}, showFeatureTab fail-open; activeTab(pathname, summary) re-highlights disabled sections onto Code (raw mapping pinned by tests). Repo.jsx gates tab strip only — routes/lists keep rendering, deep links sane. Composers (IssueNew/PullNew/ReleaseNew/Fork) hide forms behind explainers with back-links; list pages gate New-buttons + Empty CTAs. Reads stay (display-only, Forgejo semantics — stated in 07_api Decisions).

(4) API GUARDS — PASS. Star (social/service.go:49), SetWatch on-branch only (notify/watch.go:64 — unwatch path untouched), StartFork (pulls/merge.go:691, after role gate → binds everyone incl. admins, no bypass). All wrap ErrForbidden→403 with naming reasons (statusFor errors.Is in all three packages; fork via writeErr→statusFor). Fail-open on unwired/declining hook (CollabCounts precedent). Tests: TestStarDisabledRefuses/BindsEveryone/UnstarAlwaysWorks, TestSetWatchDisabledRefuses/UnwatchAlwaysWorks, TestStartForkDisabledRefuses/BindsWriter. Note: already-starred re-PUT also 403s (uniform refusal, documented) — spec-compliant since PUT is the new-star affordance. Issues/PRs/releases creation APIs deliberately unguarded (display-only decision, documented).

(5) PILLS — PASS. Star/Watch keep #447 idiom (count left), disabled+muted with reason title/aria-label only when off AND viewer not engaged; retained star/watch keep enabled Unstar/Unwatch. Fork count link stays, CTA → muted span with reason. Flip handlers belt-and-braces return early.

(6) GENERAL TAB — PASS. Six toggles on existing save path (put(doc,"") — same CAS behavior as description save, no new hazard); withFeatures replaces only [features] block; failure reseeds from server truth with 403 Surfacing in note; invalidates repo:/settings:/settings-effective/settings-history. Dirty tracking + unsaved-changes hint.

(7) GATES — all green in scratch: config/api/social/notify/pulls/cmd -race PASS; coverage config 95.9 / api 95.3 / social 99.5 / notify 95.3 / pulls 96.1 (≥95 ✓); gofmt clean; go vet clean; go build ✓; node 1211/1212 (sole fail = smoke.test.js live-fetch vs 127.0.0.1:8080 where /setup→403 — environmental, PR-code-independent); vite+esbuild ✓ (via direct binaries; pnpm shim unusable in scratch, node_modules was a symlink); e2e PASS (56s). push_budget_test pins the three Features hooks wired + fail-open. Docs: 07_api §9.1/§11 + Decisions, 12_web_ui, features/02/03/06/07 updated in-change (law 12 ✓). No new deps (go.mod/package.json untouched). Concurrency: no new goroutines/locks; guard reads precede all shard/CAS locks — no lock-order issue.

No fixes needed — nothing pushed. MERGE RECOMMENDATION: ready to merge.

REVIEW PR #532 (fix/issue-522, d592b73) — adversarial pass, verified in scratch worktree (removed afterward). No browser (tests + reasoning; no browser-facing layout risk beyond gated Shows). (1) SETTINGS MODEL — PASS. internal/config/settings.go:29-56: [features] six *bool (nil=unset→enabled), zero-migration default via Resolve()/AllFeatures(). ParseRepoSettings rejects unknown keys incl. inside [features] (Undecoded head check); wrong types fail toml decode→400. Merge ignores Features (no host counterpart; effective/describe untouched by construction). Tests: TestParseRepoSettingsFeatures{,/Rejects}, TestRepoSettingsFeaturesMergeIgnored, TestFeaturesOf, TestResolvedFeaturesETagBits. (2) SUMMARY PROJECTION — PASS. internal/api/summary.go:60,174-177,263-272: features always present (non-omitempty, #319 discipline), folded from manifest-inline TOML at +0 probes (same snapshot repoDescription uses; law 4/6 clean, off budgeted paths). ~t ETag suffix UNCONDITIONAL with fixed-order bits — the #513 lesson applied. Proven by TestSummaryFeaturesRevalidate: pre-#522 ETag (~k0, no ~t) revalidates to 200 not 304; settings-only flip busts (→~t011111); current ETag 304s; re-enable restores original ETag. TestWalSummaryFeatures covers the walView fold incl. fail-open. (3) TAB GATING — PASS. web/src/lib/tabs.js: FEATURE_TABS={issues,pulls,releases}, showFeatureTab fail-open; activeTab(pathname, summary) re-highlights disabled sections onto Code (raw mapping pinned by tests). Repo.jsx gates tab strip only — routes/lists keep rendering, deep links sane. Composers (IssueNew/PullNew/ReleaseNew/Fork) hide forms behind explainers with back-links; list pages gate New-buttons + Empty CTAs. Reads stay (display-only, Forgejo semantics — stated in 07_api Decisions). (4) API GUARDS — PASS. Star (social/service.go:49), SetWatch on-branch only (notify/watch.go:64 — unwatch path untouched), StartFork (pulls/merge.go:691, after role gate → binds everyone incl. admins, no bypass). All wrap ErrForbidden→403 with naming reasons (statusFor errors.Is in all three packages; fork via writeErr→statusFor). Fail-open on unwired/declining hook (CollabCounts precedent). Tests: TestStarDisabledRefuses/BindsEveryone/UnstarAlwaysWorks, TestSetWatchDisabledRefuses/UnwatchAlwaysWorks, TestStartForkDisabledRefuses/BindsWriter. Note: already-starred re-PUT also 403s (uniform refusal, documented) — spec-compliant since PUT is the new-star affordance. Issues/PRs/releases creation APIs deliberately unguarded (display-only decision, documented). (5) PILLS — PASS. Star/Watch keep #447 idiom (count left), disabled+muted with reason title/aria-label only when off AND viewer not engaged; retained star/watch keep enabled Unstar/Unwatch. Fork count link stays, CTA → muted span with reason. Flip handlers belt-and-braces return early. (6) GENERAL TAB — PASS. Six toggles on existing save path (put(doc,"") — same CAS behavior as description save, no new hazard); withFeatures replaces only [features] block; failure reseeds from server truth with 403 Surfacing in note; invalidates repo:/settings:/settings-effective/settings-history. Dirty tracking + unsaved-changes hint. (7) GATES — all green in scratch: config/api/social/notify/pulls/cmd -race PASS; coverage config 95.9 / api 95.3 / social 99.5 / notify 95.3 / pulls 96.1 (≥95 ✓); gofmt clean; go vet clean; go build ✓; node 1211/1212 (sole fail = smoke.test.js live-fetch vs 127.0.0.1:8080 where /setup→403 — environmental, PR-code-independent); vite+esbuild ✓ (via direct binaries; pnpm shim unusable in scratch, node_modules was a symlink); e2e PASS (56s). push_budget_test pins the three Features hooks wired + fail-open. Docs: 07_api §9.1/§11 + Decisions, 12_web_ui, features/02/03/06/07 updated in-change (law 12 ✓). No new deps (go.mod/package.json untouched). Concurrency: no new goroutines/locks; guard reads precede all shard/CAS locks — no lock-order issue. No fixes needed — nothing pushed. MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-14 19:41:54 +00:00

crueber referenced this issue from a commit

2026-09-14 19:41:54 +00:00

Merge pull request 'Fix #522: per-repo feature toggles' (#532) from fix/issue-522 into main

crueber commented

2026-09-14 19:41:54 +00:00

Author

Owner

Fixed by PR #532 (review clean — all 9 criteria pass incl. ~t unconditional per the #513 lesson, guards fail-closed, e2e green), merged. Closing.