Repository navigation
🚥 feat: Gate Synchronous Tool Results Before Release - #597
lia-by-librechat[bot] wants to merge 14 commits into
Conversation
|
Head: 68df78e C1 mandatory synchronous tool-result gate. Direct/native callbacks, host success/error, eager completion, post-hook replacement, references, nested local/programmatic execution and child inheritance use canonical releases. Required failures are terminal; unsupported artifacts/files/media/background paths stay gated. Focused baseline: 355 tests plus 3 added child/approval cases passed. Workspace types, CJS/ESM/declarations, touched lint/import order and dependency cycles passed. Tracing/approval/resume regressions and independent review are running against this head. No docs, activation or package publication. C2 requires an approved SDK release/pin and adapter/sink certification. |
|
Head: c0e10d2 Fixed independent R1-R5 in 3f3c3a8: raw-free eager rejections/cardinality, nested pre-call errors and local denial text, terminal remote protection failures. Also covered remote bash dispatch, SDK safety interruptions, native callback/error-handler ownership and opaque/accessor message aliases. Integrated current main at 265d97f without rewriting history. Current-head checks: 375 focused tests, 353 approval/tracing tests and 232 child/host-argument tests passed. Workspace types, CJS/ESM/declarations, touched lint/imports, dependency cycles and built C1 exports passed. Fresh independent review follows for this exact head. No docs, activation or package publication; C2 still requires an approved SDK release containing C1 plus exact pin/capability and adapter/sink certification. |
|
@codex review the latest head, final 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. |
![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: c0e10d264c
ℹ️ 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".
|
Head: 3d39e2f Fixed R6-R8 in 3d39e2f after a producer/reuse/projection invariant sweep. Checkpoint-owner-validated replay re-inspects canonical content without repeating settled side effects. Full approved references remain separate from model previews. Text-only tuple-format exceptions retain native error status and callbacks. 378 focused SDK tests and 353 approval/tracing tests passed. Separate types, CJS/ESM/declarations, declared exports, touched lint/import checks and dependency cycles passed. No docs, activation or publication. Fresh independent review is running for this exact head; C2 still needs an approved SDK release/pin, capability checks and adapter/app sink certification. |
|
Head: 0333d5f Fixed Codex CX1/CX2 and independent R9/R10 in 0333d5f. Host envelopes are validated before accessor reads, timestamping and host-result tracing. Cloudflare native outputs remain unsupported and gate before sandbox work. Direct approval responses are inspected in full before truncation. Local runners drain accepted bridge requests before checking required failures. Error-handler replacements are re-inspected before reuse. 432 focused/backend tests and 353 approval/tracing tests passed. Separate types, CJS/ESM/declarations, declared exports, touched lint/import checks and dependency cycles passed. Fresh independent review follows for this exact head. No docs, activation or package publication. C2 requires an approved SDK release containing C1, exact pin, capability/policy checks and adapter/app sink certification. |
|
Head: 61dad14 Fixed independent R11-R13. Uncertified executable/request aliases reject before execution; standalone native boundaries validate supplied policy versions; accepted local bridge handlers drain even after client disconnect. 75 C1 dispatch regressions and separate types passed. Broader focused/backend, approval/tracing, child/authority and static/build checks are running against this head alongside CI and fresh independent review. No docs, activation or publication. C2 still requires an approved SDK release containing C1, an exact pin, version-1 capability/policy checks and trusted adapter/app sink certification. |
|
Head: d1ec9ff Fixed independent R14/R15. Checkpoint references now retain SDK source/version/protection provenance. Selected earlier-turn values are re-inspected before restoration or substitution; legacy or newly selected uncertified reference snapshots fail closed. Unselected host outcome/outcome_patch controls survive routing validation; selected aliases stay gated. 145 result/reference regressions passed, including direct/host source replay, allowed controls, reblocking, legacy/mixed-version gating and real eager outcome controls. Types, builds, touched lint/imports, cycles and built CJS/ESM dispatch passed. Broader exact-head checks, CI and fresh independent review follow. No docs, activation or publication. C2 still requires an approved SDK release containing C1, exact pin, capability/policy version checks and trusted adapters plus app sinks. |
|
Head: 99da8a3 Fixed independent R16/R17. Required host releases use frozen dispatched identities and reject changes before inspection, during inspection and after host cleanup. Required failures take precedence over approval interrupts. SDK-owned required host handler absence/throws fail raw-free before native callback logging. Native producer identity/format mutation is gated before callbacks. 100 C1 dispatch regressions and separate types passed. Focused/reference/backend, approval/tracing, child/authority and static/build checks follow for this exact head alongside CI and fresh independent review. No docs, activation or publication. C2 still requires an approved SDK release containing C1, exact pin, capability/policy checks and trusted adapter/app sink certification. |
|
Head: ae4213e Fixed independent R18/R19. Required child-host exceptions are rejected raw-free before native callback logging. Restored registry values are inspected under the current policy before reference resolution; serialized protection flags cannot recreate live policy bindings. Unchanged live values keep their policy-bound admission, avoiding redundant inspection. Batch turns remain reserved before awaits. 163 result/reference regressions, separate types and touched lint/import checks passed. Exact-head broader checks, builds, CI and fresh independent review follow. No docs, activation or publication. C2 requires an approved SDK release containing C1, exact pin, capability/policy checks and trusted adapters plus app sinks. |
|
Head: 386dd9b Fixed independent R20. Validated producer messages are snapshotted before awaited inspection. Changes to identity, status, content, artifacts or metadata reject before native callbacks. Canonical provenance includes the immutable envelope, and metadata is copied rather than shared with the producer. 179 result/reference regressions, types and touched lint/import checks passed. Broader exact-head checks, module builds, CI and fresh independent review follow. No docs, activation or publication. C2 requires an approved SDK release containing C1, exact pin, capability/policy checks and trusted adapters plus app sinks. |
|
Head: 0781d19 Fixed independent R21. Configured local bridge cancellation becomes a raw-free terminal error before text or JSON responses. The failure latch survives ignored HTTP failures and bridge drain; SDK safety reasons retain identity. 189 result/reference regressions, types and touched lint/import checks passed. Real localhost HTTP tests cover ordinary/string canaries, typed policy errors and SDK safety controls in both response modes. Broader exact-head checks, builds, CI and fresh independent review follow. No docs, activation or publication. C2 requires an approved SDK release containing C1, exact pin, capability/policy checks and trusted adapters plus app sinks. |
|
@codex review the latest head, final review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0781d1938c
ℹ️ 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".
|
Head: 8112d9a Fixed Codex CX3/CX4 in 8112d9a. Reference collections are copied from validated indexed data before selection or inspection; proxy/subclass/iterable collections, overridden methods, accessors and symbol aliases reject before use. Detached reference fields stay stable across awaits. Envelope validation examines every own key, including non-enumerable symbols, before native callbacks or metadata copying. 208 result/reference regressions, types and touched lint/import checks passed. Broader exact-head checks and fresh independent review are running alongside CI. No docs, activation or publication. C2 still requires an approved SDK release containing C1, exact pin, capability/policy checks and trusted adapter/app sink certification. |
|
Head: b2319ce Fixed R22-R24 in b2319ce: post-callback nested revalidation, raw-free external protection errors and authenticated post-hook source envelopes. Both new Codex findings are fixed in 8112d9a; all four Codex threads are resolved.
Ledger: 21 P1 and 14 P2 fixed; none rejected or open. Independent review is incomplete. The continuation could not create Full SDK suites were not run locally. Live provider/Cloudflare and D1 serialized-exporter certification were not performed. No docs, activation, merge or package publication. C2 requires an approved SDK release containing C1, an exact pin, version-1 capability/policy checks, and trusted adapter plus app/SSE/replay certification. Unsupported paths remain gated. |
|
@codex review the latest head, final review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2319ce8ef
ℹ️ 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".
| if (requiresToolResultProtection(this.toolResultProtection, call.name) && e.message !== admittedErrorText) { | ||
| e = new Error(await protectToolText(this.toolResultProtection, call.name, call.id ?? '', e.message, 'error', config.signal) as string); |
There was a problem hiding this comment.
When a selected tool fails and errorHandler mutates the passed Error.message and then throws, the catch block logs e.message, e.stack, and the thrown handler error before this reinspection runs. A replacement containing the raw tool result therefore reaches server logs even though it is subsequently inspected for the returned ToolMessage; revalidate or normalize the error before the catch block records it.
AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
| if (toolResultProtection != null) validateToolResultProtection(toolResultProtection); | ||
| this.toolResultProtection = toolResultProtection; |
There was a problem hiding this comment.
When a tool name appears in both toolResultProtection.toolNames and clientDelegatedToolNames, this accepts the protection policy even though the delegated-call route ends the graph without passing the call through ToolNode. The client-supplied ToolMessage on the subsequent turn is then treated as conversation history and projected into the next provider request without inspect, so a result explicitly selected for mandatory protection can reach the model and generation tracing raw; reject this overlap or protect delegated-result ingestion.
AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
| if ( | ||
| existing?.policy === policy && | ||
| existing.name === name && | ||
| existing.id === id && | ||
| existing.text === message.content && | ||
| existing.status === (message.status ?? 'success') | ||
| ) | ||
| return message; |
There was a problem hiding this comment.
For a selected direct tool, the wrapper marks the canonical ToolMessage and passes that same object to native handleToolEnd callbacks; when runTool subsequently calls this function, the approved fast path returns the identical object. A callback can retain that reference and mutate its content or metadata after its awaited handler has returned, thereby changing the ToolMessage already placed in graph state before the next model request or trace consumes it, with no further provenance check. Return a detached canonical copy at this callback boundary instead of reusing the callback-visible object.
AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
Adds default-off
RunConfig.toolResultProtectionfor trusted synchronous text tools. Required inspection is separate from optional hooks and precedes native tool callbacks, host-result tracing, completion events, output references and ToolMessage/model reuse.Selected artifacts, files/media, background results, opaque/executable aliases, custom tool shells and Cloudflare-native programmatic outputs are unsupported and gated. Unselected/absent-policy behavior remains default-off. No activation, package publication or documentation changes.
Verification
Head:
b2319ce8ef5f30e1e1260813fa92def0a7eca30dtsc --noEmitReal SDK dispatch exercises A1 result/error/alias canaries and allowed controls, required handler failures, deadlines, Stop/late completion, retries, concurrent attempts, approval replay, full references, local/HTTP nested execution, standalone version checks and disconnected bridge drain. Cloudflare gating tests confirm no sandbox work for selected native outputs.
All 13 CI checks passed for this head. Independent review is incomplete: its continuation could not create the isolated lane (
already exists), and Git registration cleanup returnedDevice or resource busy. No clean review is claimed and no review is running. All focused SDK checks, including the unchanged rerank suite, passed locally. Built CJS/ESM checks exercise canonical dispatch, reference replay/restoration, hostile collections, hidden symbols, nested callback mutation, typed-error normalization, post-hook provenance and real local HTTP cancellation. Full SDK suites were not run locally. Live provider/Cloudflare certification and D1 serialized exporter certification were not performed.Review ledger
21 P1 and 14 P2 findings fixed. No rejected or open ledger entries. All four Codex threads are resolved. This ledger is not a completed independent review of the current head.
C2 prerequisite
C2 must wait for an approved SDK release containing C1, pin that exact version, require capability/policy version 1, and certify trusted text adapters plus real app/SSE/replay sinks. Merge alone does not authorize release or activation. Unsupported paths remain gated across rollback/mixed versions.
AI-2213