Add Cloud Run deployment infrastructure

[jstjoe]

Prepare for migration from Vercel to Google Cloud Run:

• Add GET /health endpoint for Cloud Run startup/liveness probes
• Add graceful SIGTERM/SIGINT shutdown handling (8s timeout)
• Add multi-stage Dockerfile (node:20-slim, pnpm, prod-only deps)
• Add .dockerignore to minimize build context
• Add GitHub Actions deploy workflow with Workload Identity Federation
• Add workflow_call trigger to test.yml for reuse by deploy workflow

https://claude.ai/code/session_01Gr1yHcWyzydrvgsiHFFqv4

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
sky-mcp-streamable	Ready	Preview, Comment	Jul 2, 2026 7:28pm

[github-actions]

✅ Gitleaks Findings: No secrets detected. Safe to proceed!

[claude]

PR Review: Add Cloud Run Deployment Infrastructure

Overall this is a clean, well-structured PR. The use of Workload Identity Federation avoids storing long-lived credentials, the multi-stage Dockerfile is correct, and tests gate the deploy. A few issues worth addressing before merging:

---

Bugs

process.exit(1) on forced shutdown timeout (src/server.ts)

setTimeout(() => {
  console.error("Forced shutdown after timeout");
  process.exit(1);  // ← should be 0
}, 8000).unref();

A forced shutdown after the grace period is expected behavior, not an error. process.exit(1) signals failure to the container runtime and may trigger unnecessary alerts or restart loops. Use process.exit(0).

---

Potential Issues

pnpm build:ui-imports not documented (Dockerfile)

RUN pnpm build:ui && pnpm build:ui-imports

pnpm build:ui-imports isn't listed in CLAUDE.md's development commands. If this script doesn't exist in package.json, the Docker build will fail. Please verify the script exists and consider adding it to CLAUDE.md.

public/ directory copy in production stage (Dockerfile)

COPY public/ ./public/

public/ isn't mentioned anywhere in the codebase docs. If this directory doesn't exist, the COPY will silently succeed (Docker doesn't error on missing source dirs in COPY if using a glob — but with a trailing / it will error). Confirm this directory exists or remove the line.

---

Security

--allow-unauthenticated on Cloud Run

The service is deployed publicly, which is likely intentional since the MCP server handles its own credential validation. That said, it means anyone can probe the /health endpoint and send unauthenticated requests (which will be rejected by the auth middleware, but still generate traffic/logs). Worth a comment in the deploy workflow to make this explicit, and confirm that rate limiting is sufficient for anonymous-mode abuse prevention.

---

Minor Suggestions

Pin the Node.js base image version (Dockerfile)

FROM node:20-slim AS builder   # ← unpinned minor/patch

node:20-slim will resolve to different patch versions over time, making builds non-reproducible. Consider node:20.19.0-slim (or pin to a digest) to ensure consistent builds.

--min-instances=0 cold starts (deploy.yml)

With zero minimum instances, cold starts will occur. Node.js with TypeScript compilation artifacts should start quickly (~1–2s), but the startup probe allows only 15 seconds total (5 retries × 3s). If any initialization (e.g., module loading) is slower than expected, the probe will fail. Consider --min-instances=1 for a production service or increasing --startup-probe-failure-threshold.

--timeout=120s vs. de-identify_file waitTime (deploy.yml)

The de-identify_file tool accepts a waitTime of up to 64 seconds. The 120-second Cloud Run request timeout provides adequate headroom, but this is a dependency worth keeping in mind if waitTime limits change.

---

What's Done Well

• Workload Identity Federation — no stored GCP credentials in GitHub secrets
• Tests required before deploy via needs: test
• cancel-in-progress: false on the deploy concurrency group prevents partial deploys
• .unref() on the shutdown timeout correctly prevents it from blocking natural process exit
• USER node in the production stage — good least-privilege practice
• .dockerignore is thorough and prevents leaking .env.local into the build context
• workflow_call addition to test.yml is minimal and non-breaking

[github-actions]

✅ Gitleaks Findings: No secrets detected. Safe to proceed!

[claude]

Code Review — Cloud Run deployment infrastructure

Solid, well-scoped migration PR. The build pipeline is correctly preserved in the multi-stage Dockerfile, the health endpoint is placed sensibly (before the rate limiter, so probes aren't throttled), and graceful shutdown wiring looks right. A few things worth addressing before/after merge.

Likely bug — deploy.yml mounts secrets that don't exist anymore

--set-secrets="...,WORKSPACE_ID=WORKSPACE_ID:latest,ACCOUNT_ID=ACCOUNT_ID:latest,..."

WORKSPACE_ID and ACCOUNT_ID are explicitly documented in CLAUDE.md as removed / never consumed by the Skyflow SDK, and grep confirms they are not referenced anywhere in src/. Mounting them via --set-secrets makes the deploy fail hard unless those secrets actually exist in Secret Manager — and if they do exist, they are dead config injected into the container. Recommend dropping both entries.

Reproducibility — pin pnpm to match the pinned Node

You pinned the base image to node:20.19.0-slim for reproducible builds (good call), but the next line floats pnpm with corepack prepare pnpm@10 --activate. pnpm@10 resolves to the latest 10.x at build time, which can drift and reintroduce the non-determinism you just removed. Pin an exact version (e.g. pnpm@10.x.y) and ideally add a matching packageManager field to package.json so local, CI (test.yml uses version 10), and Docker all agree.

Security — allow-unauthenticated is internet-exposed

The service is public and relies entirely on app-level bearer/API-key auth, with anonymous mode serving unauthenticated callers (rate-limited by IP). That is a defensible design for an MCP server, but on Cloud Run the IP seen by the limiter is typically the load-balancer/proxy IP unless you enable Express trust proxy and read X-Forwarded-For — worth confirming the anonymous rate limiter keys on the real client IP, otherwise all anon traffic could share one bucket. Flagging so it is a conscious decision, not an oversight.

Minor notes

• Dockerfile layer ordering is correct but subtle. build:ui-imports writes src/generated/ui-html.ts before COPY src/ src/. It works because the generated file is gitignored (only the .d.ts stub is in the build context, so the COPY merges rather than clobbers), but it is non-obvious — a one-line comment explaining why the COPY follows the generate step would help a future reader.
• No build cache in CI. Each deploy does a cold docker build. Consider docker buildx with cache-from/cache-to (or registry cache) to cut deploy time; optional.
• Shutdown timeout (8s) vs Cloud Run grace (10s). Good buffer. Both the graceful and forced paths now exit(0), which is reasonable — though note in-flight requests still alive at 8s are dropped silently. Fine for this workload.
• Test coverage. The /health endpoint and the shutdown handler are untested. /health is trivial and app is already exported, so a quick assertion that GET /health returns 200 with an ok status would be cheap insurance and matches the repo's existing unit-test discipline.

Things done well

• Health endpoint correctly ordered ahead of rate limiting.
• .dockerignore excludes .env*.local, coverage, and tests — no secret/test leakage into the image.
• Multi-stage build with prod-only deps in the final stage and USER node (non-root) runtime.
• Immutable image tagging by github.sha; concurrency cancel-in-progress: false is the right choice for deploys.
• Reusing test.yml via workflow_call gates deploys on tests.

Nice work overall — the secret-mount issue is the one I would treat as blocking.

[github-actions]

✅ Gitleaks Findings: No secrets detected. Safe to proceed!

[claude]

PR Review: Add Cloud Run deployment infrastructure

Overall this is a clean, well-scoped migration setup. Good use of Workload Identity Federation (no long-lived keys), least-privilege workflow permissions, sha-pinned images, Secret Manager for secrets, non-root container user, and a .dockerignore that excludes .env*.local/.git/tests. The graceful-shutdown + /health additions are correctly gated behind the import.meta.url run-directly check so they don't affect the serverless/Vercel export or tests. Nice.

A few things worth addressing, ordered by importance.

🔴 Verify: Cloud Run probe flags in deploy.yml
The gcloud run deploy step passes --startup-probe-path, --startup-probe-initial-delay, --startup-probe-period, --startup-probe-failure-threshold, --liveness-probe-path, and --liveness-probe-period. These granular --*-probe-* flags are not part of stable gcloud run deploy as far as I'm aware — HTTP startup/liveness probes are normally configured via a service YAML (gcloud run services replace) or the console. If the runner's gcloud rejects unknown flags, the deploy step fails with unrecognized arguments. Please dry-run the exact command (or gcloud run deploy --help | grep probe) against the target gcloud version. If unsupported, move probe config into a service YAML. Cloud Run also does a default TCP startup check on the container port, so the service will still come up even without explicit probes.

🟠 Duplicate test runs on merge to main
test.yml still triggers on push: branches: [main, master], and deploy.yml also invokes it via uses: ./.github/workflows/test.yml with needs: test. On a push to main the unit tests run twice (once standalone, once as the reusable call). Consider dropping main from test.yml's push trigger (keeping pull_request + workflow_call) so tests run once per PR and once as a deploy gate.

🟡 tsx is not a declared dependency but the Docker build depends on it
build:ui-imports runs npx -y tsx scripts/generate-ui-imports.ts, which downloads tsx from the network at build time. This undercuts the reproducibility you just added by pinning node@20.19.0 and pnpm@10.11.0 — the build isn't fully deterministic or offline-capable, and a registry hiccup breaks the image build. Recommend adding tsx to devDependencies and calling it directly so --frozen-lockfile covers it.

🟡 pnpm version drift
Dockerfile pins pnpm@10.11.0 but test.yml uses pnpm/action-setup@v4 with version: 10 (floating minor). Pin the same exact version in both for CI/deploy parity.

Minor / optional

• Redundant UI build in Docker: build:ui builds de-identify-file/mcp-app.html, whose tool is disabled/unregistered. Harmless, just wasted build time.
• Two dependency installs: builder and production stages each run a full pnpm install. Standard multi-stage pattern, but no Docker layer cache in CI (docker build without --cache-from), so image builds will be slow. Optional: enable BuildKit cache.
• --allow-unauthenticated makes the service publicly invocable, which is intended given the per-request credential model and rate-limited anonymous mode — just confirming it's deliberate. Anyone can exercise the anonymous demo vault (IP-rate-limited), so keep an eye on the ANON_MODE_* limits.
• Empty vars.* values: if ANON_MODE_RATE_LIMIT_REQUESTS/WINDOW_MS repo vars are unset, --set-env-vars sets them to empty strings; the code's || 10 / || 60000 fallbacks handle that gracefully — good.

Test coverage
/health and the shutdown handler are untested. There's no HTTP/endpoint test harness yet (no supertest), so the shutdown path is hard to cover, but since src/server.ts exports app, a tiny supertest test asserting GET /health -> 200 {status:ok} would be cheap, high-value, and would lock in that /health stays exempt from auth/rate-limiting.

Confirmed correct

• Port wiring is consistent: Dockerfile ENV PORT=8080 -> server reads process.env.PORT -> --port=8080 in deploy. app.listen(port) binds all interfaces, so Cloud Run can reach it.
• Shutdown timer uses .unref() and an 8s force-exit inside Cloud Run's 10s grace window; process.exit(0) on forced exit is reasonable.
• public/ is copied in the prod stage (not ignored) and holds the committed static assets; UI bundles land in dist via the builder.

🤖 Generated with Claude Code