Repository navigation
feat: send task-derived agent name as session/new _meta.sessionTitle for codex - #1392
Conversation
…for codex Delegated Codex threads titled themselves from the prepended system prompt because the system prompt travels via the first-turn prepend fallback. Send the task-derived Intent agent name out of band as session/new _meta.sessionTitle for the codex provider, on both the create and recreate paths. The resume (session/load) path is unchanged, the system prompt is never moved into _meta, other providers are unchanged, and older adapters ignore the unknown field (backward compatible; upstream support in agentclientprotocol/codex-acp#338). Fixes intent-hq/intent#3151
🤖 Augment PR SummarySummary:
🤖 Was this summary useful? React with 👍 or 👎 |
panghy
left a comment
There was a problem hiding this comment.
Reviewed for correctness, scope, test coverage, and doc accuracy. The change looks correct and well-scoped:
- Correctness: the codex arm of
build_session_metaemits{ "sessionTitle": ... }only for a non-blank trimmed title;open_acp_sessionandrecreate_acp_session(the twosession/newpaths) passSome(&stored.name), and thesession/loadresume path passesNone, so durable-resume behavior is unchanged. The system prompt stays on the first-turn prepend and is never moved into_meta. - Scope: claude-code ignores the new parameter (its
_metashape is byte-identical), all other providers still build no_meta, and no non-codex behavior changes. - Tests:
tests_meta.rscovers the title-only shape (regardless of prompt), trimming, and the no-title/no-_metacases; the wire-level tests intests.rsassert the exactparams._metaon create and recreate and the absence of_metaonsession/load. I ran the suite locally in the worktree — all 30 codex/session-title tests (and the fullagent_sessionmodule) pass. The recording mock follows the existingspawn_mock_agent_with_session_resultpattern.
One doc gap outside the diff, flagging here since the file isn't part of this PR:
crates/intent-providers/ADDING_A_PROVIDER.md — the §2 SessionMeta mechanism row says build_session_meta "builds a provider-shaped _meta sent on session/new and session/load" and lists claude-code as the only provider. After this PR, codex also receives _meta (the sessionTitle shape) — but only on session/new/recreate, not session/load. The row is still technically correct as a system-prompt delivery table (codex's prompt still travels via FirstTurnPrepend), but build_session_meta is no longer claude-code-only and its _meta is no longer uniformly sent on both methods, so a future provider author following that table would get an incomplete picture. Suggest a small follow-up (or an amendment in this PR) noting the codex sessionTitle shape and that it is session/new-only.
Not approving per instructions; leaving as comments only.
… clarify agent-name doc comment
…eate)
Address review: the sessionTitle path lacked coverage through the real
WSS transport. The mock fixture's MOCK_AGENT_SESSION_LOG seam now records
each session/new / session/load with the request's _meta verbatim, and a
new e2e drives a codex agent (hermetic providers.paths wrapper over the
mock fixture) over WSS asserting session/new carries exactly
{ sessionTitle: <agent name> } on the initial create and again on the
resume-impossible recreate after the idle child is SIGKILLed.
|
Review addressed:
|
Problem
Delegated Codex threads appear in the Codex app titled from the prepended system prompt ("Delegating Tasks Before delegating, list the tasks…"), because Intent delivers the assembled Codex system prompt through a first-turn prepend (the pinned codex-acp adapter ignores
_meta.developerInstructions, #479) and Codex title derivation sees that common heading first.Change
Send the existing task-derived Intent agent name out of band as
session/new_meta.sessionTitlefor the codex provider:build_session_metagains asession_titleparameter; forcodexit emits{ "sessionTitle": "<agent name>" }(trimmed, only when non-blank) and nothing else — the system prompt stays on the first-turn prepend and is never moved into_meta.session/newpaths carry the title:open_acp_session(initial create) andrecreate_acp_session(the resume-impossible fallback), sourced from the stored agent session'sname.load_acp_session→session/load) passesNone— durable resume behavior is unchanged._metashape untouched; all other providers still build no_meta).Backward compatible: older codex-acp adapters ignore unknown
_metafields. Upstream adapter support is proposed in agentclientprotocol/codex-acp#338; the end-to-end fix requires a released adapter containing it.Tests
tests_meta.rs: codex_metacarries onlysessionTitleregardless of prompt; title trimming; no_metawithout a usable title (resume/blank-name cases); other-provider tests updated for the new signature.tests.rs: wire-level tests via a recording mock agent asserting the exactsession/newparams._metaon the create and recreate paths, and that codexsession/loadcarries no_meta.Verification
cargo fmt --check— cleancargo clippy --workspace --all-targets -- -D warnings— cleancargo test— green except 2 pre-existing environmental failures inintent-gitauth::tests(daemon_helper_env_builds_parseable_helper,scoped_credential_env_builds_parseable_helper_without_token_bytes) that also fail on this branch with the change stashed and pass under a hermeticHOME/GIT_CONFIG_NOSYSTEM=1— caused by the host's injectedGIT_CONFIG_PARAMETERScredential-helper entries, unrelated to this change.Fixes intent-hq/intent#3151