Skip to content

Add idempotent authority mutation foundation#119

Merged
abiorh-claw merged 6 commits into
mainfrom
codex/ws-auth-001-05b-idempotency-invalidation
Jul 14, 2026
Merged

Add idempotent authority mutation foundation#119
abiorh-claw merged 6 commits into
mainfrom
codex/ws-auth-001-05b-idempotency-invalidation

Conversation

@Abiorh001

@Abiorh001 Abiorh001 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

WS-AUTH-001-05B PR Trust Bundle

Chunk

WS-AUTH-001-05B - Authority Idempotency And Invalidation Foundation

Goal

Make the ten sensitive authority mutations concurrency-safe and retry-safe,
with typed replay results and exactly linked success/invalidation evidence.

Human-Approved Intent

The human approved the repaired L1 contract and its semantic chunk boundary.
The work remains backend foundation only; AUTH-06 must not start automatically.

What Changed

  • Migration 0019 adds immutable actor-scoped idempotency reservations, a
    pending-to-committed transition, typed replay references, database timestamps,
    a deferred pending guard, and guarded downgrade custody.
  • Strict frozen request variants cover exactly ten mutation operations and hash
    only canonical, bounded, server-derived facts.
  • The authorization service reserves, replays, records privacy-neutral
    mismatches, validates concrete success evidence, constructs invalidations,
    and completes the claim in one caller-owned transaction.
  • PostgreSQL links actor, claim, operation, entity, resource, target, concrete
    cause, request, correlation, and project context and closes committed records
    to later evidence.
  • Behavior tests cover all ten operations plus project-grant replacement,
    exact and mismatched concurrency, rollback, privacy, direct bypasses,
    immutable state, and upgrade/downgrade races.

Why It Changed

Later actor, grant, and permission chunks need a trustworthy retry boundary.
Without it, a lost response or concurrent retry could duplicate authority
state, disclose another request's result, or leave misleading audit evidence.

Design Chosen

The unique namespace is actor kind, actor reference, operation, and client key.
Only the canonical request digest and typed response reference persist. The
existing shared audit service remains the sole application writer, while
PostgreSQL independently enforces linkage and immutability. Repositories never
commit, roll back, retry, or create sessions.

Alternatives Rejected

  • Storing raw requests or response bodies was rejected for privacy and schema
    stability.
  • Application-only idempotency and evidence validation was rejected because
    direct SQL must fail closed.
  • A parallel invalidation table was rejected because AUTH-05A already owns the
    shared authority evidence envelope.
  • Route, grant, actor, cache, queue, and consumer implementation remains in
    later owning chunks.

Scope Control

No API route, permission evaluator, actor/grant table, token verifier, cache,
queue, invalidation consumer, product authorization decision, dependency,
workflow, or CI threshold changed.

Product Behavior

There is no new externally reachable product behavior. Internal callers can
claim a mutation, receive an exact committed replay, or receive a private
mismatch signal. Completion requires one concrete success plus one linked
invalidation event.

Acceptance Criteria Proof

  • Independent concurrent retries serialize through PostgreSQL and produce one
    committed result and one evidence pair.
  • All ten operations and the project replacement branch have positive typed and
    database-linked proof.
  • Request target/project facts are bound to the success, response, and replay;
    cross-actor, wrong-entity, wrong-target, and mismatched-cause paths fail.
  • Committed rows reject new success or invalidation evidence and reject update,
    delete, and truncate.
  • Rejected secrets and malformed Unicode are absent from strings, args,
    diagnostics, public state, cause, and context.
  • Downgrade is observed waiting for an actual PostgreSQL writer lock, refuses
    non-empty custody, preserves compatible pre-0019 rows, and restores head.

Tests And Checks Run

  • Focused behavior/migration suite: 26 passed.
  • Authorization subsystem coverage: 96.88 percent.
  • Isolated-runner suite: 16 passed.
  • Ruff, 95.3 percent docstring coverage, stale contract scans, changed Markdown
    links, internal review evidence structure, and diff integrity passed or are
    rerun after this bundle is finalized.
  • A local repository-wide isolated suite reached approximately 94 percent with
    no failure before intentional interruption after two hours on an overloaded
    four-core host. GitHub Backend owns the authoritative full-suite and global
    coverage result.

Test Delta

Tests are additive or strengthened. Existing audit assertions were adapted for
mandatory idempotency linkage, and positive operation coverage replaced weaker
assertions. No raises guard, skip, xfail, coverage threshold, or test selection
was removed or weakened.

CI Integrity

No workflow, dependency, package script, exclusion, or coverage setting
changed. GitHub Backend must still enforce the repository-wide 78 percent floor
and the existing artifact-foundation 90 percent floor.

Reviewer Results

Exact runtime SHA e0838908dfa19227fab2f73e32c9b0c94dd39bb4 passed senior
engineering, architecture, product/ops, docs, reuse/dedup, QA/test, test delta,
CI integrity, and security/auth/privacy review with no remaining findings.

External Review

Pending GitHub checks, CodeRabbit, and human review.

Remaining Risks

  • Database-owner or DDL credentials can bypass normal-DML triggers; production
    must use the documented non-owner runtime role.
  • This chunk emits invalidation evidence but intentionally has no consumer.
  • Replay references are internal; future routes must reload and reauthorize the
    current resource before disclosure.

Follow-Up Work

After merge, post-merge memory, stop, and a separate human start, AUTH-06 may
implement canonical actor profiles and identity links. Later chunks own the
authorization kernel, grants, resource cutovers, and invalidation consumers.

Human Review Focus

Review the migration triggers and downgrade lock ordering, canonical digest
boundary, request-to-evidence binding, mismatch privacy, and proof that no
route or product authorization behavior entered scope.

Human Merge Ownership

Only the human may approve and merge this PR. Internal or external checks do
not authorize merge.

Summary by CodeRabbit

  • New Features

    • Added idempotent processing for authority mutations, including safe retries and replay handling.
    • Added strict validation for authorization requests and privacy-safe mismatch reporting.
    • Added linked audit evidence for successful mutations and invalidation events.
    • Added database safeguards for record integrity, immutability, concurrency, and controlled schema rollback.
  • Documentation

    • Documented authority idempotency behavior, replay handling, mismatch resolution, and operational procedures.
  • Tests

    • Added comprehensive coverage for authorization flows, audit linkage, migration safety, concurrency, rollback, and sensitive-data protection.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds authority mutation idempotency for ten operations, including strict request schemas, canonical digests, PostgreSQL-backed reservation and completion, linked audit evidence, invalidation events, mismatch denial handling, migration safeguards, integration tests, and updated workstream documentation.

Changes

Authority mutation idempotency

Layer / File(s) Summary
Canonical request and reservation contracts
.agent-loop/.../WS-AUTH-001-05B-idempotency-invalidation.md, backend/app/modules/authorization/schemas.py, .../reviews/WS-AUTH-001-05B-pr-trust-bundle.md
Defines strict operation-specific requests, bounded digest derivation, typed reservation outcomes, claim handles, response references, and privacy constraints.
Idempotency schema and database custody
backend/alembic/versions/0019_authority_idempotency.py, backend/app/modules/authorization/models.py, backend/app/db/models.py
Adds the idempotency table, constraints, audit linkage, PostgreSQL triggers, immutable completion rules, and guarded downgrade behavior.
Reservation and evidence orchestration
backend/app/modules/authorization/repository.py, backend/app/modules/authorization/service.py
Implements reservation, replay, mismatch, claim completion, operation-specific evidence validation, invalidation events, and privacy-bounded denial evidence.
Migration and audit validation
backend/tests/test_alembic.py, backend/tests/test_audit.py
Tests migration integrity, trigger enforcement, immutability, downgrade locking, forward references, and required idempotency links for authority events.
Authorization integration and concurrency coverage
backend/tests/test_authorization.py
Covers all authority operations, replay and mismatch behavior, rollback, forged claims, secret hygiene, actor isolation, evidence pairing, and concurrent retries.
Workstream status and operating documentation
.agent-loop/**, docs/architecture_data_model.md, docs/operations_authorization_service.md
Updates AUTH-05B status, review evidence, chunk gating, architecture details, and operational idempotency guidance.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant AuthorityMutationService
  participant AuthorityIdempotencyRepository
  participant PostgreSQL
  participant audit_events

  Caller->>AuthorityMutationService: Submit canonical authority mutation
  AuthorityMutationService->>AuthorityIdempotencyRepository: Reserve namespace and digest
  AuthorityIdempotencyRepository->>PostgreSQL: Insert or lock idempotency record
  PostgreSQL-->>AuthorityIdempotencyRepository: Claimed, replay, or mismatch
  AuthorityMutationService->>audit_events: Write success and invalidation evidence
  AuthorityMutationService->>AuthorityIdempotencyRepository: Complete pending claim
  AuthorityIdempotencyRepository->>PostgreSQL: Commit immutable response reference
Loading

Possibly related PRs

Suggested reviewers: abiorh-claw

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.93% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely summarizes the main change: adding an idempotent foundation for authority mutations.
Description check ✅ Passed The description covers the chunk, goal, intent, changes, scope, evidence, tests, and risks, but it omits several exact template sections.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/ws-auth-001-05b-idempotency-invalidation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@abiorh-claw abiorh-claw self-requested a review July 14, 2026 20:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agent-loop/LOOP_STATE.md:
- Around line 11-17: Synchronize the lifecycle artifacts with published PR `#119`
by removing the stale “PR publication pending” state and retaining only checks,
external review, human approval, merge, memory-update, and stop gates. Update
.agent-loop/LOOP_STATE.md lines 11-17, .agent-loop/WORK_QUEUE.md lines 7 and
64-67, CHUNK_MAP.md lines 24 and 112-114, and STATUS.md lines 62-65, 74-75, 96,
and 127-129; ensure each status, progress summary, chunk entry, proposed-next
narrative, active gate, blocker narrative, and stop condition reflects the
post-publication state.

In `@backend/app/modules/authorization/models.py`:
- Around line 19-79: Update the constraint names in
AuthorityIdempotencyRecord.__table_args__ to match the migration: rename the
replay namespace UniqueConstraint to
uq_authority_idempotency_records_replay_namespace and the id/actor reference
UniqueConstraint to uq_authority_idempotency_records_actor_reference. Also
rename the separate actor-reference CheckConstraint so it no longer conflicts
with the unique constraint, preserving the migration’s corresponding
check-constraint name.

In `@backend/tests/test_authorization.py`:
- Around line 1014-1041: Update the test cleanup around the loser tasks in the
authorization scenario to always cancel and await whichever loser task is active
before disposing winner_engine and loser_engine. Ensure this cleanup runs when
lock observation, _complete, or commit raises, while preserving the existing
ReplayedReservation and MismatchedReservation assertions on successful
execution.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 94a9771f-80d2-4175-a231-ad06d05cd43b

📥 Commits

Reviewing files that changed from the base of the PR and between eba7e2b and 83ca3e2.

📒 Files selected for processing (20)
  • .agent-loop/LOOP_STATE.md
  • .agent-loop/REVIEW_LOG.md
  • .agent-loop/WORK_QUEUE.md
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-05B-idempotency-invalidation.md
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-05B-internal-review-evidence.md
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-05B-pr-trust-bundle.md
  • backend/alembic/versions/0019_authority_idempotency.py
  • backend/app/db/models.py
  • backend/app/modules/authorization/__init__.py
  • backend/app/modules/authorization/models.py
  • backend/app/modules/authorization/repository.py
  • backend/app/modules/authorization/schemas.py
  • backend/app/modules/authorization/service.py
  • backend/tests/test_alembic.py
  • backend/tests/test_audit.py
  • backend/tests/test_authorization.py
  • docs/architecture_data_model.md
  • docs/operations_authorization_service.md

Comment thread .agent-loop/LOOP_STATE.md
Comment on lines +11 to +17
- Status: CAT post-merge memory merged through PR #118 as `eba7e2b` on
2026-07-14. `WS-AUTH-001-05B` implementation and repair are complete at
reviewed runtime SHA `e083890`; every required internal review track passed.
- Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836`
- Prior `WS-AUTH-001-01` final merged branch head: `b5217e1`
- Latest integrated `main` merge commit: `4c5d4fc`
- Current gate: CAT post-merge memory and stop; no runtime implementation chunk
is active.
- Latest integrated `main` merge commit: `eba7e2b`
- Current gate: PR publication, GitHub checks, CodeRabbit, and human review.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Synchronize all current lifecycle artifacts with published PR #119.

The same stale “PR publication pending” state appears across the active loop documents. Since the supplied PR objective identifies this review as PR #119, publication is complete; retain only the remaining checks, external review, human approval, merge, memory-update, and stop gates.

  • .agent-loop/LOOP_STATE.md#L11-L17: update the current gate and status.
  • .agent-loop/WORK_QUEUE.md#L7-L7: change the AUTH-05B status.
  • .agent-loop/WORK_QUEUE.md#L64-L67: update the proposed-next narrative.
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md#L24-L24: update the chunk status.
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md#L112-L114: update the stop-condition wording.
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L62-L65: update the progress summary.
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L74-L75: update the active implementation gate.
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L96-L96: update the chunk table.
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L127-L129: update the blocker/gate narrative.
📍 Affects 4 files
  • .agent-loop/LOOP_STATE.md#L11-L17 (this comment)
  • .agent-loop/WORK_QUEUE.md#L7-L7
  • .agent-loop/WORK_QUEUE.md#L64-L67
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md#L24-L24
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md#L112-L114
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L62-L65
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L74-L75
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L96-L96
  • .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md#L127-L129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agent-loop/LOOP_STATE.md around lines 11 - 17, Synchronize the lifecycle
artifacts with published PR `#119` by removing the stale “PR publication pending”
state and retaining only checks, external review, human approval, merge,
memory-update, and stop gates. Update .agent-loop/LOOP_STATE.md lines 11-17,
.agent-loop/WORK_QUEUE.md lines 7 and 64-67, CHUNK_MAP.md lines 24 and 112-114,
and STATUS.md lines 62-65, 74-75, 96, and 127-129; ensure each status, progress
summary, chunk entry, proposed-next narrative, active gate, blocker narrative,
and stop condition reflects the post-publication state.

Comment on lines +19 to +79
__table_args__ = (
UniqueConstraint(
"actor_ref_kind", "actor_ref", "operation", "idempotency_key",
name="replay_namespace",
),
UniqueConstraint("id", "actor_ref_kind", "actor_ref", name="actor_reference"),
CheckConstraint(
"actor_ref_kind in ('legacy_actor', 'actor_profile', 'system_principal')",
name="actor_kind",
),
CheckConstraint(
"((actor_ref_kind = 'system_principal' and actor_ref = "
"'workstream:system:bootstrap') or (actor_ref_kind <> 'system_principal' and "
"actor_ref ~ '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$'))",
name="actor_reference",
),
CheckConstraint(
"operation in ('service_actor.create', 'admin_role_grant.issue', "
"'admin_role_grant.revoke', 'project_role_grant.issue', "
"'project_role_grant.revoke', 'actor_profile.suspend', "
"'actor_profile.reactivate', 'actor_profile.deactivate', "
"'actor_identity_link.revoke', 'actor_identity_link.reactivate')",
name="operation",
),
CheckConstraint("request_digest ~ '^sha256:[0-9a-f]{64}$'", name="request_digest"),
CheckConstraint("status in ('pending', 'committed')", name="status"),
CheckConstraint(
"response_resource_version is null or response_resource_version > 0",
name="response_version",
),
CheckConstraint(
"(status = 'pending' and response_resource_type is null and "
"response_resource_id is null and response_resource_version is null and "
"response_http_status is null and committed_at is null) or "
"(status = 'committed' and response_resource_type is not null and "
"response_resource_id is not null and response_http_status is not null and "
"committed_at is not null)",
name="state_shape",
),
CheckConstraint(
"response_http_status is null or ((operation in ('service_actor.create', "
"'admin_role_grant.issue', 'project_role_grant.issue') and "
"response_http_status = 201) or (operation not in ('service_actor.create', "
"'admin_role_grant.issue', 'project_role_grant.issue') and "
"response_http_status = 200))",
name="response_status",
),
CheckConstraint(
"(operation = 'service_actor.create' and (response_resource_type is null or "
"response_resource_type = 'actor_profile')) or "
"(operation like 'admin_role_grant.%' and (response_resource_type is null or "
"response_resource_type = 'admin_role_grant')) or "
"(operation like 'project_role_grant.%' and (response_resource_type is null or "
"response_resource_type = 'project_role_grant')) or "
"(operation like 'actor_profile.%' and (response_resource_type is null or "
"response_resource_type = 'actor_profile')) or "
"(operation like 'actor_identity_link.%' and (response_resource_type is null or "
"response_resource_type = 'actor_identity_link'))",
name="response_type",
),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether any backend test fixture creates schema via ORM metadata rather than Alembic.
rg -n 'create_all|metadata\.create_all' backend/tests backend/app 2>/dev/null

Repository: Flow-Research/workstream

Length of output: 162


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== model excerpt =="
nl -ba backend/app/modules/authorization/models.py | sed -n '1,140p'

echo
echo "== migration excerpt =="
nl -ba backend/alembic/versions/0019_authority_idempotency.py | sed -n '1,220p'

echo
echo "== search for the constraint names =="
rg -n 'name="actor_reference"|name="replay_namespace"|uq_authority_idempotency_records_actor_reference|uq_authority_idempotency_records_replay_namespace' backend/app backend/alembic/versions

Repository: Flow-Research/workstream

Length of output: 222


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== model excerpt =="
sed -n '1,140p' backend/app/modules/authorization/models.py | cat -n

echo
echo "== migration excerpt =="
sed -n '1,220p' backend/alembic/versions/0019_authority_idempotency.py | cat -n

echo
echo "== search for the constraint names =="
rg -n 'name="actor_reference"|name="replay_namespace"|uq_authority_idempotency_records_actor_reference|uq_authority_idempotency_records_replay_namespace' backend/app backend/alembic/versions

Repository: Flow-Research/workstream

Length of output: 18845


Rename the model constraint names to match the migration

AuthorityIdempotencyRecord defines both a UniqueConstraint and a CheckConstraint as actor_reference, and the unique-constraint name on replay_namespace also differs from backend/alembic/versions/0019_authority_idempotency.py. Make the model use the same unique names as the migration (uq_authority_idempotency_records_replay_namespace and uq_authority_idempotency_records_actor_reference) to avoid metadata collisions and Alembic drift.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/modules/authorization/models.py` around lines 19 - 79, Update the
constraint names in AuthorityIdempotencyRecord.__table_args__ to match the
migration: rename the replay namespace UniqueConstraint to
uq_authority_idempotency_records_replay_namespace and the id/actor reference
UniqueConstraint to uq_authority_idempotency_records_actor_reference. Also
rename the separate actor-reference CheckConstraint so it no longer conflicts
with the unique constraint, preserving the migration’s corresponding
check-constraint name.

Comment on lines +1014 to +1041
try:
async with winner_factory() as winner:
service = AuthorityMutationService(winner)
claim = await _claim(service, actor, key, request)
loser = asyncio.create_task(lose(request))
await asyncio.wait_for(
_wait_for_database_lock(authorization_database_env, "auth05b-loser"),
timeout=5,
)
await _complete(service, claim, request)
await winner.commit()
assert isinstance(await asyncio.wait_for(loser, timeout=5), ReplayedReservation)

key = uuid4()
async with winner_factory() as winner:
service = AuthorityMutationService(winner)
claim = await _claim(service, actor, key, request)
loser = asyncio.create_task(lose(_request()))
await asyncio.wait_for(
_wait_for_database_lock(authorization_database_env, "auth05b-loser"),
timeout=5,
)
await _complete(service, claim, request)
await winner.commit()
assert isinstance(await asyncio.wait_for(loser, timeout=5), MismatchedReservation)
finally:
await winner_engine.dispose()
await loser_engine.dispose()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Always cancel and await the active loser task during cleanup.

If lock observation, completion, or commit fails before the explicit await, the task can retain its session and connection past engine disposal.

Proposed cleanup
+    loser_task = None
     try:
         async with winner_factory() as winner:
             ...
-            loser = asyncio.create_task(lose(request))
+            loser_task = asyncio.create_task(lose(request))
             ...
-        assert isinstance(await asyncio.wait_for(loser, timeout=5), ReplayedReservation)
+        assert isinstance(
+            await asyncio.wait_for(loser_task, timeout=5),
+            ReplayedReservation,
+        )
 
         ...
-            loser = asyncio.create_task(lose(_request()))
+            loser_task = asyncio.create_task(lose(_request()))
             ...
-        assert isinstance(await asyncio.wait_for(loser, timeout=5), MismatchedReservation)
+        assert isinstance(
+            await asyncio.wait_for(loser_task, timeout=5),
+            MismatchedReservation,
+        )
     finally:
+        if loser_task is not None:
+            if not loser_task.done():
+                loser_task.cancel()
+            await asyncio.gather(loser_task, return_exceptions=True)
         await winner_engine.dispose()
         await loser_engine.dispose()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try:
async with winner_factory() as winner:
service = AuthorityMutationService(winner)
claim = await _claim(service, actor, key, request)
loser = asyncio.create_task(lose(request))
await asyncio.wait_for(
_wait_for_database_lock(authorization_database_env, "auth05b-loser"),
timeout=5,
)
await _complete(service, claim, request)
await winner.commit()
assert isinstance(await asyncio.wait_for(loser, timeout=5), ReplayedReservation)
key = uuid4()
async with winner_factory() as winner:
service = AuthorityMutationService(winner)
claim = await _claim(service, actor, key, request)
loser = asyncio.create_task(lose(_request()))
await asyncio.wait_for(
_wait_for_database_lock(authorization_database_env, "auth05b-loser"),
timeout=5,
)
await _complete(service, claim, request)
await winner.commit()
assert isinstance(await asyncio.wait_for(loser, timeout=5), MismatchedReservation)
finally:
await winner_engine.dispose()
await loser_engine.dispose()
loser_task = None
try:
async with winner_factory() as winner:
service = AuthorityMutationService(winner)
claim = await _claim(service, actor, key, request)
loser_task = asyncio.create_task(lose(request))
await asyncio.wait_for(
_wait_for_database_lock(authorization_database_env, "auth05b-loser"),
timeout=5,
)
await _complete(service, claim, request)
await winner.commit()
assert isinstance(
await asyncio.wait_for(loser_task, timeout=5),
ReplayedReservation,
)
key = uuid4()
async with winner_factory() as winner:
service = AuthorityMutationService(winner)
claim = await _claim(service, actor, key, request)
loser_task = asyncio.create_task(lose(_request()))
await asyncio.wait_for(
_wait_for_database_lock(authorization_database_env, "auth05b-loser"),
timeout=5,
)
await _complete(service, claim, request)
await winner.commit()
assert isinstance(
await asyncio.wait_for(loser_task, timeout=5),
MismatchedReservation,
)
finally:
if loser_task is not None:
if not loser_task.done():
loser_task.cancel()
await asyncio.gather(loser_task, return_exceptions=True)
await winner_engine.dispose()
await loser_engine.dispose()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_authorization.py` around lines 1014 - 1041, Update the
test cleanup around the loser tasks in the authorization scenario to always
cancel and await whichever loser task is active before disposing winner_engine
and loser_engine. Ensure this cleanup runs when lock observation, _complete, or
commit raises, while preserving the existing ReplayedReservation and
MismatchedReservation assertions on successful execution.

@abiorh-claw abiorh-claw merged commit ad71c7e into main Jul 14, 2026
4 checks passed
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.

2 participants