🐛 fix(triggers): prevent compose update from destroying container on failure (#391)

app/triggers/providers/dockercompose/Dockercompose.compose-exec-and-hooks.test.ts

@@ -308,6 +308,177 @@ describe('Dockercompose Trigger', () => {
     ).rejects.toThrow('create failed');
   });
 
+  // BUG #391: rollback safety net tests
+  // -----------------------------------------------------------------------
+
+  test('[#391] updateContainerWithCompose should attempt to restore old container when recreateContainer throws', async () => {
+    trigger.configuration.dryrun = false;
+    const container = makeContainer({ name: 'nginx' });
+    vi.spyOn(trigger, 'pullImage').mockResolvedValue();
+    vi.spyOn(trigger, 'stopContainer').mockResolvedValue();
+    vi.spyOn(trigger, 'removeContainer').mockResolvedValue();
+
+    // First createContainer call fails (new image creation), second succeeds (rollback restore).
+    const createContainerSpy = vi
+      .spyOn(trigger, 'createContainer')
+      .mockRejectedValueOnce(new Error('No such image: nginx:1.1.0'))
+      .mockResolvedValueOnce({ start: vi.fn().mockResolvedValue(undefined) } as any);
+
+    await expect(
+      trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container),
+    ).rejects.toThrow('No such image: nginx:1.1.0');
+
+    // createContainer is called twice: once for the new image, once for the rollback restore.
+    expect(createContainerSpy).toHaveBeenCalledTimes(2);
+    expect(mockLog.warn).toHaveBeenCalledWith(
+      expect.stringContaining('attempting to restore the original container from captured spec'),
+    );
+    expect(mockLog.info).toHaveBeenCalledWith(
+      expect.stringContaining('restored successfully after failed update'),
+    );
+  });
+
+  test('[#391] updateContainerWithCompose should rethrow original error even when rollback restore also fails', async () => {
+    trigger.configuration.dryrun = false;
+    const container = makeContainer({ name: 'nginx' });
+    vi.spyOn(trigger, 'pullImage').mockResolvedValue();
+    vi.spyOn(trigger, 'stopContainer').mockResolvedValue();
+    vi.spyOn(trigger, 'removeContainer').mockResolvedValue();
+
+    // Both createContainer calls fail: new image and rollback restore.
+    vi.spyOn(trigger, 'createContainer')
+      .mockRejectedValueOnce(new Error('No such image: nginx:1.1.0'))
+      .mockRejectedValueOnce(new Error('restore failed'));
+
+    await expect(
+      trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container),
+    ).rejects.toThrow('No such image: nginx:1.1.0');
+
+    expect(mockLog.warn).toHaveBeenCalledWith(
+      expect.stringContaining('attempting to restore the original container from captured spec'),
+    );
+    expect(mockLog.warn).toHaveBeenCalledWith(
+      expect.stringContaining('Manual intervention may be required'),
+    );
+  });
+
+  test('[#391] updateContainerWithCompose should fall back to newImage when currentContainerSpec has no Config.Image', async () => {
+    trigger.configuration.dryrun = false;
+    const container = makeContainer({ name: 'nginx' });
+    vi.spyOn(trigger, 'pullImage').mockResolvedValue();
+    vi.spyOn(trigger, 'stopContainer').mockResolvedValue();
+    vi.spyOn(trigger, 'removeContainer').mockResolvedValue();
+
+    // Provide a spec without Config.Image to cover the `?? newImage` fallback branch.
+    vi.spyOn(trigger, 'getCurrentContainer').mockResolvedValue(
+      makeDockerContainerHandle({ name: 'nginx' }),
+    );
+    vi.spyOn(trigger, 'inspectContainer').mockResolvedValue({
+      Id: 'container-id',
+      Name: '/nginx',
+      State: { Running: true },
+      HostConfig: { AutoRemove: false },
+      NetworkSettings: { Networks: {} },
+      Config: { Env: [], Labels: {} }, // no Image field
+    } as any);
+
+    const createContainerSpy = vi
+      .spyOn(trigger, 'createContainer')
+      .mockRejectedValueOnce(new Error('create failed'))
+      .mockResolvedValueOnce({ start: vi.fn().mockResolvedValue(undefined) } as any);
+
+    // Should still attempt rollback using newImage as fallback.
+    await expect(
+      trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container),
+    ).rejects.toThrow('create failed');
+
+    expect(createContainerSpy).toHaveBeenCalledTimes(2);
+    expect(mockLog.warn).toHaveBeenCalledWith(
+      expect.stringContaining('attempting to restore the original container from captured spec'),
+    );
+  });
+
+  test('[#391] updateContainerWithCompose should NOT call stopAndRemoveContainer when pre-flight arch check fails', async () => {
+    trigger.configuration.dryrun = false;
+    const container = makeContainer({ name: 'nginx' });
+    vi.spyOn(trigger, 'pullImage').mockResolvedValue();
+    const stopContainerSpy = vi.spyOn(trigger, 'stopContainer').mockResolvedValue();
+    const removeContainerSpy = vi.spyOn(trigger, 'removeContainer').mockResolvedValue();
+
+    // Simulate an image with arm/v6 (arm) architecture on an amd64 host.
+    const incompatibleArch = process.arch === 'arm' ? 'amd64' : 'arm';
+    mockDockerApi.getImage.mockReturnValue({
+      inspect: vi.fn().mockResolvedValue({ Architecture: incompatibleArch, Os: 'linux' }),
+    });
+
+    await expect(
+      trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container),
+    ).rejects.toThrow('is not compatible with host architecture');
+
+    expect(stopContainerSpy).not.toHaveBeenCalled();
+    expect(removeContainerSpy).not.toHaveBeenCalled();
+  });
+
+  test('[#391] verifyPulledImageCompatibility should skip check when dockerApi has no getImage', async () => {
+    const apiWithoutGetImage = { modem: { socketPath: '/var/run/docker.sock' } } as any;
+    // Should not throw — treat as compatible.
+    await expect(
+      trigger.verifyPulledImageCompatibility(apiWithoutGetImage, 'nginx:1.1.0', mockLog),
+    ).resolves.toBeUndefined();
+  });
+
+  test('[#391] verifyPulledImageCompatibility should skip check when image inspect throws', async () => {
+    const failingApi = {
+      modem: { socketPath: '/var/run/docker.sock' },
+      getImage: vi.fn().mockReturnValue({
+        inspect: vi.fn().mockRejectedValue(new Error('image not found')),
+      }),
+    } as any;
+    await expect(
+      trigger.verifyPulledImageCompatibility(failingApi, 'nginx:1.1.0', mockLog),
+    ).resolves.toBeUndefined();
+  });
+
+  test('[#391] verifyPulledImageCompatibility should skip check when image inspect returns no Architecture', async () => {
+    const apiNoArch = {
+      modem: { socketPath: '/var/run/docker.sock' },
+      getImage: vi.fn().mockReturnValue({
+        inspect: vi.fn().mockResolvedValue({ Os: 'linux' }),
+      }),
+    } as any;
+    await expect(
+      trigger.verifyPulledImageCompatibility(apiNoArch, 'nginx:1.1.0', mockLog),
+    ).resolves.toBeUndefined();
+  });
+
+  test('[#391] verifyPulledImageCompatibility should skip check for unknown/exotic architectures', async () => {
+    const apiExoticArch = {
+      modem: { socketPath: '/var/run/docker.sock' },
+      getImage: vi.fn().mockReturnValue({
+        inspect: vi.fn().mockResolvedValue({ Architecture: 'loongarch64', Os: 'linux' }),
+      }),
+    } as any;
+    // Should not throw for unknown architectures (treat as compatible).
+    await expect(
+      trigger.verifyPulledImageCompatibility(apiExoticArch, 'nginx:1.1.0', mockLog),
+    ).resolves.toBeUndefined();
+  });
+
+  test('[#391] verifyPulledImageCompatibility should log compatibility on successful check', async () => {
+    const hostCompatibleArch = process.arch === 'x64' ? 'amd64' : process.arch;
+    const compatibleApi = {
+      modem: { socketPath: '/var/run/docker.sock' },
+      getImage: vi.fn().mockReturnValue({
+        inspect: vi.fn().mockResolvedValue({ Architecture: hostCompatibleArch, Os: 'linux' }),
+      }),
+    } as any;
+    await trigger.verifyPulledImageCompatibility(compatibleApi, 'nginx:1.1.0', mockLog);
+
+    expect(mockLog.info).toHaveBeenCalledWith(
+      expect.stringContaining(`architecture "${hostCompatibleArch}" is compatible`),
+    );
+  });
+
   test('updateContainerWithCompose should throw when inspectContainer returns malformed runtime state', async () => {
     trigger.configuration.dryrun = false;
     const container = makeContainer({ name: 'nginx' });

app/triggers/providers/dockercompose/Dockercompose.test.helpers.ts

@@ -239,6 +239,14 @@ export function setupDockercomposeTestContext({
     getNetwork: vi.fn().mockReturnValue({
       connect: vi.fn().mockResolvedValue(undefined),
     }),
+    // Default: image inspect returns the host-compatible architecture so the
+    // pre-flight guard does not interfere with tests that do not need it.
+    getImage: vi.fn().mockReturnValue({
+      inspect: vi.fn().mockResolvedValue({
+        Architecture: process.arch === 'x64' ? 'amd64' : process.arch,
+        Os: 'linux',
+      }),
+    }),
   };
 
   // getId is called by insertBackup to record which trigger performed the update

app/triggers/providers/dockercompose/Dockercompose.ts

@@ -78,6 +78,12 @@ interface DockerApiLike {
       }>;
     }>;
   };
+  getImage?: (imageRef: string) => {
+    inspect: () => Promise<{
+      Architecture?: string;
+      Os?: string;
+    }>;
+  };
 }
 
 type ContainersByComposeFileEntry = {
@@ -1917,6 +1923,105 @@ class Dockercompose extends Docker<DockercomposeTriggerConfiguration> {
     await this.refreshComposeServiceWithDockerApi(composeFile, service, container, options);
   }
 
+  /**
+   * Map a Docker image Architecture string to the Node.js process.arch value
+   * used on this host. Returns undefined for unknown architectures (treated as
+   * compatible to avoid false-positive blocks on exotic platforms).
+   */
+  private static dockerArchToNodeArch(dockerArch: string): string | undefined {
+    const map: Record<string, string> = {
+      amd64: 'x64',
+      arm64: 'arm64',
+      arm: 'arm',
+      '386': 'ia32',
+      ppc64le: 'ppc64',
+      s390x: 's390x',
+      riscv64: 'riscv64',
+    };
+    return map[dockerArch.toLowerCase()];
+  }
+
+  /**
+   * Pre-flight guard: inspect the just-pulled image and verify its architecture
+   * matches the host before performing any destructive step. Throws a clear,
+   * actionable error on mismatch so the running container is left untouched.
+   *
+   * Skipped when dockerApi does not expose getImage (older mocks / proxies) —
+   * in that case we fall through to the existing stop/create sequence and let
+   * Docker surface its own error.
+   */
+  async verifyPulledImageCompatibility(
+    dockerApi: DockerApiLike,
+    newImage: string,
+    logContainer: { info: (msg: string) => void; warn: (msg: string) => void },
+  ): Promise<void> {
+    if (typeof dockerApi.getImage !== 'function') {
+      return;
+    }
+    let imageInspect: { Architecture?: string; Os?: string } | undefined;
+    try {
+      imageInspect = await dockerApi.getImage(newImage).inspect();
+    } catch {
+      // Image inspect failed — not a hard error; Docker will surface the real
+      // problem during container creation.
+      return;
+    }
+    if (!imageInspect?.Architecture) {
+      return;
+    }
+    const imageArch = imageInspect.Architecture;
+    const hostNodeArch = process.arch;
+    const mappedImageArch = Dockercompose.dockerArchToNodeArch(imageArch);
+    if (mappedImageArch === undefined) {
+      // Unknown architecture — treat as compatible to avoid false-positive blocks.
+      return;
+    }
+    if (mappedImageArch !== hostNodeArch) {
+      throw new Error(
+        `Cannot update to ${newImage}: image architecture "${imageArch}" is not compatible with host architecture "${hostNodeArch}". ` +
+          `The running container has been left untouched.`,
+      );
+    }
+    logContainer.info(
+      `Image ${newImage} architecture "${imageArch}" is compatible with host "${hostNodeArch}"`,
+    );
+  }
+
+  /**
+   * Rollback safety net: after the new container creation fails, attempt to
+   * restore the original container from the captured spec. Logs the outcome
+   * but does NOT swallow the original error — the caller re-throws it.
+   */
+  private async attemptRollbackRestoreOldContainer(
+    dockerApi: DockerApiLike,
+    originalContainerSpec,
+    originalImage: string,
+    container,
+    logContainer: { info: (msg: string) => void; warn: (msg: string) => void },
+  ): Promise<void> {
+    const containerName = container.name;
+    logContainer.warn(
+      `Recreate failed for ${containerName}; attempting to restore the original container from captured spec`,
+    );
+    try {
+      await super.recreateContainer(
+        dockerApi,
+        originalContainerSpec,
+        originalImage,
+        container,
+        logContainer,
+      );
+      logContainer.info(
+        `Original container ${containerName} restored successfully after failed update`,
+      );
+    } catch (rollbackError: unknown) {
+      logContainer.warn(
+        `Failed to restore original container ${containerName} after failed update ` +
+          `(${getErrorMessage(rollbackError)}). Manual intervention may be required.`,
+      );
+    }
+  }
+
   private ensureComposeRuntimeState(currentContainerSpec, composeFile, service): void {
     if (typeof currentContainerSpec?.State?.Running !== 'boolean') {
       throw new Error(
@@ -1992,13 +2097,33 @@ class Dockercompose extends Docker<DockercomposeTriggerConfiguration> {
       );
     }
 
+    // (a) PRE-FLIGHT GUARD — verify the target image is usable on this host
+    // before performing any destructive step. On arch mismatch the old
+    // container is left running and we throw without touching it.
+    await this.verifyPulledImageCompatibility(dockerApi as DockerApiLike, newImage, logContainer);
+
+    // Capture the original container spec for rollback before we do anything
+    // destructive. The Config.Image field holds the old image reference.
+    const originalImage: string = currentContainerSpec?.Config?.Image ?? newImage;
+    const rollbackSpec = {
+      ...currentContainerSpec,
+      State: {
+        ...currentContainerSpec.State,
+        Running: currentContainerSpec.State.Running,
+      },
+    };
+
     const recreationContainerSpec = {
       ...currentContainerSpec,
       State: {
         ...currentContainerSpec.State,
         Running: serviceShouldStart,
       },
     };
+
+    // (b) ROLLBACK SAFETY NET — if recreateContainer throws after we have
+    // already removed the old container, try to restore it from the captured
+    // spec before re-throwing so the service is not left with zero containers.
     // Intentionally bypass Dockercompose.stopAndRemoveContainer() no-op: this
     // internal Engine API refresh path must perform the real stop/remove.
     await super.stopAndRemoveContainer(
@@ -2007,13 +2132,24 @@ class Dockercompose extends Docker<DockercomposeTriggerConfiguration> {
       container,
       logContainer,
     );
-    await super.recreateContainer(
-      dockerApi,
-      recreationContainerSpec,
-      newImage,
-      container,
-      logContainer,
-    );
+    try {
+      await super.recreateContainer(
+        dockerApi,
+        recreationContainerSpec,
+        newImage,
+        container,
+        logContainer,
+      );
+    } catch (recreateError: unknown) {
+      await this.attemptRollbackRestoreOldContainer(
+        dockerApi as DockerApiLike,
+        rollbackSpec,
+        originalImage,
+        container,
+        logContainer,
+      );
+      throw recreateError;
+    }
   }
 
   /**

content/docs/current/configuration/actions/index.mdx

@@ -49,7 +49,7 @@ Container labels follow the same pattern: `dd.action.include` / `dd.action.exclu
 
 <Callout type="warn">**v1.5.x soft-blocker / v1.7.0 hard-blocker change-ahead:** In v1.5.x, `dd.action.include` / `dd.action.exclude` (and legacy `dd.trigger.include` / `dd.trigger.exclude`) are **soft blockers** — they suppress auto-updates but still allow manual updates (UI button, API). In v1.7.0 these labels will become **hard blockers** — containers with a non-matching include or an active exclude label will also reject manual-update requests. Migrate your labels to `dd.action.include` / `dd.action.exclude` before upgrading to v1.7.0.</Callout>
 
-See [Triggers](/docs/configuration/triggers) for full trigger configuration reference and the [migration CLI](/docs/configuration/triggers#migration) for automated prefix rewriting.
+See [Triggers](/docs/configuration/triggers) for full trigger configuration reference and the [migration CLI](/docs/configuration/triggers#environment-variable-prefixes) for automated prefix rewriting.
 
 ## Update action
 

content/docs/current/configuration/triggers/index.mdx

@@ -32,7 +32,7 @@ All three prefixes are interchangeable -- they configure the same triggers. Use
 
 When the same trigger property is set under multiple prefixes, the merge priority is `DD_NOTIFICATION_*` > `DD_ACTION_*` > `DD_TRIGGER_*` (highest wins).
 
-<Callout type="info">A built-in migration CLI can rewrite your config files from `DD_TRIGGER_*` to `DD_ACTION_*` automatically: `drydock config migrate --source trigger`. Use `--dry-run` to preview changes before applying.</Callout>
+<Callout type="info">A built-in migration CLI can rewrite your config files from `DD_TRIGGER_*` to `DD_ACTION_*` automatically: `drydock config migrate --source trigger`. Use `--dry-run` to preview changes before applying. Note that the CLI always emits `DD_ACTION_*` for every trigger type — if you use messaging providers (Slack, SMTP, Discord, ntfy, and others), you may prefer to rename those to `DD_NOTIFICATION_*` manually to match the canonical prefix for your provider category.</Callout>
 
 ## Available triggers