Repository navigation
🧳 fix: Skip Agent Tool Schemas During File Inspection - #16804
dustinhealy wants to merge 2 commits into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused change preserves file validation and source-specific inspection, with regression coverage and no unresolved findings.
Review effort: Balanced
Findings: None
What changed in this PR
Limits agent file-reference inspection to file-bearing fields, preventing unrelated nested tool schemas from rejecting valid attachment turns.
Changes:
- Projects the four supported file-bearing fields before opaque-file validation.
- Adds a regression test for hydrated attachments alongside deeply nested tool schemas.
| File | Description |
|---|---|
| packages/api/src/middleware/modelBoundContent.ts | Restricts file-reference inspection to supported runtime fields. |
| packages/api/src/middleware/modelBoundContent.spec.ts | Tests that unrelated tool schemas do not trigger traversal failures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: eb2e6bb5e5
ℹ️ 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".
dustinhealy
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcb1c373c9
ℹ️ 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".
| resolvedFilesById: ReadonlyMap<string, CanonicalFileInspectionFile>, | ||
| context: { | ||
| readonly messageCount?: number; | ||
| readonly omittedRootKeys?: ReadonlySet<string>; |
There was a problem hiding this comment.
When files.pii.uninspectable is block and an agent has no hydrated file with a usable file_id, agentFilesById is empty, so the early return at line 1154 ignores this new omittedRootKeys option. assertInspectableFileInput consequently receives the original agent and still traverses toolDefinitions; a sufficiently deep or large initialized schema rejects an otherwise valid attachment-free turn as content_filter_uninspectable. Apply the root-key omission even for an empty resolved-file map, and retain the fast path only when no omissions were requested.
Useful? React with 👍 / 👎.
| MAX_PROVIDER_PROVENANCE_INDEX_REFS + MAX_PROVIDER_PROVENANCE_PARTS; | ||
| const MAX_PROVIDER_STORED_STATE_WORK = | ||
| MAX_PROVIDER_PROJECTION_WORK + MAX_PROVIDER_PROVENANCE_PARTS * 2; | ||
| const AGENT_FILE_INSPECTION_OMITTED_ROOT_KEYS = new Set(['toolDefinitions']); |
There was a problem hiding this comment.
With files.pii.uninspectable: block, a Responses API caller can place an opaque file payload such as data:application/pdf;base64,... in a function tool's description or parameters and have it sent to the provider whenever the agent also has a hydrated attachment. Fresh evidence beyond the earlier thread is that api/server/controllers/agents/responses.js:1136-1145 merges request.tools into primaryConfig.toolDefinitions after the initial check at line 1078, while the post-merge check at line 1196 reaches this new omission; extractAgentContent then subjects the schema only to tool-argument/agent-instruction policies, not the file policy. Preserve opaque-file validation for caller-supplied definitions before omitting trusted initialized schemas.
Useful? React with 👍 / 👎.
codegraph-librechat
Bot
added
🗺️ Content Safety
Summary
Agent file-reference inspection traverses the complete initialized runtime agent whenever the agent has a hydrated attachment. Runtime agents can also contain deeply nested initialized tool definitions, causing locator sanitization to exceed its traversal depth and reject otherwise valid attachment turns with
content_filter_uninspectable.This change omits only the top-level
toolDefinitionsbranch while sanitizing resolved agent file locators. All other agent fields, including instructions, model parameters, attachments, and tool resources, remain subject to opaque-file inspection. Tool definitions continue through their existing source-specific content inspection path.How it works
omitResolvedCanonicalFileLocatorsnow accepts a bounded set of root keys to omit from its inspection copy. The agent path uses that option fortoolDefinitions, avoiding traversal of initialized schemas without weakening inspection of other provider-bound fields.Type of change
Testing
The regression coverage verifies both sides of the boundary:
Tested environments/configuration:
Automated tests:
npm test --prefix packages/api -- --runInBand --coverage=false --watch=false modelBoundContent.spec.tsprettier --check <changed files>eslint <changed files>Screenshots / recordings
No visual UI change; this is backend content-validation behavior.
Risk / compatibility
The omission is limited to initialized tool definitions at the agent root. Provider-bound agent fields and unresolved file references retain fail-closed behavior.
Checklist