Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,32 @@ P1

```text
backend/app/modules/actors/**
backend/app/adapters/auth/dev.py
backend/app/adapters/auth/flow.py
backend/app/modules/tasks/service.py
backend/app/modules/tasks/router.py
backend/app/modules/tasks/schemas.py
backend/app/api/deps/auth.py
backend/app/api/deps/api_controls.py
backend/app/api/deps/rate_controls.py
backend/app/api/router.py
backend/app/api/routes/auth.py
backend/app/schemas/auth.py
backend/app/db/models.py
backend/app/modules/audit/**
backend/alembic/versions/0020_*.py
backend/tests/test_actors.py
backend/tests/test_auth.py
backend/tests/test_api_rate_controls.py
backend/tests/test_alembic.py
backend/tests/test_projects.py
backend/tests/test_tasks.py
backend/tests/test_checkers.py
backend/tests/conftest.py
backend/scripts/api_contract_e2e.py
docs/operations_authorization_service.md
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/**
.agent-loop/merge-intents/WS-AUTH-001-06.json
.agent-loop/LOOP_STATE.md
.agent-loop/WORK_QUEUE.md
.agent-loop/REVIEW_LOG.md
Expand Down Expand Up @@ -93,8 +101,11 @@ review or compensation models
chunk 13 removes its queue/claim/start consumers after grant provisioning;
chunk 14 removes the route, activation service/schema, token-role observation
fields, final submission consumer, and compatibility adapter together.
- Provisioning uses the established idempotency/audit/invalidation foundation
and commits the profile, link, and events atomically.
- First-human provisioning is transactionally idempotent through exact
`(issuer, subject)` uniqueness and concurrent-conflict resolution. Profile,
link, `ActorProfileProvisioned`, and `ActorIdentityLinked` evidence commit
atomically through the shared audit path. Automatic first access creates no
client-key authority-idempotency record and no invalidation event.
- `/api/v1/actors/me` returns Contributor domain with no implied project/admin
authority.
- `GET /api/v1/actors/me` and `PATCH /api/v1/actors/me` have request, privacy,
Expand All @@ -107,6 +118,8 @@ review or compensation models
classified upgrade, and preserved attribution. Downgrade/rollback must
succeed after the external classification envelope is deleted post-upgrade,
using only its durably recorded version/checksum and migration state.
- Behavior tests keep the materially changed actor subsystem at or above 90
percent branch coverage without exclusions or weakened assertions.

## Verification commands

Expand All @@ -115,7 +128,11 @@ review or compensation models
(cd backend && WORKSTREAM_DATABASE_URL=<isolated-test-db> .venv/bin/alembic downgrade -1)
(cd backend && WORKSTREAM_DATABASE_URL=<isolated-test-db> .venv/bin/alembic upgrade head)
(cd backend && .venv/bin/python -m ruff check app tests)
(cd backend && WORKSTREAM_DATABASE_URL=<test-db> .venv/bin/python -m pytest -q)
(cd backend && WORKSTREAM_TEST_DATABASE_URL=<test-db> .venv/bin/python -m pytest -q \
--cov=app.modules.actors --cov-branch --cov-report=term-missing \
--cov-fail-under=90 tests/test_actor_legacy_classification.py \
tests/test_actors.py tests/test_auth.py tests/test_tasks.py)
(cd backend && WORKSTREAM_TEST_DATABASE_URL=<test-db> .venv/bin/python -m pytest -q)
(cd backend && WORKSTREAM_DATABASE_URL=<test-db> .venv/bin/python scripts/api_contract_e2e.py)
python3 scripts/check_stale_workstream_wording.py
python3 scripts/check_markdown_links.py
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# External Review Response: WS-AUTH-001-06

## Pull Request

PR #124 - Add canonical actor profile and identity resolution

CodeRabbit run: `099fb3fe-cd93-453a-9445-e0e483fe3b79`

## Comments Addressed

1. Migration identity-link UUIDs now seed from the canonical actor UUID instead
of delimiter-ambiguous issuer/subject concatenation.
2. Downgrade copies current canonical display fields, including `null`, into
existing legacy rows so a cleared contact email cannot be resurrected. The
migration proof covers an updated display name and cleared email, and the
operations contract records the intentional rollback scrubbing behavior.
3. Development and Flow verifier boundaries reject blank or padded identity
anchors before persistence without normalizing distinct opaque values.
4. Legacy eligibility activation takes the exact external-identity advisory
lock. The synchronized concurrency proof establishes actual lock waiting,
ordered payload application, one row, and one audit per real transition.
5. Work-context claim, start, pre-submit-check, and submit affordances require
both the current compatibility role and active eligibility, matching the
mutation guards. Tests prove absent, disabled, and role-missing behavior plus
submission denial with no submission, checker-run, or audit side effects.
6. The shared privileged database reset now has one test-owned implementation
with an explicit canonical-actor option. All five affected fixture teardowns
use nested `finally` blocks so reset failure cannot skip disposal or Alembic
downgrade.
7. Canonical actor history-trigger reset tracks successful disable and always
re-enables the trigger in a fresh transaction.
8. The compatibility allowlist uses Python AST structure and binds every direct
adapter read and eligibility-gate call to its exact `TaskService` method.
9. The live API contract asserts that issuer, subject, and legacy roles are
absent from the canonical actor response.
10. Touched Contributor-facing task prose no longer describes a human actor as
a worker; legacy wire and storage identifiers remain under their declared
later cutover.
11. GitHub Backend's only failure was the strict OpenAPI inventory retaining the
pre-AUTH-06 counts and hashes. The contract now includes the two protected
actor self-service operations without relaxing either fingerprint.

## Comments Deferred

None of the actionable comments were deferred.

## Non-Actionable Review Output

CodeRabbit's 53.8 percent docstring warning is a diff-local advisory rather than
the repository's configured quality gate. Existing public behavior and complex
helpers retain useful docstrings; narration-only comments were not added.

## Human Decisions Needed

None. The repaired downgrade now favors current canonical privacy state over
retaining stale pre-AUTH-06 display data, and the runbook makes that rollback
tradeoff explicit.

## Commands Rerun

```text
pytest focused AUTH-06 migration/concurrency/lifecycle behavior
pytest test_actor_legacy_classification.py test_actors.py --cov=app.modules.actors --cov-branch
pytest test_api_controls.py
python backend/scripts/api_contract_e2e.py
ruff check app tests alembic/versions/0020_canonical_actor_profile.py scripts/api_contract_e2e.py
python3 scripts/test_agent_gates.py
python3 scripts/update_post_merge_memory.py validate-merge-intent --base-ref origin/main
python3 scripts/check_loop_memory_state.py
python3 scripts/check_stale_workstream_wording.py
python3 scripts/check_stale_authorization_docs.py
python3 scripts/check_markdown_links.py
git diff --check
```

- Actor/classification behavior: 83 passed at 90.1031 percent branch coverage.
- Real Postgres API contract: passed through migration `0020` and the complete
current task intake drill.
- Integrated engineering-loop gates: 71 passed; schema-v2 merge intent passed.
- GitHub Backend: 983 passed, one repaired OpenAPI inventory failure, and 83.11
percent repository coverage; all 27 API-control tests pass after repair.
- All required internal reviewer tracks passed on reviewed code SHA
`25d9455f6dca41b207a0ba3aaba8de9cc2683a17`.

## Remaining Risks

- Production migration still requires the documented quiesced deployment and
non-owner runtime role.
- Rollback intentionally scrubs retained legacy display data that was not set
canonically after AUTH-06.
- GitHub Backend owns the repository-wide suite and 78 percent baseline.

No GitHub thread is replied to or resolved by this evidence file. Thread writes
remain a separate explicit action after the repaired commit is pushed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# WS-AUTH-001-06 Internal Review Evidence

Reviewed code SHA: `25d9455f6dca41b207a0ba3aaba8de9cc2683a17`
Reviewed runtime SHA: `abd76c995e51645b61d4d3ac07f1ff82ab6eb740`
Reviewed at: `2026-07-15T08:35:20Z`
Reviewer run IDs: `auth06_final_senior`, `auth06_qa_review`,
`auth06_security_review`, `auth06_product_ops_review`,
`auth06_final_architecture`, `auth06_final_ci`, `auth06_final_docs`,
`auth06_final_reuse`, `auth06_final_test_delta`

## Deterministic Evidence

- Actor subsystem branch coverage reached 90.1031 percent across 804
statements and 166 branches, above the required 90 percent gate.
- Final actor and legacy-classification coverage run: 83 passed in 706.29
seconds, including synchronized compatibility activation, rollback, privacy,
exact-anchor, and lifecycle behavior.
- External-review database proof passed six migration, concurrency, and
lifecycle scenarios together; the repaired role-without-submit-access case
then passed separately against the integrated branch.
- Real Postgres API contract passed through migration `0020`, actor
provisioning, response privacy, compatibility projection, task
claim/start/submission, and authorization denials.
- Ruff passed for backend application, tests, migration, and API contract code.
- Stale Workstream wording, stale authorization documentation, changed Markdown
links, and `git diff --check` passed.
- All 71 integrated engineering-loop agent-gate tests passed, and the schema-v2
merge intent resolves the exact same-initiative AUTH-07 successor.
- GitHub Backend reached 983 passed tests and 83.11 percent repository coverage;
its only failure was the stale closed-world OpenAPI inventory. The inventory
now includes the two protected actor self-service routes, and all 27 API
control tests pass locally.
- No workflow, dependency, coverage threshold, skip, exclusion, or package
script changed. GitHub Backend remains authoritative for the repository-wide
78 percent floor; the multi-hour full suite was not repeated locally.

## Reviewer Results

| Reviewer | Result | Blocking findings | Notes |
|---|---|---|---|
| senior engineering | PASS | none | Transaction ownership, identity resolution, compatibility boundaries, and failure mapping pass. |
| qa/test | PASS | none | Provisioning, idempotency, migration, rollback, rate-control, and legacy behavior are covered. |
| security/auth | PASS | none | Issuer-subject identity, non-human denial, bounded claims, privacy, and zero-write failures pass. |
| product/ops | PASS | none | Contributor semantics and bounded legacy submitter compatibility match the approved lifecycle. |
| architecture | PASS | none | Canonical actor ownership and shared dependency boundaries pass; accepted low operational risks are documented. |
| ci integrity | PASS | none | CI policy, thresholds, dependencies, and exclusions are unchanged. |
| docs | PASS | none | Deployment, rollback, privacy, bounds, and compatibility documentation match runtime behavior. |
| reuse/dedup | PASS | none | Shared rate-control and actor failure mapping replace duplicate route behavior. |
| test delta | PASS | none | New assertions exercise meaningful behavior without skips, xfails, or weakened expectations. |

The final senior/architecture/reuse, QA/test-delta/CI, and
security/product/docs repair reviews independently verified the 48-route and
46-protected-route fingerprints. Removing only `GET` and
`PATCH /api/v1/actors/me` reproduces both prior inventories exactly.

## Findings Resolved

Review repair closed stale eligibility on assigned task start, operator-override
regression, deactivated-profile access, non-human zero-write proof, rate-control
unavailability, mid-audit rollback, repeated-access timestamp behavior,
idempotent and serialized compatibility activation, migration ID/downgrade
gaps, invalid-row privacy, exact-anchor validation, lifecycle affordance drift,
fixture cleanup safety, duplicate error mapping, and human-facing legacy
terminology.

Valid findings addressed: yes

Open sub-agent sessions: none

## Remaining Gate

GitHub Backend rerun, Agent Gates, CodeRabbit, and explicit human merge approval
remain pending. `WS-AUTH-001-07` must not start automatically.
Loading
Loading