Add the promote-to-owner endpoint for group Owners #59
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/transfer-ownership-endpoint"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implements task 132:
POST /api/groups/{groupId}/members/{userId}/promote-to-owner, Owner-only, promoting an existing accepted member to Owner. A pending invitee is refused with 409 — they hold no rights until they accept, so promoting one would create an Owner who cannot act.Shape: two independent operations, not an atomic transfer. Stepping down is already built (task 84/ADR-0052's self-demotion, gated on the last-Owner invariant); this ticket only had to add the other half, creating a second Owner. Argued in full in ADR-0054:
Concurrency:
PromoteToOwnerAsyncneeds no lock — it only ever adds an accepted Owner, so it cannot participate in the write-skew raceChangeRoleAsync'sFOR UPDATECTE exists to close. A promotion racing a concurrent self-demotion of the group's only Owner can't zero the accepted-Owner count either way it interleaves, argued in the ADR and proved with genuinely concurrent callers in bothGroupMembershipRepositoryTestsandPromoteToOwnerEndpointTests.Took ADR number 0054 — 0053 is claimed by the open
feat/remove-member-endpointPR (#58, task 85), checked immediately before pushing.Full local build/test/format run clean against the pinned SDK (10.0.100).
Verdict: mergeable
Verified the load-bearing claim empirically, not just by reading the ADR. Stress-tested
PromoteToOwnerAsync(300 iterations) racing a self-demotion of the sole Owner, and separately (300 iterations) racing a removal (task 85'sRemoveAsync, reproduced from the still-open PR #58 since it isn't onmainyet) of the group's only other Owner, plus 200 iterations of two concurrent promotions of the same member. No deadlocks, no zero-owner state, both interleavings of the demotion race actually occurred (demotion won 261/300, promotion won 39/300). The invariant ADR-0054 states — promotion is monotonic in the accepted-Owner count, so it can't participate in the write skewChangeRoleAsync's lock exists to close — holds and is stated precisely, not just asserted.Mutated the
status = 'accepted'guard out ofPromoteMemberToOwnerand confirmed bothPromoteToOwnerAsync_APendingInvitee_ReportsNotAcceptedAndLeavesTheRowUnchangedandPromoteToOwner_APendingInvitee_RespondsWith409AndLeavesTheRowUnchangedredden. Reverted before finishing.Owner-only/403/404-identical/updated_at/entities-off-the-wire/
OpenApiDocumentTestsall check out — full local suite passes (696 tests) against the pinned SDK (10.0.100).On the shape decision (ADR-0054), a real opinion rather than agreement: the ADR does confront ADR-0050/0052's "second Owner can unilaterally delete the group or remove the first" objection head-on rather than ignoring it, which is the right instinct. But its resolution folds two distinct cases under one heading. "Promote a second Owner and keep both" is the acceptance criteria's own legitimate end state, and the residual risk there really is an unavoidable cost of allowing co-ownership at all — no disagreement. "Transfer ownership" is different: the caller's intent is a clean single-owner handoff, and the two-call design opens the ADR-0050/0052 window for that caller too, even though they never wanted co-ownership even transiently. If the newly-promoted member removes the original or deletes the group in the gap between the two calls, that's not "abandoning the flow" (which the Consequences section reasons about and accepts) — it's the exact attack those two ADRs were written to prevent, now reachable in two authenticated calls instead of one. The Alternatives section's rejection of atomic transfer is really a rejection of a single endpoint trying to serve both cases behind a flag, which I agree is worse; it doesn't evaluate the additive option of a genuinely atomic transfer endpoint sitting beside promote-only, kept in scope for callers who want the handoff case with no window at all. To its credit, the Consequences section leaves that door open for later rather than foreclosing it. Given task 132's acceptance criteria only asked for promote-only plus reuse of self-demotion, I don't think this blocks the PR — but the ADR should not describe the handoff-window risk as "the accepted cost... no shape of this endpoint removes it" when a scoped atomic endpoint specifically would remove it for that one case. Worth a follow-up ticket if a real transfer-abuse scenario ever comes up.
Traced ADR-0019 end to end: sole Owner of a shared group with at least one other accepted member can promote them, then self-demote via the existing
PUTendpoint (proven together byPromoteThenSelfDemote_TransfersOwnershipEndToEnd), then delete their account. Shared groups whose only other members are pending invitees are unaffected by this endpoint but are already covered by ADR-0019's own "withdraw the invitation first" path. No gap found for task 122.mainhasn't moved past this PR's merge base (PR #58/task 85 is still open, not merged), so no rebase was needed.Fixed ADR-0054: separated the two risk shapes explicitly. Permanent co-ownership is unaffected by atomicity either way. The two-call transfer opens a real transient handoff window that an atomic endpoint would close — named plainly now, with its concrete cost (the new Owner can act unilaterally before the second call lands) and the reasoning for accepting it anyway (the window is opened and closed by the same caller, unlike ADR-0050/0052's own scenarios).
mainhadn't moved (still at98e2d7b), so no merge needed. Full local build/test/format clean against the pinned SDK.Verdict: mergeable
Diff confirmed documentation-only (
git diff fdc1a91..a1506f7 --stattouches onlydocs/adr/0054-promote-to-owner-not-atomic-transfer.md, 50 insertions/19 deletions). The corrected text names the transient handoff window explicitly with its concrete cost (new Owner can delete the group or remove the outgoing Owner's membership before the second call lands), separates it cleanly from permanent co-ownership ("atomicity does nothing for that case" vs. "this one atomicity genuinely closes"), and no longer claims the window is unavoidable — it says plainly an atomic endpoint "would not have this window at all." The two-call shape is still argued as the right trade rather than conceded: the window is self-inflicted and self-closable by the same Owner who opened it, which is a real, materially different position from ADR-0050/0052's scenarios where the risk lands on someone who had no say. Nothing further to act on.