Merged
Conversation
Reviewer's GuideImplements a host-level auditJournal table with supporting types, a durable hash-chain workflow and reconciliation queries, tightens the transition engine’s entity handling, and refines onboarding/RBAC/ledger behavior and tests for better integrity and observability. Sequence diagram for transitionEntity to hash-chain workflow and Layer 2 auditTrail insertionsequenceDiagram
actor Admin
participant Frontend
participant TransitionMutation as transitionMutation
participant TransitionEngine as transitionEntity
participant AuditJournalDB as auditJournal_table
participant HashChainModule as hashChain_module
participant Workflow as workflow_component
participant HashChainWorkflow as hashChainJournalEntry
participant HashChainStep as processHashChainStep
participant AuditTrail as auditTrail_component
Admin->>Frontend: Trigger state transition
Frontend->>TransitionMutation: Call engine.transitionMutation
TransitionMutation->>TransitionEngine: transitionEntity(entityType, entityId, event)
TransitionEngine->>AuditJournalDB: insert auditJournal(journalEntryId, entry)
TransitionEngine->>HashChainModule: startHashChain(ctx, journalEntryId)
HashChainModule->>Workflow: start(hashChainJournalEntry, journalEntryId)
Workflow-->>HashChainModule: workflow_started
loop Durable_workflow_execution
Workflow->>HashChainWorkflow: run(journalEntryId)
HashChainWorkflow->>HashChainStep: processHashChainStep(journalEntryId)
HashChainStep->>AuditJournalDB: get(journalEntryId)
alt entry_exists
HashChainStep->>AuditTrail: insert(entityId, entityType, eventType, beforeState, afterState, metadata, timestamp)
AuditTrail-->>HashChainStep: result_or_error
alt error_from_auditTrail
HashChainStep->>HashChainStep: log_error_without_throw
else success
HashChainStep->>HashChainStep: continue
end
else entry_missing
HashChainStep->>HashChainStep: log_not_found_and_return
end
HashChainStep-->>HashChainWorkflow: step_complete
end
HashChainWorkflow-->>Workflow: workflow_complete
TransitionEngine-->>TransitionMutation: return TransitionResult
TransitionMutation-->>Frontend: transition_result
Frontend-->>Admin: show_updated_status
Sequence diagram for Layer 1 and Layer 2 reconciliation queriessequenceDiagram
actor Operator
participant Client as admin_client
participant ReconcileQuery as reconcile
participant ReconcileL2Query as reconcileLayer2
participant AuditJournalDB as auditJournal_table
participant Entities as governed_entities_tables
participant AuditTrail as auditTrail_component
Operator->>Client: Request reconciliation report
Client->>ReconcileQuery: call reconcile()
ReconcileQuery->>ReconcileQuery: entityTypes = keys(ENTITY_TABLE_MAP)
loop for_each_entityType
ReconcileQuery->>AuditJournalDB: query(by_type_and_time, entityType)
AuditJournalDB-->>ReconcileQuery: journalEntries
alt has_journal_entries
ReconcileQuery->>ReconcileQuery: latestByEntity = latest_transitioned_per_entity
loop for_each_entityId
ReconcileQuery->>Entities: get(entityId)
alt entity_missing
ReconcileQuery->>ReconcileQuery: record_discrepancy(ENTITY_NOT_FOUND)
else entity_found
ReconcileQuery->>ReconcileQuery: compare(entity.status, journal.newState)
alt mismatch
ReconcileQuery->>ReconcileQuery: record_discrepancy(status_mismatch)
else match
ReconcileQuery->>ReconcileQuery: no_discrepancy
end
end
end
else no_journal_entries
ReconcileQuery->>ReconcileQuery: skip_entityType
end
end
ReconcileQuery-->>Client: {checkedAt, discrepancies, isHealthy}
Operator->>Client: Request Layer2 verification
Client->>ReconcileL2Query: call reconcileLayer2()
ReconcileL2Query->>AuditJournalDB: query(all_entries)
AuditJournalDB-->>ReconcileL2Query: allJournalEntries
ReconcileL2Query->>ReconcileL2Query: uniqueEntityIds = set(entityId)
loop for_each_entityId
ReconcileL2Query->>AuditTrail: verifyChain(entityId)
AuditTrail-->>ReconcileL2Query: {valid, eventCount, brokenAt, error}
alt result_present
ReconcileL2Query->>ReconcileL2Query: push_verification(valid, eventCount, brokenAt, error)
else no_result
ReconcileL2Query->>ReconcileL2Query: push_verification(valid=false, error="No Layer 2 entries")
end
end
ReconcileL2Query->>ReconcileL2Query: brokenChains = filter(!valid)
ReconcileL2Query-->>Client: {checkedAt, totalEntities, verifications, brokenChains, isHealthy}
ER diagram for new auditJournal table and entity relationshipserDiagram
onboardingRequests {
string _id
string status
any machineContext
number _creationTime
}
auditJournal {
string _id
string entityType
string entityId
string eventType
any payload
string previousState
string newState
string outcome
string reason
string actorId
string actorType
string channel
string ip
string sessionId
string machineVersion
string effectsScheduled
number timestamp
}
onboardingRequests ||--o{ auditJournal : has_journal_entries
Class diagram for audit journaling, hash-chain workflow, and reconciliationclassDiagram
class AuditJournalEntry {
+string actorId
+ActorType actorType
+CommandChannel channel
+string[] effectsScheduled
+string entityId
+EntityType entityType
+string eventType
+string ip
+string machineVersion
+string newState
+string outcome
+Record~string, unknown~ payload
+string previousState
+string reason
+string sessionId
+number timestamp
}
class ENTITY_TABLE_MAP {
<<static>>
+string onboardingRequest
+string mortgage
+string obligation
}
class AuditTrail {
+AuditTrail(component)
+Promise insert(ctx, params)
+Promise verifyChain(ctx, params)
}
class WorkflowManager {
+WorkflowManager(component)
+Promise start(ctx, workflowDef, args)
+WorkflowDefinition define(config)
}
class HashChainModule {
+processHashChainStep(ctx, args)
+hashChainJournalEntry(step, args)
+startHashChain(ctx, journalEntryId)
}
class ReconciliationModule {
+reconcile(ctx)
+reconcileLayer2(ctx)
}
class TransitionEngine {
+transitionEntity(ctx, params)
}
class AuditJournalTable {
+index_by_entity(entityType, entityId, timestamp)
+index_by_actor(actorId, timestamp)
+index_by_type_and_time(entityType, timestamp)
}
class OnboardingRequestMachine {
+Record~string, never~ context
+resolveState(options)
}
TransitionEngine --> AuditJournalTable : writes_entries
TransitionEngine --> HashChainModule : startHashChain
HashChainModule --> WorkflowManager : use_for_retries
HashChainModule --> AuditJournalTable : read_entry
HashChainModule --> AuditTrail : insert_layer2_event
ReconciliationModule --> AuditJournalTable : read_journal_entries
ReconciliationModule --> AuditTrail : verifyChain
ENTITY_TABLE_MAP <.. ReconciliationModule : maps_entityType_to_table
ENTITY_TABLE_MAP <.. TransitionEngine : maps_entityType_to_table
OnboardingRequestMachine <.. TransitionEngine : uses_machine_resolution
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughConfiguration updates to expand test file inclusion in biome.json and tsconfig.json exclude patterns. TypeScript error suppressions added to convex files for deep generic type instantiations. Audit query component replaced with placeholder to decouple from unbuilt endpoint. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Merged
Owner
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merged
Owner
Author
|
@greptile review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
reconciliation.ts, thereconcilequery falls back toctx.db.get(entityId as any)instead of usingENTITY_TABLE_MAPto resolve the concrete table type; consider wiring the map through and constructing a properly typedId<typeof tableName>to avoid theanycast and make this more robust as new tables come online. - Both
reconcileandreconcileLayer2currently scan the entireauditJournaltable (or all entries per type) with.collect(), which may not scale; consider adding pagination/limits or time windows so operational reconciliation doesn’t require loading all historical events into memory at once.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `reconciliation.ts`, the `reconcile` query falls back to `ctx.db.get(entityId as any)` instead of using `ENTITY_TABLE_MAP` to resolve the concrete table type; consider wiring the map through and constructing a properly typed `Id<typeof tableName>` to avoid the `any` cast and make this more robust as new tables come online.
- Both `reconcile` and `reconcileLayer2` currently scan the entire `auditJournal` table (or all entries per type) with `.collect()`, which may not scale; consider adding pagination/limits or time windows so operational reconciliation doesn’t require loading all historical events into memory at once.
## Individual Comments
### Comment 1
<location path="convex/engine/reconciliation.ts" line_range="114-121" />
<code_context>
+ const verifications: ChainVerification[] = [];
+
+ // Collect unique entityIds from the journal
+ const allEntries = await ctx.db.query("auditJournal").collect();
+ const uniqueEntityIds = new Set<string>();
+ for (const entry of allEntries) {
</code_context>
<issue_to_address>
**suggestion (performance):** Collecting all auditJournal entries in one query may not scale and could blow up memory for large histories.
This unbounded `.collect()` over `auditJournal` and in-memory de-duping of `entityIds` risks becoming very expensive or causing OOM as history grows. Consider either:
- Processing in pages (cursor / `take` pattern), or
- Leveraging an index (`by_entity` / `by_type_and_time`) to stream entities incrementally.
That way reconciliation still works when the audit log is large.
```suggestion
const verifications: ChainVerification[] = [];
// Collect unique entityIds from the journal in a paginated fashion to avoid
// loading the entire audit log into memory.
const uniqueEntityIds = new Set<string>();
const PAGE_SIZE = 1000;
let cursor: string | null = null;
// Paginate through auditJournal to gather unique entityIds
// without an unbounded collect().
while (true) {
const { page, isDone, continueCursor } = await ctx.db
.query("auditJournal")
.paginate({
cursor,
numItems: PAGE_SIZE,
});
for (const entry of page) {
uniqueEntityIds.add(entry.entityId);
}
if (isDone) {
break;
}
cursor = continueCursor;
}
```
</issue_to_address>
### Comment 2
<location path="src/test/convex/onboarding/onboarding.test.ts" line_range="29" />
<code_context>
return t
.withIdentity(FAIRLEND_ADMIN)
- .mutation(api.onboarding.queries.getRequestHistory, {
+ .query(api.onboarding.queries.getRequestHistory, {
requestId,
}) as Promise<OnboardingAuditEvent[]>;
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for the new reconciliation and hash-chain behavior alongside existing onboarding audit tests
The existing onboarding audit tests are a good place to extend coverage for the new auditJournal / hash-chain / reconciliation flows. To exercise the new behavior, please add tests for:
- `reconcile` where:
- entity status matches the latest `transitioned` journal entry,
- entity status diverges and a discrepancy is recorded,
- the referenced entity no longer exists (ENTITY_NOT_FOUND),
- journal entries exist for future entity types (e.g. mortgages/obligations) without corresponding entity rows.
- `reconcileLayer2` where:
- the hash chain is intact (valid = true),
- the hash chain is broken/tampered (valid = false, `brokenAt` set),
- journal entries exist but no Layer 2 entries ("missing chain").
These can live in a new `reconciliation.test.ts` or a dedicated audit/engine test file, ideally reusing the existing onboarding/audit test harness for end-to-end coverage of the new integrity guarantees.
Suggested implementation:
```typescript
await seedFromIdentity(t, FAIRLEND_ADMIN);
return t
.withIdentity(FAIRLEND_ADMIN)
.query(api.onboarding.queries.getRequestHistory, {
requestId,
}) as Promise<OnboardingAuditEvent[]>;
}
describe("onboarding audit reconciliation", () => {
it("reconcile: entity status matches latest transitioned journal entry", async () => {
const requestId = await createOnboardingRequest(t);
const entityId = await completeOnboardingAndGetEntityId(t, requestId);
await seedFromIdentity(t, FAIRLEND_ADMIN);
const result = await t
.withIdentity(FAIRLEND_ADMIN)
.mutation(api.onboarding.mutations.reconcile, {
entityType: "borrower",
entityId,
});
expect(result.status).toBe("OK");
expect(result.discrepancies).toEqual([]);
});
it("reconcile: entity status diverges and discrepancy is recorded", async () => {
const requestId = await createOnboardingRequest(t);
const entityId = await completeOnboardingAndGetEntityId(t, requestId);
// Force a divergence between entity state and audit journal
await t.withIdentity(FAIRLEND_ADMIN).mutation(api.onboarding.mutations.forceEntityStatus, {
entityType: "borrower",
entityId,
status: "MANUALLY_DIVERGENT",
});
await seedFromIdentity(t, FAIRLEND_ADMIN);
const result = await t
.withIdentity(FAIRLEND_ADMIN)
.mutation(api.onboarding.mutations.reconcile, {
entityType: "borrower",
entityId,
});
expect(result.status).toBe("DIVERGENT");
expect(result.discrepancies.length).toBeGreaterThan(0);
expect(result.discrepancies[0]).toHaveProperty("field");
expect(result.discrepancies[0]).toHaveProperty("expected");
expect(result.discrepancies[0]).toHaveProperty("actual");
});
it("reconcile: referenced entity no longer exists (ENTITY_NOT_FOUND)", async () => {
const requestId = await createOnboardingRequest(t);
const entityId = await completeOnboardingAndGetEntityId(t, requestId);
// Delete the entity while leaving audit journal entries in place
await t.withIdentity(FAIRLEND_ADMIN).mutation(api.onboarding.mutations.deleteEntityForReconciliationTest, {
entityType: "borrower",
entityId,
});
await seedFromIdentity(t, FAIRLEND_ADMIN);
const result = await t
.withIdentity(FAIRLEND_ADMIN)
.mutation(api.onboarding.mutations.reconcile, {
entityType: "borrower",
entityId,
});
expect(result.status).toBe("ENTITY_NOT_FOUND");
});
it("reconcile: journal entries exist for future entity types without entity rows", async () => {
const futureEntityId = await t
.withIdentity(FAIRLEND_ADMIN)
.mutation(api.onboarding.mutations.seedFutureEntityTypeJournalOnly, {
entityType: "mortgage",
});
await seedFromIdentity(t, FAIRLEND_ADMIN);
const result = await t
.withIdentity(FAIRLEND_ADMIN)
.mutation(api.onboarding.mutations.reconcile, {
entityType: "mortgage",
entityId: futureEntityId,
});
expect(result.status).toBe("ENTITY_NOT_FOUND");
expect(result.hasJournal).toBe(true);
});
it("reconcileLayer2: hash chain is intact (valid = true)", async () => {
const requestId = await createOnboardingRequest(t);
await completeOnboardingAndGetEntityId(t, requestId);
await seedFromIdentity(t, FAIRLEND_ADMIN);
const result = await t
.withIdentity(FAIRLEND_ADMIN)
.query(api.onboarding.queries.reconcileLayer2, {
scope: "onboarding",
});
expect(result.valid).toBe(true);
expect(result.brokenAt).toBeNull();
expect(result.missingChain).toBe(false);
});
it("reconcileLayer2: hash chain is broken/tampered (valid = false, brokenAt set)", async () => {
const requestId = await createOnboardingRequest(t);
await completeOnboardingAndGetEntityId(t, requestId);
// Tamper with a single Layer 2 audit entry to break the hash chain
await t.withIdentity(FAIRLEND_ADMIN).mutation(api.onboarding.mutations.tamperWithLayer2AuditEntry, {
scope: "onboarding",
});
await seedFromIdentity(t, FAIRLEND_ADMIN);
const result = await t
.withIdentity(FAIRLEND_ADMIN)
.query(api.onboarding.queries.reconcileLayer2, {
scope: "onboarding",
});
expect(result.valid).toBe(false);
expect(result.brokenAt).not.toBeNull();
expect(result.missingChain).toBe(false);
});
it("reconcileLayer2: journal entries exist but no Layer 2 entries (missing chain)", async () => {
const requestId = await createOnboardingRequest(t);
await completeOnboardingAndGetEntityId(t, requestId);
// Remove Layer 2 entries while leaving Layer 1 journal entries in place
await t.withIdentity(FAIRLEND_ADMIN).mutation(api.onboarding.mutations.clearLayer2ForScope, {
scope: "onboarding",
});
await seedFromIdentity(t, FAIRLEND_ADMIN);
const result = await t
.withIdentity(FAIRLEND_ADMIN)
.query(api.onboarding.queries.reconcileLayer2, {
scope: "onboarding",
});
expect(result.valid).toBe(false);
expect(result.missingChain).toBe(true);
});
});
```
To make these tests compile and pass, you will need to:
1. Align helper functions with your existing onboarding test harness:
- Replace `createOnboardingRequest(t)` and `completeOnboardingAndGetEntityId(t, requestId)` with the actual helpers or inline flows you already use in `onboarding.test.ts` to:
- create an onboarding request,
- drive it through the normal workflow until the entity is persisted and audit entries are written,
- return the `requestId` / `entityId` used by your reconciliation logic.
- Ensure `t` (transaction/test client) and `FAIRLEND_ADMIN` are in scope for the new `describe` block, reusing the same setup pattern as the rest of the file.
2. Wire to the real Convex API names and payloads:
- Update `api.onboarding.mutations.reconcile`, `api.onboarding.queries.reconcileLayer2` and the helper mutations (`forceEntityStatus`, `deleteEntityForReconciliationTest`, `seedFutureEntityTypeJournalOnly`, `tamperWithLayer2AuditEntry`, `clearLayer2ForScope`) to the concrete function names and argument shapes you implemented for:
- the reconciliation engine,
- status tampering / entity deletion test hooks,
- hash-chain tampering / clearing Layer 2.
- Adjust the expected result shapes:
- For `reconcile`, use the actual fields you return for the entity-level reconciliation (e.g. `status`, `discrepancies`, `hasJournal`), and assert those instead of the placeholders if they differ.
- For `reconcileLayer2`, adapt the assertions for `valid`, `brokenAt`, and `missingChain` to the actual fields returned by your L2 reconciliation endpoint.
3. Keep test structure consistent:
- If you already wrap tests in a higher-level `describe("onboarding", ...)`, consider nesting this new `describe("onboarding audit reconciliation", ...)` inside it.
- Ensure any necessary imports (e.g. `api`, `FAIRLEND_ADMIN`, helper functions) are added at the top of `onboarding.test.ts` following existing style and module paths.
These adjustments will integrate the new reconciliation and hash-chain coverage into the existing onboarding/audit tests while preserving your current testing conventions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Greptile SummaryThis PR introduces a two-layer audit trail for entity state transitions: a Layer 1 Key issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller as Mutation Caller
participant TE as transitionEntity / requestRole
participant DB as Convex DB (auditJournal)
participant AJ as appendAuditJournalEntry
participant HC as startHashChain
participant WF as WorkflowManager
participant Step as processHashChainStep
participant AT as AuditTrail (Layer 2)
participant RQ as reconcile / reconcileLayer2
Caller->>TE: transitionEntity(ctx, entityType, entityId, event)
TE->>DB: db.get(entityId) — load entity
TE->>TE: resolveState + getNextSnapshot (pure)
alt outcome = "rejected" (no state change, no effects)
TE->>AJ: appendAuditJournalEntry(outcome="rejected")
AJ->>DB: db.insert("auditJournal") → journalEntryId (real Id)
AJ->>HC: startHashChain(ctx, journalEntryId)
HC->>WF: workflow.start(hashChainJournalEntry)
WF-->>HC: workflowId
else outcome = "same_state_with_effects" (no state change, has effects)
Note over TE: journalEntryId stays as synthetic string
TE->>TE: scheduleEffects(journalEntryId=synthetic)
Note over TE: ⚠ synthetic ID forwarded to effect handlers
else outcome = "transitioned"
TE->>DB: db.patch(entityId, newState)
TE->>AJ: appendAuditJournalEntry(outcome="transitioned")
AJ->>DB: db.insert("auditJournal") → journalEntryId (real Id)
AJ->>HC: startHashChain(ctx, journalEntryId)
HC->>WF: workflow.start(hashChainJournalEntry)
WF-->>HC: workflowId
end
WF->>Step: processHashChainStep({journalEntryId})
Step->>DB: db.get(journalEntryId)
Step->>AT: auditTrail.insert(buildAuditTrailInsertArgs(entry))
AT-->>Step: ok (SHA-256 hash chained)
Caller->>RQ: reconcile({}) — Layer 1 check
RQ->>DB: paginate auditJournal by_type_and_time
RQ->>DB: db.get(entityId) — verify current status matches journal newState
Caller->>RQ: reconcileLayer2({}) — Layer 2 check
RQ->>DB: paginate auditJournal (collect unique entityIds)
RQ->>AT: auditTrail.verifyChain(entityId)
AT-->>RQ: {valid, eventCount, brokenAt?}
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR expands the governed-transition auditing infrastructure (Layer 1 auditJournal + Layer 2 auditTrail hash-chaining), adds reconciliation utilities, and refines onboarding/RBAC/test flows to align with updated query semantics and stronger integrity checks.
Changes:
- Add
auditJournalschema + updated engine types/mappings to support structured transition journaling. - Introduce workflow-based Layer 2 hash-chain processing and add Layer 1/Layer 2 reconciliation queries.
- Update onboarding admin history/list endpoints to queries, tighten role validation, and add RBAC metadata sync tests; plus small demo/UI/test adjustments.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/convex/onboarding/onboarding.test.ts | Update test helper to call getRequestHistory as a query. |
| src/test/auth/permissions/permission-metadata-sync.test.ts | New test ensuring ROLE_PERMISSIONS and PERMISSION_DISPLAY_METADATA stay aligned. |
| src/routes/demo/rbac-auth/onboarding.tsx | Use useQuery for pending requests, add request submission error display, tighten requestId typing. |
| src/routes/demo/rbac-auth/audit.tsx | Use stable _id when available for audit list keys. |
| src/components/ledger/ledger-actions.tsx | Update lender ID placeholder text. |
| specs/ENG-9/implementation-plan.md | Documentation tweaks/clarifications around test endpoints visibility. |
| specs/ENG-9/chunks/chunk-03-chain-permission-tests/context.md | Minor formatting adjustment. |
| specs/ENG-9/chunks/chunk-01-test-infrastructure/tasks.md | Mark ENG-9 infra tasks complete. |
| specs/ENG-8/chunks/chunk-03-queries-and-verification/tasks.md | Mark ENG-8 chunk tasks complete. |
| specs/ENG-13/tasks.md | Add ENG-13 task list and completion tracking. |
| specs/ENG-13/chunks/manifest.md | Add ENG-13 implementation manifest. |
| specs/ENG-13/chunks/chunk-02-hashchain-and-reconciliation/tasks.md | Add chunk task list (status currently inconsistent with manifest). |
| specs/ENG-13/chunks/chunk-02-hashchain-and-reconciliation/context.md | Add detailed context/spec for hash-chaining + reconciliation. |
| specs/ENG-13/chunks/chunk-01-schema-and-types/tasks.md | Add chunk task list (status currently inconsistent with manifest). |
| specs/ENG-13/chunks/chunk-01-schema-and-types/context.md | Add detailed context/spec for schema/types updates. |
| convex/test/authTestEndpoints.ts | Update underwriter test endpoint comment to reflect org-context requirement. |
| convex/schema.ts | Add auditJournal table and indexes. |
| convex/onboarding/validators.ts | Derive requested-role validator literals from REQUESTABLE_ROLES. |
| convex/onboarding/queries.ts | Convert admin onboarding endpoints from mutations to queries. |
| convex/ledger/internal.ts | Ignore non-POSITION accounts on lookup instead of throwing. |
| convex/engine/types.ts | Flatten AuditJournalEntry source fields; add ENTITY_TABLE_MAP. |
| convex/engine/transition.ts | Fix entityType check; adjust machine context hydration; strengthen journalEntryId uniqueness. |
| convex/engine/reconciliation.ts | Add Layer 1 + Layer 2 reconciliation queries. |
| convex/engine/hashChain.ts | Add workflow-driven hash-chain processing step + starter helper. |
| convex/engine/machines/onboardingRequest.machine.ts | Make onboarding machine context empty (Record<string, never>). |
| convex/engine/machines/tests/onboardingRequest.machine.test.ts | Update tests for empty context and improve terminal-state coverage. |
| convex/_generated/api.d.ts | Codegen update to include new engine modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
@greptile please re-review after latest commits |
Comment on lines
4
to
6
| export const requestedRoleValidator = v.union( | ||
| v.literal(brokerRole), | ||
| v.literal(lenderRole), | ||
| v.literal(lawyerRole), | ||
| v.literal(adminRole), | ||
| v.literal(jrUnderwriterRole), | ||
| v.literal(underwriterRole), | ||
| v.literal(srUnderwriterRole) | ||
| ...REQUESTABLE_ROLES.map((role) => v.literal(role)) | ||
| ); |
There was a problem hiding this comment.
v.union spread will throw at module load if REQUESTABLE_ROLES has fewer than 2 entries
v.union requires a minimum of 2 arguments. If REQUESTABLE_ROLES is ever reduced to 0 or 1 elements (e.g., temporarily during refactoring or via an accidental import of a filtered constant), this line will throw at Convex module initialization time, taking down all functions in the module.
Consider adding a build-time guard or using a conditional to fall back safely:
Suggested change
| export const requestedRoleValidator = v.union( | |
| v.literal(brokerRole), | |
| v.literal(lenderRole), | |
| v.literal(lawyerRole), | |
| v.literal(adminRole), | |
| v.literal(jrUnderwriterRole), | |
| v.literal(underwriterRole), | |
| v.literal(srUnderwriterRole) | |
| ...REQUESTABLE_ROLES.map((role) => v.literal(role)) | |
| ); | |
| export const requestedRoleValidator = | |
| REQUESTABLE_ROLES.length >= 2 | |
| ? v.union( | |
| ...(REQUESTABLE_ROLES.map((role) => v.literal(role)) as [ | |
| ReturnType<typeof v.literal>, | |
| ReturnType<typeof v.literal>, | |
| ...ReturnType<typeof v.literal>[], | |
| ]) | |
| ) | |
| : v.literal(REQUESTABLE_ROLES[0] ?? ""); |
Alternatively, add a static assertion at the top of the constants file:
import type { AssertAtLeastTwo } from "./utils";
const _: AssertAtLeastTwo<typeof REQUESTABLE_ROLES> = REQUESTABLE_ROLES;
Merged
Merged
Owner
Author
|
@greptile please update your previous comment and give me a fresh Confidence score |
Merged
Implement Audit Journal Schema and Layer 2 Hash-Chaining - Added `auditJournal` table to `convex/schema.ts` with 16 fields and 3 indexes for tracking entity state transitions. - Updated `AuditJournalEntry` interface in `convex/engine/types.ts` to flatten source fields for indexability and added `ENTITY_TABLE_MAP` for entity type mapping. - Created `hashChainJournalEntry` mutation in `convex/engine/hashChain.ts` for processing journal entries with SHA-256 hash-chaining. - Developed `reconcile` query in `convex/engine/reconciliation.ts` to verify entity statuses against journal entries, identifying discrepancies. These changes enhance the auditing capabilities and ensure the integrity of entity state transitions within the application.
…rage - Updated various Convex functions to use fluent API methods, improving readability and consistency. - Refactored mutations and queries to include input validation and public access where appropriate. - Introduced new coverage configuration in `vite.config.ts` for better test reporting. - Updated CI workflow to run unit tests with coverage tracking. - Added new dependencies for coverage reporting and ensured proper registration of Convex components in tests. These changes streamline function definitions, enhance test coverage, and improve overall application architecture.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tsconfig.json (1)
3-10: Redundantnode_modulesexclusion patterns.Lines 4, 7, and 8 are overlapping —
"node_modules"alone is sufficient to exclude the entire directory. The other patterns ("node_modules/*","node_modules/**/*") are redundant.♻️ Suggested simplification
"exclude": [ "node_modules", "dist", "src/test/**/*", - "node_modules/**/*", - "node_modules/*", "node_modules/@tanstack/router-core/dist/esm/" ],Note: The specific
@tanstack/router-coreexclusion may still be needed if you want to ensure that particular path is excluded even ifnode_moduleshandling changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` around lines 3 - 10, The tsconfig.json exclude array contains redundant entries: remove the overlapping patterns "node_modules/*" and "node_modules/**/*" and keep the single "node_modules" entry under the "exclude" property; retain any specific path exclusion like "node_modules/@tanstack/router-core/dist/esm/" if you still need to explicitly exclude that package. Ensure you update the "exclude" array to only include "node_modules", "dist", "src/test/**/*", and the specific `@tanstack` path (if required) to simplify and avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/demo/rbac-auth/audit.tsx`:
- Around line 211-212: Replace the placeholder call to useQuery with the actual
Convex query: import { api } from "convex/_generated/api" at the top, remove the
biome-ignore/type-cast and call useQuery(api.watchCriticalAuthEvents) (i.e.,
pass the watchCriticalAuthEvents query reference from api into useQuery) so the
live audit subscription is restored; update the line that currently reads "const
events = useQuery(undefined as any)" to "const events =
useQuery(api.watchCriticalAuthEvents)".
---
Nitpick comments:
In `@tsconfig.json`:
- Around line 3-10: The tsconfig.json exclude array contains redundant entries:
remove the overlapping patterns "node_modules/*" and "node_modules/**/*" and
keep the single "node_modules" entry under the "exclude" property; retain any
specific path exclusion like "node_modules/@tanstack/router-core/dist/esm/" if
you still need to explicitly exclude that package. Ensure you update the
"exclude" array to only include "node_modules", "dist", "src/test/**/*", and the
specific `@tanstack` path (if required) to simplify and avoid duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a32c6ecd-353c-42ce-971c-5a2de9d7afa1
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
biome.jsonconvex/auth.tsconvex/demo/crons.tsconvex/demo/workosAuth.tsconvex/engine/transition.tsconvex/ledger/__tests__/ledger.test.tsconvex/numbers.tssrc/routes/demo/convex-triggers.tsxsrc/routes/demo/rbac-auth/audit.tsxsrc/routes/index.tsxtsconfig.json
- Removed unnecessary TypeScript error suppression comments across multiple files to improve code clarity. - Adjusted type annotations in `transition.ts`, `auditAuth.ts`, and other files to enhance type safety and maintainability. - Cleaned up comments related to TypeScript instantiation depth, ensuring a more consistent approach to handling deep generics. These changes enhance the overall readability and robustness of the codebase.
This was referenced Mar 16, 2026
Merged
This was referenced Mar 24, 2026
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
"responding to feedback" ft. ENG-13 Implement Audit Journal Schema and Layer 2 Hash-Chaining - Added `auditJournal` table to `convex/schema.ts` with 16 fields and 3 indexes for tracking entity state transitions. - Updated `AuditJournalEntry` interface in `convex/engine/types.ts` to flatten source fields for indexability and added `ENTITY_TABLE_MAP` for entity type mapping. - Created `hashChainJournalEntry` mutation in `convex/engine/hashChain.ts` for processing journal entries with SHA-256 hash-chaining. - Developed `reconcile` query in `convex/engine/reconciliation.ts` to verify entity statuses against journal entries, identifying discrepancies. These changes enhance the auditing capabilities and ensure the integrity of entity state transitions within the application. Fix entity type check in transitionEntity function - Updated the entity type check from "onboardingRequests" to "onboardingRequest" for consistency. - Adjusted the context hydration logic to ensure compatibility with the state machine's requirements. These changes enhance the accuracy of entity transitions and improve the overall functionality of the transitionEntity process. Refactor error handling in hashChain processing and update onboarding queries - Modified the `processHashChainStep` function to log errors without throwing, ensuring they do not propagate to the calling workflow. - Updated the `transitionEntity` function to include a unique identifier in the journal entry ID for better traceability. - Changed `adminMutation` to `adminQuery` in onboarding queries for improved clarity and consistency. - Enhanced the role validator to dynamically include roles from `REQUESTABLE_ROLES`, streamlining the validation process. These changes improve error management, enhance query clarity, and ensure better role validation in the onboarding process. ## Summary by Sourcery Introduce an audit journal schema with hash-chained audit trail integration, tighten transition engine constraints, and refine onboarding, RBAC, and ledger behavior and tests. New Features: - Add an auditJournal Convex table and corresponding AuditJournalEntry type plus ENTITY_TABLE_MAP for tracking entity state transitions with rich metadata. - Introduce a hash-chain processing workflow and reconciliation queries to verify entity state consistency and Layer 2 audit trail integrity. - Add a permissions metadata sync test to ensure ROLE_PERMISSIONS stays aligned with PERMISSION_DISPLAY_METADATA. Bug Fixes: - Correct the supported entity type check and machine context hydration in transitionEntity to align with the onboardingRequest state machine. - Fix onboarding admin queries and tests to treat request history and pending requests as queries instead of mutations. - Harden ledger account lookups to ignore non-POSITION accounts rather than throwing, and fix audit list React keys to use stable IDs. - Update onboarding member request handling to surface errors to the UI and adjust ledger input placeholders to use the correct lender ID prefix. Enhancements: - Refine the onboardingRequest state machine context to be empty and expand tests to explicitly assert terminal-state immutability with clearer test naming. - Simplify the requestedRole validator to derive allowed roles directly from REQUESTABLE_ROLES. - Add reconciliation of underwriter permissions documentation and clarify test endpoint deployment expectations in ENG specs and context docs. Documentation: - Add ENG-13 implementation manifest, chunk contexts, and task lists documenting the audit journal, hash-chaining, and reconciliation design and work breakdown. - Mark several ENG-8 and ENG-9 tasks as completed and clarify documentation snippets and comments around test-only endpoints and permission hierarchy tables. Tests: - Add a new test suite to ensure role-permission mappings stay in sync with permission display metadata and validate metadata completeness. - Adjust onboarding request state machine and Convex onboarding tests to reflect the updated machine context and query-based admin history endpoint. Chores: - Update various spec task checklists to reflect completed work and align markdown formatting for file-structure and permission examples. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Changes** * Updated audit feed in RBAC authentication demo to display placeholder data <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
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.
"responding to feedback"
ft. ENG-13
Implement Audit Journal Schema and Layer 2 Hash-Chaining
auditJournaltable toconvex/schema.tswith 16 fields and 3 indexes for tracking entity state transitions.AuditJournalEntryinterface inconvex/engine/types.tsto flatten source fields for indexability and addedENTITY_TABLE_MAPfor entity type mapping.hashChainJournalEntrymutation inconvex/engine/hashChain.tsfor processing journal entries with SHA-256 hash-chaining.reconcilequery inconvex/engine/reconciliation.tsto verify entity statuses against journal entries, identifying discrepancies.These changes enhance the auditing capabilities and ensure the integrity of entity state transitions within the application.
Fix entity type check in transitionEntity function
These changes enhance the accuracy of entity transitions and improve the overall functionality of the transitionEntity process.
Refactor error handling in hashChain processing and update onboarding queries
processHashChainStepfunction to log errors without throwing, ensuring they do not propagate to the calling workflow.transitionEntityfunction to include a unique identifier in the journal entry ID for better traceability.adminMutationtoadminQueryin onboarding queries for improved clarity and consistency.REQUESTABLE_ROLES, streamlining the validation process.These changes improve error management, enhance query clarity, and ensure better role validation in the onboarding process.
Summary by Sourcery
Introduce an audit journal schema with hash-chained audit trail integration, tighten transition engine constraints, and refine onboarding, RBAC, and ledger behavior and tests.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores:
Summary by CodeRabbit