Repository navigation
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.
Fixes #157
Inside a Codex subagent, every hook payload carries
agent_id, andsession_idis the subagent's own thread id. The adapter already dropped subagent prompts but let everything else through. So when a subagent rancargo testand it failed with an error a procedure was learned from, thePostToolUsehook served that procedure to the subagent and recorded the injection under the child thread id. Subagent rollouts are never learned on their own (the parser refuses them and leaves them to the parent session), and the parent's trace has a different id, so nothing ever settled that injection: its outcome was never scored and the row stayed in the store for good. The subagent'sStop,PreCompactandSessionEndhooks had a similar problem, since each one started a background ingest of the child rollout that could only fail.Now the Codex adapter ignores any hook event that has
agent_idset, before it even looks at the event name. The parent session already captures what the subagent did when its rollout is learned, so there is nothing a subagent hook could add, and dropping them all is simpler than picking events one by one. The parent's own hooks are unchanged.Copilot CLI has the same symptom (#158), but it marks subagent events differently, and telling them apart there needs its own check, so it is left for a separate change.
To reproduce, I built the binary, used a scratch
TRODDEN_HOME, learned a procedure by ingesting a Codex rollout wherecargo testfailed with E0425, the file was edited and the test then passed, and fedtrodden hook codextwoPostToolUsepayloads with the same failing output: one from the root thread and one withagent_idand a childsession_id. Before the fix both got the procedure back, and theinjectionstable had a row for the child thread id next to the root one. After the fix the subagent payload gets no reply and only the root session's injection is recorded.The new test sends each hook event Codex fires, checks it maps to a moment from the root thread, then adds the subagent fields and checks it is ignored. It fails before the fix for
PostToolUseand passes after.