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."""