673 climcp deploy

.changeset/cli-honest-deploy-waits.md

@@ -0,0 +1,10 @@
+---
+'@walkeros/cli': patch
+---
+
+`walkeros deploy` now waits long enough to cover a full server deploy by
+default, so a slow but healthy deploy is no longer aborted early and reported as
+a failure. Each run sends a fresh idempotency key, so retrying after a failure
+starts a new deploy instead of replaying the previous result. Failures print a
+stable, machine-readable error code (with a `Retry-After` hint on rate limits),
+and `deploy create` no longer prints an empty token placeholder.

.changeset/mcp-deploy-manage-contract.md

@@ -0,0 +1,8 @@
+---
+'@walkeros/mcp': patch
+---
+
+The `deploy_manage` tool now matches its real behavior: `deploy` honors `wait`,
+`delete` removes an active deployment, and `list` accepts `cursor` and `limit`
+for pagination. A failed deployment surfaces its error reason so an assistant
+can report why a deploy did not succeed.

packages/cli/src/__tests__/unit/commands/deploy.test.ts

@@ -1,7 +1,13 @@
 import { requireProjectId } from '../../../core/auth.js';
 import { apiFetch } from '../../../core/http.js';
 import { getFlow } from '../../../commands/flows/index.js';
-import { deploy, getDeployment } from '../../../commands/deploy/index.js';
+import {
+  deploy,
+  getDeployment,
+  streamDeploymentStatus,
+  renderStatusLabel,
+  DEFAULT_DEPLOY_WAIT_MS,
+} from '../../../commands/deploy/index.js';
 import { setupMockApiClient } from '../../helpers/mock-api-client.js';
 
 jest.mock('../../../core/api-client.js');
@@ -34,6 +40,23 @@ const multiFlowContent = {
   ],
 };
 
+/** Read the Idempotency-Key from a recorded request-init argument. */
+function idempotencyKeyFromHeaders(init: unknown): string {
+  if (init && typeof init === 'object' && 'headers' in init) {
+    const headers = (init as { headers?: Record<string, string> }).headers;
+    return headers?.['Idempotency-Key'] ?? '';
+  }
+  return '';
+}
+
+/** Read the AbortSignal from a recorded request-init argument. */
+function signalFromInit(init: unknown): AbortSignal | undefined {
+  if (init && typeof init === 'object' && 'signal' in init) {
+    return (init as { signal?: AbortSignal }).signal;
+  }
+  return undefined;
+}
+
 /** Helper to create an SSE stream response body */
 function createSSEStreamBody(events: Array<{ event: string; data: object }>) {
   const text = events
@@ -54,12 +77,15 @@ describe('deploy', () => {
   describe('deploy() without flowName', () => {
     it('calls legacy POST route', async () => {
       mockPost.mockResolvedValue({
-        data: { status: 'bundling', deploymentId: 'dep_1' },
+        data: { status: 'deploying', deploymentId: 'dep_1' },
       });
       await deploy({ flowId: 'cfg_1', wait: false });
       expect(mockPost).toHaveBeenCalledWith(
         '/api/projects/{projectId}/flows/{flowId}/deploy',
-        { params: { path: { projectId: 'proj_default', flowId: 'cfg_1' } } },
+        expect.objectContaining({
+          params: { path: { projectId: 'proj_default', flowId: 'cfg_1' } },
+          headers: { 'Idempotency-Key': expect.any(String) },
+        }),
       );
     });
 
@@ -78,7 +104,7 @@ describe('deploy', () => {
 
     it('streams deployment status via SSE when wait=true', async () => {
       mockPost.mockResolvedValue({
-        data: { status: 'bundling', deploymentId: 'dep_1' },
+        data: { status: 'deploying', deploymentId: 'dep_1' },
       });
 
       // SSE stream response
@@ -87,7 +113,7 @@ describe('deploy', () => {
           createSSEStreamBody([
             {
               event: 'status',
-              data: { status: 'bundling', substatus: 'building' },
+              data: { status: 'deploying', substatus: 'building' },
             },
             {
               event: 'status',
@@ -114,7 +140,7 @@ describe('deploy', () => {
       });
 
       expect(result).toMatchObject({ status: 'published' });
-      expect(statuses).toEqual(['bundling', 'published']);
+      expect(statuses).toEqual(['deploying', 'published']);
       expect(mockApiFetch).toHaveBeenCalledWith(
         '/api/projects/proj_default/deployments/dep_1/stream',
         expect.objectContaining({
@@ -129,7 +155,7 @@ describe('deploy', () => {
       mockGetFlow.mockResolvedValue(multiFlowContent as any);
       mockApiFetch.mockResolvedValue(
         new Response(
-          JSON.stringify({ status: 'bundling', deploymentId: 'dep_1' }),
+          JSON.stringify({ status: 'deploying', deploymentId: 'dep_1' }),
           {
             status: 200,
           },
@@ -144,7 +170,10 @@ describe('deploy', () => {
       });
       expect(mockApiFetch).toHaveBeenCalledWith(
         '/api/projects/proj_default/flows/cfg_1/settings/cfg_web/deploy',
-        { method: 'POST' },
+        expect.objectContaining({
+          method: 'POST',
+          headers: { 'Idempotency-Key': expect.any(String) },
+        }),
       );
     });
 
@@ -162,7 +191,7 @@ describe('deploy', () => {
       // First call: trigger deploy
       mockApiFetch.mockResolvedValueOnce(
         new Response(
-          JSON.stringify({ status: 'bundling', deploymentId: 'dep_1' }),
+          JSON.stringify({ status: 'deploying', deploymentId: 'dep_1' }),
           { status: 200 },
         ),
       );
@@ -172,7 +201,7 @@ describe('deploy', () => {
           createSSEStreamBody([
             {
               event: 'status',
-              data: { status: 'bundling', substatus: 'publishing' },
+              data: { status: 'deploying', substatus: 'publishing' },
             },
             {
               event: 'status',
@@ -211,7 +240,7 @@ describe('deploy', () => {
   describe('deploy() SSE error handling', () => {
     it('throws on non-ok SSE stream response', async () => {
       mockPost.mockResolvedValue({
-        data: { status: 'bundling', deploymentId: 'dep_1' },
+        data: { status: 'deploying', deploymentId: 'dep_1' },
       });
       mockApiFetch.mockResolvedValueOnce(
         new Response('Not found', { status: 404 }),
@@ -224,7 +253,7 @@ describe('deploy', () => {
 
     it('throws when stream ends without terminal status', async () => {
       mockPost.mockResolvedValue({
-        data: { status: 'bundling', deploymentId: 'dep_1' },
+        data: { status: 'deploying', deploymentId: 'dep_1' },
       });
 
       // Stream that closes without done event and no status events
@@ -247,7 +276,7 @@ describe('deploy', () => {
 
     it('calls onStatus with substatus', async () => {
       mockPost.mockResolvedValue({
-        data: { status: 'bundling', deploymentId: 'dep_1' },
+        data: { status: 'deploying', deploymentId: 'dep_1' },
       });
       mockApiFetch.mockResolvedValueOnce(
         new Response(
@@ -292,6 +321,161 @@ describe('deploy', () => {
     });
   });
 
+  describe('idempotency key per invocation', () => {
+    it('legacy POST sends a fresh unique Idempotency-Key per call', async () => {
+      mockPost.mockResolvedValue({
+        data: { status: 'deploying', deploymentId: 'dep_1' },
+      });
+
+      await deploy({ flowId: 'cfg_1', wait: false });
+      await deploy({ flowId: 'cfg_1', wait: false });
+
+      const firstKey = idempotencyKeyFromHeaders(mockPost.mock.calls[0]?.[1]);
+      const secondKey = idempotencyKeyFromHeaders(mockPost.mock.calls[1]?.[1]);
+
+      expect(firstKey).toEqual(expect.any(String));
+      expect(firstKey.length).toBeGreaterThan(0);
+      expect(secondKey).not.toBe(firstKey);
+    });
+
+    it('per-settings POST sends a fresh unique Idempotency-Key per call', async () => {
+      mockGetFlow.mockResolvedValue(multiFlowContent as any);
+      // Fresh Response per call: a Response body can only be read once.
+      mockApiFetch
+        .mockResolvedValueOnce(
+          new Response(
+            JSON.stringify({ status: 'deploying', deploymentId: 'dep_1' }),
+            { status: 200 },
+          ),
+        )
+        .mockResolvedValueOnce(
+          new Response(
+            JSON.stringify({ status: 'deploying', deploymentId: 'dep_2' }),
+            { status: 200 },
+          ),
+        );
+
+      await deploy({ flowId: 'cfg_1', flowName: 'web', wait: false });
+      await deploy({ flowId: 'cfg_1', flowName: 'web', wait: false });
+
+      const firstKey = idempotencyKeyFromHeaders(
+        mockApiFetch.mock.calls[0]?.[1],
+      );
+      const secondKey = idempotencyKeyFromHeaders(
+        mockApiFetch.mock.calls[1]?.[1],
+      );
+
+      expect(firstKey).toEqual(expect.any(String));
+      expect(firstKey.length).toBeGreaterThan(0);
+      expect(secondKey).not.toBe(firstKey);
+    });
+  });
+
+  describe('default wait budget', () => {
+    it('defaults to 12 minutes so a healthy long server deploy is not aborted', () => {
+      // Server deploy budget is 10 minutes; the client default must exceed it
+      // so a slow-but-healthy deploy is never aborted client-side and read as
+      // a CI failure.
+      expect(DEFAULT_DEPLOY_WAIT_MS).toBe(12 * 60 * 1000);
+    });
+
+    it('defaults the stream timeout to cover the full server deploy budget', async () => {
+      const body = new ReadableStream({
+        start(controller) {
+          controller.close();
+        },
+      });
+      mockApiFetch.mockResolvedValueOnce(
+        new Response(body, {
+          status: 200,
+          headers: { 'Content-Type': 'text/event-stream' },
+        }),
+      );
+
+      // Stream ends without terminal status, so it rejects, but the signal
+      // passed to apiFetch must carry the 12-minute default budget.
+      await expect(
+        streamDeploymentStatus('proj_default', 'dep_1', {}),
+      ).rejects.toThrow('Stream ended without terminal status');
+
+      const signal = signalFromInit(mockApiFetch.mock.calls[0]?.[1]);
+      expect(signal).toBeInstanceOf(AbortSignal);
+      expect(signal?.aborted).toBe(false);
+    });
+
+    it('honors an explicit timeout override (the --wait flag)', async () => {
+      const body = new ReadableStream({
+        start(controller) {
+          controller.close();
+        },
+      });
+      mockApiFetch.mockResolvedValueOnce(
+        new Response(body, {
+          status: 200,
+          headers: { 'Content-Type': 'text/event-stream' },
+        }),
+      );
+
+      const explicitSignal = AbortSignal.timeout(30_000);
+      await expect(
+        streamDeploymentStatus('proj_default', 'dep_1', {
+          signal: explicitSignal,
+        }),
+      ).rejects.toThrow('Stream ended without terminal status');
+
+      const signal = signalFromInit(mockApiFetch.mock.calls[0]?.[1]);
+      expect(signal).toBe(explicitSignal);
+    });
+  });
+
+  describe('renderStatusLabel()', () => {
+    // The server's deployment status enum is
+    // ['idle', 'deploying', 'published', 'active', 'stopped', 'failed'] and the
+    // stream emits these (status, substatus) pairs (see the app's
+    // emitDeploymentUpdate calls and the snapshot route). `bundling` is never a
+    // status the stream emits; the build phase is `deploying`/`building`.
+    const emitted: Array<[string, string | null]> = [
+      ['deploying', 'building'],
+      ['deploying', 'publishing'],
+      ['deploying', 'provisioning'],
+      ['deploying', 'starting'],
+      ['deploying', null],
+      ['published', null],
+      ['active', null],
+      ['failed', null],
+      ['stopped', null],
+    ];
+
+    it.each(emitted)(
+      'maps emitted status %s/%s to a non-empty human label',
+      (status, substatus) => {
+        const label = renderStatusLabel(status, substatus);
+        expect(label.length).toBeGreaterThan(0);
+        // Must never echo the raw enum token as the whole label for a status we
+        // claim to recognize; it should be a readable phrase.
+        expect(label).not.toBe(status);
+      },
+    );
+
+    it('falls back to the substatus-less label when only the substatus is unknown', () => {
+      expect(renderStatusLabel('deploying', 'unknown-substatus')).toBe(
+        renderStatusLabel('deploying', null),
+      );
+    });
+
+    it('renders a safe fallback for an unknown status instead of nothing', () => {
+      const label = renderStatusLabel('totally-unknown', null);
+      expect(label.length).toBeGreaterThan(0);
+      expect(label).toContain('totally-unknown');
+    });
+
+    it('renders a safe fallback for an unknown status with substatus', () => {
+      const label = renderStatusLabel('totally-unknown', 'weird');
+      expect(label.length).toBeGreaterThan(0);
+      expect(label).toContain('totally-unknown');
+    });
+  });
+
   describe('getDeployment()', () => {
     it('without flowName calls typed GET route', async () => {
       mockGet.mockResolvedValue({

packages/cli/src/__tests__/unit/commands/deployments.test.ts

@@ -6,6 +6,7 @@ import {
   createDeployment,
   deleteDeployment,
   deleteDeploymentByFlowId,
+  createDeployCommand,
   DeploymentAmbiguityError,
 } from '../../../commands/deployments/index.js';
 
@@ -364,4 +365,54 @@ describe('deployments', () => {
       ).rejects.toThrow('No deployments found for flow flow_abc');
     });
   });
+
+  describe('createDeployCommand human-readable output', () => {
+    let logSpy: jest.SpyInstance;
+
+    beforeEach(() => {
+      logSpy = jest.spyOn(console, 'log').mockImplementation(() => undefined);
+    });
+
+    afterEach(() => {
+      logSpy.mockRestore();
+    });
+
+    const renderedLines = (): string =>
+      logSpy.mock.calls.map(String).join('\n');
+
+    it('never prints undefined or a raw placeholder, and points at minting a real deploy token', async () => {
+      mockApiFetch
+        .mockResolvedValueOnce(
+          new Response(
+            JSON.stringify({
+              config: { flows: { default: { server: {} } } },
+            }),
+            { status: 200 },
+          ),
+        )
+        .mockResolvedValueOnce(
+          new Response(
+            JSON.stringify({
+              id: 'dep_a1b2c3d4',
+              slug: 'k7m2x9p4q1w8',
+              type: 'server',
+            }),
+            { status: 201 },
+          ),
+        );
+
+      await createDeployCommand('cfg_remoteflow', {
+        project: 'proj_123',
+        flow: 'default',
+      });
+
+      const output = renderedLines();
+      // No undefined interpolation, and no bare `=<placeholder>` in the docker run.
+      expect(output).not.toContain('undefined');
+      expect(output).not.toContain('WALKEROS_DEPLOY_TOKEN=<');
+      // The instruction must point the user at minting a real deploy token.
+      expect(output.toLowerCase()).toContain('deploy token');
+      expect(output).toContain('WALKEROS_DEPLOY_TOKEN');
+    });
+  });
 });