fix(mesh): address Codex audit findings on F-1 PR (#1158)

- docs(sencho-mesh): split subnet_overlap troubleshooting into env-set vs env-unset paths; rewrite the "Customising the mesh subnet" intro to describe the candidate list and the adopt-existing behavior. - backend(MeshService): preserve idempotent 409 handling in the explicit-env path. On createNetwork 409 (TOCTOU race against another process), re-inspect and treat the race-winner as success when its subnet matches the operator's request; subnet_mismatch otherwise. - frontend(MeshDataPlaneBanner): trim the card variant to a true one-line strip (headline only, truncate min-w-0). Full recovery hint stays on the Routing tab variant and in docs. - tests(mesh): add five cases covering the previously untested branches — candidate-loop non-overlap bail, adopt-existing with unparseable subnet, explicit-env generic createNetwork failure, TOCTOU 409 race-winner match, TOCTOU 409 race-winner mismatch. Architecture map (gitignored per Directive 11) updated locally with the new useMeshDataPlane hook node and the mesh.dashboardBanner flow so the local interactive viewer stays accurate.
2026-07-26 11:49:16 +00:00 · 2026-05-22 13:39:12 -04:00
parent 1a03cf82af
commit e4fe4cfced
4 changed files with 163 additions and 22 deletions
@@ -327,4 +327,117 @@ describe('MeshService.setupMeshNetwork subnet auto-fallback', () => {
        expect(status.subnet).toBe('10.42.0.0/24');
        expect(createNetwork).toHaveBeenCalledTimes(1);
    });
+
+    it('bails the candidate loop on a non-overlap createNetwork failure', async () => {
+        delete process.env.SENCHO_MESH_SUBNET;
+        process.env.HOSTNAME = 'sencho';
+        const overlap = Object.assign(
+            new Error('Pool overlaps with other one on this address space'),
+            { statusCode: 500 },
+        );
+        const daemonErr = Object.assign(
+            new Error('daemon attach error: out of inodes'),
+            { statusCode: 500 },
+        );
+        // First candidate overlaps (recoverable, advance), second hits a
+        // generic daemon error (unrecoverable, bail).
+        const createNetwork = vi.fn()
+            .mockRejectedValueOnce(overlap)
+            .mockRejectedValueOnce(daemonErr);
+        const inspectNetwork = vi.fn().mockRejectedValue({ statusCode: 404, message: 'no such network' });
+        mockDocker({ createNetwork, inspectNetwork });
+
+        const svc = MeshService.getInstance();
+        await callSetup(svc);
+
+        const status = svc.getDataPlaneStatus();
+        expect(status.ok).toBe(false);
+        expect(status.reason).toBe('attach_failed');
+        // Stops at the second candidate; does not advance to the third.
+        expect(createNetwork).toHaveBeenCalledTimes(2);
+        expect(status.subnet).toBe('172.31.0.0/24');
+    });
+
+    it('classifies an unparseable existing subnet as subnet_invalid on the adopt path', async () => {
+        delete process.env.SENCHO_MESH_SUBNET;
+        process.env.HOSTNAME = 'sencho';
+        const createNetwork = vi.fn();
+        const inspectNetwork = vi.fn().mockResolvedValue({
+            IPAM: { Config: [{ Subnet: 'not-a-cidr' }] },
+        });
+        mockDocker({ createNetwork, inspectNetwork });
+
+        const svc = MeshService.getInstance();
+        await callSetup(svc);
+
+        const status = svc.getDataPlaneStatus();
+        expect(status.ok).toBe(false);
+        expect(status.reason).toBe('subnet_invalid');
+        expect(status.subnet).toBe('not-a-cidr');
+        expect(createNetwork).not.toHaveBeenCalled();
+    });
+
+    it('classifies a generic explicit-env createNetwork failure as attach_failed', async () => {
+        process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24';
+        process.env.HOSTNAME = 'sencho';
+        const daemonErr = Object.assign(
+            new Error('daemon attach error: out of inodes'),
+            { statusCode: 500 },
+        );
+        const createNetwork = vi.fn().mockRejectedValue(daemonErr);
+        const inspectNetwork = vi.fn().mockRejectedValue({ statusCode: 404, message: 'no such network' });
+        mockDocker({ createNetwork, inspectNetwork });
+
+        const svc = MeshService.getInstance();
+        await callSetup(svc);
+
+        const status = svc.getDataPlaneStatus();
+        expect(status.ok).toBe(false);
+        expect(status.reason).toBe('attach_failed');
+        expect(status.subnet).toBe('10.42.0.0/24');
+    });
+
+    it('treats a TOCTOU 409 in the explicit-env path as idempotent when the race-winner subnet matches', async () => {
+        process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24';
+        process.env.HOSTNAME = 'sencho';
+        const conflict = Object.assign(
+            new Error('network with name sencho_mesh already exists'),
+            { statusCode: 409 },
+        );
+        const createNetwork = vi.fn().mockRejectedValue(conflict);
+        // First inspect (pre-create probe): 404. Second inspect (post-409
+        // race-winner check): the matching subnet.
+        const inspectNetwork = vi.fn()
+            .mockRejectedValueOnce({ statusCode: 404, message: 'no such network' })
+            .mockResolvedValueOnce({ IPAM: { Config: [{ Subnet: '10.42.0.0/24' }] } });
+        mockDocker({ createNetwork, inspectNetwork });
+
+        const svc = MeshService.getInstance();
+        await callSetup(svc);
+
+        const status = svc.getDataPlaneStatus();
+        expect(status.ok).toBe(true);
+        expect(status.subnet).toBe('10.42.0.0/24');
+    });
+
+    it('treats a TOCTOU 409 in the explicit-env path as subnet_mismatch when the race-winner differs', async () => {
+        process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24';
+        process.env.HOSTNAME = 'sencho';
+        const conflict = Object.assign(
+            new Error('network with name sencho_mesh already exists'),
+            { statusCode: 409 },
+        );
+        const createNetwork = vi.fn().mockRejectedValue(conflict);
+        const inspectNetwork = vi.fn()
+            .mockRejectedValueOnce({ statusCode: 404, message: 'no such network' })
+            .mockResolvedValueOnce({ IPAM: { Config: [{ Subnet: '172.30.0.0/24' }] } });
+        mockDocker({ createNetwork, inspectNetwork });
+
+        const svc = MeshService.getInstance();
+        await callSetup(svc);
+
+        const status = svc.getDataPlaneStatus();
+        expect(status.ok).toBe(false);
+        expect(status.reason).toBe('subnet_mismatch');
+    });
 });
@@ -662,13 +662,43 @@ export class MeshService extends EventEmitter implements MeshForwarderHost {
                try {
                    await this.createMeshNetwork(envSubnet);
                } catch (err) {
-                    this.recordSetupFailure(
-                        this.classifyMeshNetworkError(err),
-                        err,
-                        'error',
-                        envSubnet,
-                    );
-                    return;
+                    // TOCTOU: another process may have created `sencho_mesh`
+                    // between our inspect (returned null) and our create
+                    // (rejected with 409). Re-inspect; if the existing
+                    // subnet matches what the operator requested, treat
+                    // this as idempotent success (matches the prior
+                    // ensureMeshNetwork 409-then-inspect behavior). Any
+                    // other error or a mismatch reverts to the typed
+                    // failure path.
+                    const dockerErr = err as { statusCode?: number };
+                    if (dockerErr?.statusCode === 409) {
+                        const raceWinner = await this.inspectExistingMeshSubnet().catch(() => null);
+                        if (raceWinner === envSubnet) {
+                            // Adopt the race-winner's network; proceed to attach.
+                        } else if (raceWinner) {
+                            this.recordSetupFailure(
+                                'subnet_mismatch',
+                                new Error(
+                                    `${SENCHO_MESH_NETWORK} exists with subnet ${raceWinner}, ` +
+                                    `expected ${envSubnet}. Remove the network or set SENCHO_MESH_SUBNET to match.`,
+                                ),
+                                'error',
+                                envSubnet,
+                            );
+                            return;
+                        } else {
+                            this.recordSetupFailure('attach_failed', err, 'error', envSubnet);
+                            return;
+                        }
+                    } else {
+                        this.recordSetupFailure(
+                            this.classifyMeshNetworkError(err),
+                            err,
+                            'error',
+                            envSubnet,
+                        );
+                        return;
+                    }
                }
            }
        } else if (existingSubnet) {
@@ -154,7 +154,7 @@ Use Test before assuming the issue is your application. It tells you whether the

 ## Customising the mesh subnet

-The default `172.30.0.0/24` will collide if a host already has a Docker bridge in that range. Override it per node with `SENCHO_MESH_SUBNET`:
+Sencho picks the mesh subnet automatically. When `SENCHO_MESH_SUBNET` is unset and `sencho_mesh` does not already exist on the Docker daemon, the node walks `172.30.0.0/24`, `172.31.0.0/24`, `10.42.0.0/24`, then `10.43.0.0/24` and keeps the first one Docker accepts. When `sencho_mesh` already exists, Sencho adopts its subnet (Docker is the source of truth across restarts). Override the choice per node with `SENCHO_MESH_SUBNET` when you need a specific CIDR:

 ```yaml
 services:
@@ -208,7 +208,8 @@ These are the explicit boundaries of the v1 mesh.

    The fix depends on which reason fired:

-    - `subnet_overlap`: every candidate subnet overlaps an existing Docker bridge network on this host. Run `docker network ls -q | xargs -L1 docker network inspect --format '{{.Name}} {{range .IPAM.Config}}{{.Subnet}} {{end}}'` to list every existing subnet, then set `SENCHO_MESH_SUBNET` to a free `/24` outside the candidate list and recreate the Sencho container.
+    - `subnet_overlap` (`SENCHO_MESH_SUBNET` unset): every candidate subnet overlaps an existing Docker bridge network on this host. Run `docker network ls -q | xargs -L1 docker network inspect --format '{{.Name}} {{range .IPAM.Config}}{{.Subnet}} {{end}}'` to list every existing subnet, then set `SENCHO_MESH_SUBNET` to a free `/24` outside the candidate list and recreate the Sencho container.
+    - `subnet_overlap` (`SENCHO_MESH_SUBNET` set): the explicit CIDR overlaps another Docker bridge network on this host. Either remove the conflicting network or change `SENCHO_MESH_SUBNET` to a free `/24`. Sencho does not auto-fall-back when the operator pins a specific subnet.
    - `subnet_mismatch`: `SENCHO_MESH_SUBNET` is set explicitly, but `sencho_mesh` already exists with a different subnet. Either remove the network (`docker network rm sencho_mesh` after detaching any containers) or change `SENCHO_MESH_SUBNET` to match the existing subnet. Unset the variable to let Sencho adopt whatever is on disk.
    - `subnet_invalid`: `SENCHO_MESH_SUBNET` is not a valid CIDR. Fix the value (it must look like `10.42.0.0/24`) and recreate the container.
    - `ip_in_use`: another container is squatting the IP Sencho wants on its mesh subnet. Find the squatting container with `docker network inspect sencho_mesh`, detach or remove it, then restart Sencho.
@@ -39,20 +39,17 @@ export function MeshDataPlaneBanner({
    const headline = HEADLINES[status.reason](status);

    if (variant === 'card') {
+        // Compact single-row strip for inside the Fleet Heartbeat card.
+        // Headline only — the full recovery hint lives on the Routing tab
+        // banner and in /docs/features/sencho-mesh.mdx#troubleshooting.
        return (
-            <div className="rounded-md border border-destructive/40 bg-destructive/10 px-2.5 py-1.5 mb-2 text-xs text-destructive flex items-start gap-2">
-                <AlertTriangle className="h-3.5 w-3.5 mt-0.5 shrink-0" strokeWidth={1.75} />
-                <div className="min-w-0 leading-relaxed">
-                    <span className="font-medium">Mesh data plane down · </span>
-                    <span className="font-mono text-[11px]">{status.reason}</span>
-                    <span> · </span>
-                    <span>{headline}</span>
-                    <span> Set </span>
-                    <code className="font-mono bg-destructive/15 px-1 rounded text-[10px]">SENCHO_MESH_SUBNET</code>
-                    <span> to a free </span>
-                    <code className="font-mono bg-destructive/15 px-1 rounded text-[10px]">/24</code>
-                    <span> and restart Sencho.</span>
-                </div>
+            <div className="rounded-md border border-destructive/40 bg-destructive/10 px-2.5 py-1.5 mb-2 text-xs text-destructive flex items-center gap-2">
+                <AlertTriangle className="h-3.5 w-3.5 shrink-0" strokeWidth={1.75} />
+                <span className="font-medium shrink-0">Mesh data plane down</span>
+                <span className="text-destructive/60 shrink-0">·</span>
+                <span className="font-mono text-[11px] shrink-0">{status.reason}</span>
+                <span className="text-destructive/60 shrink-0">·</span>
+                <span className="truncate min-w-0">{headline}</span>
            </div>
        );
    }