Add explicit agent communication logging#30516
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
| )) | ||
| } | ||
|
|
||
| fn new_agent_communication_metadata_unchecked( |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbfc682ded
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| test-log = { workspace = true } | ||
| tracing-opentelemetry = { workspace = true } | ||
| tracing-subscriber = { workspace = true } | ||
| tracing-subscriber = { workspace = true, features = ["json"] } |
There was a problem hiding this comment.
When this crate is built or tested through Bazel, the new tracing-subscriber json feature changes the Cargo dependency metadata, but MODULE.bazel.lock is not part of this commit. Regenerate and include the Bazel lockfile update so Bazel/CI does not reject the dependency drift.
AGENTS.md reference: AGENTS.md:L37-L39
Useful? React with 👍 / 👎.
| result | ||
| } | ||
|
|
||
| fn create_and_emit_agent_communication( |
There was a problem hiding this comment.
This private helper is only called once and just wraps metadata construction plus the emit call, so it adds one-off method surface instead of keeping the communication logging flow at its only call site; please inline it here or reuse it from the other emitters if it is meant to be shared.
AGENTS.md reference: AGENTS.md:L44-L44
Useful? React with 👍 / 👎.
| /// Runtime-only metadata used to emit communication lifecycle records. | ||
| #[serde(skip)] | ||
| #[schemars(skip)] | ||
| #[ts(skip)] | ||
| pub agent_communication_metadata: Option<AgentCommunicationMetadata>, |
There was a problem hiding this comment.
When any Rust client constructs or destructures InterAgentCommunication with a struct literal or exhaustive pattern, this new public runtime-only field is a source-breaking API change even though it is skipped on the wire; keep the logging metadata in a core-side wrapper/sidecar instead of expanding the public protocol type for an internal logging concern.
AGENTS.md reference: AGENTS.md:L87-L89
Useful? React with 👍 / 👎.
npancha-openai
force-pushed
the
codex/agent-communication-v2
branch
from
fbfc682 to
e196330
Compare
npancha-openai
force-pushed
the
codex/agent-communication-v2
branch
from
e196330 to
a4c1cb2
Compare
npancha-openai
force-pushed
the
codex/agent-communication-v2
branch
from
a4c1cb2 to
0e35d88
Compare
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e35d88be1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tracing::trace!( | ||
| { | ||
| event.name = "codex.agent_communication", |
There was a problem hiding this comment.
In normal TUI/app-server runs this trace! event is emitted under the module target codex_core::agent_communication, but I checked the installed layers and the OTEL log bridge only exports targets accepted by is_log_export_target (codex_otel*), while this PR also disables this same target in the feedback and SQLite log filters. As a result, the new lifecycle records are dropped in production unless a developer opts into a separate file logger; add a dedicated exported target/filter path while keeping the sensitive local sinks suppressed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I don't think otel matters here
| crate::agent_communication::emit_agent_communication_created( | ||
| &communication, | ||
| receiver_thread_id, | ||
| ); |
There was a problem hiding this comment.
For this new agent-logic path, the code now generates and emits lifecycle metadata from the real send_message/followup_task flow, but the added coverage is helper/session harness tests; I searched under codex-rs/core/tests/suite and found no integration test driving a v2 tool call through test_codex, so created/enqueued ID and kind correlation can regress in the actual tool path while unit tests still pass. Add an integration test for at least one tool path.
AGENTS.md reference: AGENTS.md:L114-L118
Useful? React with 👍 / 👎.
| pub use codex_thread::TryStartTurnIfIdleRejectionReason; | ||
| pub use session::turn_context::TurnContext; | ||
| mod agent; | ||
| mod agent_communication; |
There was a problem hiding this comment.
This non-mechanical logging rollout changes protocol metadata, core delivery paths, feedback/state filtering, and tests in one roughly 550-line diff; because complex logic changes are expected to stay under 500 lines, split the smallest coherent stages, such as metadata/helper plus focused tests, then tool/result wiring, then filter changes, so each stage can be validated independently.
AGENTS.md reference: AGENTS.md:L125-L131
Useful? React with 👍 / 👎.
## Why Multi-agent v2 communications currently use separate outbound paths: direct messages, follow-up tasks, and completion results go through `send_inter_agent_communication`, while a spawn's initial message goes through the generic input submission path. That split makes it difficult to add complete communication lifecycle logging in one place. This refactor makes `submit_inter_agent_communication` the common sink for those paths, preparing the follow-up observability work discussed in [#30516](#30516). ## What changed - Routed all current outbound `InterAgentCommunication` paths in `AgentControl`—direct messages, follow-up tasks, completion results, and multi-agent v2 spawn initial messages—through `submit_inter_agent_communication`. - Centralized the actual submission and last-task-message bookkeeping there, providing one place for the follow-up PR to instrument communication creation and successful enqueue. - Left non-communication input handling and the multi-agent v1 spawn flow unchanged. ## Testing - `just test -p codex-core 'agent::control::tests::'` (51 passed) - `just test -p codex-core 'suite::subagent_notifications::encrypted_multi_agent_v2_spawn_sends_agent_message_to_child'` (passed) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/30867). * #30872 * __->__ #30867
1 participant
Currently there is no unified way to observe agent communication lifecycle events. This adds structured logging for communication creation and successful mailbox enqueue.
The records are emitted at
TRACEunder thecodex_core::agent_communicationmodule target. You can opt in with:Created event:
{ "timestamp": "2026-06-29T03:27:51.433693Z", "level": "TRACE", "fields": { "message": "agent communication updated", "event.name": "codex.agent_communication", "communication_id": "44e6762e-e19e-478d-ab94-c5e0be15209b", "kind": "result", "state": "created", "sender_thread_id": "019f116b-4219-7210-b88e-7615ba34bcef", "receiver_thread_id": "019f116b-2ddb-7c41-a9f4-e234bb5e5fc7", "content": "Message Type: FINAL_ANSWER\nTask name: /root\nSender: /root/toy_worker\nPayload:\nCHILD_OK" }, "target": "codex_core::agent_communication" }Enqueued event:
{ "timestamp": "2026-06-29T03:27:51.434038Z", "level": "TRACE", "fields": { "message": "agent communication updated", "event.name": "codex.agent_communication", "communication_id": "44e6762e-e19e-478d-ab94-c5e0be15209b", "state": "enqueued" }, "target": "codex_core::agent_communication" }