[AnnatarHe]

Fixes verified tracking, sync, encryption, and permission bugs in the ShellTime CLI and daemon. Each item below was confirmed in code (and, where practical, with a regression test) before the change.

Bugs fixed

1. post.txt lines were corrupted once the file passed 512KB

GetPostCommands stored bufio.Scanner.Bytes() slices. Those slices alias the scanner buffer. After the file grows past MAX_BUFFER_SIZE (512KB), Scan shifts the buffer and earlier lines are overwritten. shelltime gc then rewrote the history from those slices.

Impact: command history on the daemon-less path was corrupted or lost.

Fix: copy each scanned line before storing it.

2. The command exactly at the cursor was uploaded again

BuildTrackingData skipped only recordingTime.Before(cursor). The command recorded at the cursor was included on every later flush. The txt store is not pruned on the daemon-less path, so that row stays on disk.

Impact: duplicate uploads of the last synced command whenever no daemon is running.

Fix: skip recording times that are not strictly after the cursor.

3. Same-second commands were paired with the wrong start time

FindClosestCommand compared Unix() seconds. Two commands in the same second both had diff 0, so the first candidate won, including a pre that started later in that second. When nothing matched, the function returns a non-nil empty command; shelltime ls treated that as a hit and showed a zero start time.

Impact: wrong durations in shelltime ls and in sync payloads.

Fix: compare UnixNano. ls ignores the empty sentinel via IsNil().

4. config.local.* could not override storage.engine

mergeConfig copied every other config field and dropped Storage.

Impact: storage.engine: bolt (or file) in config.local.yaml / config.local.toml was ignored.

Fix: merge the Storage pointer. Documented in docs/CONFIG.md.

5. Encryption could send plaintext or an unreadable payload

With encrypted: true, an empty public key skipped encryption and the plaintext was uploaded. AES key generation and RSA wrap errors were logged and then ignored, so the upload could go out with an empty wrapped key. Fetching the public key already failed closed.

Impact: commands the user configured as encrypted could leave the machine unencrypted, or be stored in a form the server cannot decrypt.

Fix: return the error and do not upload when the key is missing, empty, or encryption fails.

6. A stale daemon socket dropped the command

IsSocketReady is only os.Stat. A socket file left by a dead daemon still looks ready. track then failed to dial, returned that error, and the shell hook discards stderr.

Impact: commands run while /tmp/shelltime.sock (or a custom socket path) was stale were never saved.

Fix: log the dial failure and fall back to the local txt store.

7. Sync with no token advanced the cursor, and extra endpoints were skipped

SendLocalDataToServer returned nil without uploading when Token was empty, and callers then called SetCursor. It also returned before considering endpoints whenever the primary token was empty. On the daemon, encryption plus an empty token failed the public-key fetch, nacked the message, and SavePost ran again on each redelivery.

Impact: commands recorded before shelltime auth were permanently skipped after the user authenticated. Configured extra endpoints never received data if the primary token was blank. The daemon could duplicate the same post.

Fix: do not advance the cursor when nothing would be uploaded. Send to configured endpoints even when the primary token is empty. When the daemon has neither a token nor endpoints, it acks the message and leaves the batch buffered.

8. A failed daemon upload duplicated the post

handlePubSubTrackPost saved the post, then returned the HTTP error. The subscriber nacks and retries up to 3 times, and each delivery called SavePost again.

Impact: one failed upload could append the same command up to four times, then drop the message. The cursor did not move, but the buffer was wrong.

Fix: log the send error and return nil. The row stays buffered once, the cursor does not move, and a later post retries the upload.

9. The API token was world-readable

shelltime auth created ~/.shelltime as 0755 and config.yaml as 0644. WriteFile also does not change the mode of a file that already exists.

Impact: other users on the machine could read the API token.

Fix: directory 0700, config 0600, with chmod after every write.

10. Local command history and logs were world-readable

Pre/post/cursor files, gc rewrites, and sync-pending.jsonl were created 0644. The log file was opened 0755.

Impact: other local users could read command history (including secrets typed into the shell) and the log.

Fix: create those files 0600 and chmod them, because WriteFile / OpenFile do not tighten an existing mode. The commands directory is 0700. An existing loose log is tightened once.

11. zsh and bash recorded phantom or duplicate commands

zsh precmd used fc -ln -1, which repeats the previous history entry on the first prompt and on an empty enter. bash left LAST_COMMAND set, so an empty enter posted the same command again.

Impact: startup and blank-enter prompts created extra command records.

Fix: remember the command from preexec and clear it before the post hook returns. hooks install now replaces a hook file whose bytes differ from the embedded script, so the next shelltime hooks install or shelltime init picks this up. Identical files are left untouched. A hand-edited script under ~/.shelltime/hooks is replaced on that install.

12. track --sessionId help called the id a unix timestamp

Hooks pass date +%Y%m%d%H%M%S, a per-shell id shared by the pre and post events.

Fix: the help text matches that.

13. The socket permission error named the wrong mode

The daemon chmods the socket to 0777 and logged a failure to set 0755.

Fix: the log now says 0777. The mode is unchanged (see below).

14. A sync message that failed to encode was still published

json.Marshal failure on a sync socket message was logged, then the handler continued and published the empty buffer.

Fix: return without publishing.

15. Config docs named the wrong cleanup files

docs/CONFIG.md listed heartbeat.log and sync-pending.txt. The files on disk are coding-heartbeat.data.log and sync-pending.jsonl.

Fix: the doc matches the filenames the cleanup code deletes.

Tests

go test -timeout 3m -coverprofile=coverage.txt -covermode=atomic ./..., go vet ./..., and builds of shelltime and shelltime-daemon pass. New tests cover the scanner copy, nanosecond pairing, cursor equality, storage merge, empty and invalid encryption keys, stale-socket fallback, no-token cursor behavior, endpoints-only sync, auth file modes, and the hook scripts.

Suspected issues left unchanged

• Socket mode 0777. This is intentional (any local user must be able to connect). It also lets any local user inject track events into the daemon owner's history, and those events sync with the owner's token. Changing it would break multi-user machines; it needs a design change, not a chmod.
• gcTime is never sent or applied by the CLI. It is loaded and documented as server retention days. The CLI does not put it on the wire. Left alone so the docs do not invent a server contract.
• shelltime sync advances the local cursor when the Unix socket write succeeds, not when the server accepts the batch. The daemon nacks and drops the message after 3 retries. The circuit breaker only persists a payload once it is already open (10 consecutive failures). The daemon-owned track path does not advance the cursor until HTTP succeeds. Fixing the CLI sync path needs a request/response ack.
• SetCursor failure after a successful upload still returns an error. A nack can then duplicate that post and resend it. This is a disk error, not the normal path.
• FindClosestCommand still returns a non-nil empty command when nothing matches. ls now checks IsNil(). Sync start time stays zero either way.
• Cursor parsing uses strconv.Atoi. That is fine on the amd64 and arm64 builds Goreleaser ships. 32-bit is not a shipped arch.
• fileStore.Prune rewrites with WriteFile, so a crash mid-write can truncate the history file. gc already renames a backup first. Making prune atomic would be a rewrite, not a one-line fix.
• Posts with no matching pre are skipped, and the cursor still moves past them when a later command is included in the same batch. Keeping them would retain excluded and unpaired commands forever. Left as designed.
• zsh precmd does not clobber other hooks on zsh 5.9, and a precmd that runs true does not change the exit status the next prompt sees. fc -ln -1 also does not add a leading space on zsh 5.9, so that was not a pairing bug. Fish only fires fish_postexec after a real command.

  

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[github-actions]

⏱️ shelltime track performance

✅ No significant change in shelltime track latency · 🎲 the runner was noisy (the fork/exec floor moved too), so treat small changes with care

Comparing main @ 34a67d9 → #325 @ 07b0c21. Each scenario execs the binary the way the shell hooks do; values are the median of 10 interleaved rounds. Lower is better.

Scenario	base	head	Δ	p
Startup/floor	192 µs ±21%	243 µs ±22%	+26.29%	0.023	🎲
Startup/version	2.77 ms ±24%	3.28 ms ±44%	~	0.436	✅
Track/daemon/pre	3.19 ms ±49%	3.52 ms ±44%	~	1.000	✅
Track/daemon/post	3.76 ms ±39%	3.54 ms ±31%	~	1.000	✅
Track/direct/pre	3.25 ms ±46%	4.06 ms ±43%	~	0.353	✅
Track/direct/post	6.79 ms ±17%	7.21 ms ±26%	~	0.436	✅
Track/direct/post-sync	8.92 ms ±32%	9.59 ms ±34%	~	0.739	✅

Binary size: 19.1 MiB → 19.1 MiB (+0.0%).

CPU, tail latency and memory

Per exec, base → head (Δ when significant).

Scenario	p95 wall	user CPU	sys CPU	peak RSS
Startup/floor	236 µs → 352 µs (+49.01%)	158 µs → 193 µs (+22.09%)	6 µs → 8 µs	9.2 MiB → 9.2 MiB
Startup/version	4.10 ms → 5.15 ms	1.80 ms → 1.86 ms	984 µs → 1.33 ms	15.3 MiB → 15.3 MiB
Track/daemon/pre	5.36 ms → 5.08 ms	1.93 ms → 2.14 ms	1.32 ms → 1.47 ms	16.2 MiB → 16.2 MiB
Track/daemon/post	5.15 ms → 4.56 ms	2.19 ms → 2.23 ms	1.56 ms → 1.29 ms	16.1 MiB → 16.1 MiB
Track/direct/pre	4.78 ms → 6.24 ms	1.94 ms → 2.36 ms	1.29 ms → 1.70 ms	16.0 MiB → 16.0 MiB
Track/direct/post	9.06 ms → 9.94 ms	4.30 ms → 4.82 ms	2.56 ms → 2.56 ms	17.3 MiB → 17.4 MiB (+0.72%)
Track/direct/post-sync	13.16 ms → 12.96 ms	5.87 ms → 6.21 ms	3.19 ms → 3.40 ms	19.1 MiB → 19.3 MiB (+1.01%)

benchstat

goos: linux
goarch: amd64
pkg: github.com/malamtime/cli/perf
cpu: AMD EPYC
                         │     base     │                 head                 │
                         │    sec/op    │    sec/op     vs base                │
Startup/floor-2            192.4µ ± 21%   243.0µ ± 22%  +26.29% (p=0.023 n=10)
Startup/version-2          2.774m ± 24%   3.276m ± 44%        ~ (p=0.436 n=10)
Track/daemon/pre-2         3.193m ± 49%   3.515m ± 44%        ~ (p=1.000 n=10)
Track/daemon/post-2        3.762m ± 39%   3.537m ± 31%        ~ (p=1.000 n=10)
Track/direct/pre-2         3.249m ± 46%   4.057m ± 43%        ~ (p=0.353 n=10)
Track/direct/post-2        6.790m ± 17%   7.207m ± 26%        ~ (p=0.436 n=10)
Track/direct/post-sync-2   8.919m ± 32%   9.585m ± 34%        ~ (p=0.739 n=10)
geomean                    2.773m         3.104m        +11.91%

                         │     base     │                 head                 │
                         │  p50-sec/op  │  p50-sec/op   vs base                │
Startup/floor-2            175.4µ ± 24%   225.4µ ± 15%  +28.51% (p=0.023 n=10)
Startup/version-2          2.529m ± 28%   2.953m ± 50%        ~ (p=0.631 n=10)
Track/daemon/pre-2         2.910m ± 50%   3.297m ± 40%        ~ (p=0.912 n=10)
Track/daemon/post-2        3.586m ± 38%   3.401m ± 35%        ~ (p=0.796 n=10)
Track/direct/pre-2         3.045m ± 52%   3.871m ± 35%        ~ (p=0.247 n=10)
Track/direct/post-2        6.453m ± 16%   6.978m ± 27%        ~ (p=0.529 n=10)
Track/direct/post-sync-2   8.429m ± 35%   9.308m ± 36%        ~ (p=0.796 n=10)
geomean                    2.583m         2.935m        +13.64%

                         │     base     │                 head                 │
                         │  p95-sec/op  │  p95-sec/op   vs base                │
Startup/floor-2            236.4µ ± 36%   352.3µ ± 42%  +49.01% (p=0.015 n=10)
Startup/version-2          4.105m ± 30%   5.146m ± 49%        ~ (p=0.393 n=10)
Track/daemon/pre-2         5.357m ± 41%   5.079m ± 71%        ~ (p=0.853 n=10)
Track/daemon/post-2        5.148m ± 42%   4.561m ± 79%        ~ (p=0.912 n=10)
Track/direct/pre-2         4.782m ± 26%   6.242m ± 51%        ~ (p=0.436 n=10)
Track/direct/post-2        9.058m ± 32%   9.939m ± 36%        ~ (p=0.481 n=10)
Track/direct/post-sync-2   13.16m ± 32%   12.96m ± 40%        ~ (p=0.912 n=10)
geomean                    3.959m         4.435m        +12.01%

                         │     base     │                head                 │
                         │  peak-rss-B  │  peak-rss-B   vs base               │
Startup/floor-2            9.182Mi ± 1%   9.171Mi ± 2%       ~ (p=0.971 n=10)
Startup/version-2          15.28Mi ± 0%   15.28Mi ± 0%       ~ (p=0.897 n=10)
Track/daemon/pre-2         16.16Mi ± 0%   16.16Mi ± 0%       ~ (p=0.540 n=10)
Track/daemon/post-2        16.09Mi ± 0%   16.07Mi ± 0%       ~ (p=0.382 n=10)
Track/direct/pre-2         16.03Mi ± 0%   16.03Mi ± 0%       ~ (p=0.698 n=10)
Track/direct/post-2        17.26Mi ± 0%   17.39Mi ± 0%  +0.72% (p=0.000 n=10)
Track/direct/post-sync-2   19.09Mi ± 0%   19.29Mi ± 0%  +1.01% (p=0.000 n=10)
geomean                    15.26Mi        15.29Mi       +0.22%

                         │     base      │                  head                  │
                         │  sys-sec/op   │   sys-sec/op    vs base                │
Startup/floor-2            5.530µ ± 105%    7.655µ ± 112%        ~ (p=0.218 n=10)
Startup/version-2          984.1µ ±  27%   1325.7µ ±  58%        ~ (p=0.280 n=10)
Track/daemon/pre-2         1.319m ±  57%    1.473m ±  48%        ~ (p=0.971 n=10)
Track/daemon/post-2        1.562m ±  52%    1.293m ±  56%        ~ (p=0.684 n=10)
Track/direct/pre-2         1.293m ±  53%    1.696m ±  49%        ~ (p=0.280 n=10)
Track/direct/post-2        2.559m ±  17%    2.564m ±  32%        ~ (p=0.796 n=10)
Track/direct/post-sync-2   3.192m ±  37%    3.397m ±  52%        ~ (p=0.912 n=10)
geomean                    737.3µ           836.0µ         +13.39%

                         │     base     │                 head                 │
                         │ user-sec/op  │ user-sec/op   vs base                │
Startup/floor-2            158.0µ ± 16%   192.9µ ± 12%  +22.09% (p=0.015 n=10)
Startup/version-2          1.796m ± 21%   1.864m ± 48%        ~ (p=0.853 n=10)
Track/daemon/pre-2         1.931m ± 38%   2.138m ± 31%        ~ (p=0.912 n=10)
Track/daemon/post-2        2.191m ± 33%   2.234m ± 22%        ~ (p=0.912 n=10)
Track/direct/pre-2         1.937m ± 44%   2.358m ± 34%        ~ (p=0.280 n=10)
Track/direct/post-2        4.299m ± 16%   4.822m ± 23%        ~ (p=0.393 n=10)
Track/direct/post-sync-2   5.872m ± 31%   6.207m ± 29%        ~ (p=0.579 n=10)
geomean                    1.789m         1.984m        +10.91%

_{Δ compares medians; ~ means no significant difference (Mann-Whitney U, p ≥ 0.05). A scenario is flagged when it is more than 10% and more than 250µs slower with p < 0.05. Startup/floor runs true, not shelltime: it is an A/A check of runner noise. Runner: linux/amd64, AMD EPYC. go1.27.1, 10 rounds × 100 execs per scenario and binary.}

[codecov]

Codecov Report

❌ Patch coverage is 78.44828% with 25 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
daemon/handlers.sync.go	70.00%	9 Missing ⚠️
model/circuit_breaker.go	50.00%	4 Missing ⚠️
commands/auth.go	70.00%	3 Missing ⚠️
commands/track.go	78.57%	3 Missing ⚠️
model/command.go	82.35%	3 Missing ⚠️
daemon/socket.go	0.00%	2 Missing ⚠️
model/shell.go	80.00%	1 Missing ⚠️

Flag	Coverage Δ
unittests	86.22% <78.44%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
commands/gc.go	80.58% <100.00%> (-4.86%)	⬇️
commands/logger.go	78.78% <100.00%> (+1.36%)	⬆️
daemon/handlers.track.go	96.72% <100.00%> (+0.16%)	⬆️
model/api.go	100.00% <100.00%> (ø)
model/config.go	96.35% <100.00%> (+0.05%)	⬆️
model/db.go	90.51% <100.00%> (+0.16%)	⬆️
model/list.go	89.28% <100.00%> (ø)
model/store_file.go	83.13% <100.00%> (ø)
model/tracking_build.go	92.42% <100.00%> (ø)
model/shell.go	85.71% <80.00%> (-1.08%)	⬇️
... and 6 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[AnnatarHe]

/claude ultrareview

[claude]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

---

🤖 Generated with Claude Code

[AnnatarHe]

/claude review

[AnnatarHe]

@claude review

[claude]

Claude finished @AnnatarHe's task in 23s —— View job

---

Code review

I read the non-test diff against origin/main. I didn't run the test suite. I found no blocking issues. The fixes match the descriptions, and the "left unchanged" list is accurate.

What I checked

• GetPostCommands copy (model/db.go): this fixes the aliasing bug.
• BuildTrackingData (model/tracking_build.go): !recordingTime.After(cursor) is correct. The cursor file stores UnixNano, so the equality comparison is exact. A zero cursor still lets everything through.
• FindClosestCommand (model/command.go): comparing UnixNano fixes the same-second pairing. Returning new(Command) when nothing matches keeps the old contract for callers, and list.go now checks IsNil().
• Encryption (daemon/handlers.sync.go): it now fails closed on an empty key, key generation errors, RSA wrap errors, and empty outputs.
• SendLocalDataToServer (model/api.go): it now builds the endpoint list so extra endpoints are used when the primary token is empty. wg.Add(len(authPair)) matches the slice length.
• Daemon post handler (daemon/handlers.track.go): it acks the message when there is no token and no endpoints, and on send failure. This stops redelivery from calling SavePost repeatedly.
• Stale socket fallback (commands/track.go): forwardTrackToDaemon falls through to the local store on a dial failure. The second if forwarded || err != nil can never see a non-nil err, but that is harmless.
• Permissions: the 0600/0700 modes plus the explicit chmod after each write cover the existing-file case. The logger skips the chmod when the mode is already private, which keeps the hot path cheap.
• Hooks: the zsh and bash changes consume the remembered command in precmd, which fixes the blank-enter and startup duplicates.

Minor notes (non-blocking)

1. ensureHookFile overwrites hand-edited hooks. ensureHookFile (model/shell.go) silently overwrites a user-modified hook on the next hooks install or init. The PR description mentions this. Consider logging at info level when a differing file is replaced, so it isn't a surprise. Also, os.MkdirAll(hookDir, 0755) and the hook file mode are unchanged. That is fine because the hooks hold no secrets.
2. Dead branch in sendTrackEventToDaemon fallback. If the daemon accepts a partial write and then errors, the fallback to the local store could record the command twice (once in the daemon, once locally). The window is tiny and the behaviour is better than losing the command, but it may be worth a comment.
3. Codecov patch coverage is about 77%. The least covered paths are the handlers.sync.go encryption error returns, the commands/auth.go chmod error branches, and circuit_breaker.go Chmod failures. These are mostly defensive branches, so I'm fine with that.
4. model/api.go has gofmt-only changes. The TrackingMetaData field alignment is unrelated noise, though harmless.

Overall this looks good to merge from my side.