fix(tz): compute log filename date from configured timezone#26
Conversation
MEMORY_LOG_DATE was computed in log.sh at source-time (line 43) using TZ="$REMEMBER_TZ" date — but REMEMBER_TZ wasn't set until line 132. With an empty TZ= prefix, macOS/BSD date silently falls back to UTC, producing filenames one day ahead of the user's local date after ~20:00 in zones west of UTC. The log() function appeared correct because its timestamp was computed lazily, after REMEMBER_TZ was populated. Fix: - Move REMEMBER_CONFIG / config() / REMEMBER_TZ definitions above the MEMORY_LOG_DATE computation so the variable exists when used. - Change the fallback from "Europe/Paris" to empty string. - Introduce _remember_date() so an empty REMEMBER_TZ uses system local (no TZ= prefix), not UTC. - Export REMEMBER_TZ so Python subprocesses (haiku calls, consolidate) see the same zone. - Add pipeline/_tz.py reading REMEMBER_TZ via zoneinfo; wire log.py and shell.py through it so Python and shell agree on "today". shell.py's consolidate today-filter was silently misbehaving too — it uses the date as a substring match to exclude today's staging files. - Remove the "timezone": "UTC" default from config.example.json so new users who copy it verbatim don't inherit the same bug after the code fix lands. - Apply the same guarded TZ call in post-tool-hook.sh (runs after log.sh is sourced, so _remember_date is in scope). Regression tests cover both the Python filename/today-filter and the shell log.sh ordering: with REMEMBER_TZ=America/Los_Angeles and system TZ=UTC, MEMORY_LOG_FILE must resolve to the LA date; with no timezone config and system TZ=America/Los_Angeles, it must fall back to LA, not UTC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- README: replace "UTC" default with explicit "(system local)" so users understand the config key is optional and recognize that the default now falls back to the system clock. - resolve-paths.sh: quote the +%Y-%m-%d format in error-path logging to match the quoting of the neighboring time format. Error path runs before log.sh is sourced, so REMEMBER_TZ isn't available; system- default TZ is the correct choice here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fdaviddpt
left a comment
There was a problem hiding this comment.
Review
Nice work — the bug diagnosis is solid and the fix is correct. The load-ordering issue in log.sh is a real problem and your reproduction is convincing.
Required before merge: rebase onto current main
PR #35 landed after you branched. It changed REMEMBER_CONFIG to use PIPELINE_DIR:
# Current main:
REMEMBER_CONFIG="${PIPELINE_DIR:-${PROJECT_DIR:-.}/.claude/remember}/config.json"
# Your PR still has:
REMEMBER_CONFIG="${PROJECT_DIR:-.}/.claude/remember/config.json"This will conflict on merge. The fix is straightforward — just rebase onto main and use PIPELINE_DIR in the moved-up REMEMBER_CONFIG line.
Minor suggestions (non-blocking)
-
Frozen datetime helper is duplicated in
test_log.py,test_shell.py, andtest_tz.py. Consider extracting totests/conftest.pyas a shared fixture. -
export REMEMBER_TZ— good catch, we didn't have that. Python subprocesses inheriting the configured timezone is important for correctness.
What's good
_tz.pyis clean and minimal.ZoneInfoNotFoundError→None(system local) is the right fallback._remember_date()correctly avoidsTZ=""— that's the actual fix for the bug.- The
cmd_consolidatetoday-filter fix catches a real secondary bug. - Shell-level tests in
test_log_sh.pyare the most valuable — they reproduce the exact load-ordering bug. mock_con.assert_not_called()guard prevents accidental Haiku API calls during test failures. Smart.
Once rebased, this is ready to merge. 👍
|
Merged via local merge to main. Thanks for the solid bug fix and thorough test plan, @josemoreno801-netizen! |
Summary
The daily log filename (
memory-YYYY-MM-DD.log) is being stamped with UTC instead of the user's configured timezone, producing next-day filenames after ~20:00 local in zones west of UTC.Root cause:
scripts/log.sh:43computesMEMORY_LOG_DATEwithTZ="$REMEMBER_TZ" date +%Y-%m-%d, butREMEMBER_TZisn't defined untilscripts/log.sh:132— 89 lines later. With an emptyTZ=prefix, macOS/BSDdatesilently falls back to UTC. Thelog()function's internal timestamps were correct because they're computed lazily, afterREMEMBER_TZwas populated. Filenames were wrong; content was right.How I reproduced it
At 23:12 EDT on 2026-04-22:
TZ="" date→2026-04-23 03:12 UTC✗TZ="America/New_York" date→2026-04-22 23:12 EDT✓silent-stone/.remember/logs/had bothmemory-2026-04-22.logandmemory-2026-04-23.logside-by-side, with EDT-correct timestamps (23:12:06) written inside the 04-23-named file.Fix
scripts/log.sh— moveREMEMBER_CONFIG/config()/REMEMBER_TZdefinitions aboveMEMORY_LOG_DATE. Introduce_remember_date()so an emptyREMEMBER_TZuses system local time (omitTZ=prefix), not UTC. Change the fallback default from"Europe/Paris"to empty string.export REMEMBER_TZso Python subprocesses inherit it.pipeline/_tz.py(new) — sharednow()/today_str()/time_str()readingREMEMBER_TZviazoneinfo. Invalid TZ names fall back silently to system local.pipeline/log.py— filename + timestamp now route through_tz.pipeline/shell.py—cmd_consolidate's "today" filter uses_tz.today_str(). This was a silent correctness bug beyond filenames: the filter excludes today's staging files from consolidation by substring-matchingtodayagainst filenames. If Python disagreed with shell about "today," staging files could be consolidated prematurely.config.example.json— remove the"timezone": "UTC"landmine so users who copy the example verbatim don't inherit the same bug.scripts/post-tool-hook.sh— autonomous save-log filename now uses_remember_date(runs afterlog.shis sourced).README.md— clarify thattimezoneis optional and defaults to system local.Regression tests (all new)
tests/test_tz.py— 7 tests coveringREMEMBER_TZresolution, empty/unset/invalid fallbacks, and the specific day-boundary frozen at 2026-04-23 03:12 UTC → 2026-04-22 23:12 EDT.tests/test_log_sh.py— 3 shell-level regressions runninglog.shin subprocesses with a forcedTZ=UTCsystem env andtimezone: America/Los_Angelesconfig. Verifies filenames track the configured zone, and that an empty config correctly falls back to system local instead of UTC.tests/test_log.py— 2 new Python regressions.tests/test_shell.py— 1 new regression forcmd_consolidate's today-filter (with amock_con.assert_not_called()guard rail so a future bug here can't accidentally hit the real Haiku API, as my first RED run unfortunately did).Test plan
pytest tests/— 199 passed (baseline was 186), coverage 99.12% (≥80% enforced).log.shwithtimezone: America/New_York+ systemTZ=UTC→ filename matches EDT date.TZ=America/Los_Angeles→ filename matches LA date, not UTC.timezone: UTC+ systemTZ=America/New_York→ filename matches UTC (verifies config actually drives the date, not system clock).log()with frozen clock at 03:12 UTC +REMEMBER_TZ=America/New_York→memory-2026-04-22.logwith23:12timestamp inside.cmd_consolidate's today-filter correctly skips a staging file named for EDT "today" (04-22) even when UTC date is 04-23.🤖 Generated with Claude Code