feat: service binding + switch layer — tasks first#141
Conversation
Replace 8 direct HTTP calls to tasks.chitty.cc with serviceFetch() which routes through SVC_TASKS service binding. No Bearer tokens needed - service bindings are direct Worker-to-Worker calls. New service-switch.js provides: - Service binding routing (no auth, no network) - HTTP fallback with service token - Kill switch (disable/enable per service) - In-memory cache with 1-min TTL - KV-backed switch configuration This is the first service migrated to the binding pattern. Remaining: ledger (20 calls), finance (8), contextual (2), id (1), mint (1). -148 lines of auth + HTTP boilerplate replaced by 39 lines of serviceFetch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA new service routing and kill-switch infrastructure layer is introduced via Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as MCP Tool Handler
participant Switch as Service Switch Layer
participant Cache as In-Memory Cache
participant KV as KV Store
participant Cred as Credential Helper
participant Route as Tasks Service<br/>(Binding or HTTP)
Handler->>Switch: serviceFetch(env, "tasks", path, options)
Switch->>Cache: Check cached switch config
alt Cache Miss
Cache-->>Switch: Miss
Switch->>KV: Load service:switches
KV-->>Switch: Config override (or error)
Switch->>Cache: Store config + 60s TTL
else Cache Hit
Cache-->>Switch: Cached config
end
alt Switch Disabled
Switch-->>Handler: 503 JSON error
else Switch Enabled & Invalid Mode
Switch-->>Handler: 500 JSON error
else Valid Binding Mode
Switch->>Route: Route via Cloudflare binding
Route-->>Switch: Response
Switch-->>Handler: Response
else Valid HTTP Mode
Switch->>Cred: Get service token
Cred-->>Switch: Bearer token
Switch->>Route: HTTP request + Authorization header
Route-->>Switch: Response
Switch-->>Handler: Response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ 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 |
Replace ALL 44 direct HTTP calls to downstream services with serviceFetch() routing through Cloudflare service bindings. Services migrated: - ledger (20 calls) → SVC_LEDGER - finance (8 calls) → SVC_FINANCE - tasks (8 calls) → SVC_TASKS (from PR #141) - contextual (2 calls) → SVC_CONTEXTUAL - id (1 call) → SVC_ID - mint (1 call) → SVC_ID New bindings in wrangler.jsonc (all 3 env blocks): SVC_TASKS, SVC_LEDGER, SVC_FINANCE, SVC_CONTEXTUAL, SVC_ID, SVC_EVIDENCE, SVC_CHRONICLE, SVC_DISPUTES, SVC_SCORE Benefits: - Zero Bearer tokens needed for downstream calls - No auth chain to debug or rotate - No network hop (Worker-to-Worker via CF internal) - Kill switch per service via KV configuration - HTTP fallback mode if binding unavailable Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace ALL 44 direct HTTP calls to downstream services with serviceFetch() routing through Cloudflare service bindings. Services migrated: - ledger (20 calls) → SVC_LEDGER - finance (8 calls) → SVC_FINANCE - tasks (8 calls) → SVC_TASKS (from PR #141) - contextual (2 calls) → SVC_CONTEXTUAL - id (1 call) → SVC_ID - mint (1 call) → SVC_ID New bindings in wrangler.jsonc (all 3 env blocks): SVC_TASKS, SVC_LEDGER, SVC_FINANCE, SVC_CONTEXTUAL, SVC_ID, SVC_EVIDENCE, SVC_CHRONICLE, SVC_DISPUTES, SVC_SCORE Benefits: - Zero Bearer tokens needed for downstream calls - No auth chain to debug or rotate - No network hop (Worker-to-Worker via CF internal) - Kill switch per service via KV configuration - HTTP fallback mode if binding unavailable Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Migrates the MCP task tool dispatcher from direct tasks.chitty.cc HTTP calls to a Worker-to-Worker service binding (SVC_TASKS) via a new serviceFetch() switch layer, enabling kill-switch style routing without redeploys.
Changes:
- Add
SVC_TASKSservice binding to Wrangler envs. - Refactor
chitty_task_*tool handlers to callserviceFetch()and centralize response parsing. - Introduce
src/lib/service-switch.jsto route via service binding / HTTP / disabled modes with KV-backed configuration + in-memory cache.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| wrangler.jsonc | Adds SVC_TASKS service binding entries for dev/staging/production envs. |
| src/mcp/tool-dispatcher.js | Replaces tasks HTTP+auth boilerplate with serviceFetch() calls. |
| src/lib/service-switch.js | New routing + kill-switch layer with KV-backed switch config and caching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result = data; | ||
| if (!args.task_id) return { content: [{ type: "text", text: "Missing required parameter: task_id" }], isError: true }; | ||
| response = await serviceFetch(env, "tasks", `/api/v1/tasks/${encodeURIComponent(args.task_id)}/complete`, { | ||
| method: "POST", body: args.result ? { result: args.result } : {}, |
There was a problem hiding this comment.
chitty_task_complete changes semantics: args.result ? { result: args.result } : {} will drop valid falsy results (e.g., empty string/0). Use an args.result !== undefined check to preserve the previous behavior.
| method: "POST", body: args.result ? { result: args.result } : {}, | |
| method: "POST", body: args.result !== undefined ? { result: args.result } : {}, |
| else if (name.startsWith("chitty_task_")) { | ||
| const { error: taskErr, headers: taskAuth } = await requireServiceAuth( | ||
| "chittytask", | ||
| "ChittyTask", | ||
| ); | ||
| if (taskErr) return taskErr; | ||
| const taskHeaders = { ...taskAuth, "Content-Type": "application/json" }; | ||
| let response; | ||
|
|
||
| if (name === "chitty_task_create") { | ||
| const body = { | ||
| title: args.title, | ||
| task_type: args.task_type, | ||
| assigned_agent: args.assigned_agent, | ||
| }; | ||
| const body = { title: args.title, task_type: args.task_type, assigned_agent: args.assigned_agent }; | ||
| if (args.description !== undefined) body.description = args.description; | ||
| if (args.priority !== undefined) body.priority = args.priority; | ||
| if (args.payload !== undefined) body.payload = args.payload; | ||
| if (args.depends_on !== undefined) body.depends_on = args.depends_on; | ||
| const response = await fetch("https://tasks.chitty.cc/api/v1/tasks", { | ||
| method: "POST", | ||
| headers: taskHeaders, | ||
| body: JSON.stringify(body), | ||
| }); | ||
| const { data, error: respErr } = await checkAndParseJson( | ||
| response, | ||
| "ChittyTask", | ||
| ); | ||
| if (respErr) return respErr; | ||
| result = data; | ||
| response = await serviceFetch(env, "tasks", "/api/v1/tasks", { method: "POST", body }); | ||
| } else if (name === "chitty_task_list") { | ||
| const params = new URLSearchParams(); |
There was a problem hiding this comment.
The task tools were refactored to route through serviceFetch, but there are no tool-dispatcher tests covering these chitty_task_* paths (and they previously exercised real HTTP + auth logic). Add unit tests that stub env.SVC_TASKS.fetch (binding mode) and verify request paths/methods/bodies plus error handling (e.g., missing args).
| * Execute a fetch through the service switch layer. | ||
| * Uses service binding when available, falls back to HTTP. | ||
| * | ||
| * @param {object} env - Worker env bindings | ||
| * @param {string} serviceName - Service key (e.g., "tasks", "ledger") | ||
| * @param {string} path - API path (e.g., "/api/v1/tasks") | ||
| * @param {object} [options] - Fetch options (method, body, headers) | ||
| * @returns {Promise<Response>} | ||
| */ | ||
| export async function serviceFetch(env, serviceName, path, options = {}) { | ||
| const sw = await getSwitch(env, serviceName); | ||
|
|
||
| if (!sw.enabled) { | ||
| return new Response(JSON.stringify({ | ||
| error: `Service "${serviceName}" is disabled`, | ||
| reason: sw.reason || "maintenance", | ||
| }), { status: 503, headers: { "Content-Type": "application/json" } }); | ||
| } | ||
|
|
||
| const headers = { | ||
| "Content-Type": "application/json", | ||
| "X-ChittyOS-Caller": "chittyconnect", | ||
| ...options.headers, | ||
| }; | ||
|
|
||
| if (sw.mode === "binding" && sw.binding && env[sw.binding]) { | ||
| // Service binding — direct Worker-to-Worker, no auth needed | ||
| const req = new Request(`https://internal${path}`, { | ||
| method: options.method || "GET", | ||
| headers, | ||
| body: options.body ? JSON.stringify(options.body) : undefined, | ||
| }); | ||
| return env[sw.binding].fetch(req); | ||
| } | ||
|
|
||
| if (sw.mode === "http" && sw.url) { | ||
| // HTTP fallback — needs service token | ||
| const { getServiceToken } = await import("./credential-helper.js"); | ||
| const token = await getServiceToken(env, serviceName); | ||
| if (token) { | ||
| headers.Authorization = `Bearer ${token}`; | ||
| } | ||
|
|
||
| const req = new Request(`${sw.url}${path}`, { | ||
| method: options.method || "GET", | ||
| headers, | ||
| body: options.body ? JSON.stringify(options.body) : undefined, | ||
| }); | ||
| return fetch(req); | ||
| } | ||
|
|
||
| return new Response(JSON.stringify({ | ||
| error: `Service "${serviceName}" has invalid switch mode: ${sw.mode}`, | ||
| }), { status: 500, headers: { "Content-Type": "application/json" } }); |
There was a problem hiding this comment.
serviceFetch claims to “fall back to HTTP”, but if mode === "binding" and the binding is missing (or misnamed), the function currently returns a 500 “invalid switch mode: binding”. Either implement an actual fallback to HTTP when the binding isn’t present, or return a clearer 503 misconfiguration error (and adjust the docstring).
| if (sw.mode === "http" && sw.url) { | ||
| // HTTP fallback — needs service token | ||
| const { getServiceToken } = await import("./credential-helper.js"); | ||
| const token = await getServiceToken(env, serviceName); | ||
| if (token) { | ||
| headers.Authorization = `Bearer ${token}`; | ||
| } | ||
|
|
There was a problem hiding this comment.
In HTTP mode, token lookup uses getServiceToken(env, serviceName). For serviceName: "tasks" this maps to CHITTY_TASKS_TOKEN / services/tasks/service_token, but the repo’s secrets manifest documents CHITTY_TASK_TOKEN. Add an override/mapping (e.g., tasks→chittytask) or support tokenServiceName in the switch config so HTTP fallback won’t silently lose auth.
| /** | ||
| * Execute a fetch through the service switch layer. | ||
| * Uses service binding when available, falls back to HTTP. | ||
| * | ||
| * @param {object} env - Worker env bindings | ||
| * @param {string} serviceName - Service key (e.g., "tasks", "ledger") | ||
| * @param {string} path - API path (e.g., "/api/v1/tasks") | ||
| * @param {object} [options] - Fetch options (method, body, headers) | ||
| * @returns {Promise<Response>} | ||
| */ | ||
| export async function serviceFetch(env, serviceName, path, options = {}) { | ||
| const sw = await getSwitch(env, serviceName); | ||
|
|
||
| if (!sw.enabled) { | ||
| return new Response(JSON.stringify({ | ||
| error: `Service "${serviceName}" is disabled`, | ||
| reason: sw.reason || "maintenance", | ||
| }), { status: 503, headers: { "Content-Type": "application/json" } }); | ||
| } | ||
|
|
||
| const headers = { | ||
| "Content-Type": "application/json", | ||
| "X-ChittyOS-Caller": "chittyconnect", | ||
| ...options.headers, | ||
| }; | ||
|
|
||
| if (sw.mode === "binding" && sw.binding && env[sw.binding]) { | ||
| // Service binding — direct Worker-to-Worker, no auth needed | ||
| const req = new Request(`https://internal${path}`, { | ||
| method: options.method || "GET", | ||
| headers, | ||
| body: options.body ? JSON.stringify(options.body) : undefined, | ||
| }); | ||
| return env[sw.binding].fetch(req); | ||
| } | ||
|
|
||
| if (sw.mode === "http" && sw.url) { | ||
| // HTTP fallback — needs service token | ||
| const { getServiceToken } = await import("./credential-helper.js"); | ||
| const token = await getServiceToken(env, serviceName); | ||
| if (token) { | ||
| headers.Authorization = `Bearer ${token}`; | ||
| } | ||
|
|
||
| const req = new Request(`${sw.url}${path}`, { | ||
| method: options.method || "GET", | ||
| headers, | ||
| body: options.body ? JSON.stringify(options.body) : undefined, | ||
| }); | ||
| return fetch(req); | ||
| } | ||
|
|
||
| return new Response(JSON.stringify({ | ||
| error: `Service "${serviceName}" has invalid switch mode: ${sw.mode}`, | ||
| }), { status: 500, headers: { "Content-Type": "application/json" } }); | ||
| } |
There was a problem hiding this comment.
New routing + kill-switch logic in service-switch.js isn’t covered by tests. Add unit tests for: disabled mode (503), binding mode (Request URL/path + headers/body), HTTP mode (Authorization header behavior), and cache invalidation.
| "tail_consumers": [{ "service": "chittytrack" }] | ||
| "services": [ | ||
| { "binding": "SVC_TASKS", "service": "chittyagent-tasks", "environment": "production" } |
There was a problem hiding this comment.
The SVC_TASKS service binding in the dev env is pinned to the downstream service’s production environment. If this isn’t intentional, dev traffic will hit production tasks data. Consider binding dev→dev (and staging→staging) or document why prod is required.
| "tail_consumers": [{ "service": "chittytrack" }] | |
| "services": [ | |
| { "binding": "SVC_TASKS", "service": "chittyagent-tasks", "environment": "production" } | |
| "tail_consumers": [{ "service": "chittytrack" }], | |
| "services": [ | |
| { "binding": "SVC_TASKS", "service": "chittyagent-tasks", "environment": "dev" } |
| "tail_consumers": [{ "service": "chittytrack" }] | ||
| "services": [ | ||
| { "binding": "SVC_TASKS", "service": "chittyagent-tasks", "environment": "production" } |
There was a problem hiding this comment.
The SVC_TASKS service binding in the staging env is pinned to the downstream service’s production environment. If this isn’t intentional, staging traffic will hit production tasks data. Consider binding staging→staging or document why prod is required.
| "tail_consumers": [{ "service": "chittytrack" }] | |
| "services": [ | |
| { "binding": "SVC_TASKS", "service": "chittyagent-tasks", "environment": "production" } | |
| "tail_consumers": [{ "service": "chittytrack" }], | |
| "services": [ | |
| { "binding": "SVC_TASKS", "service": "chittyagent-tasks", "environment": "staging" } |
| * | ||
| * Switches are read from SERVICE_SWITCHES KV with fallback to defaults. | ||
| * | ||
| * @canonical-uri chittycanon://core/services/chittyconnect/lib/service-switch | ||
| */ | ||
|
|
||
| const DEFAULTS = { | ||
| tasks: { enabled: true, mode: "binding", binding: "SVC_TASKS" }, | ||
| ledger: { enabled: true, mode: "http", url: "https://ledger.chitty.cc" }, | ||
| finance: { enabled: true, mode: "http", url: "https://finance.chitty.cc" }, | ||
| contextual: { enabled: true, mode: "http", url: "https://contextual.chitty.cc" }, | ||
| id: { enabled: true, mode: "http", url: "https://id.chitty.cc" }, | ||
| mint: { enabled: true, mode: "http", url: "https://mint.chitty.cc" }, | ||
| evidence: { enabled: true, mode: "http", url: "https://evidence.chitty.cc" }, | ||
| chronicle: { enabled: true, mode: "http", url: "https://chronicle.chitty.cc" }, | ||
| disputes: { enabled: true, mode: "http", url: "https://disputes.chitty.cc" }, | ||
| score: { enabled: true, mode: "http", url: "https://score.chitty.cc" }, | ||
| }; | ||
|
|
||
| let _cache = null; | ||
| let _cacheTs = 0; | ||
| const CACHE_TTL = 60_000; // 1 minute | ||
|
|
||
| /** | ||
| * Load service switches from KV with in-memory cache. | ||
| */ | ||
| async function loadSwitches(env) { | ||
| const now = Date.now(); | ||
| if (_cache && (now - _cacheTs) < CACHE_TTL) return _cache; | ||
|
|
||
| try { | ||
| const raw = env.IDEMP_KV ? await env.IDEMP_KV.get("service:switches") : null; | ||
| _cache = raw ? { ...DEFAULTS, ...JSON.parse(raw) } : { ...DEFAULTS }; | ||
| } catch { |
There was a problem hiding this comment.
Comment says switches are read from SERVICE_SWITCHES KV, but the implementation reads service:switches from env.IDEMP_KV. Align the comment with the actual binding/key (or change the code to use a dedicated KV binding if that’s the intent).
| let _cache = null; | ||
| let _cacheTs = 0; | ||
| const CACHE_TTL = 60_000; // 1 minute | ||
|
|
||
| /** | ||
| * Load service switches from KV with in-memory cache. | ||
| */ | ||
| async function loadSwitches(env) { | ||
| const now = Date.now(); | ||
| if (_cache && (now - _cacheTs) < CACHE_TTL) return _cache; | ||
|
|
||
| try { | ||
| const raw = env.IDEMP_KV ? await env.IDEMP_KV.get("service:switches") : null; | ||
| _cache = raw ? { ...DEFAULTS, ...JSON.parse(raw) } : { ...DEFAULTS }; | ||
| } catch { | ||
| _cache = { ...DEFAULTS }; | ||
| } | ||
| _cacheTs = now; | ||
| return _cache; | ||
| } |
There was a problem hiding this comment.
The in-memory switch cache is global to the module, not per env. In tests (and any multi-env runtime), this can leak switch state across different env objects and cause flaky behavior. Consider caching per-env (e.g., WeakMap<env, {value, ts}>) similar to credential-helper’s broker cache.
Summary
Replace 8 direct HTTP calls to tasks.chitty.cc with
serviceFetch()routing throughSVC_TASKSservice binding.Before: tool dispatcher →
requireServiceAuth("chittytask")→CHITTY_TASK_TOKEN→https://tasks.chitty.cc/api/v1/tasks→ Bearer auth checkAfter: tool dispatcher →
serviceFetch(env, "tasks", path)→env.SVC_TASKS.fetch()→ direct Worker-to-Worker (no auth, no network)New:
service-switch.jsserviceFetch(env, service, path, opts)— routes via binding or HTTP fallbackgetSwitch(env, service)— returns enable/disable/mode stateImpact
CHITTY_TASK_TOKENdependencyRemaining (same pattern)
Generated with Claude Code
Summary by CodeRabbit
Release Notes