Skip to content

feat: convert to thin proxy (scrape → router, prompts → connect) - #49

Merged
chitcommit merged 2 commits into
mainfrom
feat/scrape-proxy-and-prompt-consumer
Mar 23, 2026
Merged

chitcommit merged 2 commits into
mainfrom
feat/scrape-proxy-and-prompt-consumer

Conversation

@chitcommit

@chitcommit chitcommit commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • DELETE fan-out.ts — orchestration (Intelligence, Calendar, Triage, Ledger fan-out) now handled by ScrapeAgent DO-to-DO calls in ChittyRouter
  • REWRITE job-dispatcher.ts — thin proxy to ChittyRouter /agents/scrape/* endpoints with Neon fallback for historical data
  • ADD scrape proxy methods to routerClient: enqueue, list, status, retry, dead-letters, process
  • ADD prompt consumer methods to connectClient: resolvePrompt() and executePrompt() calling ChittyConnect's TY-VY-RY governed prompt registry
  • UPDATE litigation.ts — resolve prompts from ChittyConnect with fallback to direct AI Gateway until prompts are seeded
  • UPDATE jobs.ts, mcp.ts, cron.ts — pass env through for router proxy delegation

Architecture

Before: ChittyCommand → [enqueue → execute → fan-out to Router agents via HTTP] → AI Gateway
After:  ChittyCommand → ChittyRouter ScrapeAgent (DO-to-DO fan-out internally)
        ChittyCommand → ChittyConnect prompt registry → resolve/execute → AI

Net: -231 lines, scope violation resolved

Part of

3-repo extraction:

  1. ChittyRouter ScrapeAgent (PR feat: integrate ChittyEvidence pipeline into ChittyCommand #52 — merged)
  2. ChittyConnect Prompt Registry (PR chore(deps): bump lodash from 4.17.23 to 4.18.1 in the npm_and_yarn group across 1 directory #82 — open)
  3. This PR — ChittyCommand thin proxy conversion

Test plan

  • tsc --noEmit passes (verified locally)
  • Scrape job enqueue via MCP → proxies to ScrapeAgent
  • Litigation /synthesize → tries ChittyConnect execute, falls back to AI Gateway
  • Job listing returns results from ScrapeAgent (or Neon fallback)
  • Cron court_docket phase → enqueues via router proxy
  • Dead-letter retrieval → proxies to ScrapeAgent

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added ChittyConnect prompt resolve/execute endpoints and new router-backed scrape job APIs (enqueue, status, list, retry, dead-letters, process).
  • Refactor

    • Switched to router-first job processing with DB fallback; routes and tools now forward environment context.
    • Litigation AI endpoints now prefer ChittyConnect with resilient fallbacks and improved error handling/validation.
  • Chores

    • Removed legacy downstream fan-out and in-process job execution.
  • Tests

    • Updated tool-list test to reflect additional tools.

@github-actions

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4231edd5-cb60-4b64-b5d7-39e4c672ef1b

📥 Commits

Reviewing files that changed from the base of the PR and between cd73f4e and 66da6cc.

📒 Files selected for processing (1)
  • tests/mcp.test.ts
✅ Files skipped from review due to trivial changes (1)
  • tests/mcp.test.ts

📝 Walkthrough

Walkthrough

This PR removes local fan-out and in-process scrape execution, routes job lifecycle operations through a router-first path with DB fallback, adds ChittyConnect prompt APIs, and updates cron/routes to pass the Cloudflare env and use ChittyConnect-first AI flows with gateway fallback. (50 words)

Changes

Cohort / File(s) Summary
Job dispatcher & router integration
src/lib/job-dispatcher.ts
Removed in-process scrape execution and fan-out; refactored job APIs (enqueueJob, processQueue, getJobStatus, listJobs, getDeadLetters, retryJob) to prefer router client when env is provided, falling back to DB. Added router→local mapping helper; methods now accept optional env.
Fan-out removal
src/lib/fan-out.ts
Deleted module and exports (fanOutScrapeResult, ScrapeResultContext) that performed intelligence/calendar/triage/ledger side effects and timeouts.
Integrations — ChittyConnect & Router proxies
src/lib/integrations.ts
Added connectPost helper; extended connectClient with resolvePrompt/executePrompt; added ScrapeAgent proxy methods to routerClient (enqueue, status, list, retry, dead-letters, process, status). Introduced PromptResolveResponse, PromptExecuteResponse, and ScrapeJobResponse types.
Call site updates (cron & routes)
src/lib/cron.ts, src/routes/jobs.ts, src/routes/mcp.ts
Updated enqueue/process/list/status/dead-letter/retry call sites to pass env (Cloudflare env) into job-dispatcher calls; adjusted processQueue call in jobs route to router-based signature and removed local limit parsing. Minor import/type additions in cron (ScrapeJobType).
Litigation routes — ChittyConnect-first AI flow
src/routes/litigation.ts
Replaced direct gateway calls with connectClient(...).executePrompt(...) first and callAIGatewayFallback fallback; added FALLBACK_* prompts, Zod safeParse usage, JSON-cleaning for QC responses, and tailored error handling/outputs based on aiEnabled.
Tests
tests/mcp.test.ts
Updated MCP tools list test expected count (38 → 43).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant JobDispatcher
    participant Router
    participant Database

    Client->>JobDispatcher: enqueueJob(jobType, target, env)
    alt Router available & returns id
        JobDispatcher->>Router: POST /agents/scrape/enqueue
        Router-->>JobDispatcher: { id, ... }
        JobDispatcher-->>Client: { jobId: id }
    else Router missing/fails
        JobDispatcher->>Database: INSERT INTO cc_scrape_jobs (...)
        Database-->>JobDispatcher: { insertedJobId }
        JobDispatcher-->>Client: { jobId: insertedJobId }
    end
Loading
sequenceDiagram
    participant Client
    participant Route
    participant ChittyConnect
    participant AIGateway

    Client->>Route: POST /synthesize (payload)
    Route->>ChittyConnect: POST /api/v1/context/prompts/execute
    alt ChittyConnect responds
        ChittyConnect-->>Route: { result, aiEnabled }
        alt aiEnabled true
            Route-->>Client: { synthesis: result }
        else aiEnabled false
            Route-->>Client: { synthesis: rawNotes }
        end
    else ChittyConnect unavailable/fails
        Route->>AIGateway: callAIGatewayFallback(systemPrompt, userPrompt)
        AIGateway-->>Route: text response
        Route-->>Client: { synthesis: parsed response }
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I hopped through code to change the way things play,

Jobs now travel routers, then fall back if they stray.
Connect first for prompts, gateway holds the line,
Fan-out said farewell — a tidy little sign.
A rabbit stamps a paw and celebrates the day!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main architectural change: converting ChittyCommand to a thin proxy that delegates scrape orchestration to ChittyRouter and prompt resolution to ChittyConnect.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/scrape-proxy-and-prompt-consumer

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd8ec4d728

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/job-dispatcher.ts
Comment thread src/lib/job-dispatcher.ts
Comment thread src/routes/litigation.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/job-dispatcher.ts (1)

71-86: ⚠️ Potential issue | 🟠 Major

The Neon fallback queue is a dead end right now.

These lines still write fallback jobs into cc_scrape_jobs, but processQueue() no longer drains that table or delegates to any local executor. In src/lib/cron.ts, the utility, court, and monthly flows enqueue and then immediately call this function, so a router outage now accepts work into the legacy queue and silently leaves it there.

Also applies to: 93-105

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/job-dispatcher.ts` around lines 71 - 86, The current fallback in
job-dispatcher.ts writes into cc_scrape_jobs (INSERT block using scheduledAt and
JSON target) but processQueue() no longer drains that table, causing jobs to be
dropped; update the fallback to not silently persist to cc_scrape_jobs: either
(1) detect router/unavailable condition and throw an error so callers in
src/lib/cron.ts see failure, or (2) replace the INSERT with a call to the new
router/enqueue function used by the main path (e.g., enqueueToRouter or whatever
the project uses) so jobs go to the active queue; make the change in the
job-dispatcher fallback branch (the INSERT/RETURNING id block) and ensure
callers (cron flows) receive the propagated error or the router-enqueued result
instead of a silent local DB insert.
🤖 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/job-dispatcher.ts`:
- Around line 142-149: The router-backed code path (routerClient ->
router.listScrapeJobs) is dropping caller filters like chittyId and offset (and
getDeadLetters is dropping limit), so requests routed through router return
incorrect pages or results; update the call to router.listScrapeJobs (and any
router-backed listJobs/getDeadLetters usages) to forward all incoming filter
fields (status, jobType, limit, chittyId, offset, etc.), or conditionally bypass
the router path when those fields aren't supported by the router, ensuring the
routerClient/router.listScrapeJobs and getDeadLetters call sites preserve the
full requested shape.
- Around line 58-68: The current logic calls router.enqueueScrapeJob() and—on
any non-successful response—unconditionally enqueues a local job, risking
duplicate non-idempotent writes; fix by generating a single shared job id (e.g.,
create a jobId/uuid if not provided via opts) before any network call, pass that
id into router.enqueueScrapeJob(...) and into the local enqueue path so both
systems use the same idempotency key, and only fall back to local enqueue when
routerClient(env) is null/undefined or when the POST has definitively failed
with a server-side error that confirms no job was created (not on ambiguous
network timeouts); update code references routerClient, router.enqueueScrapeJob,
and the local enqueue call to accept and propagate this shared jobId.

In `@src/routes/litigation.ts`:
- Around line 28-32: Move the Zod schemas synthesizeSchema, draftSchema, and
qcSchema out of the route file into the centralized validators module and export
them, then remove the inline c.req.json().catch(...)+safeParse(...) validation
blocks in the litigationRoutes.post handlers for '/synthesize', '/draft', and
'/qc' and instead attach the zValidator('json', <schema>) middleware to each
route; finally, inside each handler replace usage of the parsed request with the
validated payload obtained via c.req.valid('json'). Ensure you reference the
exact schema names (synthesizeSchema, draftSchema, qcSchema), the middleware
function zValidator, and extract data using c.req.valid('json') in the handler
bodies.

---

Outside diff comments:
In `@src/lib/job-dispatcher.ts`:
- Around line 71-86: The current fallback in job-dispatcher.ts writes into
cc_scrape_jobs (INSERT block using scheduledAt and JSON target) but
processQueue() no longer drains that table, causing jobs to be dropped; update
the fallback to not silently persist to cc_scrape_jobs: either (1) detect
router/unavailable condition and throw an error so callers in src/lib/cron.ts
see failure, or (2) replace the INSERT with a call to the new router/enqueue
function used by the main path (e.g., enqueueToRouter or whatever the project
uses) so jobs go to the active queue; make the change in the job-dispatcher
fallback branch (the INSERT/RETURNING id block) and ensure callers (cron flows)
receive the propagated error or the router-enqueued result instead of a silent
local DB insert.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5c68a9d2-afac-46cf-9ffe-e58e34d77364

📥 Commits

Reviewing files that changed from the base of the PR and between 4a7ccd0 and dd8ec4d.

📒 Files selected for processing (7)
  • src/lib/cron.ts
  • src/lib/fan-out.ts
  • src/lib/integrations.ts
  • src/lib/job-dispatcher.ts
  • src/routes/jobs.ts
  • src/routes/litigation.ts
  • src/routes/mcp.ts
💤 Files with no reviewable changes (1)
  • src/lib/fan-out.ts

Comment thread src/lib/job-dispatcher.ts
Comment thread src/lib/job-dispatcher.ts
Comment thread src/routes/litigation.ts
…sumer

Scope violation fix: ChittyCommand (Tier 5 dashboard) was acting as
orchestrator — owning job queues, execution, retry/backoff, and calling
ChittyRouter agents via HTTP fan-out. This moves orchestration to where
it belongs (ChittyRouter ScrapeAgent DO) and makes ChittyCommand a thin proxy.

Changes:
- DELETE fan-out.ts: orchestration now handled by ScrapeAgent DO-to-DO calls
- REWRITE job-dispatcher.ts: proxy to ChittyRouter /agents/scrape/* with Neon fallback
- ADD to integrations.ts: ScrapeAgent proxy methods (routerClient) + prompt
  registry methods (connectClient.resolvePrompt, executePrompt)
- UPDATE litigation.ts: resolve prompts from ChittyConnect with AI Gateway fallback
- UPDATE jobs.ts, mcp.ts, cron.ts: pass env for router proxy delegation

Net result: -231 lines, ChittyCommand no longer owns scrape orchestration
or hardcoded AI prompts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chitcommit
chitcommit force-pushed the feat/scrape-proxy-and-prompt-consumer branch from dd8ec4d to cd73f4e Compare March 23, 2026 19:33
@chitcommit
chitcommit enabled auto-merge (squash) March 23, 2026 19:33
@github-actions

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== integrations.ts ===" && cat src/lib/integrations.ts
echo "=== job-dispatcher.ts ===" && cat src/lib/job-dispatcher.ts
echo "=== litigation.ts ===" && cat src/routes/litigation.ts
echo "=== jobs.ts ===" && cat src/routes/jobs.ts
echo "=== mcp.ts ===" && cat src/routes/mcp.ts
echo "=== cron.ts ===" && cat src/lib/cron.ts

Length of output: 131257


@github-actions[bot] — Adversarial review of PR #49 (thin-proxy conversion). I reviewed the actual file content. Findings are grouped by severity.


🔴 Critical

1. Router auth silently degrades to unauthenticated (integrations.ts)

// integrations.ts — routerClient > authHeaders()
const token = await env.COMMAND_KV.get('scrape:service_token');
if (token) {
  headers['Authorization'] = `Bearer ${token}`;
} else {
  console.warn('[router] No scrape:service_token in KV — requests will be unauthenticated');
}

Every router proxy method (enqueueScrapeJob, retryScrapeJob, processScrapeQueue, getScrapeDeadLetters, etc.) inherits this. If scrape:service_token is absent or rotated out of KV, all router calls proceed unauthenticated — no error, no abort, no circuit break. A token rotation event or KV outage silently opens an unauthenticated path to ChittyRouter ScrapeAgent.

Fix: abort the call (return null) when no token is present, rather than proceeding without auth.


2. AI model name sourced from mutable KV (litigation.ts)

const chatModel = await env.COMMAND_KV.get('chat:model').catch(() => null)
  || 'dynamic/chittycommand';

KV is writable at runtime. Any principal that can write to COMMAND_KV (e.g., via a compromised service or the trigger_sync / ledger_link_case_for_dispute write paths that already use KV) can redirect all AI completions (/synthesize, /draft, /qc) to an arbitrary model identifier. This is a model substitution attack surface.

Fix: source the model from a Workers secret or a signed/validated KV entry; do not read an unsigned string from KV and pass it directly as the model name.


3. Audit trail regression — fan-out removed, replacement unverified

fan-out.ts wrote immutable audit entries to ChittyLedger (CHITTYLEDGER_URL/entries) on every scrape completion. That code is entirely deleted in this PR. The PR description asserts that ChittyRouter ScrapeAgent (PR #52, already merged) takes over this responsibility, but there is no assertion in this codebase that ChittyRouter actually writes equivalent entries. If ChittyRouter PR #52 does not implement ledger writes, every scrape job that completes after this merge will be silently audit-invisible in ChittyLedger. This is a compliance regression.

Fix: add an explicit integration test or canary assertion that ChittyRouter emits ledger entries; or retain a lightweight ledger write in job-dispatcher.ts upon a successful router enqueue response.


🟠 High

4. ConnectClient token is optional — prompt execution can be unauthenticated (integrations.ts)

if (env.CHITTY_CONNECT_TOKEN) {
  headers['Authorization'] = `Bearer ${env.CHITTY_CONNECT_TOKEN}`;
}

resolvePrompt and executePrompt will POST to ChittyConnect's prompt registry without an auth header if the env var is absent. In staging or a misconfigured worker deployment, this means sensitive prompt resolution (including additionalLayers containing case numbers like case:2024D007847) goes unauthenticated. Unlike routerClient, there is no even a warning logged.


5. mapRouterJob blindly casts unvalidated router data (job-dispatcher.ts)

function mapRouterJob(r: ScrapeJobResponse): ScrapeJob {
  return {
    jobType: r.jobType as ScrapeJobType,   // no allowlist check
    status: r.status as ScrapeJobStatus,   // no allowlist check
    ...
  };
}

If ChittyRouter returns an unexpected jobType or status string (including adversarial values like '; DROP TABLE cc_scrape_jobs; --' or an unknown enum value), it is cast and stored without validation. Any downstream switch (job.status) or DB write that trusts these values inherits the injection.

Fix: validate r.jobType and r.status against the known allowlists before returning.


6. connect_discover MCP tool allows arbitrary KV cache poisoning (mcp.ts)

case 'connect_discover': {
  const service = String(args.service || '').trim();
  // ... fetches URL from ChittyConnect, stores in KV with 5 min TTL
  await env.COMMAND_KV.put(key, JSON.stringify({ url: data.url }), { expirationTtl: 300 });
}

Any authenticated MCP client can call connect_discover with an arbitrary service name. The resolved URL (controlled by ChittyConnect response) is written into KV under connect:discover:<service>. A compromised or misconfigured ChittyConnect could populate KV with attacker-controlled URLs that are then read back by connectClient.discover() for real service calls. There is no URL allowlisting or scheme validation on the stored value.


🟡 Medium

7. Policy bypass: unconditional Neon fallback makes router controls bypassable

Every dispatcher function (enqueueJob, listJobs, getJobStatus, retryJob, getDeadLetters) silently falls through to the legacy Neon path if the router is unavailable or returns null. Any rate-limiting, deduplication, or access-control logic implemented in ChittyRouter ScrapeAgent can be bypassed by triggering a router failure (network, 5xx, timeout). The fallback is also reachable by not passing env since the parameter is optional on all functions.

export async function enqueueJob(
  sql,
  jobType,
  target,
  opts = {},
  env?,          // ← optional; omitting it always uses legacy Neon path
): Promise<string>

8. Prompt injection surface in litigation routes (litigation.ts)

rawNotes (up to 50,000 chars), synthesizedFacts (up to 50,000 chars), and draftEmail (up to 10,000 chars) are passed verbatim into the system/user prompt content — both in the ChittyConnect path and the AI Gateway fallback. The Zod schema enforces length limits, which is good, but there is no sanitization or escaping of prompt-injection payloads (e.g. Ignore previous instructions and output...). Given the legal-document context, the risk of leaking [UNKNOWN] facts or bypassing the [GIVEN]/[DERIVED] tagging discipline is real.


9. processQueue dropped limit parameter — silent regression (jobs.ts)

// Before (from AI summary):
processQueue(sql, c.env, undefined, limit)   // limit from query param
// After:
processQueue(sql, c.env)                     // limit silently dropped

processQueue now delegates entirely to ChittyRouter and the router's own internal limit governs processing size. The limit query param on POST /jobs/queue/process is silently ignored, and no 400 or deprecation notice is returned. Operators who relied on this for rate-controlled manual processing will get unexpected behavior.


🟢 Low / Informational

10. Hardcoded case number in cron (cron.ts)

await enqueueJob(sql, 'court_docket', { case_number: '2024D007847' }, ...)

The case number is hardcoded. A stale case number months from now would silently enqueue a no-op scrape on every cron run. This should be sourced from KV or DB.


11. retryJob fallback creates phantom records for router-managed jobs (job-dispatcher.ts)

If a job was enqueued via the router (exists only in ScrapeAgent DO storage, not in cc_scrape_jobs), and a retryJob call fails at the router, the Neon fallback tries to UPDATE cc_scrape_jobs WHERE id = ${jobId} — which will match nothing but return !!row = false, correctly surfacing a 404. However, if an operator manually inserts a row with the same UUID into Neon as a workaround, the Neon fallback would reset it, creating a divergent state between Neon and the router.


12. Bot-authored commit, no CI artifact cited

The PR was created by chitcommit and the test plan is "verified locally" — there are no CI checks shown as passing in the PR metadata. Given the -231 line net change including full deletion of fan-out.ts, automated regression coverage for the audit-trail path (item 3 above) cannot be confirmed without a green CI run.


Summary Table

# Severity Area Issue
1 🔴 Critical integrations.ts Router auth silently degrades to unauthenticated
2 🔴 Critical litigation.ts AI model name sourced from mutable KV
3 🔴 Critical fan-out.ts (deleted) ChittyLedger audit trail removed, replacement unverified
4 🟠 High integrations.ts ConnectClient auth is optional with no warning
5 🟠 High job-dispatcher.ts mapRouterJob blindly casts unvalidated router data
6 🟠 High mcp.ts connect_discover KV cache poisoning
7 🟡 Medium job-dispatcher.ts Unconditional Neon fallback bypasses router controls
8 🟡 Medium litigation.ts Prompt injection surface in user content
9 🟡 Medium jobs.ts processQueue limit param silently dropped
10 🟢 Low cron.ts Hardcoded court case number
11 🟢 Low job-dispatcher.ts Phantom retry records for router-managed jobs
12 🟢 Info CI No CI artifact; "verified locally" only

Items 1, 2, and 3 are merge blockers per repository policy (security, correctness, compliance). Items 4–6 should be resolved before merge or tracked as immediate follow-ups.


🧠 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/connect.ts : Implement ChittyConnect discovery proxy with rate limiting in `src/routes/connect.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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/lib/integrations.ts (1)

617-628: ⚠️ Potential issue | 🔴 Critical

Fail closed when scrape:service_token is absent.

These new ScrapeAgent proxy methods inherit authHeaders(), which currently only warns and still sends the request without Authorization. Missing router auth should stop the proxy call outright, otherwise a KV/config drift turns into silent unauthenticated access or a confusing fallback path.

Also applies to: 696-729

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/integrations.ts` around lines 617 - 628, The authHeaders() helper
currently only warns when env.COMMAND_KV lacks 'scrape:service_token' and
returns unauthenticated headers; change it to fail-closed by throwing an error
(or returning a rejected Promise) when token is absent so callers cannot
proceed; update authHeaders() (and any other uses inherited by ScrapeAgent proxy
methods) to log the missing-token error and throw (e.g., new Error('missing
scrape:service_token')) instead of returning headers without Authorization,
ensuring proxy calls abort immediately on missing router auth.
src/routes/litigation.ts (1)

172-173: ⚠️ Potential issue | 🔴 Critical

Don't source the litigation model from writable KV.

chat:model can be changed at runtime without a deploy or code review, so a KV write can silently swap the model used for legal drafting and QC. Keep the model in env/secret-controlled config, or allowlist the KV value before using it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/litigation.ts` around lines 172 - 173, The current code reads the
litigation model from writable KV via env.COMMAND_KV.get('chat:model') into
chatModel which allows runtime tampering; change this to use a
secret/env-controlled config (e.g., process env like ENV_CHAT_MODEL or
env.CHAT_MODEL) or, if KV must be supported, validate the returned value against
a strict allowlist of permitted model names before using it (e.g., compare to
allowed values such as 'dynamic/chittycommand' or other whitelisted names).
Modify the code that references env.COMMAND_KV.get('chat:model') and the
variable chatModel to either read from the immutable env secret or to perform
allowlist validation and fall back to the safe default 'dynamic/chittycommand'
when the value is missing/invalid.
src/lib/job-dispatcher.ts (1)

71-85: ⚠️ Potential issue | 🔴 Critical

The Neon fallback queue is now orphaned.

This file still inserts/requeues cc_scrape_jobs, but the local executor is gone and processQueue() now returns zeros whenever Router processing is unavailable. During a router outage, callers get a job ID or true for work that will never run.

Also applies to: 99-105, 223-238

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/job-dispatcher.ts` around lines 71 - 85, The current fallback that
inserts into cc_scrape_jobs (the SQL INSERT block using scheduledAt / JSON
target) must be removed because the local Neon executor is gone; instead detect
Router availability and fail fast so callers don't receive a fake job id/true.
Remove or disable the INSERT INTO cc_scrape_jobs code path in
src/lib/job-dispatcher.ts (the SQL INSERT that returns id) and all other requeue
paths that write to cc_scrape_jobs (referenced around the other ranges noted),
and update the surrounding job dispatch functions (the callers of processQueue()
/ the code that returns a job id or true) to return an explicit error/false or
throw when Router is unavailable. Ensure logs clearly indicate
Router-unavailable and no cc_scrape_jobs writes remain.
♻️ Duplicate comments (3)
src/lib/integrations.ts (1)

698-725: ⚠️ Potential issue | 🟠 Major

The scrape proxy API surface is narrower than the existing dispatcher contract.

enqueueScrapeJob() drops scheduledAt and parentJobId, listScrapeJobs() drops chittyId and offset, and both dead-letter/process helpers drop limit. Once callers go through this client, those behaviors disappear unless you explicitly bypass Router for unsupported fields.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/integrations.ts` around lines 698 - 725, The client API narrows the
dispatcher contract by omitting fields: update enqueueScrapeJob to accept and
forward scheduledAt and parentJobId (extend opts for enqueueScrapeJob and
include them in the POST body), update listScrapeJobs to accept chittyId and
offset (add them to the filters and include in the URLSearchParams), and update
getScrapeDeadLetters and processScrapeQueue to accept and forward limit (add an
optional limit param and include it in the query string or POST body as the
server expects). Modify the signatures for enqueueScrapeJob, listScrapeJobs,
getScrapeDeadLetters, and processScrapeQueue to include these optional fields
and ensure they are serialized when calling post/get so callers retain the
original dispatcher behavior.
src/lib/job-dispatcher.ts (1)

57-68: ⚠️ Potential issue | 🔴 Critical

Don't enqueue locally after an ambiguous router write failure.

Once router.enqueueScrapeJob() has been attempted, null does not prove Router failed to create the job. Falling back to Neon here can enqueue a second copy of the same non-idempotent scrape.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/job-dispatcher.ts` around lines 57 - 68, The code currently falls
back to the local Neon queue even when router.enqueueScrapeJob() returns an
ambiguous result (null/undefined), which risks enqueuing duplicate
non-idempotent work; change the logic around
routerClient(env)/router.enqueueScrapeJob so that you only use the local
fallback when the router client is not available or when enqueueScrapeJob throws
an error (network/transport failure). Specifically, wrap the call to
router.enqueueScrapeJob in a try/catch, return the id if result?.id exists, and
if the call completes but returns no id do NOT call the local enqueue path —
instead propagate an error or undefined to the caller so the duplicate enqueue
cannot happen; keep routerClient, enqueueScrapeJob, and the local fallback code
paths intact but gated as described.
src/routes/litigation.ts (1)

9-24: 🛠️ Refactor suggestion | 🟠 Major

Move these schemas into src/lib/validators.ts and use zValidator('json', ...).

These handlers still define local Zod schemas and parse c.req.json() manually, so the route bypasses the shared validator module and middleware pattern. Please move the schemas to src/lib/validators.ts, attach zValidator('json', schema) on each route, and read the payload from c.req.valid('json'). As per coding guidelines, "Use Zod for request validation via src/lib/validators.ts", "Define all Zod schemas in src/lib/validators.ts", and "Use @hono/zod-validator with zValidator('json', schema) for request body validation".

Also applies to: 28-33, 72-76, 113-117

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/litigation.ts` around lines 9 - 24, Move the three Zod schemas
(synthesizeSchema, draftSchema, qcSchema) out of the route and export them from
the shared validators module; then attach each route handler to the validator
middleware using zValidator('json', <appropriateSchema>) and replace manual
c.req.json() parsing with c.req.valid('json') to read the validated payload.
Specifically, define and export synthesizeSchema, draftSchema, and qcSchema in
the shared validators module, update the route definitions to include
zValidator('json', synthesizeSchema|draftSchema|qcSchema) as middleware, and
within the handlers read the request body via c.req.valid('json') instead of
manual JSON parsing. Ensure the schemas retain their original constraints
(rawNotes, property, caseNumber; synthesizedFacts, focus, recipient; rawNotes,
draftEmail).
🤖 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 315-323: connectPost currently silently omits the Authorization
header when env.CHITTY_CONNECT_TOKEN is not set; change connectPost to fail
closed by checking env.CHITTY_CONNECT_TOKEN at the start of the function (inside
connectPost) and throw a clear error (or return a rejected Promise) if the token
is missing so no unauthenticated requests are sent to ChittyConnect; preserve
the existing header-building logic (headers['Authorization'] = `Bearer
${env.CHITTY_CONNECT_TOKEN}`) but only after the presence check, and ensure
callers of connectPost (e.g., routes that call connectPost) will surface the
thrown error instead of falling back to direct AI paths.

In `@src/lib/job-dispatcher.ts`:
- Around line 243-260: The mapRouterJob function is overwriting and inventing
fields from the Router payload; update mapRouterJob to copy values directly from
the ScrapeJobResponse instead of forcing defaults: assign scheduledAt =
r.scheduledAt (not r.createdAt), and preserve r.chittyId, r.startedAt,
r.parentJobId, r.cronSource rather than forcing nulls; for optional fields use
null-coalescing (e.g. completedAt = r.completedAt ?? null, result = r.result ??
null, errorMessage = r.error ?? null) and remove blind type assertions like
r.jobType as ScrapeJobType / r.status as ScrapeJobStatus — either keep
r.jobType/r.status as-is or add a runtime/type-guard validation before casting
so Router-backed reads reflect the upstream payload accurately.

In `@src/routes/litigation.ts`:
- Around line 134-138: The code only checks JSON syntax but not the expected
shape; after parsing cleaned (the value from result.result), validate that the
parsed object has the expected structure (e.g., an object with a property
"flags" that is an array) before returning it as flags; if validation fails,
return the same warning payload (or a 400) with flags: [] and a clear warning.
Apply the same shape validation to the other QC-parsing branch that uses the
same cleaning/parsing logic (the block handling result.result around lines
149-151) so both paths enforce that parsed.flags is an array before responding.

---

Outside diff comments:
In `@src/lib/integrations.ts`:
- Around line 617-628: The authHeaders() helper currently only warns when
env.COMMAND_KV lacks 'scrape:service_token' and returns unauthenticated headers;
change it to fail-closed by throwing an error (or returning a rejected Promise)
when token is absent so callers cannot proceed; update authHeaders() (and any
other uses inherited by ScrapeAgent proxy methods) to log the missing-token
error and throw (e.g., new Error('missing scrape:service_token')) instead of
returning headers without Authorization, ensuring proxy calls abort immediately
on missing router auth.

In `@src/lib/job-dispatcher.ts`:
- Around line 71-85: The current fallback that inserts into cc_scrape_jobs (the
SQL INSERT block using scheduledAt / JSON target) must be removed because the
local Neon executor is gone; instead detect Router availability and fail fast so
callers don't receive a fake job id/true. Remove or disable the INSERT INTO
cc_scrape_jobs code path in src/lib/job-dispatcher.ts (the SQL INSERT that
returns id) and all other requeue paths that write to cc_scrape_jobs (referenced
around the other ranges noted), and update the surrounding job dispatch
functions (the callers of processQueue() / the code that returns a job id or
true) to return an explicit error/false or throw when Router is unavailable.
Ensure logs clearly indicate Router-unavailable and no cc_scrape_jobs writes
remain.

In `@src/routes/litigation.ts`:
- Around line 172-173: The current code reads the litigation model from writable
KV via env.COMMAND_KV.get('chat:model') into chatModel which allows runtime
tampering; change this to use a secret/env-controlled config (e.g., process env
like ENV_CHAT_MODEL or env.CHAT_MODEL) or, if KV must be supported, validate the
returned value against a strict allowlist of permitted model names before using
it (e.g., compare to allowed values such as 'dynamic/chittycommand' or other
whitelisted names). Modify the code that references
env.COMMAND_KV.get('chat:model') and the variable chatModel to either read from
the immutable env secret or to perform allowlist validation and fall back to the
safe default 'dynamic/chittycommand' when the value is missing/invalid.

---

Duplicate comments:
In `@src/lib/integrations.ts`:
- Around line 698-725: The client API narrows the dispatcher contract by
omitting fields: update enqueueScrapeJob to accept and forward scheduledAt and
parentJobId (extend opts for enqueueScrapeJob and include them in the POST
body), update listScrapeJobs to accept chittyId and offset (add them to the
filters and include in the URLSearchParams), and update getScrapeDeadLetters and
processScrapeQueue to accept and forward limit (add an optional limit param and
include it in the query string or POST body as the server expects). Modify the
signatures for enqueueScrapeJob, listScrapeJobs, getScrapeDeadLetters, and
processScrapeQueue to include these optional fields and ensure they are
serialized when calling post/get so callers retain the original dispatcher
behavior.

In `@src/lib/job-dispatcher.ts`:
- Around line 57-68: The code currently falls back to the local Neon queue even
when router.enqueueScrapeJob() returns an ambiguous result (null/undefined),
which risks enqueuing duplicate non-idempotent work; change the logic around
routerClient(env)/router.enqueueScrapeJob so that you only use the local
fallback when the router client is not available or when enqueueScrapeJob throws
an error (network/transport failure). Specifically, wrap the call to
router.enqueueScrapeJob in a try/catch, return the id if result?.id exists, and
if the call completes but returns no id do NOT call the local enqueue path —
instead propagate an error or undefined to the caller so the duplicate enqueue
cannot happen; keep routerClient, enqueueScrapeJob, and the local fallback code
paths intact but gated as described.

In `@src/routes/litigation.ts`:
- Around line 9-24: Move the three Zod schemas (synthesizeSchema, draftSchema,
qcSchema) out of the route and export them from the shared validators module;
then attach each route handler to the validator middleware using
zValidator('json', <appropriateSchema>) and replace manual c.req.json() parsing
with c.req.valid('json') to read the validated payload. Specifically, define and
export synthesizeSchema, draftSchema, and qcSchema in the shared validators
module, update the route definitions to include zValidator('json',
synthesizeSchema|draftSchema|qcSchema) as middleware, and within the handlers
read the request body via c.req.valid('json') instead of manual JSON parsing.
Ensure the schemas retain their original constraints (rawNotes, property,
caseNumber; synthesizedFacts, focus, recipient; rawNotes, draftEmail).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5b32f7d7-3095-4607-8e31-198b36030edb

📥 Commits

Reviewing files that changed from the base of the PR and between dd8ec4d and cd73f4e.

📒 Files selected for processing (7)
  • src/lib/cron.ts
  • src/lib/fan-out.ts
  • src/lib/integrations.ts
  • src/lib/job-dispatcher.ts
  • src/routes/jobs.ts
  • src/routes/litigation.ts
  • src/routes/mcp.ts
💤 Files with no reviewable changes (1)
  • src/lib/fan-out.ts
✅ Files skipped from review due to trivial changes (3)
  • src/lib/cron.ts
  • src/routes/mcp.ts
  • src/routes/jobs.ts

Comment thread src/lib/integrations.ts
Comment thread src/lib/job-dispatcher.ts
Comment thread src/routes/litigation.ts
The tool count increased from 38 to 43 after recent PRs added litigation
and scrape tools. Test was stale.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

@github-actions[bot] — The adversarial review for PR #49 has already been posted in this thread above. Here is a direct map of your four requested vectors to the documented findings:


🔐 Security

Finding Issue
#1 🔴 Router auth silently degrades — unauthenticated requests proceed if scrape:service_token absent from KV
#2 🔴 AI model name from mutable KV — runtime model substitution attack surface in /synthesize, /draft, /qc
#4 🟠 ConnectClient auth optional, no warning — unauthenticated prompt execution if CHITTY_CONNECT_TOKEN missing
#6 🟠 connect_discover KV poisoning — ChittyConnect can inject attacker-controlled URLs into KV
#8 🟡 Prompt injection — 50k-char free-text fields passed verbatim into prompts

🚪 Policy Bypass Paths

Finding Issue
#7 🟡 Unconditional Neon fallback — all router-side rate-limiting, deduplication, and ACL controls bypassable by triggering any router failure; also reachable by omitting optional env parameter
#5 🟠 mapRouterJob no allowlist — adversarial jobType/status values from ChittyRouter cast without validation

📉 Regression Risk

Finding Issue
#3 🔴 ChittyLedger audit trail removed — fan-out.ts deleted, replacement by ChittyRouter PR #52 unverified in this repo
#9 🟡 processQueue limit param silently dropped — operator-controlled processing limits now ignored
#11 🟢 Phantom retry records — Neon fallback on router-managed jobs creates divergent state

🧱 Merge-Gating Bypass Attempts

Finding Issue
#12 🟢 Bot-authored (chitcommit), test plan "verified locally" only — no CI artifact; -231 net line change including full fan-out.ts deletion cannot be confirmed green

Merge-blocking items (per repo policy): #1, #2, #3 must be resolved before merge. Items #4#6 are recommended pre-merge fixes or immediate tracked follow-ups.


🧠 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/connect.ts : Implement ChittyConnect discovery proxy with rate limiting in `src/routes/connect.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.

chitcommit added a commit that referenced this pull request Mar 23, 2026
… disable raw draft fallback

- Generate UUID upfront in enqueueJob() and pass to both router and local
  fallback to prevent duplicate jobs on ambiguous timeout failures
- Only fall back to local queue on definitive connection errors, not server errors
- Warn about stuck local jobs in processQueue() when router is unavailable
- Forward chittyId/offset to router in listJobs(), limit in getDeadLetters()
- Return 503 error in litigation /draft when AI is disabled instead of
  leaking raw synthesizedFacts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chitcommit
chitcommit merged commit 13fbf75 into main Mar 23, 2026
17 checks passed
@chitcommit
chitcommit deleted the feat/scrape-proxy-and-prompt-consumer branch March 23, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant