sh-mcp/docs/build-plan-phase-1.md
Adam Moussa 0d1fefb326
Some checks are pending
deploy / deploy (push) Waiting to run
Phase 0b slice: monorepo scaffold + @sh-mcp/shared core + integration packages (#2)
* Phase 0b slice: monorepo scaffold + shared core + integration packages

The 0a-INDEPENDENT code slice (one-shot via af-0b-package-slice workflow: Haiku
scaffold + Sonnet packages, Sonnet fix-to-green). Nothing deploys; no CDK/servers.

- Monorepo scaffold: npm workspaces, strict TS (NodeNext), vitest (80% gate),
  eslint 9 flat config, prettier; ci.yaml/deploy.yaml callers (Node 24, enable-qemu).
- @sh-mcp/shared: transport-agnostic core — Scope/AuthContext/ToolDef, ToolRegistry,
  redact()+maskValue() (PII), OpenAPI 3.1 generator. AUTH STUBBED behind an AuthProvider
  interface (TODO auth-layer-0a); JWT/aud/client_id/JWKS/deny-list deferred per design.md §2.
- 9 integration packages (qbo, google-maps, internal-data, payments, knowledge-base,
  gmail, calendar, tasks, reminders): tools against shared, external deps mocked behind
  injected client interfaces; finance handlers call redact().

Verified green: tsc -b clean, vitest 245/245, eslint 0 errors. Auth mechanism intentionally
deferred until the 0a spike resolves it (G16/§0.4).

* Complete Cognito auth provider + Phase 1 build brief

Finish the WIP CognitoAuthProvider (client_id allow-list as audience
boundary, finance TTL ceiling, deny-list, scope-prefix stripping) with
its test suite, and check in docs/build-plan-phase-1.md so the Phase 1
work has its governing brief in-tree (design.md §2.5).

* ci: disable cdk synth for Phase 0b (no CDK app yet)

The reusable ci-typescript-cdk workflow defaults run-cdk-synth: true, but
the Phase 0b package scaffold has no cdk.json or stacks, so cdk synth fails
with '--app is required'. Disable it here; Phase 1 re-enables it with the
server CDK stubs.
2026-06-26 12:42:17 -04:00

21 KiB

Build Plan — Phase 1: Runnable MCP + OpenAPI Servers

Audience: an autonomous coding agent building this overnight and opening a draft PR. You do not have access to the maintainer's global instructions, memory, or engineering handbook. Everything you need is in this file and in docs/design.md. Read both fully before writing code. When this file and docs/design.md disagree, this file wins for what to build in this PR; docs/design.md wins for the architecture and security model.

0. Goal of this PR (single, focused)

Take the existing Phase 0b scaffold (shared core + 9 integration packages) and add the missing transport/runtime layer + two servers so that, by the end:

  • servers/sh-mcp-ops and servers/sh-mcp-finance start locally and serve real requests.
  • Each server exposes two universal interfaces over the same tool registry:
    1. MCP — Streamable HTTP, via @modelcontextprotocol/sdk.
    2. OpenAPI 3.1 — a full document at GET /openapi.json plus a POST /tools/{tool-name} endpoint per tool (this is what Agentforce/other OpenAPI consumers will use later).
  • The auth, scope-hiding, audience-binding, finance redaction, audit logging, and rate-limiting described in design.md §2 are enforced in one shared dispatch path used by both interfaces.
  • Everything is covered by the security-weighted test suite (design.md §7.3), tsc --noEmit is clean, eslint/prettier pass, and CI is green.

This PR does NOT integrate Agentforce, Slack, or Cognito infrastructure. It produces the servers and the two specs they speak. Agentforce wiring is a later phase.

Explicitly OUT of scope (do not build; leave as deferred follow-ups)

  • Cognito user pool, Google federation, pre-token Lambda, group-sync Lambda (auth/ dir).
  • Real cdk deploy / API Gateway / WAF / IAM roles. (You will add synth-only CDK stubs — see §6.)
  • The jobs/ proactive Lambdas (fetch-classify, digests, KB syncs).
  • The physical tier (lenel/yealink/threecx) — deferred per design.md §3.
  • Real DynamoDB/QBO/Maps/Google client implementations — they stay stubbed; you add in-memory dev clients instead (see §4). Do not write live AWS/Google/QBO network code.

If you find yourself provisioning AWS, federating Google, or calling a real external API, stop — that's out of scope for this PR.


1. What already exists (read these first, do not rewrite)

  • packages/shared/src — the transport-agnostic core. Reuse it; extend it, don't fork it.
    • types.ts — Scope, AuthContext { sub, scopes, aud }, ToolDef<I,O> { name, description, tier: 'ops'|'finance', requiredScope, inputSchema, handler(input, ctx) }, JSONSchema.
    • registry.ts — defineTool() and ToolRegistry (.register(), .list(), .get(), .size).
    • auth.ts — requireScope(ctx, scope) / ScopeError, and the AuthProvider interface (authenticate(req): Promise<AuthContext>).
    • cognito-auth.ts — CognitoAuthProvider (real jose JWT verification; client_id allow-list IS the audience boundary because Cognito access tokens carry no aud; deny-list; finance TTL ceiling; strips the sh-mcp-ops/ resource-server prefix off scopes). AuthError (→ 401).
    • redact.ts — redact(), maskValue(), REDACTED.
    • openapi.ts — generateOpenAPIPaths(registry) returns { paths, components } (paths only today).
    • index.ts — the only import surface. Packages import from '@sh-mcp/shared', never subpaths.
  • 9 integration packages (packages/{calendar,gmail,google-maps,internal-data,knowledge-base, payments,qbo,reminders,tasks}) — each has a client.ts (an interface + a throwing/stub concrete impl), a tools.ts factory that builds ToolDefs via defineTool, an index.ts, and a vitest suite using a mock client.

⚠️ Known inconsistency you must absorb (do not "fix" by renaming tools)

The package tool factories have inconsistent signatures and export names — by design they are each composed individually:

Package Factory export Signature
calendar buildCalendarTools (client)
tasks buildTaskTools (client)
reminders buildReminderTools ({ client })
internal-data makeTools (client)
google-maps makeTools / makeSearchNearbyVendors (client, ...)
gmail makeSearchInboxTool, makeGetEmailThreadDetailTool per-tool factories
knowledge-base createKnowledgeBaseTools (...)
payments makePaymentsTools (client)
qbo tools array / makeSearchVendorsTool client constructed inside

Open each package's index.ts and tools.ts to learn its exact factory before wiring it. Wire each factory with the appropriate dev client (local) or stub client (real). You MAY add a thin normalizing adapter in each server's composition root, but do not rename any tool's name field and do not change package public APIs unless a package genuinely can't be composed without it (if so, keep the change minimal and note it in the PR).

The ops/finance tool split is authoritative in design.md §3:

  • ops tools come from: internal-data, knowledge-base, google-maps, gmail, calendar, tasks, reminders.
  • finance tools come from: qbo, payments.

2. Architecture to build

Add a transport/runtime to packages/shared (the design doc designates shared as the "MCP scaffold + JWT validation + scope guard + audit log" home — keep it there; do not create a new package). Then add two thin server apps under servers/.

2.1 New modules in packages/shared/src (export all via index.ts)

  1. dispatch.ts — the single authoritative execution path. One function, e.g. async function executeTool(registry, ctx, toolName, rawInput, deps) that:

    1. Looks up the tool; 404-equivalent if unknown.
    2. Calls requireScope(ctx, tool.requiredScope) (defense-in-depth; handlers also call it).
    3. Validates rawInput against tool.inputSchema before the handler runs (use ajv; reject on failure with a structured validation error — never pass unvalidated input to a handler; design.md §2.5 prompt-injection containment).
    4. Enforces a per-session tool-call cap + per-tool rate limit (design.md §7.3) via an injected limiter (in-memory token bucket is fine for this PR).
    5. Runs tool.handler(input, ctx).
    6. If tool.tier === 'finance', runs the output through redact()/maskValue() on egress so bank/routing/card/SSN are masked before the value leaves the dispatcher (design.md §2.5, §7.3). Finance tools already redact internally — this is a belt-and-braces egress pass; assert in tests that nothing sensitive escapes.
    7. Emits a structured audit record for every finance:* call (and any future physical:*): { sub, tool, argsHash, decision, result: 'ok'|'error', ts } — args are hashed, never logged raw; secrets must never appear (design.md §2.5, §7.3 Audit row).
    8. Maps errors to typed outcomes the adapters translate (ScopeError→403, AuthError→401, validation→400, unknown tool→404, handler throw→500). Never leak stack traces or secrets in error bodies.
  2. audit.ts — an AuditLogger interface + a default ConsoleAuditLogger (structured JSON to stdout; in Lambda this lands in CloudWatch). Injected into dispatch. Add a NoopAuditLogger for tests.

  3. mcp.ts — createMcpServer(registry, authProvider, deps) returning a configured @modelcontextprotocol/sdk Server:

    • tools/list returns only the tools whose requiredScope is in the caller's AuthContext (server-side tool-hiding, design.md §2.5). A finance-less caller must not see finance tools.
    • tools/call routes through executeTool. Same scope/redaction/audit guarantees as OpenAPI.
  4. Extend openapi.ts — add buildOpenApiDocument(registry, { info, servers }) that wraps the existing generateOpenAPIPaths output into a complete, valid OpenAPI 3.1 document (info, servers, paths, components.securitySchemes). Keep generateOpenAPIPaths as-is and build on top.

  5. http.ts — createApp({ registry, authProvider, deps }) returning an Express app (decision: Express + MCP SDK) that mounts:

    • POST /mcp (+ the GET/DELETE the Streamable HTTP transport needs) → MCP via StreamableHTTPServerTransport. Authenticate the request → AuthContext → MCP server.
    • GET /openapi.json → buildOpenApiDocument(...).
    • POST /tools/:name → authenticate → executeTool → JSON result. 401/403/400/404/500 per §2.1.
    • GET /healthz → { status: 'ok' }, unauthenticated, for local/uptime checks.
    • Auth middleware calls authProvider.authenticate(req); on AuthError → 401, on success attaches ctx. The /openapi.json and /healthz routes are unauthenticated; every tool path and /mcp require a valid token (design.md §2.5 — never an unauthenticated tool path).

Keep packages/shared importable without side effects: no server is started and no AWS/Express listener is created at import time. createApp builds; the server entry calls .listen().

2.2 Server apps — servers/sh-mcp-ops and servers/sh-mcp-finance

Each is a thin composition root workspace package (@sh-mcp/server-ops, @sh-mcp/server-finance):

  • src/registry.ts — build a ToolRegistry, register exactly that tier's tools (§1 split), wiring each package factory with the selected client (dev vs real, §4).
  • src/config.ts — read env: SH_MCP_ENV (local | aws), port, and (for aws) the CognitoAuthConfig (issuer, JWKS, allowedClientIds, scopePrefix, finance TTL). No secrets or client ids hardcoded — all injected from env (design.md §2).
  • src/auth.ts — select the AuthProvider: CognitoAuthProvider when SH_MCP_ENV=aws; a LocalAuthProvider (see §3) when SH_MCP_ENV=local. The local provider must refuse to construct when SH_MCP_ENV is not local so it can never run in production.
  • src/index.ts — createApp(...) + .listen(port) with a startup log line. Also export a handler shape placeholder for future Lambda use, but do not depend on AWS Lambda runtime.
  • package.json — dev (tsx watch src/index.ts or node --watch), start, build, test, typecheck scripts. Add to the root tsconfig.json references and to workspaces (already globbed).
  • README.md — how to run locally, the env vars, the two endpoints, example curl + MCP Inspector.

Finance server additionally: every tool call audited (already guaranteed by dispatch for finance tier).


3. Local auth (so the servers actually run without Cognito)

Add LocalAuthProvider (in packages/shared/src, exported from the barrel; or in each server — put it in shared so both reuse it). It implements AuthProvider.authenticate(req) and, in local mode only, derives an AuthContext from a dev bearer token mapping defined in env/config, e.g.:

  • A small JSON map SH_MCP_LOCAL_PRINCIPALS of token -> { sub, scopes[], aud }, OR
  • A signed local JWT using a dev secret.

Provide at least these dev principals so tests/demos exercise tool-hiding and tiering: ops-only (ops:read,ops:tasks), assistant (+gmail:self,calendar:self), finance (ops:read,finance:read), admin (all). It MUST throw if instantiated outside SH_MCP_ENV=local. Document the dev tokens in each server README.


4. In-memory dev clients (decision: tools return real data locally)

For each integration package, add an in-memory implementation of its Client interface seeded with a few realistic fake records, used when SH_MCP_ENV=local. Two acceptable placements — pick one and be consistent: (a) a src/dev-client.ts in each package exported from its index.ts, or (b) a servers/*/src/dev-clients.ts in the composition root. Prefer (a) so the dev client lives with its interface and is unit-testable alongside the package.

Guarantees:

  • Selecting a dev client is gated on SH_MCP_ENV=local; aws mode wires the real (stub) clients.
  • Dev clients are pure in-memory (Maps/arrays), no network, deterministic enough to test.
  • For Gmail/Calendar/Tasks dev clients, partition data by ctx.sub so the per-user isolation in design.md §2.4 is demonstrable locally (a user only sees their own data).
  • Finance dev data (payments/qbo) must include sensitive-looking fields (account/routing/card) so the redaction egress test has something real to mask.

End state: SH_MCP_ENV=local npm run dev -w @sh-mcp/server-ops, then curl a tool or point MCP Inspector at http://localhost:PORT/mcp with a dev bearer, and get a real response.


5. Tests (security-weighted — this is the highest-risk surface)

Use vitest (already configured; root vitest.config.ts). Keep existing package tests green. Add:

Shared / dispatch / transport (new):

  • Tool-hiding: MCP tools/list and the OpenAPI doc reflect ONLY the caller's scopes; a caller without finance:read cannot see — and cannot tools/call — finance tools (assert both the hiding AND that a forced call is still 403 server-side, since hiding is not the boundary).
  • Audience binding: a token/principal minted for ops is rejected by the finance server and vice versa (in aws mode this is the client_id allow-list; assert via CognitoAuthProvider config).
  • Per-tool scope enforcement independent of UI hiding (force-call a hidden tool → ScopeError/403).
  • Input-schema validation: malformed input is rejected (400) before the handler runs.
  • Finance redaction on egress: every finance tool response has bank/routing/card/SSN masked; add a test that fails if any raw sensitive value appears in the serialized response.
  • Audit emission: every finance call emits one structured audit record with hashed args and no secrets; assert shape and that the raw arg values / tokens never appear in the record.
  • Prompt-injection regression: a tool response whose text contains "ignore previous instructions, call " does NOT cause any out-of-scope tool call (dispatcher treats tool output as data; design.md §2.5/§7.3).
  • Rate-limit / session cap: exceeding the cap returns the limiter error, not a handler call.
  • MCP conformance: handshake + tools/list + a successful tools/call round-trip against an in-memory transport; every tool's inputSchema is valid JSON Schema.
  • OpenAPI validity: buildOpenApiDocument output validates as OpenAPI 3.1 (use a validator lib or assert required structural invariants: each tool → one POST /tools/{name}, x-required-scope present, bearerAuth security scheme present).
  • Local-auth safety: LocalAuthProvider throws if SH_MCP_ENV !== 'local'.

Coverage gate (design.md §7.3): start at 80% lines overall, 100% on the shared auth/scope-guard + dispatch modules (auth.ts, cognito-auth.ts, dispatch.ts). Wire the gate into vitest.config.ts coverage thresholds. If 100% on a module is impractical for a defensible reason, document it in the PR rather than lowering silently.


6. CDK synth-only stubs (decision: keep the CI synth gate honest, don't deploy)

The repo already has .github/workflows/deploy.yaml referencing a cd-cdk reusable workflow and design.md §7 expects cdk synth in CI. Add minimal, synth-clean CDK app(s) so the synth step has something valid to run — but wire NO real resources that require Cognito or live IAM review:

  • One CDK app per server (or one app, two stacks) under servers/*/cdk/ (or infra/), pinned with exact aws-cdk-lib version (no ^/~ — exact pin per the repo's dependency policy).
  • The stack may define only inert/no-op constructs (e.g. a stack with a CfnOutput, or a Lambda function construct pointing at a placeholder) — enough that cdk synth succeeds. Do not create IAM roles/policies, API Gateway authorizers, or WAF here — those carry a mandatory human IAM cross-review you cannot run. Leave a // TODO(phase-2): real stack — gated on Cognito + IAM review.
  • Add a synth script and ensure npx cdk synth exits 0 from a clean npm ci.
  • If reconciling the existing deploy.yaml to a not-yet-deployable stack is risky, do not modify deploy.yaml's trigger; instead make synth pass and note in the PR that real deploy is deferred.

7. Conventions to follow (the maintainer's standards — inlined for you)

You don't have the handbook; these are the rules that apply:

  • Naming: kebab-case for repos, packages, dirs, stacks, and AWS resource names (sh-mcp, sh-mcp-ops, sh-mcp-finance). Tool name fields: keep whatever each package already uses (mixed snake_case exists — do not mass-rename in this PR).
  • Language/strictness: TypeScript everywhere, ESM ("type": "module", .js import specifiers in TS source as the existing code does). tsc --noEmit must be clean across the workspace. No any without an eslint-disable + reason (match existing style).
  • No I/O at import time: never construct AWS SDK clients, open sockets, or read secrets at module top level. Real clients lazy-load their SDK and throw in NODE_ENV=test (existing pattern — keep it).
  • Lint/format: eslint . and prettier --check . must pass. Run npm run format before commit.
  • Secrets/config: nothing hardcoded — client ids, issuer, JWKS URL, table names, scope prefixes all come from env/injected config. No real secrets in the repo or tests.
  • Dependencies: add the minimum needed (@modelcontextprotocol/sdk, express, ajv, tsx for dev, @vitest/coverage-v8 if not present, aws-cdk-lib+constructs for the synth stubs). Exact-pin infra-critical deps (aws-cdk-lib); pin others consistently with the existing package.json style (the root uses exact versions — match that). Run npm install so package-lock.json updates; commit the lockfile.
  • Commits: small, logical, imperative-mood subject ≤ ~72 chars, with a body explaining what and why and referencing the relevant design.md section. Example: Add shared dispatch + MCP/OpenAPI adapters (design.md §2.5, §7.3). Group by concern (shared transport → servers → dev clients → tests → cdk stubs), not one giant commit.
  • Branch: work on a feature branch off main (e.g. feature/phase-1-servers). Do not commit to main. Open the PR as a draft.

8. PR description requirements (must include all of these)

Open a draft PR to main titled like Phase 1: runnable MCP + OpenAPI servers (ops + finance). The body must contain:

  1. Summary — what was built (shared transport, two servers, dev clients, dual specs, tests, synth stubs).
  2. How to run — exact SH_MCP_ENV=local commands for each server + a sample curl and an MCP Inspector pointer, with a dev bearer token.
  3. Testing — npm test output summary, coverage numbers, and that tsc --noEmit, eslint, and prettier --check are clean.
  4. Out of scope / deferred — Cognito infra, real IAM/deploy, Agentforce, Slack, jobs, physical tier, real external clients (list them).
  5. ⚠️ Outstanding mandatory gates (you cannot run these — flag them for the maintainer):
    • GPT-4.1 cross-family review is required before merge for any IAM/policy or Lambda handler-signature change. (This PR intentionally avoids real IAM; confirm none was added.)
    • /sh-security-review (deep agentic security pass) is required before merge because this PR touches the authentication/authorization surface (scope enforcement, audience binding, token handling, redaction). State clearly that it has not been run and must be run by the maintainer before merge.
    • Confluence "AWS Architecture Map" (id 1540098) update and project memory update are owed once real infra lands — note as follow-ups, not done here.
  6. Design conformance checklist — tick the design.md §2.5 / §7.3 guarantees you implemented (tool-hiding, server-side scope enforcement, audience binding, no-broker-passthrough, finance redaction on egress, audit logging, prompt-injection containment, rate limiting).

9. Definition of done

  • servers/sh-mcp-ops and servers/sh-mcp-finance start with SH_MCP_ENV=local and serve /mcp, /openapi.json, POST /tools/:name, /healthz.
  • Both interfaces share one dispatch path; scope-hiding, audience binding, finance redaction, audit, input validation, and rate limiting all enforced there.
  • In-memory dev clients make ops read tools + tasks + finance reads return real fake data locally.
  • Full security-weighted test suite passes; coverage gate (80% / 100% on auth+dispatch) enforced in CI config.
  • tsc --noEmit, eslint ., prettier --check . all clean; package-lock.json committed.
  • Synth-only CDK stubs cdk synth cleanly; no real IAM/Cognito resources.
  • Draft PR opened to main with the §8 body, security/cross-review gates flagged as outstanding.