Clean-cut contributor attribution to canonical human actors#153
Conversation
|
Warning Review limit reached
Next review available in: 4 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 selected for processing (3)
📝 WalkthroughWalkthroughThe PR establishes contributor-based task attribution through migration ChangesContributor Foundation
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant Contributor
participant TaskService
participant ActorService
participant Database
Contributor->>TaskService: claim task or submit work
TaskService->>ActorService: validate active human writer
ActorService->>Database: lock profile and identity link
Database-->>ActorService: canonical actor state
ActorService-->>TaskService: validation result
TaskService->>Database: lock task and assignment
Database-->>TaskService: persist contributor_id
TaskService-->>Contributor: structured task response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
6628-6639: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAssert the complete index set, not an arbitrary matching index.
scalar()returns one matching index without ordering or checking cardinality. If both the legacyworker_idindex and the newcontributor_idindex exist, this helper can select either one and still let the “exact schema shape” assertions pass. Return and assert the full ordered set (or at least assertcount(*) = 1plus the exact expected name).🤖 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 6628 - 6639, Update the async index helper around index to validate the complete matching index set instead of using scalar(), which can return an arbitrary row. Return an ordered collection of matching index names and update its callers to assert the exact expected set, or enforce count(*) = 1 and assert the precise expected index name for each table.
🤖 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 6628-6639: Update the async index helper around index to validate
the complete matching index set instead of using scalar(), which can return an
arbitrary row. Return an ordered collection of matching index names and update
its callers to assert the exact expected set, or enforce count(*) = 1 and assert
the precise expected index name for each table.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: be50a445-51ce-4c6d-83e6-aff995c59362
📒 Files selected for processing (3)
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-CONTRIBUTOR-FOUNDATION-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-CONTRIBUTOR-FOUNDATION-pr-trust-bundle.mdbackend/tests/test_alembic.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-CONTRIBUTOR-FOUNDATION-pr-trust-bundle.md
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-CONTRIBUTOR-FOUNDATION-internal-review-evidence.md
2 participants
WS-AUTH-001-CONTRIBUTOR-FOUNDATION PR Trust Bundle
Goal
Clean-cut task-assignment and submission human attribution to canonical
contributor_id, enforce human ActorProfile lineage at the database boundary,and revalidate the exact active caller inside contributor-write transactions.
Changes
dual field, guessed identity mapping, or historical audit rewrite.
function and named triggers for both tables.
canonical order for claim and submission, with stable 403 and retryable 503
responses and rollback.
persistent task coverage reporting, agent gates, and the schema-v2 merge intent.
Boundary
This PR changes no ActionId, PermissionId, evaluator, grant, role behavior,
authorization decision/evidence path, service admission, task lifecycle, review
or revision behavior. The temporary legacy role token and separately owned
attestation/profile route cleanup remain AUTH-13/14 work. AUTH-09E stays inactive.
Proof
integrity, one Alembic head, and 88 agent gates: passed.
GitHub Backend remains the authoritative mandatory proof for global 78 percent
and persistent actor/authorization/task 90 percent coverage before merge.
Internal Review
Exact implementation SHA
4d1fc507c343d483677a332c2a91885e32571693andcloseout head
4db178147bae457d9fccb3643fe6bd3919ba41c2against trusted main93dd392484b397cfdfaaa833631dc2c27f591ed7passed senior engineering,QA/test, security/auth, product/ops, architecture, CI integrity, docs,
reuse/dedup, and test-delta review after every valid finding was repaired.
Human Review Focus
Review migration refusal/redaction and reversible preservation, exact database
object names, profile-to-link-to-task-to-assignment lock order, lifecycle race
outcomes, 403/503 privacy and rollback, clean
contributor_idAPI/evidenceshape, and absence of authorization availability or later AUTH-13/14 behavior.
Merge Ownership
The agent may publish and repair this branch but may not merge it. Only the
human may approve this PR for merge. After merge, trusted-main automation owns
schema-v2 memory generation; no manual post-merge memory PR should be opened
when that workflow succeeds.
Summary by CodeRabbit
contributor_idinstead ofworker_idfor assignments and submissions.