🚦 feat: Gate Provider Text Before SDK Release - #593
lia-by-librechat[bot] wants to merge 12 commits into
Conversation
|
Head: 86129f9. B1 adds default-off awaited provider-text release, bounded shared attempt budgets and A1 dispatch/lifecycle tests. Exact-head checks and independent review are running. SDK publication and consumer activation are not included. |
|
Head: b2187d3. Fixes both initial P1 findings: preserve validated provider controls; remove completed-wait cancellation retention and bound empty/control overhead. Includes fail-closed adapter/configuration admission fixes. Focused gate tests: 43 passed. Independent review and exact-head regression checks are running. No release or activation. |
|
Verified head b2187d3: 335 focused tests across 18 suites passed, including 43 B1 dispatch/lifecycle tests and tracing/subagent regressions. Typecheck, CJS/ESM/declaration build, touched-file ESLint and import-order checks passed. The 30,000-wait GC check retained zero completed results and zero abort listeners before cancellation. Both initial P1 findings are fixed. Exact-head independent review is still running. |
|
Head: 3da55e9. Fixes R3/R4 (P1) and R5 (P2) after a producer-invariant sweep: reject uncertified compositions and internal-streaming invoke before production; preserve native nonstreaming controls. Gate tests: 53 passed. Types, CJS/ESM/declaration build, touched-file lint/imports and built ESM/CJS dispatch checks passed. Exact-head focused regressions and independent review are running. No release or activation. |
|
Head: 6bfed9d. Fixes R6/R7 (P1): reject terminal-provider custom transform routes and check effective facade/delegate invocation parameters before original generation. Regressions cover real Run publication/state, modelKwargs, concurrent admission and per-call overrides. Exact-head focused checks and independent review are running. No release or activation. |
|
Head: dc60d38. Fixture-only correction explicitly selects the guarded streaming route; the prior fixture correctly hit disabled-streaming rejection. All 58 B1 tests pass. R1-R7 remain fixed. Exact-head independent review, focused regressions and CI are running. No release or activation. |
|
Head: 9a531c7. Fixes R8 (P1): preserve validated Anthropic untyped argument fragments, including interleaved calls in real registered/local Run dispatch. Fixes R9 (P2): monotonic elapsed-time checks prevent synchronous policy work from releasing after its deadline. Coalesced fragment/tool-index bookkeeping is bounded. All 68 B1 tests, types/build and touched-file checks passed. Exact-head independent review and focused regressions are running. No release or activation. |
|
Head: 7a0c5a7. Fixes R10 (P2): queued and post-await stream consumers honor protection trips before eager tool dispatch. Real Run regression pauses tool-step publication across a required-policy rejection; parsed/raw-seal, inherited parent-entry and fallback-preparation checks pass. Shared callback/hook contracts are unchanged; C1 retains tool-result ownership. Exact-head independent review and focused checks are running. No release or activation. |
|
Head: 615c447. Fixes R11/R12 (P1): existing ToolNode admission/pass-through guards honor provider safety errors; foreground child failures abort the captured parent breaker. Hardens R13 (P2) with SDK-owned restart provenance, preserving protected steering while Stop and host-policy failures remain terminal. C1 result hooks and shared callback contracts are unchanged. Exact-head independent review and focused regressions are running. No release or activation. |
|
Head: bbb5b1c. Fixes R14 (P2): native malformed-tool diagnostics survive copying and canonical assembly instead of being reset by the chunk constructor/concat. Real OpenAI empty/prose invalid-only and mixed-valid/invalid turns preserve paired errors and next-model recovery. ESM/CJS native diagnostic checks passed. Exact-head independent review and focused checks are running. No release or activation. |
|
Head: 2239626. Fixes R15 (P1): capture inherited constructor/call signals and honor trips at existing eager dispatch guards. Fixes R16 (P2): protected attempt signals bypass SDK read-ahead smoothing; raw producers retain their charged lease until actual settlement. Configured delays and unprotected smoothing are unchanged. Real Bedrock and generic adapter tests cover retention beyond smoother close grace. Exact-head independent review and focused checks are running. No release or activation. |
|
Head: 454ad7c. Fixes R17 (P2): marked OpenRouter cumulative prose is normalized against bounded fragments before policy inspection. Only new suffixes append; signatures, unmarked repetition and other providers are unchanged. Real HTTP callback/state tests, fallback/retry, parallel isolation and next-model/tool reuse pass. Exact-head independent review and focused checks are running. No release or activation. |
|
Head: 5b7f922. Fixes R18 (P2): delayed canonical prose gets a message-creation step after tool controls instead of being dropped from live deltas. Real OpenAI streaming/native local/registered turns publish once; Stop during awaited step creation suppresses the subsequent delta. Tool controls remain timely and exact. ESM/CJS mixed-turn publication checks passed. Exact-head independent review and focused checks are running. No release or activation. |
|
Delivered head: 5b7f922. Independent exact-head review: complete, no findings. R1-R18 rechecked; earlier 10 P1 and 8 P2 findings fixed/hardened, none rejected. All GitHub CI checks passed. Local verification: 828 focused tests across 29 suites, workspace types, CJS/ESM/declaration build, touched ESLint/imports, circular check and built-entry release/publication/replay/retention checks passed. One pre-existing tool-stream benchmark is skipped. Full local suites, live-provider tests and D1 sink certification were not run. B2 requires an approved SDK release, release pin, capability version 1 check and host policy adapter. No package publication or activation. |
|
@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: 5b7f922a72
ℹ️ 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".
| for (const key of Object.keys(message.additional_kwargs)) { | ||
| if (additionalKeys.has(key)) continue; | ||
| const validate = additionalControls[key]; | ||
| if (validate == null || !validate(message.additional_kwargs[key])) { | ||
| throw new ProviderTextProtectionError('unsupported'); |
There was a problem hiding this comment.
Protected Google calls fail before inspection because the built-in Gemini converter always adds __gemini_function_call_thought_signatures__ to additional_kwargs (even when the map is empty), while this allowlist rejects that key. Consequently every streamed and non-streamed Providers.GOOGLE response throws unsupported instead of releasing approved prose; safely validate and preserve the SDK-owned signature map and other certified Gemini controls.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
| valid = metadataKeys.has(key) && (value == null || typeof value === 'string' || | ||
| (typeof value === 'number' && Number.isFinite(value))); | ||
| } | ||
| if (!valid) throw new ProviderTextProtectionError('unsupported'); |
There was a problem hiding this comment.
When ChatOpenAI selects the Responses API, its mandatory response.completed chunk copies standard response fields such as output, status, text, and tools into response_metadata; this branch rejects those keys and structured values. Thus every protected Responses API request reaches completion and then fails with unsupported, discarding otherwise-approved output, so the boundary needs a validated allowlist for the SDK's Responses metadata shape.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
| primaryError instanceof InvalidModelToolCallError || | ||
| primaryError instanceof ProviderTextProtectionError | ||
| ) { | ||
| if (primaryError instanceof ProviderTextProtectionError) attemptBreaker.abort(primaryError); |
There was a problem hiding this comment.
When one agent's policy trips while a parallel agent is compacting, this aborts the shared breaker with ProviderTextProtectionError, but findStreamLimitAbortReason and the recovery catches in src/summarization/node.ts recognize only StreamLimitExceededError. The summarization branch can therefore treat the safety abort as an ordinary provider failure, attempt fallbacks, emit completion hooks, or commit a periodic metadata stub before the run rejects; extend those summary guards to recognize and immediately rethrow the protection reason.
Useful? React with 👍 / 👎.
| if (policy != null && config?.signal?.aborted === true) { | ||
| const reason: unknown = config.signal.reason; | ||
| const cancelled = new ProviderTextProtectionError(reason instanceof ProviderTextProtectionError ? reason.code : 'cancelled'); | ||
| prepared?.finish(preparedAttempt, undefined, cancelled); | ||
| throw cancelled; |
There was a problem hiding this comment.
In a protected run, any aborted invocation is converted here to ProviderTextProtectionError('cancelled') unless the signal reason is already a provider-protection error. This also converts StreamLimitExceededError and PreparedSubagentError: for example, throwIfBreakerTripped throws the stream-limit reason, this catch replaces it, and parallel execution can surface a misleading cancellation instead of the actionable breaker failure. Preserve known safety reasons and only normalize generic provider abort errors.
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
Implements AI-2214 / PII B1. Default-off, injected provider-text protection runs before native aggregation/callback publication and SDK state/reuse. An
onChunkfilter is too late.Evidence
A1 corpus from LibreChat #16726 drives real SDK dispatch tests: every two-chunk split, allowed controls, OpenAI/Anthropic HTTP and Bedrock transports, native callbacks, registered/local dispatch, primary/fallback/retry, parallel budgets, Stop/late completion and model/tool reuse. Native Anthropic nonstreaming controls and both built package entries are exercised. OpenRouter cumulative replay is inspected once, including fallback/retry and parallel candidates, registered/local dispatch and next-model reuse.
Checks at
5b7f922atsc --noEmit, CJS/ESM/declaration build, touched-file ESLint/import order and circular checks passed.SDK source baseline:
main@bc5e34b411a500953d4cd745b1439e06780f77a6, package 4.0.2. LibreChat's A1 snapshot locks 4.0.1 and is separate evidence.Review ledger
b2187d33b2187d333da55e973da55e973da55e976bfed9d16bfed9d19a531c7d9a531c7d7a0c5a70615c447a615c447a615c447abbb5b1c82239626f2239626f454ad7c05b7f922aSwept producer admission, alternate generation routes, callback/state ordering, cancellation/leases, recovery and policy inheritance. Queued consumer regressions include real Run publication paused across a policy rejection, parsed and raw-seal eager dispatch, inherited parent trips and fallback preparation. Independent review completed with no findings for exact head
5b7f922a72a430ce242d5fec5a29f4fa567605d3. R1–R18 were rechecked. Earlier findings: 10 P1 and 8 P2, all fixed or hardened as recorded above; none rejected. All GitHub CI checks passed at this head.Ownership and release
C1 owns tool-result/ToolNode enforcement. B1 leaves shared callback/hook utilities unchanged, adds a provider-specific Run/graph field and recognizes its safety errors in existing stream-breaker guards. C1 retains tool-result release and completion ownership. ToolNode changes only add the provider safety error to existing admission/pass-through predicates; no result hooks are altered. D1 independently certifies trace exports. See
docs/provider-text-protection.mdfor supported shapes and ownership.B2 needs an approved SDK release containing this contract, a capability-version check, a release pin and its classification/policy adapter. Unsupported routes stay gated. No npm publication or app/YAML activation.