Skip to content

fix(mcp): await restart before rebinding the requesting agent - #930

Draft
breedx wants to merge 3 commits into
mpfaffenberger:mainfrom
breedx:fix/upstream-awaited-mcp-restart-0827
Draft

fix(mcp): await restart before rebinding the requesting agent#930
breedx wants to merge 3 commits into
mpfaffenberger:mainfrom
breedx:fix/upstream-awaited-mcp-restart-0827

Conversation

@breedx

@breedx breedx commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

/mcp restart currently reports success and rebuilds the agent while the old connector is still stopping and its replacement is only scheduled. Await the actual stop, replacement and startup before rebinding the requesting agent, and distinguish failures from completed restarts.

Dependency and related work

Depends on #924; do not merge before it. This branch contains #924's unchanged lifecycle commit plus two restart commits. The incremental restart diff is eb11d855..3fa8db37.

The dependency is behavioral: current main awaits lifecycle cleanup while holding the registry lock that cleanup itself needs. A direct restart port hangs on healthy shutdown; #924 provides the unlocked, bounded stop contract this workflow requires. #924 alone still allows the existing command to rebind before completion, demonstrated by the failing command control.

Given the plugin-first contribution guidance, please confirm the minimal existing command-handler seam before landing. This fixes an existing command; it adds no command or plugin architecture.

Change

  • Retain one asynchronous command task and reject duplicate pending restart commands.
  • Await stop before constructing/replacing a connector; refuse replacement after an incomplete stop or while other start/stop operations are pending.
  • Serialize direct restart callers for the same connector.
  • Check retained live-toolset ownership even after a failed attempt disables availability; repeated retries cannot replace a connector still owned by another agent.
  • Capture the requesting agent, then rebuild only after successful startup.
  • Invalidate cached agents on failure/cancellation; report rebuild failure as partial success rather than claiming everything succeeded.
  • Add translated status keys and real lifecycle/command ordering tests using local prefixed toolsets.

Reload/remove and general concurrent lifecycle ownership are deliberately outside scope. A timeout bounds waiting, not a guarantee of process termination.

Validation

Linux / Python 3.13.13, PydanticAI 2.35.0, MCP 1.27.1, fastmcp-slim 3.4.7, core plugins 0.0.44. Dedicated environment, disposable HOME/XDG, blocked socket connect/DNS/bind; no dependency changes.

  • Prerequisite-only control: real command regression fails because construction/rebinding occur before asynchronous shutdown.
  • Candidate: 16 focused tests passed, covering command order, duplicate requests, failed stop/construction/start, partial-success reporting, cancellation, direct-call serialization, pending-operation refusal and retained ownership across repeated retries.
  • Fresh review found the disabled-toolset ownership bypass. Both added regressions fail before the correction and pass afterward, including successful restart only after the last owner drains.
  • Broad agents/MCP/command/subagent selection: 1,360 passed, 6 skipped, 2 warnings. Skips are existing unimplemented MCP-search cases; both stream callback warnings reproduce on unpatched upstream.
  • Combined with the independent callback cleanup fix: 1,364 passed, 6 skipped, no warnings.
  • Ruff lint/format and diff checks pass.

No live remote connector, multi-platform or release qualification is claimed. Public CI results will follow.

Keep materialized connector objects/status during manager refresh. Cancel registered lifecycle tasks under the registry lock, then drain outside it with a five-second deadline. Retain ownership on incomplete cleanup and deduplicate the shutdown callback. Restart ordering and pending startup ownership remain separate concerns.
@breedx

breedx commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

The independent stream callback cleanup tested alongside this branch is #929. It is not a prerequisite: the restart-only suite passes with the two inherited stream warnings, and the combined suite passes without those warnings. The required lifecycle prerequisite remains #924.

@breedx

breedx commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

The awaited restart and retained-owner regressions pass all upstream CI checks. Full macOS suite: 7,834 passed, 14 skipped, 28 warnings; quality and Windows encoding checks pass. This remains a dependent draft: merge #924 first, then refresh/requalify this incremental restart proposal.

CI used Python 3.14.7 despite the 3.13 label; local qualification used 3.13.13. The existing stream callback warning remains here and is independently addressed by #929. Other warning categories (plugin httpx deprecations, SQLite resources, deprecated config APIs and unrelated test coroutines) were inspected, not suppressed; not every CI warning has an independent base reproduction. No release-qualification claim.

Run: https://github.com/mpfaffenberger/code_puppy/actions/runs/34231192129

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant