Adopt Workstream authorization service baseline#93
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (71)
📝 WalkthroughWalkthroughThe PR establishes WS-AUTH-001’s authorization baseline through ADRs, specifications, architecture and operations documentation, workflow governance updates, fail-closed stale-document scanning, rendering checks, and review evidence. No runtime authorization implementation is added. ChangesAuthorization baseline and governance
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 14
🧹 Nitpick comments (1)
docs/diagrams/workstream_context.puml (1)
33-48: 🔒 Security & Privacy | 🔵 TrivialCover PlantUML diagrams with stale-authorization validation.
check_stale_authorization_docs.pyscans.mdand.html, so this canonical.pumldocument is currently outside the gate. Extend the scanner or add a dedicated diagram check to prevent stale authorization claims from regressing here without CI detection.🤖 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 `@docs/diagrams/workstream_context.puml` around lines 33 - 48, Extend check_stale_authorization_docs.py to scan PlantUML files, including the canonical workstream_context.puml document, and apply the existing stale-authorization validation to their contents. Ensure the CI gate detects regressions in authorization claims within .puml files while preserving current Markdown and HTML scanning.
🤖 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/reviews/WS-AUTH-001-01-pr-trust-bundle.md:
- Around line 95-97: Update the External Review status to mark PR publication as
complete now that PR `#93` exists, while retaining GitHub checks, CodeRabbit, and
human review as pending. Preserve the historical publication information if
needed, but remove the stale “Pending PR publication” wording.
In @.agent-loop/REVIEW_LOG.md:
- Around line 3-5: Update the WS-AUTH-001-01 status entry to reflect that PR
publication is complete, with only external or human review remaining pending.
Alternatively, mark it explicitly as historical implementation-freeze status; do
not leave the publication state as pending.
In @.agent-loop/WORK_QUEUE.md:
- Around line 5-13: Update the WORK_QUEUE.md entry for WS-POL-002-03 so an
implemented chunk in external review is not listed under “Planned Next.” Move it
to an explicit parallel/external-review section, or revise the heading
semantics, while preserving its current title, risk, and status.
In `@docs/architecture_data_model.md`:
- Around line 1461-1464: Update the AuditEvent field documentation for
claim_snapshot and actor_roles to mark them as legacy-only and explicitly
prohibit populating them for authority events. Clarify that new authority-event
records must use only the bounded actor and authorization fields described here,
without raw claims or unnecessary profile data.
- Around line 71-75: Update the ActorIdentityLink documentation to explicitly
require uniqueness of each (issuer, subject) pair and enforce that each
ActorProfile has at most one active identity link in v0.1. State both invariants
as enforceable constraints, while preserving the existing revoked-link and
immutable provenance behavior.
- Around line 454-456: Align the policy example’s approval provenance keys with
the declared fields: update the example to use approved_by_role and
approved_by_actor, or consistently rename the declared fields and canonical
schema to the existing ID-based keys. Ensure the field list, schema, and example
use one identical representation without losing approval provenance.
In `@docs/diagrams/task_lifecycle_sequence.md`:
- Around line 21-26: Update the task lifecycle sequence flows around
AuthorizationContext resolution to show AuthorizationService.require(...),
candidate-grant evaluation, and the applicable resource/lifecycle guards before
each state-changing project, task, claim, or review operation; alternatively
label AuthorizationContext explicitly as a precondition rather than
authorization. Apply the same correction to the referenced flow sections.
In `@docs/diagrams/workstream_context.puml`:
- Line 48: Update the label on the workstream-to-flow_auth relationship to use
the canonical wording “verifies externally issued Flow tokens” instead of
“verifies identity tokens,” without changing the relationship itself.
In `@docs/glossary.md`:
- Around line 141-144: Update the glossary definition for the permission-scoped
projection of a task’s locked guide and policy provenance to include Audit
alongside Project Manager and Operator, matching the authorized roles documented
for locked-context.
In `@docs/operations_operator_workflow.md`:
- Line 46: Update step 7 in the operator workflow to explicitly list all three
failure paths: apply covered repair for eligible issues, use registered Operator
retry for infrastructure or setup failures, and route worker-fixable blockers to
NEEDS_REVISION. Preserve the existing daily-loop requirement so no checker
failure is omitted.
In `@docs/operations_roles_permissions.md`:
- Around line 45-64: Update the Capability Matrix to define Both as the union of
Submitter and Reviewer capabilities, either by adding a Both column or an
explicit inheritance rule. Preserve the existing self-review and lifecycle
guards when documenting the combined grant’s allowed actions.
In `@docs/product_brief.md`:
- Around line 60-64: Align the persona definitions in docs/product_brief.md so
Operator consistently means runtime-health inspection and registered recovery,
not task creation or evidence submission. Rename the earlier task/evidence
persona or split it from Project Manager and Operator, preserving the separate
authority boundaries and matching the contributor-versus-administrative
distinction in operations_roles_permissions.md.
In `@docs/spec_authorization_service.md`:
- Around line 80-86: Clarify the permitted scope for the system-scoped
project_manager grant in the authorization specification, explicitly defining
whether it covers all projects, represents a separate administrative capability,
or is invalid. Apply the same decision consistently in the API and guard
contracts, including the related grant-scope section, and remove any
contradictory scope language.
- Around line 91-100: Replace the persisted contributor grant name `both` with a
specific value such as `submitter_reviewer` throughout the authorization
contract. Update the related grant catalog, migration contracts, and tests
consistently, while preserving its union of submitter and reviewer capabilities
and existing lifecycle/separation-of-duties guards.
---
Nitpick comments:
In `@docs/diagrams/workstream_context.puml`:
- Around line 33-48: Extend check_stale_authorization_docs.py to scan PlantUML
files, including the canonical workstream_context.puml document, and apply the
existing stale-authorization validation to their contents. Ensure the CI gate
detects regressions in authorization claims within .puml files while preserving
current Markdown and HTML scanning.
🪄 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: f261bc4d-2d85-4c44-8406-4c52ab2baa92
⛔ Files ignored due to path filters (6)
docs/architecture_brief/images/task_lifecycle_sequence.pngis excluded by!**/*.pngdocs/architecture_brief/images/workstream_context.pngis excluded by!**/*.pngdocs/architecture_brief/images/workstream_v01_container.pngis excluded by!**/*.pngdocs/architecture_brief/workstream_architecture_brief.pdfis excluded by!**/*.pdfdocs/diagrams/rendered/workstream_context.svgis excluded by!**/*.svgdocs/diagrams/rendered/workstream_v01_container.svgis excluded by!**/*.svg
📒 Files selected for processing (54)
.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/DECISIONS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/INTENT.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/PLAN.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/RISKS.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-01-adopt-authorization-baseline.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12-project-mutation-cutover.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-01-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-01-pr-trust-bundle.md.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/CHUNK_MAP.md.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/STATUS.md.agent-loop/policies/repository-engineering-policy.md.github/workflows/agent-gates.ymlREADME.mddocs/architecture_brief/task_lifecycle_sequence.pumldocs/architecture_checker_framework.mddocs/architecture_data_model.mddocs/architecture_lifecycle_state_machine.mddocs/architecture_lockdown.mddocs/architecture_system_architecture.mddocs/current_system_data_flow.htmldocs/decision_0003_project_guides_are_first_class.mddocs/decision_0009_review_decisions_are_canonical.mddocs/decision_0011_submission_artifact_policy_drives_pre_submit.mddocs/decision_0012_workstream_authorization_service.mddocs/diagrams/task_lifecycle_sequence.mddocs/diagrams/workstream_context.pumldocs/diagrams/workstream_v01_container.pumldocs/glossary.mddocs/operations_authorization_service.mddocs/operations_operator_workflow.mddocs/operations_project_operating_manual.mddocs/operations_queue_policy.mddocs/operations_roles_permissions.mddocs/product_brief.mddocs/product_first_user_flows.mddocs/reference_specs/README.mddocs/risk_register.mddocs/roadmap_implementation_backlog.mddocs/roadmap_status.mddocs/roles_permissions.mddocs/spec_authorization_service.mddocs/spec_chunk_2_auth_actor_boundary.mddocs/template_checker_policy.mddocs/template_project_guide.mddocs/template_revision_replay.mddocs/template_submission_artifact_policy.mdscripts/check_stale_authorization_docs.pyscripts/render_authorization_docs.shscripts/test_agent_gates.py
| ### Project Contributor Grants | ||
|
|
||
| | Grant | Exact-project capability | | ||
| |---|---| | ||
| | `submitter` | Minimal project read, task queue read/claim, own submission create/read, own review-chain read. | | ||
| | `reviewer` | Minimal project read, review queue/claim/release/decision, submission read for review, review-chain read. | | ||
| | `both` | Union of submitter and reviewer candidates, still subject to separation-of-duties and lifecycle guards. | | ||
|
|
||
| `worker` describes the task participant and remains in lifecycle/audit wording. | ||
| `submitter` is the persisted authority grant. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Replace the ambiguous persisted grant name both.
both is a canonical contributor grant, but it does not identify the capabilities it unions. Rename it to an actor- or subsystem-specific value such as submitter_reviewer before locking this contract, and update the related catalog, migration contracts, and tests.
As per coding guidelines, persisted roles and lifecycle names should prefer subsystem- or actor-specific names over vague labels.
🤖 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 `@docs/spec_authorization_service.md` around lines 91 - 100, Replace the
persisted contributor grant name `both` with a specific value such as
`submitter_reviewer` throughout the authorization contract. Update the related
grant catalog, migration contracts, and tests consistently, while preserving its
union of submitter and reviewer capabilities and existing
lifecycle/separation-of-duties guards.
Source: Coding guidelines
…adopt-authorization-baseline # Conflicts: # .agent-loop/LOOP_STATE.md # .agent-loop/REVIEW_LOG.md # .agent-loop/WORK_QUEUE.md # .agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/CHUNK_MAP.md # .agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/STATUS.md # docs/architecture_lifecycle_state_machine.md # docs/operations_project_operating_manual.md # docs/operations_queue_policy.md # docs/product_first_user_flows.md
…adopt-authorization-baseline # Conflicts: # .agent-loop/LOOP_STATE.md # .agent-loop/REVIEW_LOG.md # .agent-loop/WORK_QUEUE.md # .agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/CHUNK_MAP.md # .agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/STATUS.md
PR Trust Bundle: WS-AUTH-001-01
Chunk
WS-AUTH-001-01- Adopt Authorization Baseline And Repository ContractsGoal
Make the adopted authorization model, canonical
/api/v1namespace,superseded bootstrap behavior, recovery permissions, and implementation order
unambiguous before application code changes.
Human-Approved Intent
The user explicitly approved D4-D10 and started only WS-AUTH-001-01 on
2026-07-11. No later auth or policy chunk is activated.
What Changed And Why
product authority; local grants and guards are canonical.
and bounded rendering command to prevent documentation drift.
correction/activation contract in shared docs.
Design Chosen
External Flow identity verifies authentication facts. Workstream resolves
canonical actors and evaluates system/exact-project grants, registered
permissions, resource/lifecycle guards, revocation, idempotency, invalidation,
and append-only evidence. The scanner rejects authority-shaped token/profile
wording uniformly instead of trying to interpret English negation.
Alternatives Rejected
scoped, or immediately revocable.
Scope Control
The diff contains docs, loop memory, diagram sources/generated companions, one
scanner, one renderer, scanner tests, and one additive CI step. It contains no
backend/frontend runtime, schema, migration, dependency, package, or lockfile
changes. All eight imported sources,
SHA256SUMS, andSOURCE_MANIFEST.mdareunchanged.
Product Behavior
No runtime behavior changes. Review decisions remain
accept,needs_revision, andreject. PR #90 correction clears unapproved output,preserves redacted metadata, requeues derivation, and keeps activation blocked
until the current compiled policy is approved.
Acceptance Proof
/api/v1is canonical with no alias.Tests And Checks
git diff --checkpassed.Test Delta And CI Integrity
Existing tests were not removed, skipped, or weakened. New fixtures are
independent of production rule definitions. Agent Gates invokes the scanner
directly without
continue-on-error; no existing gate or threshold changed.Reviewer Results
All required tracks passed against implementation freeze
6756e6cb397da5f813eca39fb738633bc24f2ab2, and the administrative evidencebundle was reviewed at final bound SHA
35152a001e0689782f5c0d59615a59a89e6e2677: senior engineering, QA/test,security/auth, product/ops, architecture, docs, reuse/dedup, CI integrity, and
test delta. The circuit breaker accepted the documented atomic-docs size
exception.
External Review
Pending PR publication, GitHub checks, CodeRabbit, and human review.
Remaining Risks And Follow-Up
pinned PlantUML JAR.
contract phrase remains an acknowledged low wording risk.
bounded chunks.
WS-AUTH-001-02andWS-POL-002-04remain inactive.Human Review Focus
Confirm authority precedence, stable grant/permission names,
/api/v1,recovery boundaries, PR #90 interaction, and that no runtime/review/compensation
behavior was adopted accidentally.
Human Merge Ownership
Only the user may approve and merge this PR. Publication is not merge approval.
Summary by CodeRabbit