Skip to content
Open
14 changes: 14 additions & 0 deletions .agent-loop/REVIEW_LOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2844,3 +2844,17 @@ inline finding. Its unexplained 37.61 percent docstring warning is
non-actionable because the unchanged repository gate independently passes the
same head at 87.6 percent against the 80 percent floor. Human review and
explicit approval of PR #178 remain.

## 2026-07-23 - WS-AUTH-001-10C Internal Implementation Review

AUTH-10C exact implementation SHA
`0d05b7096eb7a2cf7c68a1770c0b35f07d5b55df` activates the two
Project-Manager-only project-role mutations with PREP-bound lexical principal
locking, deterministic absence serialization, strict replay, immutable
qualification evidence, lifecycle-independent revoke, and typed invalidation.
The repair loop closed real named-index fallback and residue, lifecycle
attribution, database-observed cancellation rollback, committed same-key retry,
and crossed-principal ordering proof. Senior engineering, architecture,
reuse/dedup, security/auth, QA/test, test delta, product/ops, docs, and CI
integrity all pass with no open finding. GitHub full CI/coverage, hosted API E2E,
CodeRabbit, and human review remain.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@

## Status and prerequisite

Proposed and inactive. Start only after 10B merges, signed memory names 10C,
and a fresh explicit start event activates this exact child.
Active for implementation. AUTH-10B2 merged through PR #178 as
`73b457925b02301587b83d01ced0adb66319d134`. Signed automation run
`30014637065` activated this exact child on protected main
`bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Chat and local branch state are
not authority; the signed automation projection is the canonical start proof.

## Goal

Expand All @@ -32,10 +35,12 @@ P1
```text
backend/app/modules/actors/repository.py
backend/app/modules/authorization/**
backend/app/modules/audit/schemas.py
backend/app/modules/projects/repository.py
backend/app/api/router.py
backend/tests/test_actors.py
backend/tests/test_authorization.py
backend/tests/test_api_controls.py
backend/tests/test_projects.py
backend/scripts/api_contract_e2e.py
backend/scripts/auth_api_e2e.py
Expand Down Expand Up @@ -74,18 +79,71 @@ authority.

## Exact lock and transaction order

PREP first locks `AuthorityControl(id=1)` as the shared authorization-order
barrier used by administrative lifecycle mutations. It then accepts the caller
plus the known target human principal for issuance, sorts distinct ActorProfile
IDs lexically, and locks each profile then its exact active link. It finally
locks the caller's one deterministic covered Project Manager AdminRoleGrant.
The barrier is ordering coordination, not final-admin permission or safety.
The route consumes the existing admin-mutation rate dependency exactly once,
opens one root transaction, and reserves or locks the idempotency record before
any other mutable row. Reservation outcome is internal only: mismatch and replay
never branch to a response until PREP and current project authority have been
revalidated, so authority and concealment win over key-existence disclosure.
Fresh, replay, and mismatch paths use the same ordering.

PREP then locks `AuthorityControl(id=1)` as the shared authorization-order
barrier used by administrative lifecycle mutations. Issue uses a target-aware
prepared input containing the canonical target ActorProfile ID in addition to
the existing project scope and full request digest. PREP sorts the distinct
caller and target ActorProfile UUID strings lexically, locks each profile and
then its selected exact link immediately after that profile, and finally locks
the caller's one deterministic covered Project Manager AdminRoleGrant. The
caller's link is the authenticated exact link; the target link is the unique
active exact link selected deterministically for issue. Missing, inactive, or
nonhuman target facts are retained as a fail-closed prepared outcome rather
than raised immediately: PREP still locks and revalidates the caller's current
authority and the route still locks/revalidates the project before choosing the
public result. An unauthorized or concealed-project caller therefore observes
the same status, code, body, and mutation-free outcome for valid, missing,
inactive, agent, Space, or service targets, without a target-specific early
timing or response branch. All competing profile, link, and
administrative-grant mutations retain `AuthorityControl` for the transaction
lifetime. The barrier is ordering coordination, not final-admin permission or
safety. Revoke has no client-trusted target: PREP binds caller, project, grant
ID, idempotency key, and request digest; the locked grant supplies target facts
only during final resource recomposition. Revoke does not require the stored
target profile or identity link to remain active; a target profile row may be
locked only for deterministic concurrency or audit integrity, never as a
revocation admission gate.

The opaque handle binds action, caller/link, target when issue, project, role
when issue, grant ID when revoke, idempotency key, and full canonical request
digest. Consume rejects target, scope, role, grant, digest, session,
transaction, action, handle-owner, reuse, or lifecycle substitution. Forged,
copied, serialized, cross-service, cross-session, and already-consumed handles
fail closed.

After PREP returns, issue locks canonical project, then takes one transaction
advisory key for `(actor_profile_id, project_id, requested_role)` to serialize
absence, then reloads the target/scope facts and active exact-role selector.
Revoke locks canonical project then the exact grant. Consume recomposes every
fact and evaluates once; repositories flush only. The route commits once.
Revoke locks canonical project then the exact path-project grant. Consume
recomposes every fact and evaluates once; repositories flush only. The route
commits once.

The issue advisory key is a domain-separated deterministic PostgreSQL signed
`int8`. Its frozen input encoding is the compact UTF-8 JSON array
`["workstream.project_role_grant.issue.v1","<canonical-lowercase-actor-uuid>","<canonical-lowercase-project-uuid>","<role-value>"]`
with no insignificant whitespace and JSON escaping defined by the repository's
canonical serializer. SHA-256 hashes those exact bytes; the first eight digest
bytes are decoded big-endian as a two's-complement signed `int64`. Tests freeze
a known vector, exact encoding, UUID and role separation, stability, and signed
range.
The partial unique active-exact-role index remains the final backstop; a
uniqueness race becomes `project_role_grant_exists` without a second snapshot,
success event, or grant.

The complete order is: rate consumption -> root transaction -> idempotency
reserve/record lock -> AuthorityControl -> lexical caller/target profile and
exact-link locks -> deterministic covered Project Manager grant -> project ->
issue advisory key or exact revoke grant -> canonical fact reload -> consume
exactly once -> snapshot/grant/idempotency/audit/invalidation flush -> one
route-owned commit. Rollback and denial-evidence restaging never invert this
order or disclose reservation outcome before authorization.

Crossed tests prove two managers targeting one another, issue versus target
profile/link lifecycle, and issue versus caller-grant revocation cannot deadlock
Expand All @@ -95,8 +153,13 @@ because all paths share the barrier and compatible principal order.

Issue requires `Idempotency-Key: <uuid>` and a strict body containing exactly
`target_actor_profile_id`, `role`, `qualification`, and `reason`.
`qualification` contains the two exact availability objects and reference lists
defined by 10A. Revoke requires the same header and exactly `reason`. Reasons
`qualification` is identifier-free and contains only the two exact availability
objects and reference lists defined by 10A; the server composes path project,
body target, and body role into `ProjectRoleQualificationSnapshotInput`. The
full canonical bounded qualification value participates in the issue request
digest, idempotency equality, PREP binding, and snapshot validation. Changing
any qualification field with the same key is `idempotency_mismatch`. Revoke
requires the same header and exactly `reason`. Reasons
are 1..500 UTF-8 bytes, equal their Python `str.strip()` result, and contain no
Unicode control character. Issue returns HTTP 201 with exactly `{id,
qualification_snapshot_id, project_id, actor_profile_id, role, status:
Expand All @@ -117,14 +180,40 @@ same-key/same-body revoke replay reauthorizes and returns the prior canonical
HTTP 200 revoked response; a new-key revoke of an already-revoked grant returns
HTTP 409 `project_role_grant_already_revoked`.

Replay never trusts stored response metadata alone. Active issue replay locks
and reloads the canonical path project, grant, and snapshot and requires the
exact project/target/role tuple, snapshot ownership, `status=active`, and
`version=1`. Revoked, missing, or mismatched state becomes
`project_role_grant_replay_state_changed` only after current authority and
project concealment pass. Revoke replay locks and reloads the exact
path-project grant and original snapshot and requires `status=revoked`,
`version=2`, and unchanged ownership before returning the prior HTTP 200 shape.

The shared `AuthorityMutationService` remains the sole completion path. It is
extended to validate and atomically append an ordered tuple of success events
and an optional typed invalidation projection before completing idempotency.
Issue writes exactly `ProjectRoleQualificationSnapshotCaptured` followed by
`ProjectRoleGrantIssued` and creates no invalidation. Revoke writes exactly
`ProjectRoleGrantRevoked` followed by one linked
`AuthorityInvalidationRequested` containing grant, actor, project, role,
cause-event, and the role-specific future-obligation projection. No parallel
completion writer is permitted.

## Acceptance criteria

- Issue permits draft, active, and paused projects; terminal/archived,
nonexistent, and unauthorized projects deny without target disclosure.
- Revoke remains available for every existing project state so authority can
always be removed.
- Target is a different active human with active link. Agent, Space, service,
self-grant, and self-revoke deny stably before disclosure.
- Issue requires a different active canonical human target with its unique
active link. Agent, Space, service, and self-grant deny stably after current
caller/project authorization and concealment have been resolved.
- Revoke derives target identity and role only from the locked exact
path-project grant. After current covered-manager authorization, self-role
revoke returns HTTP 403 `self_role_revoke_forbidden`; an unauthorized caller
cannot learn self-ownership and receives the same concealed HTTP 404 as other
hidden grants. Target suspension or a revoked/missing target identity link
never blocks revocation, so granted authority cannot become irremovable.
- Issue creates one exact-role snapshot, one manual grant,
`ProjectRoleQualificationSnapshotCaptured`, and `ProjectRoleGrantIssued` in
one transaction. It never changes another role.
Expand All @@ -145,7 +234,9 @@ HTTP 409 `project_role_grant_already_revoked`.
invalidation, flush, and commit leave no orphan row or partial evidence.
- PostgreSQL tests cover identical-role issue, different-role issue, crossed
managers, revoke versus regrant, revoke versus authorization, replay after
authority loss, timeout, and cancellation.
authority loss, timeout, and cancellation. PostgreSQL and API tests also
prove a covered manager can revoke an active grant after the target profile
is suspended and after its identity link is revoked.
- Live API proof uses the same unexpired manager token to issue, read the active
grant through 10B, revoke, read the historical revoked grant, and prove a
same-key/same-body revoke replay returns the prior HTTP 200 response after
Expand All @@ -157,11 +248,44 @@ HTTP 409 `project_role_grant_already_revoked`.
issue replay changes to that exact closed conflict.
Contributor-capability denial remains explicitly deferred to AUTH-11/13 where
a real consumer action exists.
- Cross-project revoke (`project_id=A`, grant from B), nonexistent grant, and
unauthorized revoke share concealed 404 and create no mutation, success
audit, invalidation, or disclosure. Unauthorized issue with and without any
manager grant cannot distinguish valid, nonexistent, inactive, agent, Space,
service, or otherwise unauthorized targets: exact public status, code, and
body match and no mutation/evidence is written. Unauthorized revoke cannot
distinguish self-owned, cross-project, or nonexistent grants; only an
authorized covered manager receives the declared self-role 403.
- PostgreSQL concurrency tests use independent sessions, deterministic
synchronization that observes held and waiting locks, bounded
`asyncio.wait_for` plus statement/lock timeouts, and clean-retry proof.
Cancellation while waiting and after each staged flush, plus injected
route-commit failure, leaves no pending idempotency record, snapshot, grant,
audit, or invalidation; the route explicitly rolls back/closes and a clean
retry succeeds.
- Existing 10B read, cursor, concealment, rate-order, exact OpenAPI inventory,
and protected-action assertions remain intact. Only the two AUTH-10C rows
move from planned to active; owner and permission remain unchanged. The two
operations carry exact action IDs, consume the admin-mutation rate dependency
exactly once, and never attach authorization-read rate consumption.
- Hosted `backend/scripts/api_contract_e2e.py` provisions a distinct canonical
human target and covered project-scoped Project Manager through existing
public APIs with unexpired tokens and no direct database edits. The same
manager token proves issue -> 10B active read -> revoke -> 10B historical
read -> same-key revoke replay 200 -> new-key revoke 409 -> old issue replay
409, plus concealed denial after manager authority loss. `auth_api_e2e.py`
remains complementary local proof, not a substitute for the hosted drill.
- Regenerate exact combined route/protected-operation counts and SHA-256
inventories from current main after adding the two routes; never guess or
replace exact hashes with count-only assertions. No migration, configuration,
workflow, pytest marker, skip/xfail, command, or coverage-threshold change is
allowed. GitHub must pass all shards, hosted E2E, Agent Gates, the 78 percent
repository floor, and the 90 percent authorization-subsystem floor.

## Verification commands

```bash
(cd backend && .venv/bin/python -m ruff check app/modules/actors app/modules/authorization app/modules/projects tests/test_actors.py tests/test_authorization.py tests/test_projects.py scripts/auth_api_e2e.py)
(cd backend && .venv/bin/python -m ruff check app/modules/actors app/modules/authorization app/modules/projects tests/test_actors.py tests/test_authorization.py tests/test_api_controls.py tests/test_projects.py scripts/auth_api_e2e.py scripts/api_contract_e2e.py)
(cd backend && WORKSTREAM_TEST_ADMIN_DATABASE_URL=<admin-db> .venv/bin/python scripts/run_isolated_tests.py --metadata-json <path> --timeout-seconds 300 -- .venv/bin/python -m pytest -q tests/test_actors.py tests/test_authorization.py tests/test_projects.py -k 'project_role_grant')
(cd backend && WORKSTREAM_DATABASE_URL=<test-db> .venv/bin/python scripts/auth_api_e2e.py)
python3 scripts/check_stale_authorization_docs.py
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# Internal Review Evidence: WS-AUTH-001-10C

## Chunk And Reviewed Code

`WS-AUTH-001-10C` — Project Role Grant Mutations. Risk: L1 authorization,
concurrency, and audit. Trusted-main start base is
`bcf1292e1a591e3e84bf8ee212ee7191d80741fa`; signed start run is
`30014637065`.

Reviewed code SHA: `833dd6e5352e1774ffd8c669346c65e61e3e7ac1`
Reviewed at: 2026-07-23T16:05:00Z
Reviewer run IDs: /root/auth10b1_final_core, /root/auth10b1_final_security_qa, /root/auth10b1_final_ops_docs_ci

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reviewer session identifiers look inherited from a different chunk without a declared reassignment.

The run IDs are /root/auth10b1_final_core, /root/auth10b1_final_security_qa, /root/auth10b1_final_ops_docs_ci, but this evidence is for chunk WS-AUTH-001-10C, not 10b1. When reviewer-pool session identifiers are carried over from a different chunk/initiative, the evidence must explicitly state the sessions were reassigned, bind to the exact reviewed commit SHA, and map every reviewer track (the table lists nine tracks against three sessions). Add the reassignment statement and the track→session mapping, or use fresh auth10c_* identifiers.

Based on learnings: reused /root/…-style reviewer-pool session identifiers may be inherited across reviewers only if the evidence explicitly states the session was reassigned, binds to the exact reviewed commit SHA, and maps every reviewer track covered.

🤖 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/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10C-internal-review-evidence.md
at line 12, Update the reviewer evidence for chunk WS-AUTH-001-10C to use fresh
auth10c_* session identifiers, or explicitly document reassignment of the
existing sessions, including the exact reviewed commit SHA and a complete
mapping of all nine reviewer tracks to the three sessions.

Source: Learnings


## Implemented Contract

- Activates exactly the covered-Project-Manager issue and revoke actions and
their two strict `/api/v1/projects/.../role-grants` mutation routes.
- Binds idempotency, PREP, current caller/target/project/grant facts, lexical
principal locks, deterministic absence serialization, and one route commit.
- Persists one immutable qualification snapshot per issued exact role; revoke
remains possible after target suspension or identity-link revocation.
- Appends ordered typed success evidence and the revoke-only future-obligation
invalidation projection through the shared authority mutation completion path.
- Adds no migration, schema revision, worker, assignment, review-reconciliation,
automated grant, replacement-role, or authority-substitution behavior.

## Deterministic Evidence

```text
Focused project-role schema/advisory/rate/invalidation/cancellation tests — passed
Crossed-principal lexical profile/link order in both directions — passed
Python compile of authorization tests — passed
Ruff on changed authorization tests and implementation — passed
git diff --check — passed
Markdown links and stale wording scans — passed
```

The PostgreSQL proof covers durable issue/revoke evidence, revocation after
target lifecycle loss, the real named partial-unique-index fallback through the
public route, loser residue, observed database lock waiting, production
cancellation rollback, and a completed same-key retry. It runs in GitHub with
the repository database fixture. The repository-wide suite and coverage remain
GitHub-owned because this workstation's full run takes approximately four hours.

## Integrity And Review

No test was deleted, skipped, xfailed, or weakened. The exact OpenAPI inventory
is 76 routes with hash
`c8f9852035446ea59b0e929b1bd8c8cfc7df5bf838ceb544c04e899f90169318`
and 74 protected operations with hash
`9278d0183ffb87947ee4857e0325483ba7bf07feac0c38a88432840b10c2b0c3`.
No workflow, dependency, shard, command, threshold, or coverage setting changed.

| Reviewer | Result | Blocking findings |
|---|---|---|
| Senior engineering | PASS | none |
| Architecture | PASS | none |
| Reuse/dedup | PASS | none |
| Security/auth | PASS | none |
| QA/test | PASS | none |
| Test delta | PASS | none |
| Product/ops | PASS | none |
| Docs | PASS | none |
| CI integrity | PASS | none |

The repair loop closed lifecycle attribution, real unique-index fallback,
complete residue, database-observed cancellation, committed retry, and lexical
crossed-principal proof gaps.

Open sub-agent sessions: none
Valid findings addressed: yes
Loading
Loading