fix: address PR review feedback

- Use it.each() for prettifySmallcaps tests
- Add --timeout option to redeem-url subcommand
- Fix stale isCapData JSDoc (slots: unknown[] → string[])
- Reject empty OCAP_HOME env var (treat as unset)
- Extract parseTimeoutMs with positive-integer validation + tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: rekmarks

packages/kernel-cli/src/app.ts

@@ -25,7 +25,7 @@ import { getServer } from './commands/serve.ts';
 import { watchDir } from './commands/watch.ts';
 import { defaultConfig } from './config.ts';
 import type { Config } from './config.ts';
-import { withTimeout } from './utils.ts';
+import { parseTimeoutMs, withTimeout } from './utils.ts';
 
 /**
  * Console transport that omits tags from output.
@@ -319,12 +319,11 @@ const yargsInstance = yargs(hideBin(process.argv))
               execArgs.push(String(args['params-json']));
             }
             await ensureDaemon(socketPath);
+            const timeoutMs = parseTimeoutMs(args.timeout);
             await handleDaemonExec(
               execArgs,
               socketPath,
-              typeof args.timeout === 'number' && args.timeout > 0
-                ? { timeoutMs: args.timeout * 1000 }
-                : {},
+              timeoutMs === undefined ? {} : { timeoutMs },
             );
           },
         )
@@ -338,13 +337,19 @@ const yargsInstance = yargs(hideBin(process.argv))
                 type: 'string',
                 demandOption: true,
               })
+              .option('timeout', {
+                describe: 'Read timeout in seconds (default: no timeout)',
+                type: 'number',
+              })
               .example(
                 '$0 daemon redeem-url ocap:abc123@12D3KooW...,/ip4/...',
                 'Redeem an OCAP URL',
               ),
           async (args) => {
             await ensureDaemon(socketPath);
-            await handleRedeemURL(args.url, socketPath);
+            await handleRedeemURL(args.url, socketPath, {
+              timeoutMs: parseTimeoutMs(args.timeout),
+            });
           },
         )
         .command(
@@ -413,9 +418,7 @@ const yargsInstance = yargs(hideBin(process.argv))
               args: parsedArgs,
               socketPath,
               raw: args.raw,
-              ...(typeof args.timeout === 'number' && args.timeout > 0
-                ? { timeoutMs: args.timeout * 1000 }
-                : {}),
+              timeoutMs: parseTimeoutMs(args.timeout),
             });
           },
         );

packages/kernel-cli/src/commands/daemon.ts

@@ -317,15 +317,19 @@ export async function handleDaemonExec(
  *
  * @param url - The OCAP URL to redeem.
  * @param socketPath - The daemon socket path.
+ * @param options - Additional options.
+ * @param options.timeoutMs - Read timeout in milliseconds.
  */
 export async function handleRedeemURL(
   url: string,
   socketPath: string,
+  { timeoutMs }: { timeoutMs?: number } = {},
 ): Promise<void> {
   const response = await sendCommand({
     socketPath,
     method: 'redeemOcapURL',
     params: { url },
+    ...(timeoutMs === undefined ? {} : { timeoutMs }),
   });
 
   if (isJsonRpcFailure(response)) {

packages/kernel-cli/src/utils.test.ts

@@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import {
   isErrorWithCode,
   isProcessAlive,
+  parseTimeoutMs,
   readPidFile,
   sendSignal,
   waitFor,
@@ -123,6 +124,28 @@ describe('utils', () => {
     });
   });
 
+  describe('parseTimeoutMs', () => {
+    it('returns undefined when value is undefined', () => {
+      expect(parseTimeoutMs(undefined)).toBeUndefined();
+    });
+
+    it.each([1, 5, 30, 100])(
+      'converts %i seconds to milliseconds',
+      (seconds) => {
+        expect(parseTimeoutMs(seconds)).toBe(seconds * 1000);
+      },
+    );
+
+    it.each([0, -1, -100, 1.5, 0.1, NaN, Infinity, -Infinity])(
+      'throws for invalid value %s',
+      (value) => {
+        expect(() => parseTimeoutMs(value)).toThrow(
+          '--timeout must be a positive integer',
+        );
+      },
+    );
+  });
+
   describe('waitFor', () => {
     beforeEach(() => {
       vi.useFakeTimers({ now: 0 });

packages/kernel-cli/src/utils.ts

@@ -1,5 +1,22 @@
 import { readFile } from 'node:fs/promises';
 
+/**
+ * Parse and validate a `--timeout` option value (seconds → milliseconds).
+ *
+ * @param value - The raw option value from yargs.
+ * @returns The timeout in milliseconds, or `undefined` if not provided.
+ * @throws If the value is not a positive integer.
+ */
+export function parseTimeoutMs(value: number | undefined): number | undefined {
+  if (value === undefined) {
+    return undefined;
+  }
+  if (!Number.isInteger(value) || value <= 0) {
+    throw new Error(`--timeout must be a positive integer, got: ${value}`);
+  }
+  return value * 1000;
+}
+
 /**
  * Check whether an unknown error is a Node.js system error with the given code.
  *

packages/kernel-utils/src/nodejs/ocap-home.ts

@@ -9,5 +9,9 @@ import { join } from 'node:path';
  */
 export function getOcapHome(): string {
   // eslint-disable-next-line n/no-process-env
-  return process.env.OCAP_HOME ?? join(homedir(), '.ocap');
+  const envValue = process.env.OCAP_HOME;
+  if (envValue) {
+    return envValue;
+  }
+  return join(homedir(), '.ocap');
 }

packages/kernel-utils/src/prettify-smallcaps.test.ts

@@ -3,40 +3,41 @@ import { describe, it, expect } from 'vitest';
 import { prettifySmallcaps } from './prettify-smallcaps.ts';
 
 describe('prettifySmallcaps', () => {
-  it('decodes a plain string', () => {
-    expect(prettifySmallcaps({ body: '#"0xca46b9"', slots: [] })).toBe(
-      '0xca46b9',
-    );
-  });
-
-  it('decodes a number', () => {
-    expect(prettifySmallcaps({ body: '#42', slots: [] })).toBe(42);
-  });
-
-  it('decodes null', () => {
-    expect(prettifySmallcaps({ body: '#null', slots: [] })).toBeNull();
-  });
-
-  it('decodes a boolean', () => {
-    expect(prettifySmallcaps({ body: '#true', slots: [] })).toBe(true);
-  });
-
-  it('replaces a remotable slot reference', () => {
-    expect(prettifySmallcaps({ body: '#"$0"', slots: ['ko12'] })).toBe(
-      '<ko12>',
-    );
-  });
-
-  it('replaces a remotable slot reference with interface name', () => {
-    expect(
-      prettifySmallcaps({ body: '#"$0.Alleged: MyObj"', slots: ['ko12'] }),
-    ).toBe('<ko12> (Alleged: MyObj)');
-  });
-
-  it('replaces a promise slot reference', () => {
-    expect(prettifySmallcaps({ body: '#"&0"', slots: ['kp42'] })).toBe(
-      '<kp42>',
-    );
+  it.each([
+    ['plain string', '#"0xca46b9"', [], '0xca46b9'],
+    ['number', '#42', [], 42],
+    ['null', '#null', [], null],
+    ['boolean', '#true', [], true],
+  ])('decodes a %s', (_label, body, slots, expected) => {
+    expect(prettifySmallcaps({ body, slots })).toStrictEqual(expected);
+  });
+
+  it.each([
+    ['remotable', '#"$0"', ['ko12'], '<ko12>'],
+    [
+      'remotable with iface',
+      '#"$0.Alleged: MyObj"',
+      ['ko12'],
+      '<ko12> (Alleged: MyObj)',
+    ],
+    ['promise', '#"&0"', ['kp42'], '<kp42>'],
+    ['missing slot index', '#"$5"', ['ko1'], '<?5>'],
+  ])('replaces a %s slot reference', (_label, body, slots, expected) => {
+    expect(prettifySmallcaps({ body, slots })).toBe(expected);
+  });
+
+  it.each([
+    ['escaped string (!)', '#"!$0"', '$0'],
+    ['double escape (!!)', '#"!!hello"', '!hello'],
+    ['non-negative bigint (+)', '#"+7"', '7n'],
+    ['negative bigint (-)', '#"-7"', '-7n'],
+    ['#undefined', '#"#undefined"', '[undefined]'],
+    ['#NaN', '#"#NaN"', '[NaN]'],
+    ['#Infinity', '#"#Infinity"', '[Infinity]'],
+    ['#-Infinity', '#"#-Infinity"', '[-Infinity]'],
+    ['symbol (%)', '#"%foo"', '[Symbol: foo]'],
+  ])('decodes %s', (_label, body, expected) => {
+    expect(prettifySmallcaps({ body, slots: [] })).toBe(expected);
   });
 
   it('decodes an object with mixed values', () => {
@@ -60,66 +61,12 @@ describe('prettifySmallcaps', () => {
     });
   });
 
-  it('strips smallcaps escape prefix from strings', () => {
-    expect(prettifySmallcaps({ body: '#"!$0"', slots: ['ko1'] })).toBe('$0');
-  });
-
-  it('strips double escape prefix', () => {
-    expect(prettifySmallcaps({ body: '#"!!hello"', slots: [] })).toBe('!hello');
-  });
-
   it('leaves non-slot strings unchanged', () => {
     expect(
       prettifySmallcaps({ body: '#{"text":"hello world"}', slots: [] }),
     ).toStrictEqual({ text: 'hello world' });
   });
 
-  it('handles missing slot index gracefully', () => {
-    expect(prettifySmallcaps({ body: '#"$5"', slots: ['ko1'] })).toBe('<?5>');
-  });
-
-  it('throws if body does not start with #', () => {
-    expect(() => prettifySmallcaps({ body: '"hello"', slots: [] })).toThrow(
-      "Expected body to start with '#'",
-    );
-  });
-
-  it('decodes a non-negative bigint', () => {
-    expect(prettifySmallcaps({ body: '#"+7"', slots: [] })).toBe('7n');
-  });
-
-  it('decodes a negative bigint', () => {
-    expect(prettifySmallcaps({ body: '#"-7"', slots: [] })).toBe('-7n');
-  });
-
-  it('decodes #undefined', () => {
-    expect(prettifySmallcaps({ body: '#"#undefined"', slots: [] })).toBe(
-      '[undefined]',
-    );
-  });
-
-  it('decodes #NaN', () => {
-    expect(prettifySmallcaps({ body: '#"#NaN"', slots: [] })).toBe('[NaN]');
-  });
-
-  it('decodes #Infinity', () => {
-    expect(prettifySmallcaps({ body: '#"#Infinity"', slots: [] })).toBe(
-      '[Infinity]',
-    );
-  });
-
-  it('decodes #-Infinity', () => {
-    expect(prettifySmallcaps({ body: '#"#-Infinity"', slots: [] })).toBe(
-      '[-Infinity]',
-    );
-  });
-
-  it('decodes a symbol', () => {
-    expect(prettifySmallcaps({ body: '#"%foo"', slots: [] })).toBe(
-      '[Symbol: foo]',
-    );
-  });
-
   it('decodes a tagged value', () => {
     expect(
       prettifySmallcaps({
@@ -129,22 +76,15 @@ describe('prettifySmallcaps', () => {
     ).toStrictEqual({ '[Tagged: match:any]': '[undefined]' });
   });
 
-  it('decodes an error', () => {
-    expect(
-      prettifySmallcaps({
-        body: '#{"#error":"boom","name":"TypeError"}',
-        slots: [],
-      }),
-    ).toBe('[TypeError: boom]');
-  });
-
-  it('decodes an error without name', () => {
-    expect(
-      prettifySmallcaps({
-        body: '#{"#error":"something broke"}',
-        slots: [],
-      }),
-    ).toBe('[Error: something broke]');
+  it.each([
+    ['with name', '#{"#error":"boom","name":"TypeError"}', '[TypeError: boom]'],
+    [
+      'without name',
+      '#{"#error":"something broke"}',
+      '[Error: something broke]',
+    ],
+  ])('decodes an error %s', (_label, body, expected) => {
+    expect(prettifySmallcaps({ body, slots: [] })).toBe(expected);
   });
 
   it('unescapes record keys', () => {
@@ -155,4 +95,10 @@ describe('prettifySmallcaps', () => {
       }),
     ).toStrictEqual({ '#foo': 'bar', normal: 'baz' });
   });
+
+  it('throws if body does not start with #', () => {
+    expect(() => prettifySmallcaps({ body: '"hello"', slots: [] })).toThrow(
+      "Expected body to start with '#'",
+    );
+  });
 });

packages/kernel-utils/src/types.ts

@@ -96,7 +96,7 @@ export const isJsonRpcMessage = (value: unknown): value is JsonRpcMessage =>
   is(value, JsonRpcMessageStruct);
 
 /**
- * Check whether a value has the shape of Endo CapData (`{ body: string, slots: unknown[] }`).
+ * Check whether a value has the shape of Endo CapData (`{ body: string, slots: string[] }`).
  *
  * @param value - The value to check.
  * @returns `true` when `value` looks like CapData.