fix(stacks): update_stack_env defaults to merge-semantic to prevent variable data loss

README.md

@@ -131,7 +131,7 @@ Add to your MCP settings:
 | `get_stack_compose` | Read compose file |
 | `update_stack_compose` | Update compose file |
 | `get_stack_env` | Read environment variables |
-| `update_stack_env` | Update environment variables |
+| `update_stack_env` | Update environment variables (**merge** by default — safe for partial updates; use `mode="replace"` to overwrite all) |
 | `get_stack_env_raw` | Read raw .env file |
 | `validate_stack_env` | Validate env variables |
 | `scan_stacks` | Scan filesystem for stacks |
@@ -365,6 +365,26 @@ Add to your MCP settings:
 
 ## Important Notes
 
+### update_stack_env — Merge vs Replace Semantics
+
+The Dockhand REST endpoint `PUT /api/stacks/{name}/env` has **replace-semantics**: submitting a partial list of variables silently deletes all other variables from the stack. A single-variable update would wipe everything else.
+
+To prevent accidental data loss, this MCP tool defaults to **merge mode**:
+
+1. It fetches the current variable list via `GET /api/stacks/{name}/env`.
+2. It merges the incoming variables by key (new values overwrite existing ones on key collision).
+3. It writes the full combined list back via `PUT`.
+
+```
+# Safe partial update — only PRINTER_HUB_SSO_TRUST_TOKEN changes, all others preserved
+update_stack_env(environmentId=1, name="my-stack", variables=[{key: "MY_VAR", value: "new"}])
+
+# Explicit full replacement — all other variables are deleted
+update_stack_env(environmentId=1, name="my-stack", variables=[...], mode="replace")
+```
+
+Use `mode="replace"` only when you intentionally want to replace the entire variable set.
+
 ### Environment ID is Required
 
 Most Docker resource endpoints (containers, stacks, images, networks, volumes) require an `environmentId` parameter. This maps to the `?env=<id>` query parameter in the Dockhand API. Without it, endpoints return empty arrays.

src/tools/stacks.ts

@@ -5,6 +5,7 @@
 import { z } from 'zod';
 import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
 import type { DockhandClient } from '../client/dockhand-client.js';
+import type { StackEnv, EnvVariable } from '../types/dockhand.js';
 import { registerTool, jsonResponse, textResponse } from '../utils/tool-helper.js';
 import { encodePath } from '../utils/encode-path.js';
 
@@ -136,7 +137,7 @@ export function registerStackTools(server: McpServer, client: DockhandClient): v
   );
 
   registerTool(server, 'update_stack_env',
-    'Update secret environment variables (database-backed, encrypted at rest). Variables flagged isSecret:true are stored in the Dockhand database and injected into containers via shell-env at deploy time — they are NEVER written to the .env file. For non-secret variables that Docker Compose reads from the .env file at container start, use `update_stack_env_raw`.',
+    'Update secret environment variables (database-backed, encrypted at rest). Variables flagged isSecret:true are stored in the Dockhand database and injected into containers via shell-env at deploy time — they are NEVER written to the .env file. For non-secret variables that Docker Compose reads from the .env file at container start, use `update_stack_env_raw`.\n\n**IMPORTANT — merge vs replace semantics:** The underlying Dockhand REST endpoint (`PUT /api/stacks/{name}/env`) has replace-semantics: sending a partial list silently deletes all other variables. This tool therefore defaults to `mode="merge"`: it fetches the current variables first, merges your payload by key (new values win on collision), and then writes the full combined list. Use `mode="replace"` only when you intentionally want to wipe all existing variables and set exactly the provided list.',
     {
       environmentId: z.number().describe('Environment ID'),
       name: z.string().describe('Stack name'),
@@ -145,9 +146,35 @@ export function registerStackTools(server: McpServer, client: DockhandClient): v
         value: z.string().describe('Variable value as string'),
         isSecret: z.boolean().optional().describe('When true, store value in the Dockhand database (encrypted at rest) and inject via shell-env at deploy. When false/omitted, value is written to the .env file as plain text — DO NOT use for credentials.'),
       })).describe('Environment variables — flag secrets with isSecret:true'),
+      mode: z.enum(['merge', 'replace']).optional().describe('How to handle existing variables. "merge" (default): fetch existing vars, update/add the provided ones, preserve all others. "replace": overwrite the entire variable list with exactly the provided variables — all others are deleted.'),
     },
-    async ({ environmentId, name, variables }) => {
-      return jsonResponse(await client.put(`/api/stacks/${encodePath(name)}/env`, { variables }, { env: environmentId }));
+    async ({ environmentId, name, variables, mode = 'merge' }) => {
+      let finalVariables: EnvVariable[];
+
+      if (mode === 'merge') {
+        const existing = await client.get<StackEnv>(
+          `/api/stacks/${encodePath(name)}/env`,
+          { env: environmentId },
+        );
+        const existingVars: EnvVariable[] = existing?.variables ?? [];
+        const mergedByKey = new Map<string, EnvVariable>(
+          existingVars.map((v) => [v.key, v]),
+        );
+        for (const v of variables) {
+          mergedByKey.set(v.key, v);
+        }
+        finalVariables = Array.from(mergedByKey.values());
+      } else {
+        finalVariables = variables;
+      }
+
+      return jsonResponse(
+        await client.put(
+          `/api/stacks/${encodePath(name)}/env`,
+          { variables: finalVariables },
+          { env: environmentId },
+        ),
+      );
     }
   );
 

tests/stack-env-merge.test.ts

@@ -0,0 +1,170 @@
+/**
+ * Static analysis tests for the update_stack_env merge-semantic implementation.
+ *
+ * Background: The Dockhand REST endpoint PUT /api/stacks/{name}/env has
+ * replace-semantics — sending a partial variable list silently deletes all
+ * other variables. This test suite verifies that the MCP tool wrapper adds a
+ * merge layer so that partial updates do not cause data loss.
+ *
+ * See: https://github.com/strausmann/mcp-dockhand/issues/<issue>
+ * Incident: hangar-print-hub stack (2026-06-01), 7 of 8 variables deleted by
+ * a single-key update_stack_env call.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync } from 'fs';
+import { join, dirname } from 'path';
+import { fileURLToPath } from 'url';
+
+const __dirname = dirname(fileURLToPath(import.meta.url));
+const stacksSource = readFileSync(
+  join(__dirname, '..', 'src', 'tools', 'stacks.ts'),
+  'utf-8',
+);
+
+/**
+ * Extract the registerTool(...) source block for a named tool. The block
+ * spans from the registerTool(server, '<toolName>', ...) line to the next
+ * registerTool( call (or end of file if last).
+ */
+function extractToolBlock(source: string, toolName: string): string {
+  const startPattern = new RegExp(
+    `registerTool\\s*\\(\\s*server\\s*,\\s*'${toolName}'`,
+  );
+  const startMatch = startPattern.exec(source);
+  if (!startMatch) {
+    throw new Error(`Tool '${toolName}' not found in source`);
+  }
+  const startIdx = startMatch.index;
+  const afterStart = source.slice(startIdx + 1);
+  const nextToolMatch = /registerTool\s*\(/.exec(afterStart);
+  const endIdx = nextToolMatch
+    ? startIdx + 1 + nextToolMatch.index
+    : source.length;
+  return source.slice(startIdx, endIdx);
+}
+
+describe('update_stack_env — merge-semantic implementation', () => {
+  const block = extractToolBlock(stacksSource, 'update_stack_env');
+
+  describe('mode parameter', () => {
+    it('declares a mode parameter with z.enum', () => {
+      expect(block).toMatch(/mode:\s*z\.enum\s*\(\s*\[/);
+    });
+
+    it('mode enum includes "merge" and "replace"', () => {
+      // Both values must appear in the enum declaration
+      expect(block).toMatch(/'merge'/);
+      expect(block).toMatch(/'replace'/);
+    });
+
+    it('mode is optional (defaults to "merge")', () => {
+      // .optional() is chained on the enum
+      expect(block).toMatch(/z\.enum\s*\(\s*\[[\s\S]*?\]\s*\)\s*\.optional\(\)/);
+    });
+
+    it('default value is "merge" in the handler destructuring', () => {
+      // Handler must default mode to 'merge': mode = 'merge'
+      expect(block).toMatch(/mode\s*=\s*['"]merge['"]/);
+    });
+  });
+
+  describe('merge path — GET then PUT', () => {
+    it('calls client.get to fetch existing variables when merging', () => {
+      // Must call client.get for the /env endpoint
+      expect(block).toMatch(/client\.get\s*</);
+      expect(block).toMatch(/\/api\/stacks\/\$\{encodePath\(name\)\}\/env/);
+    });
+
+    it('uses a Map to merge variables by key', () => {
+      // Key-based deduplication via Map
+      expect(block).toMatch(/new\s+Map\s*</);
+    });
+
+    it('iterates over incoming variables and sets them in the Map', () => {
+      // mergedByKey.set(v.key, v) pattern
+      expect(block).toMatch(/mergedByKey\.set\s*\(\s*v\.key\s*,\s*v\s*\)/);
+    });
+
+    it('converts the Map back to an array for the PUT body', () => {
+      // Array.from(mergedByKey.values())
+      expect(block).toMatch(/Array\.from\s*\(\s*mergedByKey\.values\s*\(\s*\)\s*\)/);
+    });
+  });
+
+  describe('replace path — PUT without GET', () => {
+    it('assigns variables directly in replace mode without a GET call', () => {
+      // replace branch: finalVariables = variables
+      expect(block).toMatch(/finalVariables\s*=\s*variables/);
+    });
+  });
+
+  describe('PUT call — always uses finalVariables', () => {
+    it('PUT body references finalVariables, not the raw input variables', () => {
+      // The PUT must use finalVariables as the payload
+      expect(block).toMatch(/variables:\s*finalVariables/);
+    });
+
+    it('PUT targets the correct /env endpoint with encodePath', () => {
+      expect(block).toMatch(/client\.put\s*\(/);
+      expect(block).toMatch(/\/api\/stacks\/\$\{encodePath\(name\)\}\/env['"`,]/);
+    });
+
+    it('passes environmentId as env query parameter on PUT', () => {
+      expect(block).toMatch(/env:\s*environmentId/);
+    });
+  });
+
+  describe('description — documents the merge-default behaviour', () => {
+    it('tool description explains the Dockhand replace-semantics risk', () => {
+      expect(block).toMatch(/replace.semantics|replace-semantics/i);
+    });
+
+    it('tool description mentions the merge default', () => {
+      expect(block).toMatch(/mode="merge"|mode=.merge./i);
+    });
+
+    it('tool description mentions the replace opt-in', () => {
+      expect(block).toMatch(/mode="replace"|mode=.replace./i);
+    });
+  });
+
+  describe('type safety', () => {
+    it('imports StackEnv type from dockhand types', () => {
+      expect(stacksSource).toMatch(/import\s+type\s+\{[^}]*StackEnv[^}]*\}/);
+    });
+
+    it('imports EnvVariable type from dockhand types', () => {
+      expect(stacksSource).toMatch(/import\s+type\s+\{[^}]*EnvVariable[^}]*\}/);
+    });
+
+    it('uses typed GET for the existing variables fetch', () => {
+      // client.get<StackEnv>(...)
+      expect(block).toMatch(/client\.get\s*<\s*StackEnv\s*>/);
+    });
+
+    it('declares finalVariables with EnvVariable[] type', () => {
+      expect(block).toMatch(/finalVariables:\s*EnvVariable\[\]/);
+    });
+  });
+});
+
+describe('update_stack_env — existing contract not broken', () => {
+  const block = extractToolBlock(stacksSource, 'update_stack_env');
+
+  it('still declares variables as a required array parameter', () => {
+    expect(block).toMatch(/variables:\s*z\.array\(/);
+    // Must NOT be optional
+    expect(block).not.toMatch(/variables:\s*z\.array\([\s\S]*?\}\s*\)\s*\)\s*\.optional/);
+  });
+
+  it('still references update_stack_env_raw for non-secret writes', () => {
+    expect(block).toMatch(/update_stack_env_raw/);
+  });
+
+  it('variable schema still includes key, value, and optional isSecret', () => {
+    expect(block).toMatch(/key:\s*z\.string\(\)/);
+    expect(block).toMatch(/value:\s*z\.string\(\)/);
+    expect(block).toMatch(/isSecret:\s*z\.boolean\(\)\.optional\(\)/);
+  });
+});

tests/stack-env-tools.test.ts

@@ -71,9 +71,11 @@ describe('update_stack_env (rawContent cleanup)', () => {
     expect(block).not.toMatch(/body\.rawContent/);
   });
 
-  it('sends {variables} directly as the request body', () => {
+  it('sends variables as the "variables" key in the PUT request body', () => {
     const block = extractToolBlock(stacksSource, 'update_stack_env');
-    expect(block).toMatch(/\{\s*variables\s*\}/);
+    // After the merge-semantic refactor the PUT body uses finalVariables,
+    // not the raw input — but the JSON key sent to the API is still "variables".
+    expect(block).toMatch(/variables:\s*finalVariables/);
   });
 
   it('declares variables as a required parameter', () => {