Handle missing session ids in pytest steering

[matdev83]

Summary

• guard the pytest full-suite steering handler against empty session identifiers so one request cannot leak state to others
• add a regression test ensuring missing session ids still trigger the steering response for each request

Testing

• python -m pytest -o addopts='' tests/unit/core/services/tool_call_handlers/test_pytest_full_suite_handler.py
• python -m pytest -o addopts=''

---

https://chatgpt.com/codex/tasks/task_e_68ec25089a4c8333a7c641442c526119

Summary by CodeRabbit

• Bug Fixes

  • Improved handling when session information is missing to prevent unintended actions and cross-session state leakage.
  • Ensures repeated commands are handled correctly within a session and improves log clarity with consistent identifiers.

• Tests

  • Added test coverage for scenarios with missing session information to validate safe, consistent behavior.

[coderabbitai]

Walkthrough

Updates PytestFullSuiteHandler to treat missing session_id as a new command, swallowing it without mutating state. With session_id present, it uses per-session state to prevent repeating the last command. Tests added to validate behavior for missing session_id scenarios and ensure no state leakage.

Changes

Cohort / File(s)	Summary
Handler session management logic src/core/services/tool_call_handlers/pytest_full_suite_handler.py	Adjusts can_handle to return True when session_id is absent and to de-duplicate repeated commands per session. Updates handle to swallow commands without session_id, avoid global state mutation, and use per-session state keyed by a stripped session_key. Logging updated to use session_key.
Unit tests for missing session_id behavior tests/unit/core/services/tool_call_handlers/test_pytest_full_suite_handler.py	Adds test ensuring handler can_handle and handle both work with empty session_id, swallowing commands and preventing state leakage across calls without IDs.

Sequence Diagram(s)

sequenceDiagram
  actor Client
  participant Handler as PytestFullSuiteHandler
  participant SessionStore as Per-Session State

  Client->>Handler: can_handle(context)
  alt session_id missing
    Handler-->>Client: True (treat as new command)
  else session_id present
    Handler->>SessionStore: lookup(session_key)
    Handler-->>Client: True/False (dedupe vs new)
  end

  Client->>Handler: handle(context)
  alt session_id missing
    Note over Handler: Swallow command<br/>No state mutation
    Handler-->>Client: replacement_response{ should_swallow: True }
  else session_id present
    Handler->>SessionStore: state = setdefault(session_key)
    alt same as last_command
      Handler-->>Client: replacement_response (deduped)
    else new command
      Handler->>SessionStore: update last_command
      Handler-->>Client: success response
    end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

In burrows of code, I twitch my nose,
Guarding sessions where the logic flows.
No ID? I nibble, then let it pass—
No crumbs of state on meadow grass.
With keys in paw, I track the run,
One hop per command—no double sun. 🌕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly highlights the main change—adding handling for missing session IDs in the pytest steering handler—without extraneous detail, making it clear and relevant to the changes described.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/fix-unexpected-state-changes-in-proxy-n1brhz

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b959ef and 3b52b1e.

📒 Files selected for processing (2)

• src/core/services/tool_call_handlers/pytest_full_suite_handler.py (2 hunks)
• tests/unit/core/services/tool_call_handlers/test_pytest_full_suite_handler.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Avoid using emojis in Python code comments or docstrings
Follow PEP 8 and use type hints for all functions
Import order must be: standard library, third-party, then local imports, separated by blank lines
Naming conventions: snake_case for variables and functions; PascalCase for classes
Error handling: use specific exceptions and include meaningful error messages
Prefer f-strings for string formatting

Files:

• tests/unit/core/services/tool_call_handlers/test_pytest_full_suite_handler.py
• src/core/services/tool_call_handlers/pytest_full_suite_handler.py

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Catch specific exceptions; avoid broad except Exception blocks
If a broad exception must be caught, log with exc_info=True and re-raise a specific custom exception
Use the most specific exception class from src.core.common.exceptions that accurately describes the error
Provide clear, helpful error messages and include relevant details in the details dictionary of custom exceptions

Files:

• src/core/services/tool_call_handlers/pytest_full_suite_handler.py

🧬 Code graph analysis (2)

tests/unit/core/services/tool_call_handlers/test_pytest_full_suite_handler.py (1)

src/core/services/tool_call_handlers/pytest_full_suite_handler.py (3)

• PytestFullSuiteHandler (172-298)
• can_handle (189-209)
• handle (211-265)

src/core/services/tool_call_handlers/pytest_full_suite_handler.py (2)

src/core/domain/session.py (1)

• session_id (578-580)

src/core/interfaces/tool_call_reactor_interface.py (1)

• ToolCallReactionResult (47-57)

🔇 Additional comments (4)

src/core/services/tool_call_handlers/pytest_full_suite_handler.py (3)

201-209: LGTM! Correctly prevents state leakage for missing session IDs.

The logic correctly handles missing or whitespace-only session IDs by treating them as new commands. The implementation ensures that when session_id is empty or whitespace-only, it's normalized to None and causes an immediate return of True, preventing state from being shared across unrelated requests.

---

223-242: LGTM! Safe handling of missing session IDs.

The missing session ID path correctly:

• Logs the steering decision without exposing potentially sensitive session data
• Swallows the command and returns appropriate metadata
• Avoids mutating self._session_state, which is critical for preventing state leakage

---

244-265: LGTM! Consistent use of session_key.

The existing session handling logic now consistently uses session_key instead of the raw context.session_id, which improves clarity and aligns with the new missing-session-id handling.

tests/unit/core/services/tool_call_handlers/test_pytest_full_suite_handler.py (1)

107-120: LGTM! Effective regression test for missing session ID handling.

The test correctly validates that:

• Two consecutive contexts with empty session_id both trigger can_handle to return True
• Both handle calls return should_swallow=True
• No state leakage occurs between the two requests

This ensures the fix prevents the state leakage issue described in the PR objectives.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}