fix: redirect phantom ledger API calls to correct services - #59
chitcommit wants to merge 4 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
📝 WalkthroughWalkthroughReplaces ChittyLedger case/evidence/custody APIs with a generic ledger entry API and a separate ChittyEvidence client. Routes, dispute-sync, MCP, and timeline logic updated to use Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant DB as Database
participant Evidence as ChittyEvidence
participant Ledger as ChittyLedger
App->>Evidence: submitDocument(payload)
Evidence-->>App: { id, submission_id? }
Note right of App: fire-and-forget update to document metadata
App->>DB: UPDATE cc_documents.metadata (ledger_evidence_id = id)
alt when dispute creation / audit needed
App->>Ledger: addEntry({ entityType: 'audit', entityId: caseRef, action, actor, metadata })
Ledger-->>App: { id: entryId, sequenceNumber, hash }
App->>DB: UPDATE dispute.metadata (ledger_case_id = caseRef, ledger_entry_id = entryId)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
To use Codex here, create a Codex account and connect to github. |
ledgerClient was calling /api/evidence, /api/cases/*, and /api/evidence/*/custody — none of which exist on ChittyLedger. These endpoints belong to ChittyEvidence. - Rewrite ledgerClient to only expose real ChittyLedger endpoints: POST /entries, GET /entries, GET /custody/:id, GET /verify, GET /statistics - Add submitDocument and addCustodyEntry to evidenceClient - Update all callers (bridge, mcp, documents, timeline, dispute-sync) to use evidenceClient for evidence/case operations - Remove duplicate evidence fetch in timeline route - All operations include Bearer auth via CHITTYLEDGER_TOKEN Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The get_case_timeline handler still called ledger.getEvidenceByCase() which was removed from the rewritten ledgerClient. Evidence documents are already fetched via evidenceClient in the facts section above. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
906fc3d to
db09ebd
Compare
|
|
To use Codex here, create a Codex account and connect to github. |
![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 (3)
src/routes/mcp.ts (1)
610-611: Behavior change: empty arrays instead of errors when unconfigured.
ledger_factsandledger_contradictionsnow return empty arrays ({ facts: [] },{ contradictions: [] }) whenevidenceClientis unavailable, whereasledger_get_evidencereturns{ error: 'ChittyEvidence not configured' }.This inconsistency may confuse MCP clients. Consider aligning the behavior—either all return errors or all return empty results.
Also applies to: 619-620
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/mcp.ts` around lines 610 - 611, The handlers ledger_facts and ledger_contradictions are returning empty arrays when evidenceClient(env) is unavailable while ledger_get_evidence returns { error: 'ChittyEvidence not configured' }, causing inconsistent responses; pick one consistent behavior and implement it across all three handlers by changing the early-return in ledger_facts and ledger_contradictions (where evidenceClient(env) yields falsy) to return the same shape as ledger_get_evidence (or conversely adjust ledger_get_evidence to return an empty result shape) so all three (ledger_facts, ledger_contradictions, ledger_get_evidence) return the identical error/result structure when evidenceClient is unconfigured.src/routes/ledger.ts (1)
5-5: MissingAuthVariablesin route typing.Same issue as
documents.ts. AddingAuthVariableswould also eliminate the@ts-expect-erroron line 24-25.♻️ Proposed fix
+import type { AuthVariables } from '../middleware/auth'; + -export const ledgerRoutes = new Hono<{ Bindings: Env }>(); +export const ledgerRoutes = new Hono<{ Bindings: Env; Variables: AuthVariables }>();Then remove the
@ts-expect-error:- // `@ts-expect-error` app-level vars const userId = (c.get('userId') as string | undefined) || 'api-client';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/ledger.ts` at line 5, The route typing for ledgerRoutes is missing AuthVariables; update the Hono generic to include Variables: AuthVariables (e.g., change export const ledgerRoutes = new Hono<{ Bindings: Env; Variables: AuthVariables }>();), import or reference the AuthVariables type if not already available, then remove the related `@ts-expect-error` in this file (the same fix you used in documents.ts) so TypeScript correctly recognizes route variables; ensure any handlers using request.param/request.env align with the new typing.src/routes/documents.ts (1)
6-6: MissingAuthVariablesin route typing.Per coding guidelines, route files should type Hono apps with
Variables: AuthVariables. This enables type-safe access to context variables set by auth middleware.♻️ Proposed fix
+import type { AuthVariables } from '../middleware/auth'; + -export const documentRoutes = new Hono<{ Bindings: Env }>(); +export const documentRoutes = new Hono<{ Bindings: Env; Variables: AuthVariables }>();As per coding guidelines: "Import
AuthVariablesfromsrc/middleware/auth.tsfor route-level context variables" and "Type all Hono apps/routes with generic bindings:new Hono<{ Bindings: Env; Variables: AuthVariables }>()"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/documents.ts` at line 6, The documentRoutes Hono app is missing route-level Variables typing; import the exported AuthVariables symbol from the auth middleware module and change the declaration of documentRoutes (where new Hono(...) is called) to include the generic Variables: AuthVariables — i.e. new Hono<{ Bindings: Env; Variables: AuthVariables }>() — so route context variables set by auth middleware are typed.
🤖 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/lib/integrations.ts`:
- Around line 147-153: The submitDocument helper is dropping payload.fileSize
and payload.evidenceTier when calling post('/collect'); update the request body
in submitDocument so it forwards file_size: payload.fileSize and evidence_tier:
payload.evidenceTier along with the existing file_name, document_type,
description, and case_id fields (keep the function name submitDocument and the
POST to '/collect' unchanged).
In `@src/routes/timeline.ts`:
- Line 119: Comment is misleading because only fact events are fetched via
evidenceClient.getEnrichedFacts(), yet the response counts document events using
events.filter(e => e.type === 'document').length; either fetch documents and
append them as type:'document' events or remove the document-related comment and
count. Fix by one of two approaches: (A) implement document fetching — call the
appropriate evidenceClient method (e.g., evidenceClient.getDocuments() or
evidenceClient.getEnrichedDocuments() if available), map results into events
with type: 'document' and push into the existing events array before the
response, and update the comment to describe both facts and documents; or (B)
remove the misleading comment at the top of the block and remove the documents
count expression (events.filter(e => e.type === 'document').length) from the
response payload so the code only reports facts; ensure you update or delete the
comment and keep use of evidenceClient.getEnrichedFacts() and the events array
consistent.
---
Nitpick comments:
In `@src/routes/documents.ts`:
- Line 6: The documentRoutes Hono app is missing route-level Variables typing;
import the exported AuthVariables symbol from the auth middleware module and
change the declaration of documentRoutes (where new Hono(...) is called) to
include the generic Variables: AuthVariables — i.e. new Hono<{ Bindings: Env;
Variables: AuthVariables }>() — so route context variables set by auth
middleware are typed.
In `@src/routes/ledger.ts`:
- Line 5: The route typing for ledgerRoutes is missing AuthVariables; update the
Hono generic to include Variables: AuthVariables (e.g., change export const
ledgerRoutes = new Hono<{ Bindings: Env; Variables: AuthVariables }>();), import
or reference the AuthVariables type if not already available, then remove the
related `@ts-expect-error` in this file (the same fix you used in documents.ts) so
TypeScript correctly recognizes route variables; ensure any handlers using
request.param/request.env align with the new typing.
In `@src/routes/mcp.ts`:
- Around line 610-611: The handlers ledger_facts and ledger_contradictions are
returning empty arrays when evidenceClient(env) is unavailable while
ledger_get_evidence returns { error: 'ChittyEvidence not configured' }, causing
inconsistent responses; pick one consistent behavior and implement it across all
three handlers by changing the early-return in ledger_facts and
ledger_contradictions (where evidenceClient(env) yields falsy) to return the
same shape as ledger_get_evidence (or conversely adjust ledger_get_evidence to
return an empty result shape) so all three (ledger_facts, ledger_contradictions,
ledger_get_evidence) return the identical error/result structure when
evidenceClient is unconfigured.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4be2e445-71e5-475b-9c9a-dd8dfbe4d698
📒 Files selected for processing (8)
src/index.tssrc/lib/dispute-sync.tssrc/lib/integrations.tssrc/routes/bridge/ledger.tssrc/routes/documents.tssrc/routes/ledger.tssrc/routes/mcp.tssrc/routes/timeline.ts
| submitDocument: (payload: { filename: string; fileType: string; fileSize?: string; description?: string; evidenceTier?: string; caseId?: string }) => | ||
| post<{ id: string; submission_id?: string }>('/collect', { | ||
| file_name: payload.filename, | ||
| document_type: payload.fileType, | ||
| description: payload.description, | ||
| case_id: payload.caseId, | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how submitDocument is called to see if fileSize/evidenceTier are expected
rg -n "submitDocument" --type ts -A 5Repository: chittyos/chittycommand
Length of output: 1416
🏁 Script executed:
#!/bin/bash
# Check for any documentation or tests about the ChittyEvidence /collect endpoint
rg -n "/collect" --type ts -B 2 -A 2Repository: chittyos/chittycommand
Length of output: 556
🏁 Script executed:
#!/bin/bash
# Look for any ChittyEvidence API schema or type definitions
rg -n "ChittyEvidence|collect" --type ts -i | head -30Repository: chittyos/chittycommand
Length of output: 3277
🏁 Script executed:
#!/bin/bash
# Check if there are other integrations files or clients that might show the expected structure
find . -type f -name "*evidence*" -o -name "*type*" | grep -E "\.(ts|tsx|d\.ts)$"Repository: chittyos/chittycommand
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Read the full integrations.ts file around the submitDocument method to see documentation
cat -n src/lib/integrations.ts | sed -n '130,170p'Repository: chittyos/chittycommand
Length of output: 2021
🏁 Script executed:
#!/bin/bash
# Check if there are any constants or configuration about ChittyEvidence API expectations
rg -n "CHITTY.*EVIDENCE|evidence.*url|/collect|submit" --type ts -i -B 2 -A 2 src/lib/integrations.tsRepository: chittyos/chittycommand
Length of output: 2172
🏁 Script executed:
#!/bin/bash
# Look for any recent commits or history mentioning these fields
git log --oneline --all -S "fileSize\|evidenceTier" -- src/lib/integrations.ts 2>/dev/null | head -10Repository: chittyos/chittycommand
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Look for any tests related to submitDocument or evidenceClient
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.test.js" \) | xargs rg -l "submitDocument|evidenceClient" 2>/dev/nullRepository: chittyos/chittycommand
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check the complete integrations.ts to see if there are any comments about field handling
cat -n src/lib/integrations.ts | sed -n '115,125p'Repository: chittyos/chittycommand
Length of output: 478
Include fileSize and evidenceTier fields in the /collect POST request.
The submitDocument method accepts fileSize and evidenceTier in the payload but drops them from the request body sent to /collect. Both callers in src/routes/documents.ts and src/routes/bridge/ledger.ts explicitly pass evidenceTier, indicating these fields are intentional and should be forwarded to the ChittyEvidence service.
Current code (lines 151–157)
submitDocument: (payload: { filename: string; fileType: string; fileSize?: string; description?: string; evidenceTier?: string; caseId?: string }) =>
post<{ id: string; submission_id?: string }>('/collect', {
file_name: payload.filename,
document_type: payload.fileType,
description: payload.description,
case_id: payload.caseId,
}),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/integrations.ts` around lines 147 - 153, The submitDocument helper is
dropping payload.fileSize and payload.evidenceTier when calling
post('/collect'); update the request body in submitDocument so it forwards
file_size: payload.fileSize and evidence_tier: payload.evidenceTier along with
the existing file_name, document_type, description, and case_id fields (keep the
function name submitDocument and the POST to '/collect' unchanged).
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/integrations.ts (1)
23-33:⚠️ Potential issue | 🟠 MajorFail fast when the ledger token is missing.
These helpers only hit authenticated ChittyLedger data endpoints, but
ledgerClient()is still treated as configured when onlyCHITTYLEDGER_URLis present. In that state every call degrades to a silent 401 →null/[], which looks like empty data instead of a config error.♻️ Suggested patch
export function ledgerClient(env: Env) { const baseUrl = env.CHITTYLEDGER_URL; - if (!baseUrl) return null; + const token = env.CHITTYLEDGER_TOKEN; + if (!baseUrl || !token) return null; const headers: Record<string, string> = { 'Content-Type': 'application/json', 'X-Source-Service': 'chittycommand', + 'Authorization': `Bearer ${token}`, }; - if (env.CHITTYLEDGER_TOKEN) { - headers['Authorization'] = `Bearer ${env.CHITTYLEDGER_TOKEN}`; - } return {Also applies to: 35-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/integrations.ts` around lines 23 - 33, The ledgerClient helper currently treats presence of CHITTYLEDGER_URL alone as “configured” which leads to silent 401s; update ledgerClient(env: Env) to require both env.CHITTYLEDGER_URL and env.CHITTYLEDGER_TOKEN and fail fast when the token is missing (throw a clear Error or return a rejected result) instead of returning a partially-configured client, and apply the same check to the other authenticated helpers in this file (the functions using CHITTYLEDGER_* between lines ~35-84) so any call sees an explicit configuration error when the token is absent.
♻️ Duplicate comments (2)
src/lib/integrations.ts (1)
151-157:⚠️ Potential issue | 🟡 MinorForward
fileSizeandevidenceTierto/collect.
submitDocument()accepts both values, and the new callers pass them, but the request body drops them. That loses caller intent and can change downstream intake/classification.♻️ Suggested patch
submitDocument: (payload: { filename: string; fileType: string; fileSize?: string; description?: string; evidenceTier?: string; caseId?: string }) => post<{ id: string; submission_id?: string }>('/collect', { file_name: payload.filename, document_type: payload.fileType, + file_size: payload.fileSize, description: payload.description, + evidence_tier: payload.evidenceTier, case_id: payload.caseId, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/integrations.ts` around lines 151 - 157, submitDocument currently accepts fileSize and evidenceTier but doesn't forward them in the request body to /collect; update the object passed to post in submitDocument to include file_size: payload.fileSize and evidence_tier: payload.evidenceTier (matching the existing snake_case keys like file_name and document_type) so callers' values are preserved and sent to the downstream intake/classification.src/routes/timeline.ts (1)
119-135:⚠️ Potential issue | 🟡 MinorDon’t claim document coverage when no document events are emitted.
Section 1 only pushes
type: 'fact'events, so this new comment is incorrect andsources.documentswill always be0. Either add document events here or remove the document-specific claim/count from the response.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/timeline.ts` around lines 119 - 135, The response claims document coverage but no document events are ever emitted (section 1 only pushes events with type: 'fact'), so update either the event emission or the response: either remove the sources.documents entry from the returned object or add proper document events (type === 'document') where events are created (the code that pushes into the events array in section 1). Ensure the change targets the events array population (where events.push is called for facts) or the response construction that references sources.documents so the API doesn't report a misleading document count.
🤖 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/ledger.ts`:
- Around line 11-14: The route is calling evidence.getEnrichedFacts(caseId)
which returns only EvidenceFact[] (facts-only) so the response no longer
includes uploaded documents; change the handler to call the ChittyEvidence
document lookup API (replace getEnrichedFacts with the evidence document method
such as getEvidence, getEvidenceByCaseId or getDocumentsForCase on the
evidenceClient instance) to fetch the actual uploaded evidence/documents and
return them under the existing { case_id, evidence: [...] } contract; keep the
existing fallback of returning an empty array when no documents are found and
preserve the earlier error response when evidenceClient(c.env) is not
configured.
In `@src/routes/mcp.ts`:
- Around line 638-658: The code currently writes the audit entry id
(entryResult.id from ledger.addEntry) into metadata.ledger_case_id which should
be reserved for real case IDs; instead, invoke the real case-creation flow for
disputes (replace the ledger.addEntry call with the canonical case creation
function used in your system—e.g., createCaseForDispute or the service that
returns a case id), store that returned real case id into
metadata.ledger_case_id, and if you still need to persist the audit entry id
keep it under a separate metadata key (e.g., ledger_audit_entry_id) and update
the SQL UPDATE to write both keys accordingly while keeping the existing dispute
lookup and error handling intact.
---
Outside diff comments:
In `@src/lib/integrations.ts`:
- Around line 23-33: The ledgerClient helper currently treats presence of
CHITTYLEDGER_URL alone as “configured” which leads to silent 401s; update
ledgerClient(env: Env) to require both env.CHITTYLEDGER_URL and
env.CHITTYLEDGER_TOKEN and fail fast when the token is missing (throw a clear
Error or return a rejected result) instead of returning a partially-configured
client, and apply the same check to the other authenticated helpers in this file
(the functions using CHITTYLEDGER_* between lines ~35-84) so any call sees an
explicit configuration error when the token is absent.
---
Duplicate comments:
In `@src/lib/integrations.ts`:
- Around line 151-157: submitDocument currently accepts fileSize and
evidenceTier but doesn't forward them in the request body to /collect; update
the object passed to post in submitDocument to include file_size:
payload.fileSize and evidence_tier: payload.evidenceTier (matching the existing
snake_case keys like file_name and document_type) so callers' values are
preserved and sent to the downstream intake/classification.
In `@src/routes/timeline.ts`:
- Around line 119-135: The response claims document coverage but no document
events are ever emitted (section 1 only pushes events with type: 'fact'), so
update either the event emission or the response: either remove the
sources.documents entry from the returned object or add proper document events
(type === 'document') where events are created (the code that pushes into the
events array in section 1). Ensure the change targets the events array
population (where events.push is called for facts) or the response construction
that references sources.documents so the API doesn't report a misleading
document count.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d99a5d12-fa0f-43d3-817d-ca667734c567
📒 Files selected for processing (7)
src/lib/dispute-sync.tssrc/lib/integrations.tssrc/routes/bridge/ledger.tssrc/routes/documents.tssrc/routes/ledger.tssrc/routes/mcp.tssrc/routes/timeline.ts
✅ Files skipped from review due to trivial changes (1)
- src/routes/bridge/ledger.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/dispute-sync.ts
| case 'ledger_create_case_for_dispute': { | ||
| const disputeId = String(args.dispute_id || '').trim(); | ||
| if (!disputeId) throw new Error('Missing argument: dispute_id'); | ||
| if (!env.CHITTYLEDGER_URL) return { error: 'ChittyLedger not configured' }; | ||
| const ledger = ledgerClient(env); | ||
| if (!ledger) return { error: 'ChittyLedger not configured' }; | ||
| const rows = await sql`SELECT id, title, dispute_type, description, metadata FROM cc_disputes WHERE id = ${disputeId}`; | ||
| if (rows.length === 0) throw new Error('Dispute not found'); | ||
| const d = rows[0] as any; | ||
| const metadata = (d.metadata as any) || {}; | ||
| if (metadata.ledger_case_id) return { dispute_id: disputeId, case_id: metadata.ledger_case_id, linked: true }; | ||
| try { | ||
| const payload = { caseNumber: `CC-DISPUTE-${String(d.id).slice(0,8)}`, title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }; | ||
| const res = await fetch(`${env.CHITTYLEDGER_URL}/api/cases`, { | ||
| method: 'POST', headers: { 'Content-Type': 'application/json', 'X-Source-Service': 'chittycommand' }, body: JSON.stringify(payload) | ||
| }); | ||
| if (!res.ok) return { error: 'Failed to create case', code: res.status }; | ||
| const data = await res.json() as { id: string }; | ||
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: data.id })}::jsonb WHERE id = ${disputeId}`; | ||
| return { dispute_id: disputeId, case_id: data.id, linked: true }; | ||
| } catch (err) { | ||
| return { error: String(err) }; | ||
| } | ||
| const entryResult = await ledger.addEntry({ | ||
| entityType: 'audit', | ||
| entityId: `CC-DISPUTE-${String(d.id).slice(0, 8)}`, | ||
| action: 'dispute:created', | ||
| actor: 'chittycommand', | ||
| actorType: 'service', | ||
| metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }, | ||
| }); | ||
| if (!entryResult) return { error: 'Failed to create ledger entry' }; | ||
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`; | ||
| return { dispute_id: disputeId, case_id: entryResult.id, linked: true }; |
There was a problem hiding this comment.
Keep ledger_case_id reserved for real case IDs.
ledger.addEntry() returns an audit entry ID, but metadata.ledger_case_id is later used as the case reference in deadline/timeline joins. Writing entryResult.id into that field will break those lookups and mix two identifier types in the same metadata key. If this tool still needs to create a case, it has to call the real case-creation flow instead of addEntry().
♻️ Suggested patch
const metadata = (d.metadata as any) || {};
if (metadata.ledger_case_id) return { dispute_id: disputeId, case_id: metadata.ledger_case_id, linked: true };
const entryResult = await ledger.addEntry({
entityType: 'audit',
entityId: `CC-DISPUTE-${String(d.id).slice(0, 8)}`,
action: 'dispute:created',
actor: 'chittycommand',
actorType: 'service',
metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined },
});
if (!entryResult) return { error: 'Failed to create ledger entry' };
- await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`;
- return { dispute_id: disputeId, case_id: entryResult.id, linked: true };
+ await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_entry_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`;
+ return { dispute_id: disputeId, entry_id: entryResult.id, linked: true };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case 'ledger_create_case_for_dispute': { | |
| const disputeId = String(args.dispute_id || '').trim(); | |
| if (!disputeId) throw new Error('Missing argument: dispute_id'); | |
| if (!env.CHITTYLEDGER_URL) return { error: 'ChittyLedger not configured' }; | |
| const ledger = ledgerClient(env); | |
| if (!ledger) return { error: 'ChittyLedger not configured' }; | |
| const rows = await sql`SELECT id, title, dispute_type, description, metadata FROM cc_disputes WHERE id = ${disputeId}`; | |
| if (rows.length === 0) throw new Error('Dispute not found'); | |
| const d = rows[0] as any; | |
| const metadata = (d.metadata as any) || {}; | |
| if (metadata.ledger_case_id) return { dispute_id: disputeId, case_id: metadata.ledger_case_id, linked: true }; | |
| try { | |
| const payload = { caseNumber: `CC-DISPUTE-${String(d.id).slice(0,8)}`, title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }; | |
| const res = await fetch(`${env.CHITTYLEDGER_URL}/api/cases`, { | |
| method: 'POST', headers: { 'Content-Type': 'application/json', 'X-Source-Service': 'chittycommand' }, body: JSON.stringify(payload) | |
| }); | |
| if (!res.ok) return { error: 'Failed to create case', code: res.status }; | |
| const data = await res.json() as { id: string }; | |
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: data.id })}::jsonb WHERE id = ${disputeId}`; | |
| return { dispute_id: disputeId, case_id: data.id, linked: true }; | |
| } catch (err) { | |
| return { error: String(err) }; | |
| } | |
| const entryResult = await ledger.addEntry({ | |
| entityType: 'audit', | |
| entityId: `CC-DISPUTE-${String(d.id).slice(0, 8)}`, | |
| action: 'dispute:created', | |
| actor: 'chittycommand', | |
| actorType: 'service', | |
| metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }, | |
| }); | |
| if (!entryResult) return { error: 'Failed to create ledger entry' }; | |
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`; | |
| return { dispute_id: disputeId, case_id: entryResult.id, linked: true }; | |
| case 'ledger_create_case_for_dispute': { | |
| const disputeId = String(args.dispute_id || '').trim(); | |
| if (!disputeId) throw new Error('Missing argument: dispute_id'); | |
| const ledger = ledgerClient(env); | |
| if (!ledger) return { error: 'ChittyLedger not configured' }; | |
| const rows = await sql`SELECT id, title, dispute_type, description, metadata FROM cc_disputes WHERE id = ${disputeId}`; | |
| if (rows.length === 0) throw new Error('Dispute not found'); | |
| const d = rows[0] as any; | |
| const metadata = (d.metadata as any) || {}; | |
| if (metadata.ledger_case_id) return { dispute_id: disputeId, case_id: metadata.ledger_case_id, linked: true }; | |
| const entryResult = await ledger.addEntry({ | |
| entityType: 'audit', | |
| entityId: `CC-DISPUTE-${String(d.id).slice(0, 8)}`, | |
| action: 'dispute:created', | |
| actor: 'chittycommand', | |
| actorType: 'service', | |
| metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }, | |
| }); | |
| if (!entryResult) return { error: 'Failed to create ledger entry' }; | |
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_entry_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`; | |
| return { dispute_id: disputeId, entry_id: entryResult.id, linked: true }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/mcp.ts` around lines 638 - 658, The code currently writes the
audit entry id (entryResult.id from ledger.addEntry) into
metadata.ledger_case_id which should be reserved for real case IDs; instead,
invoke the real case-creation flow for disputes (replace the ledger.addEntry
call with the canonical case creation function used in your system—e.g.,
createCaseForDispute or the service that returns a case id), store that returned
real case id into metadata.ledger_case_id, and if you still need to persist the
audit entry id keep it under a separate metadata key (e.g.,
ledger_audit_entry_id) and update the SQL UPDATE to write both keys accordingly
while keeping the existing dispute lookup and error handling intact.
ledger_case_id is used for timeline/deadline joins and must be a stable
case reference (CC-DISPUTE-{prefix}), not a ledger audit entry UUID.
Store the entry UUID separately as ledger_entry_id.
Fixes CodeRabbit review findings on both mcp.ts and dispute-sync.ts.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
To use Codex here, create a Codex account and connect to github. |
🧩 Analysis chain🏁 Script executed: Length of output: 119059
Adversarial Security & Regression Review — PR
|
| # | File | Issue | Severity |
|---|---|---|---|
| 1 | integrations.ts |
evidenceClient — no auth header |
🔴 High |
| 2 | integrations.ts |
ledgerClient — silent no-auth fallback |
🔴 High |
| 3 | integrations.ts |
No AbortSignal.timeout on either client |
🔴 High |
| 4 | bridge/ledger.ts |
/sync-disputes — unbounded SELECT * |
🔴 High |
| 5 | mcp.ts |
ledger_link_case_for_dispute — no ownership check |
🔴 High |
| 6 | routes/ledger.ts |
Custody actor defaults to 'api-client' |
🟠 Medium |
| 7 | mcp.ts |
No scope enforcement across all 48 MCP tools | 🟠 Medium |
| 8 | dispute-sync.ts / mcp.ts |
8-char UUID truncation → caseRef collision | 🟠 Medium |
| 9 | bridge/ledger.ts / mcp.ts |
PII in ledger metadata — no scrubbing |
🟠 Medium |
| 10 | routes/ledger.ts |
Text search replacing case-scoped lookup | 🟡 Regression |
| 11 | integrations.ts |
fileSize + evidenceTier silently dropped |
🟡 Regression |
| 12 | routes/documents.ts |
linked_dispute_id not forwarded as caseId |
🟡 Regression |
Issues 1–5 are merge-blocking per the repository's policy of resolving must-fix review comments (security, correctness) before merge.
🧠 Learnings used
Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-04T04:42:57.380Z
Learning: Resolve must-fix review comments (security, correctness, compliance, merge blockers) before merge
Learnt from: CR
Repo: chittyos/chittycommand PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T16:02:20.597Z
Learning: Applies to src/routes/ledger.ts : Implement ChittyLedger evidence/custody passthrough in `src/routes/ledger.ts`
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/mcp.ts`:
- Around line 1242-1243: The change removed the document-only branch from the
timeline builder causing uploads with no extracted facts to be dropped; restore
the document-only branch in the get_case_timeline flow (the code that previously
emitted document events alongside ChittyEvidence facts) so documents with
pending OCR/failed extraction/manual attachments are included as explicit
timeline events, and add deduplication when merging events (e.g., in
timelineEvents or the mergeTimeline logic) by keying on document id + timestamp
(or a stable event signature) to avoid duplicate entries.
- Around line 603-606: The evidence route now returns getEnrichedFacts() output
but addCustodyEntry() is document-scoped, causing mismatched IDs; update the
integration so both tools use the same resource identifier: either (A) make
addCustodyEntry(caseId, ...) accept the same case-level ID used by
ledger_get_evidence/getEnrichedFacts(), or (B) make ledger_get_evidence return
document-scoped IDs that addCustodyEntry expects and document that type; modify
the caller in src/routes/mcp.ts to pass the matching ID and update
addCustodyEntry() signature/implementation in src/lib/integrations.ts
accordingly (and mirror the same change for the second occurrence referenced),
ensuring getEnrichedFacts(), ledger_get_evidence, and addCustodyEntry() all
agree on the ID type.
- Around line 614-617: Replace the hardcoded performer with the authenticated
caller: call evidenceClient(env) as before and pass caller.userId into
ev.addCustodyEntry instead of the string 'mcp-client', and if caller or
caller.userId is missing return an error/refuse the write (e.g., { error:
'missing authenticated caller' }) before calling addCustodyEntry; update the
call site using evidenceId, action, notes to build the custody payload
(performedBy: caller.userId) so each custody record records the real user.
- Around line 648-659: The current flow builds a short, collision-prone caseRef
(caseRef = `CC-DISPUTE-${String(d.id).slice(0,8)}`) and performs a
non-idempotent ledger.addEntry() without any row lock or idempotency guard,
allowing concurrent requests or retries to create duplicate or cross-linked
ledger entries; fix by wrapping the logic in a DB transaction that SELECTs the
cc_disputes row FOR UPDATE (by disputeId), check the metadata for existing
ledger_case_id/ledger_entry_id and return them if present, otherwise compute a
collision-safe caseRef (use the full dispute id or a server-generated UUID
instead of slice), call ledger.addEntry(), and then UPDATE cc_disputes.metadata
with the ledger ids before committing so the operation is atomic and idempotent
(refer to symbols: caseRef, ledger.addEntry, sql UPDATE on cc_disputes, metadata
ledger_case_id/ledger_entry_id).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf984430-6d06-4bdc-8698-a2bec868e3cd
📒 Files selected for processing (3)
src/lib/dispute-sync.tssrc/routes/ledger.tssrc/routes/mcp.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/ledger.ts
- src/lib/dispute-sync.ts
| const ev = evidenceClient(env); | ||
| if (!ev) return { error: 'ChittyEvidence not configured' }; | ||
| const facts = await ev.getEnrichedFacts(caseId); | ||
| return { case_id: caseId, evidence: facts || [] }; |
There was a problem hiding this comment.
Don't split the evidence and custody tools across different ID types.
ledger_get_evidence now returns getEnrichedFacts() output, but addCustodyEntry() is document-scoped in src/lib/integrations.ts. After this change, an MCP caller can't take an ID from the evidence tool and feed it into the custody tool without knowing a different resource type first.
Also applies to: 614-617
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/mcp.ts` around lines 603 - 606, The evidence route now returns
getEnrichedFacts() output but addCustodyEntry() is document-scoped, causing
mismatched IDs; update the integration so both tools use the same resource
identifier: either (A) make addCustodyEntry(caseId, ...) accept the same
case-level ID used by ledger_get_evidence/getEnrichedFacts(), or (B) make
ledger_get_evidence return document-scoped IDs that addCustodyEntry expects and
document that type; modify the caller in src/routes/mcp.ts to pass the matching
ID and update addCustodyEntry() signature/implementation in
src/lib/integrations.ts accordingly (and mirror the same change for the second
occurrence referenced), ensuring getEnrichedFacts(), ledger_get_evidence, and
addCustodyEntry() all agree on the ID type.
| const ev = evidenceClient(env); | ||
| if (!ev) return { error: 'ChittyEvidence not configured' }; | ||
| const result = await ev.addCustodyEntry(evidenceId, { action, performedBy: 'mcp-client', notes }); | ||
| return { ok: !!result, result }; |
There was a problem hiding this comment.
Use the authenticated caller in custody records.
Hardcoding performedBy: 'mcp-client' makes every chain-of-custody event indistinguishable. Pass caller.userId through here and refuse the write when it is missing.
🐛 Proposed fix
- const result = await ev.addCustodyEntry(evidenceId, { action, performedBy: 'mcp-client', notes });
+ if (!caller.userId) return { error: 'Unauthorized' };
+ const result = await ev.addCustodyEntry(evidenceId, {
+ action,
+ performedBy: caller.userId,
+ notes,
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/mcp.ts` around lines 614 - 617, Replace the hardcoded performer
with the authenticated caller: call evidenceClient(env) as before and pass
caller.userId into ev.addCustodyEntry instead of the string 'mcp-client', and if
caller or caller.userId is missing return an error/refuse the write (e.g., {
error: 'missing authenticated caller' }) before calling addCustodyEntry; update
the call site using evidenceId, action, notes to build the custody payload
(performedBy: caller.userId) so each custody record records the real user.
| const caseRef = `CC-DISPUTE-${String(d.id).slice(0, 8)}`; | ||
| const entryResult = await ledger.addEntry({ | ||
| entityType: 'audit', | ||
| entityId: caseRef, | ||
| action: 'dispute:created', | ||
| actor: 'chittycommand', | ||
| actorType: 'service', | ||
| metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }, | ||
| }); | ||
| if (!entryResult) return { error: 'Failed to create ledger entry' }; | ||
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: caseRef, ledger_entry_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`; | ||
| return { dispute_id: disputeId, case_id: caseRef, ledger_entry_id: entryResult.id, linked: true }; |
There was a problem hiding this comment.
Make dispute case creation collision-safe and idempotent.
CC-DISPUTE-${String(d.id).slice(0, 8)} only preserves 32 bits of uniqueness, and this block still does a read → upstream write → DB update around a non-idempotent addEntry() call. A collision or concurrent retry can cross-link disputes or create multiple ledger entries for the same dispute.
🐛 Minimal fix for the collision part
- const caseRef = `CC-DISPUTE-${String(d.id).slice(0, 8)}`;
+ const caseRef = `CC-DISPUTE-${String(d.id)}`;You’ll still need a row lock or idempotency key around addEntry() to close the retry/concurrency gap.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const caseRef = `CC-DISPUTE-${String(d.id).slice(0, 8)}`; | |
| const entryResult = await ledger.addEntry({ | |
| entityType: 'audit', | |
| entityId: caseRef, | |
| action: 'dispute:created', | |
| actor: 'chittycommand', | |
| actorType: 'service', | |
| metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }, | |
| }); | |
| if (!entryResult) return { error: 'Failed to create ledger entry' }; | |
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: caseRef, ledger_entry_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`; | |
| return { dispute_id: disputeId, case_id: caseRef, ledger_entry_id: entryResult.id, linked: true }; | |
| const caseRef = `CC-DISPUTE-${String(d.id)}`; | |
| const entryResult = await ledger.addEntry({ | |
| entityType: 'audit', | |
| entityId: caseRef, | |
| action: 'dispute:created', | |
| actor: 'chittycommand', | |
| actorType: 'service', | |
| metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }, | |
| }); | |
| if (!entryResult) return { error: 'Failed to create ledger entry' }; | |
| await sql`UPDATE cc_disputes SET metadata = COALESCE(metadata, '{}'::jsonb) || ${JSON.stringify({ ledger_case_id: caseRef, ledger_entry_id: entryResult.id })}::jsonb WHERE id = ${disputeId}`; | |
| return { dispute_id: disputeId, case_id: caseRef, ledger_entry_id: entryResult.id, linked: true }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/mcp.ts` around lines 648 - 659, The current flow builds a short,
collision-prone caseRef (caseRef = `CC-DISPUTE-${String(d.id).slice(0,8)}`) and
performs a non-idempotent ledger.addEntry() without any row lock or idempotency
guard, allowing concurrent requests or retries to create duplicate or
cross-linked ledger entries; fix by wrapping the logic in a DB transaction that
SELECTs the cc_disputes row FOR UPDATE (by disputeId), check the metadata for
existing ledger_case_id/ledger_entry_id and return them if present, otherwise
compute a collision-safe caseRef (use the full dispute id or a server-generated
UUID instead of slice), call ledger.addEntry(), and then UPDATE
cc_disputes.metadata with the ledger ids before committing so the operation is
atomic and idempotent (refer to symbols: caseRef, ledger.addEntry, sql UPDATE on
cc_disputes, metadata ledger_case_id/ledger_entry_id).
| // Documents already covered by ChittyEvidence facts above | ||
|
|
There was a problem hiding this comment.
Keep document-only events in the case timeline.
Removing the document branch means uploads with no extracted facts yet—pending OCR, failed extraction, or manual attachments—disappear from get_case_timeline. If duplicate events were the problem, dedupe them, but keep explicit document coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/mcp.ts` around lines 1242 - 1243, The change removed the
document-only branch from the timeline builder causing uploads with no extracted
facts to be dropped; restore the document-only branch in the get_case_timeline
flow (the code that previously emitted document events alongside ChittyEvidence
facts) so documents with pending OCR/failed extraction/manual attachments are
included as explicit timeline events, and add deduplication when merging events
(e.g., in timelineEvents or the mergeTimeline logic) by keying on document id +
timestamp (or a stable event signature) to avoid duplicate entries.
… null guard - Fix Neon SQL template literal: conditional WHERE clauses don't work inline, use separate query branches for date filtering in get_case_timeline - Fix cc_disputes query: 'domain' column doesn't exist, use 'dispute_type' - Fix listJobs null guard: check Array.isArray(result.jobs) before .map() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
To use Codex here, create a Codex account and connect to github. |
|
Medium-severity policy concerns (issues 6–9) and regression risks (issues 10–12, including the Per the repository's merge policy, issues 1–5 must be resolved before this PR is eligible for merge. Triggering an incremental review now to catch any new commits since the last pass. 🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/routes/timeline.ts (1)
119-119:⚠️ Potential issue | 🟡 MinorComment is inaccurate and documents count is always zero.
Section 1 fetches facts via
evidenceClient.getEnrichedFacts()/getFactsByDateRange(), not documents. Notype: 'document'events are ever pushed to the array, sosources.documents(line 135) will always be0.Either:
- Remove the misleading comment and the
documentscount from the response, or- Implement actual document fetching if documents should be included in the timeline
♻️ Option A: Remove misleading comment and dead code
- // 4. Evidence documents already fetched in section 1 via evidenceClient + // Note: Document events are not currently fetched; only facts from ChittyEvidence // Sort by date ascending events.sort((a, b) => a.date.localeCompare(b.date)); return c.json({ caseId, eventCount: events.length, dateRange: { earliest: events[0]?.date || null, latest: events[events.length - 1]?.date || null, }, sources: { facts: events.filter(e => e.type === 'fact').length, deadlines: events.filter(e => e.type === 'deadline').length, disputes: events.filter(e => e.type === 'dispute').length, - documents: events.filter(e => e.type === 'document').length, }, events, });Also applies to: 135-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/timeline.ts` at line 119, The comment claiming "Evidence documents already fetched in section 1 via evidenceClient" is incorrect because section 1 calls evidenceClient.getEnrichedFacts() / getFactsByDateRange(), which returns facts not documents, so sources.documents (referenced as sources.documents) is always zero; either remove the misleading comment and drop the documents count from the response construction (remove the comment and any code that sets or returns sources.documents) or, if documents must be included, add a real fetch using the correct evidenceClient method to retrieve documents and push events with type: 'document' into the timeline array before computing sources.documents; locate usages around evidenceClient.getEnrichedFacts() / getFactsByDateRange() and the code that computes or returns sources.documents to make the change.
🧹 Nitpick comments (2)
src/routes/mcp.ts (2)
632-635: Same inconsistency applies here.This has the same silent fallback to
{ contradictions: [] }instead of returning an error when ChittyEvidence is not configured.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/mcp.ts` around lines 632 - 635, The route currently silently returns { contradictions: [] } when evidenceClient(env) returns falsy; instead detect when ev is not configured (the ev variable from evidenceClient) and return or throw an explicit error response indicating ChittyEvidence is not configured (rather than an empty contradictions array). Update the handler around evidenceClient(env) and the call to ev.getContradictions(caseId) so that when ev is falsy you return a clear error (or non-200 response) referencing the caseId and a message like "ChittyEvidence not configured", ensuring callers can distinguish "no contradictions" from "evidence service unavailable".
623-626: Inconsistent error handling when ChittyEvidence is not configured.Lines 604 and 615 return
{ error: 'ChittyEvidence not configured' }, but line 624 silently returns{ facts: [] }. This inconsistency can mislead MCP callers—they cannot distinguish between "no facts exist" and "service unavailable."♻️ Proposed fix for consistency
const ev = evidenceClient(env); - if (!ev) return { facts: [] }; + if (!ev) return { error: 'ChittyEvidence not configured' }; const facts = await ev.getStatementOfFacts(caseId); return { case_id: caseId, facts: facts || [] };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/mcp.ts` around lines 623 - 626, The handler currently returns an empty facts array when evidenceClient(env) is missing, causing inconsistency with other branches that return { error: 'ChittyEvidence not configured' }; update the branch where ev is falsy (the code creating ev via evidenceClient(env) and calling ev.getStatementOfFacts(caseId)) to return the same error object used elsewhere ({ error: 'ChittyEvidence not configured' }) so callers can distinguish "service unavailable" from "no facts".
🤖 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/mcp.ts`:
- Line 655: The metadata object is writing raw d.title and d.description to the
external ChittyLedger (metadata: { title: String(d.title), ... description:
d.description || undefined }), which can leak PII; before constructing metadata,
run the values through a sanitization/allowlist function (e.g.,
sanitizeDisputeTitle, sanitizeDisputeDescription or redactPII) that strips or
masks names, addresses, account numbers and enforces length/character limits,
then use those sanitized values in metadata; add tests for edge cases and ensure
the ChittyLedger write path (the code that builds metadata) uses the sanitized
variables instead of d.title/d.description.
---
Duplicate comments:
In `@src/routes/timeline.ts`:
- Line 119: The comment claiming "Evidence documents already fetched in section
1 via evidenceClient" is incorrect because section 1 calls
evidenceClient.getEnrichedFacts() / getFactsByDateRange(), which returns facts
not documents, so sources.documents (referenced as sources.documents) is always
zero; either remove the misleading comment and drop the documents count from the
response construction (remove the comment and any code that sets or returns
sources.documents) or, if documents must be included, add a real fetch using the
correct evidenceClient method to retrieve documents and push events with type:
'document' into the timeline array before computing sources.documents; locate
usages around evidenceClient.getEnrichedFacts() / getFactsByDateRange() and the
code that computes or returns sources.documents to make the change.
---
Nitpick comments:
In `@src/routes/mcp.ts`:
- Around line 632-635: The route currently silently returns { contradictions: []
} when evidenceClient(env) returns falsy; instead detect when ev is not
configured (the ev variable from evidenceClient) and return or throw an explicit
error response indicating ChittyEvidence is not configured (rather than an empty
contradictions array). Update the handler around evidenceClient(env) and the
call to ev.getContradictions(caseId) so that when ev is falsy you return a clear
error (or non-200 response) referencing the caseId and a message like
"ChittyEvidence not configured", ensuring callers can distinguish "no
contradictions" from "evidence service unavailable".
- Around line 623-626: The handler currently returns an empty facts array when
evidenceClient(env) is missing, causing inconsistency with other branches that
return { error: 'ChittyEvidence not configured' }; update the branch where ev is
falsy (the code creating ev via evidenceClient(env) and calling
ev.getStatementOfFacts(caseId)) to return the same error object used elsewhere
({ error: 'ChittyEvidence not configured' }) so callers can distinguish "service
unavailable" from "no facts".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c7526b3-9973-468d-933e-e42ae7c434ef
📒 Files selected for processing (3)
src/lib/job-dispatcher.tssrc/routes/mcp.tssrc/routes/timeline.ts
| action: 'dispute:created', | ||
| actor: 'chittycommand', | ||
| actorType: 'service', | ||
| metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined }, |
There was a problem hiding this comment.
Dispute PII written to external ledger without scrubbing.
d.title and d.description may contain personally identifiable information (names, addresses, account numbers). Writing them verbatim to the ChittyLedger metadata leaks PII to an external service. Consider an allowlist or sanitization pass before sending.
♻️ Example: sanitize or strip sensitive fields
const entryResult = await ledger.addEntry({
entityType: 'audit',
entityId: caseRef,
action: 'dispute:created',
actor: 'chittycommand',
actorType: 'service',
- metadata: { title: String(d.title), caseType: 'CIVIL', description: d.description || undefined },
+ metadata: { caseType: 'CIVIL', disputeType: d.dispute_type },
});If the title/description are required for legal workflows, document the data classification and ensure ChittyLedger's retention and access policies are compliant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/mcp.ts` at line 655, The metadata object is writing raw d.title
and d.description to the external ChittyLedger (metadata: { title:
String(d.title), ... description: d.description || undefined }), which can leak
PII; before constructing metadata, run the values through a
sanitization/allowlist function (e.g., sanitizeDisputeTitle,
sanitizeDisputeDescription or redactPII) that strips or masks names, addresses,
account numbers and enforces length/character limits, then use those sanitized
values in metadata; add tests for edge cases and ensure the ChittyLedger write
path (the code that builds metadata) uses the sanitized variables instead of
d.title/d.description.
Pull request was closed
1 participant
Summary
ledgerClientwas calling/api/evidence,/api/cases/*,/api/evidence/*/custody— none of which exist on ChittyLedger (they 404 silently)ledgerClientto only expose real ChittyLedger endpoints with Bearer authsubmitDocumentandaddCustodyEntrytoevidenceClientTest plan
npx tsc --noEmitpasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor