Skip to content

Atomic, idempotent acceptContribution for contribution proposals - #76

Draft
phoebus-84 wants to merge 2 commits into
feat/atomic-resource-metadatafrom
feat/atomic-accept-contribution
Draft

phoebus-84 wants to merge 2 commits into
feat/atomic-resource-metadatafrom
feat/atomic-accept-contribution

Conversation

@phoebus-84

Copy link
Copy Markdown
Collaborator

Workstream: #74 (non-closing reference). Stacked on #75 → #73 → #72 → #71; review after them. Draft: nothing to merge or deploy yet.

Why

Accepting a contribution today takes two non-transactional requests, from both the SDK's acceptProposal and the GUI's pages/proposal/[id].tsx: three events, then three satisfactions. If the second request fails, the events are already committed and the proposal stays PENDING. A retry then creates the events again: the SDK opens a new process on every call, so the one-accept-per-process check doesn't stop it. The GUI's modify event also sends a whole metadata map built in the browser, which replaces the origin's metadata at commit time.

What

acceptContribution(proposalId: ID!) → { proposal citeEvent acceptEvent modifyEvent }, in one transaction:

  • cite/accept/modify events in the proposer's process (the slice-3 invited-party rule);
  • the three satisfactions;
  • the fork appended once to the origin's current, locked relations; every other key is kept.
Proposal state Result
pending accepted
already accepted same events returned, nothing written (safe retry)
rejected / half-accepted CONFLICT "proposal is not pending"
not a contribution shape BAD_USER_INPUT

No new policy: each write goes through the existing event.create / satisfaction.create rules in the same transaction. Carol, the proposer, or an owner who lost control get FORBIDDEN, and nothing is written.

Locks: resource rows FOR UPDATE (ascending id), then intent rows FOR SHARE, following the slice-3 order. Design: docs/atomic-accept-contribution.md.

Satisfaction.Domain.multi_insert/4 is extracted from create/2 without changing its behavior.

Verification (local sandbox)

  • test/authz/accept_contribution.test.exs: 12 tests (GraphQL + domain), including rollback after the first event.
  • test/authz/accept_contribution_concurrency.test.exs: 5 tests on separate PostgreSQL connections: accept/accept (waiting, and 4 free racers → one set of events), reject-first → CONFLICT, accept-first → reject waits and status stays ACCEPTED, a metadata patch racing an accept → both fields kept.
  • Full suite with the same seed (590095): Atomic resource metadata patches with content CAS and audit #75 base 872 tests, 2 failures; this branch 889 tests, the same 2 failures. Both failures are on the base too: classifications/1, already known, and claimable_agents defaults to empty, which depends on the local stack's claimable-agent config.
  • One earlier full run on this branch also failed the two DB.Page cursor tests. They didn't reproduce with that seed, or with 8 seeds on the base. Those tests count every committed person, so they are sensitive to the existing tests that commit outside the sandbox. Recorded here, not claimed fixed.

Not in this PR

  • The SDK method and the GUI switch come in separate stacked PRs.
  • Old sequences and mutations keep working and keep their partial-commit risk.
  • Proposals already left half-accepted are reported, never repaired.
  • After acceptance the intents stay unfinished, as before, so the owner's invitation to the proposer's process remains open. This is pre-existing behavior, flagged for a later decision.

🤖 Generated with Claude Code

acceptContribution(proposalId) writes the owner's cite/accept/modify
events in the proposer's process, the three satisfactions and the fork
appended to the origin's current relations in one transaction, through
the existing event.create and satisfaction.create policy. Retrying an
accepted proposal returns its events and writes nothing; rejected or
half-accepted proposals are a CONFLICT. Resource rows are locked before
intent rows, following the slice-3 lock order.

Satisfaction.Domain gains a composable multi_insert; create is unchanged.

Refs #74

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
acceptContribution now marks the cite/accept/modify intents finished in
the same transaction, through intent.update. The proposal stays ACCEPTED
(status counts satisfactions) and the owner's invitation to the
proposer's process (slice 3 §3.2) ends with the acceptance instead of
staying open indefinitely.

Refs #74

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@phoebus-84

Copy link
Copy Markdown
Collaborator Author

Update (a31d216): acceptance now finishes the three intents in the same transaction.

Before, the cite/accept/modify intents stayed unfinished after acceptance, so the owner stayed invited to attach events to the proposer's process (slice 3 §3.2) indefinitely. Now acceptContribution marks them finished through intent.update (the same-proposal finished rule). The status stays ACCEPTED because it counts satisfactions; retries still return the same events; a later reject is a no-op.

Tests: +2 (the invitation ends after acceptance, with a control case showing it existed before). Targeted 26/26 including concurrency and the SDK-sequence API tests. Full suite, seed 590095: 891 tests, the same 2 known failures. Real signed checks re-run green on a rebuilt local server. Docs updated (contract + lock note).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant