fix(connections): resolve the machine a project runs on instead of comparing serverId columns

apps/api/src/modules/projects/project-connection.service.ts

@@ -16,12 +16,12 @@ import {
   getAppEndpoints,
   getAppConnection,
   getOutputPort,
-  deriveProjectDeployTarget,
   type AppTemplate,
 } from "@repo/core";
 import { getTemplateForOrg } from "../apps/catalog-source";
 import type { RequestContext } from "../../lib/request-context";
 import { assertResourceInOrg } from "../../lib/controller-helpers";
+import { isLocalHostRow } from "../../lib/box-org";
 import { permission } from "../../lib/permission";
 import { getAppConnectionView, type AppConnectionOutput } from "../apps/app-settings.service";
 import { mergeEnvVars } from "./project-env.service";
@@ -234,6 +234,63 @@ async function loadConnectionEnds(
   return { source, target, output, template, declaredPort: spec ? getOutputPort(spec) : null };
 }
 
+/**
+ * WHICH MACHINE a project's workload sits on, collapsed so that every encoding of
+ * "this box" compares equal.
+ *
+ * `box` covers all of them deliberately: a project with no server binding deploys to
+ * the host docker socket, and so does the auto-registered isLocal "This Server" row
+ * AND a plain loopback/SERVER_IP row for this host (deployment-runtime
+ * `resolveTargetPlatform`, `isLocalHostRow`). One daemon means one set of
+ * `openship-<slug>` networks, so treating those as different machines refuses pairs
+ * that are demonstrably co-located.
+ */
+type ProjectHost = { kind: "cloud" } | { kind: "box" } | { kind: "server"; id: string };
+
+const hostKey = (h: ProjectHost): string => (h.kind === "server" ? `server:${h.id}` : h.kind);
+
+/**
+ * Resolve which machine a project's workload sits on.
+ *
+ * Snapshot serverId FIRST, durable column second — the order `readDeployMeta` uses,
+ * and for its reason: the snapshot is where the live release ACTUALLY runs while the
+ * column is where the project is bound, and an alias only resolves next to the
+ * running container. (`resolveSnapshotTarget` and `countActiveByServer` deliberately
+ * invert this — they answer "where would the NEXT deploy go", a different question.)
+ *
+ * Cloud is the UNION of both signals, mirroring project-resources.service: the
+ * `cloudWorkspaceId` column alone is not enough, because a self-hosted instance
+ * orchestrating a cloud deploy deliberately leaves it null to stay local-canonical
+ * (deployment-lifecycle, `isLocalOrchestratedCloud`) — for that shape the snapshot is
+ * the only cloud signal there is, and reading the column alone declares it local.
+ *
+ * A project bound to nothing and never deployed is NOT unknown: `resolveSnapshotTarget`
+ * resolves exactly that shape to the host default, so its first deploy lands on this
+ * box and `box` is the honest answer. Reporting it as "no idea" instead read as
+ * "nothing to refuse" at the call sites, which handed a remote source's alias to a
+ * project that was about to deploy somewhere else entirely.
+ */
+async function resolveProjectHost(project: Project): Promise<ProjectHost> {
+  const dep = project.activeDeploymentId
+    ? await repos.deployment.findById(project.activeDeploymentId).catch(() => null)
+    : null;
+  const meta = (dep?.meta ?? null) as { deployTarget?: string; serverId?: string } | null;
+  if (meta?.deployTarget === "cloud" || project.cloudWorkspaceId) return { kind: "cloud" };
+
+  const serverId = meta?.serverId ?? project.serverId ?? null;
+  if (!serverId) return { kind: "box" };
+
+  const row = await repos.server
+    .getInOrganization(serverId, project.organizationId)
+    .catch(() => null);
+  // A row we cannot read is not PROVABLY this box, so it stays its own machine — the
+  // failure mode of guessing "local" here is a dead alias, which is what this check
+  // exists to prevent.
+  return row && (await isLocalHostRow(row).catch(() => false))
+    ? { kind: "box" }
+    : { kind: "server", id: serverId };
+}
+
 /** The east-west value an internal connection should inject, or why it can't. */
 type InternalResolution = { value: string } | { error: string };
 
@@ -243,31 +300,30 @@ type InternalResolution = { value: string } | { error: string };
  * infer availability from WHICH error `createConnection` happened to throw first —
  * so the answer depended on the shape of an unrelated output's value.
  */
-function resolveInternalValue(ends: ConnectionEnds): InternalResolution {
+async function resolveInternalValue(ends: ConnectionEnds): Promise<InternalResolution> {
   const { source, target, output, template, declaredPort } = ends;
 
   // GH-631: a non-URL value (password, token, API key, bucket name) names no host,
   // so internal mode has nothing to rewrite and nothing to reach — inject it
   // verbatim. Answered BEFORE reachability on purpose: a credential is equally
-  // valid whichever box the source runs on, and gating it on network topology
-  // would re-break the case that was reported.
+  // valid whichever box the source runs on, gating it on network topology would
+  // re-break the case that was reported, and resolving hosts costs queries this
+  // path has no use for.
   if (!isNetworkUrl(output.value)) return { value: output.value };
 
   // Everything below hands the consumer a HOST to reach east-west, which rides on
-  // joining the source app's docker network at deploy (attachLinkedNetworks).
-  // Those networks are per-HOST, so both ends must land on the same box: a
-  // cloud-hosted project has no attachable shared network on this pipe yet, and
-  // one server cannot see another's — the alias would resolve nowhere, and the
-  // attach only WARNS, so the deploy goes green around a dead host. `serverId` is
-  // the durable binding behind `deriveProjectDeployTarget`; the per-deployment
-  // snapshot is re-derived, and a deploy that failed to would silently pass here.
-  if (
-    deriveProjectDeployTarget(source) === "cloud" ||
-    deriveProjectDeployTarget(target) === "cloud"
-  ) {
+  // joining the source app's docker network at deploy (attachLinkedNetworks). Those
+  // networks are per-machine and a failed attach only WARNS, so a link across two
+  // machines injects an alias that resolves nowhere and still deploys green.
+  const [sourceHost, targetHost] = await Promise.all([
+    resolveProjectHost(source),
+    resolveProjectHost(target),
+  ]);
+
+  if (sourceHost.kind === "cloud" || targetHost.kind === "cloud") {
     return { error: "Internal mode isn't available for a cloud-hosted app yet — use Public." };
   }
-  if ((source.serverId ?? null) !== (target.serverId ?? null)) {
+  if (hostKey(sourceHost) !== hostKey(targetHost)) {
     return {
       error:
         "Internal mode needs both projects on the same server — they're on different servers, so use Public.",
@@ -310,7 +366,7 @@ export async function internalModeAvailable(
     input.sourceProjectId,
     input.outputId,
   );
-  return !("error" in resolveInternalValue(ends));
+  return !("error" in (await resolveInternalValue(ends)));
 }
 
 export async function createConnection(
@@ -350,7 +406,7 @@ export async function createConnection(
 
   let value = output.value;
   if (mode === "internal") {
-    const resolved = resolveInternalValue(ends);
+    const resolved = await resolveInternalValue(ends);
     if ("error" in resolved) {
       // An EXPLICIT internal ask gets the reason. A DEFAULTED one falls back to
       // public — failing a request that never asked for internal turns a working

apps/api/test/modules/projects/connection-internal-synth.test.ts

@@ -6,15 +6,20 @@ import { describe, it, expect, beforeEach, vi } from "vitest";
  * (`http://<alias>:<port>`), tagged `internal: true`. `createConnection` must
  * inject that value VERBATIM in internal mode — never route it through
  * `toInternalUrl`, which needs a template and would return null (killing the
- * link). The co-location guard still fires first, so a cloud-hosted source — or
- * one on a different server, since `openship-<slug>` networks are per-host — is
- * steered to Public even when it carries a synthesized internal value.
+ * link). The co-location guard still fires first, so a cloud-hosted source — or one
+ * on a genuinely different machine, since `openship-<slug>` networks are per-machine
+ * — is steered to Public even when it carries a synthesized internal value. What
+ * counts as "a different machine" is the delicate part, and most of the cases below
+ * are about the pairs that are NOT: every encoding of this box is one machine, and a
+ * project with no destination yet is not a second one.
  */
 
 const h = vi.hoisted(() => ({
   findById: vi.fn(),
   listEnvVars: vi.fn(),
   deploymentFindById: vi.fn(),
+  serverGetInOrganization: vi.fn(),
+  isLocalHostRow: vi.fn(),
   listByTarget: vi.fn(),
   upsert: vi.fn(),
   getTemplateForOrg: vi.fn(),
@@ -27,10 +32,15 @@ vi.mock("@repo/db", () => ({
   repos: {
     project: { findById: h.findById, listEnvVars: h.listEnvVars },
     deployment: { findById: h.deploymentFindById },
+    server: { getInOrganization: h.serverGetInOrganization },
     projectConnection: { listByTarget: h.listByTarget, upsert: h.upsert },
   },
 }));
 
+vi.mock("../../../src/lib/box-org", () => ({
+  isLocalHostRow: h.isLocalHostRow,
+}));
+
 vi.mock("../../../src/modules/apps/catalog-source", () => ({
   getTemplateForOrg: h.getTemplateForOrg,
 }));
@@ -93,6 +103,9 @@ beforeEach(() => {
   h.listEnvVars.mockResolvedValue([]);
   h.listByTarget.mockResolvedValue([]);
   h.deploymentFindById.mockResolvedValue(null);
+  // Default: a server row that is NOT this box, so a serverId means a real remote.
+  h.serverGetInOrganization.mockImplementation(async (id: string) => ({ id, isLocal: false }));
+  h.isLocalHostRow.mockImplementation(async (row: { isLocal?: boolean }) => !!row?.isLocal);
   h.toInternalUrl.mockReturnValue(null);
   h.upsert.mockImplementation(async (row: Record<string, unknown>) => ({ ...row, id: "conn_new" }));
   h.mergeEnvVars.mockResolvedValue(undefined);
@@ -122,10 +135,9 @@ describe("createConnection — synthesized internal source", () => {
   });
 
   it("still steers a CLOUD-hosted source to Public before injecting anything", async () => {
-    // Read from the DURABLE binding (`cloudWorkspaceId`, via
-    // deriveProjectDeployTarget), not the per-deployment meta snapshot: a project
-    // bound to cloud that hasn't redeployed has no snapshot to read, and used to
-    // sail through this guard.
+    // The DURABLE half of the union: a project bound to cloud that hasn't redeployed
+    // has no snapshot to read. The snapshot half is pinned separately below —
+    // neither signal alone catches every cloud shape.
     h.findById.mockImplementation(async (id: string) =>
       id === "app-a"
         ? { id: "app-a", name: "App A", slug: "app-a", organizationId: "org1", activeDeploymentId: null }
@@ -169,6 +181,110 @@ describe("createConnection — synthesized internal source", () => {
     expect(h.upsert).not.toHaveBeenCalled();
   });
 
+  it("treats the isLocal 'This Server' row and an unbound-but-deployed project as ONE machine", async () => {
+    // The regression this pins: comparing raw serverId columns called these two
+    // different machines. They are the same docker daemon — a project with no server
+    // binding deploys to the host socket, and so does the isLocal row
+    // (deployment-runtime `resolveTargetPlatform`) — so the same
+    // `openship-<slug>` networks are reachable and internal MUST be allowed.
+    h.serverGetInOrganization.mockImplementation(async (id: string) => ({ id, isLocal: true }));
+    h.findById.mockImplementation(async (id: string) =>
+      id === "app-a"
+        ? { id: "app-a", name: "App A", slug: "app-a", organizationId: "org1", activeDeploymentId: "dep-a", serverId: null }
+        : { id: "db-c", name: "Plain App", slug: "plain-app", organizationId: "org1", appTemplateId: null, activeDeploymentId: "dep-c", serverId: "srv-local" },
+    );
+
+    const res = await createConnection(
+      ctx,
+      "app-a",
+      { sourceProjectId: "db-c", outputId: "svc", envKey: "DB_URL", mode: "internal" },
+      { defer: true },
+    );
+
+    expect(res.connection.mode).toBe("internal");
+    expect(h.mergeEnvVars).toHaveBeenCalledWith(
+      "app-a",
+      "org1",
+      expect.objectContaining({
+        upserts: [{ key: "DB_URL", value: "http://my-app:8080", isSecret: true }],
+      }),
+    );
+  });
+
+  it("allows connect-before-deploy when the source is on THIS box", async () => {
+    // The app-install wizard wires declared connections BEFORE the first deploy. A
+    // project bound to nothing is not unknown — resolveSnapshotTarget resolves that
+    // exact shape to the host default — so its first deploy lands next to a source
+    // that is already here, and refusing would reject a co-located pair.
+    h.findById.mockImplementation(async (id: string) =>
+      id === "app-a"
+        ? { id: "app-a", name: "App A", slug: "app-a", organizationId: "org1", activeDeploymentId: null, serverId: null }
+        : { id: "db-c", name: "Plain App", slug: "plain-app", organizationId: "org1", appTemplateId: null, activeDeploymentId: "dep-c", serverId: null },
+    );
+
+    const res = await createConnection(
+      ctx,
+      "app-a",
+      { sourceProjectId: "db-c", outputId: "svc", envKey: "DB_URL", mode: "internal" },
+      { defer: true },
+    );
+
+    expect(res.connection.mode).toBe("internal");
+  });
+
+  it("still refuses connect-before-deploy when the source is on a REMOTE server", async () => {
+    // The other half, and the one that makes "no binding yet" a real answer rather
+    // than an excuse to skip the check: an unbound target deploys HERE by default, so
+    // a source on server A is a different machine and its alias would resolve nowhere.
+    // Treating the unbound end as "nothing to refuse" injected that dead alias and
+    // reported success.
+    h.findById.mockImplementation(async (id: string) =>
+      id === "app-a"
+        ? { id: "app-a", name: "App A", slug: "app-a", organizationId: "org1", activeDeploymentId: null, serverId: null }
+        : { id: "db-c", name: "Plain App", slug: "plain-app", organizationId: "org1", appTemplateId: null, activeDeploymentId: "dep-c", serverId: "srv-a" },
+    );
+
+    await expect(
+      createConnection(
+        ctx,
+        "app-a",
+        { sourceProjectId: "db-c", outputId: "svc", envKey: "DB_URL", mode: "internal" },
+        { defer: true },
+      ),
+    ).rejects.toThrow(/same server/i);
+
+    expect(h.mergeEnvVars).not.toHaveBeenCalled();
+    expect(h.upsert).not.toHaveBeenCalled();
+  });
+
+  it("catches a cloud source from the deployment SNAPSHOT when cloudWorkspaceId is null", async () => {
+    // A self-hosted instance orchestrating a cloud deploy deliberately leaves
+    // `cloudWorkspaceId` null to stay local-canonical (deployment-lifecycle,
+    // isLocalOrchestratedCloud), so the column alone reads "local" and the guard went
+    // silent. `meta.deployTarget` is that shape's only cloud signal.
+    h.findById.mockImplementation(async (id: string) =>
+      id === "app-a"
+        ? { id: "app-a", name: "App A", slug: "app-a", organizationId: "org1", activeDeploymentId: null, serverId: null }
+        : { id: "db-c", name: "Plain App", slug: "plain-app", organizationId: "org1", appTemplateId: null, activeDeploymentId: "dep-c", serverId: null, cloudWorkspaceId: null },
+    );
+    h.deploymentFindById.mockResolvedValue({
+      id: "dep-c",
+      meta: { deployTarget: "cloud", buildStrategy: "local" },
+    });
+
+    await expect(
+      createConnection(
+        ctx,
+        "app-a",
+        { sourceProjectId: "db-c", outputId: "svc", envKey: "DB_URL", mode: "internal" },
+        { defer: true },
+      ),
+    ).rejects.toThrow(/cloud-hosted/i);
+
+    expect(h.mergeEnvVars).not.toHaveBeenCalled();
+    expect(h.upsert).not.toHaveBeenCalled();
+  });
+
   it("allows internal for two projects on the SAME server", async () => {
     h.findById.mockImplementation(async (id: string) =>
       id === "app-a"