From 17513ee270d5bb5a15ea09705ef45d26272185c4 Mon Sep 17 00:00:00 2001 From: josemoreno801-netizen Date: Tue, 28 Apr 2026 01:51:39 -0400 Subject: [PATCH] fix(log): resolve config.json under .claude/remember/ for marketplace installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In marketplace cache layouts (~/.claude/plugins/cache//remember//), config.json sits at $PIPELINE_DIR/.claude/remember/config.json — but log.sh was looking at $PIPELINE_DIR/config.json (one directory too shallow). Every config lookup silently fell through to defaults, causing two cascading bugs: - REMEMBER_TZ defaulted to "Europe/Paris" (save-session.sh:62), so all timestamps in today-*.md drifted by hours from the user's actual local time. A user in America/New_York saw "## 07:09 | branch" written at 1:09 AM ET because Haiku echoed the formatted Paris time back into the today file. - time_format defaulted to "24h" so users who set "12h" never got AM/PM output. Fix: log.sh now resolves REMEMBER_CONFIG via three branches — 1. marketplace cache: $PIPELINE_DIR/.claude/remember/config.json 2. legacy/flat: $PIPELINE_DIR/config.json (preserved for compat) 3. local install: $PROJECT_DIR/.claude/remember/config.json Also harden save-session.sh:62 — its hardcoded "Europe/Paris" fallback was actively harmful as a defense, since log.sh already exports REMEMBER_TZ="" which _remember_date correctly translates to system-local. The Paris fallback would have shifted every timestamp if config ever genuinely went missing. Default is now empty string, matching log.sh:59's convention. Tests: - tests/test_log_sh.py: new test sources log.sh with PIPELINE_DIR set and config under .claude/remember/, asserts REMEMBER_TZ resolves. - tests/test_path_resolution.py: new test asserts no hardcoded city as .timezone fallback in save-session.sh. - Existing test_log_sh_config_uses_pipeline_dir relaxed to allow indented REMEMBER_CONFIG= lines (the new resolution branches). Verified end-to-end on macOS: prior save wrote "## 07:09 | main" (Paris), post-fix save writes "## 1:47 AM | branch" (Eastern, 12h). 258/258 pytest pass at 99.12% coverage. Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/log.sh | 14 ++++++++++- scripts/save-session.sh | 2 +- tests/test_log_sh.py | 43 ++++++++++++++++++++++++++++++++ tests/test_path_resolution.py | 47 ++++++++++++++++++++++++++++------- 4 files changed, 95 insertions(+), 11 deletions(-) diff --git a/scripts/log.sh b/scripts/log.sh index 96137a8..33edfcc 100644 --- a/scripts/log.sh +++ b/scripts/log.sh @@ -43,7 +43,19 @@ fi # Read .timezone from config.json BEFORE computing MEMORY_LOG_DATE — otherwise # TZ="" falls back to UTC on macOS/BSD and produces next-day filenames after # ~20:00 local in zones west of UTC. -REMEMBER_CONFIG="${PIPELINE_DIR:-${PROJECT_DIR:-.}/.claude/remember}/config.json" +# +# Path resolution covers three install layouts: +# 1. Marketplace cache: $PIPELINE_DIR/.claude/remember/config.json +# (~/.claude/plugins/cache//remember//.claude/remember/...) +# 2. Legacy/flat: $PIPELINE_DIR/config.json +# 3. Local install: $PROJECT_DIR/.claude/remember/config.json +if [ -n "$PIPELINE_DIR" ] && [ -f "$PIPELINE_DIR/.claude/remember/config.json" ]; then + REMEMBER_CONFIG="$PIPELINE_DIR/.claude/remember/config.json" +elif [ -n "$PIPELINE_DIR" ] && [ -f "$PIPELINE_DIR/config.json" ]; then + REMEMBER_CONFIG="$PIPELINE_DIR/config.json" +else + REMEMBER_CONFIG="${PROJECT_DIR:-.}/.claude/remember/config.json" +fi config() { local key="$1" local default="$2" diff --git a/scripts/save-session.sh b/scripts/save-session.sh index eac85f9..246de8f 100755 --- a/scripts/save-session.sh +++ b/scripts/save-session.sh @@ -59,7 +59,7 @@ source "$(dirname "$0")/detect-tools.sh" source "$(dirname "$0")/log.sh" log "hook" "save-session: PROJECT_DIR=$PROJECT_DIR PIPELINE_DIR=$PIPELINE_DIR PYTHON=$PYTHON" -REMEMBER_TZ=$(config ".timezone" "Europe/Paris") +REMEMBER_TZ=$(config ".timezone" "") REMEMBER_DATA="${PROJECT_DIR}/.remember" LOCK_FILE="${REMEMBER_DATA}/tmp/save.lock" diff --git a/tests/test_log_sh.py b/tests/test_log_sh.py index c84a209..1a0354e 100644 --- a/tests/test_log_sh.py +++ b/tests/test_log_sh.py @@ -236,6 +236,49 @@ def test_log_sh_timestamp_inside_file_uses_configured_tz(tmp_path): ) +def test_log_sh_marketplace_layout_finds_config_under_dot_claude_remember(tmp_path): + """Regression: marketplace installs put config at PIPELINE_DIR/.claude/remember/config.json. + + When PIPELINE_DIR is set (the marketplace case), log.sh must look for + config.json at ``$PIPELINE_DIR/.claude/remember/config.json``, not at + ``$PIPELINE_DIR/config.json`` directly. The marketplace cache layout + (``~/.claude/plugins/cache//remember//``) places the config + inside a ``.claude/remember/`` subdirectory next to the plugin code. + + Failure mode before the fix: REMEMBER_TZ resolves to "" (config not + found) → log lines and date computations fall through to system local + (or a hard-coded fallback in save-session.sh of "Europe/Paris"). + """ + plugin = tmp_path / "plugin" + (plugin / ".claude" / "remember").mkdir(parents=True) + (plugin / ".claude" / "remember" / "config.json").write_text( + '{"timezone": "America/Los_Angeles"}' + ) + project = tmp_path / "proj" + (project / ".remember" / "logs").mkdir(parents=True) + script = f""" + set -e + export PROJECT_DIR={project} + export PIPELINE_DIR={plugin} + source {LOG_SH} + echo "REMEMBER_TZ=$REMEMBER_TZ" + """ + result = subprocess.run( + ["bash", "-c", script], + env={**os.environ, "TZ": "UTC"}, + capture_output=True, text=True, + ) + assert result.returncode == 0, f"log.sh failed: {result.stderr}" + parsed = dict( + line.split("=", 1) for line in result.stdout.strip().splitlines() if "=" in line + ) + assert parsed.get("REMEMBER_TZ") == "America/Los_Angeles", ( + f"log.sh did not find config.json under PIPELINE_DIR/.claude/remember/. " + f"Got REMEMBER_TZ={parsed.get('REMEMBER_TZ')!r}. " + "This means marketplace installs silently lose their timezone (and time_format) settings." + ) + + def test_config_example_json_is_valid(): """config.example.json must be parseable JSON. diff --git a/tests/test_path_resolution.py b/tests/test_path_resolution.py index 9f7be63..25bdef8 100644 --- a/tests/test_path_resolution.py +++ b/tests/test_path_resolution.py @@ -2212,6 +2212,27 @@ def test_default_24h_when_no_config(self): "save-session.sh should default to '24h' when config key is absent" ) + def test_no_hardcoded_city_timezone_default(self): + """save-session.sh must not fall back to a hardcoded city when config is missing. + + Earlier versions defaulted REMEMBER_TZ to "Europe/Paris" — when a marketplace + install couldn't find config.json (resolved to wrong path), every timestamp + silently shifted to Paris time. The correct fallback is empty-string, which + log.sh's _remember_date helper translates to "system local" via a bare ``date`` + invocation (no ``TZ=...`` prefix). + """ + script_path = os.path.join( + os.path.dirname(__file__), "..", "scripts", "save-session.sh" + ) + with open(script_path) as f: + content = f.read() + for forbidden in ("Europe/Paris", "America/New_York", "America/Los_Angeles", "UTC"): + assert f'config ".timezone" "{forbidden}"' not in content, ( + f"save-session.sh hardcodes {forbidden} as the .timezone fallback. " + "Use \"\" so missing config falls through to system local instead " + "of silently shifting every timestamp to that city." + ) + class TestMarketplacePathResolution: """Issue #19: log.sh hardcodes paths relative to PROJECT_DIR/.claude/remember/. @@ -2229,15 +2250,23 @@ def test_log_sh_config_uses_pipeline_dir(self): with open(log_path) as f: content = f.read() - for line in content.split("\n"): - if line.startswith("REMEMBER_CONFIG="): - assert "PIPELINE_DIR" in line or "PLUGIN_ROOT" in line, ( - f"REMEMBER_CONFIG should use PIPELINE_DIR for marketplace " - f"compat, not hardcoded .claude/remember/. Line: {line}" - ) - break - else: - assert False, "REMEMBER_CONFIG not found in log.sh" + # Collect every line that assigns REMEMBER_CONFIG (may be inside an + # if/elif block, so allow leading whitespace — the original single-line + # version was at column 0 but the marketplace-aware version branches). + assignment_lines = [ + line for line in content.split("\n") + if line.lstrip().startswith("REMEMBER_CONFIG=") + ] + assert assignment_lines, "REMEMBER_CONFIG not found in log.sh" + # At least one assignment must reference PIPELINE_DIR (or PLUGIN_ROOT) + # so marketplace installs find their config. + assert any( + "PIPELINE_DIR" in line or "PLUGIN_ROOT" in line + for line in assignment_lines + ), ( + "REMEMBER_CONFIG should use PIPELINE_DIR for marketplace " + f"compat. Lines: {assignment_lines}" + ) def test_log_sh_hooks_dir_uses_pipeline_dir(self): """log.sh REMEMBER_HOOKS_DIR must use PIPELINE_DIR, not PROJECT_DIR."""