feat(hooks): SessionEnd lifecycle + PreCompact context injection - #172
Open
raymondginger2018-sudo wants to merge 6 commits into
Open
feat(hooks): SessionEnd lifecycle + PreCompact context injection#172raymondginger2018-sudo wants to merge 6 commits into
raymondginger2018-sudo wants to merge 6 commits into
Conversation
Zongwei9888
requested changes
Aug 9, 2026
Zongwei9888
left a comment
Zongwei9888
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for the contribution. The SessionEnd persistence use case and preserving useful context across compaction are both valuable directions. I pulled 7393f0c, ran Ruff/format checks, and ran the existing hook/session/runner suites locally (96 passed). The existing tests remain green, but the new behavior currently has no tests, and there are several contract issues that need to be resolved before merge:
SessionEndis wired to turn completion, not session termination._run_user_input()invokes it in thefinallyblock after every prompt. In the compatible hook lifecycle,Stopis the per-turn event;SessionEndfires when the session itself terminates. Please move this to the actual session-close lifecycle. If a distinct per-turn notification is needed, it should useStopor a clearly named DeepCode-specific event rather than changingSessionEndsemantics.- Matcher and reason semantics are incompatible. The patch makes
SessionEndmatcher-less and dispatches withNone, while the reference contract matches on the exit reason. It also emitscomplete/interrupted/error, rather than the documented session-exit reasons. Please preserve reason-based matching and define/map the supported reasons at the real teardown boundary. - The
hooks_config.jsonadapter needs an explicit schema and tests. The current converter accepts list-shapedhooksin every discovered source, ignores fields such aspriority(despite documenting it), and does not establish type-specific execution semantics. Please restrict the alternate shape to its intended source/version, document its provenance, validate it, and cover ordering, disabled entries, invalid entries, timeout, environment, and event aliases. - PreCompact reinjection needs a bounded, provider-safe contract. Appending arbitrary hook output as a raw
role: usermessage changes conversation semantics. Please use a clearly delimited/bounded context representation and add tests showing that it survives only a successful compaction, is absent when compaction is blocked or fails, persists into the next model request, and remains valid for the supported provider adapters. - Please add end-to-end regression tests for every new lifecycle path. At minimum: exactly-once firing at actual session termination, reason matcher selection, hook failure remaining non-fatal, no
SessionEndon ordinary turn completion, PreCompact block/failure behavior, successful checkpoint persistence, and list-format discovery.
The current red dependency-audit check is unrelated to these hook changes: the branch is behind main and still contains the vulnerable nanoid lock state fixed by 019828a. Please rebase onto the latest main so CI reruns with the security and CI fixes.
Once these points are addressed, I will be happy to re-review.
- SessionEnd: notification-only hook fired on every terminal path (complete / interrupted / error), so summaries can be persisted even when compaction never ran
…ompact context - SessionEnd fires exactly once at real session termination (AgentSession.submit(Shutdown)), never per turn; per-turn notifications belong to the Stop event. - SessionEnd honours its matcher: the session-exit reason (shutdown/interrupted/error) is the matcher input. - hooks_config.json (deepcode-hooks MCP list format) is accepted only from the user-mcp source, with explicit validation, priority ordering, timeout parsing, event aliases and optional matchers. - PreCompact checkpoint re-injection is bounded and provider-safe: per-context and total limits, only after a successful compaction. - Add e2e regression tests (tests/test_session_end_lifecycle.py).
raymondginger2018-sudo
force-pushed
the
feat/hooks-sessionend-precompact
branch
from
August 11, 2026 04:12
7393f0c to
7208044
Compare
…paths - execution._default_shell(): prefer a POSIX shell (Git Bash sh) on Windows so POSIX-syntax hook commands run; fall back to cmd.exe. - sandbox.build_exec_command(): resolve POSIX-style shell paths to a real executable on Windows (CreateProcessW cannot launch /bin/bash); job backend injects PYTHONPATH so the windows_sandbox wrapper can import core. - tools/shell.BashTool: pass wrapped.extra_env into the subprocess env so the injected PYTHONPATH reaches the sandbox wrapper. - tests/test_hooks.py: use capture.as_posix() in shell commands so WindowsPath backslashes are not escaped by sh. Local result: tests/test_hooks.py 53 passed; tests/test_agent_session.py 25 passed.
…th bare The Windows shell-path resolution in build_exec_command() was applied to all paths, including the sandbox-disabled one. That broke the upstream-locked contract (test_disabled_via_env_returns_bare expects the bare '/bin/bash -c' argv when sandboxing is disabled). Move the resolution after the disabled early return so only the Job Object sandbox path (CreateProcessW) gets a real executable path, while the disabled path keeps the bare argv untouched.
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
3 participants
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\n\nThis PR enhances the hooks subsystem with two lifecycle improvements:\n\n1. SessionEnd hook — a notification-only hook fired on every terminal turn path (complete / interrupted / error), so session summaries can be persisted even when history compaction never ran.\n2. PreCompact checkpoint reinjection — the PreCompact hook's additionalContexts are appended as a
[PreCompact checkpoint]user message after compaction, so the checkpoint survives history compression and the model can recover context.\n3. Hooks config — accepts the deepcode-hooks MCP hooks_config.json list shape with camelCase event aliases.\n\n## Changes\n\n-core/events/session.py: add_run_end_hookinvoked from the turn finally block (reason mapped from stop_reason)\n-core/agent_runtime/runner.py: capture PreCompact additional_contexts and reinject into compacted history\n-core/harness/hooks/events.py: registerSessionEndevent, add to matcher-less set\n-core/harness/hooks/engine.py: addrun_session_end; pass additional_contexts throughrun_pre_compact\n-core/harness/hooks/discovery.py: support deepcode-hooks MCP hooks_config.json list shape with camelCase aliases\n-core/harness/hooks/execution.py: register SessionEnd / PreCompact decoders\n\n## Notes\n\n- SessionEnd is notification-only: a hook failure is logged and never crashes a turn.\n- All 6 files passpy_compile; total diff +150/-12.