Skip to content

fix: stop gating local log parsing on a cloud credential#68

Merged
pitimon merged 2 commits into
mainfrom
fix/notify-local-parse-without-token
Jul 20, 2026
Merged

fix: stop gating local log parsing on a cloud credential#68
pitimon merged 2 commits into
mainfrom
fix/notify-local-parse-without-token

Conversation

@pitimon

@pitimon pitimon commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Fixes #67

Summary

The generated notify.cjs 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 to decide the first — so a local-only install refreshed its queue once at service startup and then went stale forever, however many sessions ended.

The throttle write sat inside that same credential branch, which is why sync.throttle was never created either. That made it look like a throttle bug rather than a gating bug.

The hook now spawns sync --auto --from-notify on the 20-second throttle alone. Also drops the now-unused configPath binding from the generated template.

The privacy requirement in #67 is fully honored

"Any fix must preserve a true local-only mode and must not send data to the cloud."

  • Upload unchanged. Still gated on a device token inside sync itself — src/commands/sync.js:936 if (runtime.deviceToken && runtime.baseUrl). All parsing and the cursor write complete at :923-929, before that gate, so a tokenless sync fills queue.jsonl locally and reaches no upload code.
  • No token is created, obtained, or read. The hook no longer touches TOKENTRACKER_DEVICE_TOKEN or config.json at all, and a test asserts the generated file contains no credential read in any form.
  • This path is network-free, not merely upload-free. I checked rather than assumed: ensurePricingLoaded — the one unconditional outbound call in the CLI — is reached only from src/commands/serve.js:13 and src/lib/local-api.js:1750, never from src/commands/sync.js, and getModelPricing falls back to the bundled snapshot (src/lib/pricing/index.js:75). A tokenless notify → sync makes no network request whatsoever.

Test plan

  • node --test test/notify-local-parse.test.js — 3/3
  • Verified the tests fail 0/3 against the unfixed template before accepting them, by restoring main's init.js and re-running. A regression guard that passes both before and after proves nothing.
  • All existing notify-path suites pass untouched: notify-debug-log 1/1, codex-config-multiline-notify 3/3, passive-mode 8/8, init-uninstall 18/18, grok-hook 6/6, diagnostics 3/3, status 2/2
  • Full npm test 784/788; failing names diffed against main and byte-identical
  • Field re-verify by the reporter: notify fires → sync.throttle appears → queue.jsonl grows, with no device token configured

What the tests pin

  1. a tokenless notify creates the throttle stamp — the exact observable Local notify sync is skipped without deviceToken, leaving local-only dashboard stale #67 reported missing at step 5
  2. the 20-second throttle survived the change (removing the credential gate must not remove the throttle with it)
  3. the generated hook contains no credential read at all — so the gate cannot grow back in a different shape

Note on src/commands/init.js as a template literal

The hook is built as a template string, so a stray backtick or ${ in the edit would corrupt every generated notify.cjs. The added comment deliberately contains neither; git diff confirms zero backtick/interpolation characters among the added lines, and the suites above exercise a real init followed by running the generated file.

Pre-existing CI failure

ci:local will be red for the same 4 parseKiroCliIncremental failures that are red on main (see #65). This PR does not touch that path.

itarun.p and others added 2 commits July 20, 2026 06:11
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
@pitimon
pitimon merged commit a31e09c into main Jul 20, 2026
1 check passed
@pitimon
pitimon deleted the fix/notify-local-parse-without-token branch July 20, 2026 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local notify sync is skipped without deviceToken, leaving local-only dashboard stale

1 participant