Commit a01d1c0e3824c8e4ec32d86b41eb16c6451fc9a2
1 parent
b1e0a879
FR-1: new-Goal auto-dispatch (queued tasks no longer need a manual Start click)
New src/new_goal_dispatch.py (find_queued_tasks() + dispatch_once(), mirroring task_reconciliation.py's shape but deliberately a separate module/loop -- scheduler-task-reconciliation-design.md §7 already established the "don't merge into one mega scheduler loop" principle, this follows it). Wired into backend_server.py::main() as its own daemon thread with --new-goal-dispatch-interval-seconds (default 45s, shorter than reconciliation's 300s to satisfy the near-real-time acceptance bar) and --disable-new-goal-dispatch-loop. Also added the matching --disable-new-goal-dispatch-loop flag to supervisor/deploy_stage.py::_launch_candidate() -- same reasoning as the existing --disable-reconciliation-loop flag there: a health-checking deploy candidate runs against a copy of the production DB and must stay passive until Deploy Supervisor decides it's live, otherwise it would start autonomously dispatching real Goals off stale/copied state. User-confirmed 2026-08-13: full auto-dispatch, not a conservative default-disabled rollout -- this was previously flagged as a standalone "changes the system's own autonomy boundary" decision requiring separate review (docs/done/scheduler-new-goal-dispatch-requirements.md), now resolved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
7 changed files
with
110 additions
and
7 deletions
src/backend_server.py
| ... | ... | @@ -40,7 +40,7 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer |
| 40 | 40 | |
| 41 | 41 | from src import ( |
| 42 | 42 | artifact_query, budget_guardrail, bug_record, deployment_query, execution_log, |
| 43 | - org_loop_route, paths, project_query, project_scaffold, schema, settings_config, | |
| 43 | + new_goal_dispatch, org_loop_route, paths, project_query, project_scaffold, schema, settings_config, | |
| 44 | 44 | skill_query, skill_update, task_create, task_execution, task_query, task_reconciliation, |
| 45 | 45 | work_graph_fork, work_graph_run_query, ws_subscribe, |
| 46 | 46 | ) |
| ... | ... | @@ -1122,6 +1122,41 @@ def _start_reconciliation_loop(db_path, interval_seconds): |
| 1122 | 1122 | return thread |
| 1123 | 1123 | |
| 1124 | 1124 | |
| 1125 | +# 45s (2026-08-13, docs/done/scheduler-new-goal-dispatch-requirements.md FR-1): | |
| 1126 | +# deliberately far below _DEFAULT_RECONCILIATION_INTERVAL_SECONDS -- the | |
| 1127 | +# acceptance bar is a brand-new Goal starting without a human clicking Start, | |
| 1128 | +# and the requirements doc's own framing ("接近实时") rules out reusing the | |
| 1129 | +# 300s figure, which was tuned for "eventually recover a stuck task", not | |
| 1130 | +# "don't make someone wait a Pomodoro to see their new Goal start". Not | |
| 1131 | +# sub-10s either: this is a full-table scan (same "no index" simplification | |
| 1132 | +# task_reconciliation.py already documents) plus a DB connect/close per pass. | |
| 1133 | +_DEFAULT_NEW_GOAL_DISPATCH_INTERVAL_SECONDS = 45 | |
| 1134 | + | |
| 1135 | + | |
| 1136 | +def _start_new_goal_dispatch_loop(db_path, interval_seconds): | |
| 1137 | + """Sibling of `_start_reconciliation_loop` for | |
| 1138 | + `new_goal_dispatch.dispatch_once()` -- same "own daemon thread, only | |
| 1139 | + started from `main()`, run immediately then repeat, never let one bad | |
| 1140 | + pass kill the loop, traceback to stderr instead of a silent `except: | |
| 1141 | + pass`" shape, kept as a genuinely separate loop/thread rather than | |
| 1142 | + folded into the reconciliation one: `docs/done/ | |
| 1143 | + scheduler-task-reconciliation-design.md` §7 records that decision -- | |
| 1144 | + new-Goal dispatch and stuck-task recovery are independent concerns with | |
| 1145 | + different urgency, and merging them into one "万能调度循环" was | |
| 1146 | + deliberately rejected.""" | |
| 1147 | + def loop(): | |
| 1148 | + while True: | |
| 1149 | + try: | |
| 1150 | + new_goal_dispatch.dispatch_once(db_path) | |
| 1151 | + except Exception: | |
| 1152 | + traceback.print_exc() | |
| 1153 | + time.sleep(interval_seconds) | |
| 1154 | + | |
| 1155 | + thread = threading.Thread(target=loop, daemon=True) | |
| 1156 | + thread.start() | |
| 1157 | + return thread | |
| 1158 | + | |
| 1159 | + | |
| 1125 | 1160 | def main(): |
| 1126 | 1161 | """U-037: CLI entry point (`python3 -m src.backend_server ...`) -- until |
| 1127 | 1162 | this Unit, `create_server()` was only ever instantiated in-process by |
| ... | ... | @@ -1141,6 +1176,10 @@ def main(): |
| 1141 | 1176 | parser.add_argument("--skill-repo-path", default=None) |
| 1142 | 1177 | parser.add_argument("--reconciliation-interval-seconds", type=int, default=_DEFAULT_RECONCILIATION_INTERVAL_SECONDS) |
| 1143 | 1178 | parser.add_argument("--disable-reconciliation-loop", action="store_true") |
| 1179 | + parser.add_argument( | |
| 1180 | + "--new-goal-dispatch-interval-seconds", type=int, default=_DEFAULT_NEW_GOAL_DISPATCH_INTERVAL_SECONDS | |
| 1181 | + ) | |
| 1182 | + parser.add_argument("--disable-new-goal-dispatch-loop", action="store_true") | |
| 1144 | 1183 | args = parser.parse_args() |
| 1145 | 1184 | |
| 1146 | 1185 | server = create_server(args.host, args.port, args.db_path, args.skill_repo_path) |
| ... | ... | @@ -1148,6 +1187,9 @@ def main(): |
| 1148 | 1187 | if not args.disable_reconciliation_loop: |
| 1149 | 1188 | _start_reconciliation_loop(server.db_path, args.reconciliation_interval_seconds) |
| 1150 | 1189 | |
| 1190 | + if not args.disable_new_goal_dispatch_loop: | |
| 1191 | + _start_new_goal_dispatch_loop(server.db_path, args.new_goal_dispatch_interval_seconds) | |
| 1192 | + | |
| 1151 | 1193 | def _handle_sigterm(signum, frame): |
| 1152 | 1194 | # server.shutdown() blocks until serve_forever()'s loop notices and |
| 1153 | 1195 | # returns -- calling it from the SAME thread that's running | ... | ... |
src/new_goal_dispatch.py
| 1 | 1 | """Periodic auto-dispatch for new Goals that were created but never |
| 2 | -started (2026-08-13, `docs/todo/scheduler-new-goal-dispatch-requirements.md` | |
| 2 | +started (2026-08-13, `docs/done/scheduler-new-goal-dispatch-requirements.md` | |
| 3 | 3 | FR-1). `scheduler.run_scheduler_once()` exists and works, but nothing in |
| 4 | 4 | production ever called it: a `task.status='queued'` row (created via the |
| 5 | 5 | API/console, never given to `run_goal_to_completion`) sat there until a | ... | ... |
src/task_reconciliation.py
| ... | ... | @@ -8,7 +8,7 @@ Scope: `docs/done/scheduler-task-reconciliation-requirements.md` (in-flight |
| 8 | 8 | task reconciliation) -- deliberately NOT new-goal auto-dispatch via |
| 9 | 9 | `scheduler.run_scheduler_once()`, which is a separate, larger, |
| 10 | 10 | deliberately-not-bundled decision tracked in |
| 11 | -`docs/todo/scheduler-new-goal-dispatch-requirements.md` (see that doc's | |
| 11 | +`docs/done/scheduler-new-goal-dispatch-requirements.md` (see that doc's | |
| 12 | 12 | non-goals/risks: "改变系统自主权边界"). This module only ever resumes a |
| 13 | 13 | task that already exists and was already in progress; it never creates a |
| 14 | 14 | new task or starts work nobody asked for. | ... | ... |
src/test_backend_server.py
| ... | ... | @@ -19,7 +19,7 @@ import threading |
| 19 | 19 | import time |
| 20 | 20 | import unittest |
| 21 | 21 | import urllib.parse |
| 22 | -from unittest.mock import patch | |
| 22 | +from unittest.mock import MagicMock, patch | |
| 23 | 23 | |
| 24 | 24 | from src import backend_server, execution_log, skill_update, ws_subscribe |
| 25 | 25 | |
| ... | ... | @@ -1850,5 +1850,52 @@ class ZeroThirdPartyDependencyTest(unittest.TestCase): |
| 1850 | 1850 | self.assertEqual(non_stdlib, set(), "non-stdlib imports found: {}".format(non_stdlib)) |
| 1851 | 1851 | |
| 1852 | 1852 | |
| 1853 | +class MainCliFlagsTest(unittest.TestCase): | |
| 1854 | + """2026-08-13 (new_goal_dispatch, FR-1): `main()` itself has no prior | |
| 1855 | + test coverage of its argparse wiring at all (the reconciliation loop's | |
| 1856 | + flags only ever got exercised indirectly, via `LaunchCandidateArgsTest` | |
| 1857 | + asserting the flag on a `subprocess.Popen` argv, not via `main()` | |
| 1858 | + parsing it) -- this covers the new flags at the point they're actually | |
| 1859 | + parsed and wired to the two background loops, mocking everything below | |
| 1860 | + `create_server()` so no real server/thread/subprocess is involved.""" | |
| 1861 | + | |
| 1862 | + def _run_main_with_argv(self, argv): | |
| 1863 | + fake_server = MagicMock() | |
| 1864 | + fake_server.db_path = "/some/db.sqlite" | |
| 1865 | + fake_server.server_address = ("127.0.0.1", 0) | |
| 1866 | + fake_server.serve_forever.side_effect = KeyboardInterrupt | |
| 1867 | + with patch.object(sys, "argv", ["backend_server"] + argv), \ | |
| 1868 | + patch.object(backend_server, "create_server", return_value=fake_server), \ | |
| 1869 | + patch.object(backend_server, "_start_reconciliation_loop") as mock_reconciliation, \ | |
| 1870 | + patch.object(backend_server, "_start_new_goal_dispatch_loop") as mock_new_goal_dispatch, \ | |
| 1871 | + patch("signal.signal"): | |
| 1872 | + # main() does `import signal` locally, not at module level -- | |
| 1873 | + # patching the real `signal` module's `.signal` attribute (not | |
| 1874 | + # `backend_server.signal`, which doesn't exist as a module | |
| 1875 | + # attribute) still takes effect since `import signal` just binds | |
| 1876 | + # the same already-imported module object. | |
| 1877 | + backend_server.main() | |
| 1878 | + return mock_reconciliation, mock_new_goal_dispatch | |
| 1879 | + | |
| 1880 | + def test_new_goal_dispatch_loop_started_with_default_interval(self): | |
| 1881 | + _, mock_new_goal_dispatch = self._run_main_with_argv([]) | |
| 1882 | + mock_new_goal_dispatch.assert_called_once_with( | |
| 1883 | + "/some/db.sqlite", backend_server._DEFAULT_NEW_GOAL_DISPATCH_INTERVAL_SECONDS | |
| 1884 | + ) | |
| 1885 | + | |
| 1886 | + def test_new_goal_dispatch_interval_flag_overrides_default(self): | |
| 1887 | + _, mock_new_goal_dispatch = self._run_main_with_argv(["--new-goal-dispatch-interval-seconds", "10"]) | |
| 1888 | + mock_new_goal_dispatch.assert_called_once_with("/some/db.sqlite", 10) | |
| 1889 | + | |
| 1890 | + def test_disable_new_goal_dispatch_loop_flag_skips_loop(self): | |
| 1891 | + _, mock_new_goal_dispatch = self._run_main_with_argv(["--disable-new-goal-dispatch-loop"]) | |
| 1892 | + mock_new_goal_dispatch.assert_not_called() | |
| 1893 | + | |
| 1894 | + def test_disabling_new_goal_dispatch_loop_does_not_affect_reconciliation_loop(self): | |
| 1895 | + mock_reconciliation, mock_new_goal_dispatch = self._run_main_with_argv(["--disable-new-goal-dispatch-loop"]) | |
| 1896 | + mock_reconciliation.assert_called_once() | |
| 1897 | + mock_new_goal_dispatch.assert_not_called() | |
| 1898 | + | |
| 1899 | + | |
| 1853 | 1900 | if __name__ == "__main__": |
| 1854 | 1901 | unittest.main() | ... | ... |
src/test_new_goal_dispatch.py
| 1 | 1 | """new_goal_dispatch.py tests (2026-08-13, FR-1 -- |
| 2 | -docs/todo/scheduler-new-goal-dispatch-requirements.md): same mocked-at-the- | |
| 2 | +docs/done/scheduler-new-goal-dispatch-requirements.md): same mocked-at-the- | |
| 3 | 3 | subprocess-boundary / real-sqlite split as test_task_reconciliation.py. |
| 4 | 4 | |
| 5 | 5 | Run: python3 -m unittest src.test_new_goal_dispatch -v | ... | ... |
supervisor/deploy_stage.py
| ... | ... | @@ -129,12 +129,17 @@ def _launch_candidate(candidate_dir, port, db_path, python_executable=None): |
| 129 | 129 | process, before Deploy Supervisor has even decided whether this |
| 130 | 130 | candidate is good. Found for real: this exact scenario made candidate |
| 131 | 131 | health checks fail/hang in `supervisor`'s own test suite the first time |
| 132 | - the reconciliation loop shipped with no guard here.""" | |
| 132 | + the reconciliation loop shipped with no guard here. | |
| 133 | + | |
| 134 | + `--disable-new-goal-dispatch-loop` (2026-08-13, new_goal_dispatch): same | |
| 135 | + risk, same fix, for the sibling loop that auto-starts `status='queued'` | |
| 136 | + tasks -- the copied db can contain those too, and a candidate must stay | |
| 137 | + passive under health-check until Deploy Supervisor decides it's live.""" | |
| 133 | 138 | python_executable = python_executable or sys.executable |
| 134 | 139 | return subprocess.Popen( |
| 135 | 140 | [ |
| 136 | 141 | python_executable, "-m", "src.backend_server", "--host", "127.0.0.1", "--port", str(port), |
| 137 | - "--db-path", db_path, "--disable-reconciliation-loop", | |
| 142 | + "--db-path", db_path, "--disable-reconciliation-loop", "--disable-new-goal-dispatch-loop", | |
| 138 | 143 | ], |
| 139 | 144 | cwd=candidate_dir, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, |
| 140 | 145 | ) | ... | ... |
supervisor/test_deploy_stage.py
| ... | ... | @@ -214,6 +214,15 @@ class LaunchCandidateArgsTest(unittest.TestCase): |
| 214 | 214 | argv = mock_popen.call_args[0][0] |
| 215 | 215 | self.assertIn("--disable-reconciliation-loop", argv) |
| 216 | 216 | |
| 217 | + def test_disable_new_goal_dispatch_loop_flag_present(self): | |
| 218 | + # 2026-08-13 (new_goal_dispatch): same risk as the reconciliation | |
| 219 | + # flag above -- the copied candidate db can contain status='queued' | |
| 220 | + # tasks too. | |
| 221 | + with patch.object(deploy_stage.subprocess, "Popen") as mock_popen: | |
| 222 | + deploy_stage._launch_candidate("/some/dir", 12345, "/some/db.sqlite") | |
| 223 | + argv = mock_popen.call_args[0][0] | |
| 224 | + self.assertIn("--disable-new-goal-dispatch-loop", argv) | |
| 225 | + | |
| 217 | 226 | |
| 218 | 227 | class RealCandidateLaunchTest(unittest.TestCase): |
| 219 | 228 | """Not mocked -- stages a real candidate checkout of graphlemon's OWN | ... | ... |