Skip to content

🧭 refactor: Settle Turn Delivery Routing Once in Agent Initialization - #15948

Closed
danny-avila wants to merge 1 commit into
devfrom
danny-avila/turn-attachment-routing
Closed

danny-avila wants to merge 1 commit into
devfrom
danny-avila/turn-attachment-routing

Conversation

@danny-avila

Copy link
Copy Markdown
Collaborator

Summary

Three places answered "how does this agent receive its attachments this turn" from the agent object, and each read it at a different moment: BaseClient.getAttachmentDeliveryPath took the custom-endpoint dialect from agent.provider after initialization had swapped it for the backing client, the run-file encoder took it from the same field on whatever child config it was handed, and both read the Responses API setting the way the finished client holds it, while initializeAgent resolved the provider and its options only after the turn's files were loaded and admitted. Every reader of a turn route now consumes one deliveryRouting value that initializeAgent settles once, after getProviderConfig and getOptions have run and before any attachment is loaded: the file policy under the endpoint's own name, the media dialect its config declares, the Responses API decision the model call uses, and the transcription setting. This is the precursor for delivering the text of a tool-routed file when no tool can read it (#15940), whose remaining reviews all traced back to inputs that were not final when a route was read.

How it works

Runtime order in initializeAgent, keeping only the steps that carry the change:

initializeAgent
  agent.endpoint = agent.provider          # the name the file policy is configured under
  getProviderConfig / getOptions           # moved ahead of file discovery; nothing in between fed them
  agent.provider = backing client
  deliveryRouting = resolveTurnDeliveryRouting(...)   # once, with every input final
  discover + admit the turn's files
  loadTools
  ...
  return { ..., deliveryRouting }

Who consumes it, as a shallow tree:

packages/data-provider/src/resolve-llm-delivery-path.ts
└── resolveTurnLLMDeliveryPath(routing, file)   # inferred route re-resolved; chosen and legacy records stand
packages/api/src/agents/files/delivery.ts
└── resolveTurnDeliveryRouting({ agent, config }) # the one derivation
api/app/clients/BaseClient.js                    # getAttachmentDeliveryPath reads options.agent.deliveryRouting
packages/api/src/agents/files/encode.ts          # child encoder reads agent.deliveryRouting
api/server/services/Endpoints/agents/skillDeps.js # fileEncodingAgent carries deliveryRouting

Behavior that changes with the reorder: a request that fails attachment admission now resolves the provider's client options first, so a missing user key surfaces before a blocked file rather than after it, and a custom endpoint's cached model fetch is not skipped for such a request. The run-file encoder previously read the dialect from agent.provider, so a child loaded under a custom endpoint's own name did not honor that endpoint's media opt-in; it now does, matching the upload route.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

  • packages/data-provider: npx tsc --noEmit; npx jest src/resolve-llm-delivery-path.spec.ts (75 passing, 6 new: inferred re-resolution, chosen destination, legacy record, no routing, converted type, custom endpoint dialect).
  • packages/api: npx tsc --noEmit; npx jest src/agents/files/delivery.spec.ts src/agents/files/encode.spec.ts src/agents/files/session.spec.ts src/agents/__tests__/initialize.test.ts src/agents/steering (324 passing; new: routing settled after the swap with the final Responses decision, provider options resolved before any attachment load, builder dialect before and after the swap).
  • api: npx jest app/clients/specs/BaseClient.test.js server/controllers/agents/client.test.js server/services/Endpoints/agents (the delivery-path suite now configures the routing initialization settles instead of resetting client caches).
  • npx eslint and npm run sort-imports:check on every changed file.

Test Configuration:

Windows 11, Node 24.16.0, Jest per workspace.

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

`initializeAgent` resolved the provider and its client options only after the
turn's files were loaded and admitted, and the two delivery readers each rebuilt
the attachment routing from the agent object at their own moment: `BaseClient`
took the custom-endpoint dialect from the already-swapped `agent.provider`, the
run-file encoder from whatever child config it was handed.

Move `getProviderConfig`/`getOptions` ahead of file discovery, where nothing in
between fed them, and settle one `deliveryRouting` value with every input final:
the file policy under the endpoint's own name, the dialect its config declares,
the Responses API decision the model call uses, and the transcription setting.
`InitializedAgent`, the child encoder and `BaseClient` consume that value;
`resolveTurnLLMDeliveryPath` is the one place a stored route is resolved again.
@danny-avila

Copy link
Copy Markdown
Collaborator Author

@codex review

Head: 87b9a03

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-15T00:05:42.076118Z 87b9a03 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87b9a037bb

ℹ️ 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".

Comment thread api/app/clients/BaseClient.js
@danny-avila

Copy link
Copy Markdown
Collaborator Author

Closing as incorporated by #15940, merged as 2ffee7e.

I verified that this PR’s head, 87b9a03, is an ancestor of the completed #15940 head, and all 15 files touched by this PR are identical between that audited head and current dev. There is no remaining implementation to merge here. The apparent conflicts result from the squash merge.

The combined implementation passed 1,394 focused tests, both affected workspace typechecks, and local Lighthouse. Codex reported no major issues at c300dcc (review). I also rechecked and resolved the outstanding non-agent BaseClient finding: the assumed production caller does not exist; all production turns receive initialized agent routing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant