Activate unified guide compilation authorization - #312
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughActivates project-guide compilation request and execution authorization. The change adds typed resource contexts, digest binding, exact project grants, fixed-service execution checks, guarded migration support, ownership records, documentation, CI lane updates, and contract tests. ChangesAUTH-12I compilation authorization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectManager
participant ProjectGuideCompilationAuthorizationAdapter
participant PreparedAuthorizationService
participant ProjectSetupService
ProjectManager->>ProjectGuideCompilationAuthorizationAdapter: prepare and consume request authorization
ProjectGuideCompilationAuthorizationAdapter->>PreparedAuthorizationService: bind grant, facts, resource, and digest
ProjectSetupService->>ProjectGuideCompilationAuthorizationAdapter: request execution preflight
ProjectGuideCompilationAuthorizationAdapter->>PreparedAuthorizationService: prepare and consume persist authorization
Possibly related PRs
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/app/modules/authorization/catalogue.py (1)
946-990: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRestore the exactness of the service-matrix check.
The previous whole-dictionary equality rejected any identity whose action set differed, including an identity mapped to an empty
frozenset. The flattened membership comparison cannot detect that case: an identity with zero actions contributes no pairs, and the identity check at line 1081 still passes.Add a per-identity emptiness guard so the closed matrix stays exact.
♻️ Proposed guard
if set(rows) != SERVICE_IDENTITIES: raise RuntimeError("service action matrix identity mismatch") + if any(not actions for actions in rows.values()): + raise RuntimeError("service action matrix row mismatch") if frozenset((i, a) for i, actions in rows.items() for a in actions) != _EXPECTED_SERVICE_ACTION_MEMBERSHIPS: raise RuntimeError("service action matrix row mismatch")Also applies to: 1083-1083
🤖 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/catalogue.py` around lines 946 - 990, Update the service-matrix validation around _index_service_actions and its identity check to explicitly reject any identity whose mapped action set is empty when the expected matrix requires actions. Preserve the flattened membership comparison, but add a per-identity emptiness guard so empty frozenset mappings cannot pass as exact matches.backend/app/modules/authorization/repository.py (1)
342-342: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMake
exact_project_scopefail closed whenscope_project_idisNone.If a caller passes
exact_project_scope=Truewithscope_project_id=None, both branches are skipped.scope_guardthen staysAdminRoleGrant.scope_type == "system", so the query returns a system-scope grant for a caller that requested an exact project scope.The current kernel path is safe, because
_prepare_prelockedrejectsscope.project_id is Nonebefore the call and re-checks the matched grant scope. The repository contract is still fail-open for future callers. Reject the combination instead.🛡️ Proposed guard
scope_guard = AdminRoleGrant.scope_type == "system" - if exact_project_scope and scope_project_id is not None: + if exact_project_scope: + if scope_project_id is None: + raise ValueError("exact project scope requires one project identifier") scope_guard = and_( AdminRoleGrant.scope_type == "project", AdminRoleGrant.scope_project_id == str(scope_project_id), ) elif not system_scope_only and scope_project_id is not None:Also applies to: 354-359
🤖 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/repository.py` at line 342, Update the repository method parameterized by exact_project_scope, including its scope_guard logic, to reject or raise when exact_project_scope is True and scope_project_id is None. Ensure this validation occurs before query construction so the system-scope default cannot be returned for an exact project-scope request.
🤖 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/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12I-unified-compilation-activation.md:
- Around line 220-231: Update the coverage verification gate and its reported
trust-bundle result to measure the complete materially changed AUTH surface,
including the AUTH kernel and PREP enforcement modules rather than only
app.modules.authorization.guide_compilation. Use a changed-surface coverage
configuration if available, and preserve the required 90-percent threshold.
In
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12I-pr-trust-bundle.md:
- Around line 22-23: Update the migration reference in the trust bundle summary
from 0063_compilation_authority to 0063_guide_compilation_authority, matching
the activation contract’s implemented migration identifier.
In `@docs/spec_authorization_service.md`:
- Around line 1090-1101: Update the `project.guide_compilation.execute`
pre-provider check description to state that it writes no authorization evidence
for either successful or denied outcomes, not merely that it avoids writing
allowed evidence. Preserve the existing validation and post-provider PREP
behavior.
---
Nitpick comments:
In `@backend/app/modules/authorization/catalogue.py`:
- Around line 946-990: Update the service-matrix validation around
_index_service_actions and its identity check to explicitly reject any identity
whose mapped action set is empty when the expected matrix requires actions.
Preserve the flattened membership comparison, but add a per-identity emptiness
guard so empty frozenset mappings cannot pass as exact matches.
In `@backend/app/modules/authorization/repository.py`:
- Line 342: Update the repository method parameterized by exact_project_scope,
including its scope_guard logic, to reject or raise when exact_project_scope is
True and scope_project_id is None. Ensure this validation occurs before query
construction so the system-scope default cannot be returned for an exact
project-scope request.
🪄 Autofix
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: fde5a67b-5744-4a8e-a301-ccbc7ebeb615
📒 Files selected for processing (41)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12I-unified-compilation-activation.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12I-pr-trust-bundle.md.agent-loop/initiatives/WS-AUTH-003-module-boundary-recovery/TEST_STRUCTURE_DEBT.json.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/STATUS.md.ci/behavior-ownership/auth/authorization-audit-domain.json.ci/behavior-ownership/auth/project-create-domain.json.ci/behavior-ownership/auth/project-guide-compilation-adapter.json.ci/behavior-ownership/auth/project-guide-compilation-domain.json.ci/behavior-ownership/auth/project-guide-compilation-facts.json.ci/behavior-ownership/auth/project-guide-compilation-prepared-domain.json.ci/behavior-ownership/auth/project-guide-compilation-service-domain.json.ci/behavior-ownership/partition.v1.jsonbackend/alembic/versions/0063_guide_compilation_authority.pybackend/app/modules/audit/schemas.pybackend/app/modules/authorization/api/__init__.pybackend/app/modules/authorization/api/project_guide_compilation.pybackend/app/modules/authorization/catalogue.pybackend/app/modules/authorization/domain/__init__.pybackend/app/modules/authorization/domain/audit.pybackend/app/modules/authorization/domain/guide_compilation.pybackend/app/modules/authorization/domain/prepared_compilation.pybackend/app/modules/authorization/domain/prepared_service.pybackend/app/modules/authorization/domain/project_create.pybackend/app/modules/authorization/guide_compilation.pybackend/app/modules/authorization/kernel.pybackend/app/modules/authorization/policy.pybackend/app/modules/authorization/prepared.pybackend/app/modules/authorization/repository.pybackend/app/modules/authorization/runtime.pybackend/scripts/behavior_ownership.pybackend/scripts/run_test_lanes.pybackend/tests/authorization/__init__.pybackend/tests/authorization/guide_compilation/__init__.pybackend/tests/authorization/guide_compilation/test_adapter_contract.pybackend/tests/authorization/guide_compilation/test_domain_contract.pybackend/tests/authorization/guide_compilation/test_migration_contract.pybackend/tests/test_alembic.pybackend/tests/test_authorization.pydocs/operations_authorization_service.mddocs/spec_authorization_service.md
…-unified-compilation-activation # Conflicts: # .ci/behavior-ownership/partition.v1.json # backend/scripts/behavior_ownership.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests/projects/guide_compilation/test_migration_contract.py`:
- Around line 78-89: Extend the existing migration_lock() scope in the migration
contract test to include the intermediate-state assertion for _schema_state and
the final upgrade to head. Ensure the downgrade, both assertions, and upgrade
execute within one lock scope, removing any gap where another migration test can
modify the shared database.
🪄 Autofix
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: 635106ef-72ad-4814-8f5f-9326a5689c92
📒 Files selected for processing (14)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12I-unified-compilation-activation.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12I-pr-trust-bundle.md.agent-loop/initiatives/WS-AUTH-003-module-boundary-recovery/TEST_STRUCTURE_DEBT.json.ci/behavior-ownership/partition.v1.jsonbackend/alembic/versions/0063_guide_compilation_authority.pybackend/app/modules/authorization/repository.pybackend/scripts/behavior_ownership.pybackend/scripts/run_test_lanes.pybackend/tests/authorization/guide_compilation/test_adapter_contract.pybackend/tests/authorization/guide_compilation/test_migration_contract.pybackend/tests/conftest.pybackend/tests/projects/guide_compilation/test_migration_contract.pybackend/tests/test_alembic.pydocs/spec_authorization_service.md
🚧 Files skipped from review as they are similar to previous changes (8)
- backend/scripts/behavior_ownership.py
- .ci/behavior-ownership/partition.v1.json
- backend/app/modules/authorization/repository.py
- backend/scripts/run_test_lanes.py
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-12I-pr-trust-bundle.md
- backend/tests/authorization/guide_compilation/test_adapter_contract.py
- backend/tests/test_alembic.py
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12I-unified-compilation-activation.md
|
Temporarily closing and reopening to retrigger missing GitHub Actions check suites for exact head 9e0a412; no code or gate change. |
…-unified-compilation-activation # Conflicts: # .agent-loop/initiatives/WS-AUTH-003-module-boundary-recovery/TEST_STRUCTURE_DEBT.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/scripts/validate_test_lane_evidence.py`:
- Around line 573-575: Update the type annotation for isolation_namespaces to
list[tuple[str, str, str, str, str]] so it matches the five-element tuples
appended by the existing namespace_fields expression.
🪄 Autofix
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: 5bef56f3-33eb-4348-83e8-a85e8cd44ba0
📒 Files selected for processing (18)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12I-unified-compilation-activation.md.agent-loop/initiatives/WS-AUTH-003-module-boundary-recovery/TEST_STRUCTURE_DEBT.json.github/workflows/backend.ymlbackend/app/modules/authorization/admin_schemas.pybackend/scripts/run_isolated_tests.pybackend/scripts/run_test_lanes.pybackend/scripts/validate_test_lane_evidence.pybackend/tests/projects/guide_compilation/test_migration_contract.pybackend/tests/test_audit.pybackend/tests/test_auth.pybackend/tests/test_authorization.pybackend/tests/test_ci_test_lanes.pybackend/tests/test_isolated_database_runner.pybackend/tests/test_merge_test_lane_evidence.pybackend/tests/test_test_lane_evidence.pydocs/operations_backend_testing.mddocs/spec_authorization_service.mdscripts/test_lightweight_agent_gates.py
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/spec_authorization_service.md
- backend/tests/projects/guide_compilation/test_migration_contract.py
- .agent-loop/initiatives/WS-AUTH-003-module-boundary-recovery/TEST_STRUCTURE_DEBT.json
- backend/tests/test_authorization.py
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12I-unified-compilation-activation.md
…-unified-compilation-activation # Conflicts: # .ci/behavior-ownership/partition.v1.json
Workstream PR Trust Bundle
Chunk
WS-AUTH-001-12I- Unified Compilation Authorization ActivationGoal
Activate only
project.guide_compilation.requestfor an exact-project ProjectManager and
project.guide_compilation.executefor the fixedworkstream.project.setupservice, while leaving POL's hidden compilationworkflow inactive until WS-POL-003-03B composes it.
Human-approved intent
Continue AUTH-12 after the ART/AUTH prerequisites, preserve strict module
boundaries, avoid local full-suite execution, and use hosted GitHub Backend
lanes for repository-wide coverage.
What changed
parity plus migration
0063_compilation_authority.final PREP with AUTH-verified result digest.
debt and preserving the cross-module import ledger.
downgrade-refusal proof.
Why it changed
POL-03B must not call a provider or persist an accepted compilation until AUTH
can prove the exact current human request and fixed-service execution authority.
Design chosen
The existing opaque PREP protocol remains the sole durable authorization path.
Preflight validates the complete typed attempt context but issues no handle and
stages no evidence. Final persistence uses a new transaction and a single-use
handle whose result digest AUTH recomputes. POL-03B retains atomic product
idempotency custody; AUTH does not add a competing durable replay protocol.
Alternatives rejected
provider I/O.
Scope control
No route, worker, provider call, prompt, product row, checker, ART, REV, task,
submission, or guide-activation behavior is added. Only the two 12I actions are
activated. The allowed-file contract was kept explicit.
Product behavior
A covered PM may authorize dispatch/recovery for one immutable compilation
context. Only
workstream.project.setupmay pass exact preflight and authorizeaccepted-result persistence. The workflow remains hidden until POL-03B wires
the port and product transaction.
Acceptance criteria proof
deny.
a handle or evidence.
downgrade.
Tests/checks run
Repository-wide tests and the 78% global floor run only in hosted GitHub
Backend lanes on the exact pushed head.
Test delta
No tests were removed, skipped, weakened, or marked xfail. New focused tests
cover real-kernel positive and negative behavior rather than only mocks.
CI integrity
No workflow, threshold, lint, typecheck, or failure-masking behavior was
weakened. Focused tests were added to the existing semantic lane and behavior
ownership manifests.
Reviewer results
Architecture, security, QA, product/operations, senior engineering, CI
integrity, reuse/dedup, test-delta, and documentation reviews pass after their
findings were fixed.
External review
GitHub Actions and CodeRabbit remain pending until the branch is pushed and the
ready PR exists.
Remaining risks
operation/attempt transition; AUTH activation alone does not make the product
flow live.
domain/runtimepartition has a non-blocking layeringsmell recorded by architecture review for later boundary recovery.
Follow-up work
WS-POL-003-03B installs the live composition and provider/product ordering.
AUTH then resumes its approved post-12I sequence.
Human review focus
Human merge ownership
Summary by CodeRabbit
New Features
Bug Fixes
Documentation