Skip to content

fix(agent): preserve raw Claude session lines#2768

Open
daryllimyt wants to merge 1 commit into
mainfrom
fix/session-nul-chars
Open

fix(agent): preserve raw Claude session lines#2768
daryllimyt wants to merge 1 commit into
mainfrom
fix/session-nul-chars

Conversation

@daryllimyt

@daryllimyt daryllimyt commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Store raw Claude session JSONL bytes for exact resume reconstruction.
  • Escape NUL codepoints in the JSONB projection so Postgres accepts affected session lines.
  • Prefer raw session lines during resume while preserving the existing JSONB fallback path.

Tests

  • uv run pytest tests/unit/test_agent_session_messages.py
  • uv run ruff check tests/unit/test_agent_session_messages.py tracecat/agent/executor/loopback.py tracecat/agent/session/service.py tracecat/db/models.py alembic/versions/0e4c0a2b7f91_add_raw_agent_session_line.py
  • uv run ruff format --check tests/unit/test_agent_session_messages.py tracecat/agent/executor/loopback.py tracecat/agent/session/service.py tracecat/db/models.py alembic/versions/0e4c0a2b7f91_add_raw_agent_session_line.py
  • uv run basedpyright tests/unit/test_agent_session_messages.py tracecat/agent/executor/loopback.py tracecat/agent/session/service.py tracecat/db/models.py alembic/versions/0e4c0a2b7f91_add_raw_agent_session_line.py

@daryllimyt daryllimyt added fix Bug fix agents LLM agents migration Database migration labels May 27, 2026
@blacksmith-sh

This comment has been minimized.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@zeropath-ai

zeropath-ai Bot commented May 27, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 2bfadef.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► tests/unit/test_agent_session_messages.py
    Add test for _session_line_db_content and _session_line_jsonb_safe_content
    Add test for persist_session_line logic including NUL sanitization and sdk_session_id update
► tracecat/agent/executor/loopback.py
    Implement _jsonb_safe_value and _session_line_jsonb_safe_content for NUL sanitization
    Implement _is_jsonb_nul_error to detect PostgreSQL NUL errors
    Add retry logic in _persist_session_line for NUL errors
    Update _persist_session_line to handle setting sdk_session_id on the first line

@daryllimyt daryllimyt requested a review from jordan-umusu May 27, 2026 14:11
@daryllimyt

Copy link
Copy Markdown
Contributor Author

there is a migration in this but should be non-disruptive

@jordan-umusu jordan-umusu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but why go beyond escaping the invalid null checkpoint here?

@daryllimyt daryllimyt force-pushed the fix/session-nul-chars branch from f15be17 to 2bfadef Compare May 28, 2026 19:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2bfadef7ca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

def _jsonb_safe_value(value: object) -> object:
match value:
case str() as text:
return text.replace("\x00", "\\u0000")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve NUL session bytes for resume

When a Claude JSONL line contains \u0000, _session_line_from_json decodes it to an actual NUL, and this fallback changes it to the six-character string \\u0000 before storing it. AgentSessionService.load_session_history later reconstructs the resume JSONL from entry.content with orjson.dumps(content), so affected sessions resume with a literal backslash-u sequence instead of the original NUL value. For tool outputs or message text containing NULs, persistence now succeeds but the stored session is no longer an exact Claude session line, which undermines the raw-resume fix this change is meant to provide.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents LLM agents fix Bug fix migration Database migration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants