Repository navigation
fix: keep forked Codex threads subscribed - #37
Merged
Merged
Conversation
thread/fork leaves the child subscribed. runTurn waits on turn/completed notifications, so unsubscribing the live child leaves the first prompt hanging. Unsubscribe only if install fails before the session is returned; closeSession still unsubscribes.
Collaborator
|
thanks |
Leeeon233
added a commit
to LodyAI/Lody
that referenced
this pull request
Sep 9, 2026
* fix: keep forked Codex sessions subscribed Pin acp-extension-codex to fc91dce (LodyAI/acp-extension-codex#37) so thread/fork children stay subscribed for turn/completed. Pin acp-extension-core 0.1.2 because that adapter already depends on it. This does not take Lody#534 host worktree identity changes. Adapter #35 is already on Codex main and comes along with #37. Closes #543 * docs: record Codex fork subscription pin Agent Note for pinning acp-extension-codex fc91dce and Core 0.1.2 without taking Lody #534 host identity changes. Model: grok-4.6 * fix: pin DSH main with awaited smoke tests Use merged acp-extension-dsh #14 to fix the no-floating-promises CI error without weakening lint checks. Model: gpt-6 --------- Co-authored-by: Leon Zhao <leeeon233@gmail.com>
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.
Closes LodyAI/Lody#543
Problem / pressure
thread/forkreturns a live child thread. The adapter then calledthreadUnsubscribeon that child.runTurnwaits forturn/completednotifications, so the first prompt on a Fork session never finishes. Codex can complete the turn on the server; the adapter never sees it. Stop reportsno active turn to interrupt. The next prompt isA Codex prompt is already active. The parent session still works.onSubscribedis only a lifecycle callback. It does not subscribe. This is not Codexthread/forkfailing.Summary
Keep the child subscribed when fork install succeeds. Unsubscribe only if
assignProjectfails before the session is returned.closeSessionstill unsubscribes.Visual explanation
Before / after
already activeTest plan
vitest run src/__tests__/CodexACPAgent/session-fork.test.ts --no-file-parallelism --retry=0— 3 passed (successful fork does not unsubscribe; install failure still unsubscribes).vitest run src/__tests__/CodexACPAgent/CodexAcpClient.test.ts -t 'forks an ACP session through thread/fork'— passed.turn/completed. This PR is that change plus failure-path cleanup. Live Electron E2Eforks a session and continueswas not re-run here (RUN_E2E_TESTS+ API key).Context
Authored with an AI coding agent. Reviewers should challenge: leaving the child subscribed vs any reason the old tests required unsubscribe; whether
assignProjectfailure is the only install-fail path that must unsubscribe.Lody still needs a submodule bump after this merge.