feat: Plan 1+2 — Trust engine + Control-plane TS foundation - #4
Conversation
…twork_score_history
…it, audit middleware
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 28 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (24)
WalkthroughThe pull request adds a TypeScript/Express control plane with PostgreSQL migrations, identity and attestation domains, tenant synchronization, authentication, shared HTTP infrastructure, REST/SSE routes, and graceful startup and shutdown handling. ChangesControl-plane foundation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthMiddleware
participant ControlPlaneRoute
participant DomainService
participant PostgreSQL
Client->>AuthMiddleware: Send bearer token or API key
AuthMiddleware->>PostgreSQL: Validate credential and tenant
AuthMiddleware->>ControlPlaneRoute: Attach authenticated request context
ControlPlaneRoute->>DomainService: Process principal, attestation, or sync request
DomainService->>PostgreSQL: Read or persist control-plane state
DomainService-->>ControlPlaneRoute: Return domain result
ControlPlaneRoute-->>Client: Return JSON or SSE response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 29
🤖 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 `@control-plane/.gitignore`:
- Line 3: Update the .env ignore rule in .gitignore to match the base file and
environment-specific variants such as .env.local and .env.production, while
preserving the existing ignore behavior for .env.
In `@control-plane/migrations/001_graph/migration.sql`:
- Line 41: Enforce the non-negative, non-null weight domain in both migration
sites: update control-plane/migrations/001_graph/migration.sql lines 41-41 so
trust_weight is NOT NULL with a non-negative check, and update
control-plane/migrations/003_sync/migration.sql lines 21-21 so current_weight
has the same non-negative check.
In `@control-plane/migrations/002_attestations/migration.sql`:
- Line 36: Remove the redundant idx_attestations_token_digest CREATE INDEX
statement from the attestations migration, relying on the unique constraint on
token_digest to provide indexing.
In `@control-plane/migrations/004_tenancy/migration.sql`:
- Around line 14-20: Provision the citext extension in an earlier migration
before the users table is created. Add CREATE EXTENSION IF NOT EXISTS citext so
the CITEXT email column in users is available on fresh databases.
In `@control-plane/migrations/005_policy/migration.sql`:
- Around line 15-16: Update the policy table constraints for
max_snapshot_age_seconds and allow_sample_rate so snapshot age cannot be
negative and sample rate is restricted to the inclusive range [0, 1]. Preserve
their existing types and defaults while enforcing these bounds at the database
level.
In `@control-plane/migrations/006_audit/migration.sql`:
- Around line 24-33: Constrain the decision telemetry schema by adding a CHECK
constraint requiring decision_aggregates.count to be nonnegative, and restrict
sample action values to the same allow/deny/passthrough domain used by
decision_aggregates.action. Apply the corresponding validation in the sample
table definition referenced by the comment.
- Around line 26-28: Enforce tenant-scoped edge-node ownership on all three
decision-record table definitions by adding a composite foreign key for
(tenant_id, edge_node_id) referencing edge_nodes(tenant_id, id) at
control-plane/migrations/006_audit/migration.sql lines 26-28, 39-41, and 55-57.
Ensure each table rejects records whose edge node belongs to a different tenant.
In `@control-plane/src/config.ts`:
- Line 29: Remove the localhost fallback from the database url configuration so
`config.database.url` remains unset when `DATABASE_URL` is absent. Preserve the
existing validation at the database configuration assertion around lines 56–59
so startup fails before migrations when no explicit URL is provided.
In `@control-plane/src/db/client.ts`:
- Around line 12-14: Update the pool error handler in the pool.on('error')
callback to use the structured service logger instead of console.error. Call
logger.error with the error object under the err field and retain a descriptive
pool-failure message.
In `@control-plane/src/db/migrate.ts`:
- Around line 63-67: Update the migration validation around discoverMigrations
and appliedMap to compare every applied migration name with the locally
discovered migrations, and fail startup when an applied-only entry is found.
Preserve existing checksum validation for discovered migrations while ensuring
missing artifact directories cannot be silently ignored.
- Around line 49-52: Remove the locked.rows[0].pg_advisory_lock condition and
its error throw from the migration lock flow; retain the client.query call so
migrations proceed after PostgreSQL acquires the advisory lock.
In `@control-plane/src/domains/attestation/attestationService.ts`:
- Around line 38-43: Make deduplication in the attestation submission flow
atomic rather than relying on the pre-check in attestationService: enforce
uniqueness for tokenDigest at the database/repository boundary and translate a
uniqueness violation during insertion into AppError(CODES.CONFLICT). Update the
flow around findByTokenDigest and the attestation insert so concurrent
submissions consistently produce the intended conflict response.
- Around line 104-108: Update getAttestation to resolve attestations by their
actual id rather than passing the id to findByTokenDigest. Add or use an
attestationRepo repository lookup that queries the attestation ID, remove the
unresolved “or by UUID” comment, and preserve the existing NOT_FOUND error
behavior when no record is returned.
- Around line 8-11: Update canonicalize to preserve all nested object properties
while producing deterministic JSON: replace the top-level replacer-array
approach with recursive key sorting, or use a proper RFC 8785 JCS
implementation. Ensure nested facts fields are included in the serialized input
and consistently ordered for hashing.
In `@control-plane/src/domains/principal/principalRepository.ts`:
- Around line 113-123: Update createIssuer so re-registration without an
explicitly supplied trustWeight preserves the issuer’s existing database value
instead of overwriting it with 1.0; distinguish omitted weights from explicit
values and only apply the upsert update when a weight was provided, while
retaining the default for new issuers.
In `@control-plane/src/domains/principal/principalService.ts`:
- Around line 53-61: Synchronize issuer promotion and eligibility: in
control-plane/src/domains/principal/principalService.ts lines 53-61, update
createIssuer to change entity_kind from agent to both (and preserve issuer for
other valid kinds) when creating the issuer record; in
control-plane/src/domains/attestation/attestationService.ts lines 59-66, replace
the entity_kind-only check with an issuers-table membership lookup so principals
without an issuer row, including missing trust_weight records, are rejected.
- Around line 31-46: Update addKey to catch the repository’s database
unique-violation error from principalRepo.addKey, including duplicate key_id
conflicts, and translate it into AppError(CODES.CONFLICT, ...) with a clear
conflict message; rethrow all other errors unchanged.
In `@control-plane/src/domains/sync/syncRepository.ts`:
- Around line 13-27: Update appendEvent to serialize sync_version allocation
with a transaction-scoped PostgreSQL advisory lock before calculating
MAX(sync_version). Keep the allocation and INSERT in the same transaction, then
return the inserted sync_version while preserving the existing event fields and
payload handling.
In `@control-plane/src/index.ts`:
- Around line 23-39: Make the shutdown handler idempotent by adding a shared
shuttingDown guard around shutdown. Return immediately when shutdown has already
started, and set the guard before calling server.close so repeated
SIGTERM/SIGINT signals cannot re-enter the draining or cleanup logic.
In `@control-plane/src/middleware/auth.ts`:
- Around line 124-135: Update the membership lookup and request-authentication
flow around req.user so it never assigns memberships[0] from an unordered LIMIT
1 query. Require a tenant ID from a verified claim or trusted request context,
query and validate that the user belongs to that tenant, and reject requests
when the tenant is missing, invalid, or ambiguous; only then populate tenantId
and role.
- Around line 143-149: Update apiKeyOnly to propagate asynchronous failures from
authenticateApiKey to Express by making the middleware async and awaiting the
helper, or by returning its promise with a catch that calls next(err). Preserve
the existing missing-key handling and successful authentication flow.
In `@control-plane/src/routes/attestations.ts`:
- Around line 10-21: The /submit handler currently persists client-supplied
verified claims without authenticating the JWS token. Replace the placeholder
flow in the handler with the trust-engine VerifyAttestation result, using token
as the verification input and passing only the returned verified payload to
attestationService.submitAttestation; reject verification failures and malformed
requests before persistence, and remove reliance on req.body.verified.
In `@control-plane/src/routes/principals.ts`:
- Around line 28-38: Validate the request body’s entity_kind in the POST handler
before calling principalService.createPrincipal, restricting it to the same
allowed values as the GET handler: agent, issuer, or both. Reject invalid values
through the route’s existing validation/error mechanism and only pass validated
input to createPrincipal.
- Around line 48-61: Validate public_key_raw in the POST /:id/keys handler
before passing it to Buffer.from or principalService.addKey, rejecting missing,
empty, or malformed base64 input through the route’s established
validation/error mechanism. Update the sibling validation site in
control-plane/src/routes/principals.ts lines 28-38 consistently if it handles
the same key-body fields; otherwise make no direct change there.
In `@control-plane/src/routes/sync.ts`:
- Around line 44-48: Update the sync route’s high-water-mark handling to stop
calling syncService.getEventsSince(0) and instead obtain the value from
syncRepository.getHighWaterVersion() through a corresponding syncService method.
Use that version to send the intended high-water-mark cursor event before ending
the response, removing the unused result and incomplete comments.
- Around line 10-19: Update the tenant validation in the snapshot handler to
throw the established AppError for the forbidden/missing-tenant case instead of
writing the response directly. Let defineHandler route the error through
next(err) and preserve the existing syncService.getSnapshot and ok response flow
for valid tenant IDs.
In `@control-plane/src/shared/errors/AppError.ts`:
- Around line 46-51: Update AppError.from() so all non-AppError inputs produce
an INTERNAL AppError with a generic client-safe message instead of propagating
err.message or String(err); retain the original Error as the cause for
server-side logging when available.
In `@control-plane/src/shared/http/defineHandler.ts`:
- Around line 20-29: Extend validateParam to enforce each ParamDef’s declared
type, min, and max constraints before accepting a value. Reject array-valued
query parameters, validate numeric, boolean, and UUID formats, and apply numeric
bounds inclusively so constraints such as limit 1..200 reach the handler only
when valid; preserve the existing required and enum checks.
In `@control-plane/src/shared/http/responses.ts`:
- Around line 32-35: Replace console.error with the shared logger.error in the
error helper, preserving the existing AppError context and message while routing
fields through Pino redaction. Apply the same change in
control-plane/src/shared/http/responses.ts lines 32-35 and
control-plane/src/middleware/audit.ts lines 32-35, using each file’s existing
logger symbol.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b1aa9d94-86c7-4ea0-a87f-d21355929819
⛔ Files ignored due to path filters (1)
control-plane/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
control-plane/.env.examplecontrol-plane/.gitignorecontrol-plane/migrations/001_graph/migration.sqlcontrol-plane/migrations/002_attestations/migration.sqlcontrol-plane/migrations/003_sync/migration.sqlcontrol-plane/migrations/004_tenancy/migration.sqlcontrol-plane/migrations/005_policy/migration.sqlcontrol-plane/migrations/006_audit/migration.sqlcontrol-plane/package.jsoncontrol-plane/src/app.tscontrol-plane/src/config.tscontrol-plane/src/db/client.tscontrol-plane/src/db/migrate.tscontrol-plane/src/db/transaction.tscontrol-plane/src/domains/attestation/attestationRepository.tscontrol-plane/src/domains/attestation/attestationService.tscontrol-plane/src/domains/principal/principalRepository.tscontrol-plane/src/domains/principal/principalService.tscontrol-plane/src/domains/sync/syncRepository.tscontrol-plane/src/domains/sync/syncService.tscontrol-plane/src/index.tscontrol-plane/src/middleware/audit.tscontrol-plane/src/middleware/auth.tscontrol-plane/src/middleware/rateLimit.tscontrol-plane/src/middleware/requestTracker.tscontrol-plane/src/routes/attestations.tscontrol-plane/src/routes/principals.tscontrol-plane/src/routes/sync.tscontrol-plane/src/shared/errors/AppError.tscontrol-plane/src/shared/http/defineHandler.tscontrol-plane/src/shared/http/responses.tscontrol-plane/src/shared/logger.tscontrol-plane/tsconfig.jsondocs/superpowers/plans/2026-07-27-verilink-2-control-plane-foundation.md
Migrations: - Add citext extension before users table (004_tenancy) - Add NOT NULL + non-negative CHECK on trust_weight (001_graph) - Add non-negative CHECK on current_weight (003_sync) - Remove redundant token_digest index (002_attestations) - Add CHECK constraints on max_snapshot_age_seconds, allow_sample_rate (005_policy) - Add CHECK(count>=0), CHECK(action IN ...), FK to edge_nodes on decision tables (006_audit) Config: - Remove localhost fallback for DATABASE_URL DB: - Use logger instead of console.error in pool error handler - Add migration drift detection, remove unnecessary lock boolean check Attestation domain: - Fix getAttestation to query by UUID (findById), not token_digest - Make canonicalize recursive (sort nested object keys) - Make dedup atomic via DB unique constraint Principal domain: - createIssuer: only update trust_weight when explicitly provided - addKey: catch DB unique violation → AppError CONFLICT - createIssuer: update entity_kind to 'both' when upgrading agent Sync domain: - appendEvent: use transaction + advisory lock for version allocation - syncService: add getHighWaterVersion re-export Server: - Make shutdown handler idempotent (shuttingDown guard) Auth: - Make apiKeyOnly async with proper error handling Routes: - Validate entity_kind in POST /principals - Validate public_key_raw in POST /principals/:id/keys - Use AppError for 403 in sync snapshot - Fix high water mark SSE cursor event Shared: - AppError.from(): generic message for non-AppError inputs - defineHandler: enforce type/min/max validation - responses.ts: use logger instead of console.error - audit.ts: use logger instead of console.error Other: - .gitignore: ignore .env.* except .env.example
Summary
Implements Plan 1 (Trust Engine + gRPC server) and Plan 2 (Control-plane TypeScript foundation + data model) from the VeriLink productization spec.
Plan 1: Trust Engine + gRPC (8 commits)
pkg/trust/): deterministic scoring, trust_weight, weighted roots, blacklist/score_reasoncmd/trust-engine/): RunVeriRank, VerifyAttestation, GetFingerprint, health endpointsPlan 2: Control-plane TypeScript Foundation (18 commits)
process.envoutside config.ts)withTransaction()helper, migration runner with advisory lockAppError, response helpers (ok/created/error),defineHandler, pino logger/v1/principals,/v1/attestations,/v1/syncwith defineHandler validationWhat's NOT in this PR (covered by subsequent plans)
Test plan
npx tsc --noEmit— zero errors)npm run dev+ curl (requires Postgres + Clerk credentials)Files changed
See commit history for full file list. Key directories:
control-plane/— New TypeScript Express apppkg/trust/— VeriRank engine (from Plan 1)cmd/trust-engine/— gRPC server (from Plan 1)Co-Authored-By: opencode opencode@numeracode.com
Summary by CodeRabbit