From a01d1c0e3824c8e4ec32d86b41eb16c6451fc9a2 Mon Sep 17 00:00:00 2001 From: bboonnee Date: Thu, 13 Aug 2026 11:16:51 +0800 Subject: [PATCH] FR-1: new-Goal auto-dispatch (queued tasks no longer need a manual Start click) --- src/backend_server.py | 44 +++++++++++++++++++++++++++++++++++++++++++- src/new_goal_dispatch.py | 2 +- src/task_reconciliation.py | 2 +- src/test_backend_server.py | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- src/test_new_goal_dispatch.py | 2 +- supervisor/deploy_stage.py | 9 +++++++-- supervisor/test_deploy_stage.py | 9 +++++++++ 7 files changed, 110 insertions(+), 7 deletions(-) diff --git a/src/backend_server.py b/src/backend_server.py index c770636..ff69462 100644 --- a/src/backend_server.py +++ b/src/backend_server.py @@ -40,7 +40,7 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from src import ( artifact_query, budget_guardrail, bug_record, deployment_query, execution_log, - org_loop_route, paths, project_query, project_scaffold, schema, settings_config, + new_goal_dispatch, org_loop_route, paths, project_query, project_scaffold, schema, settings_config, skill_query, skill_update, task_create, task_execution, task_query, task_reconciliation, work_graph_fork, work_graph_run_query, ws_subscribe, ) @@ -1122,6 +1122,41 @@ def _start_reconciliation_loop(db_path, interval_seconds): return thread +# 45s (2026-08-13, docs/done/scheduler-new-goal-dispatch-requirements.md FR-1): +# deliberately far below _DEFAULT_RECONCILIATION_INTERVAL_SECONDS -- the +# acceptance bar is a brand-new Goal starting without a human clicking Start, +# and the requirements doc's own framing ("接近实时") rules out reusing the +# 300s figure, which was tuned for "eventually recover a stuck task", not +# "don't make someone wait a Pomodoro to see their new Goal start". Not +# sub-10s either: this is a full-table scan (same "no index" simplification +# task_reconciliation.py already documents) plus a DB connect/close per pass. +_DEFAULT_NEW_GOAL_DISPATCH_INTERVAL_SECONDS = 45 + + +def _start_new_goal_dispatch_loop(db_path, interval_seconds): + """Sibling of `_start_reconciliation_loop` for + `new_goal_dispatch.dispatch_once()` -- same "own daemon thread, only + started from `main()`, run immediately then repeat, never let one bad + pass kill the loop, traceback to stderr instead of a silent `except: + pass`" shape, kept as a genuinely separate loop/thread rather than + folded into the reconciliation one: `docs/done/ + scheduler-task-reconciliation-design.md` §7 records that decision -- + new-Goal dispatch and stuck-task recovery are independent concerns with + different urgency, and merging them into one "万能调度循环" was + deliberately rejected.""" + def loop(): + while True: + try: + new_goal_dispatch.dispatch_once(db_path) + except Exception: + traceback.print_exc() + time.sleep(interval_seconds) + + thread = threading.Thread(target=loop, daemon=True) + thread.start() + return thread + + def main(): """U-037: CLI entry point (`python3 -m src.backend_server ...`) -- until this Unit, `create_server()` was only ever instantiated in-process by @@ -1141,6 +1176,10 @@ def main(): parser.add_argument("--skill-repo-path", default=None) parser.add_argument("--reconciliation-interval-seconds", type=int, default=_DEFAULT_RECONCILIATION_INTERVAL_SECONDS) parser.add_argument("--disable-reconciliation-loop", action="store_true") + parser.add_argument( + "--new-goal-dispatch-interval-seconds", type=int, default=_DEFAULT_NEW_GOAL_DISPATCH_INTERVAL_SECONDS + ) + parser.add_argument("--disable-new-goal-dispatch-loop", action="store_true") args = parser.parse_args() server = create_server(args.host, args.port, args.db_path, args.skill_repo_path) @@ -1148,6 +1187,9 @@ def main(): if not args.disable_reconciliation_loop: _start_reconciliation_loop(server.db_path, args.reconciliation_interval_seconds) + if not args.disable_new_goal_dispatch_loop: + _start_new_goal_dispatch_loop(server.db_path, args.new_goal_dispatch_interval_seconds) + def _handle_sigterm(signum, frame): # server.shutdown() blocks until serve_forever()'s loop notices and # returns -- calling it from the SAME thread that's running diff --git a/src/new_goal_dispatch.py b/src/new_goal_dispatch.py index 1dfcf5f..0fcce8f 100644 --- a/src/new_goal_dispatch.py +++ b/src/new_goal_dispatch.py @@ -1,5 +1,5 @@ """Periodic auto-dispatch for new Goals that were created but never -started (2026-08-13, `docs/todo/scheduler-new-goal-dispatch-requirements.md` +started (2026-08-13, `docs/done/scheduler-new-goal-dispatch-requirements.md` FR-1). `scheduler.run_scheduler_once()` exists and works, but nothing in production ever called it: a `task.status='queued'` row (created via the API/console, never given to `run_goal_to_completion`) sat there until a diff --git a/src/task_reconciliation.py b/src/task_reconciliation.py index 2847b12..a26de59 100644 --- a/src/task_reconciliation.py +++ b/src/task_reconciliation.py @@ -8,7 +8,7 @@ Scope: `docs/done/scheduler-task-reconciliation-requirements.md` (in-flight task reconciliation) -- deliberately NOT new-goal auto-dispatch via `scheduler.run_scheduler_once()`, which is a separate, larger, deliberately-not-bundled decision tracked in -`docs/todo/scheduler-new-goal-dispatch-requirements.md` (see that doc's +`docs/done/scheduler-new-goal-dispatch-requirements.md` (see that doc's non-goals/risks: "改变系统自主权边界"). This module only ever resumes a task that already exists and was already in progress; it never creates a new task or starts work nobody asked for. diff --git a/src/test_backend_server.py b/src/test_backend_server.py index 156b2a0..b17baee 100644 --- a/src/test_backend_server.py +++ b/src/test_backend_server.py @@ -19,7 +19,7 @@ import threading import time import unittest import urllib.parse -from unittest.mock import patch +from unittest.mock import MagicMock, patch from src import backend_server, execution_log, skill_update, ws_subscribe @@ -1850,5 +1850,52 @@ class ZeroThirdPartyDependencyTest(unittest.TestCase): self.assertEqual(non_stdlib, set(), "non-stdlib imports found: {}".format(non_stdlib)) +class MainCliFlagsTest(unittest.TestCase): + """2026-08-13 (new_goal_dispatch, FR-1): `main()` itself has no prior + test coverage of its argparse wiring at all (the reconciliation loop's + flags only ever got exercised indirectly, via `LaunchCandidateArgsTest` + asserting the flag on a `subprocess.Popen` argv, not via `main()` + parsing it) -- this covers the new flags at the point they're actually + parsed and wired to the two background loops, mocking everything below + `create_server()` so no real server/thread/subprocess is involved.""" + + def _run_main_with_argv(self, argv): + fake_server = MagicMock() + fake_server.db_path = "/some/db.sqlite" + fake_server.server_address = ("127.0.0.1", 0) + fake_server.serve_forever.side_effect = KeyboardInterrupt + with patch.object(sys, "argv", ["backend_server"] + argv), \ + patch.object(backend_server, "create_server", return_value=fake_server), \ + patch.object(backend_server, "_start_reconciliation_loop") as mock_reconciliation, \ + patch.object(backend_server, "_start_new_goal_dispatch_loop") as mock_new_goal_dispatch, \ + patch("signal.signal"): + # main() does `import signal` locally, not at module level -- + # patching the real `signal` module's `.signal` attribute (not + # `backend_server.signal`, which doesn't exist as a module + # attribute) still takes effect since `import signal` just binds + # the same already-imported module object. + backend_server.main() + return mock_reconciliation, mock_new_goal_dispatch + + def test_new_goal_dispatch_loop_started_with_default_interval(self): + _, mock_new_goal_dispatch = self._run_main_with_argv([]) + mock_new_goal_dispatch.assert_called_once_with( + "/some/db.sqlite", backend_server._DEFAULT_NEW_GOAL_DISPATCH_INTERVAL_SECONDS + ) + + def test_new_goal_dispatch_interval_flag_overrides_default(self): + _, mock_new_goal_dispatch = self._run_main_with_argv(["--new-goal-dispatch-interval-seconds", "10"]) + mock_new_goal_dispatch.assert_called_once_with("/some/db.sqlite", 10) + + def test_disable_new_goal_dispatch_loop_flag_skips_loop(self): + _, mock_new_goal_dispatch = self._run_main_with_argv(["--disable-new-goal-dispatch-loop"]) + mock_new_goal_dispatch.assert_not_called() + + def test_disabling_new_goal_dispatch_loop_does_not_affect_reconciliation_loop(self): + mock_reconciliation, mock_new_goal_dispatch = self._run_main_with_argv(["--disable-new-goal-dispatch-loop"]) + mock_reconciliation.assert_called_once() + mock_new_goal_dispatch.assert_not_called() + + if __name__ == "__main__": unittest.main() diff --git a/src/test_new_goal_dispatch.py b/src/test_new_goal_dispatch.py index 5613b57..b67eb71 100644 --- a/src/test_new_goal_dispatch.py +++ b/src/test_new_goal_dispatch.py @@ -1,5 +1,5 @@ """new_goal_dispatch.py tests (2026-08-13, FR-1 -- -docs/todo/scheduler-new-goal-dispatch-requirements.md): same mocked-at-the- +docs/done/scheduler-new-goal-dispatch-requirements.md): same mocked-at-the- subprocess-boundary / real-sqlite split as test_task_reconciliation.py. Run: python3 -m unittest src.test_new_goal_dispatch -v diff --git a/supervisor/deploy_stage.py b/supervisor/deploy_stage.py index 661cfa8..9c1803a 100644 --- a/supervisor/deploy_stage.py +++ b/supervisor/deploy_stage.py @@ -129,12 +129,17 @@ def _launch_candidate(candidate_dir, port, db_path, python_executable=None): process, before Deploy Supervisor has even decided whether this candidate is good. Found for real: this exact scenario made candidate health checks fail/hang in `supervisor`'s own test suite the first time - the reconciliation loop shipped with no guard here.""" + the reconciliation loop shipped with no guard here. + + `--disable-new-goal-dispatch-loop` (2026-08-13, new_goal_dispatch): same + risk, same fix, for the sibling loop that auto-starts `status='queued'` + tasks -- the copied db can contain those too, and a candidate must stay + passive under health-check until Deploy Supervisor decides it's live.""" python_executable = python_executable or sys.executable return subprocess.Popen( [ python_executable, "-m", "src.backend_server", "--host", "127.0.0.1", "--port", str(port), - "--db-path", db_path, "--disable-reconciliation-loop", + "--db-path", db_path, "--disable-reconciliation-loop", "--disable-new-goal-dispatch-loop", ], cwd=candidate_dir, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, ) diff --git a/supervisor/test_deploy_stage.py b/supervisor/test_deploy_stage.py index da5ee71..57ed7f6 100644 --- a/supervisor/test_deploy_stage.py +++ b/supervisor/test_deploy_stage.py @@ -214,6 +214,15 @@ class LaunchCandidateArgsTest(unittest.TestCase): argv = mock_popen.call_args[0][0] self.assertIn("--disable-reconciliation-loop", argv) + def test_disable_new_goal_dispatch_loop_flag_present(self): + # 2026-08-13 (new_goal_dispatch): same risk as the reconciliation + # flag above -- the copied candidate db can contain status='queued' + # tasks too. + with patch.object(deploy_stage.subprocess, "Popen") as mock_popen: + deploy_stage._launch_candidate("/some/dir", 12345, "/some/db.sqlite") + argv = mock_popen.call_args[0][0] + self.assertIn("--disable-new-goal-dispatch-loop", argv) + class RealCandidateLaunchTest(unittest.TestCase): """Not mocked -- stages a real candidate checkout of graphlemon's OWN -- libgit2 0.21.4