fix(deployer): address CodeRabbit review feedback

Harden the deployer tool and CLI against the issues raised in the #86
review. Each fix verified against current code and covered by tests.

* cli/logger: route `--json` pino logs to STDERR (fd 2) so STDOUT
  carries only the single result object.
* cli/prompt: write the passphrase prompt + newline to STDERR, and
  settle the promise on stdin `end`/`error` so non-interactive input
  (closed/newline-less pipe) can no longer hang the CLI.
* config/schema: mark the file/module ref union members `.strict()`
  so an ambiguous `{ file, module }` is rejected instead of silently
  dropping a key.
* deployments: write the ledger atomically (temp file + rename) so a
  crash mid-write can't truncate `*.json`.
* loaders/artifact: bind compiled assets to `dirname(entry)` rather
  than a hardcoded `contract/`, fixing the top-level-`index.js` case.
* loaders/constructor-meta: allow an empty named-args object for a
  no-arg constructor (resolves to `[]`); `reorderNamedArgs` still
  rejects unexpected keys.
* loaders/contract-resolve: use `path.isAbsolute` instead of
  `startsWith('/')` for OS-correct absolute-path detection.
* providers/network: drop `mainnet` from the allow-list while the
  deployer is testnet/preview-only.
* providers/proof-server: reject partial-numeric and out-of-range
  `PROOF_SERVER_PORT` values (full `\d+` match + 1..65535 bounds).
* wallet/handler: use `path.dirname`/`basename` instead of manual
  `/` splitting so cache paths work on Windows.
* wallet/keystore: validate shape in `fromJSON` and throw `WalletError`
  (not a raw `TypeError`) on malformed keystore JSON.

Refs: #86

Author: 0xisk

packages/cli/src/logger.ts

@@ -3,10 +3,11 @@ import { join } from 'node:path';
 import pino, { type Logger } from 'pino';
 
 /**
- * Pino factory for the three CLI modes: `--json` (raw JSON, no transports),
- * default (pretty `info+`), `--verbose` (pretty `info+` to stdout AND
- * `debug+` mirrored to `.compact/logs/<ts>.log` so the transcript survives
- * spinner overwrites).
+ * Pino factory for the three CLI modes: `--json` (raw JSON to STDERR, no
+ * transports — STDOUT is reserved for the single result object), default
+ * (pretty `info+`), `--verbose` (pretty `info+` to stdout AND `debug+`
+ * mirrored to `.compact/logs/<ts>.log` so the transcript survives spinner
+ * overwrites).
  */
 export interface CreateLoggerOptions {
   verbose: boolean;
@@ -16,7 +17,11 @@ export interface CreateLoggerOptions {
 
 export function createLogger(opts: CreateLoggerOptions): Logger {
   if (opts.json) {
-    return pino({ level: opts.verbose ? 'debug' : 'info' });
+    // fd 2 = STDERR; keeps STDOUT carrying only the final JSON result.
+    return pino(
+      { level: opts.verbose ? 'debug' : 'info' },
+      pino.destination(2),
+    );
   }
 
   if (opts.verbose) {

packages/cli/src/prompt.ts

@@ -1,8 +1,12 @@
-import { stdin, stdout } from 'node:process';
+import { stderr, stdin } from 'node:process';
 
-/** Prompt for a keystore passphrase with terminal echo suppressed; falls back to plain line-read off a TTY. */
+/**
+ * Prompt for a keystore passphrase with terminal echo suppressed; falls back
+ * to plain line-read off a TTY. Prompt text and the trailing newline go to
+ * STDERR so `--json` (and any piped) callers keep a clean single-object STDOUT.
+ */
 export async function promptPassphrase(label: string): Promise<string> {
-  stdout.write(`Passphrase for ${label}: `);
+  stderr.write(`Passphrase for ${label}: `);
   return readMaskedLine();
 }
 
@@ -15,7 +19,23 @@ function readMaskedLine(): Promise<string> {
       if (isTTY) stdin.setRawMode(false);
       stdin.pause();
       stdin.removeListener('data', onData);
-      stdout.write('\n');
+      stdin.removeListener('end', onEnd);
+      stdin.removeListener('error', onError);
+      stderr.write('\n');
+    };
+
+    // Without these, a non-interactive stdin that closes without a trailing
+    // newline (e.g. piped input, or a closed pipe) would never settle the
+    // promise and the CLI would hang.
+    const onEnd = () => {
+      cleanup();
+      if (buffer.length > 0) resolveFn(buffer);
+      else rejectFn(new Error('Aborted'));
+    };
+
+    const onError = (err: Error) => {
+      cleanup();
+      rejectFn(err);
     };
 
     const onData = (chunk: Buffer) => {
@@ -44,5 +64,7 @@ function readMaskedLine(): Promise<string> {
     stdin.resume();
     stdin.setEncoding('utf8');
     stdin.on('data', onData);
+    stdin.on('end', onEnd);
+    stdin.on('error', onError);
   });
 }

packages/cli/test/logger.test.ts

@@ -1,7 +1,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
-const { mockMkdirSync, mockPino, mockTransport, fakeLogger } = vi.hoisted(
-  () => {
+const { mockMkdirSync, mockPino, mockTransport, mockDestination, fakeLogger } =
+  vi.hoisted(() => {
     const fakeLogger = {
       trace: vi.fn(),
       debug: vi.fn(),
@@ -15,10 +15,10 @@ const { mockMkdirSync, mockPino, mockTransport, fakeLogger } = vi.hoisted(
       mockMkdirSync: vi.fn(),
       mockPino: vi.fn(() => fakeLogger),
       mockTransport: vi.fn((cfg: unknown) => ({ __transport: cfg })),
+      mockDestination: vi.fn((fd: number) => ({ __destination: fd })),
       fakeLogger,
     };
-  },
-);
+  });
 
 vi.mock('node:fs', async (importOriginal) => {
   const actual = await importOriginal<typeof import('node:fs')>();
@@ -29,6 +29,8 @@ vi.mock('pino', () => {
   const pinoFn = (...args: unknown[]) => mockPino(...(args as [])) as unknown;
   (pinoFn as unknown as { transport: typeof mockTransport }).transport =
     mockTransport;
+  (pinoFn as unknown as { destination: typeof mockDestination }).destination =
+    mockDestination;
   return { default: pinoFn };
 });
 
@@ -44,20 +46,29 @@ describe('createLogger', () => {
   });
 
   describe('json mode', () => {
-    it('should return logger at info level when verbose false', () => {
+    it('should return logger at info level routed to STDERR when verbose false', () => {
       const logger = createLogger({ verbose: false, json: true });
 
       expect(mockPino).toHaveBeenCalledTimes(1);
-      expect(mockPino).toHaveBeenCalledWith({ level: 'info' });
+      // fd 2 = STDERR; STDOUT stays reserved for the single JSON result.
+      expect(mockDestination).toHaveBeenCalledWith(2);
+      expect(mockPino).toHaveBeenCalledWith(
+        { level: 'info' },
+        { __destination: 2 },
+      );
       expect(mockTransport).not.toHaveBeenCalled();
       expect(mockMkdirSync).not.toHaveBeenCalled();
       expect(logger).toBe(fakeLogger);
     });
 
-    it('should return logger at debug level when verbose true', () => {
+    it('should return logger at debug level routed to STDERR when verbose true', () => {
       const logger = createLogger({ verbose: true, json: true });
 
-      expect(mockPino).toHaveBeenCalledWith({ level: 'debug' });
+      expect(mockDestination).toHaveBeenCalledWith(2);
+      expect(mockPino).toHaveBeenCalledWith(
+        { level: 'debug' },
+        { __destination: 2 },
+      );
       expect(mockTransport).not.toHaveBeenCalled();
       expect(mockMkdirSync).not.toHaveBeenCalled();
       expect(logger).toBe(fakeLogger);

packages/cli/test/prompt.test.ts

@@ -1,7 +1,7 @@
 import type { EventEmitter } from 'node:events';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
-const { mockStdin, mockStdout } = await vi.hoisted(async () => {
+const { mockStdin, mockStderr } = await vi.hoisted(async () => {
   const { EventEmitter } = await import('node:events');
   type FakeStdin = InstanceType<typeof EventEmitter> & {
     isTTY: boolean;
@@ -21,13 +21,13 @@ const { mockStdin, mockStdout } = await vi.hoisted(async () => {
     stdin,
   ) as FakeStdin['removeListener'];
 
-  const stdout = { write: vi.fn() };
-  return { mockStdin: stdin, mockStdout: stdout };
+  const stderr = { write: vi.fn() };
+  return { mockStdin: stdin, mockStderr: stderr };
 });
 
 vi.mock('node:process', () => ({
   stdin: mockStdin,
-  stdout: mockStdout,
+  stderr: mockStderr,
 }));
 
 import { promptPassphrase } from '../src/prompt.ts';
@@ -39,7 +39,7 @@ function resetStdin(opts: { tty: boolean } = { tty: true }): void {
   (mockStdin.pause as ReturnType<typeof vi.fn>).mockClear();
   (mockStdin.resume as ReturnType<typeof vi.fn>).mockClear();
   (mockStdin.setEncoding as ReturnType<typeof vi.fn>).mockClear();
-  mockStdout.write.mockClear();
+  mockStderr.write.mockClear();
 }
 
 describe('promptPassphrase', () => {
@@ -57,7 +57,7 @@ describe('promptPassphrase', () => {
       mockStdin.emit('data', Buffer.from('x\n'));
       await promise;
 
-      expect(mockStdout.write).toHaveBeenCalledWith(
+      expect(mockStderr.write).toHaveBeenCalledWith(
         'Passphrase for Alice keystore: ',
       );
       expect(mockStdin.setRawMode).toHaveBeenCalledWith(true);
@@ -86,7 +86,7 @@ describe('promptPassphrase', () => {
       expect(pp).toBe('hunter2');
       expect(mockStdin.setRawMode).toHaveBeenLastCalledWith(false);
       expect(mockStdin.pause).toHaveBeenCalled();
-      expect(mockStdout.write).toHaveBeenLastCalledWith('\n');
+      expect(mockStderr.write).toHaveBeenLastCalledWith('\n');
     });
 
     it('should resolve on LF (0x0a)', async () => {
@@ -147,4 +147,25 @@ describe('promptPassphrase', () => {
       await expect(promise).rejects.toThrow('Aborted');
     });
   });
+
+  describe('stream close path', () => {
+    it('should reject with "Aborted" when stdin ends with an empty buffer', async () => {
+      const promise = promptPassphrase('label');
+      mockStdin.emit('end');
+      await expect(promise).rejects.toThrow('Aborted');
+    });
+
+    it('should resolve with the buffer when stdin ends without a trailing newline', async () => {
+      const promise = promptPassphrase('label');
+      mockStdin.emit('data', Buffer.from('piped-secret'));
+      mockStdin.emit('end');
+      await expect(promise).resolves.toBe('piped-secret');
+    });
+
+    it('should reject when stdin emits an error', async () => {
+      const promise = promptPassphrase('label');
+      mockStdin.emit('error', new Error('stream boom'));
+      await expect(promise).rejects.toThrow('stream boom');
+    });
+  });
 });

packages/deployer/src/config/schema.test.ts

@@ -208,6 +208,20 @@ describe('configSchema — contract args', () => {
       export: 'default',
     });
   });
+
+  it('should reject an ambiguous ref carrying both file and module', () => {
+    expect(() =>
+      configSchema.parse({
+        ...baseConfig,
+        contracts: {
+          Counter: {
+            ...validContract,
+            args: { file: 'args.json', module: 'args.ts' },
+          },
+        },
+      }),
+    ).toThrow();
+  });
 });
 
 describe('configSchema — required fields', () => {

packages/deployer/src/config/schema.ts

@@ -40,11 +40,13 @@ const walletObjectSchema = z.object({
 });
 const walletSchema = walletObjectSchema.optional();
 
-const fileRefSchema = z.object({ file: z.string().min(1) });
-const moduleRefSchema = z.object({
-  module: z.string().min(1),
-  export: z.string().default('default'),
-});
+const fileRefSchema = z.object({ file: z.string().min(1) }).strict();
+const moduleRefSchema = z
+  .object({
+    module: z.string().min(1),
+    export: z.string().default('default'),
+  })
+  .strict();
 const fileOrModuleRefSchema = z.union([fileRefSchema, moduleRefSchema]);
 
 const argsSchema = z.union([z.array(z.unknown()), fileOrModuleRefSchema]);

packages/deployer/src/deployments.ts

@@ -1,5 +1,6 @@
+import { randomUUID } from 'node:crypto';
 import { existsSync } from 'node:fs';
-import { mkdir, readFile, writeFile } from 'node:fs/promises';
+import { mkdir, readFile, rename, writeFile } from 'node:fs/promises';
 import { dirname, isAbsolute, resolve } from 'node:path';
 
 /**
@@ -108,6 +109,13 @@ async function readJson<T>(path: string, fallback: T): Promise<T> {
   return JSON.parse(raw) as T;
 }
 
+// Write atomically: a crash mid-write would otherwise leave a truncated
+// `*.json`, breaking subsequent reads and losing durable deploy state.
+// Write to a sibling temp file, then rename it into place (atomic on the
+// same filesystem).
 async function writeJson(path: string, value: unknown): Promise<void> {
-  await writeFile(path, `${JSON.stringify(value, null, 2)}\n`);
+  await mkdir(dirname(path), { recursive: true });
+  const tmp = `${path}.${randomUUID()}.tmp`;
+  await writeFile(tmp, `${JSON.stringify(value, null, 2)}\n`);
+  await rename(tmp, path);
 }

packages/deployer/src/loaders/artifact.ts

@@ -1,5 +1,5 @@
 import { existsSync, readdirSync } from 'node:fs';
-import { isAbsolute, resolve } from 'node:path';
+import { dirname, isAbsolute, resolve } from 'node:path';
 import { CompiledContract, type Contract } from '@midnight-ntwrk/compact-js';
 import type { Types } from 'effect';
 import {
@@ -62,13 +62,16 @@ export class Artifact {
       throw new ArtifactNotFoundError(artifactPath);
     }
 
-    const contractDir = resolve(artifactPath, 'contract');
-    const entry = findEntry(contractDir, artifactPath);
+    const entry = findEntry(resolve(artifactPath, 'contract'), artifactPath);
     if (!entry) {
       throw new ArtifactNotFoundError(
         `${artifactPath} (no contract/index.{cjs,js} or index.{cjs,js} found)`,
       );
     }
+    // Bind compiled assets to the directory the entry actually lives in:
+    // `findEntry` may resolve a top-level `index.{cjs,js}` rather than the
+    // `contract/` subdir, in which case the hardcoded path would be wrong.
+    const contractDir = dirname(entry);
 
     const keysDir = resolve(artifactPath, 'keys');
     const zkirDir = resolve(artifactPath, 'zkir');

packages/deployer/src/loaders/constructor-meta.test.ts

@@ -72,4 +72,14 @@ describe('reorderNamedArgs', () => {
       /unknown constructor parameter\(s\): x/,
     );
   });
+
+  it('accepts an empty named object for a no-arg constructor', () => {
+    expect(reorderNamedArgs({}, [])).toEqual([]);
+  });
+
+  it('rejects a non-empty named object for a no-arg constructor', () => {
+    expect(() => reorderNamedArgs({ a: 1 }, [])).toThrow(
+      /unknown constructor parameter\(s\): a/,
+    );
+  });
 });

packages/deployer/src/loaders/constructor-meta.ts

@@ -17,13 +17,10 @@ export function loadConstructorParamNames(artifactPath: string): string[] {
     );
   }
   const source = readFileSync(dtsPath, 'utf8');
-  const names = parseConstructorParamNames(source);
-  if (names.length === 0) {
-    throw new ConfigError(
-      `Contract ${artifactPath} has a no-arg constructor; named args object should be empty`,
-    );
-  }
-  return names;
+  // A no-arg constructor yields `[]`. `reorderNamedArgs` then accepts an
+  // empty named-args object and rejects any unexpected keys, so we don't
+  // need to special-case the empty result here.
+  return parseConstructorParamNames(source);
 }
 
 /** Reorders a named-object args record into a positional tuple. */