Repository navigation
🔋 feat: Renew Scheduled OBO MCP Grants Offline - #16427
lia-by-librechat[bot] wants to merge 37 commits into
Conversation
|
Exact-head review handoff for |
|
Exact-head review handoff for |
|
Exact-head review handoff for |
|
Exact-head review handoff for |
|
@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. |
![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: 7ecb549949
ℹ️ 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".
|
Exact-head review handoff for |
|
Exact-head review handoff for |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f906952c3
ℹ️ 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".
9f90695 to
b82af25
Compare
|
Review handoff for exact pushed head |
|
Exact-head review handoff for |
|
@codex review the latest head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44051ef6b3
ℹ️ 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".
|
Exact-head review handoff for |
|
Exact-head review handoff for |
5555830 to
c0d9b48
Compare
|
Review head: |
|
Review head: |
|
Head: Independent exact-head review: Complete, no findings. Full frozen-diff/critical-consumer review, eight provenance checks and in-memory revocation verification completed. Reviewer dependencies were unavailable, so real-Mongo/Jest/UI/provider/type/Lighthouse checks were not independently rerun.
Earlier B1-R1/R2/S1 remain fixed. No findings rejected. No unresolved inline threads at the final read.
Schedule UI (109), host/OpenID (94), and shared request-contract (1) tests passed on unchanged UI/wiring at earlier head CI at this read: 36 successful, 2 skipped, 6 running, no failures. CI Lighthouse passed at this exact head. Remaining: e2e (memory, shard 3/6), e2e (memory, shard 5/6), e2e (memory, shard 2/6), e2e (memory, shard 4/6), e2e (memory, shard 6/6), e2e (memory, shard 1/6). Local Legacy rollout requires quiesced writers and inventory/backfill before client TTL expiry across dormant owners. Ambiguous or already-orphaned unmarked records require operator provenance recovery. Names are never proof. No production inventory/apply, provider certification, replica rollout, merge or deployment was performed. Default application OBO activation remains fail-closed. |
|
Review head: |
|
Review head: |
|
Head: Independent exact-head review: Complete, no findings. Full frozen-diff and merge-sensitive caller review completed. No independent runtime tests ran because reviewer-lane dependencies were unavailable. B1-R8 (P2), manual provenance lost in the added OBO liveness probe: fixed in this head. Only a trusted caller's explicit A disposable localhost inspector breakpoint observed
On integration parent Local Full local design-suppression validation and CI at this read: 36 successful, 2 skipped, 6 running, no failures. CI Lighthouse passed at this exact head. Remaining: e2e (memory, shard 4/6), e2e (memory, shard 2/6), e2e (memory, shard 6/6), e2e (memory, shard 3/6), e2e (memory, shard 5/6), e2e (memory, shard 1/6). Review source lanes are absent. Git administrative cleanup remains blocked by busy metadata; no forced deletion was attempted. |
|
@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: a76aa219f5
ℹ️ 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".
|
Review head: |
|
Review head: |
|
Review head: |
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
|
Review head: |
|
Review head: |
|
Review head: |
|
Review head: |
|
Head/reviewed head: The linked token-expiry failure is fixed without restoring unsafe Error logging. Its API shard passed on this head. Finding dispositions:
Independent review: Complete, no findings at this exact head. Frozen-source probes covered plaintext/auxiliary persistence confidentiality, rotating renewal, shared/different keys, old bindings, explicit replacement, retained inventory, ordinary isolation, outer diagnostics and late revoke/purge delivery. No independent Jest, browser, real MongoDB/SDK or distributed/provider certification is claimed.
UI 111, real-Mongo/provenance 98 and the other workspace typechecks were verified at parent Local Lighthouse with the existing Chromium binary exceeded the bounded worker limit (124); no completed local metric is claimed. Exact-head CI Lighthouse and Static checks passed. Local design-suppression validation previously timed out. Default capability remains fail-closed. No production inventory/migration, provider certification, activation, replica rollout, merge or deployment occurred. CI snapshot: 38 passed, 4 running, 2 skipped, no failures. The remaining jobs are memory E2E shards 1/6, 2/6, 5/6 and 6/6. The previously failing API shard, Lighthouse and Static checks passed at this head. All inline review threads are resolved. CI is not yet complete. |
|
@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: 7b259a7bd8
ℹ️ 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".
| private static async executeTokenRefresh({ | ||
| userId, | ||
| serverName, | ||
| scheduledGrant, |
There was a problem hiding this comment.
When a scheduled refresh provider throws an unstructured error recognized by isInvalidClientMessage (for example, Error('invalid_client') rather than an object with an error field), this function retains scheduledGrant for the refresh row but the cleanup branch calls deleteClientRegistration without forwarding it. That helper consequently targets mcp: instead of scheduled-mcp:, leaving the scheduled client record orphaned after the refresh row disappears from the grant list; pass scheduledGrant into that cleanup call as well.
AGENTS.md reference: AGENTS.md:L44-L46
Useful? React with 👍 / 👎.
| throw missingGrant(); | ||
| let response: GrantResponse; | ||
| try { | ||
| response = await provider.exchange(accessToken, `${target.scopes} offline_access`); |
There was a problem hiding this comment.
When the configured OpenID issuer or token endpoint changes while an existing browser session remains live, the enrollment checks bind the session only to the persisted user's old issuer; they never verify that issuer is still accepted by the current getOpenIdConfig() provider. The preview fingerprint also omits provider metadata, so clicking a previously opened confirmation can send the old provider's bearer assertion to the newly configured token endpoint. Validate the session issuer against the active provider (including supported issuer templates), or bind the provider configuration into the preview, before performing this exchange.
AGENTS.md reference: AGENTS.md:L44-L46
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
Scheduled Chats cannot renew an
obo:MCP credential after the browser session ends. B1 adds explicit resource-grant enrollment, encrypted downstream refresh grants, coordinated offline renewal, revocation, and preflight/execution/resume wiring. Related to #16157.The default OBO host remains fail-closed until its invocation-authority adapter is installed and supported-provider activation gates pass. An operator allowlist does not enable enrollment or credential delivery. Retained grants remain visible for cleanup. No browser-login refresh token is persisted; direct bearer passthrough and interactive resource-server rejection are outside this PR.
How it works
The preview binding covers owner, tenant, schedule revision, root agent, server, resolved URL and scopes. Enrollment checks it before exchange and persistence. Decrypted custom variables never enter the preview response. Preview initializes only the requested selected server. The preparation form uses the shared themed Checkbox with label and keyboard support.
Scheduled storage and refresh/teardown keys use a separate purpose. Decrypted resource URLs are not persisted in token metadata: a domain-separated keyed fingerprint binds the exact URL during enrollment, retrieval and renewal. Schedule deletion quiesces renewals and rollback, advances each grant persistence fence, and holds teardown through removal. Credential diagnostics are bounded metadata through storage, the token coordinator, custom-variable lookup/decryption, OBO trust lookup, and immediate and outer discovery/recovery/runtime consumers. Error causes and failure propagation are retained. Storage propagates failures without duplicate raw logs; helper throw/fallback semantics are retained. Live operator, role and credential-binding authorization is rechecked after retrieval/renewal/adoption before delivery and after enrollment exchange before persistence. Final authority I/O precedes the last teardown-fenced generation snapshot, so completed grant revocation or purge cannot be bypassed by a manual read. Denied grants remain available for cleanup; dependency outages remain retryable. Ordinary MCP server names, including
schedule-obo:names, remain valid. Legacy pre-release OBO records are unavailable to ordinary OAuth, remain revocable/deletable, and require explicit re-enrollment before use. Their purpose is retained on refresh rows, and client metadata covers the refresh lifetime. A dry-run-first, tenant-safe inventory/backfill protects dormant owners before legacy client TTL expiry. Listing and maintenance share the same duplicate-aware provenance rule. Conflicting client records cannot be tagged by listing or produce a rollout-ready result, even when a refresh row was previously tagged.Type of change
Testing
Focused regressions cover disclosure, preview drift, ordinary/scheduled credential coexistence, legacy cleanup, purpose-specific coordination, rotation/adoption, cancellation, exact resource binding, owner/tenant/agent checks, activation/resume and separate consent UI. MCP SDK, OpenID adapter and real MongoDB tests are used. Exact-head results and review coverage are recorded in the handoff comment. Builds and workspace typechecks are separate checks.
Screenshots / recordings
The retained-grant cleanup surface has no prior equivalent. Real local-app light/dark captures were generated earlier, but GitHub rejected attachment upload with
unsupported authentication type. No uploaded screenshots are claimed. Current preview and cleanup behavior is covered by focused UI tests.Risk / compatibility
Updated from dev
114394c6a02a3343bab495e7baa9a9a6be89235b, preserving its consent, execution-policy, resource-bearer and receipt-recovery paths alongside the B1 grant lifecycle. Trusted manual provenance is captured once in the immutable credential context across liveness/preflight, verified Run Now admission and authenticated job restoration. Paused manual grants can renew while retaining fresh authority checks; automatic, unknown and body-spoofed classifications receive no disabled-row exception. Host-injected authority must enforce current consent, absolute expiry/revocation and the applicable read-only policy; storage, provider and authority dependencies remain injected.Plaintext pre-release scheduled URL bindings and changed replica keys require explicit re-enrollment; retained grants remain visible for cleanup. Inventory readiness alone is not provider or credential acceptance proof.
For installations with pre-release grants, quiesce legacy writers and run
npm run migrate:scheduled-obobefore client TTL expiry. Apply with-- --applyonly after the inventory has no ambiguous rows. A verifiedready: trueresult is required before rollout. Already-orphaned unmarked records require operator provenance recovery; server names are never proof. No production migration was executed.Supported-provider acceptance, recurring runs spanning expiry without a browser, compatible replica rollout and rollback remain activation gates. Synthetic fixtures do not certify a deployed provider. #16157 remains open. No production credentials, configuration or services were changed.