crueber commented

Owner

Survey: crueber/walhub#349 candidate 3 (+ extra: no delete-org UI).

Evidence

internal/identity/orgs.go:620-636 DeleteOrg refuses with 409 "org owns %d repos; transfer or delete them first" — but no repo transfer mechanism exists anywhere: grep for transfer|Transfer across internal/ finds only git bundle-uri flags (transfer.bundleURI), LFS transfer spec, and unrelated identifiers. No TransferRepo service, no transfer route, no UI.
So the 409 message references a capability that does not exist — a live contradiction: an org that owns repos can never be deleted, and the error tells the owner to use a feature that is missing.
Extra gap on the same surface: DELETE /api/v1/orgs/{org} exists (internal/identity/http.go routeOrg, owner-only) and SDK client.orgs.delete exists (web/sdk/src/orgs.js:45), but web/src/pages/Org.jsx has no delete affordance (tabs are Profile/Members/Teams/Invitations only; repo DangerZone in Settings.jsx is per-repo).

Design

Add a repo transfer service + route (owner-or-admin gated, CAS on manifest/access): move repos/// to repos/// (or rename owner), carrying access.json with owner-subject rewrite, gated by CheckCreateOwner-style admission on the destination and ownership on the source. Forgejo has repo transfer; mirror its semantics (transfer + accept, or direct for owners — keep v1 simple: direct, owner-initiated).
Fix the 409 wording to name only capabilities that exist until transfer lands (or land both together — preferred).
Add an owner-only Danger Zone to the org settings page (typed-confirm idiom from Settings.jsx DangerConfirm) wired to client.orgs.delete, surfacing the 409 honestly.

Acceptance criteria

Transferring a repo between owners works end-to-end (API + SDK + UI entry point) with access preserved per the moved bindings.
DeleteOrg 409 names only existing capabilities, or transfer exists so the message is true.
Org settings page exposes delete-org to owners; non-owners see no affordance; server still gates.
Tests: transfer unit + handler tests, DeleteOrg-with-repos cases, -race clean.

Survey: crueber/walhub#349 candidate 3 (+ extra: no delete-org UI). ## Evidence - internal/identity/orgs.go:620-636 DeleteOrg refuses with 409 "org owns %d repos; transfer or delete them first" — but no repo transfer mechanism exists anywhere: grep for transfer|Transfer across internal/ finds only git bundle-uri flags (transfer.bundleURI), LFS transfer spec, and unrelated identifiers. No TransferRepo service, no transfer route, no UI. - So the 409 message references a capability that does not exist — a live contradiction: an org that owns repos can never be deleted, and the error tells the owner to use a feature that is missing. - Extra gap on the same surface: DELETE /api/v1/orgs/{org} exists (internal/identity/http.go routeOrg, owner-only) and SDK client.orgs.delete exists (web/sdk/src/orgs.js:45), but web/src/pages/Org.jsx has no delete affordance (tabs are Profile/Members/Teams/Invitations only; repo DangerZone in Settings.jsx is per-repo). ## Design - Add a repo transfer service + route (owner-or-admin gated, CAS on manifest/access): move repos/<src>/<r>/ to repos/<dst>/<r>/ (or rename owner), carrying access.json with owner-subject rewrite, gated by CheckCreateOwner-style admission on the destination and ownership on the source. Forgejo has repo transfer; mirror its semantics (transfer + accept, or direct for owners — keep v1 simple: direct, owner-initiated). - Fix the 409 wording to name only capabilities that exist until transfer lands (or land both together — preferred). - Add an owner-only Danger Zone to the org settings page (typed-confirm idiom from Settings.jsx DangerConfirm) wired to client.orgs.delete, surfacing the 409 honestly. ## Acceptance criteria - [ ] Transferring a repo between owners works end-to-end (API + SDK + UI entry point) with access preserved per the moved bindings. - [ ] DeleteOrg 409 names only existing capabilities, or transfer exists so the message is true. - [ ] Org settings page exposes delete-org to owners; non-owners see no affordance; server still gates. - [ ] Tests: transfer unit + handler tests, DeleteOrg-with-repos cases, -race clean.

crueber added this to the v1 milestone

2026-09-12 00:48:40 +00:00

crueber added the

issues

oidc

labels

2026-09-12 00:48:40 +00:00

crueber referenced this issue

2026-09-12 00:49:08 +00:00

Org support gap survey: evaluate and file remaining org-feature tickets vs Forgejo/GitHub parity #349

crueber referenced this issue

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

Org support gap survey: evaluate and file remaining org-feature tickets vs Forgejo/GitHub parity #349

crueber referenced this issue from a commit

2026-09-12 01:01:58 +00:00

Fix #358: repo transfer + org delete

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

2026-09-12 01:01:59 +00:00

Fix #358: repo transfer + org delete #365

crueber commented

2026-09-12 01:02:03 +00:00

Author

Owner

PR #365 (fix/issue-358) implements this: direct owner-initiated transfer (POST /{owner}/{repo}/api/transfer), DeleteOrg 409 now true without a wording change, owner-only org Danger Zone + repo transfer entry. #365

PR #365 (fix/issue-358) implements this: direct owner-initiated transfer (POST /{owner}/{repo}/api/transfer), DeleteOrg 409 now true without a wording change, owner-only org Danger Zone + repo transfer entry. https://git.packden.us/crueber/walhub/pulls/365

crueber referenced this issue from a commit

2026-09-12 01:06:01 +00:00

Fix #358 review: correct transfer recovery + dst-org 403 wording

crueber commented

2026-09-12 01:06:19 +00:00

Author

Owner

PR #365 review (fix/issue-358) — adversarial data-move pass

Verified in scratch worktree at 0b49b5f (incl. my 3-line wording fix, pushed). No browser per instructions; tests + reasoning. Note: vite build run via vite+esbuild directly (pnpm absent in this env).

1. Move atomicity — SOUND, no loss in any crash interleaving

transfer.go:71 copy-then-delete, every dst write PutCreate. Cleanup (transfer.go:113) only deletes keys this attempt PutCreated, and only runs on the copy-failure path before any delete — so a cleaned-up dst key always still has its src copy. Provably no object loss.
Crash between copy and delete → duplicate (both prefixes complete); retry 409s on occupied manifest, honestly. Crash during delete → partial src + complete dst, error surfaces (transfer.go:169). Union of both sides is always whole. Recovery is manual (delete one side) — acceptable for an admin path.
Re-list race detection (transfer.go:178) is sound for new keys (the content-addressed pack case): 409 names the residue count with dst complete. Mid-move PutCreate loss → 409 + cleanup, src untouched — tested (TestTransferRepoMidMoveCollisionCleansUp).

2. access.json rewrite — CORRECT all four directions

transfer.go:255: org→org untouched (no owner-subject either side); org→user keeps + adds user:<dst>/admin; user→org drops seller; user→user drops + adds. Self-transfer converges (handler test asserts). normPrincipal lowercases so the ToLower compare is sound; nonNilBindings (access.go:186) prevents null bindings; corrupt → opaque bytes, missing → synthesize user-dst default only. All covered by tests.

3. Gates — VERBATIM, honest statuses, both lanes

http_transfer.go:28,48: source CheckRole admin then CheckCreateOwner on dst, in that order (fail-closed: strangers learn nothing). 401 anon / 403 non-admin + foreign dst / 404 ghost source / 409 occupied / 400 bad body / 405 — each pinned by TestTransferHandler subtests incl. browser lane (201) and host-admin bypass (201).

4. DeleteOrg 409 — TRUE + PINNED

orgs.go:635 message names transfer, which now exists; TestDeleteOrgNamesTransfer asserts the 409 contains "transfer". No wording change needed — correct call.

5. UI — owner-only, typed-confirm, SDK idiom matches

Org.jsx Danger tab derived from server-authoritative can_edit, double-guarded by Show; server still gates. Repo transfer reuses exported DangerConfirm, navigates to the new address with cache invalidation. sdk/transfer.js matches the access.js _path/_call idiom; transferBody trims + omits empty repo (server defaults).

6/7. Cost + streaming — OK for a cold admin path

2 manifest HEADs + 1 LIST + 2 ops/key, copyObject streams Get→Put (transfer.go:199), access.json via GetBytes only. LIST is prefix-bounded, off every hot path. Law 6/4 hold.

8. Verification results

go test -race ./internal/identity/... clean; coverage 96.8% (≥95 gate holds; TransferRepo 90%, copyObject 88.9%).
-race -count=10 -run 'TestTransfer|TestDeleteOrg' clean. gofmt/vet clean.
Node: 695/698 pass incl. new transfer.test.js 3/3. The 3 failures are all smoke.test.js, which fetches a live server at :8080 — that port is held by a foreign listener returning 503 in this env, so they fail identically on main (environmental, not a regression).
vite build + esbuild SDK bundle OK; go build ./... OK.

9. Docs — accurate; fixed 3 small inaccuracies (pushed as `0b49b5f`)

http_transfer.go:16: unknown dst org is 403 via CheckCreateOwner, not 404.
transfer.go:22 + 01_identity_permissions.md §5.4: a raced transfer's src residue must be deleted (re-transfer would 409 on the occupied manifest); only a mistyped transfer recovers via transfer-back.

Known limitation (NON-BLOCKING follow-up, not a merge gate)

A push that fully lands — manifest CAS included — between the copy of manifest.pb and its unconditional Delete(..., "") is silently regressed (dst holds the stale copy). Window is tiny and admin-path-only; new-key races are already caught honestly. Suggested hardening: capture source versions during the copy LIST and conditional-delete, or re-read + compare the source manifest before the delete pass (abort-with-cleanup still possible there). Separately, the stale WAL handle on the source id degrades fail-closed (freshen → store error; fresh open → 404; auto-create semantics identical to repo delete), and orphaned disk cache is already a handled concept — no action needed.

MERGE RECOMMENDATION: ready to merge (pending CI)

# PR #365 review (fix/issue-358) — adversarial data-move pass Verified in scratch worktree at `0b49b5f` (incl. my 3-line wording fix, pushed). No browser per instructions; tests + reasoning. Note: vite build run via `vite`+`esbuild` directly (`pnpm` absent in this env). ## 1. Move atomicity — SOUND, no loss in any crash interleaving - `transfer.go:71` copy-then-delete, every dst write `PutCreate`. Cleanup (`transfer.go:113`) only deletes keys this attempt `PutCreate`d, and only runs on the copy-failure path before any delete — so a cleaned-up dst key always still has its src copy. Provably no object loss. - Crash between copy and delete → duplicate (both prefixes complete); retry 409s on occupied manifest, honestly. Crash during delete → partial src + complete dst, error surfaces (`transfer.go:169`). Union of both sides is always whole. Recovery is manual (delete one side) — acceptable for an admin path. - Re-list race detection (`transfer.go:178`) is sound for new keys (the content-addressed pack case): 409 names the residue count with dst complete. Mid-move `PutCreate` loss → 409 + cleanup, src untouched — tested (`TestTransferRepoMidMoveCollisionCleansUp`). ## 2. access.json rewrite — CORRECT all four directions `transfer.go:255`: org→org untouched (no owner-subject either side); org→user keeps + adds `user:<dst>`/admin; user→org drops seller; user→user drops + adds. Self-transfer converges (handler test asserts). `normPrincipal` lowercases so the `ToLower` compare is sound; `nonNilBindings` (`access.go:186`) prevents null bindings; corrupt → opaque bytes, missing → synthesize user-dst default only. All covered by tests. ## 3. Gates — VERBATIM, honest statuses, both lanes `http_transfer.go:28,48`: source `CheckRole` admin then `CheckCreateOwner` on dst, in that order (fail-closed: strangers learn nothing). 401 anon / 403 non-admin + foreign dst / 404 ghost source / 409 occupied / 400 bad body / 405 — each pinned by `TestTransferHandler` subtests incl. browser lane (201) and host-admin bypass (201). ## 4. DeleteOrg 409 — TRUE + PINNED `orgs.go:635` message names transfer, which now exists; `TestDeleteOrgNamesTransfer` asserts the 409 contains "transfer". No wording change needed — correct call. ## 5. UI — owner-only, typed-confirm, SDK idiom matches `Org.jsx` Danger tab derived from server-authoritative `can_edit`, double-guarded by `Show`; server still gates. Repo transfer reuses exported `DangerConfirm`, navigates to the new address with cache invalidation. `sdk/transfer.js` matches the `access.js` `_path`/`_call` idiom; `transferBody` trims + omits empty repo (server defaults). ## 6/7. Cost + streaming — OK for a cold admin path 2 manifest HEADs + 1 LIST + 2 ops/key, `copyObject` streams Get→Put (`transfer.go:199`), access.json via `GetBytes` only. LIST is prefix-bounded, off every hot path. Law 6/4 hold. ## 8. Verification results - `go test -race ./internal/identity/...` clean; coverage **96.8%** (≥95 gate holds; TransferRepo 90%, copyObject 88.9%). - `-race -count=10 -run 'TestTransfer|TestDeleteOrg'` clean. `gofmt`/`vet` clean. - Node: **695/698** pass incl. new `transfer.test.js` 3/3. The 3 failures are all `smoke.test.js`, which fetches a live server at `:8080` — that port is held by a foreign listener returning 503 in this env, so they fail identically on main (environmental, not a regression). - `vite build` + `esbuild` SDK bundle OK; `go build ./...` OK. ## 9. Docs — accurate; fixed 3 small inaccuracies (pushed as `0b49b5f`) - `http_transfer.go:16`: unknown dst org is **403** via CheckCreateOwner, not 404. - `transfer.go:22` + `01_identity_permissions.md` §5.4: a *raced* transfer's src residue must be deleted (re-transfer would 409 on the occupied manifest); only a *mistyped* transfer recovers via transfer-back. ## Known limitation (NON-BLOCKING follow-up, not a merge gate) A push that fully lands — manifest CAS included — between the copy of `manifest.pb` and its unconditional `Delete(..., "")` is silently regressed (dst holds the stale copy). Window is tiny and admin-path-only; new-key races are already caught honestly. Suggested hardening: capture source versions during the copy LIST and conditional-delete, or re-read + compare the source manifest before the delete pass (abort-with-cleanup still possible there). Separately, the stale WAL handle on the source id degrades fail-closed (freshen → store error; fresh open → 404; auto-create semantics identical to repo delete), and orphaned disk cache is already a handled concept — no action needed. ## MERGE RECOMMENDATION: ready to merge (pending CI)

crueber closed this issue

2026-09-12 01:06:34 +00:00

crueber referenced this issue from a commit

2026-09-12 01:06:34 +00:00

Merge pull request 'Fix #358: repo transfer + org delete' (#365) from fix/issue-358 into main

crueber commented

2026-09-12 01:06:35 +00:00

Author

Owner

Fixed by PR #365 (review clean + two hardening fixes by reviewer: unknown-org 403, src-residue note; atomicity + gates + rewrite verified), merged. Closing.

crueber referenced this issue

2026-09-12 01:17:14 +00:00

Org profile parity + avatar: location/timezone/bio fields and avatar storage #359

crueber referenced this issue

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

Org support gap survey: evaluate and file remaining org-feature tickets vs Forgejo/GitHub parity #349

Rows
Columns

Org repos: transfer between owners; fix DeleteOrg 409 dangling reference; delete-org UI #358

Evidence

Design

Acceptance criteria

PR #365 review (fix/issue-358) — adversarial data-move pass

1. Move atomicity — SOUND, no loss in any crash interleaving

2. access.json rewrite — CORRECT all four directions

3. Gates — VERBATIM, honest statuses, both lanes

4. DeleteOrg 409 — TRUE + PINNED

5. UI — owner-only, typed-confirm, SDK idiom matches

6/7. Cost + streaming — OK for a cold admin path

8. Verification results

9. Docs — accurate; fixed 3 small inaccuracies (pushed as 0b49b5f)

Known limitation (NON-BLOCKING follow-up, not a merge gate)

MERGE RECOMMENDATION: ready to merge (pending CI)

9. Docs — accurate; fixed 3 small inaccuracies (pushed as `0b49b5f`)