fix: address CodeRabbit review findings on the v0.4.12 merge

Makefile

@@ -1,4 +1,4 @@
-.PHONY: all omnilock anyone-can-pay xudt spore ckb-js-vm nostr-lock
+.PHONY: all omnilock anyone-can-pay xudt spore ckb-js-vm nostr-lock pw-lock secp256k1_multisig_v2
 
 all: omnilock anyone-can-pay xudt spore ckb-js-vm nostr-lock pw-lock secp256k1_multisig_v2
 

README.md

@@ -83,6 +83,7 @@ Commands:
   transfer [options] [toAddress] [amountInCKB]  Transfer CKB tokens to address, only devnet and testnet
   transfer-all [options] [toAddress]            Transfer All CKB tokens to address, only devnet and testnet
   balance [options] [toAddress]                 Check account balance, only devnet and testnet
+  install <tool>                                Install a tool binary used by offckb (e.g. the native ckb-debugger)
   debugger                                      Port of the raw CKB Standalone Debugger
   status [options]                              Show ckb-tui status interface
   logs [options] [target]                       Show devnet logs: node (default), contract script debug output, miner, or RPC proxy events

src/tools/ckb-debugger-install.ts

@@ -351,9 +351,14 @@ export class CKBDebuggerInstaller {
 
     const binName = isWindows ? 'ckb-debugger.cmd' : 'ckb-debugger';
     const targetPath = path.join(path.dirname(offckbPath), binName);
-    // The marker distinguishes our own shim (and legacy v0.4.x fallback shims
-    // that `exec offckb debugger`) from a foreign file — e.g. a real binary a
-    // user installed via cargo — which must never be silently clobbered.
+    // The marker distinguishes our own shim from a foreign file — e.g. a real
+    // binary a user installed via cargo — which must never be silently
+    // clobbered. Legacy v0.4.x fallback shims are also upgraded in place, but
+    // only when the whole file is exactly the body offckb wrote back then (a
+    // shebang line plus `exec offckb debugger "$@"` on Unix, `@echo off` plus
+    // `offckb debugger %*` on Windows), line endings normalized and one
+    // trailing newline ignored; a mere mention of `offckb debugger`, or a
+    // legacy body with extra user content, is not enough to claim the file.
     const marker = isWindows ? '@rem offckb-managed ckb-debugger shim' : '# offckb-managed ckb-debugger shim';
     const content = isWindows
       ? `${marker}\r\n@echo off\r\n"${binaryPath}" %*\r\n`
@@ -362,7 +367,9 @@ export class CKBDebuggerInstaller {
     try {
       if (fs.existsSync(targetPath)) {
         const existing = fs.readFileSync(targetPath, 'utf8');
-        if (!existing.includes(marker) && !existing.includes('offckb debugger')) {
+        const legacyBody = isWindows ? '@echo off\noffckb debugger %*' : '#!/bin/sh\nexec offckb debugger "$@"';
+        const isLegacyShim = existing.replace(/\r\n/g, '\n').replace(/\n$/, '') === legacyBody;
+        if (!existing.includes(marker) && !isLegacyShim) {
           logger.warn(
             `A file already exists at ${targetPath} that was not created by offckb; leaving it untouched. ` +
               'Projects that call `ckb-debugger` directly will need it added to PATH manually.',

src/tools/proxy-events.ts

@@ -117,7 +117,15 @@ export function handleProxyRequestBody(reqData: string, ctx: ProxyEventContext):
         ctx.sink.warn('skipping malformed JSON-RPC batch member');
         continue;
       }
-      handleOneRequest(jsonRpcContent, ctx);
+      try {
+        handleOneRequest(jsonRpcContent, ctx);
+      } catch (error) {
+        // Thrown values are not necessarily Error instances (user code can
+        // throw anything), so normalize before logging; reading .message off
+        // a non-Error (e.g. null) would itself throw and abort the batch.
+        const message = error instanceof Error ? error.message : String(error);
+        ctx.sink.warn(`skipping JSON-RPC request event: ${message}`);
+      }
     }
   } catch (err) {
     ctx.sink.error('Error parsing JSON-RPC req content:', (err as Error).message);

tests/ckb-debugger-install.test.ts

@@ -253,6 +253,27 @@ describe('CKBDebuggerInstaller', () => {
     expect(fs.readFileSync(shimPath, 'utf-8')).toBe('#!/bin/sh\nexec /usr/bin/ckb-debugger "$@"\n');
   });
 
+  it('does not overwrite a user file that merely mentions offckb debugger', async () => {
+    const buffer = await buildTarGz('0.208.0');
+    mockRelease('0.208.0', buffer);
+    const shimPath = path.join(root, 'bin', 'ckb-debugger');
+    mockSpawnSync.mockImplementation((cmd: string) => {
+      if (cmd === 'which') {
+        return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' };
+      }
+      return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' };
+    });
+    fs.mkdirSync(path.dirname(shimPath), { recursive: true });
+    // A user wrapper that documents offckb debugger without being the exact
+    // v0.4.x fallback body must stay untouched.
+    const userScript = '#!/bin/sh\n# How to call: offckb debugger --help\n/usr/bin/ckb-debugger "$@"\n';
+    fs.writeFileSync(shimPath, userScript);
+
+    await CKBDebuggerInstaller.install();
+
+    expect(fs.readFileSync(shimPath, 'utf-8')).toBe(userScript);
+  });
+
   it('upgrades a legacy v0.4.x fallback shim', async () => {
     const buffer = await buildTarGz('0.208.0');
     mockRelease('0.208.0', buffer);
@@ -272,6 +293,72 @@ describe('CKBDebuggerInstaller', () => {
     expect(fs.readFileSync(shimPath, 'utf-8')).toContain(`exec "${binaryPath}" "$@"`);
   });
 
+  it('does not overwrite a legacy shim that has extra user content', async () => {
+    const buffer = await buildTarGz('0.208.0');
+    mockRelease('0.208.0', buffer);
+    const shimPath = path.join(root, 'bin', 'ckb-debugger');
+    mockSpawnSync.mockImplementation((cmd: string) => {
+      if (cmd === 'which') {
+        return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' };
+      }
+      return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' };
+    });
+    fs.mkdirSync(path.dirname(shimPath), { recursive: true });
+    // The complete legacy body plus one user line is not an offckb shim: the
+    // whole file must match exactly, so it stays untouched.
+    const userScript = '#!/bin/sh\nexec offckb debugger "$@"\n# custom wrapper\n';
+    fs.writeFileSync(shimPath, userScript);
+
+    await CKBDebuggerInstaller.install();
+
+    expect(fs.readFileSync(shimPath, 'utf-8')).toBe(userScript);
+  });
+
+  it('upgrades an exact legacy v0.4.x fallback shim on Windows', async () => {
+    mockHost('win32', 'x64');
+    const winBinaryPath = path.join(mockDirs.toolsRoot, 'ckb-debugger.exe');
+    fs.mkdirSync(path.dirname(winBinaryPath), { recursive: true });
+    fs.writeFileSync(winBinaryPath, fakeBinaryPayload('0.208.0'));
+    const shimPath = path.join(root, 'bin', 'ckb-debugger.cmd');
+    mockSpawnSync.mockImplementation((cmd: string) => {
+      if (cmd === 'where') {
+        return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' };
+      }
+      return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' };
+    });
+    fs.mkdirSync(path.dirname(shimPath), { recursive: true });
+    // CRLF line endings are normalized before the exact-body comparison.
+    fs.writeFileSync(shimPath, '@echo off\r\noffckb debugger %*\r\n');
+
+    const result = await CKBDebuggerInstaller.install();
+
+    expect(result.alreadyInstalled).toBe(true);
+    const upgraded = fs.readFileSync(shimPath, 'utf-8');
+    expect(upgraded).toContain('offckb-managed');
+    expect(upgraded).toContain(`"${winBinaryPath}" %*`);
+  });
+
+  it('does not overwrite a legacy shim with extra user content on Windows', async () => {
+    mockHost('win32', 'x64');
+    const winBinaryPath = path.join(mockDirs.toolsRoot, 'ckb-debugger.exe');
+    fs.mkdirSync(path.dirname(winBinaryPath), { recursive: true });
+    fs.writeFileSync(winBinaryPath, fakeBinaryPayload('0.208.0'));
+    const shimPath = path.join(root, 'bin', 'ckb-debugger.cmd');
+    mockSpawnSync.mockImplementation((cmd: string) => {
+      if (cmd === 'where') {
+        return { status: 0, stdout: `${path.join(root, 'bin', 'offckb')}\n`, stderr: '' };
+      }
+      return { status: 0, stdout: 'ckb-debugger 0.208.0\n', stderr: '' };
+    });
+    fs.mkdirSync(path.dirname(shimPath), { recursive: true });
+    const userScript = '@echo off\r\noffckb debugger %*\r\nREM custom wrapper\r\n';
+    fs.writeFileSync(shimPath, userScript);
+
+    await CKBDebuggerInstaller.install();
+
+    expect(fs.readFileSync(shimPath, 'utf-8')).toBe(userScript);
+  });
+
   it('throws when the platform has no prebuilt asset', async () => {
     mockHost('freebsd', 'x64');
     await expect(CKBDebuggerInstaller.install()).rejects.toThrow(/no prebuilt binary/);

tests/proxy-events.test.ts

@@ -98,6 +98,82 @@ describe('handleProxyRequestBody', () => {
     expect(content).toMatch(/send_transaction 0xhash/);
   });
 
+  it('isolates a failing batch member and still records a valid request after it', () => {
+    const ctx = makeCtx(transactionsPath);
+    // Simulate a member whose tx dump blows up mid-processing.
+    ctx.hashTransaction = jest.fn((tx: unknown) => {
+      if ((tx as { bad?: boolean }).bad) {
+        throw new Error('hash boom');
+      }
+      return '0xhash';
+    });
+    const goodTx = { cell_deps: [], inputs: [], outputs: [] };
+    handleProxyRequestBody(
+      JSON.stringify([
+        { jsonrpc: '2.0', id: 1, method: 'send_transaction', params: [{ bad: true }] },
+        { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [goodTx] },
+      ]),
+      ctx,
+    );
+    // The failing member is skipped with a warning instead of aborting the batch.
+    expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('hash boom'));
+    expect(ctx.sink.error).not.toHaveBeenCalled();
+    // The valid send_transaction after it is still recorded.
+    expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash'));
+    expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true);
+    const content = fs.readFileSync(ctx.events.filePath, 'utf8');
+    expect(content).toMatch(/send_transaction 0xhash/);
+  });
+
+  it('normalizes a non-Error throw when isolating a failing batch member', () => {
+    const ctx = makeCtx(transactionsPath);
+    ctx.hashTransaction = jest.fn((tx: unknown) => {
+      if ((tx as { bad?: boolean }).bad) {
+        throw 'hash boom';
+      }
+      return '0xhash';
+    });
+    const goodTx = { cell_deps: [], inputs: [], outputs: [] };
+    handleProxyRequestBody(
+      JSON.stringify([
+        { jsonrpc: '2.0', id: 1, method: 'send_transaction', params: [{ bad: true }] },
+        { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [goodTx] },
+      ]),
+      ctx,
+    );
+    // The thrown string is logged as-is instead of rendering as "undefined".
+    expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('hash boom'));
+    expect(ctx.sink.warn).not.toHaveBeenCalledWith(expect.stringContaining('undefined'));
+    expect(ctx.sink.error).not.toHaveBeenCalled();
+    // The valid send_transaction after it is still recorded.
+    expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash'));
+    expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true);
+  });
+
+  it('tolerates a null throw when isolating a failing batch member', () => {
+    const ctx = makeCtx(transactionsPath);
+    ctx.hashTransaction = jest.fn((tx: unknown) => {
+      if ((tx as { bad?: boolean }).bad) {
+        throw null;
+      }
+      return '0xhash';
+    });
+    const goodTx = { cell_deps: [], inputs: [], outputs: [] };
+    handleProxyRequestBody(
+      JSON.stringify([
+        { jsonrpc: '2.0', id: 1, method: 'send_transaction', params: [{ bad: true }] },
+        { jsonrpc: '2.0', id: 2, method: 'send_transaction', params: [goodTx] },
+      ]),
+      ctx,
+    );
+    // Reading .message off null would itself throw; the warning must not.
+    expect(ctx.sink.warn).toHaveBeenCalledWith(expect.stringContaining('null'));
+    expect(ctx.sink.error).not.toHaveBeenCalled();
+    // The valid send_transaction after it is still recorded.
+    expect(ctx.sink.info).toHaveBeenCalledWith(expect.stringContaining('0xhash'));
+    expect(fs.existsSync(path.join(transactionsPath, '0xhash.json'))).toBe(true);
+  });
+
   it('warns and skips the tx dump when send_transaction has no usable params', () => {
     const ctx = makeCtx(transactionsPath);
     handleProxyRequestBody(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'send_transaction' }), ctx);