[codex major] Failed org creation leaves irrecoverable ownerless org #75

New issue

Closed

opened 2026-09-05 00:15:03 +00:00 by crueber · 3 comments

crueber commented

2026-09-05 00:15:03 +00:00

Owner

[codex major] Failed org creation leaves irrecoverable ownerless org

internal/identity/orgs.go:86 reserves org.json, then :94 creates members.json. A non-412 error at :96 returns, leaving the namespace reserved; retries hit "org already exists" at :89 with no recovery path — all owner-gated management is inaccessible, permanently.

Fix

Make org creation atomic-or-recoverable: roll back the reservation on members failure (best-effort delete + return the original error), or complete the owner binding on retry when the org is ownerless (idempotent resume), or both. Deterministic regression test (fault-injected members write → retry succeeds, owner present). Coverage gate holds; doc Decisions entry (law 12).

Acceptance criteria

No interleaving leaves an ownerless org; fault-injection test green -race.

# [codex major] Failed org creation leaves irrecoverable ownerless org `internal/identity/orgs.go:86` reserves `org.json`, then `:94` creates `members.json`. A non-412 error at `:96` returns, leaving the namespace reserved; retries hit "org already exists" at `:89` with no recovery path — all owner-gated management is inaccessible, permanently. ## Fix Make org creation atomic-or-recoverable: roll back the reservation on members failure (best-effort delete + return the original error), or complete the owner binding on retry when the org is ownerless (idempotent resume), or both. Deterministic regression test (fault-injected members write → retry succeeds, owner present). Coverage gate holds; doc Decisions entry (law 12). ## Acceptance criteria - [ ] No interleaving leaves an ownerless org; fault-injection test green `-race`.

crueber referenced this issue from a commit

2026-09-05 00:58:03 +00:00

Fix #75: atomic-or-recoverable org creation

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

2026-09-05 00:58:06 +00:00

Fix #75: atomic-or-recoverable org creation #84

crueber commented

2026-09-05 00:58:08 +00:00

Author

Owner

Fixed by PR #84 (#84): atomic-or-recoverable org creation — rollback on members-seed failure + CAS resume healing ownerless reservations, with fault-injection regression tests. Not merging; review requested.

Fixed by PR #84 (https://git.packden.us/crueber/walhub/pulls/84): atomic-or-recoverable org creation — rollback on members-seed failure + CAS resume healing ownerless reservations, with fault-injection regression tests. Not merging; review requested.

crueber referenced this issue from a commit

2026-09-05 01:01:36 +00:00

Review fix for #75: never roll back under a concurrent heal winner

crueber commented

2026-09-05 01:01:49 +00:00

Author

Owner

PR #84 review (branch fix/issue-75), verified in scratch worktree at eb19e74.

ONE REAL BUG FOUND AND FIXED (pushed as fix/issue-75-review, commit 55d9297 — please fold into fix/issue-75):

internal/identity/orgs.go:121-125 (rollback): the version-guarded Delete does NOT protect a concurrent heal winner. Walk the race: A reserves org.json v1 and its members seed fails with a non-412; B concurrently CreateOrgs the same name, 412s on org.json, and heals via resumeOrgCreate by Creating members.json with B as owner, then returns success. B never rewrites org.json, so the version guard cannot fire, and A's rollback deletes org.json out from under B's observed success — orphaning B's roster (org-less members.json that 409s every non-B creator until B retries). Fix: check members.json before deleting; a present roster means someone won under our reservation, so arbitrate read-only via confirmOrgOwner and never delete. Added TestCreateOrgRollbackSkipsDeleteUnderWinner (pre-seeded foreign roster + failing-once seed: asserts 409 for the non-owner, org.json intact, winner roster untouched, winner re-create succeeds). Verified it FAILS pre-fix (returns boom + deletes the reservation) and passes post-fix. Residual TOCTOU (heal landing between check and delete) is unavoidable without cross-object CAS and still converges via resume; the code comment now says so honestly instead of claiming never deleted.

REST VERIFIED, no action needed:

Resume precision (orgs.go resumeOrgCreate/confirmOrgOwner): rival can never gain ownership — the CAS writes only to ownerless rosters (rosterHasOwner gate) and 409s otherwise with zero writes; already-owner is idempotent (write=false path); genuinely-owned-by-other 409s with roster byte-identical (TestCreateOrgConflictKeepsRoster asserts). Each branch covered by a dedicated test.
Crash residue heals through the same casUpdate path, no divergent special case (missing-members and ownerless-roster both flow through resume).
16-way concurrent-create test is genuine (real goroutines over Memory CAS, asserts exactly 1 winner + 15 ErrConflict + sole-owner roster); ran -race -count=5 clean.
Idempotent-201 contract: same creator/owner re-POST now 201s instead of 409. No masked real conflict — a different creator still 409s; documented in the doc Decisions entry. Non-blocking nit: handler always returns 201, so automation cannot distinguish created vs already-owned (no 200); acceptable pre-1.0, docs do not promise a status split.
Stale/corrupt roster fail-closed: corrupt members.json surfaces ErrInvalid (400), stale foreign roster 409s untouched; confirmOrgOwner ownerless corner 409s on first call but converges via resume on retry (documented in code comment; requires out-of-band residue, accepted as-is).
CAS-only coordination: only Put Create/Update + versioned Delete + casUpdate loop; no new locks, no new imports, go.mod untouched (PR = 4 files: orgs.go, orgs_recover_test.go, http_test.go, doc).
Coverage 97.3% package (>=95% gate holds); CreateOrg 100%, resume 92.3%, confirm 83.3% (uncovered: getMembers-error propagation). gofmt/vet clean. Full package go test -race green.
Base check: standalone repro of the core scenario FAILS on base (retry 409s conflict: org already exists — the exact #75 wedge) and passes on the PR. Scratch restored clean afterward; main worktree never touched (fetch/diff only).

MERGE RECOMMENDATION: blocked: fold fix/issue-75-review (rollback-under-winner fix + regression test) into fix/issue-75 first; with that in, ready to merge.

PR #84 review (branch fix/issue-75), verified in scratch worktree at eb19e74. ONE REAL BUG FOUND AND FIXED (pushed as fix/issue-75-review, commit 55d9297 — please fold into fix/issue-75): - internal/identity/orgs.go:121-125 (rollback): the version-guarded Delete does NOT protect a concurrent heal winner. Walk the race: A reserves org.json v1 and its members seed fails with a non-412; B concurrently CreateOrgs the same name, 412s on org.json, and heals via resumeOrgCreate by Creating members.json with B as owner, then returns success. B never rewrites org.json, so the version guard cannot fire, and A's rollback deletes org.json out from under B's observed success — orphaning B's roster (org-less members.json that 409s every non-B creator until B retries). Fix: check members.json before deleting; a present roster means someone won under our reservation, so arbitrate read-only via confirmOrgOwner and never delete. Added TestCreateOrgRollbackSkipsDeleteUnderWinner (pre-seeded foreign roster + failing-once seed: asserts 409 for the non-owner, org.json intact, winner roster untouched, winner re-create succeeds). Verified it FAILS pre-fix (returns boom + deletes the reservation) and passes post-fix. Residual TOCTOU (heal landing between check and delete) is unavoidable without cross-object CAS and still converges via resume; the code comment now says so honestly instead of claiming never deleted. REST VERIFIED, no action needed: - Resume precision (orgs.go resumeOrgCreate/confirmOrgOwner): rival can never gain ownership — the CAS writes only to ownerless rosters (rosterHasOwner gate) and 409s otherwise with zero writes; already-owner is idempotent (write=false path); genuinely-owned-by-other 409s with roster byte-identical (TestCreateOrgConflictKeepsRoster asserts). Each branch covered by a dedicated test. - Crash residue heals through the same casUpdate path, no divergent special case (missing-members and ownerless-roster both flow through resume). - 16-way concurrent-create test is genuine (real goroutines over Memory CAS, asserts exactly 1 winner + 15 ErrConflict + sole-owner roster); ran -race -count=5 clean. - Idempotent-201 contract: same creator/owner re-POST now 201s instead of 409. No masked real conflict — a different creator still 409s; documented in the doc Decisions entry. Non-blocking nit: handler always returns 201, so automation cannot distinguish created vs already-owned (no 200); acceptable pre-1.0, docs do not promise a status split. - Stale/corrupt roster fail-closed: corrupt members.json surfaces ErrInvalid (400), stale foreign roster 409s untouched; confirmOrgOwner ownerless corner 409s on first call but converges via resume on retry (documented in code comment; requires out-of-band residue, accepted as-is). - CAS-only coordination: only Put Create/Update + versioned Delete + casUpdate loop; no new locks, no new imports, go.mod untouched (PR = 4 files: orgs.go, orgs_recover_test.go, http_test.go, doc). - Coverage 97.3% package (>=95% gate holds); CreateOrg 100%, resume 92.3%, confirm 83.3% (uncovered: getMembers-error propagation). gofmt/vet clean. Full package go test -race green. - Base check: standalone repro of the core scenario FAILS on base (retry 409s conflict: org already exists — the exact #75 wedge) and passes on the PR. Scratch restored clean afterward; main worktree never touched (fetch/diff only). MERGE RECOMMENDATION: blocked: fold fix/issue-75-review (rollback-under-winner fix + regression test) into fix/issue-75 first; with that in, ready to merge.

crueber closed this issue

2026-09-05 01:02:25 +00:00

crueber commented

2026-09-05 01:02:25 +00:00

Author

Owner

Fixed by PR #84 incl. review-found rollback-under-winner race fix, merged. Closing.

crueber referenced this issue from a commit

2026-09-05 01:02:25 +00:00

Merge pull request 'Fix #75: atomic-or-recoverable org creation' (#84) from fix/issue-75 into main

crueber referenced this issue

2026-09-08 19:13:16 +00:00