Address code-review findings on the #1283 log-path fix

Follow-ups to the review of PR #1415. The substantive one: degrading to a
NullHandler was completely silent. The 'django' logger has only the 'file'
handler and the 'website' logger's console handler is gated by
require_debug_true (False in prod), so an unwritable log dir means the server
runs blind — and there is no console access on -test or prod to notice it.

Surface the degraded state two web-reachable ways:
- /version.json gains log_to_file + log_file (scriptable deploy check).
- The admin dashboard gains a superuser-only warning callout naming the path
  that failed. Red-tinted so it doesn't read as another amber tip box; both
  light and dark variants clear WCAG AA (ratios noted inline).

Also from the review:
- Rename _log_dir_is_writable -> _ensure_log_dir_writable; the function calls
  os.makedirs, so a pure-predicate name was misleading. Document the os.access
  root caveat (the deployed container runs as apache, so the guard still bites).
- Extract _file_log_handler() so both branches are testable and the LOGGING
  literal loses its two-dict inline ternary. LOGGING is evaluated once at
  import, so the degrade branch was otherwise untestable — settings_test.py
  swaps the handler before any test runs.
- Cross-reference LOG_DIR and MEDIA_ROOT at both sites, and add a test pinning
  LOG_FILE to MEDIA_ROOT/debug.log — the contract the log's readability rests on.
- Document ML_LOG_DIR (previously undocumented) in DEPLOYMENT.md and CLAUDE.md.

Correcting a stale premise while here: the /logs/ URL that settings.py and
DEPLOYMENT.md cite as the way to read debug.log 404s on BOTH prod and test
(verified by curl, 2026-07-28). SSH to the shared filesystem is the reliable
path; the docs now say so. The INFO-not-DEBUG conservatism stays, since the
file does still live in a web-served tree.

Tests: 724 pass. New coverage for the helper's dir creation, both handler
branches, the MEDIA_ROOT contract, the version.json fields, and the admin
callout (shown to superusers when degraded, hidden otherwise).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: jonfroehlich

CLAUDE.md

@@ -130,6 +130,7 @@ the existing viewset/serializer pattern and keep `v1` fields additive-only
 - **Prod/test `config.ini` has only a `[Django]` section — no `[Postgres]` section.** Per `settings.py`, a missing `[Postgres]` section means Django uses the fallback `DATABASES` default (`HOST='db'`) — i.e. the dockerized `db` service of the active compose file. A `[Postgres]` section, if added, would override it. So the DB is the in-stack `db` container in **every** environment (no external Postgres); on the servers that's the `db` service in `docker-compose.yml`.
 - `DEBUG` resolution order: `DJANGO_ENV=PROD` forces False → `config.ini [Django] DEBUG` → `DJANGO_ENV=DEBUG` forces True → default False.
 - `TIME_ZONE = 'America/Los_Angeles'`. `ML_WEBSITE_VERSION` in settings is shown in the admin header and used in release tagging.
+- **Logging (#1283):** `debug.log` lives at `LOG_DIR/debug.log`, where `LOG_DIR` is `$ML_LOG_DIR` or `<BASE_DIR>/media` (`/code/media` in the container). Keep it inside `MEDIA_ROOT` — the web-served `/logs/debug.log` depends on that. `ML_LOG_DIR` is unset everywhere today; it exists for non-`/code` hosts. If the dir isn't writable the file handler degrades to a `NullHandler` rather than crashing `django.setup()`, and since there's no console on the servers that state surfaces via `/version.json` (`log_to_file`) and a superuser-only callout on the admin dashboard.
 
 ### Container startup side effects (`docker-entrypoint.sh`)
 

docs/DEPLOYMENT.md

@@ -188,6 +188,64 @@ email.
 | `httpd-access.log` | HTTP request logs | On the Docker host — web `/logs/` URL. |
 | `httpd-error.log` | HTTP error logs | On the Docker host — web `/logs/` URL. |
 
+#### Where `debug.log` is written, and what happens if that fails (#1283)
+
+The path is derived in `makeabilitylab/settings.py`:
+
+```
+LOG_DIR  = $ML_LOG_DIR, defaulting to <BASE_DIR>/media   # /code/media in the container
+LOG_FILE = $LOG_DIR/debug.log                            # /code/media/debug.log
+```
+
+`LOG_DIR` must stay inside `MEDIA_ROOT`, because that is the directory bind-mounted
+out to the shared CSE filesystem — it's what makes `debug.log` readable over SSH at
+`/cse/web/research/makelab/www/debug.log` (prod) and `www-test/debug.log` (test).
+
+> **Note:** `https://<host>/logs/debug.log` **404s on both prod and test** as of
+> 2026-07-28 (verified with `curl`; it falls through to Django's custom 404). The
+> web-URL rows in the table above are stale. SSH is the reliable path — see
+> "Reading `debug.log` over SSH" below. Because the log nonetheless lives in a
+> web-served tree, we still log at INFO rather than DEBUG when `DEBUG` is off.
+
+- **`ML_LOG_DIR`** is an optional environment override for hosts that don't use
+  `/code`. It is **not set** on prod, test, or local dev, and shouldn't need to
+  be. If you do set it outside `MEDIA_ROOT`, the web `/logs/` URL stops working.
+- **If the log directory can't be created or written**, Django does *not* crash
+  (it used to: `LOGGING` is evaluated at `django.setup()`, so a bad path killed
+  startup before a single request). The file handler degrades to a `NullHandler`
+  instead — meaning the server runs fine but **writes no logs at all**.
+
+Because a degraded state is otherwise invisible (there is no console access on
+these servers), it is surfaced two ways:
+
+1. **`/version.json`** → `"log_to_file": false` and `"log_file": "<path that failed>"`.
+2. **The `/admin/` dashboard** → a warning callout, shown to superusers only.
+
+### Verifying logging after a deploy
+
+`log_to_file: true` only means the *directory* was writable at startup, so check
+both the flag and that records are actually landing on disk:
+
+```bash
+HOST=https://makeabilitylab-test.cs.washington.edu   # or the prod host
+curl -s $HOST/version.json | python3 -m json.tool
+# expect: "log_to_file": true, "log_file": "/code/media/debug.log",
+#         and a "git_sha" matching the commit you pushed
+```
+
+Match `git_sha` — **not `built_at`**, which has shown fresh on a stuck auto-deploy
+serving stale code. Then confirm records are really being written (the web `/logs/`
+URL 404s, so this has to be SSH):
+
+```bash
+ssh makelab1                                          # or makelab2 / recycle
+ls -l /cse/web/research/makelab/www-test/debug.log     # www/ for prod
+tail -5 /cse/web/research/makelab/www-test/debug.log   # timestamps after the deploy
+```
+
+The log rotates at 5 MB, so check `debug.log.1` too when hunting a container-start
+sequence.
+
 ### Accessing Logs via Web
 
 - **Test:** https://makeabilitylab-test.cs.washington.edu/logs/

makeabilitylab/settings.py

@@ -86,8 +86,8 @@
     SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https')
 
 # Makeability Lab Global Variables, including Makeability Lab version
-ML_WEBSITE_VERSION = "2.31.0" # Keep this updated with each release and also change the short description below
-ML_WEBSITE_VERSION_DESCRIPTION = "A project's roster in the public API now reports each person's title over the span of their role (#1435), so someone on a project since 2012 reads as what they are today rather than the title they held back then. Finished stints still read as what that person was at the time."
+ML_WEBSITE_VERSION = "2.31.1" # Keep this updated with each release and also change the short description below
+ML_WEBSITE_VERSION_DESCRIPTION = "The Django log path is now derived from the project root instead of a hardcoded container path, and an unwritable log directory degrades gracefully instead of killing startup (#1283). Because the servers have no console, /version.json now reports whether file logging is actually live."
 DATE_MAKEABILITYLAB_FORMED = datetime.date(2012, 1, 1)  # Date Makeability Lab was formed
 MAX_BANNERS = 7 # Maximum number of banners on a page
 
@@ -110,21 +110,39 @@
 # an absolute container-specific path. Django evaluates LOGGING at django.setup(),
 # so on any host lacking that exact directory (e.g. GitHub Actions CI) startup died
 # with FileNotFoundError before a single request/test ran. Derive the path from
-# BASE_DIR instead (still under media/ so it stays reachable via the intentional
-# /logs/ URL — see docs/DEPLOYMENT.md), allow an ML_LOG_DIR env override, and if
+# BASE_DIR instead (still under media/, the bind-mounted tree, so the file stays
+# readable over SSH — see docs/DEPLOYMENT.md), allow an ML_LOG_DIR env override, and if
 # the directory can't be created or written, fall back to a NullHandler so a bad
 # log path never crashes startup.
-def _log_dir_is_writable(log_dir):
-    """Return True if ``log_dir`` exists (or can be created) and looks writable.
-
-    Used to decide whether the file log handler is active or degrades to a
-    NullHandler so a bad log path never crashes ``django.setup()`` (issue #1283).
-
-    Note: this checks the *directory*, not the eventual log file. A dir that is
-    writable but already holds a root-owned, read-only ``debug.log`` would still
-    let RotatingFileHandler raise on open — an edge case we accept, since it is
-    strictly better than the previous unconditional crash and matches the real
-    deploy model (media/ is owned by the app's own user).
+#
+# Degrading is silent by default, and that is dangerous here: the 'django' logger
+# has only the 'file' handler, and the 'website' logger's console handler is gated
+# by require_debug_true (False in prod), so an unwritable log dir means the app runs
+# completely blind. We have no console access on the -test or prod servers, so the
+# degraded state is surfaced two web-reachable ways instead: the 'log_to_file' field
+# on /version.json (website/views/version.py) and a warning callout on the admin
+# dashboard (website/templates/admin/index.html).
+def _ensure_log_dir_writable(log_dir):
+    """Create ``log_dir`` if needed and return True if it looks writable.
+
+    Named for the side effect: this *creates* the directory (``os.makedirs``)
+    rather than merely inspecting it. Used to decide whether the file log handler
+    is active or degrades to a NullHandler, so a bad log path never crashes
+    ``django.setup()`` (issue #1283).
+
+    Two known limits, both accepted as strictly better than the previous
+    unconditional crash:
+
+    1. This checks the *directory*, not the eventual log file. A dir that is
+       writable but already holds a root-owned, read-only ``debug.log`` would
+       still let RotatingFileHandler raise on open. That doesn't match the real
+       deploy model, where media/ is owned by the app's own user.
+    2. ``os.access(dir, os.W_OK)`` returns True for root regardless of the
+       directory mode, so a mode-555 dir wouldn't be caught when running as root.
+       The deployed container runs as ``apache`` (UID 48, see Dockerfile), so the
+       guard is meaningful where it matters; only the root devcontainer bypasses
+       it. The common failures — missing dir, uncreatable dir, read-only
+       filesystem — are caught either way.
     """
     try:
         os.makedirs(log_dir, exist_ok=True)
@@ -133,9 +151,49 @@ def _log_dir_is_writable(log_dir):
         return False
 
 
+def _file_log_handler(log_file, level, enabled):
+    """Return the ``LOGGING['handlers']['file']`` config dict.
+
+    When ``enabled`` is False (the log dir isn't writable) this returns a
+    NullHandler instead, which keeps every logger's ``'file'`` handler reference
+    valid while never touching disk — so startup degrades instead of dying.
+
+    Split out of the ``LOGGING`` literal so both branches are directly testable;
+    ``LOGGING`` is evaluated once at import, so a test can't re-derive it.
+    See ``website/tests/test_logging_config.py``.
+    """
+    if not enabled:
+        return {'class': 'logging.NullHandler'}
+    return {
+        'level': level,
+        'class': 'logging.handlers.RotatingFileHandler',
+        'filename': log_file,
+        'maxBytes': 1024*1024*5,  # 5 MB
+        'backupCount': 6,
+        'formatter': 'verbose',  # can switch between verbose and simple
+    }
+
+
+# NOTE: this default must stay in sync with MEDIA_ROOT (defined further down as
+# os.path.join(BASE_DIR, 'media')) — the web-served /logs/debug.log URL only works
+# because the log lives inside the media root. MEDIA_ROOT isn't defined yet here
+# (LOGGING has to be built before it), hence the duplicated expression;
+# test_default_log_file_is_under_media_root pins the two together.
 LOG_DIR = os.environ.get('ML_LOG_DIR', os.path.join(BASE_DIR, 'media'))
 LOG_FILE = os.path.join(LOG_DIR, 'debug.log')
-_LOG_TO_FILE = _log_dir_is_writable(LOG_DIR)
+
+# Uppercase on purpose: Django only exposes uppercase module attributes through
+# django.conf.settings, and both the /version.json view and the admin dashboard
+# read this to surface a degraded-logging warning.
+LOG_TO_FILE = _ensure_log_dir_writable(LOG_DIR)
+
+if not LOG_TO_FILE:
+    # Secondary signal only. There is no console access on the -test or prod
+    # servers, so this print is really for local dev and the emailed buildlog;
+    # the channels that actually work remotely are /version.json (log_to_file
+    # field) and the warning callout on the admin dashboard.
+    print(f"WARNING: log dir {LOG_DIR!r} is not writable — file logging disabled "
+          f"(NullHandler). Check /version.json 'log_to_file'.")
 
 LOGGING = {
     'version': 1,
@@ -157,24 +215,16 @@ def _log_dir_is_writable(log_dir):
         },
     },
     'handlers': {
-        # The file handler writes LOG_FILE (media/debug.log by default), which
-        # lands in the bind-mounted web root and is intentionally exposed via the
-        # /logs/ URL per docs/DEPLOYMENT.md (Jason Howe's design — convenient
-        # remote debugging in exchange for some info disclosure). To shrink that
-        # exposure in production, we log at INFO when DEBUG is off, but keep
-        # DEBUG-level file logging in local dev where DEBUG is on and the file
-        # isn't publicly reachable. If the log dir isn't writable (_LOG_TO_FILE
-        # is False), degrade to a NullHandler so startup never dies (issue #1283).
-        'file': {
-            'level': 'DEBUG' if DEBUG else 'INFO',
-            'class': 'logging.handlers.RotatingFileHandler',
-            'filename': LOG_FILE,
-            'maxBytes': 1024*1024*5,  # 5 MB
-            'backupCount': 6,
-            'formatter': 'verbose',  # can switch between verbose and simple
-        } if _LOG_TO_FILE else {
-            'class': 'logging.NullHandler',
-        },
+        # The file handler writes LOG_FILE (media/debug.log by default), which lands
+        # in the bind-mounted web root — that's what makes it readable over SSH at
+        # /cse/web/research/makelab/www[-test]/debug.log. (docs/DEPLOYMENT.md also
+        # describes a /logs/ URL per Jason Howe's design, but that URL 404s on both
+        # prod and test as of 2026-07-28.) Since the file still sits in a web-served
+        # tree, stay conservative: log at INFO when DEBUG is off, but keep DEBUG-level
+        # file logging in local dev where DEBUG is on and nothing is public. If the
+        # log dir isn't writable (LOG_TO_FILE is False), degrade to a NullHandler so
+        # startup never dies (issue #1283).
+        'file': _file_log_handler(LOG_FILE, 'DEBUG' if DEBUG else 'INFO', LOG_TO_FILE),
         'console': {
             'level': 'DEBUG',
             'filters': ['require_debug_true'],
@@ -401,6 +451,11 @@ def _log_dir_is_writable(log_dir):
 # The MEDIA_URL is required by Django see and is a URL that handles the media served 
 # from MEDIA_ROOT, used for managing stored files. 
 # See: https://docs.djangoproject.com/en/4.2/ref/settings/#media-url
+#
+# NOTE: LOG_DIR (defined up with LOGGING, which has to be built before this) hard-codes
+# the same expression, because the web-served /logs/debug.log URL only works while the
+# log file lives inside the media root. If you move MEDIA_ROOT, move LOG_DIR with it —
+# test_default_log_file_is_under_media_root fails loudly if the two ever diverge.
 MEDIA_ROOT = os.path.join(BASE_DIR, 'media')
 MEDIA_URL = '/media/'
 

website/context_processors.py

@@ -21,14 +21,21 @@ def admin_version_info(request):
     """
     Make version and debug info available to all templates.
     
-    This is used by the admin interface to display the current 
+    This is used by the admin interface to display the current
     website version in the header and show a debug indicator.
-    
+
+    LOG_TO_FILE / LOG_FILE (#1283) let the admin dashboard warn when the LOGGING
+    file handler degraded to a NullHandler — i.e. this server is writing no logs
+    at all. We have no console access on -test/prod, so the dashboard callout and
+    the /version.json 'log_to_file' field are the only ways to notice.
+
     Returns:
-        dict: Context variables for ML_WEBSITE_VERSION and DEBUG.
+        dict: Context variables for ML_WEBSITE_VERSION, DEBUG, and logging health.
     """
     return {
         'ML_WEBSITE_VERSION': settings.ML_WEBSITE_VERSION,
         'ML_WEBSITE_VERSION_DESCRIPTION': settings.ML_WEBSITE_VERSION_DESCRIPTION,
         'DEBUG': settings.DEBUG,
+        'LOG_TO_FILE': settings.LOG_TO_FILE,
+        'LOG_FILE': settings.LOG_FILE,
     }

website/templates/admin/index.html

@@ -9,6 +9,25 @@
   {% endif %}
 </div>
 
+{% comment %}
+  Degraded-logging warning (#1283). LOGGING's file handler falls back to a
+  NullHandler when the log directory isn't writable, which would otherwise be
+  completely silent: the 'django' logger has only that handler, and there is no
+  console access on the -test or prod servers. Superuser-gated because only the
+  maintainer can act on it. The scriptable equivalent is /version.json's
+  'log_to_file' field.
+{% endcomment %}
+{% if user.is_superuser and not LOG_TO_FILE %}
+  <div class="ml-log-warning">
+    <span class="ml-log-warning-icon" aria-hidden="true">⚠️</span>
+    <strong>Warning: file logging is disabled.</strong>
+    <span class="ml-log-warning-desc">Django could not write to the log directory,
+      so log records are being discarded and <code>debug.log</code> will not update.
+      Expected path: <code>{{ LOG_FILE }}</code>. Check that directory's permissions,
+      or the <code>ML_LOG_DIR</code> environment variable if it is set.</span>
+  </div>
+{% endif %}
+
 {% if user.is_superuser %}
   <div class="ml-data-health">
     <span class="ml-data-health-icon" aria-hidden="true">🩺</span>
@@ -122,6 +141,37 @@
     color: #666;
   }
 
+  /* Degraded-logging warning (#1283) — only rendered when logging is dead.
+     Red-tinted rather than amber so it doesn't read as another .ml-help-text tip. */
+  .ml-log-warning {
+    background-color: #fdecea;
+    border: 1px solid #f5c6c0;
+    border-left: 4px solid #b3261e;
+    padding: 10px 15px;
+    margin-bottom: 20px;
+    border-radius: 4px;
+    font-size: 14px;
+  }
+
+  .ml-log-warning strong {
+    color: #a3261d; /* 6.4:1 on #fdecea — WCAG AA */
+  }
+
+  .ml-log-warning-icon {
+    margin-right: 6px;
+  }
+
+  .ml-log-warning-desc {
+    color: #5f2c28; /* 9.8:1 on #fdecea — WCAG AA */
+  }
+
+  /* Inherit the callout's text color rather than the admin's muted <code> grey,
+     which doesn't clear AA against these backgrounds (especially in dark mode). */
+  .ml-log-warning code {
+    color: inherit;
+    background: transparent;
+  }
+
   @media (prefers-color-scheme: dark) {
     .ml-data-health {
       background-color: #25302a;
@@ -130,6 +180,17 @@
     .ml-data-health-desc {
       color: #aaa;
     }
+    .ml-log-warning {
+      background-color: #3a201d;
+      border-color: #5a3430;
+      border-left-color: #e06c5f;
+    }
+    .ml-log-warning strong {
+      color: #f2a099; /* 7.3:1 on #3a201d — WCAG AA */
+    }
+    .ml-log-warning-desc {
+      color: #d9b8b4; /* 8.2:1 on #3a201d — WCAG AA */
+    }
   }
 
   /* Help text styling within category tables */