fix: P0 council bug fixes (atomic writes, BRAIN_DIR hard-fail, thread-safety) [rebased]

[Gradata]

Clean rebase of #145. Cherry-picked only the meaningful Phase B commit(s) onto current main; drops 41 stale m1 commits already merged via #144.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 859dbad4-2ff5-4bcd-bf77-32bee1c9461c

📥 Commits

Reviewing files that changed from the base of the PR and between 14de072 and e239641.

📒 Files selected for processing (17)

• Gradata/CHANGES.md
• Gradata/REPORT.md
• Gradata/src/gradata/__init__.py
• Gradata/src/gradata/_atomic.py
• Gradata/src/gradata/_brain_lock.py
• Gradata/src/gradata/_doctor.py
• Gradata/src/gradata/daemon.py
• Gradata/src/gradata/exceptions.py
• Gradata/src/gradata/hooks/_base.py
• Gradata/src/gradata/hooks/implicit_feedback.py
• Gradata/src/gradata/mcp_server.py
• Gradata/src/gradata/rules/rule_graph.py
• Gradata/tests/test_brain_dir_required.py
• Gradata/tests/test_brain_lock.py
• Gradata/tests/test_implicit_feedback.py
• Gradata/tests/test_import_integrity.py
• Gradata/tests/test_rule_graph_atomic.py

---

📝 Walkthrough

Summary

• Atomic File Writes: New atomic_write_text() utility ensures data integrity for RuleGraph persistence via temp file + fsync + atomic replacement pattern
• New Exception Hierarchy: Introduced GradataError base class with BrainNotConfiguredError and BrainLockedError subclasses; all exported in public API
• Brain Lock Implementation: Added cross-process/thread locking via acquire_brain_lock() context manager (POSIX fcntl + Windows msvcrt support) with in-process duplicate-lock guard
• Hard-Fail on Missing BRAIN_DIR: Brain configuration errors no longer silently suppressed; implicit_feedback hook and doctor check now explicitly propagate BrainNotConfiguredError
• Daemon/MCP Server Lock Acquisition: Both GradataDaemon.start() and run_server() now acquire brain lock for their entire lifetime
• Breaking Change: BrainNotConfiguredError now propagates instead of being caught; code relying on silent failures will need updates
• New Tests: Added test coverage for atomic writes, brain locking, import integrity, and hard-fail BRAIN_DIR behavior
• Public API Additions: Exported BrainLockedError, BrainNotConfiguredError, GradataError from main package

Walkthrough

This PR implements four P0 improvements: atomic writes for rule_graph.json via a new atomic_write_text() utility, converting missing BRAIN_DIR into hard failures with new exception types, adding subprocess and Brain.init() smoke tests for import integrity and initialization, and implementing cross-process/thread brain directory locking with OS-specific file locking plus in-process guards.

Changes

Cohort / File(s)	Summary
Documentation Gradata/CHANGES.md, Gradata/REPORT.md	Documents four P0 work items (atomic writes, hard BRAIN_DIR failure, smoke tests, brain directory locking) and records Phase B fix progress and constraints encountered during implementation.
Exception Hierarchy & Exports Gradata/src/gradata/exceptions.py, Gradata/src/gradata/__init__.py	Introduces GradataError base class, re-roots BrainError to inherit from it, adds BrainNotConfiguredError and BrainLockedError subclasses, and re-exports all three new exception types from package __init__.
Atomic Write Utility Gradata/src/gradata/_atomic.py	Adds atomic_write_text() function and helper _fsync_dir() to perform atomic file writes via temp file + os.replace() with directory fsync on non-Windows platforms.
Brain Locking Mechanism Gradata/src/gradata/_brain_lock.py	Implements acquire_brain_lock() context manager that enforces single-process exclusivity via in-process threading lock and OS-level file locking (fcntl.flock/msvcrt.locking on <brain_dir>/.brain.lock), raising BrainLockedError on contention.
Core Modules Gradata/src/gradata/rules/rule_graph.py, Gradata/src/gradata/_doctor.py, Gradata/src/gradata/daemon.py, Gradata/src/gradata/mcp_server.py	Updates RuleGraph.save() to use atomic writes; changes doctor brain_dir check from skip to fail state; integrates brain lock acquisition and proper cleanup into daemon startup and MCP server lifecycle; ensures lock lifetime aligns with service processing.
Hook Runners Gradata/src/gradata/hooks/_base.py, Gradata/src/gradata/hooks/implicit_feedback.py	Makes run_hook() propagate BrainNotConfiguredError without suppression; adds explicit brain directory validation and dedicated exception handling in implicit_feedback.main() to ensure configuration errors escape the generic exception handler.
Tests Gradata/tests/test_brain_dir_required.py, Gradata/tests/test_brain_lock.py, Gradata/tests/test_implicit_feedback.py, Gradata/tests/test_import_integrity.py, Gradata/tests/test_rule_graph_atomic.py	Adds five test files covering brain configuration requirement enforcement, brain lock exclusivity, implicit feedback error propagation, public import integrity and Brain.init() smoke test, and atomic save regression for RuleGraph.

Sequence Diagram(s)

sequenceDiagram
    participant Daemon as GradataDaemon.start()
    participant Lock as acquire_brain_lock()
    participant FS as File System
    participant Server as HTTP Server Lifecycle

    Daemon->>Lock: acquire_brain_lock(brain_dir)
    activate Lock
    Lock->>FS: Open/create .brain.lock file
    Lock->>FS: flock/msvcrt.locking (exclusive, non-blocking)
    alt Lock Acquired
        Lock->>FS: Truncate & write PID, fsync
        Lock-->>Daemon: Return context manager
        Daemon->>Server: Start within lock context
        Server->>Server: Port selection, bind, workers
        Server->>Server: serve_forever()
        Server-->>Daemon: Complete/Exception
        Daemon->>Lock: Exit context (via finally)
        Lock->>FS: Unlock OS lock, close file
        deactivate Lock
    else Lock Unavailable (BrainLockedError)
        Lock-->>Daemon: Raise BrainLockedError
        Daemon->>Daemon: Cleanup resources
    end

Loading

sequenceDiagram
    participant Hook as Hook Runner (run_hook)
    participant FB as implicit_feedback.main()
    participant Resolve as resolve_brain_dir()
    participant Signals as Signal Detection

    Hook->>FB: Execute hook
    activate FB
    FB->>Signals: _detect_signals()
    alt Signals Found
        FB->>Resolve: resolve_brain_dir()
        alt BRAIN_DIR Resolved
            FB->>FB: Persist feedback
            FB-->>Hook: Return None
        else BRAIN_DIR Unresolved
            FB-->>Hook: Raise BrainNotConfiguredError
            Hook->>Hook: Re-raise (no suppression)
        end
    else No Signals
        FB-->>Hook: Return None (early exit)
    end
    deactivate FB

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: daemon + detection layer + plugin tests #18 — Modifies daemon and MCP server lifecycle; overlaps with brain-locking integration and lifecycle cleanup in daemon.py and mcp_server.py.

Suggested labels

bug

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rebase/council-phase-b

---

_{Review rate limit: 3/5 reviews remaining, refill in 24 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.