AUTH-12C: activate project creation authority - #229
Merged
Conversation
📝 WalkthroughWalkthroughProject creation now uses active, system-scoped ChangesProject creation authorization
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant ProjectCreateRouter
participant ProjectCreateService
participant ProjectCreateRepository
participant AuthorizationService
Client->>ProjectCreateRouter: POST /api/v1/projects with Idempotency-Key
ProjectCreateRouter->>ProjectCreateService: create request
ProjectCreateService->>ProjectCreateRepository: reserve actor-scoped request
ProjectCreateService->>AuthorizationService: consume system-scoped PROJECT_CREATE
ProjectCreateService->>ProjectCreateRepository: complete reservation
ProjectCreateRouter-->>Client: ProjectResponse or replayed response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
Abiorh001
marked this pull request as ready for review
July 30, 2026 18:35
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
backend/tests/test_audit.py (1)
187-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIndentation of these three set members is off by four spaces relative to the rest of the literal. Harmless at runtime, but it makes the added entries look nested.
🤖 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_audit.py` around lines 187 - 189, Align the three ActionId members PROJECT_ACTIVE_GUIDE_READ, PROJECT_CREATE, and PROJECT_READ with the other members in the surrounding set literal by removing the extra four spaces of indentation.backend/tests/test_authorization.py (1)
3456-3465: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead
PROJECT_CREATEbranches remain after thecontinue. With the skip at 3456-3457, the conditionals at 3459-3464 can never take thePROJECT_CREATEpath; they can be collapsed to the project scope.♻️ Suggested simplification
if action_id is ActionId.PROJECT_CREATE: continue scope = PreparedAuthorityScope( - kind=( - PreparedAuthorityScopeKind.SYSTEM - if action_id is ActionId.PROJECT_CREATE - else PreparedAuthorityScopeKind.PROJECT - ), - project_id=None if action_id is ActionId.PROJECT_CREATE else project_id, + kind=PreparedAuthorityScopeKind.PROJECT, + project_id=project_id, )🤖 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 3456 - 3465, Remove the unreachable PROJECT_CREATE branches from the PreparedAuthorityScope construction after the existing skip in the authorization test, and always use PreparedAuthorityScopeKind.PROJECT with the existing project_id value.backend/tests/project_create_fixtures.py (3)
38-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePre-cutover branch returns a transient
Projectthat is not attached to the session.The row is inserted via raw SQL, but the returned object is a detached ORM instance with no identity in the session. Callers that only read
project_idare fine, but any caller that mutates the return value (or expectssession.get(Project, id)to return the same object) will silently diverge from the post-cutover branch, which returns a session-managed instance. Consider returning the persisted instance (e.g.await session.get(Project, project_id)guarded by the historical column set) or documenting the contract difference in the docstring.🤖 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/project_create_fixtures.py` around lines 38 - 55, Update the pre-cutover branch of the project fixture helper to return the persisted, session-managed Project instance after the raw SQL insert, using session.get(Project, project_id) while preserving the historical column set. Ensure the returned object matches the post-cutover branch’s session identity behavior rather than the transient instance currently constructed locally.
129-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
UUID(project_id)makes the post-cutover branch stricter than the pre-cutover branch.The historical path accepts any 36-char string, while this path raises
ValueErrorfor non-UUID ids. All current call sites passstr(uuid4()), so this is only a latent trap for future callers; a short docstring note or an explicitUUID(project_id)validation up front would keep both branches consistent.🤖 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/project_create_fixtures.py` around lines 129 - 137, Document the UUID requirement for project_id in the fixture path around ProjectCreateResourceContext, or validate it explicitly before constructing the resource so invalid identifiers fail consistently with the post-cutover behavior. Keep the existing requested_project_id assignment for valid UUID strings and ensure both historical and post-cutover branches enforce the same contract.
105-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe existing-grant lookup can never hit for a freshly minted actor.
actor_id/linkare created with a newuuid4()on every call (Lines 56-79), so theproject_managerlookup at Lines 105-112 always returnsNoneand a new grant is always inserted. The branch is dead work; either drop the query or reuse an existing fixture actor when one is present.🤖 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/project_create_fixtures.py` around lines 105 - 127, The existing-grant lookup in the project fixture setup is unreachable because each call creates a fresh actor via the actor/link creation flow. Remove the grant query and conditional reuse, or change the fixture setup to reuse an existing actor when available; ensure the project-manager grant is created only when needed without retaining dead lookup logic..agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md (1)
162-184: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRuff/coverage command lists omit
app/modules/audit/schemas.py.It's in the allowed-files list (line 43) and is materially changed by this chunk, but it is neither linted here nor included in the coverage
--source, while the criteria at lines 153-154 require every changed module to be covered.♻️ Proposed adjustment
app/modules/authorization/runtime.py app/api/deps/authorization.py \ + app/modules/audit/schemas.py \ alembic/versions/0044_project_create_authority.py \- --source=app.modules.projects,app.modules.authorization \ + --source=app.modules.projects,app.modules.authorization,app.modules.audit \🤖 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/chunks/WS-AUTH-001-12C-project-create.md around lines 162 - 184, Update the Ruff command and coverage configuration in this validation block to include app/modules/audit/schemas.py, adding it to the linted file list and its module path to coverage --source and --include patterns so every changed allowed module is linted and covered.backend/app/modules/authorization/prepared.py (1)
162-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the project-create identity comparison.
The same three-field binding/resource comparison is duplicated in
consumeanddeny_unsupported; a small predicate keeps them from drifting.♻️ Suggested helper
`@staticmethod` def _project_create_binding_matches( binding: _PreparedAuthorizationBinding, resource: ProjectCreateResourceContext, ) -> bool: return ( binding.project_create_operation_id == resource.resource_id and binding.project_create_project_id == resource.requested_project_id and binding.project_create_generation == resource.operation_generation )Also applies to: 191-197
🤖 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/prepared.py` around lines 162 - 169, Extract the repeated project-create identity comparison into a shared predicate, such as _project_create_binding_matches, accepting _PreparedAuthorizationBinding and ProjectCreateResourceContext and returning whether all three fields match. Replace the inline comparisons in both consume and deny_unsupported with this helper, preserving the existing invalid-handle behavior when it returns false.
🤖 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 `@backend/alembic/versions/0044_project_create_authority.py`:
- Around line 239-261: Update the custody validation in the project-create
trigger so every nullable provenance and evidence comparison uses NULL-safe IS
DISTINCT FROM semantics instead of <>. Apply this to the reservation/project
checks and the evidence fields in the blocks that raise “project create custody
mismatch” and “project create evidence mismatch,” while preserving the existing
expected-value validations and exception behavior.
- Around line 111-120: Update the Alembic migration’s check-constraint
operations for the constraint shown to use the logical name suffix
“creation_authority_shape” rather than the convention-rendered
“ck_projects_creation_authority_shape”. Apply the same logical name to both
create and drop operations so Alembic’s naming convention adds the prefix
exactly once.
In `@backend/app/modules/projects/repository.py`:
- Around line 101-128: The idempotency reservation flow in the repository method
must handle concurrent same-key requests without raising
ProjectRepositoryIntegrityError. Replace the on_conflict_do_nothing path on the
ProjectCreateIdempotencyRecord insert with a no-op on_conflict_do_update that
targets the same actor_profile_id, action_id, and idempotency_key constraint and
returns the existing row ID, causing competing requests to wait for the first
writer; preserve the claimed result for new records and the documented
idempotency_pending result for conflicts.
In `@backend/app/modules/projects/router.py`:
- Around line 173-183: Update the IntegrityError handling around the existing
constraint_name extraction to support asyncpg, psycopg2, and direct
constraint_name shapes by falling back through
exc.orig.__cause__.constraint_name, exc.orig.constraint_name, and
exc.orig.diag.constraint_name. Retain the rollback and re-raise behavior for
unrelated constraints, while ensuring the allowed slug-conflict set includes
ix_projects_slug.
In `@backend/tests/project_create_fixtures.py`:
- Around line 138-161: The project seed fixture creates authority audit events
and system-scoped grants that invalidate absolute-count assertions. In
backend/tests/project_create_fixtures.py:138-161, add an emit_evidence opt-out
to seed_authorized_project and skip both side effects when disabled; in
backend/tests/test_artifact_admission.py:3661-3687 and
backend/tests/test_artifact_recovery.py:180-185, convert audit-event checks to
baseline deltas or appropriate filters; in backend/tests/test_auth.py:1799-1810,
adjust system-scoped grant totals to account for the two fixture-created
project_manager grants.
In `@backend/tests/test_alembic.py`:
- Around line 2181-2192: Update the privacy constraint check in the async
inspection flow to handle a None result from connection.scalar before testing
for substrings. Preserve the existing true condition when the constraint
definition contains both required terms, while returning false when the
constraint is missing so the assertion reports clearly.
In `@backend/tests/test_audit.py`:
- Around line 204-222: Move
test_project_create_audit_event_binds_operation_to_future_project so it appears
after test_action_aware_audit_input_enforces_mapping_and_action_availability has
fully ended, including its artifact digest, invalid-digest, unknown-action, and
ADMIN_ROLE_GRANT_ISSUE_DENIED assertions. Ensure those assertions remain owned
by the mapping test, and keep the project-create test as a separate function
containing only its project binding assertions.
In `@backend/tests/test_projects.py`:
- Around line 8761-8766: Update the pending-reservation assertion in the project
creation test to count all pending ProjectCreateIdempotencyRecord rows for this
slug’s attempts, rather than filtering by project_id != first.json()["id"]. Keep
the assertion expecting zero so NULL project_id reservations are included and
leaked pending rows are detected.
---
Nitpick comments:
In
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md:
- Around line 162-184: Update the Ruff command and coverage configuration in
this validation block to include app/modules/audit/schemas.py, adding it to the
linted file list and its module path to coverage --source and --include patterns
so every changed allowed module is linted and covered.
In `@backend/app/modules/authorization/prepared.py`:
- Around line 162-169: Extract the repeated project-create identity comparison
into a shared predicate, such as _project_create_binding_matches, accepting
_PreparedAuthorizationBinding and ProjectCreateResourceContext and returning
whether all three fields match. Replace the inline comparisons in both consume
and deny_unsupported with this helper, preserving the existing invalid-handle
behavior when it returns false.
In `@backend/tests/project_create_fixtures.py`:
- Around line 38-55: Update the pre-cutover branch of the project fixture helper
to return the persisted, session-managed Project instance after the raw SQL
insert, using session.get(Project, project_id) while preserving the historical
column set. Ensure the returned object matches the post-cutover branch’s session
identity behavior rather than the transient instance currently constructed
locally.
- Around line 129-137: Document the UUID requirement for project_id in the
fixture path around ProjectCreateResourceContext, or validate it explicitly
before constructing the resource so invalid identifiers fail consistently with
the post-cutover behavior. Keep the existing requested_project_id assignment for
valid UUID strings and ensure both historical and post-cutover branches enforce
the same contract.
- Around line 105-127: The existing-grant lookup in the project fixture setup is
unreachable because each call creates a fresh actor via the actor/link creation
flow. Remove the grant query and conditional reuse, or change the fixture setup
to reuse an existing actor when available; ensure the project-manager grant is
created only when needed without retaining dead lookup logic.
In `@backend/tests/test_audit.py`:
- Around line 187-189: Align the three ActionId members
PROJECT_ACTIVE_GUIDE_READ, PROJECT_CREATE, and PROJECT_READ with the other
members in the surrounding set literal by removing the extra four spaces of
indentation.
In `@backend/tests/test_authorization.py`:
- Around line 3456-3465: Remove the unreachable PROJECT_CREATE branches from the
PreparedAuthorityScope construction after the existing skip in the authorization
test, and always use PreparedAuthorityScopeKind.PROJECT with the existing
project_id value.
🪄 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: d8bcc265-377c-4c4c-a287-23d5acfcc74d
📒 Files selected for processing (27)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md.github/workflows/backend.ymlbackend/alembic/versions/0044_project_create_authority.pybackend/app/api/deps/authorization.pybackend/app/modules/audit/schemas.pybackend/app/modules/authorization/catalogue.pybackend/app/modules/authorization/kernel.pybackend/app/modules/authorization/prepared.pybackend/app/modules/authorization/runtime.pybackend/app/modules/projects/models.pybackend/app/modules/projects/repository.pybackend/app/modules/projects/router.pybackend/app/modules/projects/service.pybackend/scripts/api_contract_e2e.pybackend/tests/conftest.pybackend/tests/project_create_fixtures.pybackend/tests/test_alembic.pybackend/tests/test_api_controls.pybackend/tests/test_artifact_admission.pybackend/tests/test_artifact_recovery.pybackend/tests/test_audit.pybackend/tests/test_auth.pybackend/tests/test_authorization.pybackend/tests/test_guide_bindings.pybackend/tests/test_projects.pydocs/operations_authorization_service.mddocs/spec_authorization_service.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/modules/projects/router.py (1)
169-172: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFinalize the transaction before raising service errors.
reserve_project_create()performs a write before it can returnidempotency_pendingoridempotency_mismatch, but this branch raises without a rollback. Explicitly roll back before propagating non-success service errors; if denied audit evidence must persist, commit that narrowly and test it rather than relying on dependency teardown.Proposed fix
except PermissionDenied as exc: + await session.rollback() raise permission_http_error(exc) from exc except ProjectServiceError as exc: + await session.rollback() raise project_http_error(exc) from exc🤖 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/projects/router.py` around lines 169 - 172, Update the error-handling path around reserve_project_create so every non-success ProjectServiceError finalizes its transaction before project_http_error is raised, including idempotency_pending and idempotency_mismatch; roll back ordinary failures, while preserving denied-audit evidence only through an explicit narrow commit, and cover both behaviors with tests.
🧹 Nitpick comments (1)
backend/tests/test_projects.py (1)
705-730: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse the shared
grant_system_project_managerhelper for both grants.The admission call is aligned with
task_client/checker_client, butproject_clientstill manually creates both theaccess_administratorbootstrap and the scopedproject_managergrant. Delegate those steps tobackend/tests/project_create_fixtures.py:grant_system_project_manager, including itsAuthorityControl.bootstrap_grant_idhandling, so this fixture cannot drift from the grant/evidence semantics used by the shared fixtures.🤖 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_projects.py` around lines 705 - 730, The project_client fixture should stop manually creating the access_administrator bootstrap and project_manager grant. Replace that setup with the shared grant_system_project_manager helper from project_create_fixtures.py, using its returned actor/link/grant values as needed and preserving AuthorityControl.bootstrap_grant_id handling so both grants follow shared grant/evidence semantics.
🤖 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.
Outside diff comments:
In `@backend/app/modules/projects/router.py`:
- Around line 169-172: Update the error-handling path around
reserve_project_create so every non-success ProjectServiceError finalizes its
transaction before project_http_error is raised, including idempotency_pending
and idempotency_mismatch; roll back ordinary failures, while preserving
denied-audit evidence only through an explicit narrow commit, and cover both
behaviors with tests.
---
Nitpick comments:
In `@backend/tests/test_projects.py`:
- Around line 705-730: The project_client fixture should stop manually creating
the access_administrator bootstrap and project_manager grant. Replace that setup
with the shared grant_system_project_manager helper from
project_create_fixtures.py, using its returned actor/link/grant values as needed
and preserving AuthorityControl.bootstrap_grant_id handling so both grants
follow shared grant/evidence semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9436e899-2979-4aa1-8500-4253a36d7b4b
📒 Files selected for processing (16)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12C-external-review-response.mdbackend/alembic/versions/0044_project_create_authority.pybackend/app/modules/authorization/prepared.pybackend/app/modules/projects/repository.pybackend/app/modules/projects/router.pybackend/tests/conftest.pybackend/tests/project_create_fixtures.pybackend/tests/test_alembic.pybackend/tests/test_artifact_admission.pybackend/tests/test_audit.pybackend/tests/test_auth.pybackend/tests/test_authorization.pybackend/tests/test_checkers.pybackend/tests/test_projects.pybackend/tests/test_tasks.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/tests/test_authorization.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/test_alembic.py (1)
2269-2273: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that the historical project survived the upgrade.
At Line 2270, the migration runs, but no assertion verifies
historical_idstill exists; the laterDELETEis a no-op if migration 0044 removed it. A destructive migration would therefore pass this test.Proposed test tightening
asyncio.run(seed_historical()) command.upgrade(config, "head") + async def assert_historical_survives() -> None: + engine = create_async_engine(isolated_database_env) + try: + async with engine.connect() as connection: + assert await connection.scalar( + text("select exists(select 1 from projects where id=:id)"), + {"id": historical_id}, + ) + finally: + await engine.dispose() + + asyncio.run(assert_historical_survives()) asyncio.run( _assert_0044_rejects_new_unattributed_project(isolated_database_env) )🤖 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_alembic.py` around lines 2269 - 2273, Update the migration test around seed_historical and command.upgrade to explicitly verify that the historical project identified by historical_id still exists after upgrading to head, before invoking _assert_0044_rejects_new_unattributed_project. Keep the existing rejection assertion unchanged.
🤖 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.
Outside diff comments:
In `@backend/tests/test_alembic.py`:
- Around line 2269-2273: Update the migration test around seed_historical and
command.upgrade to explicitly verify that the historical project identified by
historical_id still exists after upgrading to head, before invoking
_assert_0044_rejects_new_unattributed_project. Keep the existing rejection
assertion unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 083ed569-ee23-4c42-91ab-76d44c877686
📒 Files selected for processing (12)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12C-external-review-response.mdbackend/tests/project_create_fixtures.pybackend/tests/test_alembic.pybackend/tests/test_artifact_admission.pybackend/tests/test_artifact_internal_authorization.pybackend/tests/test_artifact_recovery.pybackend/tests/test_auth.pybackend/tests/test_authorization.pybackend/tests/test_guide_bindings.pybackend/tests/test_outbox.pybackend/tests/test_projects.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12C-external-review-response.md
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/modules/projects/create_router.py (1)
105-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the constraint-name lookup into one shared helper.
The three-shape extraction (asyncpg
__cause__, direct attribute, psycopgdiag) is now duplicated in this module and inbackend/app/modules/authorization/project_role_service.py. Move it to one shared helper and call it from both places. That keeps future driver shapes in a single location.♻️ Proposed shape
# backend/app/db/errors.py def integrity_constraint_name(exc: IntegrityError) -> str | None: """Resolve the violated constraint across supported PostgreSQL drivers.""" orig = exc.orig return ( getattr(getattr(orig, "__cause__", None), "constraint_name", None) or getattr(orig, "constraint_name", None) or getattr(getattr(orig, "diag", None), "constraint_name", None) )- constraint_name = getattr( - getattr(exc.orig, "__cause__", None), "constraint_name", None - ) or getattr(exc.orig, "constraint_name", None) - if constraint_name is None: - constraint_name = getattr( - getattr(exc.orig, "diag", None), "constraint_name", None - ) + constraint_name = integrity_constraint_name(exc)🤖 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/projects/create_router.py` around lines 105 - 119, Extract the duplicated three-shape constraint lookup from the create-router IntegrityError handling and project-role service into a shared integrity_constraint_name helper in the database errors module. Update both callers to use this helper while preserving the existing constraint validation and re-raise behavior.
🤖 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.
Nitpick comments:
In `@backend/app/modules/projects/create_router.py`:
- Around line 105-119: Extract the duplicated three-shape constraint lookup from
the create-router IntegrityError handling and project-role service into a shared
integrity_constraint_name helper in the database errors module. Update both
callers to use this helper while preserving the existing constraint validation
and re-raise behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df5ebf1e-24bd-469e-a062-a9c1638d271c
📒 Files selected for processing (11)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12C-external-review-response.md.github/workflows/backend.ymlbackend/app/api/router.pybackend/app/modules/projects/create_repository.pybackend/app/modules/projects/create_router.pybackend/app/modules/projects/create_service.pybackend/app/modules/projects/router.pybackend/app/modules/projects/service.pybackend/tests/test_authorization.pybackend/tests/test_projects.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/backend.yml
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12C-project-create.md
abiorh-claw
self-requested a review
July 31, 2026 05:35
abiorh-claw
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chunk
WS-AUTH-001-12C — project.create
Goal
Activate one fail-closed, transaction-bound path for system-scoped Project Managers to create projects.
Human-approved intent
Continue AUTH-11/12 while preserving the landed ART guide-ingestion authorization. Contributor task-submission authorization remains owned by its later dedicated chunks.
What changed
project.createin the closed authorization catalogue.Idempotency-Keybefore actor provisioning.Scope control
This PR does not activate project update/archive, guide mutations, contributor task submission, submission preparation, or generic artifact access.
Product behavior
Only an active human with an active identity link and effective system-scoped Project Manager
project.creategrant can create a project. Exact committed retries return the same project. Changed, copied, cross-actor, stale, revoked-new-request, service-actor, contributor, or project-scoped attempts deny safely.Authorization design
The existing opaque
PreparedAuthorizationHandlebinds actor, identity link, system grant, action, request digest, idempotency key, operation identity and generation, requested project, session, and transaction. Allowed decision evidence, idempotency completion, and project provenance commit atomically.Verification
b9f3a99b1bbf242684407466b45044f4327e767f, current withmain.Internal reviewers
Architecture/senior PASS; security PASS; QA PASS; product operations PASS; CI integrity PASS; docs PASS; test delta PASS; reuse/dedup PASS WITH LOW RISKS. All valid findings were fixed.
External review
CodeRabbit reviewed the complete project-create boundary through
6fd23074, raised one valid deduplication comment, and that comment is fixed byab7584f4with focused tests and internal re-review. The subsequent exact-head automatic attempt is currently rate-limited; this is not represented as a fresh CodeRabbit review. There are zero unresolved, non-outdated review threads.Remaining risks
No known code or CI blocker. GitHub reports
REVIEW_REQUIRED; a human approval is still required.Follow-up work
Later AUTH chunks activate remaining project/guide mutations. Contributor submission authorization remains separate and will connect task submission through the ART submission-bundle boundary.
Human review focus
Review migration custody triggers, exact replay semantics, denial evidence after rollback, system-scope-only authority, and atomic project provenance.
Human merge ownership
A human must approve and merge this specific PR.