Fix five bugs found by deploying a real fleet

Deployed a two-replica fleet to Linux VMs behind a Caddy load balancer
doing active health checks. Every one of these passed the unit suite and
the dry run, and every one broke a real deploy.

setup ran deploy-user commands from root's home. sudo inherits the
caller's working directory, and setup normally runs over SSH as root
whose home is mode 700. Node's spawn() returns EACCES from an unreadable
cwd, so `pm2 startup` failed on the documented first-run path with a
message naming the node binary rather than the cause. Now runs with -H
from the target user's home.

--no-deploy-user did nothing. Commander turns it into deployUser: false,
not noDeployUser: true, so the option was read as undefined and the user
was created anyway.

Caddy was never installed on fleet replicas. Both install gates tested
app.domain, and a fleet replica deliberately has none — but it still
needs Caddy for the readiness endpoint. The first deploy died writing its
site file into a directory that was never created.

The health check required primary-pinned processes on every replica. A
placement: 'primary' worker is absent from secondary replicas by design,
so placement working correctly is what failed the health check.

configureAll wrote Caddy sites and never reloaded. Only the blue-green
path reloaded, and it does so for its own colour flip — so a frontend, a
recreate backend, and every fleet replica wrote a site that never took
effect. Pre-existing, not fleet-specific.

Also: a fleet replica now answers to the app's domain as well as its
private address. A load balancer forwards the client's Host, which
matched neither the private-address site nor anything else, so real
traffic fell through to Caddy's welcome page while the health check
passed. The domain is bound http:// only, so no replica requests a
certificate and the ACME race the design avoids stays avoided.

Author: devalade

src/cli/commands/setup.ts

@@ -88,16 +88,40 @@ async function bootstrapDeployUser(
  * user is already root, and the conditional `$SUDO` fallback breaks on `sudo -u`
  * because it leaves `-u` as the leading token when SUDO="".
  */
+/**
+ * Run a command as another user, from a directory that user can actually read.
+ *
+ * `sudo` inherits the caller's working directory, and setup normally runs over
+ * SSH as root, whose home is mode 700. Node's `spawn()` returns EACCES when the
+ * working directory is unreadable, so every pm2 command that starts the daemon
+ * failed — reporting `spawn /home/deploy/.../node EACCES`, which names the node
+ * binary and hides the real cause. `-H` sets HOME to the target user's rather
+ * than leaving root's.
+ */
 function asUser(user: string | null, cmd: string): string {
   if (!user) return cmd;
-  const escaped = cmd.replace(/'/g, "'\\''");
-  return `sudo -u "${user}" bash -lc '${escaped}'`;
+  const escaped = `cd "$HOME" 2>/dev/null || cd /; ${cmd}`.replace(/'/g, "'\\''");
+  return `sudo -u "${user}" -H bash -lc '${escaped}'`;
 }
 
 function shellQuote(value: string): string {
   return `'${value.replace(/'/g, `'"'"'`)}'`;
 }
 
+/**
+ * Whether this server needs Caddy.
+ *
+ * A domain is the obvious case. A fleet replica is the non-obvious one: it
+ * deliberately has *no* domain — five replicas claiming one name would race
+ * Let's Encrypt — but it still needs Caddy to serve the `/_shipnode/ready`
+ * endpoint the load balancer health-checks, and to reverse-proxy the app. Gating
+ * on `domain` alone left fleet replicas without Caddy, and the first deploy died
+ * writing its site file into a directory that was never created.
+ */
+function needsCaddy(config: ShipnodeConfig): boolean {
+  return config.apps.some((app) => app.domain !== undefined || app.fleet !== undefined);
+}
+
 export function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig, ownerUser: string | null) {
   const nodeVersion = config.nodeVersion === 'lts' ? '24' : config.nodeVersion;
   const mise = `export PATH="$HOME/.local/bin:$HOME/.local/share/mise/shims:$PATH"`;
@@ -188,7 +212,7 @@ export function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig, own
           },
         ], { concurrent: false }),
       }] : []),
-      ...(hasApps && config.apps.some((app) => app.domain) ? [{
+      ...(hasApps && needsCaddy(config) ? [{
         title: 'Caddy',
         task: () =>
           executor.execOrThrow(
@@ -270,7 +294,7 @@ export function buildTasks(executor: RemoteExecutor, config: ShipnodeConfig, own
               `${shellQuote(`${config.remotePath}/shared`)} ${shellQuote(`${config.remotePath}/.shipnode`)}`,
             ),
           },
-          ...(config.apps.some((app) => app.domain) ? [{
+          ...(needsCaddy(config) ? [{
             title: 'Configure Caddy include',
             task: () => executor.execOrThrow(
               'SUDO=""; [ "$EUID" -ne 0 ] && SUDO="sudo"; ' +
@@ -294,9 +318,7 @@ async function configurePm2Startup(
   nodeVersion: string,
   mise: string,
 ): Promise<ResultType<void, Pm2StartupError>> {
-  const resolvePm2 = runAsUser
-    ? `sudo -u "${runAsUser}" bash -lc '${mise}; mise exec "node@${nodeVersion}" -- which pm2'`
-    : `${mise}; mise exec "node@${nodeVersion}" -- which pm2`;
+  const resolvePm2 = asUser(runAsUser, `${mise}; mise exec "node@${nodeVersion}" -- which pm2`);
   const savePm2 = asUser(
     runAsUser,
     `${mise}; mise exec "node@${nodeVersion}" -- pm2 save --force`,

src/cli/index.ts

@@ -65,7 +65,10 @@ program
   .description('Setup a new server with required dependencies')
   .option('--no-deploy-user', 'Skip creating the deploy user (advanced)')
   .option('--config <path>', 'Use a specific config file')
-  .action((opts) => cmdSetup(process.cwd(), opts));
+  // Commander turns `--no-deploy-user` into `deployUser: false`, not
+  // `noDeployUser: true`, so passing opts straight through silently ignored the
+  // flag and created the user anyway.
+  .action((opts) => cmdSetup(process.cwd(), { ...opts, noDeployUser: opts.deployUser === false }));
 
 program
   .command('deploy')

src/domain/deploy/orchestrator.ts

@@ -161,7 +161,10 @@ export class DeployOrchestrator {
       let healthResponseMs: number | undefined;
 
       if (app.appType === 'backend' && app.healthCheck.enabled) {
-        const healthResult = await this.healthCheck.perform(app, this.healthOpts(app, target));
+        const healthResult = await this.healthCheck.perform(
+          app,
+          this.healthOpts(app, target, fleetRole.primary),
+        );
         healthAttempts = healthResult.attempts;
         healthResponseMs = healthResult.responseMs;
       }
@@ -261,12 +264,24 @@ export class DeployOrchestrator {
   /**
    * Health-check options that point the probe at the blue-green target colour
    * (its port and colour-suffixed pm2 name) instead of the static config.
+   *
+   * `primaryReplica` also narrows which processes must be online. The status
+   * check requires every declared pm2 app to be running, and a `placement:
+   * 'primary'` process is deliberately absent from every other replica — so
+   * without this, placement working correctly is what fails the health check.
    */
   private healthOpts(
     app: ShipnodeApp,
     target: DeployTarget | undefined,
+    primaryReplica: boolean,
   ): { httpPort?: number; resolvePm2Name?: (a: Pm2App) => string; pm2Apps?: Pm2App[] } | undefined {
-    if (!target) return undefined;
+    if (!target) {
+      const declared = app.pm2?.apps ?? [];
+      const placed = declared.filter((a) => primaryReplica || a.placement !== 'primary');
+      // Only override when placement actually removed something, so the default
+      // path stays byte-identical for every config that does not use it.
+      return placed.length === declared.length ? undefined : { pm2Apps: placed };
+    }
     const namespace = app.pm2?.apps[0]?.name ?? app.name;
     // Workers are still on the previous release until afterHealthy — only the
     // new web colour must be online for this probe.

src/services/caddy.service.ts

@@ -32,6 +32,17 @@ ${append}}`;
  * The readiness endpoint is what the load balancer polls. It answers 503 while
  * the drain sentinel exists and 200 otherwise, which is the whole of shipnode's
  * LB integration — matching on the file means no reload is needed to flip it.
+ *
+ * The block answers to two addresses, because two different Hosts arrive on this
+ * port. The load balancer's health check dials the replica directly, so it sends
+ * the private address; forwarded client traffic carries whatever Host the client
+ * asked for, which is the app's domain. Binding only the private address made
+ * the health check pass while every real request fell through to whatever else
+ * held port 80 — on a stock install, Caddy's welcome page.
+ *
+ * `http://` on the domain is load-bearing: without the scheme Caddy would try to
+ * provision a certificate for it, which is the ACME race this design exists to
+ * avoid.
  */
 export function generateFleetCaddyfile(
   app: ShipnodeApp,
@@ -49,7 +60,9 @@ export function generateFleetCaddyfile(
     stateDir: string;
   },
 ): string {
-  const address = `http://${options.bind ?? ''}:${options.listen}`;
+  const addresses = [`http://${options.bind ?? ''}:${options.listen}`];
+  if (app.domain) addresses.push(`http://${app.domain}:${options.listen}`);
+  const address = addresses.join(', ');
   const append = renderCaddyAppend(app);
 
   const body = options.servePath
@@ -104,6 +117,8 @@ export class CaddyService {
   ) {}
 
   async configureAll(): Promise<void> {
+    let wrote = false;
+
     for (const app of this.config.apps) {
       // A fleet replica serves a private port and never claims the public
       // domain, so it is configured even without one.
@@ -118,7 +133,15 @@ export class CaddyService {
       } else {
         await this.configureFrontend(app);
       }
+      wrote = true;
     }
+
+    // Writing the site file changes nothing until Caddy re-reads it. Only the
+    // blue-green path reloaded, and it reloads for its own colour flip — so a
+    // frontend, a recreate backend, and every fleet replica (which has no domain
+    // and is therefore never blue-green) wrote a site that never took effect.
+    // Once at the end rather than per app: one reload covers every site written.
+    if (wrote) await this.reload();
   }
 
   /** Where the drain sentinel for an app lives on this host. */

tests/unit/caddy-snippets.test.ts

@@ -66,3 +66,63 @@ describe('CaddyService snippets', () => {
 }`);
   });
 });
+
+describe('configureAll reloads what it writes', () => {
+  const fleetApp: ShipnodeApp = {
+    ...baseApp,
+    domain: undefined,
+    zeroDowntime: false,
+    fleet: { batch: 1, port: 80, drainWait: 8, readyPath: '/_shipnode/ready' },
+  };
+
+  it('reloads Caddy after writing a fleet replica site', async () => {
+    // The site file changes nothing until Caddy re-reads it. Only the blue-green
+    // path reloaded, and a fleet replica has no domain so is never blue-green —
+    // its readiness endpoint 404'd and the load balancer took it out of rotation.
+    const executor = new FakeRemoteExecutor();
+    const fleetConfig: ShipnodeConfig = {
+      ...config,
+      servers: { 'web-a': { host: '1.2.3.4', user: 'deploy', port: 22, privateHost: '10.0.0.11' } },
+      ssh: { host: '1.2.3.4', user: 'deploy', port: 22, privateHost: '10.0.0.11' },
+      apps: [fleetApp],
+    };
+
+    await new CaddyService(executor, fleetConfig).configureAll();
+
+    const commands = executor.getHistory().map((entry) => entry.command);
+    const wrote = commands.findIndex((c) => c.includes('/etc/caddy/conf.d/api.caddy'));
+    const reloaded = commands.findIndex((c) => c.includes('systemctl reload caddy'));
+
+    expect(wrote).toBeGreaterThanOrEqual(0);
+    expect(reloaded).toBeGreaterThan(wrote);
+  });
+
+  it('reloads once, not once per app', async () => {
+    const executor = new FakeRemoteExecutor();
+    const twoApps: ShipnodeConfig = {
+      ...config,
+      apps: [
+        { ...baseApp, name: 'api', zeroDowntime: false },
+        { ...baseApp, name: 'web', appType: 'frontend', pm2: undefined, domain: 'example.com' },
+      ],
+    };
+
+    await new CaddyService(executor, twoApps).configureAll();
+
+    const reloads = executor.getHistory()
+      .filter((entry) => entry.command.includes('systemctl reload caddy'));
+    expect(reloads).toHaveLength(1);
+  });
+
+  it('does not reload when there is nothing to write', async () => {
+    const executor = new FakeRemoteExecutor();
+    const noSites: ShipnodeConfig = {
+      ...config,
+      apps: [{ ...baseApp, domain: undefined, fleet: undefined }],
+    };
+
+    await new CaddyService(executor, noSites).configureAll();
+
+    expect(executor.getHistory().some((e) => e.command.includes('reload caddy'))).toBe(false);
+  });
+});

tests/unit/deploy-command.test.ts

@@ -180,10 +180,11 @@ describe('deploy command dry run', () => {
     expect(output).toContain('Wait 20s for the load balancer to notice');
     expect(output).toContain('Undrain (/_shipnode/ready → 200)');
 
-    // A replica serves its private port; claiming the domain would make every
-    // replica race the others for the same certificate.
-    expect(output).toContain('http://10.0.0.11:80 {');
-    expect(output).not.toContain('api.example.com {');
+    // A replica answers to the domain so forwarded traffic reaches it, but over
+    // plain HTTP — claiming it for TLS would make every replica race the others
+    // for the same certificate.
+    expect(output).toContain('http://10.0.0.11:80, http://api.example.com:80 {');
+    expect(output).not.toMatch(/(?<!http:\/\/)api\.example\.com \{/);
   });
 
   it('says where a pinned worker lands and where the run-once hooks fire', async () => {

tests/unit/drain.test.ts

@@ -30,16 +30,22 @@ const config: ShipnodeConfig = {
 };
 
 describe('fleet Caddy site', () => {
-  it('never claims the public domain', async () => {
-    // Every replica writing `api.example.com { ... }` means every replica
-    // races the others for a Let's Encrypt certificate on the same name.
+  it('answers to the domain over plain HTTP, never claiming it for TLS', async () => {
+    // Two different Hosts arrive on this port: the load balancer's health check
+    // dials the private address, while forwarded client traffic carries the
+    // app's domain. Binding only the private address made the health check pass
+    // while every real request fell through to whatever else held port 80.
+    //
+    // The `http://` scheme is what keeps this safe — a bare `api.example.com {`
+    // would have every replica racing the others for one Let's Encrypt cert.
     const executor = new FakeRemoteExecutor();
 
     await new CaddyService(executor, config).configureBackend(fleetApp);
 
     const written = executor.getLastCommand()?.command ?? '';
-    expect(written).not.toContain('api.example.com {');
-    expect(written).toContain('http://10.0.0.11:80 {');
+    expect(written).toContain('http://10.0.0.11:80, http://api.example.com:80 {');
+    expect(written).not.toContain('api.example.com {\n');
+    expect(written).not.toMatch(/(?<!http:\/\/)api\.example\.com \{/);
   });
 
   it('serves a readiness endpoint gated on the drain sentinel', () => {
@@ -77,7 +83,20 @@ describe('fleet Caddy site', () => {
       stateDir: '/state',
     });
 
-    expect(site).toContain('http://:8080 {');
+    expect(site).toContain('http://:8080, http://api.example.com:8080 {');
+  });
+
+  it('binds only the private address when the app declares no domain', () => {
+    const site = generateFleetCaddyfile({ ...fleetApp, domain: undefined }, {
+      listen: 80,
+      bind: '10.0.0.11',
+      upstream: 3000,
+      readyPath: '/ready',
+      stateDir: '/state',
+    });
+
+    expect(site).toContain('http://10.0.0.11:80 {');
+    expect(site).not.toContain(',');
   });
 
   it('serves static files for a frontend replica', async () => {

tests/unit/orchestrator.test.ts

@@ -198,6 +198,75 @@ describe('DeployOrchestrator', () => {
   });
 });
 
+describe('health check and worker placement', () => {
+  async function deployOn(primary: boolean): Promise<string[]> {
+    const executor = new FakeRemoteExecutor();
+    const config = makeConfig({
+      pm2: {
+        apps: [
+          { name: 'app', port: 3000 },
+          { name: 'cron', command: 'node cron.js', placement: 'primary' },
+        ],
+      },
+      healthCheck: { enabled: true, path: '/health', timeout: 30, retries: 3, startupDelay: 0 },
+    });
+
+    executor
+      .when((cmd) => cmd.includes('deploy.lock'), { stdout: 'OK', exitCode: 0 })
+      .when((cmd) => cmd.includes('mkdir -p'), { stdout: '', exitCode: 0 })
+      .when((cmd) => cmd.includes('ln -sfn'), { stdout: '', exitCode: 0 })
+      .when((cmd) => cmd.includes('mv -Tf'), { stdout: '', exitCode: 0 })
+      .when((cmd) => cmd.includes('cat') && cmd.includes('releases.json'), { stdout: '[]', exitCode: 0 })
+      .when((cmd) => cmd.includes('releases.json') && cmd.includes('base64'), { stdout: '', exitCode: 0 })
+      .when((cmd) => cmd.includes('ls -1t'), { stdout: '', exitCode: 0 })
+      .when((cmd) => cmd.includes('date') && cmd.includes('curl'), { stdout: '200 42', exitCode: 0 })
+      // Only the web process is running: the cron is pinned to the primary and
+      // genuinely absent everywhere else.
+      .when((cmd) => cmd.includes('pm2 jlist'), {
+        stdout: JSON.stringify([{ name: 'app', pm2_env: { status: 'online', restart_time: 0 } }]),
+        exitCode: 0,
+      });
+
+    const { DeployLock } = await import('../../src/domain/release/manager.js');
+    const { HealthCheckService } = await import('../../src/services/health.service.js');
+    const { CaddyService } = await import('../../src/services/caddy.service.js');
+
+    const orchestrator = new DeployOrchestrator(
+      config,
+      executor,
+      new DeployLock(executor, config.remotePath),
+      new HealthCheckService(executor, config),
+      new CaddyService(executor, config),
+    );
+
+    const errors: string[] = [];
+    try {
+      await orchestrator.deploy({
+        cwd: '/test',
+        skipBuild: false,
+        fleetRole: { first: true, last: true, primary },
+      });
+    } catch (error) {
+      errors.push(error instanceof Error ? error.message : String(error));
+    }
+    return errors;
+  }
+
+  it('does not require a primary-pinned process on a secondary replica', async () => {
+    // The status check requires every declared pm2 app to be online. A pinned
+    // cron is deliberately absent here, so without the placement filter the
+    // health check fails precisely when placement is working.
+    expect(await deployOn(false)).toEqual([]);
+  });
+
+  it('still requires it on the primary, where it is supposed to be running', async () => {
+    const errors = await deployOn(true);
+
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toContain('cron');
+  });
+});
+
 describe('run-once fleet hooks', () => {
   async function deployWith(fleetRole?: { first: boolean; last: boolean }): Promise<string[]> {
     const ran: string[] = [];