Skip to content
This repository was archived by the owner on Feb 23, 2026. It is now read-only.

Commit c6b6e5a

Browse files
wesmclaude
andauthored
Miscellaneous bug fixes (#10)
* Fix code review findings for project name handling and type safety - Expand bad project name detection to include _tmp and _var prefixes - Add marker directories (code, projects, etc.) to fallback skip list - Harden escape_html to handle non-string values gracefully - Add type validation and exception handling in extract_project_from_cwd - Add comprehensive unit tests for all fixes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix review findings: accept PathLike, allow project names like src/code - Accept Path and os.PathLike in extract_project_from_cwd using os.fspath - Only skip true system directories (users, home, var, tmp, private) in fallback; names like code, src, dev are now valid project names - Fix test_non_string_with_html_chars to use custom object with HTML in __str__ - Add test for Path objects in extract_project_from_cwd Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent a7c3445 commit c6b6e5a

5 files changed

Lines changed: 276 additions & 7 deletions

File tree

‎agent_session_viewer/main.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,14 @@ async def export_session(session_id: str):
301301

302302

303303
def escape_html(text: str) -> str:
304-
"""Escape HTML special characters."""
304+
"""Escape HTML special characters.
305+
306+
Handles non-string values gracefully by coercing to string.
307+
"""
308+
if text is None:
309+
return ""
310+
if not isinstance(text, str):
311+
text = str(text)
305312
if not text:
306313
return ""
307314
return (

‎agent_session_viewer/parser.py‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
"""Parse Claude Code JSONL session files."""
22

33
import json
4+
import os
45
from pathlib import Path
56
from datetime import datetime
6-
from typing import Optional, Generator
7+
from typing import Optional, Generator, Union
78
from dataclasses import dataclass
89

910

@@ -246,18 +247,27 @@ def normalize_project_name(name: str) -> str:
246247
return name.replace("-", "_") if name else ""
247248

248249

249-
def extract_project_from_cwd(cwd: str) -> str:
250+
def extract_project_from_cwd(cwd: Union[str, os.PathLike]) -> str:
250251
"""Extract project name from a cwd path.
251252
252253
Uses the last component of the path as the project name.
253254
Works for both Claude Code and Codex sessions.
254255
Guards against unsafe path components like . and ..
255256
Normalizes the name (replaces - with _) for consistency.
257+
Accepts str, Path, or any os.PathLike object.
256258
"""
257259
if not cwd:
258260
return ""
259-
path = Path(cwd)
260-
name = path.name or ""
261+
try:
262+
# Normalize PathLike objects to string, then to Path
263+
if isinstance(cwd, os.PathLike):
264+
cwd = os.fspath(cwd)
265+
if not isinstance(cwd, str):
266+
return ""
267+
path = Path(cwd)
268+
name = path.name or ""
269+
except (ValueError, TypeError):
270+
return ""
261271

262272
# Guard against unsafe path components
263273
if name in (".", "..", "") or "/" in name or "\\" in name:

‎agent_session_viewer/sync.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,10 @@ def get_project_name(dir_path: Path) -> str:
7272

7373
# No marker found - take the last non-empty part as the project name
7474
# This handles cases like -Users-wesm -> wesm
75+
# Only skip true system directories; names like "code", "src", "dev" are valid project names
76+
system_dirs = {"users", "home", "var", "tmp", "private"}
7577
for part in reversed(parts):
76-
if part and part.lower() not in ("users", "home", "var", "tmp", "private"):
78+
if part and part.lower() not in system_dirs:
7779
return part.replace("-", "_")
7880

7981
# Ultimate fallback
@@ -209,11 +211,16 @@ def sync_session_file(
209211
stored_project = stored_session.get("project", "") if stored_session else ""
210212

211213
# Detect bad project names that look like encoded paths
214+
# Covers: _Users*, _home*, _private*, _tmp*, _var*
215+
# and paths containing system directory segments
212216
needs_reparse = (
213217
stored_project.startswith("_Users") or
214218
stored_project.startswith("_home") or
215219
stored_project.startswith("_private") or
216-
"_var_folders_" in stored_project
220+
stored_project.startswith("_tmp") or
221+
stored_project.startswith("_var") or
222+
"_var_folders_" in stored_project or
223+
"_var_tmp_" in stored_project
217224
)
218225

219226
if not needs_reparse:

‎tests/test_main.py‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,35 @@ def test_normal_text(self):
9898
"""Normal text should pass through."""
9999
assert escape_html("Hello world") == "Hello world"
100100

101+
def test_non_string_integer(self):
102+
"""Integer should be coerced to string."""
103+
assert escape_html(42) == "42"
104+
assert escape_html(0) == "0"
105+
106+
def test_non_string_list(self):
107+
"""List should be coerced to string."""
108+
result = escape_html(["a", "b"])
109+
assert isinstance(result, str)
110+
assert result # Not empty
111+
112+
def test_non_string_dict(self):
113+
"""Dict should be coerced to string."""
114+
result = escape_html({"key": "value"})
115+
assert isinstance(result, str)
116+
assert result # Not empty
117+
118+
def test_non_string_with_html_chars(self):
119+
"""Non-string values that produce HTML chars should be escaped."""
120+
# Use a custom object whose __str__ returns HTML characters
121+
class HtmlObject:
122+
def __str__(self):
123+
return "<tag>content</tag>"
124+
125+
result = escape_html(HtmlObject())
126+
assert "&lt;tag&gt;" in result
127+
assert "&lt;/tag&gt;" in result
128+
assert "<tag>" not in result
129+
101130

102131
class TestGenerateExportHtml:
103132
"""Tests for HTML export generation."""
@@ -143,3 +172,41 @@ def test_url_encoding_in_filename(self):
143172
filename = sanitize_filename("my project#1.html")
144173
assert '"' not in filename
145174
assert '\n' not in filename
175+
176+
def test_non_string_role_does_not_crash(self):
177+
"""Non-string role values should not crash export."""
178+
session = {"project": "test", "agent": "claude", "message_count": 1}
179+
messages = [{"role": 123, "content": "test", "timestamp": ""}]
180+
181+
# Should not raise, should produce valid HTML
182+
html = generate_export_html(session, messages)
183+
assert "123" in html
184+
assert isinstance(html, str)
185+
186+
def test_non_string_agent_does_not_crash(self):
187+
"""Non-string agent values should not crash export."""
188+
session = {"project": "test", "agent": 456, "message_count": 1}
189+
messages = []
190+
191+
# Should not raise, should produce valid HTML
192+
html = generate_export_html(session, messages)
193+
assert "456" in html
194+
assert isinstance(html, str)
195+
196+
def test_none_role_does_not_crash(self):
197+
"""None role values should not crash export."""
198+
session = {"project": "test", "agent": "claude", "message_count": 1}
199+
messages = [{"role": None, "content": "test", "timestamp": ""}]
200+
201+
# Should not raise
202+
html = generate_export_html(session, messages)
203+
assert isinstance(html, str)
204+
205+
def test_missing_role_does_not_crash(self):
206+
"""Missing role key should not crash export."""
207+
session = {"project": "test", "agent": "claude", "message_count": 1}
208+
messages = [{"content": "test", "timestamp": ""}]
209+
210+
# Should not raise
211+
html = generate_export_html(session, messages)
212+
assert isinstance(html, str)

‎tests/test_sync.py‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,3 +395,181 @@ def test_extract_project_from_cwd_rejects_dotdot(self):
395395
def test_extract_project_from_cwd_no_hyphens_passthrough(self):
396396
"""Project names without hyphens should pass through."""
397397
assert extract_project_from_cwd("/Users/user/myproject") == "myproject"
398+
399+
def test_extract_project_from_cwd_non_string_types(self):
400+
"""Should return empty string for invalid types like int, list, dict."""
401+
assert extract_project_from_cwd(123) == ""
402+
assert extract_project_from_cwd(["list"]) == ""
403+
assert extract_project_from_cwd({"dict": "value"}) == ""
404+
405+
def test_extract_project_from_cwd_accepts_path_objects(self):
406+
"""Should accept Path and os.PathLike objects."""
407+
assert extract_project_from_cwd(Path("/Users/user/my-app")) == "my_app"
408+
assert extract_project_from_cwd(Path("/Users/user/Projects/project")) == "project"
409+
410+
def test_extract_project_from_cwd_embedded_null(self):
411+
"""Should handle strings with embedded null safely."""
412+
# Path() may raise or behave unexpectedly with embedded NUL
413+
result = extract_project_from_cwd("/Users/user\x00/project")
414+
# Just verify it doesn't crash - result may vary by platform
415+
assert isinstance(result, str)
416+
417+
418+
class TestGetProjectNameFallback:
419+
"""Tests for get_project_name fallback behavior."""
420+
421+
def test_accepts_marker_directories_as_project_names(self):
422+
"""Common directory names (code, src, etc.) are valid project names."""
423+
# These are legitimate project names when they're the working directory
424+
assert get_project_name(Path("-Users-wesm-code")) == "code"
425+
assert get_project_name(Path("-Users-wesm-projects")) == "projects"
426+
assert get_project_name(Path("-Users-wesm-repos")) == "repos"
427+
assert get_project_name(Path("-Users-wesm-src")) == "src"
428+
assert get_project_name(Path("-Users-wesm-work")) == "work"
429+
assert get_project_name(Path("-Users-wesm-dev")) == "dev"
430+
431+
def test_skips_system_directories_in_fallback(self):
432+
"""System directories (users, home, var, tmp, private) should be skipped."""
433+
assert get_project_name(Path("-Users-wesm")) == "wesm"
434+
assert get_project_name(Path("-home-ubuntu")) == "ubuntu"
435+
# "home" is a system directory and should be skipped even if it appears last
436+
# (unlike "code", "src" which are valid project names)
437+
assert get_project_name(Path("-Users-wesm-home")) == "wesm"
438+
assert get_project_name(Path("-Users-wesm-tmp")) == "wesm"
439+
assert get_project_name(Path("-Users-wesm-var")) == "wesm"
440+
441+
def test_marker_extraction_takes_precedence(self):
442+
"""When a marker has content after it, that content is the project name."""
443+
# code/my-app -> my_app (marker extraction works)
444+
assert get_project_name(Path("-Users-wesm-code-my-app")) == "my_app"
445+
assert get_project_name(Path("-Users-wesm-src-project")) == "project"
446+
447+
448+
class TestReparseBehavior:
449+
"""Tests for reparse detection of bad project names."""
450+
451+
def test_sync_reparses_users_prefix(self, tmp_path):
452+
"""Sessions with _Users* project names should be reparsed."""
453+
from agent_session_viewer import db
454+
from agent_session_viewer import sync as sync_module
455+
456+
# Create a session file with cwd
457+
project_dir = tmp_path / "-Users-alice-code-my-app"
458+
project_dir.mkdir()
459+
session_file = project_dir / "test-session.jsonl"
460+
session_file.write_text(
461+
'{"type": "user", "cwd": "/Users/alice/code/my-app", "message": {"content": "hello"}, "timestamp": "2025-01-01T00:00:00Z"}\n'
462+
)
463+
464+
# Mock db functions and CLAUDE_PROJECTS_DIR
465+
with patch.object(sync_module, "CLAUDE_PROJECTS_DIR", tmp_path), \
466+
patch.object(sync_module, "SESSIONS_DIR", tmp_path / "sessions"), \
467+
patch.object(db, "get_session_file_info") as mock_file_info, \
468+
patch.object(db, "get_session") as mock_get_session, \
469+
patch.object(db, "upsert_session") as mock_upsert, \
470+
patch.object(db, "delete_session_messages"), \
471+
patch.object(db, "insert_messages_batch"):
472+
473+
# Simulate: file hash matches but stored project is bad
474+
file_hash = sync_module.compute_file_hash(session_file)
475+
mock_file_info.return_value = (session_file.stat().st_size, file_hash)
476+
mock_get_session.return_value = {"project": "_Users_alice_code_my_app"}
477+
478+
result = sync_module.sync_session_file(
479+
session_file, "_Users_alice_code_my_app", "local"
480+
)
481+
482+
# Should have reparsed (not skipped) and updated with correct project name
483+
assert result is not None
484+
assert result.get("skipped") is False
485+
assert result.get("project") == "my_app"
486+
487+
# Verify upsert was called with the correct project name
488+
mock_upsert.assert_called_once()
489+
call_kwargs = mock_upsert.call_args.kwargs
490+
assert call_kwargs["project"] == "my_app"
491+
492+
def test_sync_reparses_tmp_prefix(self, tmp_path):
493+
"""Sessions with _tmp* project names should be reparsed."""
494+
from agent_session_viewer import db
495+
from agent_session_viewer import sync as sync_module
496+
497+
project_dir = tmp_path / "-tmp-my-app"
498+
project_dir.mkdir()
499+
session_file = project_dir / "test-session.jsonl"
500+
session_file.write_text(
501+
'{"type": "user", "cwd": "/tmp/my-app", "message": {"content": "hello"}, "timestamp": "2025-01-01T00:00:00Z"}\n'
502+
)
503+
504+
with patch.object(sync_module, "CLAUDE_PROJECTS_DIR", tmp_path), \
505+
patch.object(sync_module, "SESSIONS_DIR", tmp_path / "sessions"), \
506+
patch.object(db, "get_session_file_info") as mock_file_info, \
507+
patch.object(db, "get_session") as mock_get_session, \
508+
patch.object(db, "upsert_session") as mock_upsert, \
509+
patch.object(db, "delete_session_messages"), \
510+
patch.object(db, "insert_messages_batch"):
511+
512+
file_hash = sync_module.compute_file_hash(session_file)
513+
mock_file_info.return_value = (session_file.stat().st_size, file_hash)
514+
mock_get_session.return_value = {"project": "_tmp_my_app"}
515+
516+
result = sync_module.sync_session_file(
517+
session_file, "_tmp_my_app", "local"
518+
)
519+
520+
assert result is not None
521+
assert result.get("skipped") is False
522+
523+
def test_sync_reparses_var_prefix(self, tmp_path):
524+
"""Sessions with _var* project names should be reparsed."""
525+
from agent_session_viewer import db
526+
from agent_session_viewer import sync as sync_module
527+
528+
project_dir = tmp_path / "-var-tmp-my-app"
529+
project_dir.mkdir()
530+
session_file = project_dir / "test-session.jsonl"
531+
session_file.write_text(
532+
'{"type": "user", "cwd": "/var/tmp/my-app", "message": {"content": "hello"}, "timestamp": "2025-01-01T00:00:00Z"}\n'
533+
)
534+
535+
with patch.object(sync_module, "CLAUDE_PROJECTS_DIR", tmp_path), \
536+
patch.object(sync_module, "SESSIONS_DIR", tmp_path / "sessions"), \
537+
patch.object(db, "get_session_file_info") as mock_file_info, \
538+
patch.object(db, "get_session") as mock_get_session, \
539+
patch.object(db, "upsert_session") as mock_upsert, \
540+
patch.object(db, "delete_session_messages"), \
541+
patch.object(db, "insert_messages_batch"):
542+
543+
file_hash = sync_module.compute_file_hash(session_file)
544+
mock_file_info.return_value = (session_file.stat().st_size, file_hash)
545+
mock_get_session.return_value = {"project": "_var_tmp_my_app"}
546+
547+
result = sync_module.sync_session_file(
548+
session_file, "_var_tmp_my_app", "local"
549+
)
550+
551+
assert result is not None
552+
assert result.get("skipped") is False
553+
554+
def test_sync_skips_good_project_names(self, tmp_path):
555+
"""Sessions with good project names should be skipped if hash matches."""
556+
from agent_session_viewer import db
557+
from agent_session_viewer import sync as sync_module
558+
559+
project_dir = tmp_path / "my_app"
560+
project_dir.mkdir()
561+
session_file = project_dir / "test-session.jsonl"
562+
session_file.write_text('{"type": "user", "message": {"content": "hello"}}\n')
563+
564+
with patch.object(sync_module, "CLAUDE_PROJECTS_DIR", tmp_path), \
565+
patch.object(db, "get_session_file_info") as mock_file_info, \
566+
patch.object(db, "get_session") as mock_get_session:
567+
568+
file_hash = sync_module.compute_file_hash(session_file)
569+
mock_file_info.return_value = (session_file.stat().st_size, file_hash)
570+
mock_get_session.return_value = {"project": "my_app"}
571+
572+
result = sync_module.sync_session_file(session_file, "my_app", "local")
573+
574+
assert result is not None
575+
assert result.get("skipped") is True

0 commit comments

Comments
 (0)