Skip to content

fix(tasks): preserve terminal states against late watcher callbacks - #1007

Open
Jackkp0t wants to merge 1 commit into
modelscope:mainfrom
Jackkp0t:fix/task-manager-preserve-terminal-state
Open

Jackkp0t wants to merge 1 commit into
modelscope:mainfrom
Jackkp0t:fix/task-manager-preserve-terminal-state

Conversation

@Jackkp0t

Copy link
Copy Markdown

Late background watchers can overwrite a cancelled task as completed or failed, and duplicate callbacks can replace a finished task's result and enqueue another notification. Allow complete() and fail() to transition only running tasks, preserving the first terminal state and its timestamp/result/error.

Adds regression coverage for both watcher callbacks after cancellation and all completed/failed callback combinations. Existing normal completion/failure paths remain covered.

Validation (Python 3.12.13, Windows, upstream a56afcc):

  • Before the fix, the new regression cases fail on terminal state/result changes (6 failures including subtests).
  • python -m pytest tests/utils/test_task_manager_smoke.py -k TestTaskManager -q: 12 passed, 4 subtests passed.
  • Expanded TaskManager, local shell, and Windows executor suites: 34 passed, 4 subtests passed, 2 failed. Both failures also occur on the untouched upstream files: TestAgentToolTranscript.test_save_transcript_writes_message_objects_and_dicts and test_plugin_tools_are_available_without_an_explicit_shell_env.
  • pytest tests -q --maxfail=1 cannot collect on Windows: tests/release/test_business_distribution.py imports Unix-only fcntl.
  • Changed production file passes flake8, isort and YAPF checks; git diff --check passes.

This preserves task state only. It does not change process termination or process-group handling (the latter is covered separately in #962).

AI assistance: Codex implemented the change, ran the offline regressions and reviewed the diff. No live model calls or credentials were used in the tests.

This branch has not been deployed

No deployments
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