sh-mcp/docs/build-plan-phase-1.md

# 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 `ToolDef`s 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 <finance tool>" 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.