The pre-OSS reviews contain a lot of the reasoning that got NICo to where it is now, but that reasoning no longer sits next to the public code. The idea here is to pull the recurring concurrent database-update guidance into STYLE_GUIDE.md so new contributors -- human and agentic alike -- can see it, question it, and update it deliberately instead of losing it.
What this involves
- Assume read-modify-write operations can run concurrently instead of relying on ordinary transaction behavior to reject stale writes.
- Require a row lock, version predicate, atomic SQL expression, narrow column update, database constraint, or equivalent invariant.
- Keep transactions short, and explain that
txn_held_across_await permits related database work using the same transaction or tracked connection, or a nested transaction derived from it, while still making the callee responsible for avoiding unrelated work.
- Keep any production or test allowance for
txn_held_across_await narrow, reviewed, and justified next to the allowance.
- Use
WorkLockManager when long-running work needs database-coordinated admission across NICo process instances instead of holding a pooled transaction open; use an in-process owner or mutex for task-local exclusion.
- Require a safe protocol for any old and new workers that overlap after lease expiry, and fence protected database writes with
WorkLock::fence_transaction.
- Treat a
WorkLock as an expiring lease rather than a fencing token; external effects need their own fencing, idempotency, or a reconciliation protocol proven safe when execution repeats or overlaps.
- Make zero affected rows from a version-checked update explicit instead of silently reporting the mutation as applied.
- Add a concurrent-update test when the contract promises stale-write rejection.
- Preserve the exceptions that row locks are not required when another mechanism already proves safety, and that a deliberately conditional API may return a clear not-applied result.
- Keep this change scoped to database concurrency while cross-linking the lease limitation from the state-controller crash guidance.
Part of #4621.
The pre-OSS reviews contain a lot of the reasoning that got NICo to where it is now, but that reasoning no longer sits next to the public code. The idea here is to pull the recurring concurrent database-update guidance into
STYLE_GUIDE.mdso new contributors -- human and agentic alike -- can see it, question it, and update it deliberately instead of losing it.What this involves
txn_held_across_awaitpermits related database work using the same transaction or tracked connection, or a nested transaction derived from it, while still making the callee responsible for avoiding unrelated work.txn_held_across_awaitnarrow, reviewed, and justified next to the allowance.WorkLockManagerwhen long-running work needs database-coordinated admission across NICo process instances instead of holding a pooled transaction open; use an in-process owner or mutex for task-local exclusion.WorkLock::fence_transaction.WorkLockas an expiring lease rather than a fencing token; external effects need their own fencing, idempotency, or a reconciliation protocol proven safe when execution repeats or overlaps.Part of #4621.