Fix unattended-capture tooling defects (#69, #70, part of #68)

.github/workflows/metrics.yml

@@ -43,8 +43,16 @@ jobs:
           # --- traffic (needs an admin-read token in METRICS_TOKEN) ---
           views=null; uniques=null; clones=null; clone_uniques=null
           if [ -n "${TRAFFIC_TOKEN:-}" ]; then
-            v=$(GH_TOKEN="$TRAFFIC_TOKEN" gh api "repos/$REPO/traffic/views"  2>/dev/null || echo '{}')
-            c=$(GH_TOKEN="$TRAFFIC_TOKEN" gh api "repos/$REPO/traffic/clones" 2>/dev/null || echo '{}')
+            # A failed call (expired or under-scoped token) must not become a silent null row.
+            # Keep the snapshot, but surface the gap as a workflow warning.
+            if ! v=$(GH_TOKEN="$TRAFFIC_TOKEN" gh api "repos/$REPO/traffic/views" 2>/tmp/views.err); then
+              echo "::warning::traffic/views failed, recording null: $(head -c 300 /tmp/views.err)"
+              v='{}'
+            fi
+            if ! c=$(GH_TOKEN="$TRAFFIC_TOKEN" gh api "repos/$REPO/traffic/clones" 2>/tmp/clones.err); then
+              echo "::warning::traffic/clones failed, recording null: $(head -c 300 /tmp/clones.err)"
+              c='{}'
+            fi
             views=$(echo "$v" | jq '.count // null')
             uniques=$(echo "$v" | jq '.uniques // null')
             clones=$(echo "$c" | jq '.count // null')

extension-src/background.js

@@ -106,7 +106,11 @@ function handleMessage(message, _sender, sendResponse) {
     return false;
   }
   const controller = new AbortController();
-  const timer = setTimeout(() => controller.abort(), request.timeout || 30000);
+  let timedOut = false;
+  const timer = setTimeout(() => {
+    timedOut = true;
+    controller.abort();
+  }, request.timeout || 30000);
   const body = request.bodyBase64
     ? base64ToBytes(request.bodyBase64)
     : request.bodyText === null
@@ -137,7 +141,9 @@ function handleMessage(message, _sender, sendResponse) {
           : { responseText: new TextDecoder().decode(bytes) }),
       });
     })
-    .catch((error) => sendResponse({ ok: false, error: error.message }))
+    // compat.js routes /timeout/i to the userscript's ontimeout, so say so when our own
+    // timer aborted the fetch; the raw abort message ("This operation was aborted") never matches.
+    .catch((error) => sendResponse({ ok: false, error: timedOut ? 'timeout' : error.message }))
     .finally(() => clearTimeout(timer));
   return true;
 }
@@ -238,18 +244,38 @@ async function captureShare(request) {
   if (!tabId) {
     return { ok: false, error: 'no_tab', message: 'Could not open a capture tab.' };
   }
+  // chrome.tabs.sendMessage waits as long as the capture runs, so bound it ourselves. The
+  // finally below closes the capture window, which is what actually stops the capture.
+  let deadlineTimer = null;
+  const deadline = new Promise((resolve) => {
+    deadlineTimer = setTimeout(
+      () =>
+        resolve({
+          ok: false,
+          error: 'timeout',
+          message: `The capture did not finish within ${timeoutMs}ms.`,
+        }),
+      timeoutMs
+    );
+  });
   try {
-    await waitForContentScript(tabId, Math.min(timeoutMs, 90000));
-    const result = await chrome.tabs.sendMessage(tabId, {
-      type: 'sourcecapsule:controller',
-      version: 1,
-      action: 'capture-share',
-      value: { expiryDays: Number(request.expiryDays) || 0 },
-    });
+    const result = await Promise.race([
+      deadline,
+      (async () => {
+        await waitForContentScript(tabId, Math.min(timeoutMs, 90000));
+        return chrome.tabs.sendMessage(tabId, {
+          type: 'sourcecapsule:controller',
+          version: 1,
+          action: 'capture-share',
+          value: { expiryDays: Number(request.expiryDays) || 0 },
+        });
+      })(),
+    ]);
     return result || { ok: false, error: 'no_result', message: 'The capture returned nothing.' };
   } catch (error) {
     return { ok: false, error: 'capture_failed', message: error.message };
   } finally {
+    clearTimeout(deadlineTimer);
     try {
       await chrome.windows.remove(window.id);
     } catch (error) {

extension-src/page-bridge.js

@@ -198,5 +198,28 @@
     };
   }
 
+  // The userscript runs in the isolated world under the extension, where X's own
+  // history.pushState/replaceState calls never reach its patch. Tell it about in-app
+  // navigation from here (MAIN world) so per-page capture state is reset.
+  const history = window.history;
+  if (history) {
+    for (const method of ['pushState', 'replaceState']) {
+      const original = history[method];
+      if (typeof original !== 'function') continue;
+      history[method] = function (...args) {
+        const result = original.apply(this, args);
+        try {
+          window.postMessage(
+            { source: 'SourceCapsule:navigation', contractVersion: 1, type: 'navigate' },
+            window.location.origin
+          );
+        } catch {
+          // Observation must never interfere with X's router.
+        }
+        return result;
+      };
+    }
+  }
+
   announce();
 })();

native-host/sourcecapsule-host.mjs

@@ -7,8 +7,9 @@
 //   host --(native messaging framing over stdio)------>  extension service worker
 // and routes the reply back by request id.
 //
-// Exactly one host owns the pipe. A second instance fails to listen and exits, which
-// is also what enforces one-capture-at-a-time across Chrome profiles.
+// Exactly one host owns the pipe. A second instance cannot listen, stays attached to
+// its browser, and retries every few seconds, so it takes over when the owner exits.
+// The pending entry (one per in-flight capture) is what enforces one capture at a time.
 'use strict';
 
 import net from 'node:net';
@@ -23,6 +24,10 @@ const DEFAULT_PIPE = String.raw`\\.\pipe\sourcecapsule-capture`;
 const PIPE = process.env.SOURCECAPSULE_PIPE || DEFAULT_PIPE;
 const LOG = path.join(os.tmpdir(), 'sourcecapsule-native-host.log');
 const REQUEST_MAX_MS = 15 * 60 * 1000;
+// After a request timeout the capture may still be running in the extension. Keep the
+// lock this much longer, then give up so a dead worker cannot hold it forever.
+const LOCK_GRACE_MS = Number(process.env.SOURCECAPSULE_LOCK_GRACE_MS) || 2 * 60 * 1000;
+const LISTEN_RETRY_MS = Number(process.env.SOURCECAPSULE_LISTEN_RETRY_MS) || 3000;
 
 function log(...parts) {
   const line = `${new Date().toISOString()} ${parts.join(' ')}\n`;
@@ -85,14 +90,42 @@ function readChromeMessages(onMessage) {
 // ---- CLI side: named pipe -------------------------------------------------------
 
 // id -> { socket, timer, cliGone }. An entry is the capture lock: it lives until the
-// extension replies or the request timer fires, NOT until the CLI socket closes. The
-// extension keeps capturing after the CLI walks away, and dropping the entry then let a
-// second capture start on top of it and threw away the finished capsule's reply.
+// extension replies (or a hard cap after the request timer), NOT until the CLI socket
+// closes or the request timer fires. The extension keeps capturing after the CLI walks
+// away, and dropping the entry then let a second capture start on top of it and threw
+// away the finished capsule's reply.
 const pending = new Map();
 
-function replyToCli(id, payload) {
+function onRequestTimeout(id, timeoutMs) {
   const entry = pending.get(id);
   if (!entry) return;
+  if (!entry.cliGone && !entry.socket.destroyed) {
+    try {
+      entry.socket.write(
+        `${JSON.stringify({ ok: false, error: 'timeout', message: `No reply within ${timeoutMs}ms.` })}
+`
+      );
+      entry.socket.end();
+    } catch (error) {
+      log('failed writing timeout reply', id, error.message);
+    }
+  }
+  // Answer the CLI, keep the lock: the capture may still publish.
+  entry.cliGone = true;
+  entry.timer = setTimeout(() => {
+    pending.delete(id);
+    log('giving up on', id, `${LOCK_GRACE_MS}ms after its timeout; releasing the capture lock`);
+  }, LOCK_GRACE_MS);
+}
+
+function replyToCli(id, payload) {
+  const entry = pending.get(id);
+  if (!entry) {
+    // A capture the host already gave up on can still publish. Keep its link.
+    const link = payload && payload.viewUrl ? ` viewUrl=${payload.viewUrl}` : '';
+    log('reply for unknown or expired id', id, `ok=${!!(payload && payload.ok)}`, link);
+    return;
+  }
   clearTimeout(entry.timer);
   pending.delete(id);
   if (entry.cliGone || entry.socket.destroyed) {
@@ -152,9 +185,7 @@ const server = net.createServer((socket) => {
         continue;
       }
       const timeoutMs = Math.min(Number(request.timeoutMs) || REQUEST_MAX_MS, REQUEST_MAX_MS);
-      const timer = setTimeout(() => {
-        replyToCli(id, { ok: false, error: 'timeout', message: `No reply within ${timeoutMs}ms.` });
-      }, timeoutMs);
+      const timer = setTimeout(() => onRequestTimeout(id, timeoutMs), timeoutMs);
       pending.set(id, { socket, timer, cliGone: false });
       log('forwarding', id, request.action || 'unknown');
       sendToChrome({ ...request, id });
@@ -163,18 +194,33 @@ const server = net.createServer((socket) => {
   socket.on('error', (error) => log('cli socket error:', error.message));
 });
 
+let listenRetry = null;
+let reportedInUse = false;
+
+function listen() {
+  listenRetry = null;
+  server.listen(PIPE);
+}
+
 server.on('error', (error) => {
   log('pipe listen failed:', error.message);
-  // Another host already owns the pipe. Stay attached to Chrome so this instance is
-  // harmless, but never race for requests.
-  sendToChrome({ type: 'sourcecapsule:host-status', ok: false, error: 'pipe_in_use' });
+  // Another host already owns the pipe. Stay attached to Chrome, never race for
+  // requests, and keep trying so this instance takes over when the owner exits.
+  if (!reportedInUse) {
+    reportedInUse = true;
+    sendToChrome({ type: 'sourcecapsule:host-status', ok: false, error: 'pipe_in_use' });
+  }
+  if (!listenRetry) listenRetry = setTimeout(listen, LISTEN_RETRY_MS);
 });
 
-server.listen(PIPE, () => {
+server.on('listening', () => {
   log('listening on', PIPE, 'pid', process.pid);
+  reportedInUse = false;
   sendToChrome({ type: 'sourcecapsule:host-status', ok: true, pipe: PIPE, pid: process.pid });
 });
 
+listen();
+
 // Chrome suspends an idle MV3 service worker after ~30s. An inbound native message
 // resets that timer, so the host, not the worker, owns the keepalive.
 setInterval(() => sendToChrome({ type: 'sourcecapsule:heartbeat', at: Date.now() }), 20000);

scripts/install-native-host.ps1

@@ -67,6 +67,20 @@ if ($null -eq $node) {
     throw 'node was not found on PATH. Install Node 18+ and re-run.'
 }
 $nodeExe = $node.Source
+# The .cmd below is written as ASCII, which turns every non-ASCII character into '?' and
+# yields a host that cannot start (for example Node under C:\Users\José\). Use the 8.3 short
+# path when there is one, and refuse rather than write a broken file when there is not.
+if ($nodeExe -match '[^\x00-\x7F]') {
+    $short = $null
+    try { $short = (New-Object -ComObject Scripting.FileSystemObject).GetFile($nodeExe).ShortPath } catch { }
+    if ($short -and $short -notmatch '[^\x00-\x7F]') {
+        Write-Host "Node path is not ASCII; using its short path: $short"
+        $nodeExe = $short
+    }
+    else {
+        throw "The node.exe path contains non-ASCII characters and has no 8.3 short name: $nodeExe. Install Node to an ASCII-only path (for example C:\Program Files\nodejs) and re-run."
+    }
+}
 Write-Host "Node: $nodeExe"
 
 # The registered host is a two-line .cmd that runs the signed node.exe. Smart App Control

scripts/sourcecapsule-capture.mjs

@@ -9,6 +9,7 @@
 // native messaging host and prints what comes back.
 'use strict';
 
+import fs from 'node:fs';
 import net from 'node:net';
 import process from 'node:process';
 import { pathToFileURL } from 'node:url';
@@ -29,8 +30,8 @@ const RELOAD_WAIT_MS = 45000;
 const CANONICAL_X_URL =
   /^https:\/\/(?:www\.|mobile\.)?(?:x|twitter)\.com\/([A-Za-z0-9_]{1,15})\/status\/(\d{1,25})(?:[/?#].*)?$/;
 
-// The extension clamps the capture to 900s, so a larger value would only promise more
-// than it can deliver. The Worker accepts exactly these expiries.
+// The extension and the host both cap the capture at 900s, so a larger value would only
+// promise more than it can deliver. The Worker accepts exactly these expiries.
 const MAX_TIMEOUT_SECONDS = 900;
 const VALID_EXPIRY_DAYS = [1, 7, 30];
 
@@ -46,6 +47,7 @@ function parseArgs(argv) {
   const args = { json: false, timeoutMs: DEFAULT_TIMEOUT_MS };
   for (let i = 0; i < argv.length; i++) {
     const arg = argv[i];
+    // JSON is the only output format; the flag is accepted so existing callers keep working.
     if (arg === '--json') args.json = true;
     else if (arg === '--url') args.url = argv[++i];
     else if (arg === '--ping') args.ping = true;
@@ -100,6 +102,25 @@ export function formatResult(reply) {
   return out;
 }
 
+// --ping and --reload replies get the same treatment: an allowlist, never the raw envelope.
+const CONTROL_FIELDS = [
+  'ok',
+  'extensionVersion',
+  'extensionId',
+  'reloading',
+  'reloaded',
+  'error',
+  'message',
+];
+
+export function formatControlReply(reply) {
+  const out = {};
+  for (const field of CONTROL_FIELDS) {
+    if (reply && Object.prototype.hasOwnProperty.call(reply, field)) out[field] = reply[field];
+  }
+  return out;
+}
+
 function progress(...parts) {
   process.stderr.write(`[sourcecapsule] ${parts.join(' ')}\n`);
 }
@@ -182,7 +203,7 @@ async function main() {
   if (args.ping) {
     progress('pinging extension through the native host');
     const reply = await request({ id, action: 'ping', timeoutMs: 15000 }, 15000);
-    process.stdout.write(`${JSON.stringify(reply, null, 2)}\n`);
+    process.stdout.write(`${JSON.stringify(formatControlReply(reply), null, 2)}\n`);
     return reply.ok ? 0 : 1;
   }
 
@@ -194,7 +215,7 @@ async function main() {
         ask.message =
           'The running extension predates --reload. Reload it once from brave://extensions (or restart with the launcher); later builds can reload themselves.';
       }
-      process.stdout.write(`${JSON.stringify(ask, null, 2)}\n`);
+      process.stdout.write(`${JSON.stringify(formatControlReply(ask), null, 2)}\n`);
       return 1;
     }
     // The old worker and its host go away; the new worker starts its own host. Wait for it.
@@ -206,7 +227,9 @@ async function main() {
         const pong = await request({ id: `${id}-p`, action: 'ping', timeoutMs: 5000 }, 5000);
         if (pong.ok) {
           progress('extension reloaded and answering');
-          process.stdout.write(`${JSON.stringify({ ...pong, reloaded: true }, null, 2)}\n`);
+          process.stdout.write(
+            `${JSON.stringify(formatControlReply({ ...pong, reloaded: true }), null, 2)}\n`
+          );
           return 0;
         }
         lastError = new Error(pong.message || pong.error);
@@ -244,7 +267,19 @@ async function main() {
 }
 
 // Only run when invoked directly, so tests can import canonicalXUrl without side effects.
-const invokedDirectly = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href;
+// import.meta.url is the REAL path, so resolve argv[1] too: through a junction or symlink the
+// two differ, main() never ran, and the CLI exited 0 with empty stdout.
+function isInvokedDirectly() {
+  if (!process.argv[1]) return false;
+  let entry = process.argv[1];
+  try {
+    entry = fs.realpathSync(entry);
+  } catch {
+    /* fall back to the path as given */
+  }
+  return import.meta.url === pathToFileURL(entry).href;
+}
+const invokedDirectly = isInvokedDirectly();
 
 if (invokedDirectly) {
   main()