Add legacy actor classification preflight#109
Merged
abiorh-claw merged 12 commits intoJul 13, 2026
Conversation
|
Warning Review limit reached
Next review available in: 27 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 (10)
📝 WalkthroughWalkthroughAdds a fail-closed legacy actor classification workflow with deterministic evidence contracts, read-only database snapshots, secure envelope publication, a dry-run-first CLI, extensive tests, operational documentation, and updated workstream gating state. ChangesLegacy actor classification
Sequence Diagram(s)sequenceDiagram
participant Operator
participant ClassificationCLI
participant PostgreSQL
participant EvidencePublisher
Operator->>ClassificationCLI: run dry-run or explicit export
ClassificationCLI->>PostgreSQL: acquire read-only repeatable-read snapshot
PostgreSQL-->>ClassificationCLI: legacy actor rows and database binding
ClassificationCLI->>ClassificationCLI: validate manifest and build envelope
ClassificationCLI->>EvidencePublisher: publish canonical envelope when requested
EvidencePublisher-->>ClassificationCLI: write status or stable failure
ClassificationCLI-->>Operator: bounded JSON report
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
…legacy-actor-classification # Conflicts: # .agent-loop/LOOP_STATE.md # .agent-loop/WORK_QUEUE.md
abiorh-claw
approved these changes
Jul 13, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
backend/scripts/legacy_actor_classification.py (1)
119-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd internal logging for unexpected exceptions.
The generic
except Exceptionhandler reports all non-LegacyClassificationErrorfailures as"database_operation_failed", which is misleading for file I/O errors frompublish_envelopeor manifest parsing errors fromload_manifest. Consider logging the actual exception internally for debugging while preserving the generic external message.Add
import loggingat the top of the file, then:♻️ Proposed refactor
except Exception: + logging.getLogger(__name__).exception("unexpected classification error") stderr_message = '{"error":"database_operation_failed","status":"error"}' result = 2🤖 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/scripts/legacy_actor_classification.py` around lines 119 - 121, Update the generic `except Exception` handler in the main classification flow to log the caught exception internally with Python’s `logging` module, while preserving the existing generic `stderr_message` and result code. Add the `logging` import and use an appropriate logger call so failures from `publish_envelope`, `load_manifest`, or other unexpected operations retain diagnostic details without changing the external response.
🤖 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-03-legacy-actor-classification.md:
- Around line 123-130: Update the “Implementation review status” record to match
the durable internal review evidence: label
a70b89c91a8950eabaa750e340d4f853529d66f0 as the reviewed revision and
8e2ae489834a3934d6ef507834139a1009dac2e6 as the implementation SHA, using these
labels consistently across the related records.
In
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-03-internal-review-evidence.md:
- Around line 19-36: Replace the host-local /root/auth03_parallel review paths
in the recorded review run ID fields with durable run identifiers or
repository-relative artifact references. Update all entries under the reviewer,
merge-resolution, and evidence-binding sections while preserving each review
role and its association with the correct run.
In
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md:
- Around line 21-22: Reconcile the PR `#108` lifecycle-head reference in STATUS.md
with the authoritative 5c47aba SHA recorded in LOOP_STATE.md and REVIEW_LOG.md.
Update the entry to use 5c47aba, or explicitly clarify why a70b89c is a distinct
reviewed head so operators can identify the correct integrated revision.
- Around line 60-62: Update the status statement in the AUTH-03 documentation to
distinguish implementation readiness from merge readiness: state that there is
no implementation blocker, while explicitly noting that PR `#109` external checks,
required human review, and repository-wide coverage proof remain pending.
In @.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/STATUS.md:
- Around line 5-13: Update the Branch metadata in STATUS.md to reference the
authoritative post-merge branch associated with the merged R10 state, or
explicitly label the existing B1B branch as historical; keep the merged status
and active implementation chunk unchanged.
In `@backend/scripts/legacy_actor_classification.py`:
- Around line 122-127: Update the cleanup exception handler around
dispose_engine() so it only assigns the database_cleanup_failed stderr_message
and result = 2 when no prior failure has been recorded. Preserve the existing
stdout_message, stderr_message, and result values when the main operation
already failed, including specific exit codes such as 130.
---
Nitpick comments:
In `@backend/scripts/legacy_actor_classification.py`:
- Around line 119-121: Update the generic `except Exception` handler in the main
classification flow to log the caught exception internally with Python’s
`logging` module, while preserving the existing generic `stderr_message` and
result code. Add the `logging` import and use an appropriate logger call so
failures from `publish_envelope`, `load_manifest`, or other unexpected
operations retain diagnostic details without changing the external response.
🪄 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: 9920e7b9-1e8d-4b04-85ae-5615362cb8af
📒 Files selected for processing (15)
.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/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-03-legacy-actor-classification.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-03-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-03-pr-trust-bundle.md.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/CHUNK_MAP.md.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/STATUS.mdbackend/app/modules/actors/legacy_classification.pybackend/scripts/legacy_actor_classification.pybackend/tests/test_actor_legacy_classification.pybackend/tests/test_actors.pydocs/operations_authorization_service.md
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.
PR Trust Bundle: WS-AUTH-001-03
Chunk
WS-AUTH-001-03- Legacy Actor Classification PreflightGoal
Provide the supported, explicit, dry-run-first evidence workflow that classifies
every legacy actor identity before canonical actor migration without inference,
manual SQL, grants, or actor-state mutation.
Human-Approved Intent
The user explicitly started AUTH-03 after PR #107 merged AUTH-02 as
060b780.This PR implements only the existing AUTH-03 contract. Coverage work remains
independent, and AUTH-04 remains inactive pending a separate post-merge start.
What Changed
failure contracts.
identity projection.
and CLI behavior tests.
Why It Changed
Existing actor registry rows have issuer and opaque subject but no trustworthy
subject kind. The later canonical actor migration must fail closed rather than
guessing whether a legacy identity is human or an internal service.
Design Chosen
An operator-owned strict JSON manifest binds each exact legacy UUIDv5, HTTPS
issuer, case-sensitive subject, and
humanorservicekind. One read-onlyrepeatable-read snapshot verifies completeness and exact identity, then binds a
canonical confidential envelope to the live row set, manifest, database, UTC
generated-at value, and its complete contents. Publication uses an owner-only
directory, mode
0600, temp-filefsync, atomic no-overwrite link, directoryfsync, and rollback on post-link durability failure.Pure envelope loading and verification remain independent from the mutable
actor ORM so the future synchronous Alembic migration can reuse them. That
migration will accept the file only through
WORKSTREAM_LEGACY_ACTOR_CLASSIFICATION_FILEand must recompute live bindingsinside its transaction.
Alternatives Rejected
Scope Control
The diff is limited to the contract-approved classifier module, CLI, focused
tests, actor database test, authorization runbook, and durable AUTH loop memory.
There is no schema, Alembic, dependency, CI, grant, role, actor-state, API,
product lifecycle, frontend, review, payment, or later AUTH implementation.
Product Behavior
This chunk adds no public API and grants no authority. Operators receive a
privacy-bounded dry-run report by default. Non-empty registries require a
complete exact manifest. Empty registries produce explicit empty proof. Export
is explicit, confidential, canonical, owner-only, and no-overwrite.
Acceptance Criteria Proof
Tests cover strict/duplicate/oversized/non-finite JSON, supported kinds,
canonical UUIDv5 derivation, exact issuer/subject matching, missing/stale/extra
rows, deterministic hashes, complete envelope checksum, database drift,
TOCTOU, empty proof, canonical bytes, permissions/ownership, main and linked
worktrees, symlinks, idempotence, link and
fsyncfailures, environment-onlyhandoff, cleanup redaction, migration import independence, and real PostgreSQL
snapshot isolation under a concurrent commit.
Tests And Checks
links, docstring coverage, and
git diff --checkpass.--cov-fail-under=78gate will provide therepository-wide coverage and complete-suite evidence before merge.
Test Delta
No tests were deleted, skipped, xfailed, or weakened. The changed actor test
uses real PostgreSQL and proves read-only repeatable-read state plus concurrent
snapshot stability. File and CLI tests exercise observable failure behavior,
not execution-only coverage lines.
CI Integrity
No workflow, dependency, package script, threshold, exclusion, or bypass
changed. Focused coverage exceeds 90 percent. Repository-wide coverage remains
enforced at the unchanged 78 percent CI floor while the separate coverage
initiative raises that floor toward the authoritative 90 percent target.
Reviewer Results
All required tracks pass implementation SHA
8e2ae489834a3934d6ef507834139a1009dac2e6, and the final evidence-boundrevision
db123f7d06cadf433f02f1210ab7927b85a26b61passed engineering,security, QA, test-delta, and CI evidence re-review. Initial High and Medium
findings were repaired and re-reviewed. Only documented low risks remain.
External Review
Ready PR #109 is open at
https://github.com/Flow-Research/workstream/pull/109. Agent Gates pass;GitHub Backend CI, CodeRabbit, and explicit human review are pending.
Remaining Risks
identity; clone/restore/recreation requires fresh evidence.
evidence and are not available or required for deterministic implementation.
does not alter schema or actor state.
Follow-Up Work
AUTH-04 remains inactive. The future actor migration consumes this preflight in
its owning reviewed chunk. Confidential manifest/envelope deletion follows the
durable migration-record and rollback-window procedure in the runbook.
Human Review Focus
Inspect absence of inference, exact UUIDv5/issuer/subject matching, complete
row-set and envelope checksums, owner-only custody, every-worktree exclusion,
snapshot/TOCTOU behavior, future migration independence, and absence of grants
or database writes.
Human Merge Ownership
Only the user may approve and merge this PR. PR publication is not merge
approval, and AUTH-04 must not start automatically.
Summary by CodeRabbit