Fix compose-publish x-sam-routes overrides

apps/api/src/routes/mcp/deployment-guide-tools.ts

@@ -80,7 +80,7 @@ In your Compose file, reference these as normal interpolation placeholders, for
 
 ### Step 3 — Author the Compose stack
 
-Write a standard Docker Compose YAML file in the workspace. SAM supports multi-service stacks and derives public routes from either \`x-sam-routes\` or each service's \`ports:\`.
+Write a standard Docker Compose YAML file in the workspace. SAM supports multi-service stacks and derives public routes from each service's \`ports:\` by default. Use \`x-sam-routes\` only when you need an explicit override, such as exposing a service port without \`ports:\` or marking a matching \`ports:\` entry as \`private\`.
 
 \`\`\`yaml
 services:
@@ -94,11 +94,13 @@ services:
       interval: 30s
       timeout: 5s
       retries: 3
+    ports:
+      - "3000"
 
 x-sam-routes:
   - service: web
     port: 3000
-    mode: public
+    mode: private
 \`\`\`
 
 Constraints for compose-publish releases:
@@ -150,7 +152,7 @@ After the publish job reaches \`succeeded\`:
 
 1. \`list_deployment_environments()\` → pick the target.
 2. \`list_deployment_environment_config(environment)\` → review; \`set_deployment_environment_config(...)\` for anything missing (Secrets via \`isSecret: true\`).
-3. Author the Compose stack with \`\${VAR}\` placeholders and \`x-sam-routes\`.
+3. Author the Compose stack with \`\${VAR}\` placeholders and \`ports:\` for public app routes; add \`x-sam-routes\` only for explicit overrides.
 4. \`build_and_publish(environment)\` → capture \`publishJobId\`.
 5. \`get_publish_status(publishJobId)\` until terminal.
 6. \`read_deployment_logs(environment)\` and \`check_dns_status()\` → verify it is actually running.`;

apps/api/src/services/compose-publish-apply.ts

@@ -21,10 +21,13 @@
  *    re-labelled).
  *  - `build:` is replaced with the digest-pinned `image:` that the publish
  *    orchestrator already pushed to the project registry (`pushedRef`).
- *  - `ports:` is TRANSFORMED (not stripped): each published container port
- *    becomes a public route (hostname + loopback hostPort) and the service's
- *    ports are rewritten to `127.0.0.1:<hostPort>:<containerPort>` so node-local
- *    Caddy can reverse-proxy to it.
+ *  - `x-sam-routes` is the explicit route override layer. `ports:` still
+ *    creates public routes by default, but an explicit `mode: private`
+ *    suppresses a matching `ports:` route.
+ *  - Public routes are TRANSFORMED into loopback host bindings: each routed
+ *    container port gets a hostname + loopback hostPort and the service's ports
+ *    are rewritten to `127.0.0.1:<hostPort>:<containerPort>` so node-local Caddy
+ *    can reverse-proxy to it.
  *  - Every other denied field (DENIED_SERVICE_FIELDS) is stripped with a
  *    warning. `logging`/`labels` are denied because SAM re-injects its own.
  *  - SAM injects: the per-environment bridge network, sam.* labels,
@@ -58,9 +61,9 @@ import {
 } from './compose-image-artifacts';
 import {
   assignRouteTargets,
+  collectComposePublishPublicRoutes,
   type DeploymentRouteTarget,
   type DeploymentRouteTargetOptions,
-  type PublicRouteInput,
 } from './deployment-routing';
 
 export const DEFAULT_COMPOSE_PUBLISH_MEMORY_LIMIT_MB = 256;
@@ -381,10 +384,9 @@ export function buildComposePublishApplyPayload(
   const artifactByService = buildArtifactMap(submission);
   const networkName = `sam-internal-${opts.environmentId.replace(/[^a-zA-Z0-9_-]/g, '-')}`;
 
-  const publicRoutes: PublicRouteInput[] = [];
-  // Track which service each route maps to, in route order, so we can rewrite
-  // the service's ports to loopback bindings after host ports are assigned.
-  const routeServiceByIndex: Array<{ service: string; containerPort: number }> = [];
+  const publicRoutes = collectComposePublishPublicRoutes(doc, {
+    portExtractor: collectServiceContainerPorts,
+  });
 
   let hasModelProvider = false;
   const artifacts: ComposeImageArtifactDescriptor[] = [];
@@ -444,14 +446,9 @@ export function buildComposePublishApplyPayload(
       }
     }
 
-    // Collect public routes from ports: (transform exception — NOT stripped).
-    const containerPorts = collectServiceContainerPorts(name, service.ports);
-    for (const port of containerPorts) {
-      publicRoutes.push({ service: name, port });
-      routeServiceByIndex.push({ service: name, containerPort: port });
-    }
-    // Remove ports for now; rewritten to loopback bindings after host-port
-    // assignment below. A service with no resolvable ports keeps none.
+    // Remove ports for now; public routes are rewritten to loopback bindings
+    // after host-port assignment below. Explicit `mode: private` routes leave
+    // the matching service port unpublished.
     delete service.ports;
 
     // Strip every other denied service field (WARN, never error).
@@ -487,14 +484,11 @@ export function buildComposePublishApplyPayload(
   // Rewrite each routed service's ports to loopback bindings now that host
   // ports are assigned. routes preserves publicRoutes order.
   const loopbackPortsByService = new Map<string, string[]>();
-  routes.forEach((route, index) => {
-    const mapped = routeServiceByIndex[index];
-    // Defensive: assignRouteTargets preserves order, so mapped.service === route.service.
-    const serviceName = mapped?.service ?? route.service;
-    const list = loopbackPortsByService.get(serviceName) ?? [];
+  for (const route of routes) {
+    const list = loopbackPortsByService.get(route.service) ?? [];
     list.push(`127.0.0.1:${route.hostPort}:${route.containerPort}`);
-    loopbackPortsByService.set(serviceName, list);
-  });
+    loopbackPortsByService.set(route.service, list);
+  }
   for (const [serviceName, loopbackPorts] of loopbackPortsByService) {
     const service = outServices[serviceName];
     if (isPlainObject(service)) {

apps/api/src/services/deployment-routing.ts

@@ -107,6 +107,163 @@
   port: number;
 }
 
+type ComposePublishRouteMode = 'public' | 'private';
+
+interface ComposePublishRouteInput extends PublicRouteInput {
+  mode: ComposePublishRouteMode;
+}
+
+interface ComposePublishPublicRouteOptions {
+  portExtractor?: (serviceName: string, ports: unknown) => number[];
+}
+
+function routeKey(service: string, port: number): string {
+  return `${service}\u0000${port}`;
+}
+
+function extractComposePorts(_serviceName: string, ports: unknown): number[] {
+  if (!Array.isArray(ports)) {
+    return [];
+  }
+  const result: number[] = [];
+  for (const portSpec of ports) {
+    const port = extractContainerPort(portSpec);
+    if (port !== null) {
+      result.push(port);
+    }
+  }
+  return result;
+}
+
+function formatRouteErrors(errors: Array<{ path: string; message: string }>): string {
+  return errors.map((err) => `${err.path}: ${err.message}`).join('; ');
+}
+
+function parseComposePublishExplicitRoutes(
+  xSamRoutes: unknown,
+  services: Record<string, unknown>
+): ComposePublishRouteInput[] {
+  if (xSamRoutes === undefined || xSamRoutes === null) {
+    return [];
+  }
+
+  const errors: Array<{ path: string; message: string }> = [];
+  const routes: ComposePublishRouteInput[] = [];
+
+  if (!Array.isArray(xSamRoutes)) {
+    throw new Error(
+      'Compose-publish route validation failed: x-sam-routes: "x-sam-routes" must be an array of route definitions.'
+    );
+  }
+
+  for (const [index, route] of xSamRoutes.entries()) {
+    if (!isPlainObject(route)) {
+      errors.push({
+        path: `x-sam-routes[${index}]`,
+        message: 'Each route must be an object with service, port, and mode fields.',
+      });
+      continue;
+    }
+
+    const service = route.service;
+    const port = route.port;
+    const mode = route.mode ?? 'public';
+
+    if (typeof service !== 'string' || service.trim() === '') {
+      errors.push({
+        path: `x-sam-routes[${index}].service`,
+        message: 'Route must specify a "service" name.',
+      });
+      continue;
+    }
+
+    if (!Number.isInteger(port) || typeof port !== 'number' || port < 1 || port > MAX_TCP_PORT) {
+      errors.push({
+        path: `x-sam-routes[${index}].port`,
+        message: `Route must specify a valid "port" (1-${MAX_TCP_PORT}).`,
+      });
+      continue;
+    }
+
+    if (mode !== 'public' && mode !== 'private') {
+      errors.push({
+        path: `x-sam-routes[${index}].mode`,
+        message: `Route mode must be "public" or "private", got "${String(mode)}".`,
+      });
+      continue;
+    }
+
+    if (!(service in services)) {
+      errors.push({
+        path: `x-sam-routes[${index}].service`,
+        message: `Route references service "${service}" which is not defined in "services". Defined services: ${Object.keys(services).join(', ')}`,
+      });
+      continue;
+    }
+
+    const serviceConfig = services[service];
+    if (!isPlainObject(serviceConfig)) {
+      errors.push({
+        path: `x-sam-routes[${index}].service`,
+        message: `Route references service "${service}" which is not a service mapping and cannot be exposed.`,
+      });
+      continue;
+    }
+
+    if ('provider' in serviceConfig) {
+      errors.push({
+        path: `x-sam-routes[${index}].service`,
+        message: `Route references provider service "${service}", but provider services cannot be exposed as SAM app routes.`,
+      });
+      continue;
+    }
+
+    routes.push({ service, port, mode });
+  }
+
+  if (errors.length > 0) {
+    throw new Error(`Compose-publish route validation failed: ${formatRouteErrors(errors)}`);
+  }
+
+  return routes;
+}
+
+/**
+ * Derive compose-publish public route inputs from explicit `x-sam-routes` plus
+ * `ports:` hints. This mirrors the normalized Compose parser's precedence:
+ * explicit service+port routes win, including `mode: private` suppressing a
+ * matching public `ports:` hint.
+ */
+export function collectComposePublishPublicRoutes(
+  doc: Record<string, unknown>,
+  opts: ComposePublishPublicRouteOptions = {}
+): PublicRouteInput[] {
+  if (!isPlainObject(doc.services)) {
+    return [];
+  }
+
+  const portExtractor = opts.portExtractor ?? extractComposePorts;
+  const explicitRoutes = parseComposePublishExplicitRoutes(doc['x-sam-routes'], doc.services);
+  const explicitRouteKeys = new Set(explicitRoutes.map((route) => routeKey(route.service, route.port)));
+  const publicRoutes = explicitRoutes
+    .filter((route) => route.mode === 'public')
+    .map((route) => ({ service: route.service, port: route.port }));
+
+  for (const [serviceName, rawService] of Object.entries(doc.services)) {
+    if (!isPlainObject(rawService) || 'provider' in rawService) {
+      continue;
+    }
+
+    for (const port of portExtractor(serviceName, rawService.ports)) {
+      if (!explicitRouteKeys.has(routeKey(serviceName, port))) {
+        publicRoutes.push({ service: serviceName, port });
+      }
+    }
+  }
+
+  return publicRoutes;
+}
+
 /**
  * Assign loopback host ports + grey-cloud hostnames to an ordered list of
  * public routes. This is the shared derivation used by BOTH the normalized
@@ -194,20 +351,7 @@
     return [];
   }
 
-  const publicRoutes: PublicRouteInput[] = [];
-  for (const [serviceName, rawService] of Object.entries(doc.services)) {
-    if (!isPlainObject(rawService) || !Array.isArray(rawService.ports)) {
-      continue;
-    }
-    for (const portSpec of rawService.ports) {
-      const port = extractContainerPort(portSpec);
-      if (port !== null) {
-        publicRoutes.push({ service: serviceName, port });
-      }
-    }
-  }
-
-  return assignRouteTargets(publicRoutes, opts);
+  return assignRouteTargets(collectComposePublishPublicRoutes(doc), opts);
 }
 
 /**

apps/api/tests/unit/services/compose-publish-apply.test.ts

@@ -181,6 +181,96 @@ describe('buildComposePublishApplyPayload', () => {
     expect(doc.services.worker.ports).toBeUndefined();
   });
 
+  it('publishes explicit x-sam-routes public routes even without service ports', () => {
+    const composeWithExplicitRoute = `services:
+  app:
+    image: example/app:1
+x-sam-routes:
+  - service: app
+    port: 8000
+    mode: public
+`;
+    const result = buildComposePublishApplyPayload(
+      makeSubmission({ composeYaml: composeWithExplicitRoute, services: [] }),
+      OPTS
+    );
+    const doc = parseYaml(result.composeYaml) as { services: Record<string, { ports?: string[] }> };
+    const route = result.routes.at(0);
+
+    expect(result.routes).toHaveLength(1);
+    expect(route).toEqual(
+      expect.objectContaining({
+        hostname: `r1-app-8000-${ENVIRONMENT_ID}.apps.${BASE_DOMAIN}`,
+        service: 'app',
+        containerPort: 8000,
+      })
+    );
+    expect(doc.services.app?.ports).toEqual([`127.0.0.1:${route?.hostPort}:8000`]);
+  });
+
+  it('lets x-sam-routes private suppress a matching ports-derived public route', () => {
+    const composeWithPrivateOverride = `services:
+  app:
+    image: example/app:1
+    ports:
+      - "8000:8000"
+x-sam-routes:
+  - service: app
+    port: 8000
+    mode: private
+`;
+    const result = buildComposePublishApplyPayload(
+      makeSubmission({ composeYaml: composeWithPrivateOverride, services: [] }),
+      OPTS
+    );
+    const doc = parseYaml(result.composeYaml) as { services: Record<string, { ports?: string[] }> };
+
+    expect(result.routes).toEqual([]);
+    expect(doc.services.app?.ports).toBeUndefined();
+  });
+
+  it('does not duplicate a route when x-sam-routes and ports cover the same public port', () => {
+    const composeWithDuplicateHints = `services:
+  app:
+    image: example/app:1
+    ports:
+      - "8000:8000"
+x-sam-routes:
+  - service: app
+    port: 8000
+    mode: public
+`;
+    const result = buildComposePublishApplyPayload(
+      makeSubmission({ composeYaml: composeWithDuplicateHints, services: [] }),
+      OPTS
+    );
+    const doc = parseYaml(result.composeYaml) as { services: Record<string, { ports?: string[] }> };
+    const route = result.routes.at(0);
+
+    expect(result.routes).toHaveLength(1);
+    expect(route?.service).toBe('app');
+    expect(route?.containerPort).toBe(8000);
+    expect(doc.services.app?.ports).toEqual([`127.0.0.1:${route?.hostPort}:8000`]);
+  });
+
+  it('fails fast for invalid compose-publish x-sam-routes entries', () => {
+    const composeWithInvalidRoute = `services:
+  app:
+    image: example/app:1
+x-sam-routes:
+  - service: app
+    port: 8000
+    mode: external
+`;
+
+    expect(() =>
+      buildComposePublishApplyPayload(
+        makeSubmission({ composeYaml: composeWithInvalidRoute, services: [] }),
+        OPTS
+      )
+    ).toThrow(/x-sam-routes\[0\]\.mode.*public.*private/i);
+  });
+
   it('allows interpolated host ports because SAM rewrites host bindings', () => {
     const composeWithInterpolatedHostPort = `services:
   app: