mikeboone Claude Opus 5 commited on
Commit
767a6ca
·
1 Parent(s): 76513d3

fix(logging): harden PromptLogger against unwritable log dir

Browse files

Backport of the prompt_logger.py half of 4b87c36 (cherry-picked from
deploy/mcp) onto the branch that feeds hf-test.

4b87c36 fixed a prod MCP outage: PromptLogger.__init__ did an unguarded
mkdir on <script_dir>/logs/prompts. HF Spaces run the container as a
non-root user inside a root-owned WORKDIR and logs/ is gitignored, so it
raised PermissionError: '/app/logs' and failed every MCP build.

prompt_logger.py is SHARED code, so the app Spaces carried the same latent
bug — it just never fired there because only the MCP path constructed a
PromptLogger. This closes that gap: the log dir is resolved (module dir ->
temp -> memory-only) and every read/write degrades instead of raising.

The mcp_server.py half of 4b87c36 is deliberately NOT taken. It guards the
run-logger block added by 47edf58, which does not exist on this branch;
pulling it in would import an unrelated feature through a conflict
resolution. That guard stays on deploy/mcp where the code it protects lives.

Regression coverage: tests/test_prompt_logger_readonly.py (3 tests).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

prompt_logger.py CHANGED
@@ -30,6 +30,8 @@ Usage:
30
 
31
  import os
32
  import json
 
 
33
  import time
34
  from datetime import datetime
35
  from pathlib import Path
@@ -44,25 +46,48 @@ class PromptLogger:
44
  self.entries: List[Dict] = []
45
  self.call_count = 0
46
 
47
- # Create log directory
48
- script_dir = Path(__file__).parent
49
- self.log_dir = script_dir / "logs" / "prompts"
50
- self.log_dir.mkdir(parents=True, exist_ok=True)
51
- self.log_file = self.log_dir / f"{self.session_id}.md"
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
52
 
53
  # Write header
54
  self._write_header()
55
 
56
  def _write_header(self):
57
- """Write markdown header to log file."""
 
 
58
  header = f"""# Prompt Log — Session {self.session_id}
59
  **Started:** {datetime.now().strftime("%Y-%m-%d %H:%M:%S")}
60
 
61
  ---
62
 
63
  """
64
- with open(self.log_file, "w") as f:
65
- f.write(header)
 
 
 
 
66
 
67
  def log_prompt(
68
  self,
@@ -126,7 +151,9 @@ class PromptLogger:
126
  )
127
 
128
  def _append_entry(self, entry: Dict):
129
- """Append a formatted entry to the markdown log file."""
 
 
130
  n = entry["entry_num"]
131
  stage = entry["stage"]
132
  model = entry["model"]
@@ -185,8 +212,12 @@ class PromptLogger:
185
  lines.append("---")
186
  lines.append("")
187
 
188
- with open(self.log_file, "a") as f:
189
- f.write("\n".join(lines))
 
 
 
 
190
 
191
  def get_summary(self) -> str:
192
  """Return a text summary of all prompts logged this session."""
@@ -229,12 +260,14 @@ class PromptLogger:
229
  lines.append(
230
  f"**Totals:** {total_tokens_in} tokens in, {total_tokens_out} tokens out, {total_duration}ms"
231
  )
232
- lines.append(f"**Log file:** `{self.log_file}`")
233
 
234
  return "\n".join(lines)
235
 
236
  def get_full_log(self) -> str:
237
  """Return the full markdown log file contents for display in Gradio."""
 
 
238
  try:
239
  return self.log_file.read_text()
240
  except Exception:
 
30
 
31
  import os
32
  import json
33
+ import sys
34
+ import tempfile
35
  import time
36
  from datetime import datetime
37
  from pathlib import Path
 
46
  self.entries: List[Dict] = []
47
  self.call_count = 0
48
 
49
+ # Locate a writable log directory. This must NEVER raise: on HF Spaces
50
+ # the container runs as a non-root user inside a root-owned WORKDIR, so
51
+ # creating ./logs raises PermissionError and would otherwise kill the
52
+ # whole build. Fall back to the system temp dir, then to memory-only
53
+ # (log_file = None) prompt logging is diagnostics, never load-bearing.
54
+ self.log_dir = None
55
+ self.log_file = None
56
+ candidates = [
57
+ Path(__file__).parent / "logs" / "prompts",
58
+ Path(tempfile.gettempdir()) / "demoprep" / "logs" / "prompts",
59
+ ]
60
+ for candidate in candidates:
61
+ try:
62
+ candidate.mkdir(parents=True, exist_ok=True)
63
+ self.log_dir = candidate
64
+ self.log_file = candidate / f"{self.session_id}.md"
65
+ break
66
+ except Exception as e:
67
+ print(f"[PromptLogger] Log dir {candidate} unusable: {e}", file=sys.stderr)
68
+ if self.log_file is None:
69
+ print("[PromptLogger] No writable log dir — prompt logging is memory-only.",
70
+ file=sys.stderr)
71
 
72
  # Write header
73
  self._write_header()
74
 
75
  def _write_header(self):
76
+ """Write markdown header to log file. Never raises."""
77
+ if self.log_file is None:
78
+ return
79
  header = f"""# Prompt Log — Session {self.session_id}
80
  **Started:** {datetime.now().strftime("%Y-%m-%d %H:%M:%S")}
81
 
82
  ---
83
 
84
  """
85
+ try:
86
+ with open(self.log_file, "w") as f:
87
+ f.write(header)
88
+ except Exception as e:
89
+ print(f"[PromptLogger] Header write failed, disabling file log: {e}", file=sys.stderr)
90
+ self.log_file = None
91
 
92
  def log_prompt(
93
  self,
 
151
  )
152
 
153
  def _append_entry(self, entry: Dict):
154
+ """Append a formatted entry to the markdown log file. Never raises."""
155
+ if self.log_file is None:
156
+ return
157
  n = entry["entry_num"]
158
  stage = entry["stage"]
159
  model = entry["model"]
 
212
  lines.append("---")
213
  lines.append("")
214
 
215
+ try:
216
+ with open(self.log_file, "a") as f:
217
+ f.write("\n".join(lines))
218
+ except Exception as e:
219
+ print(f"[PromptLogger] Entry write failed, disabling file log: {e}", file=sys.stderr)
220
+ self.log_file = None
221
 
222
  def get_summary(self) -> str:
223
  """Return a text summary of all prompts logged this session."""
 
260
  lines.append(
261
  f"**Totals:** {total_tokens_in} tokens in, {total_tokens_out} tokens out, {total_duration}ms"
262
  )
263
+ lines.append(f"**Log file:** `{self.log_file or 'memory-only (no writable log dir)'}`")
264
 
265
  return "\n".join(lines)
266
 
267
  def get_full_log(self) -> str:
268
  """Return the full markdown log file contents for display in Gradio."""
269
+ if self.log_file is None:
270
+ return "Prompt log is memory-only (no writable log dir) — see summary."
271
  try:
272
  return self.log_file.read_text()
273
  except Exception:
tests/test_prompt_logger_readonly.py ADDED
@@ -0,0 +1,66 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """
2
+ Regression tests for PromptLogger under an unwritable log directory.
3
+
4
+ Outage 2026-08-25: HF Spaces run the container as a non-root user inside a
5
+ root-owned WORKDIR, so `<script_dir>/logs` cannot be created. PromptLogger's
6
+ unguarded mkdir raised PermissionError, which propagated out of
7
+ _create_run_loggers() and failed EVERY MCP build.
8
+
9
+ Prompt logging is diagnostics — it must degrade, never take a build down.
10
+ """
11
+
12
+ from pathlib import Path
13
+
14
+ import pytest
15
+
16
+ from prompt_logger import PromptLogger
17
+
18
+
19
+ def test_unwritable_primary_dir_falls_back(monkeypatch, tmp_path):
20
+ """mkdir denied on the module dir -> falls back to temp, still logs to a file."""
21
+ real_mkdir = Path.mkdir
22
+ blocked = Path(__file__).parent.parent / "logs"
23
+
24
+ def selective_mkdir(self, *args, **kwargs):
25
+ if str(self).startswith(str(blocked)):
26
+ raise PermissionError(f"[Errno 13] Permission denied: '{self}'")
27
+ return real_mkdir(self, *args, **kwargs)
28
+
29
+ monkeypatch.setattr(Path, "mkdir", selective_mkdir)
30
+
31
+ logger = PromptLogger(session_id="fallback_test")
32
+ assert logger.log_file is not None
33
+ assert str(blocked) not in str(logger.log_file)
34
+ # Logging still works end to end
35
+ logger.log_prompt(stage="ddl", model="m",
36
+ messages=[{"role": "user", "content": "hi"}],
37
+ response_text="ok")
38
+ assert len(logger.entries) == 1
39
+
40
+
41
+ def test_no_writable_dir_degrades_to_memory(monkeypatch):
42
+ """mkdir denied everywhere -> memory-only, and nothing raises."""
43
+ def always_denied(self, *args, **kwargs):
44
+ raise PermissionError("denied everywhere")
45
+
46
+ monkeypatch.setattr(Path, "mkdir", always_denied)
47
+
48
+ logger = PromptLogger(session_id="memory_only_test")
49
+ assert logger.log_file is None
50
+ # Every public path must stay non-fatal
51
+ logger.log_prompt(stage="ddl", model="m",
52
+ messages=[{"role": "user", "content": "hi"}],
53
+ response_text="ok")
54
+ assert len(logger.entries) == 1
55
+ assert "memory-only" in logger.get_full_log()
56
+ assert "Log file:" in logger.get_summary()
57
+
58
+
59
+ def test_construction_never_raises_on_oserror(monkeypatch):
60
+ """Any OSError from mkdir (not just PermissionError) must be survivable."""
61
+ def raise_oserror(self, *args, **kwargs):
62
+ raise OSError("read-only file system")
63
+
64
+ monkeypatch.setattr(Path, "mkdir", raise_oserror)
65
+ logger = PromptLogger(session_id="oserror_test") # must not raise
66
+ assert logger.log_file is None