fix: make task markers inert outside docket workers - #11
Open
BrennanTM wants to merge 2 commits into
Open
Conversation
A plain tools/call resolves the task markers with no docket worker context. Pre registered red: dependency resolution raises for _task_concurrency, the admission marker cannot reach its cleanup stack, and the concurrency marker delegates unconditionally.
FastMCP resolves Dependency typed defaults on plain tools/call, where no docket worker context exists. The concurrency marker now delegates to docket only inside a worker and reads as inert otherwise, and the admission marker returns before reserving queue capacity, letting _enqueue_request apply its own backpressure with the client facing Server busy error. Worker side task enforcement is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the Demo Image smoke failure on
codex/fastmcp-v4-beta, the branch behind #7. FastMCP's dependency resolver resolves any parameter default that is aDependencyinstance on plaintools/calltoo, and both task markers placed bybuild_torch_module_fastmcp_toolare worker-only. On the direct path there is no docket worker, so resolving_task_concurrencyraisesLookupErroroncurrent_execution, which surfaces as theRuntimeError: Failed to resolve dependency '_task_concurrency' for fnin the CI log._task_admissioncarries the same exposure in its queue-full branch, whereAdmissionBlocked(current_execution.get(), ...)would replace the manager's gracefulServer busyerror with the same internalRuntimeError.The change
_WorkerScopedConcurrencyLimitsubclasses docket'sConcurrencyLimitand no-ops on enter and exit unlesscurrent_executionis set, delegating to the real enforcement inside a worker.DocketQueueAdmission.__aenter__returns before reserving when no worker owns the execution. Direct calls therefore fall through to_enqueue_request's own backpressure, the same manager-side limit that governs direct calls on main, and worker-side task enforcement is unchanged. No dependency changes: pydocket stays at 0.20.1 (its 0.21 to 0.24 releases rewrite the concurrency engine and still require worker context, so a bump neither fixes this nor is needed by it).Verification
run()of a built tool outside any worker (the smoke failure's exact shape), the admission marker reserving nothing outside a worker, and the concurrency marker delegating only inside worker context.tools/callreturns real MIST inference, and a six-way concurrent burst atmax_pending_per_tool: 1yields two successes, four gracefulServer busyerrors, zero marker errors, and a follow-up success, so no reservation leaks.test_uses_workspace_virtualenv_for_runtime) fails on my machine with or without this change.This is your migration, so treat this as a ready-made option; happy to adjust or step back if you prefer a different shape.