Skip to content

fix(hooks): guard fcntl import on Windows — _shared + session_start#345

Open
Huntehhh wants to merge 2 commits into
buildingjoshbetter:mainfrom
Huntehhh:fix/hooks-fcntl-windows-portability
Open

fix(hooks): guard fcntl import on Windows — _shared + session_start#345
Huntehhh wants to merge 2 commits into
buildingjoshbetter:mainfrom
Huntehhh:fix/hooks-fcntl-windows-portability

Conversation

@Huntehhh
Copy link
Copy Markdown
Contributor

@Huntehhh Huntehhh commented May 16, 2026

Summary

truememory/ingest/hooks/_shared.py and truememory/ingest/hooks/session_start.py
both called bare import fcntl at module top with no try / except ImportError
guard. fcntl is POSIX-only — on Windows every import raised
ModuleNotFoundError before any function ran. That cascaded to every Claude
Code hook subprocess that imports from _shared (SessionStart,
UserPromptSubmit, Stop), silently breaking the hook-based memory-extraction
pipeline on Windows.

Fix mirrors the established _HAS_FCNTL flag pattern already used by
ingest/pipeline.py, hooks/core.py, and ingest/hooks/user_prompt_submit.py.
Both call sites (_shared.check_extraction_budget and
session_start._scan_stale_sessions) now gate their fcntl.flock calls on
_HAS_FCNTL. POSIX behavior is unchanged; Windows falls through to the
non-locking path with documented worst-case race semantics (1-2 extra
extractions per hour for the budget check, deduplicated overlap for the stale
scan).

This is a sibling fix to the WNOHANG guard in the cold-start resilience PR
(if that one is open) — same POSIX-only-API class of bug, same hasattr/
try-except-guard pattern.

Changes

File Change
truememory/ingest/hooks/_shared.py Bare import fcntltry / except ImportError + _HAS_FCNTL flag; check_extraction_budget guards fcntl.flock(LOCK_EX) on the flag; documented fallback semantics
truememory/ingest/hooks/session_start.py Bare import fcntl → same _HAS_FCNTL guard; _scan_stale_sessions guards fcntl.flock(LOCK_EX | LOCK_NB) on the flag with inline comment explaining the dedup path
tests/ingest/test_hooks_windows_portability.py New file (8 tests): _HAS_FCNTL flag contract on both modules, check_extraction_budget behaves correctly without fcntl (allows, enforces cap, resets hourly), POSIX-only regression lock that fcntl.flock(LOCK_EX) is still acquired on platforms where it's available
CHANGELOG.md Unreleased entry documenting the bug, fix, and test coverage

Test Plan

  • python -m pytest tests/ingest/test_hooks_windows_portability.py -v → 7 passed + 1 skipped on Windows (the POSIX regression lock); 8 passed on POSIX
  • On Windows: python -c "from truememory.ingest.hooks import _shared, session_start, user_prompt_submit; print('OK')" → exits 0
  • Trigger a Claude Code SessionStart hook on Windows and confirm no ModuleNotFoundError: No module named 'fcntl' in stderr — the hook should run to completion and inject memory context
  • On POSIX, spawn two concurrent python -c "from truememory.ingest.hooks._shared import check_extraction_budget; print(check_extraction_budget())" and confirm the budget counter increments correctly (i.e. the lock is still doing its job)

Design Notes

  • Why fall back to non-atomic read-modify-write on Windows instead of a
    named mutex?
    The Windows-specific Mutex API (win32event.CreateMutex /
    pywin32) would add a non-stdlib dependency for a code path whose
    worst-case bug (1-2 extra extractions per hour, never a corruption) is
    far below the threshold that justifies a new dependency.
  • Why skip the non-blocking scan lock on Windows instead of polling a
    filesystem-marker file?
    The orphaned-session scan only runs every 15
    minutes per session-start; even if two scans run concurrently, the
    backlog drainer's atomic .json → .processing rename deduplicates any
    overlapping work. Adding a marker-file lock would be more code with
    similar race characteristics.
  • Why not delete the entire scan on Windows? Hooks should have the
    same observable behavior across platforms wherever the trade-off is
    acceptable. Disabling functionality on Windows would create a
    silent-feature-gap that's harder to debug than a documented race window.

Co-Authored-By: claude-opus-4-7 wontreply@getfucked.ai

Merge ordering

Status: MERGEABLE (clean against origin/main).

Blocks: #351 (Windows subprocess portability — session_start.py:~226 Popen edit needs this PR's _HAS_FCNTL guard at file top to be in place first).

Depends on: none. Disjoint from #344 / #346 / #347 / #348 / #349 / #350 / #352 / #353.

Recommended sequence position: between #344 and #346.

Full sequence (10 PRs from a 3-agent coordination):

#353 (CI runner) → #344 → #345 (this) → #346 → #351 → #348 → #352 → #347 → #349 → #350

Huntehhh and others added 2 commits May 16, 2026 17:12
truememory/ingest/hooks/_shared.py and session_start.py both had bare
`import fcntl` at module top with no `try / except ImportError` guard.
fcntl is POSIX-only — on Windows every import raised ModuleNotFoundError
before any function ran, which cascaded to every Claude Code hook
subprocess that imports from _shared (SessionStart, UserPromptSubmit,
Stop). Silent failure mode: no memory extraction after sessions, no
context injection at session start, no buffer writes on user prompts.

Fix mirrors the established `_HAS_FCNTL` flag pattern already used by
ingest/pipeline.py, hooks/core.py, and ingest/hooks/user_prompt_submit.py.
The two affected call sites:

- _shared.check_extraction_budget — POSIX uses fcntl.flock(LOCK_EX) for
  atomic read-modify-write across concurrent ingest processes. Windows
  falls back to non-atomic R/M/W; worst-case race window allows 1-2
  extra extractions per hour, acceptable given the alternative is no
  enforcement at all.

- session_start._scan_stale_sessions — POSIX uses a non-blocking advisory
  lock to prevent concurrent scans. Windows skips the lock; the backlog
  drainer's atomic .json → .processing rename already deduplicates any
  overlapping work.

Tests in tests/ingest/test_hooks_windows_portability.py (8 tests) pin
the _HAS_FCNTL flag contract on both modules, verify check_extraction_budget
behaves correctly without fcntl (allows, enforces cap, resets hourly),
and include a POSIX-only regression lock that fcntl.flock(LOCK_EX) is
still acquired on platforms where it's available — so a future refactor
can't silently drop cross-process coordination.

Co-Authored-By: claude-opus-4-7 <wontreply@getfucked.ai>
Documents the bug (every Claude Code hook crashing on Windows), the fix
(_HAS_FCNTL guard pattern matching the rest of the codebase), and the
test coverage (8 regression tests).

Co-Authored-By: claude-opus-4-7 <wontreply@getfucked.ai>
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.

1 participant