Skip to content

fix(log): resolve config.json path for marketplace installs#38

Closed
josemoreno801-netizen wants to merge 1 commit intoDigital-Process-Tools:mainfrom
josemoreno801-netizen:fix/log-sh-marketplace-config-path
Closed

fix(log): resolve config.json path for marketplace installs#38
josemoreno801-netizen wants to merge 1 commit intoDigital-Process-Tools:mainfrom
josemoreno801-netizen:fix/log-sh-marketplace-config-path

Conversation

@josemoreno801-netizen
Copy link
Copy Markdown
Contributor

Summary

  • Marketplace installs (~/.claude/plugins/cache/<mkt>/remember/<ver>/) keep config.json inside a .claude/remember/ subdir, but scripts/log.sh:46 was reading from $PIPELINE_DIR/config.json — one level too shallow. Every config lookup silently fell through to defaults.
  • Symptom save-session.sh and run-consolidation.sh fail when installed via plugin cache (PROJECT_DIR path traversal) #1: REMEMBER_TZ defaulted to "Europe/Paris" (per save-session.sh:62), so timestamps in today-*.md drifted by hours. A user in America/New_York would see ## 07:09 | branch written at 1:09 AM local because Haiku echoed the formatted Paris time straight back into the file.
  • Symptom Fix PROJECT_DIR path traversal when installed via plugin cache #2: time_format defaulted to "24h", so users who set "12h" never got AM/PM output.
  • Fix: REMEMBER_CONFIG now resolves through three branches — marketplace ($PIPELINE_DIR/.claude/remember/config.json), legacy flat ($PIPELINE_DIR/config.json, preserved for back-compat), then local-install ($PROJECT_DIR/.claude/remember/config.json).
  • Also hardened save-session.sh:62: its "Europe/Paris" fallback was actively harmful — log.sh:59 already exports REMEMBER_TZ="" which _remember_date translates to system-local, so a hardcoded Paris fallback would shift every timestamp if config ever genuinely went missing. Default now matches log.sh:59's empty-string convention.

Test plan

  • python3 -m pytest tests/ -q258 passed at 99.12% coverage
  • New test test_log_sh_marketplace_layout_finds_config_under_dot_claude_remember — sources log.sh with PIPELINE_DIR set and config under .claude/remember/, asserts REMEMBER_TZ resolves to the configured value (would fail before this fix)
  • New test test_no_hardcoded_city_timezone_default — guards against any future timezone-default landmines in save-session.sh
  • Existing test_log_sh_config_uses_pipeline_dir updated to accept indented REMEMBER_CONFIG= lines (the new resolution uses an if/elif/else block)
  • All other existing path-resolution and timezone tests still pass — three install layouts (marketplace, legacy flat, local) covered
  • bash scripts/run-tests.sh — 25 pass / 1 pre-existing fail (Test 7: save-session.sh --dry fails on bare main too, unrelated to this PR — separate CLAUDE_PROJECT_DIR export issue in scripts/run-tests.sh)
  • Live E2E on macOS: prior save wrote ## 07:09 | main (Paris, 24h). Post-fix save on the same machine writes ## 1:47 AM | fix/log-sh-marketplace-config-path (Eastern, 12h, AM/PM)

🤖 Generated with Claude Code

… installs

In marketplace cache layouts (~/.claude/plugins/cache/<mkt>/remember/<ver>/),
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) <noreply@anthropic.com>
fdaviddpt pushed a commit that referenced this pull request Apr 28, 2026
…pts (#38)

Resolves marketplace install path issue where config.json and hooks.d
were read from PROJECT_DIR instead of PIPELINE_DIR.

- log.sh: set PIPELINE_DIR with fallback, derive REMEMBER_CONFIG and REMEMBER_HOOKS_DIR from it
- user-prompt-hook.sh: source resolve-paths.sh for consistent path setup
- session-start-hook.sh: remove duplicate cfg(), use config() from log.sh
- post-tool-hook.sh: use config() instead of direct jq
- save-session.sh, run-consolidation.sh: remove redundant REMEMBER_TZ re-reads
- 67 new edge-case tests (marketplace layout, symlinks, config validation, unicode, coercion)

Based on issue reported by @josemoreno801-netizen in PR #38.

Co-Authored-By: Max <noreply>
Co-Authored-By: josemoreno801-netizen <josemoreno801-netizen@users.noreply.github.com>
@fdaviddpt
Copy link
Copy Markdown
Contributor

Thanks for reporting this and putting together the fix, @josemoreno801-netizen! Your PR helped us identify the full scope of the issue — we ended up unifying the config reader across all scripts and adding 67 edge-case tests on top of your original fix.

This is now resolved in v0.7.0 (commit 1d67a6d) with you credited as co-author. Closing in favor of the merged fix.

Appreciate the contribution!

@fdaviddpt fdaviddpt closed this Apr 28, 2026
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.

2 participants