fix(hook): complete the SessionEnd run after the CLI shutdown - #63
Draft
milanagm wants to merge 2 commits into
Draft
fix(hook): complete the SessionEnd run after the CLI shutdown#63milanagm wants to merge 2 commits into
milanagm wants to merge 2 commits into
Conversation
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
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.
This PR fixes #13.
Linear: LFE-10758.
Problem
hooks/langfuse_hook.pyimportedlangfuseandopentelemetryat module load. The module started the import before it knew the plugin configuration, and before it knew the number of new bytes in thetranscript. Thus every hook run did the slow import.
The issue has two causes, and thus this PR makes two changes, one commit for each:
fix(hook): import the SDK only when a run has work to emitfeat(hook): finish the SessionEnd upload in a child processThe second commit stays separate on purpose. A reviewer can read the fork on its own, and a
revert of that one commit removes it.
Fix
The fast checks run first. The SDK import starts only when the run has work to do.
from __future__ import annotationsmakes all annotations strings. Thus an annotation thatnames
Langfusedoes not start the import at definition time.langfuseandopentelemetryimports moved into_ensure_langfuse_imported(), andcreate_langfuse_client()calls that function. The optionallangfuse.mediaimport movedthere too, because that import also loads the
langfusepackage.PATH and
uvto the log. The function returnsNone, and the hook exits with code 0._has_pending_work(session_id, transcript_path)runs first inmain(). It readsthe entry in the state file and the size of the transcript file. It takes no lock and it starts
no import. The run has work to do when the transcript has bytes after the saved offset. The run
also has work to do when the entry contains deferred agent turns, stashed task notifications,
or an open turn.
uses the normal path for an unknown session.
Second change: a child process completes the SessionEnd run
_detach_from_cli()makes one fork, and only for a SessionEnd run with work to do. The parentprocess exits immediately, and thus the CLI continues immediately. The child process leaves the
process group with
setsid, redirects the three standard descriptors to/dev/null, and thencompletes the upload. The redirect is necessary, because the CLI closes those pipes. The log file
is the only output channel of the child process.
Important details:
create_langfuse_client. Thus the process hasno SDK thread yet. A fork is not safe when the process has an exporter thread.
in the foreground shows its errors.
os.fork. Whenos.forkis absent, or when the fork fails, the run stays in theforeground and writes the reason to the log. The behavior is then the same as the behavior
before this PR.
CC_LANGFUSE_SYNC_SESSION_END=truekeeps the SessionEnd run in the foreground. A user who looksfor a missing last turn can set it. The CLI then waits for the upload and shows the failure. The
README documents the option and has a row for it in the troubleshooting table.
SessionEnd payload forks the pytest process, and the rest of the suite runs two times.
Tests
The suite has 205 tests, and 19 of them are new. All of them pass. The first commit alone passes
197 tests, so both states in this PR are green.
tests/unit/test_pending_work_check.pytests_has_pending_workfor these inputs: new bytes inthe transcript, deferred agent turns, stashed task notifications, and an open turn. It also tests
an old timestamp, an entry with an empty timestamp field, and an unknown session. Two more tests
go through
main().tests/unit/test_lazy_import_skips_when_no_work.pyloads a new copy of the module and does notstart the import. It then tests that
main()leavesLangfuseunset for three cases: a pluginwithout keys, a missing transcript, and a SessionEnd run with no work. A fourth test shows that a
run with work starts the import and builds a client.
tests/unit/test_import_failure.pytests the contract of the old guard at module level. When theSDK is not available and the session has work,
main()returns 0 and the diagnostic reaches thelog.
tests/unit/test_session_end_detach.pytests the fork for four cases. Two cases are the branch ofthe parent process and the branch of the child process with
setsidand the three redirects. Theother two cases are a fork that fails and the
CC_LANGFUSE_SYNC_SESSION_ENDoption.main(). A SessionEnd parent process returns beforeit builds a client, and the child process does the work. A Stop run stays in the foreground, and
it makes no fork. A run with no work to do returns before the fork.
open_turncondition,then the staleness condition. The suite fails for each of the three changes.
fork at all, no
setsid, and a parent that continues instead of a return.tests/unit/test_image_capture.pyfrom feat(hook): capture image blocks as markers and opt-in Langfuse media #56 still passes with the media import now lazy.An end-to-end test against a local Langfuse with a real nested Claude Code session shows three
results. The normal path stays the same. The Stop run marks the turn as an open turn, the SessionEnd
run completes it, and the trace has the output field. A second SessionEnd run skips the import. An
entry with a timestamp of 8 days uses the normal path and gets a new timestamp.