Polyglot MCP fleet + image-host server + Edison catalog contract (edison-jwt verify) - #12
Open
Miyamura80 wants to merge 13 commits into
Open
Polyglot MCP fleet + image-host server + Edison catalog contract (edison-jwt verify)#12Miyamura80 wants to merge 13 commits into
Miyamura80 wants to merge 13 commits into
Conversation
Captures the decision to move from a single Gmail MCP to a polyglot, per-server-deploy fleet: TS/Cloudflare-Workers default (Python/FastMCP for Gmail + heavy cases), shared React MCP-UI lib, Edison-minted per-user JWT auth (pluggable, v1 bearer), edison_hosted catalog flag, image hosting as wave 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
Add a Direction section noting the transition from a single Gmail MCP to a fleet of first-party, open-source streamable-HTTP servers (TS/Cloudflare Workers default; Python/FastMCP for Gmail + heavy cases), linking the new strategy doc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
Resolve the repo-shape open question: this repo is refactored in place into the
polyglot monorepo (not greenfield). Grow additively — add servers/ (TS Workers)
+ shared/{ui,auth,catalog} around the existing Python/Gmail app, whose deploy is
untouched; Gmail becomes a co-tenant, optional servers/gmail/ relocation later.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
… server
Add the additive monorepo skeleton and the first TypeScript-on-Cloudflare-Workers
first-party server, per docs/mcp_commodity_fleet_strategy.md.
- servers/image-host: streamable-HTTP MCP (McpAgent + R2 binding). upload_image
takes base64, validates by magic bytes (png/jpeg/webp/gif; svg rejected),
stores under an unguessable key, and returns a public non-expiring URL served
by the worker at /i/<key>. delete_image companion. Pure logic (validation,
key-gen, auth) is dependency-free and unit-tested offline via `bun test`
(28 tests); typechecks clean against the real SDKs.
- Pluggable auth (open | bearer | edison-jwt); v1 ships bearer, edison-jwt is a
loud 501 stub so the drop-in seam is real but never silently open.
- shared/{auth,catalog,ui}: fleet contracts (placeholders + auth reference).
- .github/workflows/servers_ts.yaml: path-scoped bun typecheck + test, matrix
over servers. .gitignore: TS toolchain artifacts.
The existing Python/Gmail app and its deploy are untouched.
…hardening - delete_image: head-check before delete so `deleted` is truthful (R2 delete is a no-op on a missing key and can't report it). - upload_image: fail loudly when PUBLIC_BASE_URL is unset instead of returning a non-embeddable relative URL (the tool's whole promise is an absolute URL). - upload_image: reject oversized payloads before base64 decode (base64PayloadTooLarge) to avoid the ~3x peak allocation on a large blob. - serveImage: guard decodeURIComponent (malformed %-encoding -> 404, not 500) and set X-Content-Type-Options: nosniff on served bytes. - parsePositiveInt: use Number + isInteger so junk like "10MB" is rejected, not silently truncated to 10. - resolveAuthMode: return AuthMode | string so an unknown mode is passed through and rejected (fail closed) rather than cast into the AuthMode type; fix the misleading "read-only-ish" open-mode comment (open still exposes mutating tools). - Tests: +4 (base64PayloadTooLarge, unknown-mode passthrough). 32/32 pass; tsc clean.
Harden image-host to production bar with a second test tier and the ctx.props follow-up the review deferred. - Zero-config URLs: the fetch handler injects the connected origin via ctx.props so upload_image returns an absolute, embeddable URL even when PUBLIC_BASE_URL is unset. Explicit PUBLIC_BASE_URL still wins (override for custom domains); fail-loud remains only if neither resolves. - Integration tier: @cloudflare/vitest-pool-workers runs the real Worker in workerd with real R2 + Durable Object bindings. Drives the full MCP streamable-HTTP handshake over SELF.fetch and asserts the wire format, the origin-derived URL, R2 round-trip + serving, nosniff, the /i/%ZZ 404 guard, and the bearer auth gate (8 tests). Reads only the first SSE result event so tool calls return in ms instead of waiting on the idle stream close. - Test layout split into tiers: test/unit (bun test, offline, 32 tests) and test/integration (vitest + workerd). bunfig scopes bare `bun test` to unit; wrangler config drives the pool; ajv pre-bundled via deps.optimizer so its JSON require resolves under workerd. - CI (servers_ts.yaml) now runs unit + typecheck + integration. All green: 32 unit, 8 integration, tsc clean.
…entry) Promote shared/catalog from placeholder to a real contract: each server declares one servers/<id>/catalog-entry.json (connector spec) plus its co-located icon; shared/catalog/aggregate.py validates every entry against shared/catalog/schema.json and builds the combined dist/catalog.json. - servers/image-host/catalog-entry.json + image-host.svg: the worked example. - make catalog_check (wired into make ci) fails on any invalid entry. edison-watch's scripts/sync_fleet_connectors.py mirrors these source entries one-way into the marketplace catalog. See docs/mcp_commodity_fleet_strategy.md §7 and edison-watch first_party_mcp_integration.md §(1)-(2). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
Replace the edison-jwt 501 stub with stateless verification: fetch Edison's JWKS (cached, kid-aware refetch), verify the RS256 signature via Workers WebCrypto, and enforce iss/aud/exp before admitting a call — subject = the token's `sub` for per-user attribution. RS256 only (pins alg; no downgrade). - src/jwt.ts: verifyJwtWithJwks (pure, unit-tested) + verifyEdisonJwt (JWKS fetch/cache). checkAuth is now async and fails closed on incomplete config. - test/unit/jwt.test.ts: real RS256 keypair, covers valid/expired/wrong-iss/ wrong-aud/bad-sig/wrong-alg/kid-miss/malformed. 43 unit tests green. AUTH_MODE stays bearer; the flip to edison-jwt is a coordinated deploy once Edison's issuer is live (wrangler.jsonc documents the three config vars). Contract mirrors edison-watch src/mcp_jwt.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
Add "edison-jwt" to the catalog auth enum so a first-party server can declare that Edison mints + injects a per-user JWT (no user-supplied token). aggregate.py enforces the invariant that edison-jwt implies edison_hosted: true. No entry uses it yet — image-host stays on "token" (bearer) until the coordinated flip. This just makes the flip pure data. Verify + inject sides: image-host/src/jwt.ts and edison-watch src/mcp_jwt*.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
Thermo-nuclear review hardening on the verify side: - Rate-limit JWKS refetches (JWKS_MIN_REFETCH_MS): an attacker-chosen `kid` was fully unauthenticated and forced one upstream /.well-known/jwks.json GET per crafted request. Now, past the cooldown we serve the cached set (a kid miss then 401s) instead of hammering Edison. - Require `kid` and match it exactly (Edison always sets it), dropping the "try every RSA key" fallback for kid-less tokens. Adds tests for the missing-kid rejection and the verifyEdisonJwt fetch/cooldown path (previously uncovered). 45 unit tests green, tsc clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
PR Summary by QodoPolyglot MCP fleet scaffold with image-host Worker, catalog contract, and Edison JWT verify
AI Description
Diagram
High-Level Assessment
Files changed (32)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
31 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+160
to
+161
| if __name__ == "__main__": | ||
| sys.exit(main()) |
There was a problem hiding this comment.
1. No python version guard 📘 Rule violation ≡ Correctness
shared/catalog/aggregate.py is an executable entry point but does not enforce the repo’s configured minimum Python version via a sys.version_info guard. Running it under an older interpreter could proceed (or fail later) instead of exiting immediately with a clear error.
Agent Prompt
## Issue description
`shared/catalog/aggregate.py` is a runtime entry point and should enforce the project’s minimum Python version (`>=3.12`) with an early `sys.version_info` guard that exits non-zero with a clear message.
## Issue Context
The project declares `requires-python = ">= 3.12"` in `pyproject.toml`, and the compliance rule requires runtime enforcement in entry points.
## Fix Focus Areas
- shared/catalog/aggregate.py[30-50]
- shared/catalog/aggregate.py[160-161]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
|
||
| // Streamable-HTTP replies are SSE (`data: {json}\n\n`). Pull the first JSON-RPC | ||
| // message carrying a result/error out of the buffered stream so far. | ||
| function extractMessage(buffer: string): Record<string, any> | null { |
There was a problem hiding this comment.
3. any and as casts 📜 Skill insight ≡ Correctness
New TypeScript code introduces any and several unchecked casts (including double casts) that weaken type safety at key parsing/mocking boundaries. These should be replaced with explicit types and/or runtime validation (e.g., Zod parsing) to avoid masking shape errors.
Agent Prompt
## Issue description
New TS code uses `any` and unchecked type assertions (`as T`, `as unknown as ...`), which can hide real shape mismatches coming from `JSON.parse()` / `res.json()` and from test stubs.
## Issue Context
The compliance rule requires avoiding `any`/`unknown`/casts when explicit types or clearer boundaries can be defined.
## Fix Focus Areas
- servers/image-host/test/integration/worker.spec.ts[21-33]
- servers/image-host/src/jwt.ts[77-79]
- servers/image-host/test/unit/jwt.test.ts[134-140]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
18 issues found across 32 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="shared/auth/README.md">
<violation number="1" location="shared/auth/README.md:26">
P3: This status line is now stale and inaccurate: it claims edison-jwt is "stubbed to fail loudly with 501," but the verify layer in servers/image-host/src/jwt.ts (RS256 + JWKS + iss/aud/exp/kid) is implemented and wired into the edison-jwt case in auth.ts, per the PR itself. Update the status to reflect that edison-jwt verify is implemented for image-host, and keep the note about TS/Python ports being promoted later.</violation>
</file>
<file name="servers/image-host/README.md">
<violation number="1" location="servers/image-host/README.md:49">
P3: The authentication documentation describes `edison-jwt` as an unimplemented stub returning 501, but this checkout contains the implemented verification path: `checkAuth` calls `verifyEdisonJwt`, which returns authentication/configuration statuses such as 401, 500, and 503. This stale description can cause operators to avoid or incorrectly diagnose the supported mode; the `.dev.vars.example` comment also repeats that it is not implemented. Updating the docs to describe JWKS-backed verification would align the deployment contract with the code.</violation>
<violation number="2" location="servers/image-host/README.md:103">
P3: The deployment checklist incorrectly says uploads fail until `PUBLIC_BASE_URL` is configured. The implementation intentionally falls back to the connected request origin when that variable is empty, and the integration test asserts this origin-derived URL behavior. This contradiction makes the documented two-deploy requirement and the `wrangler.jsonc` “fails loudly” comments misleading; the checklist should say the variable is optional when the request origin is the public URL.</violation>
<violation number="3" location="servers/image-host/README.md:119">
P3: The Layout block shows tests flat under test/ (images.test.ts, auth.test.ts), but they actually live under test/unit/ (plus test/integration/ and a jwt.test.ts). This contradicts the Develop section above it and is stale. Update the tree to match the real layout.</violation>
</file>
<file name="servers/image-host/vitest.config.ts">
<violation number="1" location="servers/image-host/vitest.config.ts:33">
P2: The integration tier runs under compatibilityDate 2025-04-17, but production (wrangler.jsonc) uses 2026-01-01 with nodejs_compat. Test runtime and prod runtime have diverged, so the workerd tier can't catch regressions introduced by compat/flag behavior after 2025-04-17. The comment frames this as a pool constraint — consider bumping @cloudflare/vitest-pool-workers to a version whose bundled workerd supports a date matching production, and note any features that rely on newer nodejs_compat semantics. If the pin must stay, add a CI guard so the mismatch is visible rather than silent.</violation>
</file>
<file name="servers/image-host/tsconfig.json">
<violation number="1" location="servers/image-host/tsconfig.json:7">
P2: Including "bun" in the global types for the same compilation that type-checks production `src/` lets Node/Bun-only globals (process, Bun.*, etc.) type-check cleanly inside code that actually runs in workerd, and lets the overlapping Request/Response/crypto/WebSocket globals from @types/bun shadow the Cloudflare workers-types ones. That can mask a real runtime failure at deploy time, since the deployed target is workerd, not Bun/Node. The `bun` types are only needed for the `bun:test` unit tests, so consider giving the unit tests their own tsconfig (or an `extends` split that keeps `src/` on workers-types alone) rather than polluting the worker source compilation.</violation>
</file>
<file name="servers/image-host/package.json">
<violation number="1" location="servers/image-host/package.json:24">
P3: @vitest/runner and @vitest/snapshot are already transitive dependencies of vitest (vitest → @vitest/runner, @vitest/snapshot). Listing them as direct devDependencies is redundant and risks accidental version drift if they fall out of sync with the vitest install. Recommend removing them and letting vitest pin its own internals.</violation>
</file>
<file name="docs/mcp_commodity_fleet_strategy.md">
<violation number="1" location="docs/mcp_commodity_fleet_strategy.md:3">
P2: The Status header says "Draft / decisions locked, implementation not started", but this PR actually delivers the Wave 1 image-host server, the catalog contract (schema.json + aggregate.py + catalog-entry.json), and the edison-jwt verify layer. That reads as stale and will mislead a reader into thinking this strategy is purely aspirational. Consider updating the status to reflect that image-host + the catalog contract + edison-jwt verify have landed (deploy/rollout still pending) and that only the issuer + catalog sync remain open.</violation>
<violation number="2" location="docs/mcp_commodity_fleet_strategy.md:190">
P3: The JWT-signing "open question" is already answered by the delivered code: jwt.ts verifies RS256 via JWKS and deliberately rejects a shared-secret path, and that behavior is tested. Listing "shared HS256 secret vs RS256/EdDSA + JWKS" as an open question (and the §6 diagram's "(or shared HS256 secret)" fallback) is stale and contradicts the implementation. Recommend closing this question in §10 and dropping the HS256 mention in §6.</violation>
</file>
<file name="servers/image-host/src/auth.ts">
<violation number="1" location="servers/image-host/src/auth.ts:118">
P2: JWKS/network or key-parsing failures can reject `checkAuth` instead of returning an auth result, turning an Edison-authenticated request into an unstructured Worker error. Converting verifier exceptions to a fail-closed 503 would preserve the auth gate and give callers a stable response during issuer outages.</violation>
<violation number="2" location="servers/image-host/src/auth.ts:118">
P3: The implementation and published auth contract now disagree: selecting `edison-jwt` can authenticate requests, while both auth READMEs promise 501 until the issuer exists. Either retain the 501 stub or update the rollout/status documentation in this change so operators do not get contradictory deployment guidance.</violation>
</file>
<file name="servers/image-host/src/index.ts">
<violation number="1" location="servers/image-host/src/index.ts:60">
P3: Returned upload URLs can be malformed when PUBLIC_BASE_URL is set to a non-absolute value because the base is not validated. Validating http/https absolute URLs here would preserve the tool’s absolute URL guarantee and surface misconfiguration early.</violation>
<violation number="2" location="servers/image-host/src/index.ts:107">
P2: Upload limit configuration can be silently ignored, so operators may believe a stricter cap is active when uploads still allow the 10 MiB default. Consider failing closed when MAX_UPLOAD_BYTES is present but not a positive integer instead of defaulting.</violation>
</file>
<file name=".github/workflows/servers_ts.yaml">
<violation number="1" location=".github/workflows/servers_ts.yaml:41">
P2: This CI job can silently continue with a freshly resolved dependency tree when the lockfile is stale or incompatible, so a green result may not be reproducible locally or on deployment. Removing the fallback and failing on lockfile drift would make the committed `servers/image-host/bun.lock` an effective CI contract.</violation>
</file>
<file name="shared/README.md">
<violation number="1" location="shared/README.md:9">
P3: The status table marks [`catalog/`](./catalog) as "placeholder", but this PR ships the catalog contract: `shared/catalog/schema.json`, the `aggregate.py` validator, and a worked `servers/image-host/catalog-entry.json` example. The row should read implemented/active, not placeholder, so a reader of this README isn't misled about delivered working code.</violation>
<violation number="2" location="shared/README.md:14">
P3: The closing paragraph says the `catalog` generator "get[s] promoted here out of `servers/image-host/`" once a second server lands, but the catalog generator already lives in `shared/catalog/aggregate.py` in this PR, not in `servers/image-host/`. That sentence is only accurate for the auth verify layer today; as written it conflates the two and misstates where the catalog generator currently resides.</violation>
</file>
<file name="servers/image-host/test/integration/worker.spec.ts">
<violation number="1" location="servers/image-host/test/integration/worker.spec.ts:69">
P2: The post-initialize MCP requests omit the required `MCP-Protocol-Version` header. As a result, this integration client does not actually speak the negotiated 2025-06-18 Streamable HTTP protocol and can fail or silently rely on compatibility fallback with a stricter server. Reading the initialize response and adding the negotiated version to both `notifications/initialized` and `tools/call` requests would make the handshake test conformant.</violation>
<violation number="2" location="servers/image-host/test/integration/worker.spec.ts:83">
P3: callTool silently converts a JSON-RPC `error` message into `{}`, so if the server ever rejects a tool call via the protocol-level error channel (instead of a result-level `isError`), the helper returns an empty object and assertions like `result.isError`/`result.structuredContent` fail with a confusing undefined. Consider surfacing `msg.error` (e.g. throw it) so failures are diagnosable.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| miniflare: { | ||
| // The pool's bundled workerd supports compat dates up to 2025-04-17; | ||
| // pin the test runtime there (production uses wrangler.jsonc's date). | ||
| compatibilityDate: "2025-04-17", |
There was a problem hiding this comment.
P2: The integration tier runs under compatibilityDate 2025-04-17, but production (wrangler.jsonc) uses 2026-01-01 with nodejs_compat. Test runtime and prod runtime have diverged, so the workerd tier can't catch regressions introduced by compat/flag behavior after 2025-04-17. The comment frames this as a pool constraint — consider bumping @cloudflare/vitest-pool-workers to a version whose bundled workerd supports a date matching production, and note any features that rely on newer nodejs_compat semantics. If the pin must stay, add a CI guard so the mismatch is visible rather than silent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At servers/image-host/vitest.config.ts, line 33:
<comment>The integration tier runs under compatibilityDate 2025-04-17, but production (wrangler.jsonc) uses 2026-01-01 with nodejs_compat. Test runtime and prod runtime have diverged, so the workerd tier can't catch regressions introduced by compat/flag behavior after 2025-04-17. The comment frames this as a pool constraint — consider bumping @cloudflare/vitest-pool-workers to a version whose bundled workerd supports a date matching production, and note any features that rely on newer nodejs_compat semantics. If the pin must stay, add a CI guard so the mismatch is visible rather than silent.</comment>
<file context>
@@ -0,0 +1,42 @@
+ miniflare: {
+ // The pool's bundled workerd supports compat dates up to 2025-04-17;
+ // pin the test runtime there (production uses wrangler.jsonc's date).
+ compatibilityDate: "2025-04-17",
+ // wrangler.jsonc leaves PUBLIC_BASE_URL empty on purpose so the
+ // integration tests exercise the request-origin fallback; provide the
</file context>
| "module": "es2022", | ||
| "moduleResolution": "bundler", | ||
| "lib": ["es2022"], | ||
| "types": ["@cloudflare/workers-types", "bun"], |
There was a problem hiding this comment.
P2: Including "bun" in the global types for the same compilation that type-checks production src/ lets Node/Bun-only globals (process, Bun.*, etc.) type-check cleanly inside code that actually runs in workerd, and lets the overlapping Request/Response/crypto/WebSocket globals from @types/bun shadow the Cloudflare workers-types ones. That can mask a real runtime failure at deploy time, since the deployed target is workerd, not Bun/Node. The bun types are only needed for the bun:test unit tests, so consider giving the unit tests their own tsconfig (or an extends split that keeps src/ on workers-types alone) rather than polluting the worker source compilation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At servers/image-host/tsconfig.json, line 7:
<comment>Including "bun" in the global types for the same compilation that type-checks production `src/` lets Node/Bun-only globals (process, Bun.*, etc.) type-check cleanly inside code that actually runs in workerd, and lets the overlapping Request/Response/crypto/WebSocket globals from @types/bun shadow the Cloudflare workers-types ones. That can mask a real runtime failure at deploy time, since the deployed target is workerd, not Bun/Node. The `bun` types are only needed for the `bun:test` unit tests, so consider giving the unit tests their own tsconfig (or an `extends` split that keeps `src/` on workers-types alone) rather than polluting the worker source compilation.</comment>
<file context>
@@ -0,0 +1,18 @@
+ "module": "es2022",
+ "moduleResolution": "bundler",
+ "lib": ["es2022"],
+ "types": ["@cloudflare/workers-types", "bun"],
+ "strict": true,
+ "skipLibCheck": true,
</file context>
| const { value, done } = await reader.read(); | ||
| if (value) buffer += decoder.decode(value, { stream: true }); | ||
| const msg = extractMessage(buffer); | ||
| if (msg) return (msg.result ?? {}) as Record<string, any>; |
There was a problem hiding this comment.
P3: callTool silently converts a JSON-RPC error message into {}, so if the server ever rejects a tool call via the protocol-level error channel (instead of a result-level isError), the helper returns an empty object and assertions like result.isError/result.structuredContent fail with a confusing undefined. Consider surfacing msg.error (e.g. throw it) so failures are diagnosable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At servers/image-host/test/integration/worker.spec.ts, line 83:
<comment>callTool silently converts a JSON-RPC `error` message into `{}`, so if the server ever rejects a tool call via the protocol-level error channel (instead of a result-level `isError`), the helper returns an empty object and assertions like `result.isError`/`result.structuredContent` fail with a confusing undefined. Consider surfacing `msg.error` (e.g. throw it) so failures are diagnosable.</comment>
<file context>
@@ -0,0 +1,168 @@
+ const { value, done } = await reader.read();
+ if (value) buffer += decoder.decode(value, { stream: true });
+ const msg = extractMessage(buffer);
+ if (msg) return (msg.result ?? {}) as Record<string, any>;
+ if (done) break;
+ }
</file context>
| } | ||
| const presented = extractBearer(request.headers.get("authorization")); | ||
| if (!presented) return { ok: false, status: 401, message: "missing bearer token" }; | ||
| const result = await verifyEdisonJwt(presented, { |
There was a problem hiding this comment.
P3: The implementation and published auth contract now disagree: selecting edison-jwt can authenticate requests, while both auth READMEs promise 501 until the issuer exists. Either retain the 501 stub or update the rollout/status documentation in this change so operators do not get contradictory deployment guidance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At servers/image-host/src/auth.ts, line 118:
<comment>The implementation and published auth contract now disagree: selecting `edison-jwt` can authenticate requests, while both auth READMEs promise 501 until the issuer exists. Either retain the 501 stub or update the rollout/status documentation in this change so operators do not get contradictory deployment guidance.</comment>
<file context>
@@ -0,0 +1,129 @@
+ }
+ const presented = extractBearer(request.headers.get("authorization"));
+ if (!presented) return { ok: false, status: 401, message: "missing bearer token" };
+ const result = await verifyEdisonJwt(presented, {
+ jwksUrl: EDISON_JWKS_URL,
+ issuer: EDISON_JWT_ISSUER,
</file context>
| /** Absolute base for returned URLs, e.g. `https://image-host.acme.workers.dev`. */ | ||
| function resolvePublicBase(env: Env): string | null { | ||
| const raw = env.PUBLIC_BASE_URL?.trim(); | ||
| return raw ? raw.replace(/\/+$/, "") : null; |
There was a problem hiding this comment.
P3: Returned upload URLs can be malformed when PUBLIC_BASE_URL is set to a non-absolute value because the base is not validated. Validating http/https absolute URLs here would preserve the tool’s absolute URL guarantee and surface misconfiguration early.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At servers/image-host/src/index.ts, line 60:
<comment>Returned upload URLs can be malformed when PUBLIC_BASE_URL is set to a non-absolute value because the base is not validated. Validating http/https absolute URLs here would preserve the tool’s absolute URL guarantee and surface misconfiguration early.</comment>
<file context>
@@ -0,0 +1,215 @@
+/** Absolute base for returned URLs, e.g. `https://image-host.acme.workers.dev`. */
+function resolvePublicBase(env: Env): string | null {
+ const raw = env.PUBLIC_BASE_URL?.trim();
+ return raw ? raw.replace(/\/+$/, "") : null;
+}
+
</file context>
| ```bash | ||
| wrangler deploy | ||
| ``` | ||
| 4. **Set `PUBLIC_BASE_URL`** to the deployed worker URL (or a custom domain) in |
There was a problem hiding this comment.
P3: The deployment checklist incorrectly says uploads fail until PUBLIC_BASE_URL is configured. The implementation intentionally falls back to the connected request origin when that variable is empty, and the integration test asserts this origin-derived URL behavior. This contradiction makes the documented two-deploy requirement and the wrangler.jsonc “fails loudly” comments misleading; the checklist should say the variable is optional when the request origin is the public URL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At servers/image-host/README.md, line 103:
<comment>The deployment checklist incorrectly says uploads fail until `PUBLIC_BASE_URL` is configured. The implementation intentionally falls back to the connected request origin when that variable is empty, and the integration test asserts this origin-derived URL behavior. This contradiction makes the documented two-deploy requirement and the `wrangler.jsonc` “fails loudly” comments misleading; the checklist should say the variable is optional when the request origin is the public URL.</comment>
<file context>
@@ -0,0 +1,122 @@
+ ```bash
+ wrangler deploy
+ ```
+4. **Set `PUBLIC_BASE_URL`** to the deployed worker URL (or a custom domain) in
+ `wrangler.jsonc` `vars`, then `wrangler deploy` again so returned URLs are
+ absolute. **Required:** until it's set, `upload_image` fails loudly rather
</file context>
| Pluggable via [`src/auth.ts`](./src/auth.ts): `open` | `bearer` | `edison-jwt`. | ||
| v1 ships **`bearer`** — set `AUTH_TOKEN` and clients send | ||
| `Authorization: Bearer <token>`. `edison-jwt` (Edison mints a per-user JWT and | ||
| injects it, no consent screen) is stubbed as an explicit drop-in and returns |
There was a problem hiding this comment.
P3: The authentication documentation describes edison-jwt as an unimplemented stub returning 501, but this checkout contains the implemented verification path: checkAuth calls verifyEdisonJwt, which returns authentication/configuration statuses such as 401, 500, and 503. This stale description can cause operators to avoid or incorrectly diagnose the supported mode; the .dev.vars.example comment also repeats that it is not implemented. Updating the docs to describe JWKS-backed verification would align the deployment contract with the code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At servers/image-host/README.md, line 49:
<comment>The authentication documentation describes `edison-jwt` as an unimplemented stub returning 501, but this checkout contains the implemented verification path: `checkAuth` calls `verifyEdisonJwt`, which returns authentication/configuration statuses such as 401, 500, and 503. This stale description can cause operators to avoid or incorrectly diagnose the supported mode; the `.dev.vars.example` comment also repeats that it is not implemented. Updating the docs to describe JWKS-backed verification would align the deployment contract with the code.</comment>
<file context>
@@ -0,0 +1,122 @@
+Pluggable via [`src/auth.ts`](./src/auth.ts): `open` | `bearer` | `edison-jwt`.
+v1 ships **`bearer`** — set `AUTH_TOKEN` and clients send
+`Authorization: Bearer <token>`. `edison-jwt` (Edison mints a per-user JWT and
+injects it, no consent screen) is stubbed as an explicit drop-in and returns
+`501` until the Edison issuer exists. With no token configured the server
+defaults to `open` (self-host friendly). See
</file context>
Address reviewer findings on PR #12 and clear the em-dash lint gate. Security / correctness (image-host): - jwt.ts: wrap WebCrypto importKey/verify in try/catch (malformed JWK -> 401, never an uncaught 500); require n/e on candidate keys; fetchJwks catches network/parse failures -> null (503, not 500); ensureJwks only falls back to a same-url cached JWKS and advances the refetch cooldown after a *successful* fetch so a blip can retry next request. - auth.ts: a defined-but-empty AUTH_TOKEN no longer collapses to `open` - it stays `bearer` and fails closed (500 misconfig); bearer guard trims so a whitespace-only token is also a misconfig. Refresh the stale "edison-jwt is stubbed" header comment (it is implemented). - images.ts: match the FULL 8-byte PNG and 6-byte GIF signatures (not just the ascii prefix); reject a declared content_type we can't normalize instead of silently serving under the sniffed type. Catalog contract: - aggregate.py: reject a non-object top-level entry, an empty url host (`https:///mcp`), and an icon with path separators or one that isn't `<id>.svg`. - schema.json: add an allOf so auth `edison-jwt` requires `edison_hosted: true`, mirroring aggregate.py. Tests + docs: - new unit coverage for the empty-token, full-signature, and bad-declared-type paths (49 unit tests pass). - refresh .dev.vars.example and shared/catalog/README to reflect edison-jwt being implemented end to end. - replace U+2014 em dashes with hyphens across tracked markdown/source to pass the repo's lint-em-dash CI gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
There was a problem hiding this comment.
All reported issues were addressed across 21 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Second round of PR #12 review fixes. jwt.ts (cubic P2): coalesce a concurrent cold/stale-cache JWKS burst into a single upstream GET via a URL-keyed in-flight promise. The refetch cooldown only throttles sequential requests, so without dedup every request in a cold-isolate burst opened its own fetch; a failed fetch still doesn't advance the cooldown, so the next request retries immediately. Adds concurrency + failed-retry tests. images.ts (cubic P3): treat an explicitly-supplied empty content_type ("") as a declaration (`!== undefined`), rejecting it like any other unsupported value instead of silently serving under the sniffed type. aggregate.py (qodo): bring the stdlib validator into real parity with schema.json - reject unknown fields (additionalProperties:false), enforce the id regex, require tags to be non-empty strings, and validate the headers / template_fields.env object shapes. The doc comment's "validates against schema.json" claim now holds. package.json (qodo): drop the redundant `version` field - the package is private and versioning is single-sourced in pyproject.toml. 52 unit tests pass; catalog --check still validates the image-host entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Python's `$` also matches just before a final newline, so `ID_RE.match` would accept an id like 'image-host\n' that schema.json (and downstream consumers) reject. `fullmatch` anchors the whole string, keeping the stdlib validator in lockstep with the schema. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors this repo in place into a polyglot MCP fleet monorepo and ships the first Edison-hosted server (image-host), the Edison catalog contract (how a fleet server advertises itself to the marketplace), and the
edison-jwtverify half of the per-user JWT auth model. Pairs withedison-watch/edison-watch#<first-party MCP integration PR>(issuer + catalog sync +edison_hostedbadge).Changes
Fleet scaffold
servers/+shared/{auth,catalog,ui}added additively around the existing Python app; strategy docdocs/mcp_commodity_fleet_strategy.md; README reframed around the commodity-fleet direction.image-host (Cloudflare Worker + R2) —
servers/image-host/upload_image/delete_imagetools: base64 in, public non-expiring URL out (R2 via binding, no S3 keys); zero-config origin-derived URLs whenPUBLIC_BASE_URLis unset; content-type allowlist + size cap + unguessable keys; public/i/<key>read path, guarded/mcpwrite path.Edison catalog contract —
shared/catalog/schema.json(entry shape) +aggregate.pyvalidator (stdlib, no deps);make catalog_checkwired intomake ci.servers/image-host/catalog-entry.json+ co-located icon: the worked example. edison-watch's sync mirrors these source entries one-way into the marketplace catalog.Auth: pluggable
open|bearer|edison-jwt—servers/image-host/src/edison-jwtverify implemented: fetch Edison's JWKS (cached, kid-aware refetch with a rate-limit cooldown), verify RS256 via Workers WebCrypto, enforceiss/aud/exp, requirekid;checkAuthis now async and fails closed on incomplete config.authenum gainsedison-jwt(requiresedison_hosted: true).Testing
bun test test/unit(incl. RS256 mint↔verify, expiry/aud/iss/kid, JWKS fetch + cooldown),tsc --noEmitclean.make catalog_checkgreen;ruffclean onshared/catalog/.make ci/make testrun in CI (sandbox lacks a workinguv).Rollout note
Not live on flip. image-host stays on
auth: "token"(bearer) and the current dev Worker URL. Switching toedison-jwtis a coordinated deploy (Edison issuer key + WorkerEDISON_JWKS_URL/ISSUER/AUDIENCE+ catalogauthflip), done per-environment after the issuer is live. For release, image-host also moves to an Edison-owned Cloudflare account/domain.Related Issues
Closes #
🤖 Generated with Claude Code
https://claude.ai/code/session_01GVK1ngvGgmfoGYxrjtAF5g
Generated by Claude Code
Summary by cubic
Refactors this repo into a polyglot MCP fleet and ships the first TypeScript server,
image-host, plus the Edison catalog contract andedison-jwtverification. Adds shared auth/catalog scaffolding, CI for TS servers, and hardens auth/jwt/catalog and image validation.New Features
servers/andshared/{auth,catalog,ui}added around the existing Python app; TS server CI; strategy doc.image-host(Cloudflare Worker + R2):upload_image/delete_image; base64 in → public non-expiring URL;/i/<key>serves images; origin-derived URLs whenPUBLIC_BASE_URLis unset; size/type checks; unguessable keys.open|bearer|edison-jwt;edison-jwtverify via JWKS (RS256) with cache, cooldown, and in‑flight fetch dedup; enforceiss/aud/exp; requirekid; fails closed on bad/missing config.shared/catalog/schema.json+aggregate.py;make catalog_checkin CI;servers/image-host/catalog-entry.json+ icon; catalog enum includesedison-jwt(requiresedison_hosted: true). Tests/CI: offline unit and workerd integration tiers; path‑scoped Actions; 52 unit tests; typecheck clean.Bug Fixes
n/e; JWKS fetch/parse failures return 503;kidrequired and must match; refetch cooldown advances only on successful fetch; coalesce concurrent JWKS fetches per URL.AUTH_TOKENnow fails closed; PNG/GIF signature checks hardened; reject unsupported declaredcontent_typevalues, including an explicit empty string.idregex (uses fullmatch to reject trailing-newline ids), non-empty string tags, valid icon name<id>.svg, non-empty URL hosts, and correctheaders/template_fields.envshapes.Written for commit d2b552f. Summary will update on new commits.