Phase 1 of the audit-log re-enablement plan. Adds three opt-in env
vars that let us ship one instrumented build for the user to run,
without flipping the kill switch on for everybody. **Default
behaviour is byte-identical to today**: with no env vars set the
kill switch wins, no writer thread starts, no file is written, no
stderr line is printed.
Env vars (do NOT set in prod):
- ``DATATOOLS_AUDIT_ENABLED=1`` — bypass ``_DISABLED`` for one
session. ``_DISABLED = True`` stays in the source so an upgrade
with no env var is still safe.
- ``DATATOOLS_AUDIT_TRACE=1`` — print ``[audit] ...`` lines to
stderr at module import, every writer-thread state change, and
every producer entry point. Lets the user share a small log
instead of attaching a debugger.
- ``DATATOOLS_AUDIT_PROBE=<value>`` — bisect the producer path
for Phase 2. Values: ``full`` (default), ``noop``, ``no-events``,
``no-page-open``, ``no-session-start``. The named variants
return early from the corresponding ``log_*`` function so we can
isolate which call is implicated in the blank-pages symptom.
Also:
- ``_writer_loop`` gets an outer ``try/except BaseException`` so
silent thread death now surfaces a ``"writer thread died: ..."``
line in the launcher terminal instead of looking like a hang.
- Existing first-write-failure stderr print gets ``flush=True`` so
the user actually sees it before the process is killed.
- Test fixture switches from the previous-commit ``_DISABLED = False``
override to ``_ENABLE_OVERRIDE = True`` so tests exercise the same
bypass path the diagnostic build uses.
- Two new tests pin the safety contract: with the kill switch on
and no override, every producer is a true no-op (no writer
thread, no file). And ``DATATOOLS_AUDIT_PROBE=no-events`` bypasses
``log_event`` even when the override is on — guards the bisect.
Rollback: ``git revert HEAD`` removes Phase 1 cleanly. The deadlock
fix from the previous commit stays in place.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-existing latent bug since d9e32e5: ``audit_log_path()`` acquires
the non-reentrant ``_LOCK`` and, while holding it, calls
``_session_id()`` which also takes ``_LOCK``. On a clean module state
(both ``_LOG_PATH`` and ``_SESSION_ID`` unset) the first caller
deadlocks.
``log_session_start`` triggers it in practice — it's the first GUI
call after import and the ``log_file=str(audit_log_path())`` arg is
evaluated before any ``log_event`` has had a chance to lazy-init the
session id. Strong candidate contributor to the blank-pages symptom
the kill switch was put back to mask: the writer thread (and any
producer reaching ``audit_log_path``) would freeze forever, and
Ctrl+C would not free the GIL — matches the launcher-can't-be-killed
behaviour reported in 1caedbb.
Fix: resolve the session id BEFORE acquiring ``_LOCK`` in
``audit_log_path``. ``_session_id`` already double-checks under its
own lock, so the call is safe and self-synchronising.
Test fixture in ``tests/test_audit.py`` now bypasses the kill switch
via ``monkeypatch.setattr(audit, "_DISABLED", False)`` — env vars are
captured at import time and ``monkeypatch.setenv`` won't reach the
module-level flag. With the fix in place, all 6 tests pass in 0.15s;
without it, ``test_session_start_renders`` (and any test exercising
the log_session_start path) hangs indefinitely.
Kill switch behaviour is unchanged in production (`_DISABLED = True`
in the shipped module); this is purely a correctness fix for the
code path that gets exercised when the switch is off.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reported earlier: synchronous file writes in ``log_event`` blocked
the GUI render thread on hostile filesystems (Windows antivirus on
``~/.datatools/logs/`` is the prime suspect). A blocking ``open``
call doesn't raise — try/except can't recover from it — so the
only safe re-enable is to take file I/O off the render path.
Refactor:
- ``log_event`` and friends push events onto a ``deque(maxlen=5000)``
via ``put_nowait`` and return in microseconds.
- A single daemon thread (``datatools-audit-writer``) drains the
queue and writes batches. Holds the queue lock only long enough to
snapshot + clear, then does I/O outside the lock so producers can
keep enqueueing.
- ``audit_log_path()`` is now pure path arithmetic — no ``mkdir``
no ``open``. The writer thread does the directory creation off
the request path, so any hang there only affects the writer.
- Bounded queue means an unwritable disk doesn't unbounded-grow
memory; the queue caps at 5000 and overflow drops OLDEST events
so the most-recent (most-diagnostic) ones survive.
- First write failure prints once to stderr; subsequent failures
are silent so logs don't drown the launcher terminal.
- ``flush_audit_log(timeout_s=0.5)`` drains the queue and signals
the writer to exit; bounded so a stuck disk can't delay shutdown.
Other changes in this commit:
- ``shutdown_app`` now emits a "Session ending" event and calls
``flush_audit_log`` before kicking the os._exit timer, so the
closing session's events make it to disk.
- The Diagnostics sidebar in ``hide_streamlit_chrome`` is
re-enabled (the ``if False:`` gate is removed). Wrapped in
try/except defensively — render errors print to stderr, never
blank the page.
- ``_DISABLED`` kill-switch is gone. The async design IS the
safety mechanism now.
Tests in ``tests/test_audit.py``:
- log_event burst of 1000 events completes in well under 1s
(proves non-blocking).
- Events queued before flush land on disk with the expected JSON
shape; session_start renders; idempotent.
- Pointing the audit dir at a file (so mkdir fails) doesn't hang
or crash the producer.
- Non-JSON extras are str()-coerced rather than dropped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
