cli(session): defer module-level _STATE_DIR expansion to runtime (#137) by armorbreak001 · Pull Request #146 · memtomem/memtomem

armorbreak001 · 2026-04-16T00:56:23Z

Summary

Fixes #137 — Replace module-level _STATE_DIR = Path("~/.memtomem").expanduser() with a lazy _state_dir() -> Path helper that resolves Path.home() at each call.

Problem

The old code captured ~ expansion once at import time:

_STATE_DIR = Path("~/.memtomem").expanduser()  # frozen at import!

Any subsequent monkeypatch.setenv("HOME", ...) in test code cannot relocate it, limiting test isolation for the session CLI.

Solution

def _state_dir() -> Path:
    """Return the memtomem state directory, resolving HOME at call time."""
    return Path.home() / ".memtomem"

_STATE_FILE still computed at module level (harmless — path construction only)
_STATE_DIR.mkdir(...) call site updated to _state_dir().mkdir(...)
Zero behavior change for production usage

Test Plan

Import succeeds: from memtomem.cli.session_cmd import _state_dir; _state_dir() returns valid Path
uv run mypy packages/memtomem/src/memtomem/cli/session_cmd.py — 0 errors
Existing tests pass: uv run pytest -m "not ollama" — 1272/1272 OK
No new # type: ignore lines

AI Usage Disclosure

None — human-authored.

Files Changed (1 file, +6/-3)

packages/memtomem/src/memtomem/cli/session_cmd.py

armorbreak001 · 2026-04-16T00:56:28Z

I have read the CLA Document and I hereby sign the CLA on this PR

github-actions · 2026-04-16T00:56:32Z

All contributors have signed the CLA. ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

…tomem#137) Replace module-level constant _STATE_DIR = Path('~').expanduser() with a lazy _state_dir() -> Path helper that calls Path.home() at each invocation. This allows test isolation via monkeypatch.setenv(). - _STATE_DIR usages at mkdir() call site updated to _state_dir() - _STATE_FILE still computed at module level (harmless: only used for path construction, not for existence/mkdir) mypy: 0 errors (no new type:ignore)

armorbreak001 · 2026-04-16T00:59:55Z

I have read the CLA Document and I hereby sign the CLA on this PR

armorbreak001 · 2026-04-16T01:06:37Z

recheck

armorbreak001 · 2026-04-16T01:10:30Z

I have read the CLA Document and I hereby sign the CLA

armorbreak001 · 2026-04-16T01:10:33Z

recheck

memtomem

Thanks for identifying the frozen-at-import issue — the fix direction is right, but there's an inconsistency that undermines the test-isolation goal:

`_STATE_FILE` is still resolved at import time:

def _state_dir() -> Path:
    return Path.home() / ".memtomem"

_STATE_FILE = _state_dir() / ".current_session"  # ← called once at import

After this PR, `_write_current_session` does:

_state_dir().mkdir(...)   # resolves HOME dynamically ✓
_STATE_FILE.write_text()  # uses the import-time path ✗

If a test patches `HOME`, the `mkdir` creates the directory under the new home, but `write_text` / `read_text` / `unlink` still target the original home. Same issue in `_read_current_session` and `_clear_current_session`.

Suggested fix — make both lazy:

def _state_dir() -> Path:
    return Path.home() / ".memtomem"

def _state_file() -> Path:
    return _state_dir() / ".current_session"

Then replace the 4 `_STATE_FILE` references (lines 20, 28, 33) with `_state_file()`. This keeps the same production behavior while actually making test isolation work end-to-end.

CLA note: please re-sign the CLA per the comment on #145 — the earlier signature from #130 didn't persist due to a now-fixed infrastructure issue (#143).

memtomem · 2026-04-16T05:20:03Z

Thanks for tackling #137! Flagging a subtle issue before merge:

_STATE_FILE module-level computation still freezes ~

The new _state_dir() -> Path helper is lazy, but line 14 still evaluates at import time:

_STATE_FILE = _state_dir() / ".current_session"  # Path.home() called once, result frozen

_read_current_session / _write_current_session / _clear_current_session all reference _STATE_FILE for the actual file ops, so monkeypatch.setenv("HOME", ...) can't redirect them. Only _write_current_session's mkdir() resolves at runtime — meaning a test under a patched HOME would create the directory in the new location but write/read the file from the original. That's the exact isolation gap #137 asks to close.

Suggested fix — mirror _state_dir with a lazy file accessor:

def _state_file() -> Path:
    return _state_dir() / ".current_session"

Then replace _STATE_FILE with _state_file() in the three helpers.

Testing suggestion — add a round-trip test to lock the behavior in:

def test_session_state_respects_home_override(tmp_path, monkeypatch):
    monkeypatch.setenv("HOME", str(tmp_path))
    _write_current_session("abc-123")
    assert _state_file().exists()
    assert _read_current_session() == "abc-123"

This proves the fix end-to-end (not just mkdir) and guards against regression. Happy to re-review once updated!

memtomem

Nice idea — deferring HOME resolution is the right fix for test isolation.

One issue: _STATE_FILE is still computed at module level:

_STATE_FILE = _state_dir() / ".current_session"

This calls _state_dir() (and therefore Path.home()) at import time, so _STATE_FILE is frozen. _read_current_session and _clear_current_session both use _STATE_FILE directly, so monkeypatching HOME after import won't affect them.

Suggestion — make _STATE_FILE lazy too:

def _state_file() -> Path:
    return _state_dir() / ".current_session"

Then replace _STATE_FILE references in _read_current_session, _write_current_session, and _clear_current_session with _state_file(). That completes the deferral.

@memtomem

Per @memtomem's review: _STATE_FILE was still computed at module level via _state_dir() / '.current_session', which calls Path.home() at import time. This defeats the test-isolation goal of memtomem#137. Changed to a _state_file() -> Path function so every call site resolves HOME at call time, not at import time. Replaces: _STATE_FILE (module-level) → _state_file() (lazy)

armorbreak001 · 2026-04-16T09:55:27Z

Thanks @memtomem — great catch! I made _state_dir() lazy but forgot that _STATE_FILE = _state_dir() / ...' still evaluates at module level.

Fixed (pushed):

- _STATE_FILE = _state_dir() / ".current_session"
+ def _state_file() -> Path:
+     return _state_dir() / ".current_session"

All 4 call sites updated (_read_current_session, _write_current_session, _clear_current_session + implicit). mypy passes clean.

memtomem

Looks good — both _state_dir() and _state_file() are now lazy, completing the HOME deferral. Thanks for the quick fix!

armorbreak001 force-pushed the fix/defer-state-dir-expansion branch from f7f7fac to f4bd68a Compare April 16, 2026 00:59

github-actions Bot added a commit that referenced this pull request Apr 16, 2026

@armorbreak001 has signed the CLA in #146

6e3dc94

memtomem mentioned this pull request Apr 16, 2026

typing: tighten 4 incomplete list/dict element-type annotations (#138) #145

Merged

memtomem reviewed Apr 16, 2026

View reviewed changes

memtomem mentioned this pull request Apr 16, 2026

fix: improve Windows compatibility — enforce UTF-8 encoding #162

Merged

memtomem requested changes Apr 16, 2026

View reviewed changes

memtomem mentioned this pull request Apr 16, 2026

Log warning instead of silently swallowing exceptions in activity hook #157

Closed

3 tasks

memtomem approved these changes Apr 16, 2026

View reviewed changes

memtomem merged commit badd22a into memtomem:main Apr 16, 2026
5 checks passed

github-actions Bot locked and limited conversation to collaborators Apr 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli(session): defer module-level _STATE_DIR expansion to runtime (#137)#146

cli(session): defer module-level _STATE_DIR expansion to runtime (#137)#146
memtomem merged 2 commits intomemtomem:mainfrom
armorbreak001:fix/defer-state-dir-expansion

armorbreak001 commented Apr 16, 2026

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

github-actions Bot commented Apr 16, 2026 •

edited

Loading

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

memtomem left a comment

Uh oh!

memtomem commented Apr 16, 2026

Uh oh!

memtomem left a comment

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

memtomem left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

armorbreak001 commented Apr 16, 2026

Summary

Problem

Solution

Test Plan

AI Usage Disclosure

Files Changed (1 file, +6/-3)

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

github-actions Bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

memtomem left a comment

Choose a reason for hiding this comment

Uh oh!

memtomem commented Apr 16, 2026

Uh oh!

memtomem left a comment

Choose a reason for hiding this comment

Uh oh!

armorbreak001 commented Apr 16, 2026

Uh oh!

memtomem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 16, 2026 •

edited

Loading