[agentserver-core] Gate resilient TaskManager auto-init behind durable-task opt-in - #48367
[agentserver-core] Gate resilient TaskManager auto-init behind durable-task opt-in#48367Nathandrake229 wants to merge 8 commits into
Conversation
…e-task opt-in AgentServerHost's lifespan unconditionally initialized the resilient TaskManager at startup, issuing a blocking hosted task-store list() round-trip (plus DefaultAzureCredential token acquisition) BEFORE the lifespan yield. This gated server readiness on a network call even for apps that never use the task primitive (e.g. invocations-only hosts), adding tens of seconds of first-connection latency with nothing to recover. Gate auto-init on a non-empty _REGISTERED_DESCRIPTORS registry, which every durable decorator populates: @task and @multi_turn_task both construct a Task whose __init__ registers (MultiTurnTask wraps an inner Task, so it registers too). Declaring a durable task is now the opt-in for standing up the TaskManager and its recovery scan; apps that never opt in skip it entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5222ab61-7d5e-4642-b75f-ef365e375f4f
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Makes resilient TaskManager startup conditional on durable-task registration, avoiding unnecessary startup network work.
Changes:
- Adds registry-based opt-in gating.
- Adds lifecycle and decorator coverage.
- Documents the behavior change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
_base.py |
Gates TaskManager initialization. |
test_task_manager_optin.py |
Tests opt-in and lifecycle behavior. |
CHANGELOG.md |
Records the startup optimization. |
This comment has been minimized.
This comment has been minimized.
Address review: gating eager auto-init on a non-empty descriptor registry broke the supported late-registration path. Task.__init__ pushes decorators declared after startup into an already-live manager, but when the registry was empty at startup no manager existed, so a task declared during the active lifespan failed on first run/start with TaskManagerNotInitialized. Fix with lazy initialization instead of a hard skip: - The host always installs a lazy factory (set_task_manager_factory) that builds a TaskManager from config and loads registered descriptors. - Eager init + recovery scan still run when a durable task is declared by startup (unchanged behavior); otherwise nothing is constructed and the expensive startup path is deferred. - get_task_manager() lazily bootstraps via the factory on first task use, so a task declared mid-lifespan works instead of raising. - set_task_manager(None) also clears the factory; shutdown tears down whichever manager is live (eager or lazily bootstrapped) via a peek helper. - Extract TaskManager.load_registered_descriptors() reused by startup() and the factory. Outside a host lifespan (no factory) get_task_manager() still raises, so existing behavior is preserved. Adds coverage for declaring the first task inside the lifespan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5222ab61-7d5e-4642-b75f-ef365e375f4f
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_base.py:332
- The lazy path never calls
TaskManager.startup(). Loading descriptors alone skips both the initial recovery scan and, importantly, creation of_periodic_recovery_task(tasks/_manager.py:777-804), so a manager bootstrapped by a late task has no recovery loop for the rest of the host lifespan. Please initialize the lazy manager through a lifecycle path that starts periodic recovery before task operations use it; this likely requires an async initialization design rather than the current synchronous factory.
manager.load_registered_descriptors()
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_base.py:398
- Startup explicitly tolerates
ImportErrorwhen the resilient-task module is unavailable, but shutdown now imports the same module outside any guard. In that supported fallback path, lifespan entry succeeds and teardown then raises the same import error. Guard this shutdown import as well so task-less/optional installations can stop cleanly.
from .tasks._manager import ( # pylint: disable=import-outside-toplevel
_peek_task_manager,
set_task_manager as _clear_manager,
)
sdk/agentserver/azure-ai-agentserver-core/tests/tasks/test_task_manager_optin.py:186
- This test calls the internal getter directly, so it does not cover the advertised late-registration behavior that the task's first
run/startsucceeds. It also cannot detect that the lazily created manager was never started. Exercise_late.run(...)or_late.start(...)with a provider-capable manager and assert the manager lifecycle/recovery setup, so the public behavior in the changelog is protected.
# First use resolves the manager — must not raise, and must
# lazily construct + load descriptors.
mgr = get_task_manager()
assert mgr is _fake_task_manager.instances[0]
assert mgr.descriptors_loaded is True
Address review round 2 (suppressed low-confidence findings): - Real bug: the lifespan shutdown path imported .tasks._manager without an ImportError guard, while startup tolerates it. In a task-less/optional install, lifespan entry would succeed but teardown would raise ImportError. Guard the shutdown import the same way (try/except ImportError) so optional installations stop cleanly. Add a test that blocks the module import and asserts both lifespan entry and exit succeed. - Make the late-registration test honest about the intentional limitation: assert the lazily-bootstrapped manager's startup() is NOT called (no initial recovery scan / periodic loop), documenting current behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5222ab61-7d5e-4642-b75f-ef365e375f4f
|
Addressed the three low-confidence findings from the latest review (commit 9282136): 1. Shutdown import not guarded ( 2. Late-registration test used the internal getter — hardened. The test now documents the intentional limitation explicitly: it asserts the lazily-bootstrapped manager's 3. Lazy path skips |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_base.py:336
- The lazy factory is only cleared by the shutdown code after
yield. If the lifespan body exits with an exception,asynccontextmanagerthrows that exception at theyield, so the subsequent shutdown block is skipped and this host-bound factory remains globally installed. A laterget_task_manager()outside the lifespan can then construct a manager from the stale host config instead of raisingTaskManagerNotInitialized, contradicting the stated state-hygiene boundary. Please place the post-yield shutdown/clear logic in afinallyblock and add an exceptional-lifespan test.
set_task_manager_factory(_bootstrap_task_manager)
This comment has been minimized.
This comment has been minimized.
…tory
Replace the registry-only opt-in with a double gate per reviewer feedback
(Shivakishore14: _REGISTERED_DESCRIPTORS alone is a process-global heuristic
prone to false positives). AgentServerHost now stands up the resilient
TaskManager (and its network-backed startup recovery scan) only when BOTH:
1. set_resilient_tasks_enabled(True) was called (new process-global switch,
default False), AND
2. the _REGISTERED_DESCRIPTORS list is non-empty (a durable task declared).
Both signals are read directly at lifespan startup — no lazy factory, no
opted-in abstraction. Adds set_resilient_tasks_enabled / resilient_tasks_enabled
to the tasks public API. Reverts the _manager.py factory machinery from earlier
commits (back to origin/main); the double gate makes it unnecessary.
- New tasks/_enablement.py: process-global switch (default disabled).
- _base.py: gate = _resilient_tasks_enabled() and _has_registered_tasks().
- Tests rewritten for the AND truth table (4 lifespan cases + signal units).
- Updated public-API-surface expected set, tasks-guide.md, api.md, CHANGELOG.
Note: the responses layer (uses tasks internally) must call
set_resilient_tasks_enabled(True) in its host — tracked for PR #47275.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5222ab61-7d5e-4642-b75f-ef365e375f4f
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_base.py:353
- The current skip path still installs no lazy factory, so the supported late-registration path remains broken.
_manager.pyonly has_manager;get_task_manager()raises when it isNone, andTask.__init__only updates an already-live manager. Therefore, with no task at lifespan startup, declaring the first task inside the lifespan still fails onrun/start. Please implement the factory/peek/cleanup path described in the PR and add the promised mid-lifespan bootstrap test.
else:
logger.info(
"TaskManager not initialized (enabled=%s, tasks_declared=%s)",
_resilient_tasks_enabled(),
_has_registered_tasks(),
[Pilot] PR Pipeline Failure AnalysisA CI pipeline failed on this pull request. Here is an automated analysis of what went wrong and how to get the build green. What failedA single test — Recommended next steps
Raw pipeline analysis (azsdk ci analyze)
|
Refine the design so it fixes the startup latency WITHOUT the backward-compat break the reviewer (Copilot) flagged: existing @task apps that don't call the new switch previously would hit TaskManagerNotInitialized on first run/start. Now the TaskManager is CONSTRUCTED unconditionally at lifespan startup — construction is cheap and makes no task-store calls (only in-memory state). This keeps get_task_manager()/.run()/.start() working for existing task apps. The network-backed startup RECOVERY SCAN (the blocking hosted task-store list() + credential-token acquisition that caused the 20-36s stall) runs only when BOTH set_resilient_tasks_enabled(True) was called AND at least one durable task is declared. Plain servers make no task-store call at startup. - _base.py: construct + set_task_manager always; gate await startup() on (enabled AND registered). shutdown() is safe on a never-started manager. - Tests rewritten: manager always constructed + retrievable via get_task_manager(); startup_called only in the enabled+task case. - Fix second public-API guard set (test_contract_completeness) that CI caught; updated tasks-guide.md + CHANGELOG to the always-construct semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5222ab61-7d5e-4642-b75f-ef365e375f4f
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_base.py:320
- The PR description and truth table say that nothing is constructed when either gate is false, but this new comment documents—and the implementation performs—unconditional
TaskManagerconstruction. The changelog and task guide also describe only the recovery scan as gated. Please update the PR description/truth table to match the intended construction-versus-recovery behavior.
# The TaskManager is CONSTRUCTED unconditionally (whenever the
# resilient tasks module is importable). Construction is cheap and
# makes NO task-store calls — it only builds in-memory state (an
# idle provider client, empty routing tables, a lease-owner string).
# This keeps ``get_task_manager()`` working so a ``@task``-based app
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/tasks/_enablement.py:32
- This public docstring says the switch enables or disables the task subsystem, but a false value still leaves the manager installed and
.run()/.start()operational; it disables only automatic startup and periodic recovery. Document that narrower behavior soresilient_tasks_enabled() == Falseis not interpreted as tasks being unavailable.
"""Enable or disable the resilient task subsystem process-wide.
Must be called before ``AgentServerHost`` lifespan startup (typically at
import time) to take effect for eager initialization. Defaults the switch
to enabled when called with no argument.
sdk/agentserver/azure-ai-agentserver-core/CHANGELOG.md:12
- Defaulting this switch to false changes existing
@taskapplications from automatic startup/periodic recovery to no automatic recovery. This entry emphasizes manager availability but does not identify that migration requirement, so upgrading users can silently lose crash recovery. Record the behavior underBreaking Changesand tell existing task apps to callset_resilient_tasks_enabled(True)before host startup.
- `AgentServerHost` no longer makes a blocking hosted task-store `list()` round-trip (plus credential-token acquisition) at startup unless resilient tasks are actually in use. The `TaskManager` is still constructed so `get_task_manager()` keeps working (no `TaskManagerNotInitialized` for existing `@task` apps), but its network-backed **startup recovery scan** now runs only when **both** the subsystem was explicitly enabled via `set_resilient_tasks_enabled(True)` **and** at least one durable task (`@task` / `@multi_turn_task`) has been declared. Plain servers (e.g. invocations-only hosts) no longer pay tens of seconds of startup latency for a scan that has nothing to recover.
Switch the double gate from AND to OR so an existing @task app keeps automatic crash recovery WITHOUT calling the new switch (backward compatible), and the switch becomes a force-enable that starts the recovery loop even before any task is declared — which also closes the recovery gap for tasks registered lazily under an enabled switch (the running periodic loop picks them up dynamically), without touching _manager.py. Recovery scan now runs when: at least one durable task is declared (@task / @multi_turn_task) OR set_resilient_tasks_enabled(True) was set. The TaskManager is still always constructed (no TaskManagerNotInitialized). Deferred: if the switch is off AND no task is declared at startup, a task declared later that lifetime runs but its prior-crash orphans aren't scanned until the next restart (no recovery loop was started). Fully closing this needs a lazy manager-start on first late registration — tracked as future work (the manager can be made lazy, not fully async). Also address review: reword set_resilient_tasks_enabled docstring (it force-enables recovery, does not gate .run()/.start()); add the switch to the guide's standalone examples; add release date; drop the breaking-change framing (OR is backward compatible). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5222ab61-7d5e-4642-b75f-ef365e375f4f
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (5)
sdk/agentserver/azure-ai-agentserver-core/CHANGELOG.md:12
- This release note says a registered task automatically triggers recovery with no opt-in, contradicting the PR's default-off AND gate. Document that recovery runs only when the switch is enabled and at least one task is registered; task applications that do not opt in should also avoid the startup scan.
- `AgentServerHost` no longer makes a blocking hosted task-store `list()` round-trip (plus credential-token acquisition) at startup unless resilient tasks are actually in use. The `TaskManager` is still constructed so `get_task_manager()` and `.run()` / `.start()` keep working, but its network-backed **startup recovery scan** now runs only when **either** at least one durable task (`@task` / `@multi_turn_task`) has been declared **or** `set_resilient_tasks_enabled(True)` was called. Apps that use tasks keep automatic crash recovery with no code change; plain servers (e.g. invocations-only hosts) no longer pay tens of seconds of startup latency for a scan that has nothing to recover.
sdk/agentserver/azure-ai-agentserver-core/tests/tasks/test_task_manager_optin.py:139
- This test class codifies OR semantics, and its cases below assert recovery for each single-gate state. The stated contract is an AND gate: startup recovery must be skipped unless both the switch and a registered task are present. Update the two single-signal cases to expect
startup_called is False.
class TestLifespanManagerAndRecovery:
"""The TaskManager is ALWAYS constructed (cheap, no task-store calls);
the network-backed startup recovery scan runs when EITHER the switch is
enabled OR at least one durable task is declared."""
sdk/agentserver/azure-ai-agentserver-core/docs/tasks-guide.md:124
- The guide documents OR/automatic-recovery semantics, but the PR promises an explicit double-gated opt-in where a declared task alone does not trigger recovery. Update this section and its examples so they require both
set_resilient_tasks_enabled(True)and a registered task.
The resilient `TaskManager`'s **startup recovery scan** — a network round-trip
to the hosted task store that reclaims tasks left in-flight by a crashed prior
instance — runs at startup when **either** of the following holds:
1. at least one durable task has been declared (`@task` / `@multi_turn_task`) —
an app that uses tasks gets recovery automatically, **or**
2. it was explicitly force-enabled via `set_resilient_tasks_enabled(True)`.
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/tasks/_enablement.py:17
- These module docs define the switch as a force-enable under OR semantics. Under the advertised double gate, enabling the switch without any registered task must not start recovery, and a task without the switch must remain disabled. Revise this module and both public function docstrings to describe the AND contract.
That recovery runs at startup when EITHER a durable task has been declared
(``@task`` / ``@multi_turn_task``) OR this switch is on. So an app that uses
tasks gets recovery automatically; this switch is a **force-enable** that
starts the recovery loop even before any task is declared (useful when tasks
are registered lazily after startup — the running loop then picks them up).
sdk/agentserver/azure-ai-agentserver-core/CHANGELOG.md:8
- The new API is described as force-enabling recovery before any task exists, which conflicts with the stated AND gate. Describe it as the explicit opt-in that enables startup recovery only when a durable task is also registered.
This issue also appears on line 12 of the same file.
- Added `set_resilient_tasks_enabled` / `resilient_tasks_enabled` to `azure.ai.agentserver.core.tasks` — a process-global switch (default off) that force-enables the resilient `TaskManager`'s startup recovery scan even before any durable task is declared (useful when tasks are registered lazily after startup).
The test module docstring still described the earlier AND gate; update it to the OR contract (recovery runs when a task is declared OR the switch is set) to match the implementation, tests, docs, and CHANGELOG. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5222ab61-7d5e-4642-b75f-ef365e375f4f
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/_base.py:322
- This claims an eagerly declared
@taskapp pays no network cost until the task is used, but that declaration makes_has_registered_tasks()true andstartup()performs the recovery-store request before the lifespan yields. Please distinguish cheap construction from the conditional recovery scan.
# This keeps ``get_task_manager()`` working so a ``@task``-based app
# can run tasks without hitting ``TaskManagerNotInitialized``,
# while paying zero network cost until a task is actually used.
sdk/agentserver/azure-ai-agentserver-core/tests/tasks/test_task_manager_optin.py:134
- This heading still says
AND, but the tested and documented contract is theORgate. Rename it to avoid contradicting the truth-table tests below.
# The AND gate at lifespan startup
sdk/agentserver/azure-ai-agentserver-core/azure/ai/agentserver/core/tasks/_enablement.py:10
TaskManagerdoes make task-store calls whenstartup()is selected by either gate signal, even if no task has been invoked. Clarify that only construction is network-free; otherwise this module-level API documentation contradicts the behavior described immediately below.
The resilient ``TaskManager`` is always constructed by ``AgentServerHost`` (a
cheap, in-memory object that makes no task-store calls), so ``get_task_manager``
and ``.run()`` / ``.start()`` work regardless of this switch. What this switch
affects is only the network-backed **startup recovery scan** (a hosted
task-store ``list()`` plus credential-token acquisition) and the periodic
sdk/agentserver/azure-ai-agentserver-core/docs/tasks-guide.md:140
- The manager can make a task-store call before any task is used: either a declared task or the force-enable switch causes the startup recovery scan. Rephrase this as a property of construction so the latency contract is accurate.
The `TaskManager` itself is always constructed (a cheap, in-memory object that
makes no task-store calls until a task is used), so `get_task_manager()` and
`.run()` / `.start()` work regardless of the switch. A server that neither
sdk/agentserver/azure-ai-agentserver-core/tests/tasks/test_task_manager_optin.py:8
- As written, this says the manager object never makes task-store calls, although its gated
startup()does. Describe construction—not the manager generally—as network-free to match the behavior under test.
``AgentServerHost`` always constructs the resilient ``TaskManager`` (a cheap,
in-memory object that makes no task-store calls), so ``get_task_manager()`` and
``.run()`` / ``.start()`` work regardless. Its network-backed **startup
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
sdk/agentserver/azure-ai-agentserver-core/tests/tasks/test_task_manager_optin.py:134
- This heading still describes an AND gate, but the implementation and truth-table tests use the intentional OR contract. Rename it to avoid documenting the obsolete behavior.
# The AND gate at lifespan startup
Summary
AgentServerHost's lifespan unconditionally ran the resilientTaskManager's startup recovery scan — a blocking hosted task-storelist()round-trip (plusDefaultAzureCredentialtoken acquisition) before the lifespanyield. This gated server readiness on a network call even for apps that never use tasks (e.g. invocations-only hosts), adding tens of seconds of first-connection latency with nothing to recover.Design
The
TaskManageris always constructed at lifespan startup — construction is cheap and makes no task-store calls (only in-memory state: an idle provider client, empty routing tables, a lease-owner string). Soget_task_manager()and.run()/.start()keep working for every task app — noTaskManagerNotInitialized, fully backward compatible.Only the network-backed startup recovery scan (
await task_manager.startup()→ hosted task-storelist()+ token acquisition, plus the periodic recovery loop it spawns) is gated. It runs when either:@task/@multi_turn_task, via_REGISTERED_DESCRIPTORS) — an app that uses tasks gets recovery automatically, orset_resilient_tasks_enabled(True)was called — a force-enable that starts the recovery loop even before any task is declared.Truth table (recovery scan runs?)
set_resilient_tasks_enabled(True)@taskapp → recovery automatic, no code changeThe
TaskManageris constructed in all rows; only the scan differs.Why
OR(notAND)@taskapp keeps automatic crash recovery without calling the new switch (row 2).ANDwould have silently disabled recovery for such apps unless they opted in.startup()still runs, starting the periodic recovery loop. A task declared later in that lifetime is then recovered dynamically (its callback is added to the live_resume_callbacksand the next periodic scan reclaims its orphans) — without touching_manager.py.Public API
New in
azure.ai.agentserver.core.tasks:set_resilient_tasks_enabled(value: bool = True) -> None— force-enable the recovery scan (does not gate.run()/.start()).resilient_tasks_enabled() -> bool.Deferred (future work)
If the switch is off and no task is declared at startup, the recovery loop is not started, so a task declared later that same lifetime will run but its prior-crash orphans are not scanned until the next restart. Fully closing this requires a lazy manager-start on first late registration (the manager can be made lazily started, not fully async). Tracked as follow-up; not in scope here.
_manager.pyis unchanged by this PR.Consumer note
The responses package uses tasks internally and declares its
@taskeagerly in its host, so it gets recovery automatically under theORgate — no change required there.Tests / validation
tests/tasks/test_task_manager_optin.pycovers the fullORtruth table over the real host lifespan, plus the switch unit behavior and the "manager always constructed /get_task_manager()works with no opt-in" case. Validated on Python 3.12: full core suite 941 passed / 61 skipped / 0 failed;pylint10.00/10 on_base.py+_enablement.py;mypyclean; public-API-surface + contract-completeness + dev-guide drift tests updated;api.mdregenerated; cspell clean._manager.pyunchanged (0 lines vsmain).