feat: prepare exact private action tickets - #76
Conversation
|
Warning Review limit reached
Next review available in: 39 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 (1)
📒 Files selected for processing (51)
📝 WalkthroughWalkthroughAdds a private TypeScript ChangesActionPlane prepare implementation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
migrations/versions/20260723_0022_action_prepare.py (1)
376-418: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: collapse the duplicate
delivery_evidencelookup.
expected_identity_digestis computed by scanningdelivery_evidence(376-413), then the same row is re-fetched intoevidence_record(415-418) with an identical predicate. You can fetchevidence_recordfirst and then derive the identity digest fromevidence_record.authentication_binding_refvia a plain assignment (noFROM), removing one point lookup on the prepare hot path and keeping the existence check in one place.🤖 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 `@migrations/versions/20260723_0022_action_prepare.py` around lines 376 - 418, Collapse the duplicate delivery_evidence lookup in the surrounding action-prepare logic: fetch the matching row once into evidence_record using the existing organization_id and evidence_digest predicate, then compute expected_identity_digest by assigning the same digest expression from evidence_record.authentication_binding_ref without a FROM query. Preserve the existing existence-check behavior and digest construction.
🤖 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 `@scripts/database_harness.sh`:
- Around line 303-305: Update migrate_legacy_action_identity to validate
CONTEXT_ENGINE_ACTION_ROLE, CONTEXT_ENGINE_ACTION_PASSWORD, and
CONTEXT_ENGINE_ACTION_URL together, requiring exactly one entry for each; when
any variable is missing or duplicated, atomically replace all three action
entries, matching the existing learning-role migration behavior.
In `@tests/integration/test_postgres_harness.py`:
- Around line 154-157: Update the finally cleanup path in the test harness to
call guarded_action_engine.dispose() alongside the other guarded engine
disposals, ensuring it runs on failures as well as the normal path. Remove or
avoid relying solely on the separate normal-path disposal while preserving
cleanup of all existing engines.
---
Nitpick comments:
In `@migrations/versions/20260723_0022_action_prepare.py`:
- Around line 376-418: Collapse the duplicate delivery_evidence lookup in the
surrounding action-prepare logic: fetch the matching row once into
evidence_record using the existing organization_id and evidence_digest
predicate, then compute expected_identity_digest by assigning the same digest
expression from evidence_record.authentication_binding_ref without a FROM query.
Preserve the existing existence-check behavior and digest construction.
🪄 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: 496c9d2b-e3eb-4ca2-8ec4-724626cfb1f0
⛔ Files ignored due to path filters (1)
action_plane/typescript/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (50)
.github/workflows/ci.yml.gitignoreCONTEXT.mdMakefileaction_plane/typescript/.node-versionaction_plane/typescript/README.mdaction_plane/typescript/THIRD_PARTY_NOTICES.mdaction_plane/typescript/package.jsonaction_plane/typescript/scripts/clean.mjsaction_plane/typescript/src/index.tsaction_plane/typescript/src/internal.tsaction_plane/typescript/test/action-plane.test.mjsaction_plane/typescript/test/live-prepare.mjsaction_plane/typescript/tsconfig.build.jsonaction_plane/typescript/tsconfig.jsoncompose.yamldocs/decisions/0049-prepare-one-exact-private-effect.mddocs/decisions/README.mdengine/persistence/__init__.pyengine/persistence/configuration.pyengine/persistence/role_guard.pyengine/persistence/schema_security_manifest.yamleval/catalogs/m0-security-evidence.yamleval/catalogs/security-catalog.schema.jsoneval/catalogs/security-invariants.yamlinfra/postgres/init/10-security-roles.shmigrations/versions/20260723_0022_action_prepare.pyscripts/database_harness.shscripts/provision_database_roles.pyscripts/security_gate/rls.pyscripts/security_gate/runner.pyscripts/validate_security_catalog.pyscripts/wait_for_database.pytests/catalog/test_m0_security_gate.pytests/catalog/test_validate_security_catalog.pytests/integration/conftest.pytests/integration/test_action_prepare.pytests/integration/test_file_import_tracer.pytests/integration/test_m0_security_gate_rls.pytests/integration/test_membership_schema.pytests/integration/test_migrations.pytests/integration/test_postgres_harness.pytests/integration/test_zz_file_content_noop.pytests/unit/test_database_configuration.pytests/unit/test_database_harness_contract.pytests/unit/test_learning_database_configuration.pytests/unit/test_learning_database_role_provisioning.pytests/unit/test_m0_delivery_carriers.pytests/unit/test_m0_rls_inventory.pytests/unit/test_schema_security_manifest.py
| guarded_action_engine: Engine, | ||
| identity_configuration: DatabaseConfiguration, | ||
| egress_configuration: DatabaseConfiguration, | ||
| action_configuration: DatabaseConfiguration, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Dispose guarded_action_engine in the finally path.
The new engine is disposed only on the normal path at Line 195, while the finally block disposes the other guarded engines but not this one. An earlier failure can leave the action-role pool alive during role teardown and contaminate subsequent integration tests. Add guarded_action_engine.dispose() alongside the other finally disposals.
Also applies to: 187-195
🤖 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 `@tests/integration/test_postgres_harness.py` around lines 154 - 157, Update
the finally cleanup path in the test harness to call
guarded_action_engine.dispose() alongside the other guarded engine disposals,
ensuring it runs on failures as well as the normal path. Remove or avoid relying
solely on the separate normal-path disposal while preserving cleanup of all
existing engines.
5981143 to
11c02e1
Compare
1 participant
Closes #67
Outcome
Implements the closed
ActionPlane.prepare(TrustedEffectIntent)boundary for one exact private delivery effect. The trusted TypeScript module now validates the service, Organization, destination, audience, purpose, payload digest, policy epoch, approval tier, and idempotency identity before returning an operation-specific one-shot ticket or a closed zero-effect refusal.Security and persistence
Verification
Fresh
make db-reset && make check:The PostgreSQL evidence includes all three operations across the wrong-field matrix, payload/idempotency conflicts, unknown operation rejection, exact retry, stale/disabled/expired context, RLS isolation, restricted digest scans, least privilege, and zero Sender calls.
Final independent standards and specification reviews report no actionable findings.
Summary by CodeRabbit
New Features
Documentation