Skip to content

fix(state): preserve SQLite locks during macOS permission hardening - #109752

Closed
JiehoonKwak wants to merge 1 commit into
NousResearch:mainfrom
JiehoonKwak:fix/macos-state-db-permission-locks
Closed

JiehoonKwak wants to merge 1 commit into
NousResearch:mainfrom
JiehoonKwak:fix/macos-state-db-permission-locks

Conversation

@JiehoonKwak

Copy link
Copy Markdown

What does this PR do?

Preserves SQLite locks when SessionDB hardens state.db and its sidecar permissions on macOS. After #109509, the open/fchmod/close sequence can release locks belonging to existing SQLite connections in the same process. A competing connection can then remove live WAL/SHM files, leaving running sessions with DeletedWalGenerationError.

The shared permission helper now uses macOS pathname chmod with follow_symlinks=False for existing regular files. Only a missing database is opened, with exclusive creation and mode 0600. Existing symlinks and other non-regular files remain rejected. This fixes all macOS SessionDB callers, including gateway and Dashboard clients, without a client-specific workaround or a new dependency. Linux and Windows behavior is unchanged.

Related Issue

Related to #109728 and the macOS symptoms in #109641. Complements the Linux-only O_PATH implementation in #109734; macOS has no O_PATH, and an O_EVTONLY descriptor also releases these locks when closed. This does not implement the separate macOS preconnect deleted-sidecar detector requested in #109641.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • Route macOS permission hardening to the existing hermes_state_dbfile.py module.
  • Preserve private creation and permissions without opening/closing existing SQLite files.
  • Add native macOS regression coverage using a real competing process in WAL and DELETE journal modes, plus private creation and symlink rejection checks.

How to Test

scripts/run_tests.sh -j 3 --file-retries 0 tests/test_state_permission_locks_macos.py

On macOS 26.6, both lock tests fail on unmodified main at b6b53c6 and pass with this patch. The competing writer stays blocked after permission hardening in both journal modes. All three new cases pass.

Broader focused run:

scripts/run_tests.sh -j 3 --file-retries 0 tests/test_state_permission_locks_macos.py tests/test_hermes_state.py tests/tools/test_async_delegation.py tests/hermes_state/test_deleted_wal_generation_guard.py tests/hermes_state/test_deleted_wal_checkpoint_guard.py

Result: 318 passed, 14 skipped, 1 failed. The failure is TestPerformancePragmasEndToEnd::test_configured_pragmas_reach_all_connection_types (read pool is None). The same full-file failure reproduces in an isolated worktree at unmodified b6b53c6; that test passes alone on both revisions. It is an existing test-order problem, not resolved by this patch.

Also verified with the installed Python 3.11 / SQLite 3.53.1 runtime: two SessionDB handles, repeated external SQLite reads and closes, then a session write and cross-handle readback; WAL/SHM inodes stayed unchanged. Ruff and git diff --check pass.

Checklist

Code

  • I've read the Contributing Guide.
  • My commit message follows Conventional Commits.
  • I searched existing PRs; the overlapping Linux-only fix is linked above.
  • This PR contains only this fix and its regression tests.
  • I've run the complete test suite and all tests pass (focused results and baseline failure documented above).
  • I've added tests for the change.
  • I've tested on macOS 26.6 (Apple Silicon).

Documentation & Housekeeping

  • Relevant helper docstring updated; no user-facing configuration change.
  • Config example, architecture instructions, and tool schemas: N/A.
  • Cross-platform impact considered; the new implementation is macOS-only.

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 13, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Superseded by #109841 (merged, e16f686): platform-independent fix to the same _secure_state_db_files helper — chmod(2) on the path for existing files, O_EXCL so only a brand-new inode ever gets a descriptor. Thank you for the clear analysis; it converged on the same mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants