Add comprehensive test coverage (67 tests)

.github/workflows/ci.yml

@@ -6,6 +6,10 @@ on:
   pull_request:
     branches: [main]
 
+concurrency:
+  group: ci-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   test:
     runs-on: ubuntu-latest

lib/node.js

@@ -383,13 +383,21 @@ class SymNode extends EventEmitter {
       try { this._dnssdBrowse.kill(); } catch {}
       this._dnssdBrowse = null;
     }
+    if (this._browser) {
+      try { this._browser.stop(); } catch {}
+      this._browser = null;
+    }
     if (this._bonjour) {
       try { this._bonjour.destroy(); } catch {}
       this._bonjour = null;
     }
 
     if (this._server) {
-      this._server.close();
+      await new Promise((resolve) => {
+        this._server.close(() => resolve());
+        // Timeout in case server.close() hangs
+        setTimeout(resolve, 1000);
+      });
       this._server = null;
     }
 
@@ -724,13 +732,23 @@ class SymNode extends EventEmitter {
     ], { stdio: 'ignore' });
 
     this._dnssdRegister.on('error', (err) => {
-      // Fallback to JavaScript mDNS if dns-sd not available (Linux)
+      // dns-sd not available (Linux, containers) — fallback to JavaScript mDNS
       this._log(`dns-sd not available, falling back to bonjour-service: ${err.message}`);
+      this._dnssdRegister = null;
+      // Kill browse process too — it won't work without dns-sd
+      if (this._dnssdBrowse) {
+        try { this._dnssdBrowse.kill(); } catch {}
+        this._dnssdBrowse = null;
+      }
       this._startBonjourFallback();
     });
 
-    // Browse for peers via dns-sd
+    // Browse for peers via dns-sd (only useful if dns-sd is available)
     this._dnssdBrowse = spawn('dns-sd', ['-B', '_sym._tcp'], { stdio: ['ignore', 'pipe', 'ignore'] });
+    this._dnssdBrowse.on('error', () => {
+      // Handled by register error above — just prevent unhandled error crash
+      this._dnssdBrowse = null;
+    });
     let browseBuffer = '';
     this._dnssdBrowse.stdout.on('data', (data) => {
       browseBuffer += data.toString();

tests/config.test.js

@@ -0,0 +1,162 @@
+'use strict';
+
+const { describe, it, before, after } = require('node:test');
+const assert = require('node:assert');
+const fs = require('fs');
+const path = require('path');
+const os = require('os');
+const {
+  SYM_DIR, NODES_DIR, ensureDir, nodeDir,
+  uuidv7, validateName, generateSigningKeyPair, loadOrCreateIdentity, log,
+} = require('../lib/config');
+
+describe('uuidv7', () => {
+  it('should return lowercase 8-4-4-4-12 hex format', () => {
+    const id = uuidv7();
+    assert.match(id, /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/);
+  });
+
+  it('should have version nibble 7', () => {
+    const id = uuidv7();
+    assert.strictEqual(id[14], '7');
+  });
+
+  it('should have variant bits 10xx', () => {
+    const id = uuidv7();
+    const variantChar = id[19];
+    assert.ok('89ab'.includes(variantChar), `variant char should be 8/9/a/b, got '${variantChar}'`);
+  });
+
+  it('should produce unique IDs', () => {
+    const ids = new Set(Array.from({ length: 100 }, () => uuidv7()));
+    assert.strictEqual(ids.size, 100);
+  });
+
+  it('should be time-ordered (monotonic timestamps)', () => {
+    const a = uuidv7();
+    const b = uuidv7();
+    // Extract timestamp hex (first 12 chars without hyphen)
+    const tsA = a.replace(/-/g, '').slice(0, 12);
+    const tsB = b.replace(/-/g, '').slice(0, 12);
+    assert.ok(tsB >= tsA, `${tsB} should be >= ${tsA}`);
+  });
+});
+
+describe('validateName', () => {
+  it('should accept valid names', () => {
+    assert.doesNotThrow(() => validateName('claude-code'));
+    assert.doesNotThrow(() => validateName('a'));
+    assert.doesNotThrow(() => validateName('my-node-123'));
+  });
+
+  it('should accept unicode names within 64 bytes', () => {
+    assert.doesNotThrow(() => validateName('日本語'));
+  });
+
+  it('should reject empty string', () => {
+    assert.throws(() => validateName(''), /non-empty/);
+  });
+
+  it('should reject names > 64 bytes', () => {
+    assert.throws(() => validateName('a'.repeat(65)), /1-64 bytes/);
+  });
+
+  it('should reject control characters', () => {
+    assert.throws(() => validateName('test\x00node'), /control/);
+    assert.throws(() => validateName('test\nnewline'), /control/);
+    assert.throws(() => validateName('test\ttab'), /control/);
+  });
+
+  it('should reject non-string input', () => {
+    assert.throws(() => validateName(null), /non-empty/);
+    assert.throws(() => validateName(undefined), /non-empty/);
+  });
+});
+
+describe('generateSigningKeyPair', () => {
+  it('should return 32-byte raw Buffer keys', () => {
+    const kp = generateSigningKeyPair();
+    assert.ok(Buffer.isBuffer(kp.publicKey), 'publicKey should be Buffer');
+    assert.ok(Buffer.isBuffer(kp.privateKey), 'privateKey should be Buffer');
+    assert.strictEqual(kp.publicKey.length, 32);
+    assert.strictEqual(kp.privateKey.length, 32);
+  });
+
+  it('should produce different keys each call', () => {
+    const a = generateSigningKeyPair();
+    const b = generateSigningKeyPair();
+    assert.ok(!a.publicKey.equals(b.publicKey), 'different calls should produce different keys');
+  });
+
+  it('should produce base64url-safe strings when encoded', () => {
+    const kp = generateSigningKeyPair();
+    const encoded = kp.publicKey.toString('base64url');
+    assert.ok(!encoded.includes('+'), 'base64url should not contain +');
+    assert.ok(!encoded.includes('='), 'base64url should not contain =');
+    assert.ok(!encoded.includes('/'), 'base64url should not contain /');
+  });
+});
+
+describe('loadOrCreateIdentity', () => {
+  const testName = `test-identity-${Date.now()}`;
+
+  after(() => {
+    fs.rmSync(nodeDir(testName), { recursive: true, force: true });
+  });
+
+  it('should create new identity with UUID v7 and keypair', () => {
+    const id = loadOrCreateIdentity(testName);
+    assert.ok(id.nodeId, 'should have nodeId');
+    assert.strictEqual(id.nodeId[14], '7', 'new node should use UUID v7');
+    assert.strictEqual(id.name, testName);
+    assert.ok(id.hostname, 'should have hostname');
+    assert.ok(id.createdAt, 'should have createdAt');
+    assert.ok(id.publicKey, 'should have Ed25519 publicKey');
+    assert.ok(id.privateKey, 'should have Ed25519 privateKey');
+  });
+
+  it('should return same identity on second call', () => {
+    const a = loadOrCreateIdentity(testName);
+    const b = loadOrCreateIdentity(testName);
+    assert.strictEqual(a.nodeId, b.nodeId);
+    assert.strictEqual(a.publicKey, b.publicKey);
+  });
+
+  it('should migrate legacy identity without keypair', () => {
+    const legacyName = `test-legacy-${Date.now()}`;
+    const dir = nodeDir(legacyName);
+    ensureDir(dir);
+    const legacy = { nodeId: 'aaaaaaaa-bbbb-4ccc-dddd-eeeeeeeeeeee', name: legacyName, hostname: 'test', createdAt: Date.now() };
+    fs.writeFileSync(path.join(dir, 'identity.json'), JSON.stringify(legacy));
+
+    const id = loadOrCreateIdentity(legacyName);
+    assert.strictEqual(id.nodeId, legacy.nodeId, 'should preserve v4 nodeId');
+    assert.ok(id.publicKey, 'should add publicKey during migration');
+    assert.ok(id.privateKey, 'should add privateKey during migration');
+
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+});
+
+describe('ensureDir', () => {
+  it('should create nested directories', () => {
+    const dir = path.join(os.tmpdir(), `sym-test-${Date.now()}`, 'a', 'b');
+    ensureDir(dir);
+    assert.ok(fs.existsSync(dir));
+    fs.rmSync(path.join(os.tmpdir(), `sym-test-${Date.now()}`), { recursive: true, force: true });
+  });
+});
+
+describe('nodeDir', () => {
+  it('should return path under NODES_DIR', () => {
+    const dir = nodeDir('test-node');
+    assert.ok(dir.startsWith(NODES_DIR));
+    assert.ok(dir.endsWith('test-node'));
+  });
+});
+
+describe('log', () => {
+  it('should not throw', () => {
+    assert.doesNotThrow(() => log('test', 'hello'));
+  });
+});

tests/frame-parser.test.js

@@ -0,0 +1,125 @@
+'use strict';
+
+const { describe, it } = require('node:test');
+const assert = require('node:assert');
+const { EventEmitter } = require('events');
+const { FrameParser, sendFrame } = require('../lib/frame-parser');
+
+function encodeFrame(msg) {
+  const payload = Buffer.from(JSON.stringify(msg), 'utf8');
+  const header = Buffer.alloc(4);
+  header.writeUInt32BE(payload.length, 0);
+  return Buffer.concat([header, payload]);
+}
+
+describe('FrameParser', () => {
+  it('should parse a single frame', () => {
+    const parser = new FrameParser();
+    const messages = [];
+    parser.on('message', (m) => messages.push(m));
+
+    parser.feed(encodeFrame({ type: 'ping' }));
+    assert.strictEqual(messages.length, 1);
+    assert.strictEqual(messages[0].type, 'ping');
+  });
+
+  it('should parse multiple frames in one chunk', () => {
+    const parser = new FrameParser();
+    const messages = [];
+    parser.on('message', (m) => messages.push(m));
+
+    const buf = Buffer.concat([
+      encodeFrame({ type: 'ping' }),
+      encodeFrame({ type: 'pong' }),
+      encodeFrame({ type: 'handshake', nodeId: 'abc' }),
+    ]);
+    parser.feed(buf);
+    assert.strictEqual(messages.length, 3);
+    assert.strictEqual(messages[0].type, 'ping');
+    assert.strictEqual(messages[1].type, 'pong');
+    assert.strictEqual(messages[2].nodeId, 'abc');
+  });
+
+  it('should handle partial frames across chunks', () => {
+    const parser = new FrameParser();
+    const messages = [];
+    parser.on('message', (m) => messages.push(m));
+
+    const full = encodeFrame({ type: 'state-sync', h1: [1, 2, 3] });
+    // Split in the middle
+    const mid = Math.floor(full.length / 2);
+    parser.feed(full.subarray(0, mid));
+    assert.strictEqual(messages.length, 0, 'should not emit on partial data');
+    parser.feed(full.subarray(mid));
+    assert.strictEqual(messages.length, 1);
+    assert.deepStrictEqual(messages[0].h1, [1, 2, 3]);
+  });
+
+  it('should emit error for zero-length frame', () => {
+    const parser = new FrameParser();
+    const errors = [];
+    parser.on('error', (e) => errors.push(e));
+
+    const header = Buffer.alloc(4);
+    header.writeUInt32BE(0, 0);
+    parser.feed(header);
+    assert.strictEqual(errors.length, 1);
+    assert.match(errors[0].message, /Invalid frame length/);
+  });
+
+  it('should emit error for oversized frame', () => {
+    const parser = new FrameParser();
+    const errors = [];
+    parser.on('error', (e) => errors.push(e));
+
+    const header = Buffer.alloc(4);
+    header.writeUInt32BE(2 * 1024 * 1024, 0); // 2 MiB > 1 MiB limit
+    parser.feed(header);
+    assert.strictEqual(errors.length, 1);
+    assert.match(errors[0].message, /Invalid frame length/);
+  });
+
+  it('should emit error for invalid JSON', () => {
+    const parser = new FrameParser();
+    const errors = [];
+    parser.on('error', (e) => errors.push(e));
+
+    const payload = Buffer.from('not json{{{', 'utf8');
+    const header = Buffer.alloc(4);
+    header.writeUInt32BE(payload.length, 0);
+    parser.feed(Buffer.concat([header, payload]));
+    assert.strictEqual(errors.length, 1);
+    assert.match(errors[0].message, /Invalid JSON/);
+  });
+});
+
+describe('sendFrame', () => {
+  it('should write length-prefixed frame to socket', () => {
+    const chunks = [];
+    const mockSocket = { write: (data) => chunks.push(data) };
+    const result = sendFrame(mockSocket, { type: 'ping' });
+    assert.strictEqual(result, true);
+    assert.strictEqual(chunks.length, 1);
+
+    // Parse it back
+    const parser = new FrameParser();
+    const messages = [];
+    parser.on('message', (m) => messages.push(m));
+    parser.feed(chunks[0]);
+    assert.strictEqual(messages.length, 1);
+    assert.strictEqual(messages[0].type, 'ping');
+  });
+
+  it('should reject oversized payloads', () => {
+    const mockSocket = { write: () => {} };
+    const bigContent = 'x'.repeat(1024 * 1024 + 1);
+    const result = sendFrame(mockSocket, { data: bigContent });
+    assert.strictEqual(result, false);
+  });
+
+  it('should return false if socket.write throws', () => {
+    const mockSocket = { write: () => { throw new Error('broken pipe'); } };
+    const result = sendFrame(mockSocket, { type: 'ping' });
+    assert.strictEqual(result, false);
+  });
+});