Implementation Plan: Thread session_locator into flush_session_log and Migrate 15 Monkeypatch Sites#3982
Open
Trecek wants to merge 3 commits into
Conversation
…atch Replace the direct claude_code_log_path(cwd, session_id) call in flush_session_log with Protocol-dispatched resolution through SessionLocator.session_log_path(). Add a session_locator kwarg with lazy ClaudeSessionLocator() fallback to avoid import-time coupling between session_log and the backends module. Migration: - 15 monkeypatch.setattr sites in test_session_log_fields.py replaced with explicit injection via _FakeLocator test helper - conftest._flush() updated to accept and forward session_locator - 2 new structural guard tests: - test_session_locator_is_optional_parameter (signature guard) - test_flush_helper_forwards_session_locator (forwarding guard) This unblocks dispatching the cc_log resolution through any backend that supplies a SessionLocator (Codex, future backends) without hard-coupling to the claude module.
…-before-optional convention session_locator (optional, None default) was placed before telemetry (required, no default) breaking the keyword-only parameter ordering convention. All callers use keyword syntax so this is a non-breaking reorder.
…ion_locator param The helper's session_locator parameter was untyped, diverging from the flush_session_log signature it wraps.
Trecek
force-pushed
the
t4-p3-a8-wp1-thread-session-locator-into-flush-session-log-a/3928
branch
from
76494bd to
a21650b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace the direct
claude_code_log_path(cwd, session_id)call inflush_session_log(session_log.py:190) with Protocol-dispatchedSessionLocator.session_log_path(). Add asession_locator: SessionLocator | None = Nonekwarg with lazyClaudeSessionLocator()fallback. Migrate all 15monkeypatch.setattrsites intest_session_log_fields.pyto explicit injection via a_FakeLocatortest helper, update_flush()inconftest.pyto forward the new parameter, and update structural guard tests intest_flush_completeness_guard.pyandtest_session_log_flush.py.Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260610-002248-121925/.autoskillit/temp/make-plan/thread_session_locator_flush_session_log_plan_2026-06-10_002700.mdCloses #3928
🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
* Step used a non-Anthropic provider; caching behavior may differ.
Token Efficiency
Model Usage Breakdown