From bcd0f0913605bd22557490e5903696e7ff070cd3 Mon Sep 17 00:00:00 2001 From: "itarun.p" Date: Mon, 20 Jul 2026 06:11:53 +0700 Subject: [PATCH] fix: stop gating local log parsing on a cloud credential MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generated notify.cjs hook read a device token and refused to spawn anything without one. Parsing local logs and uploading them are two separate decisions, and the hook was making the second one to decide the first. A local-only install therefore refreshed its queue once at service startup and then went stale forever, no matter how many sessions ended. The throttle write lived inside that same credential branch, which is why sync.throttle was never created either — the symptom that made this look like a throttle bug rather than a gating bug. The hook now spawns `sync --auto --from-notify` on the 20s throttle alone. Nothing else changes: upload is still gated on a device token inside sync itself (src/commands/sync.js:936), so a tokenless install parses locally and transmits nothing. Verified that this path is not merely upload-free but network-free: ensurePricingLoaded — the only unconditional outbound call in the CLI — is reached from serve.js and local-api.js only, never from sync.js, and getModelPricing falls back to the bundled snapshot. Drops the now-unused configPath binding from the generated hook. Tests were confirmed to fail 0/3 against the unfixed template before being accepted: one pins that a tokenless notify creates the throttle stamp, one pins that the 20s throttle survived the change, and one pins that the generated hook contains no credential read at all — so the gate cannot grow back in a different shape. Fixes #67 --- src/commands/init.js | 24 +++---- test/notify-local-parse.test.js | 108 ++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 14 deletions(-) create mode 100644 test/notify-local-parse.test.js diff --git a/src/commands/init.js b/src/commands/init.js index 3935e2e4..5e908eda 100644 --- a/src/commands/init.js +++ b/src/commands/init.js @@ -883,7 +883,6 @@ const codexOriginalPath = ${JSON.stringify(originalPath)}; const codeOriginalPath = ${JSON.stringify(path.join(trackerDir, "code_notify_original.json"))}; const trackerBinPath = ${JSON.stringify(trackerBinPath)}; const depsMarkerPath = path.join(trackerDir, 'app', 'bin', 'tracker.js'); - const configPath = path.join(trackerDir, 'config.json'); const fallbackPkg = ${JSON.stringify(fallbackPkg)}; const selfPath = path.resolve(__filename); const home = os.homedir(); @@ -919,20 +918,17 @@ if (debugEnabled) { } // Throttle spawn: at most once per 20 seconds. +// Parsing local logs and uploading to the cloud are two separate decisions. +// This hook makes only the first one. It runs whether or not a device token +// exists, because a local-only install still needs its queue refreshed when a +// session ends. Upload stays gated on a device token inside sync itself, so +// no credential here means local parse happens and nothing is transmitted. try { - const throttlePath = path.join(trackerDir, 'sync.throttle'); - let deviceToken = process.env.TOKENTRACKER_DEVICE_TOKEN || null; - if (!deviceToken) { - try { - const cfg = JSON.parse(fs.readFileSync(configPath, 'utf8')); - if (cfg && typeof cfg.deviceToken === 'string') deviceToken = cfg.deviceToken; - } catch (_) {} - } - const canSync = Boolean(deviceToken && deviceToken.length > 0); - const now = Date.now(); - let last = 0; - try { last = Number(fs.readFileSync(throttlePath, 'utf8')) || 0; } catch (_) {} - if (canSync && now - last > 20_000) { + const throttlePath = path.join(trackerDir, 'sync.throttle'); + const now = Date.now(); + let last = 0; + try { last = Number(fs.readFileSync(throttlePath, 'utf8')) || 0; } catch (_) {} + if (now - last > 20_000) { try { fs.writeFileSync(throttlePath, String(now), 'utf8'); } catch (_) {} const hasLocalRuntime = fs.existsSync(trackerBinPath); const hasLocalDeps = fs.existsSync(depsMarkerPath); diff --git a/test/notify-local-parse.test.js b/test/notify-local-parse.test.js new file mode 100644 index 00000000..dafe9668 --- /dev/null +++ b/test/notify-local-parse.test.js @@ -0,0 +1,108 @@ +const assert = require("node:assert/strict"); +const os = require("node:os"); +const path = require("node:path"); +const fs = require("node:fs/promises"); +const cp = require("node:child_process"); +const { test } = require("node:test"); + +// Regression guard for #67: the generated notify hook used to gate local +// parsing on a cloud credential, so a local-only install never refreshed its +// queue after a session ended. Parsing local logs and uploading them are two +// separate decisions; this hook makes only the first. + +function runNode(args, env) { + return new Promise((resolve, reject) => { + cp.execFile(process.execPath, args, { env }, (err) => (err ? reject(err) : resolve())); + }); +} + +async function rmWithRetry(target, retries = 3) { + let lastErr = null; + for (let i = 0; i <= retries; i++) { + try { + await fs.rm(target, { recursive: true, force: true }); + return; + } catch (err) { + lastErr = err; + if (!err || err.code !== "ENOTEMPTY") throw err; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } + throw lastErr; +} + +// Sets up an install with no device token anywhere: not in the environment, +// not in config.json. This is the local-only configuration from #67. +async function setupTokenlessInstall(t) { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "tokentracker-notify-local-")); + t.after(() => rmWithRetry(tmp)); + + const codexHome = path.join(tmp, ".codex"); + await fs.mkdir(codexHome, { recursive: true }); + + const env = { + ...process.env, + HOME: tmp, + CODEX_HOME: codexHome, + OPENCODE_CONFIG_DIR: path.join(tmp, ".config", "opencode"), + }; + delete env.TOKENTRACKER_DEVICE_TOKEN; + + const entry = path.join(path.resolve(__dirname, ".."), "bin", "tracker.js"); + await runNode([entry, "init", "--yes", "--no-auth", "--no-open", "--base-url", "https://example.invalid"], env); + + const trackerDir = path.join(tmp, ".tokentracker", "tracker"); + await fs.mkdir(trackerDir, { recursive: true }); + await fs.writeFile(path.join(trackerDir, "config.json"), "{}", "utf8"); + + return { + env, + trackerDir, + notifyPath: path.join(tmp, ".tokentracker", "bin", "notify.cjs"), + throttlePath: path.join(trackerDir, "sync.throttle"), + }; +} + +test("notify starts a local sync when no device token is configured", async (t) => { + const ctx = await setupTokenlessInstall(t); + + // #67 step 5: sync.throttle was never created, because the throttle write + // lived inside the credential gate. Its absence is the observable symptom. + await assert.rejects(fs.stat(ctx.throttlePath), /ENOENT/); + + await runNode([ctx.notifyPath, "--source=codex"], ctx.env); + + const stat = await fs.stat(ctx.throttlePath); + assert.ok(stat.isFile(), "notify must record a throttle stamp for a tokenless install"); + + const stamp = Number(await fs.readFile(ctx.throttlePath, "utf8")); + assert.ok(Number.isFinite(stamp) && stamp > 0, `expected a timestamp, got ${stamp}`); +}); + +test("notify still throttles repeat events for a tokenless install", async (t) => { + const ctx = await setupTokenlessInstall(t); + + await runNode([ctx.notifyPath, "--source=codex"], ctx.env); + const first = await fs.readFile(ctx.throttlePath, "utf8"); + + await runNode([ctx.notifyPath, "--source=codex"], ctx.env); + const second = await fs.readFile(ctx.throttlePath, "utf8"); + + // Removing the credential gate must not remove the 20s throttle with it. + assert.equal(second, first, "a second notify inside the throttle window must not re-stamp"); +}); + +test("the generated notify hook reads no cloud credential at all", async (t) => { + const ctx = await setupTokenlessInstall(t); + const source = await fs.readFile(ctx.notifyPath, "utf8"); + + // The defect was not "the gate was set wrong" — it was that this hook + // consulted a cloud credential to decide whether to do local work. Asserting + // the credential is never read keeps the gate from growing back in any form. + assert.doesNotMatch(source, /TOKENTRACKER_DEVICE_TOKEN/, "notify must not read the device-token env var"); + assert.doesNotMatch(source, /deviceToken/, "notify must not read a device token from config"); + assert.doesNotMatch(source, /canSync/, "notify must not branch on cloud-sync capability"); + + // It must still only ever invoke the local sync entry point. + assert.match(source, /'sync', '--auto', '--from-notify'/); +});