fix: tighten codex namespace invariants#91
Merged
Conversation
franciscojavierarceo
force-pushed
the
codex/codex-namespace-followups
branch
2 times, most recently
from
July 9, 2026 17:59
9a1cc43 to
4700030
Compare
franciscojavierarceo
marked this pull request as ready for review
July 9, 2026 18:03
franciscojavierarceo
requested review from
bbrowning,
jiahuei,
leseb,
maralbahari,
noobHappylife,
qandrew and
tjtanaa
as code owners
July 9, 2026 18:03
ashwing
approved these changes
Jul 9, 2026
Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
franciscojavierarceo
force-pushed
the
codex/codex-namespace-followups
branch
from
July 9, 2026 19:02
4700030 to
b1274ed
Compare
ashwing
added a commit
to ashwing/agentic-api
that referenced
this pull request
Jul 10, 2026
The framework shipped across vllm-project#80/vllm-project#82/vllm-project#84/vllm-project#85/vllm-project#91 (with vllm-project#83/vllm-project#89 in review) and diverged from the original proposal in several ways. Update the doc to the design-of-record: - Status: Proposal -> Accepted; add as-built preamble. - ToolHandler split into ToolHandler + GatewayExecutor (execute() only applies to gateway-owned types); Pin<Box<dyn Future>> not #[async_trait]; no discover()/event_prefix()/output_item_type() on the trait. - ToolType gains CodexNamespace (6 variants); ResponsesTool shown as the shipped #[non_exhaustive] 7-variant enum incl. namespace + Unknown catch-all and the web_search aliases. - ToolEntry carries handler: Option<Arc<dyn GatewayExecutor>>; dispatch is two layers (per-call registry.dispatch + multi-turn classify_round/ LoopDecision) with a diagram. - LoopDecision trimmed to 4 unit variants; mixed turns resolved by classify_round precedence, not ContinuePartial. - Add Implementation Status (PR mapping) and Future Work (layering ADR, GatewayAccumulator, per-type config, remaining handlers). - Mark Open Questions Q1-Q4 resolved; clarify collision handling is a hard error only for namespace-vs-top-level, warn-level last-write-wins otherwise. Rationale sections (Principles, Alternatives Considered) preserved. Signed-off-by: Ashwin Giridharan <girida@amazon.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
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
RequestPayload::to_upstream_requestfallible so direct callers cannot bypass collision validationtool_choicevalues with the existingNonEmptyToolNametype so empty function choices are rejected at the serde boundaryTest Plan
cargo fmt -- --checkcargo test -p agentic-core --test stateful_responses_integration test_codex_namespace_collision_with_top_level_function_is_rejected --quietcargo test -p agentic-core tool::codex::tests --quietcargo test -p agentic-core --test tool_normalization_test --quietcargo test -p agentic-core codex --quietcargo clippy -p agentic-core --tests -- -D warningsgit diff --checkcargo test -p agentic-core --quiet