feat: add transactional object coordination APIs - #113
arvindh123 wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 681f37d233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut keys: Vec<_> = guards | ||
| .iter() | ||
| .map(|g| format!("lease:{}:{}", g.object_kind.label(), g.object_id.as_str())) | ||
| .collect(); |
There was a problem hiding this comment.
Canonicalize lease IDs before taking advisory locks
When a guard uses an alternate UUID spelling accepted by Uuid::parse_str—such as uppercase, simple, or URN form—this key is derived from the raw GraphQL text, while acquire/renew/release derive it from the canonical parsed UUID. Those operations therefore take different advisory locks; a release or takeover can occur after the final lease_guard check but before commit, allowing work protected by a stale fence to commit. Build every lease lock key from the parsed Uuid.
Useful? React with 👍 / 👎.
| // Deterministic ordering prevents cross-batch object lock inversion. | ||
| let mut ordered = changes.iter().collect::<Vec<_>>(); | ||
| ordered.sort_by_key(|c| (c.object_kind.label(), c.id.as_str())); |
There was a problem hiding this comment.
Lock tenant rows in a consistent global order
When concurrent batches touch objects in multiple tenants, sorting by object ID does not establish a consistent tenant-lock order: locked_object locks each object's tenant before its object row, so two batches with different objects can acquire tenant A then B and B then A. PostgreSQL will abort one batch as a deadlock even though their object sets do not overlap. Collect and lock all involved tenant rows in UUID order before taking per-object locks.
Useful? React with 👍 / 👎.
| for event in audit_events { | ||
| crate::audit::write(&state.pool, false, event).await; |
There was a problem hiding this comment.
Keep batch creates out of the compliance audit
When a batch contains a create, its entity.create or resource.create event is added to audit_events and this loop persists it through audit::write, changing the defined audit_logs compliance set. Create mutations are required to use observation-only handling, while only entity/resource updates and deletes belong in the persisted audit trail; filter create events from this write path while retaining their transactional outbox event and stdout observation.
AGENTS.md reference: AGENTS.md:L77-L81
Useful? React with 👍 / 👎.
Concurrent callers can overwrite object metadata through unconditional updates. This adds an opt-in GraphQL batch API with expected revisions, atomic changes, request replay protection, and expiring fenced leases.
Resources and unprofiled application entities support batch writes; all entities/resources expose revisions. Existing update APIs remain unconditional. Includes the forward migration, GraphQL schema/auth matrix and event catalog updates, database integration tests, and documentation. The current diff also limits Docker build parallelism and assigns UID/GID 1000 to the runtime user.
Draft review items:
Validation:
cargo fmt --checkandgit diff --cached --checkpassed.