feat: name Codex threads from ACP session metadata - #425
Closed
Hacktor-Banned wants to merge 2 commits into
Closed
Conversation
ACP clients that run several sessions against one agent had no way to label them: every thread showed the same derived title. The title is carried out of band in `_meta` so it never enters the prompt and costs no tokens. The applied title is reported back through `SessionMetadata` so the session's title state is seeded synchronously as "explicit", which stops a later prompt- or history-derived fallback from overwriting a title the client asked for. Naming is cosmetic, so a failed `thread/name/set` is logged and swallowed rather than failing session creation. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> (cherry picked from commit bba8856)
`readMetaSessionTitle` accepted any non-blank string. A client could persist an unbounded title, or one carrying a BEL, a zero-width space, or a bidi override, straight into the Codex thread store — `normalize_thread_name` only trims. This is the ACP boundary for arbitrary clients, so the adapter sanitizes independently of its callers: whitespace collapses, `Cc`/`Cf`/`Cs` are dropped, and the result is capped at `SESSION_TITLE_MAX_CHARS` code points so the cap cannot split a surrogate pair. Scoped to the `_meta` path; history- and prompt-derived titles are trusted inputs. Naming the thread also left ACP title state untouched. `thread/name/set` resolves before any thread notification handler exists, so Codex's `thread/name/updated` is dropped, and `NewSessionResponse` has no title field — a client that supplied the title through opaque `_meta` had no way to see it reflected. The adapter now publishes one `session_info_update` carrying it. That publish must land after the `session/new` response: the SDK client installs its per-session update queue only when `session/new` resolves and drops updates for sessions with no queue, so an update emitted inside the create path would be lost. `setImmediate` puts it behind a macrotask boundary, and the publish re-checks session liveness, generation, and title identity so a session closed or renamed in the gap is not told about a title that is no longer its own. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> (cherry picked from commit 37369ba)
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
2 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
sessionTitlefromsession/new_metaand apply it with Codexthread/name/set.session_info_updateafter thesession/newresponse.Motivation
Buzz already sends a bounded, out-of-band session title in block/buzz#3028. Without the adapter half, Codex still derives the thread title from the first prompt, which can turn a complete orchestrator prompt into the task title. Using ACP metadata keeps the label out of the prompt and avoids spending tokens on title plumbing.
This rebases and supersedes #338 on current
main. It preserves the original commits, authorship, and DCO sign-offs from @wpfleger96 while resolving the intervening thread-fork and test-fixture API changes.Testing
npx vitest run --no-file-parallelism src/__tests__/CodexACPAgent/session-title-from-meta.test.ts src/__tests__/CodexACPAgent/session-title-ordering.test.ts(15 passed)npm run typechecknpm test(454 passed, 28 skipped)npm run buildnpm run codex-test -- -p "Reply with exactly: title-ok" -o summary_meta.sessionTitle, read the persisted thread name back, and archived the test threadRelated