Skip to content

🔌 fix: Prevent Repeated Idle Check Triggers for Users With Failed MCP Connections#12853

Merged
danny-avila merged 1 commit into
danny-avila:devfrom
darthhexx:fix/user-last-activity-leak-perpetual-disconnect
Apr 29, 2026
Merged

🔌 fix: Prevent Repeated Idle Check Triggers for Users With Failed MCP Connections#12853
danny-avila merged 1 commit into
danny-avila:devfrom
darthhexx:fix/user-last-activity-leak-perpetual-disconnect

Conversation

@darthhexx

@darthhexx darthhexx commented Apr 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

We’re seeing a group of users’ MCP connections being continuously disconnected because user activity is being recorded before a connection is successfully established. If the connection never succeeds, the activity timestamp is never removed.

This PR changes the cleanup to always clear the activity timestamp, even when userMap was missing, because updateUserLastActivity is called before a connection is established in MCPManager.callTool and when that connection attempt fails, the activity entry would be present and trigger the idle check, but fail to get a userMap of connections, so it would fire repeatedly for the same userId.

Another fix would be to only set the activity after the connection succeeds, but I assume that goes against the way activity is intended to be recorded.

Change Type

Please delete any irrelevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Translation update

Testing

Please describe your test process and include instructions so that we can reproduce your test. If there are any important variables for your testing configuration, list them here.

Test Configuration:

Checklist

Please delete any irrelevant options.

  • 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
  • I have made pertinent documentation changes
  • 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
  • Any changes dependent on mine have been merged and published in downstream modules.
  • A pull request for updating the documentation has been submitted.

Copilot AI review requested due to automatic review settings April 28, 2026 03:38
Copilot AI reviewed Apr 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adjusts MCP user connection cleanup to prevent leaked userLastActivity entries from repeatedly triggering idle-disconnect logic when a connection never successfully establishes.

Changes:

  • Always clear userLastActivity in disconnectUserConnections, even when no userConnections map exists for the user.
  • Adds an inline rationale explaining why the activity entry can exist prior to a successful connection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@danny-avila

danny-avila commented Apr 28, 2026

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Apr 28, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

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

@danny-avila danny-avila changed the title Prevent triggering the idle check repeatedly for the same userId 🔌 fix: Prevent Repeated Idle Check Triggers for Users With Failed MCP Connections Apr 29, 2026
@danny-avila danny-avila changed the base branch from main to dev April 29, 2026 00:19
@danny-avila danny-avila merged commit 1f37ec8 into danny-avila:dev Apr 29, 2026
13 of 14 checks passed
fuuuzzy pushed a commit to fuuuzzy/LibreChat that referenced this pull request May 3, 2026
@darthhexx @fuuuzzy
🔌 fix: Prevent Repeated Idle Check Triggers for Users With Failed MCP…
a28bddc 
… Connections (danny-avila#12853)
mbiskach pushed a commit to mbiskach/LibreChat that referenced this pull request May 3, 2026
@claude
docs: cross-reference upstream LibreChat MCP work in plan
b5450b3 
Adds an "Upstream context and related work" section pointing at
relevant work on danny-avila/LibreChat:

- PR danny-avila#11799 + Issue danny-avila#10641: direct overlap with this plan's MCP
  Apps work; lists the 10 axes on which v8 intentionally diverges
  (self-contained HTML, no direct browser networking, one proxy
  per instance, hop-specific relay validation, manifest-hash
  approval, etc.).
- Issue danny-avila#11997: the only upstream artifact for MCP Tasks (no PR
  yet); plan's Tasks work is greenfield.
- PR danny-avila#12850: 307/308 redirect handling and credential stripping
  is merged into dev but NOT in HEAD 738003b. Earlier revisions
  of this plan claimed main already had it; corrected. Phase 0
  now tracks the upstream merge or ports the work if it slips.
- PR danny-avila#12535, danny-avila#12853, danny-avila#12910, Issue danny-avila#12802: adjacent transport
  reliability and OAuth hygiene worth tracking.
- PRs already in HEAD listed for context (danny-avila#12782, danny-avila#12763,
  danny-avila#12755, danny-avila#12745, danny-avila#12812).
- Notably absent upstream: session-id reuse correctness, header
  consistency, 404 → re-init, per-user token scoping,
  outstanding-task revalidation, legacy renderer retirement.

References section reorganized into Specs / Upstream LibreChat /
MDN subsections.

https://claude.ai/code/session_011NZqb4xN9QcXpdY2LCtnuH
mbiskach pushed a commit to mbiskach/LibreChat that referenced this pull request May 3, 2026
@claude
docs: revise MCP Apps + Tasks plan to v9 (adopt-then-layer; Preview v…
7dc07e0 
…s Hardened split)

Restructures the implementation strategy after the eighth review:

- Stop parallel-building. Phase 1 inherits upstream PR danny-avila#11799
  as the Apps Preview substrate (per-instance outer iframes,
  same-server binding, /api/mcp/sandbox auth, ui:// URI
  validation, capability gating, mcp_app artifact, test server,
  stableMCPAppRef) and verifies these with regression tests
  rather than re-implementing them.

- Split Apps phasing into:
  * Phase 2P (Apps Preview, ~1-2w): adopt danny-avila#11799 substantially
    as-is on chat surface only; consolidate to single ACL;
    truthful HostContext; payload truncation; rate limits;
    fullscreen behind appSettings.allowFullscreen; capability
    advertise behind MCP_APPS_PREVIEW_ENABLED.
  * Phase 2H (Apps Hardened GA, +2-3w later release): layer
    the v8 security delta - dedicated MCP_SANDBOX_ORIGIN,
    explicit-target-origin transport, hop-specific relay +
    proxy-stamped nonce, folded-in ui/initialize probe,
    connect-src 'none', self-contained HTML, MCPAppLaunchManifest
    review, MCPAppInstance write path, full UIResourceRenderer
    retirement across all surfaces - behind
    MCP_APPS_HARDENED_ENABLED.

- Narrow first Apps release to live chat only. Share, search,
  and plugin-rendered surfaces fall back to text in both
  Preview and Hardened GA. Cross-surface support is
  post-Hardened-GA.

- Defer MCPAppInstance full-conversation remount to Hardened
  GA. Preview relies on upstream stableMCPAppRef for parent-
  re-render survival.

- Accept upstream fullscreen support behind appSettings.allowFullscreen
  (default OFF per server) instead of stripping in delta.

- Trim Tasks v1 to status-first jobs panel. Progress bars
  deferred until upstream PR danny-avila#12535 lands; reuse rather than
  rebuild. Tasks v1 is independent of Apps tracks.

- Phase 0 starts from dev (inheriting PRs danny-avila#12850, danny-avila#12853,
  danny-avila#12910) rather than older main HEAD; remaining work is
  session reuse, header consistency, basic 404 -> re-init,
  per-user token scoping, authContextHash.

- Three independent feature flags: MCP_APPS_PREVIEW_ENABLED,
  MCP_APPS_HARDENED_ENABLED, MCP_TASKS_ENABLED. Old
  MCP_APPS_ENABLED becomes a deprecated alias for Preview
  with a startup warning.

- Default-deny sandboxPermissions on the legacy renderer
  applies even when both Apps flags are off.

Updated changelog header, Closed go/no-go decisions (added
19/20/21), Carried-forward list with track tags, current-state
table with track markers, phases (Phase 0/1/2P/2H/4),
test matrix (P/H/A tags), risks, and effort summary
(~3-4w for Preview+Tasks; full v1 ~5-7w). References section
unchanged.

https://claude.ai/code/session_011NZqb4xN9QcXpdY2LCtnuH
jcbartle pushed a commit to jcbartle/LibreChat that referenced this pull request May 11, 2026
@darthhexx
🔌 fix: Prevent Repeated Idle Check Triggers for Users With Failed MCP…
f9d9eda 
… Connections (danny-avila#12853)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@danny-avila danny-avila danny-avila approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@darthhexx @danny-avila