Phase 1 low-risk backend batch (on top of Batch A). Both byte-verified vs PR head, Codex SAFE, 64 own + 749 regression tests pass, coexists with Batch A's #5833. - #5828 (ai-ag2026): frame CORS OPTIONS preflight with Content-Length: 0 (keep-alive hang fix) - #5834 (ai-ag2026): parse session _index.json from bytes not decoded str (all 10 sites) Co-authored-by: t <a@b> Co-authored-by: ai-ag2026 <ai-ag2026@users.noreply.github.com>
This commit is contained in:
9 files changed
+185
-15
No files matched your search
@@ -11,6 +11,10 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **CORS preflight (`OPTIONS`) responses are now framed with `Content-Length: 0`.** Under HTTP/1.1 keep-alive the preflight `200` was sent with no `Content-Length`, so a browser/proxy/curl client read the body until connection close (RFC 7230 §3.3.3) — a keep-alive preflight would hang to the 30s timeout and couldn't reuse the connection. The handler now emits `Content-Length: 0` before `end_headers()`; status stays `200` and CORS headers are unchanged. Thanks @ai-ag2026. (#5828)
|
||||
|
||||
- **The session index is now parsed from bytes instead of a decoded string.** The `_index.json` read sites decoded to `str` before `json.loads`; they now pass `read_bytes()` directly (`json.loads` accepts bytes with RFC-4627 auto-detect), which is behavior-equivalent for the BOM-less UTF-8 the app writes and slightly more tolerant of a BOM. Shaves a redundant decode off the session-list hot path. Thanks @ai-ag2026. (#5834)
|
||||
|
||||
- **Creating a worktree-backed session no longer fails when another `cli` package shadows the agent's `cli.py`.** The worktree helper imported the agent's `cli` module with a bare `from cli import ...`, which an unrelated `cli/` namespace package earlier on `sys.path` (e.g. a dependency like `stringzilla`) could preempt — bricking worktree creation. It now loads the agent's `cli.py` by absolute path via `importlib`, bypassing `sys.path` resolution, and preserves the existing failure contract. Thanks @silent-reader-cn. (#5832)
|
||||
|
||||
- **`settings.json` is now written atomically so a crash mid-write can't wipe your settings.** `save_settings()` and the startup default-workspace rewrite used a truncating in-place write, so a crash, full disk, or power loss between truncate and full write could leave `settings.json` empty — losing theme, workspace, tab order, and the login `password_hash` (silently dropping to no-auth). Both writes now go through a same-directory temp file → `fsync` → `os.replace`, preserving the file's existing permission mode (a hardened `0600` stays `0600`) and writing through symlinks. Thanks @ai-ag2026. (#5829)
|
||||
|
||||
+6
-6
@@ -393,7 +393,7 @@ def _write_session_index(updates=None, *, session_dir: Path | None = None, sessi
|
||||
# Avoid N filesystem exists() checks under LOCK by collecting
|
||||
# on-disk IDs once before entering the critical section.
|
||||
on_disk_ids = _persisted_session_ids_snapshot()
|
||||
existing = json.loads(session_index_file.read_text(encoding='utf-8'))
|
||||
existing = json.loads(session_index_file.read_bytes())
|
||||
if not isinstance(existing, list):
|
||||
raise ValueError("session index must be a list")
|
||||
with LOCK:
|
||||
@@ -457,7 +457,7 @@ def prune_session_from_index(session_id: str) -> None:
|
||||
with _INDEX_WRITE_LOCK:
|
||||
try:
|
||||
with LOCK:
|
||||
existing = json.loads(SESSION_INDEX_FILE.read_text(encoding='utf-8'))
|
||||
existing = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
if not isinstance(existing, list):
|
||||
raise ValueError("session index must be a list")
|
||||
pruned = [e for e in existing if e.get('session_id') != sid]
|
||||
@@ -1023,7 +1023,7 @@ def _index_message_count_map(entries=None) -> dict[str, int]:
|
||||
"""
|
||||
if entries is None:
|
||||
try:
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_text(encoding='utf-8'))
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
except Exception:
|
||||
return {}
|
||||
if not isinstance(entries, list):
|
||||
@@ -2637,7 +2637,7 @@ def _has_compression_continuation(session) -> bool:
|
||||
|
||||
try:
|
||||
if SESSION_INDEX_FILE.exists():
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_text(encoding='utf-8'))
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
if isinstance(entries, list) and any(_row_is_continuation(e) for e in entries):
|
||||
return True
|
||||
except Exception:
|
||||
@@ -4666,7 +4666,7 @@ def all_sessions(diag=None, *, include_lineage_metadata: bool = True):
|
||||
if SESSION_INDEX_FILE.exists():
|
||||
try:
|
||||
_diag_stage(diag, "all_sessions.read_index")
|
||||
index = json.loads(SESSION_INDEX_FILE.read_text(encoding='utf-8'))
|
||||
index = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
_diag_stage(diag, "all_sessions.prune_index")
|
||||
with LOCK:
|
||||
in_memory_ids = set(SESSIONS.keys())
|
||||
@@ -4908,7 +4908,7 @@ def _backfill_project_profiles_if_needed(projects: list) -> bool:
|
||||
session_profile_by_project: dict[str, str] = {}
|
||||
if SESSION_INDEX_FILE.exists():
|
||||
try:
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_text(encoding='utf-8'))
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
untagged_ids = {p['project_id'] for p in untagged if p.get('project_id')}
|
||||
for e in entries:
|
||||
pid = e.get('project_id')
|
||||
|
||||
+4
-4
@@ -7239,7 +7239,7 @@ def _session_index_marks_was_webui(sid: str) -> bool:
|
||||
if not SESSION_INDEX_FILE.exists():
|
||||
return False
|
||||
try:
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_text(encoding="utf-8"))
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
except Exception:
|
||||
return False
|
||||
for entry in entries if isinstance(entries, list) else []:
|
||||
@@ -8812,7 +8812,7 @@ def _pre_compression_continuation_session_id(session) -> str | None:
|
||||
if not SESSION_INDEX_FILE.exists():
|
||||
return None
|
||||
try:
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_text(encoding="utf-8"))
|
||||
entries = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
except Exception:
|
||||
return None
|
||||
if not isinstance(entries, list):
|
||||
@@ -15237,7 +15237,7 @@ def handle_post(handler, parsed) -> bool:
|
||||
# update so one slow/failing session can't abort the whole request.
|
||||
if SESSION_INDEX_FILE.exists():
|
||||
try:
|
||||
index = json.loads(SESSION_INDEX_FILE.read_text(encoding="utf-8"))
|
||||
index = json.loads(SESSION_INDEX_FILE.read_bytes())
|
||||
active_ids = _active_stream_ids()
|
||||
deferred_to_stream = []
|
||||
for entry in index:
|
||||
@@ -19305,7 +19305,7 @@ def _handle_sessions_cleanup(handler, body, zero_only=False):
|
||||
|
||||
with _INDEX_WRITE_LOCK:
|
||||
index_file_data = json.loads(
|
||||
SESSION_INDEX_FILE.read_text(encoding="utf-8")
|
||||
SESSION_INDEX_FILE.read_bytes()
|
||||
)
|
||||
if isinstance(index_file_data, list):
|
||||
live_ids = {
|
||||
|
||||
@@ -439,6 +439,9 @@ class Handler(BaseHTTPRequestHandler):
|
||||
self._req_t0 = time.time()
|
||||
self.send_response(200)
|
||||
apply_cors_preflight_headers(self)
|
||||
# Frame the empty preflight: without Content-Length an HTTP/1.1 keep-alive
|
||||
# 200 is read-until-close, hanging the client until the 30s timeout.
|
||||
self.send_header("Content-Length", "0")
|
||||
self.end_headers()
|
||||
|
||||
def do_DELETE(self) -> None:
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
"""Regression test — the CORS preflight (OPTIONS) 200 must be framed.
|
||||
|
||||
`Handler.do_OPTIONS` answered every preflight with `send_response(200)` +
|
||||
CORS headers + `end_headers()` but no `Content-Length`. The class runs
|
||||
`protocol_version = "HTTP/1.1"` (keep-alive on), and its own docstring states
|
||||
"every response must declare framing". An unframed 200 is "read body until the
|
||||
connection closes" (RFC 7230 §3.3.3), so a keep-alive client issuing a preflight
|
||||
(browser CORS for an allowlisted cross-origin front-end, curl, a proxy, a
|
||||
monitor) blocks reading a body that never arrives until the 30s connection
|
||||
timeout — and can't reuse the connection.
|
||||
|
||||
This pins that the preflight now carries `Content-Length: 0`, sent before
|
||||
`end_headers()` (so it lands among the emitted headers), mirroring the framing
|
||||
every other bodyless response in the server already sets. The CORS-header policy
|
||||
itself (which Origins are echoed) is unchanged and covered separately.
|
||||
"""
|
||||
class _FakeHeaders(dict):
|
||||
def get(self, key, default=None):
|
||||
return dict.get(self, key, default)
|
||||
|
||||
|
||||
class _OptionsHandler:
|
||||
"""Captures the send_response / send_header / end_headers sequence."""
|
||||
|
||||
def __init__(self, headers=None):
|
||||
self.headers = _FakeHeaders(headers or {})
|
||||
self.client_address = ("127.0.0.1", 12345)
|
||||
self.status = None
|
||||
self.order = []
|
||||
|
||||
def send_response(self, status):
|
||||
self.status = status
|
||||
self.order.append(("status", status, None))
|
||||
|
||||
def send_header(self, key, value):
|
||||
self.order.append(("hdr", key.lower(), value))
|
||||
|
||||
def end_headers(self):
|
||||
self.order.append(("end", None, None))
|
||||
|
||||
|
||||
def _run_options(headers=None):
|
||||
import server
|
||||
|
||||
handler = _OptionsHandler(headers)
|
||||
server.Handler.do_OPTIONS(handler)
|
||||
return handler
|
||||
|
||||
|
||||
def test_options_sets_content_length_zero():
|
||||
handler = _run_options()
|
||||
assert handler.status == 200
|
||||
header_map = {k: v for kind, k, v in handler.order if kind == "hdr"}
|
||||
assert header_map.get("content-length") == "0", (
|
||||
"OPTIONS preflight 200 must be framed with Content-Length: 0"
|
||||
)
|
||||
|
||||
|
||||
def test_options_content_length_precedes_end_headers():
|
||||
"""Framing must land before end_headers() to be emitted at all."""
|
||||
handler = _run_options()
|
||||
end_idx = next(i for i, e in enumerate(handler.order) if e[0] == "end")
|
||||
header_names = [k for kind, k, _ in handler.order[:end_idx] if kind == "hdr"]
|
||||
assert "content-length" in header_names
|
||||
|
||||
|
||||
def test_options_framed_even_for_allowlisted_origin(monkeypatch):
|
||||
"""The framing is unconditional — a preflight that DOES emit CORS headers
|
||||
(allowlisted origin) must still be framed, not just the no-origin case."""
|
||||
import api.routes as routes
|
||||
|
||||
monkeypatch.setattr(
|
||||
routes, "_check_same_origin_browser_request", lambda handler: True
|
||||
)
|
||||
handler = _run_options({"Origin": "https://app.example"})
|
||||
header_map = {k: v for kind, k, v in handler.order if kind == "hdr"}
|
||||
assert header_map.get("content-length") == "0"
|
||||
@@ -1304,16 +1304,18 @@ def test_all_sessions_reuses_loaded_index_counts_for_legacy_sidecar_refresh(monk
|
||||
})
|
||||
_write_index_file(index_file, rows)
|
||||
|
||||
original_read_text = Path.read_text
|
||||
# The index is parsed from raw bytes (json.loads decodes UTF-8 in one pass),
|
||||
# so count read_bytes rather than read_text.
|
||||
original_read_bytes = Path.read_bytes
|
||||
index_reads = 0
|
||||
|
||||
def _counting_read_text(self, *args, **kwargs):
|
||||
def _counting_read_bytes(self, *args, **kwargs):
|
||||
nonlocal index_reads
|
||||
if self == index_file:
|
||||
index_reads += 1
|
||||
return original_read_text(self, *args, **kwargs)
|
||||
return original_read_bytes(self, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(Path, "read_text", _counting_read_text)
|
||||
monkeypatch.setattr(Path, "read_bytes", _counting_read_bytes)
|
||||
monkeypatch.setattr(models, "_enrich_sidebar_lineage_metadata", lambda _sessions: None)
|
||||
|
||||
result = models.all_sessions()
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
"""Regression tests — the session index is parsed from bytes, not a decoded str.
|
||||
|
||||
`/api/sessions` and its rebuild path read the session index (a multi-MB JSON
|
||||
list on a busy install) with `json.loads(SESSION_INDEX_FILE.read_text(
|
||||
encoding='utf-8'))`. That decodes the whole file to a Python `str` first, then
|
||||
re-scans it in the JSON parser. Handing the raw `bytes` to `json.loads` lets its
|
||||
C parser decode UTF-8 in one pass — measured ~22% faster on a real 6.2 MB index
|
||||
(46 ms -> 36 ms), on every cache-miss rebuild.
|
||||
|
||||
`json.loads` accepts `bytes` and auto-detects the (UTF-8) encoding, producing an
|
||||
identical object for our BOM-less index. These pin the equivalence (including
|
||||
non-ASCII content) and that no index reader silently reverts to the slower
|
||||
decoded-string path.
|
||||
"""
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import api.models as models
|
||||
import api.routes as routes
|
||||
|
||||
|
||||
def test_bytes_and_text_parse_identically(tmp_path: Path):
|
||||
"""A UTF-8 index with non-ASCII titles parses identically from bytes."""
|
||||
index = [
|
||||
{"session_id": "s1", "title": "Grüße 🌍", "last_message_at": 1.0},
|
||||
{"session_id": "s2", "title": "日本語のセッション", "last_message_at": 2.0},
|
||||
]
|
||||
idx = tmp_path / "_index.json"
|
||||
idx.write_text(json.dumps(index, ensure_ascii=False, indent=2), encoding="utf-8")
|
||||
|
||||
from_text = json.loads(idx.read_text(encoding="utf-8"))
|
||||
from_bytes = json.loads(idx.read_bytes())
|
||||
assert from_bytes == from_text == index
|
||||
|
||||
|
||||
def test_all_sessions_reads_unicode_index_via_bytes(tmp_path, monkeypatch):
|
||||
"""End-to-end: all_sessions reads a non-ASCII index correctly through the
|
||||
bytes path (a str/bytes decode mismatch would corrupt or drop titles)."""
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir()
|
||||
idx = session_dir / "_index.json"
|
||||
entries = [
|
||||
{"session_id": "unic-1", "title": "Café ☕", "last_message_at": 10.0},
|
||||
]
|
||||
idx.write_text(json.dumps(entries, ensure_ascii=False), encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
|
||||
monkeypatch.setattr(models, "SESSION_INDEX_FILE", idx)
|
||||
# Treat the indexed id as persisted so the prune step keeps it.
|
||||
monkeypatch.setattr(models, "_persisted_session_ids_snapshot", lambda: frozenset({"unic-1"}))
|
||||
monkeypatch.setattr(models, "_active_stream_ids", lambda: set())
|
||||
|
||||
result = models.all_sessions(include_lineage_metadata=False)
|
||||
|
||||
titles = {s.get("session_id"): s.get("title") for s in result}
|
||||
assert titles.get("unic-1") == "Café ☕"
|
||||
|
||||
|
||||
def test_no_index_reader_uses_decoded_str(tmp_path):
|
||||
"""Guard against a refactor reintroducing the slower read_text path on the
|
||||
session index."""
|
||||
for module in (models, routes):
|
||||
src = Path(module.__file__).read_text(encoding="utf-8")
|
||||
assert "SESSION_INDEX_FILE.read_text" not in src, (
|
||||
f"{module.__name__} reads the session index via read_text; "
|
||||
"use read_bytes so json.loads parses UTF-8 in one pass"
|
||||
)
|
||||
assert "session_index_file.read_text" not in src
|
||||
@@ -17,9 +17,11 @@ def test_compression_continuation_fallback_reads_only_file_head(monkeypatch, tmp
|
||||
|
||||
opened_for_read = []
|
||||
read_text_calls = []
|
||||
read_bytes_calls = []
|
||||
|
||||
original_path_open = models.Path.open
|
||||
original_path_read_text = models.Path.read_text
|
||||
original_path_read_bytes = models.Path.read_bytes
|
||||
|
||||
class _TrackingHandle:
|
||||
def __init__(self, path, inner):
|
||||
@@ -47,15 +49,24 @@ def test_compression_continuation_fallback_reads_only_file_head(monkeypatch, tmp
|
||||
read_text_calls.append(str(self))
|
||||
return original_path_read_text(self, *args, **kwargs)
|
||||
|
||||
def tracking_read_bytes(self, *args, **kwargs):
|
||||
read_bytes_calls.append(str(self))
|
||||
return original_path_read_bytes(self, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(models.Path, "open", tracking_open)
|
||||
monkeypatch.setattr(models.Path, "read_text", tracking_read_text)
|
||||
monkeypatch.setattr(models.Path, "read_bytes", tracking_read_bytes)
|
||||
|
||||
session = models.Session(session_id=parent_sid)
|
||||
|
||||
assert models._has_compression_continuation(session) is False
|
||||
|
||||
assert str(index_file) in read_text_calls
|
||||
# The index is read whole (now via read_bytes — json.loads decodes UTF-8 in
|
||||
# one pass); the candidate sidecar is NOT slurped, only its head is read
|
||||
# through the bounded open below.
|
||||
assert str(index_file) in read_bytes_calls
|
||||
assert all(path != str(candidate) for path in read_text_calls)
|
||||
assert all(path != str(candidate) for path in read_bytes_calls)
|
||||
|
||||
candidate_reads = [size for path, size in opened_for_read if path == str(candidate)]
|
||||
assert candidate_reads, "fallback should read the candidate sidecar"
|
||||
|
||||
@@ -191,6 +191,11 @@ def _invoke_api_session_keyerror(*, index_json, cli_messages):
|
||||
def read_text(self, encoding="utf-8"):
|
||||
return index_json
|
||||
|
||||
def read_bytes(self):
|
||||
# The index reader now parses raw bytes (json.loads decodes UTF-8 in
|
||||
# one pass); model that so this fake matches the real Path interface.
|
||||
return None if index_json is None else index_json.encode("utf-8")
|
||||
|
||||
parsed = urlparse("/api/session?session_id=gone_001&messages=0&resolve_model=0")
|
||||
with patch("api.routes.get_session", side_effect=KeyError("gone_001")), \
|
||||
patch("api.routes.SESSION_INDEX_FILE", _FakeIndexFile()), \
|
||||
|
||||
Reference in new issue
Block a user